From 598ed35e4a510bf45fbed3ecff948fb407f3791a Mon Sep 17 00:00:00 2001 From: irbekrm Date: Thu, 7 Oct 2021 13:56:32 +0100 Subject: [PATCH] Uses go/crypto ListCertAlternates function to fetch alternative certificate chains This allows us to use upstream go/crypto again instead of our own fork Signed-off-by: irbekrm --- go.mod | 8 +----- go.sum | 36 ++++++++++++++++++++++++-- hack/build/repos.bzl | 5 ++-- pkg/acme/client/fake.go | 17 ++++++------ pkg/acme/client/interfaces.go | 2 +- pkg/acme/client/middleware/logger.go | 6 ++--- pkg/controller/acmeorders/sync.go | 22 ++++++++++------ pkg/controller/acmeorders/sync_test.go | 20 ++++++++++++-- 8 files changed, 81 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index be4a289b3..4ab0e8e52 100644 --- a/go.mod +++ b/go.mod @@ -2,12 +2,6 @@ module github.com/jetstack/cert-manager go 1.17 -// This fork allows us to add alternative certificate chains for ACME see -// https://github.com/cert-manager/crypto#cert-manager-fork-of-golangxcrypto . -// It will be replaced after -// https://go-review.googlesource.com/c/crypto/+/277294/ gets merged. -replace golang.org/x/crypto => github.com/cert-manager/crypto v0.0.0-20210409161129-d4c19753215a - require ( github.com/Azure/azure-sdk-for-go v56.2.0+incompatible github.com/Azure/go-autorest/autorest v0.11.19 @@ -37,7 +31,7 @@ require ( github.com/spf13/cobra v1.2.1 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.7.0 - golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97 + golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d golang.org/x/oauth2 v0.0.0-20210810183815-faf39c7919d5 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c diff --git a/go.sum b/go.sum index 1139ea341..2b67e1e01 100644 --- a/go.sum +++ b/go.sum @@ -207,8 +207,6 @@ github.com/bugsnag/panicwrap v0.0.0-20151223152923-e2c28503fcd0/go.mod h1:D/8v3k github.com/cenkalti/backoff/v3 v3.0.0 h1:ske+9nBpD9qZsTBoF41nW5L+AIuFBKMeze18XQ3eG1c= github.com/cenkalti/backoff/v3 v3.0.0/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= -github.com/cert-manager/crypto v0.0.0-20210409161129-d4c19753215a h1:HXp46OGPFPV7He+NPxUbCgEDCBL56R7BkQRGWEkznVQ= -github.com/cert-manager/crypto v0.0.0-20210409161129-d4c19753215a/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= github.com/certifi/gocertifi v0.0.0-20191021191039-0944d244cd40/go.mod h1:sGbDF6GwGcLpkNXPUTkMRoywsNa/ol15pxFe6ERfguA= github.com/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054/go.mod h1:sGbDF6GwGcLpkNXPUTkMRoywsNa/ol15pxFe6ERfguA= github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko= @@ -1297,6 +1295,37 @@ go.uber.org/zap v1.15.0/go.mod h1:Mb2vm2krFEG5DV0W9qcHBYFtp/Wku1cvYaqPsS/WYfc= go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo= go.uber.org/zap v1.19.0 h1:mZQZefskPPCMIBCSEH0v2/iUqqLrYtaeqwD6FUGUnFE= go.uber.org/zap v1.19.0/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= +golang.org/x/crypto v0.0.0-20171113213409-9f005a07e0d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20181009213950-7c1a557ab941/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20181029021203-45a5f77698d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190320223903-b7391e95e576/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190325154230-a5d413f7728c/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190418165655-df01cb2cc480/go.mod h1:WFFai1msRO1wXaEeE5yQxYXgSfI8pQAWXbQop6sCtWE= +golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20190617133340-57b3e21c3d56/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20191122220453-ac88ee75c92c/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200414173820-0848c9571904/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= +golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= +golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 h1:7I4JAnoQBe7ZtJcBaYHi5UtiO8tQHbUSXxL+pnGRANg= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -1381,6 +1410,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190321052220-f7bb7a8bee54/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -1461,11 +1491,13 @@ golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210514084401-e8d321eab015/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210603125802-9665404d3644/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210806184541-e5e7981a1069/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2 h1:c8PlLMqBbOHoqtjteWm5/kbe6rNY2pbRfbIMVnepueo= golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d h1:SZxvLBoTP5yHO3Frd4z4vrF+DBX9vMVanchswa69toE= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= diff --git a/hack/build/repos.bzl b/hack/build/repos.bzl index c858ae672..b151aa890 100644 --- a/hack/build/repos.bzl +++ b/hack/build/repos.bzl @@ -4512,9 +4512,8 @@ def go_repositories(): build_file_generation = "on", build_file_proto_mode = "disable", importpath = "golang.org/x/crypto", - replace = "github.com/cert-manager/crypto", - sum = "h1:HXp46OGPFPV7He+NPxUbCgEDCBL56R7BkQRGWEkznVQ=", - version = "v0.0.0-20210409161129-d4c19753215a", + sum = "h1:7I4JAnoQBe7ZtJcBaYHi5UtiO8tQHbUSXxL+pnGRANg=", + version = "v0.0.0-20210921155107-089bfa567519", ) go_repository( diff --git a/pkg/acme/client/fake.go b/pkg/acme/client/fake.go index ec68cbfe1..a989bfca0 100644 --- a/pkg/acme/client/fake.go +++ b/pkg/acme/client/fake.go @@ -31,7 +31,7 @@ type FakeACME struct { FakeAuthorizeOrder func(ctx context.Context, id []acme.AuthzID, opt ...acme.OrderOption) (*acme.Order, error) FakeGetOrder func(ctx context.Context, url string) (*acme.Order, error) FakeFetchCert func(ctx context.Context, url string, bundle bool) ([][]byte, error) - FakeFetchCertAlternatives func(ctx context.Context, url string, bundle bool) ([][][]byte, error) + FakeListCertAlternates func(ctx context.Context, url string) ([]string, error) FakeWaitOrder func(ctx context.Context, url string) (*acme.Order, error) FakeCreateOrderCert func(ctx context.Context, finalizeURL string, csr []byte, bundle bool) (der [][]byte, certURL string, err error) FakeAccept func(ctx context.Context, chal *acme.Challenge) (*acme.Challenge, error) @@ -69,14 +69,6 @@ func (f *FakeACME) FetchCert(ctx context.Context, url string, bundle bool) ([][] return nil, fmt.Errorf("FetchCert not implemented") } -func (f *FakeACME) FetchCertAlternatives(ctx context.Context, url string, bundle bool) ([][][]byte, error) { - //TODO: make actual fake - if f.FakeFetchCertAlternatives != nil { - return f.FakeFetchCertAlternatives(ctx, url, bundle) - } - return nil, fmt.Errorf("FetchCertAlternatives not implemented") -} - func (f *FakeACME) WaitOrder(ctx context.Context, url string) (*acme.Order, error) { if f.FakeWaitOrder != nil { return f.FakeWaitOrder(ctx, url) @@ -162,3 +154,10 @@ func (f *FakeACME) UpdateReg(ctx context.Context, a *acme.Account) (*acme.Accoun } return nil, fmt.Errorf("UpdateReg not implemented") } + +func (f *FakeACME) ListCertAlternates(ctx context.Context, url string) ([]string, error) { + if f.FakeListCertAlternates != nil { + return f.FakeListCertAlternates(ctx, url) + } + return nil, fmt.Errorf("ListCertAlternates not implemented") +} diff --git a/pkg/acme/client/interfaces.go b/pkg/acme/client/interfaces.go index e920d46ea..912bd0f56 100644 --- a/pkg/acme/client/interfaces.go +++ b/pkg/acme/client/interfaces.go @@ -33,7 +33,7 @@ type Interface interface { AuthorizeOrder(ctx context.Context, id []acme.AuthzID, opt ...acme.OrderOption) (*acme.Order, error) GetOrder(ctx context.Context, url string) (*acme.Order, error) FetchCert(ctx context.Context, url string, bundle bool) ([][]byte, error) - FetchCertAlternatives(ctx context.Context, url string, bundle bool) ([][][]byte, error) + ListCertAlternates(ctx context.Context, url string) ([]string, error) WaitOrder(ctx context.Context, url string) (*acme.Order, error) CreateOrderCert(ctx context.Context, finalizeURL string, csr []byte, bundle bool) (der [][]byte, certURL string, err error) Accept(ctx context.Context, chal *acme.Challenge) (*acme.Challenge, error) diff --git a/pkg/acme/client/middleware/logger.go b/pkg/acme/client/middleware/logger.go index ec3789e17..11945a2fa 100644 --- a/pkg/acme/client/middleware/logger.go +++ b/pkg/acme/client/middleware/logger.go @@ -73,13 +73,13 @@ func (l *Logger) FetchCert(ctx context.Context, url string, bundle bool) ([][]by return l.baseCl.FetchCert(ctx, url, bundle) } -func (l *Logger) FetchCertAlternatives(ctx context.Context, url string, bundle bool) ([][][]byte, error) { - l.log.V(logf.TraceLevel).Info("Calling FetchCertAlternatives") +func (l *Logger) ListCertAlternates(ctx context.Context, url string) ([]string, error) { + l.log.V(logf.TraceLevel).Info("Calling ListCertAlternates") ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - return l.baseCl.FetchCertAlternatives(ctx, url, bundle) + return l.baseCl.ListCertAlternates(ctx, url) } func (l *Logger) WaitOrder(ctx context.Context, url string) (*acme.Order, error) { diff --git a/pkg/controller/acmeorders/sync.go b/pkg/controller/acmeorders/sync.go index 46f47c2a3..c0183903b 100644 --- a/pkg/controller/acmeorders/sync.go +++ b/pkg/controller/acmeorders/sync.go @@ -531,22 +531,28 @@ func (c *controller) finalizeOrder(ctx context.Context, cl acmecl.Interface, o * } if issuer.GetSpec().ACME != nil && issuer.GetSpec().ACME.PreferredChain != "" { - altBundles, err := cl.FetchCertAlternatives(ctx, certURL, true) + altURLs, err := cl.ListCertAlternates(ctx, certURL) if err != nil { - return fmt.Errorf("error fetching alternate certificates: %w", err) + return fmt.Errorf("error listing alternate certificate URLs: %w", err) } - for _, altBundle := range altBundles { - for _, certPEM := range altBundle { - cert, err := x509.ParseCertificate(certPEM) + // Loop over all alternative chains + for _, altURL := range altURLs { + altChain, err := cl.FetchCert(ctx, altURL, true) + if err != nil { + return fmt.Errorf("error fetching alternate certificate chain from %s: %w", altURL, err) + } + // Loop over each cert in this alternative chain + for _, altCert := range altChain { + cert, err := x509.ParseCertificate(altCert) if err != nil { - return fmt.Errorf("error parsing alternate certificates: %w", err) + return fmt.Errorf("error parsing alternate certificate chain: %w", err) } - log.V(logf.DebugLevel).WithValues("Issuer CN", cert.Issuer.CommonName).Info("Found alternative ACME bundle") if cert.Issuer.CommonName == issuer.GetSpec().ACME.PreferredChain { // if the issuer's CN matched the preferred chain it means this bundle is // signed by the requested chain - return c.storeCertificateOnStatus(ctx, o, altBundle) + log.V(logf.DebugLevel).WithValues("Issuer CN", cert.Issuer.CommonName).Info("Selecting alternative ACME bundle with a mathing Common Name from %s", altURL) + return c.storeCertificateOnStatus(ctx, o, altChain) } } } diff --git a/pkg/controller/acmeorders/sync_test.go b/pkg/controller/acmeorders/sync_test.go index 51294b5e5..ae82f925a 100644 --- a/pkg/controller/acmeorders/sync_test.go +++ b/pkg/controller/acmeorders/sync_test.go @@ -529,12 +529,28 @@ rUCGwbCUDI0mxadJ3Bz4WxR6fyNpBK2yAinWEsikxqEt testData := []byte("test") return [][]byte{testData}, "http://testurl", nil }, - FakeFetchCertAlternatives: func(_ context.Context, url string, bundle bool) ([][][]byte, error) { + FakeListCertAlternates: func(_ context.Context, url string) ([]string, error) { if url != "http://testurl" { return nil, errors.New("Cert URL is incorrect") } + return []string{"http://alturl"}, nil - return [][][]byte{{rawTestCert.Bytes}}, nil + }, + FakeFetchCert: func(_ context.Context, url string, bundle bool) ([][]byte, error) { + if url != "http://alturl" { + // This bit just ensures that we + // call it from the correct + // place. This is the same URL + // that is returned from + // FakeCertAlternates that + // should have been called + // before this. + return nil, errors.New("Cert URL is incorrect") + } + if !bundle { + return nil, errors.New("Expecting to be called with bundle=true") + } + return [][]byte{rawTestCert.Bytes}, nil }, FakeHTTP01ChallengeResponse: func(s string) (string, error) { // TODO: assert s = "token"