diff --git a/LICENSES b/LICENSES index aa2f001c671..f8df5b96166 100644 --- a/LICENSES +++ b/LICENSES @@ -122,6 +122,7 @@ go.uber.org/atomic,https://github.com/uber-go/atomic/blob/v1.9.0/LICENSE.txt,MIT go.uber.org/multierr,https://github.com/uber-go/multierr/blob/v1.6.0/LICENSE.txt,MIT go.uber.org/zap,https://github.com/uber-go/zap/blob/v1.24.0/LICENSE.txt,MIT golang.org/x/crypto,https://cs.opensource.google/go/x/crypto/+/v0.6.0:LICENSE,BSD-3-Clause +golang.org/x/exp,https://cs.opensource.google/go/x/exp/+/2e198f4a:LICENSE,BSD-3-Clause golang.org/x/net,https://cs.opensource.google/go/x/net/+/v0.10.0:LICENSE,BSD-3-Clause golang.org/x/oauth2,https://cs.opensource.google/go/x/oauth2/+/v0.5.0:LICENSE,BSD-3-Clause golang.org/x/sync,https://cs.opensource.google/go/x/sync/+/v0.2.0:LICENSE,BSD-3-Clause diff --git a/cmd/controller/LICENSES b/cmd/controller/LICENSES index 95670db6ec6..abf09e2a8df 100644 --- a/cmd/controller/LICENSES +++ b/cmd/controller/LICENSES @@ -111,6 +111,7 @@ go.uber.org/atomic,https://github.com/uber-go/atomic/blob/v1.9.0/LICENSE.txt,MIT go.uber.org/multierr,https://github.com/uber-go/multierr/blob/v1.6.0/LICENSE.txt,MIT go.uber.org/zap,https://github.com/uber-go/zap/blob/v1.24.0/LICENSE.txt,MIT golang.org/x/crypto,https://cs.opensource.google/go/x/crypto/+/v0.6.0:LICENSE,BSD-3-Clause +golang.org/x/exp,https://cs.opensource.google/go/x/exp/+/2e198f4a:LICENSE,BSD-3-Clause golang.org/x/net,https://cs.opensource.google/go/x/net/+/v0.10.0:LICENSE,BSD-3-Clause golang.org/x/oauth2,https://cs.opensource.google/go/x/oauth2/+/v0.5.0:LICENSE,BSD-3-Clause golang.org/x/sync/errgroup,https://cs.opensource.google/go/x/sync/+/v0.2.0:LICENSE,BSD-3-Clause diff --git a/cmd/controller/go.mod b/cmd/controller/go.mod index 5ea810514bd..4a5d285bfd0 100644 --- a/cmd/controller/go.mod +++ b/cmd/controller/go.mod @@ -121,6 +121,7 @@ require ( go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.24.0 // indirect golang.org/x/crypto v0.6.0 // indirect + golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect golang.org/x/mod v0.10.0 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/oauth2 v0.5.0 // indirect diff --git a/cmd/controller/go.sum b/cmd/controller/go.sum index 46a6780ca3f..4e2650a5010 100644 --- a/cmd/controller/go.sum +++ b/cmd/controller/go.sum @@ -578,6 +578,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/cmd/ctl/LICENSES b/cmd/ctl/LICENSES index bd025678e2e..02988cfd5c9 100644 --- a/cmd/ctl/LICENSES +++ b/cmd/ctl/LICENSES @@ -110,6 +110,7 @@ go.uber.org/atomic,https://github.com/uber-go/atomic/blob/v1.9.0/LICENSE.txt,MIT go.uber.org/multierr,https://github.com/uber-go/multierr/blob/v1.6.0/LICENSE.txt,MIT go.uber.org/zap,https://github.com/uber-go/zap/blob/v1.24.0/LICENSE.txt,MIT golang.org/x/crypto,https://cs.opensource.google/go/x/crypto/+/v0.6.0:LICENSE,BSD-3-Clause +golang.org/x/exp,https://cs.opensource.google/go/x/exp/+/2e198f4a:LICENSE,BSD-3-Clause golang.org/x/net,https://cs.opensource.google/go/x/net/+/v0.10.0:LICENSE,BSD-3-Clause golang.org/x/oauth2,https://cs.opensource.google/go/x/oauth2/+/v0.5.0:LICENSE,BSD-3-Clause golang.org/x/sync,https://cs.opensource.google/go/x/sync/+/v0.2.0:LICENSE,BSD-3-Clause diff --git a/cmd/webhook/LICENSES b/cmd/webhook/LICENSES index dab42c88a60..029eaf1f3ce 100644 --- a/cmd/webhook/LICENSES +++ b/cmd/webhook/LICENSES @@ -54,6 +54,7 @@ go.uber.org/atomic,https://github.com/uber-go/atomic/blob/v1.9.0/LICENSE.txt,MIT go.uber.org/multierr,https://github.com/uber-go/multierr/blob/v1.6.0/LICENSE.txt,MIT go.uber.org/zap,https://github.com/uber-go/zap/blob/v1.24.0/LICENSE.txt,MIT golang.org/x/crypto/md4,https://cs.opensource.google/go/x/crypto/+/v0.6.0:LICENSE,BSD-3-Clause +golang.org/x/exp,https://cs.opensource.google/go/x/exp/+/2e198f4a:LICENSE,BSD-3-Clause golang.org/x/net,https://cs.opensource.google/go/x/net/+/v0.10.0:LICENSE,BSD-3-Clause golang.org/x/oauth2,https://cs.opensource.google/go/x/oauth2/+/v0.5.0:LICENSE,BSD-3-Clause golang.org/x/sync/errgroup,https://cs.opensource.google/go/x/sync/+/v0.2.0:LICENSE,BSD-3-Clause diff --git a/cmd/webhook/go.mod b/cmd/webhook/go.mod index 0552bc13f40..1f23ed62a46 100644 --- a/cmd/webhook/go.mod +++ b/cmd/webhook/go.mod @@ -64,6 +64,7 @@ require ( go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.24.0 // indirect golang.org/x/crypto v0.6.0 // indirect + golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/oauth2 v0.5.0 // indirect golang.org/x/sync v0.2.0 // indirect diff --git a/cmd/webhook/go.sum b/cmd/webhook/go.sum index e67baf292c7..a9906bc4d3c 100644 --- a/cmd/webhook/go.sum +++ b/cmd/webhook/go.sum @@ -309,6 +309,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/go.mod b/go.mod index 85e86b1a2ae..6d68860fa6f 100644 --- a/go.mod +++ b/go.mod @@ -36,6 +36,7 @@ require ( github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.2 golang.org/x/crypto v0.6.0 + golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 golang.org/x/oauth2 v0.5.0 golang.org/x/sync v0.2.0 gomodules.xyz/jsonpatch/v2 v2.3.0 diff --git a/go.sum b/go.sum index 4a36931e518..f79174c462e 100644 --- a/go.sum +++ b/go.sum @@ -619,6 +619,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/internal/controller/feature/features.go b/internal/controller/feature/features.go index 87d4c9ed5a7..64c69bd34d6 100644 --- a/internal/controller/feature/features.go +++ b/internal/controller/feature/features.go @@ -85,6 +85,13 @@ const ( // they know cert-manager will need access to to speed up issuance. // See https://github.com/cert-manager/cert-manager/blob/master/design/20221205-memory-management.md SecretsFilteredCaching featuregate.Feature = "SecretsFilteredCaching" + + // Owner (responsible for graduating feature through to GA): @inteon + // GA: v1.13 + // DontAllowInsecureCSRUsageDefinition will prevent the webhook from allowing + // CertificateRequest's usages to be only defined in the CSR, while leaving + // the usages field empty. + DontAllowInsecureCSRUsageDefinition featuregate.Feature = "DontAllowInsecureCSRUsageDefinition" ) func init() { @@ -95,6 +102,8 @@ func init() { // To add a new feature, define a key for it above and add it here. The features will be // available on the cert-manager controller binary. var defaultCertManagerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ + DontAllowInsecureCSRUsageDefinition: {Default: true, PreRelease: featuregate.GA}, + ValidateCAA: {Default: false, PreRelease: featuregate.Alpha}, ExperimentalCertificateSigningRequestControllers: {Default: false, PreRelease: featuregate.Alpha}, ExperimentalGatewayAPISupport: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/controller/certificaterequests/acme/acme_test.go b/pkg/controller/certificaterequests/acme/acme_test.go index 6629e9e1610..eff9837a75e 100644 --- a/pkg/controller/certificaterequests/acme/acme_test.go +++ b/pkg/controller/certificaterequests/acme/acme_test.go @@ -172,8 +172,8 @@ func TestSign(t *testing.T) { template2, err := pki.CertificateTemplateFromCSRPEM( generateCSR(t, sk2, "example.com", "example.com", "foo.com"), pki.CertificateTemplateOverrideDuration(time.Hour), - pki.CertificateTemplateOverrideBasicConstraints(false, nil), - pki.CertificateTemplateOverrideKeyUsages(0, nil), + pki.CertificateTemplateValidateAndOverrideBasicConstraints(false, nil), + pki.CertificateTemplateValidateAndOverrideKeyUsages(0, nil), ) if err != nil { t.Fatal(err) diff --git a/pkg/controller/certificaterequests/ca/ca.go b/pkg/controller/certificaterequests/ca/ca.go index 9c6afd5167e..83329222ca4 100644 --- a/pkg/controller/certificaterequests/ca/ca.go +++ b/pkg/controller/certificaterequests/ca/ca.go @@ -24,6 +24,7 @@ import ( k8sErrors "k8s.io/apimachinery/pkg/api/errors" + "github.com/cert-manager/cert-manager/internal/controller/feature" internalinformers "github.com/cert-manager/cert-manager/internal/informers" apiutil "github.com/cert-manager/cert-manager/pkg/api/util" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" @@ -33,6 +34,7 @@ import ( issuerpkg "github.com/cert-manager/cert-manager/pkg/issuer" logf "github.com/cert-manager/cert-manager/pkg/logs" cmerrors "github.com/cert-manager/cert-manager/pkg/util/errors" + utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature" "github.com/cert-manager/cert-manager/pkg/util/kube" "github.com/cert-manager/cert-manager/pkg/util/pki" ) @@ -66,11 +68,17 @@ func init() { func NewCA(ctx *controllerpkg.Context) certificaterequests.Issuer { return &CA{ - issuerOptions: ctx.IssuerOptions, - secretsLister: ctx.KubeSharedInformerFactory.Secrets().Lister(), - reporter: crutil.NewReporter(ctx.Clock, ctx.Recorder), - templateGenerator: pki.CertificateTemplateFromCertificateRequest, - signingFn: pki.SignCSRTemplate, + issuerOptions: ctx.IssuerOptions, + secretsLister: ctx.KubeSharedInformerFactory.Secrets().Lister(), + reporter: crutil.NewReporter(ctx.Clock, ctx.Recorder), + templateGenerator: func(cr *cmapi.CertificateRequest) (*x509.Certificate, error) { + if !utilfeature.DefaultMutableFeatureGate.Enabled(feature.DontAllowInsecureCSRUsageDefinition) { + return pki.DeprecatedCertificateTemplateFromCertificateRequestAndAllowInsecureCSRUsageDefinition(cr) + } + + return pki.CertificateTemplateFromCertificateRequest(cr) + }, + signingFn: pki.SignCSRTemplate, } } diff --git a/pkg/controller/certificaterequests/selfsigned/selfsigned.go b/pkg/controller/certificaterequests/selfsigned/selfsigned.go index 1d3cac27b45..6e4f6b22567 100644 --- a/pkg/controller/certificaterequests/selfsigned/selfsigned.go +++ b/pkg/controller/certificaterequests/selfsigned/selfsigned.go @@ -29,6 +29,7 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" + "github.com/cert-manager/cert-manager/internal/controller/feature" internalinformers "github.com/cert-manager/cert-manager/internal/informers" apiutil "github.com/cert-manager/cert-manager/pkg/api/util" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" @@ -38,6 +39,7 @@ import ( "github.com/cert-manager/cert-manager/pkg/issuer" logf "github.com/cert-manager/cert-manager/pkg/logs" cmerrors "github.com/cert-manager/cert-manager/pkg/util/errors" + utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature" "github.com/cert-manager/cert-manager/pkg/util/kube" "github.com/cert-manager/cert-manager/pkg/util/pki" "github.com/go-logr/logr" @@ -147,7 +149,12 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *cmapi.CertificateRequest, iss return nil, err } - template, err := pki.CertificateTemplateFromCertificateRequest(cr) + var template *x509.Certificate + if !utilfeature.DefaultMutableFeatureGate.Enabled(feature.DontAllowInsecureCSRUsageDefinition) { + template, err = pki.DeprecatedCertificateTemplateFromCertificateRequestAndAllowInsecureCSRUsageDefinition(cr) + } else { + template, err = pki.CertificateTemplateFromCertificateRequest(cr) + } if err != nil { message := "Error generating certificate template" s.reporter.Failed(cr, err, "ErrorGenerating", message) diff --git a/pkg/controller/certificatesigningrequests/venafi/venafi_test.go b/pkg/controller/certificatesigningrequests/venafi/venafi_test.go index e20b3b98088..218971a4cbf 100644 --- a/pkg/controller/certificatesigningrequests/venafi/venafi_test.go +++ b/pkg/controller/certificatesigningrequests/venafi/venafi_test.go @@ -72,8 +72,8 @@ func TestProcessItem(t *testing.T) { rootTmpl, err := pki.CertificateTemplateFromCSRPEM( rootCSRPEM, pki.CertificateTemplateOverrideDuration(time.Hour), - pki.CertificateTemplateOverrideBasicConstraints(true, nil), - pki.CertificateTemplateOverrideKeyUsages(0, nil), + pki.CertificateTemplateValidateAndOverrideBasicConstraints(true, nil), + pki.CertificateTemplateValidateAndOverrideKeyUsages(0, nil), ) if err != nil { t.Fatal(err) @@ -92,8 +92,8 @@ func TestProcessItem(t *testing.T) { leafTmpl, err := pki.CertificateTemplateFromCSRPEM( leafCSRPEM, pki.CertificateTemplateOverrideDuration(time.Hour), - pki.CertificateTemplateOverrideBasicConstraints(false, nil), - pki.CertificateTemplateOverrideKeyUsages(0, nil), + pki.CertificateTemplateValidateAndOverrideBasicConstraints(false, nil), + pki.CertificateTemplateValidateAndOverrideKeyUsages(0, nil), ) if err != nil { t.Fatal(err) diff --git a/pkg/issuer/venafi/client/request.go b/pkg/issuer/venafi/client/request.go index 9fd1e2c8aa2..a05b3400fec 100644 --- a/pkg/issuer/venafi/client/request.go +++ b/pkg/issuer/venafi/client/request.go @@ -83,12 +83,7 @@ func (v *Venafi) buildVReq(csrPEM []byte, duration time.Duration, customFields [ return nil, err } - tmpl, err := pki.CertificateTemplateFromCSRPEM( - csrPEM, - pki.CertificateTemplateOverrideDuration(duration), - pki.CertificateTemplateOverrideBasicConstraints(false, nil), - pki.CertificateTemplateOverrideKeyUsages(0, nil), - ) + tmpl, err := pki.CertificateTemplateFromCSRPEM(csrPEM) if err != nil { return nil, err } diff --git a/pkg/util/pki/certificatetemplate.go b/pkg/util/pki/certificatetemplate.go index f043d8a8d76..2633498932a 100644 --- a/pkg/util/pki/certificatetemplate.go +++ b/pkg/util/pki/certificatetemplate.go @@ -20,30 +20,75 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" + "encoding/asn1" "fmt" + "strings" "time" apiutil "github.com/cert-manager/cert-manager/pkg/api/util" v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" experimentalapi "github.com/cert-manager/cert-manager/pkg/apis/experimental/v1alpha1" + "golang.org/x/exp/slices" certificatesv1 "k8s.io/api/certificates/v1" ) -type CertificateTemplateMutator func(*x509.Certificate) +type CertificateTemplateValidatorMutator func(*x509.CertificateRequest, *x509.Certificate) error -// CertificateTemplateOverrideDuration returns a CertificateTemplateMutator that overrides the +func hasExtension(checkReq *x509.CertificateRequest, extensionID asn1.ObjectIdentifier) bool { + for _, ext := range checkReq.Extensions { + if ext.Id.Equal(extensionID) { + return true + } + } + + for _, ext := range checkReq.ExtraExtensions { + if ext.Id.Equal(extensionID) { + return true + } + } + + return false +} + +// CertificateTemplateOverrideDuration returns a CertificateTemplateValidatorMutator that overrides the // certificate duration. -func CertificateTemplateOverrideDuration(duration time.Duration) CertificateTemplateMutator { - return func(cert *x509.Certificate) { +func CertificateTemplateOverrideDuration(duration time.Duration) CertificateTemplateValidatorMutator { + return func(req *x509.CertificateRequest, cert *x509.Certificate) error { cert.NotBefore = time.Now() cert.NotAfter = cert.NotBefore.Add(duration) + return nil } } -// CertificateTemplateOverrideBasicConstraints returns a CertificateTemplateMutator that overrides +// CertificateTemplateValidateAndOverrideBasicConstraints returns a CertificateTemplateValidatorMutator that overrides // the certificate basic constraints. -func CertificateTemplateOverrideBasicConstraints(isCA bool, maxPathLen *int) CertificateTemplateMutator { - return func(cert *x509.Certificate) { +func CertificateTemplateValidateAndOverrideBasicConstraints(isCA bool, maxPathLen *int) CertificateTemplateValidatorMutator { + return func(req *x509.CertificateRequest, cert *x509.Certificate) error { + if hasExtension(req, OIDExtensionBasicConstraints) { + if !cert.BasicConstraintsValid { + return fmt.Errorf("encoded CSR error: BasicConstraintsValid is not true") + } + + if cert.IsCA != isCA { + return fmt.Errorf("encoded CSR error: IsCA %v does not match expected value %v", cert.IsCA, isCA) + } + + expectedMaxPathLen := 0 + expectedMaxPathLenZero := false + if maxPathLen != nil { + expectedMaxPathLen = *maxPathLen + expectedMaxPathLenZero = *maxPathLen == 0 + } + + if cert.MaxPathLen != expectedMaxPathLen { + return fmt.Errorf("encoded CSR error: MaxPathLen %v does not match expected value %v", cert.MaxPathLen, expectedMaxPathLen) + } + + if cert.MaxPathLenZero != expectedMaxPathLenZero { + return fmt.Errorf("encoded CSR error: MaxPathLenZero %v does not match expected value %v", cert.MaxPathLenZero, expectedMaxPathLenZero) + } + } + cert.BasicConstraintsValid = true cert.IsCA = isCA if maxPathLen != nil { @@ -53,22 +98,68 @@ func CertificateTemplateOverrideBasicConstraints(isCA bool, maxPathLen *int) Cer cert.MaxPathLen = 0 cert.MaxPathLenZero = false } + return nil } } -// OverrideTemplateKeyUsages returns a CertificateTemplateMutator that overrides the +// CertificateTemplateValidateAndOverrideKeyUsages returns a CertificateTemplateValidatorMutator that overrides the // certificate key usages. -func CertificateTemplateOverrideKeyUsages(keyUsage x509.KeyUsage, extKeyUsage []x509.ExtKeyUsage) CertificateTemplateMutator { - return func(cert *x509.Certificate) { +func CertificateTemplateValidateAndOverrideKeyUsages(keyUsage x509.KeyUsage, extKeyUsage []x509.ExtKeyUsage) CertificateTemplateValidatorMutator { + return func(req *x509.CertificateRequest, cert *x509.Certificate) error { + if hasExtension(req, OIDExtensionKeyUsage) || hasExtension(req, OIDExtensionExtendedKeyUsage) { + if cert.KeyUsage != keyUsage { + return fmt.Errorf("encoded CSR error: the KeyUsages %s do not match the expected KeyUsages %s", + printKeyUsage(apiutil.KeyUsageStrings(cert.KeyUsage)), + printKeyUsage(apiutil.KeyUsageStrings(keyUsage)), + ) + } + + if !slices.Equal(cert.ExtKeyUsage, extKeyUsage) { + return fmt.Errorf("encoded CSR error: the ExtKeyUsages %s do not match the expected ExtKeyUsages %s", + printKeyUsage(apiutil.ExtKeyUsageStrings(cert.ExtKeyUsage)), + printKeyUsage(apiutil.ExtKeyUsageStrings(extKeyUsage)), + ) + } + } + cert.KeyUsage = keyUsage cert.ExtKeyUsage = extKeyUsage + return nil + } +} + +type printKeyUsage []v1.KeyUsage + +func (k printKeyUsage) String() string { + var sb strings.Builder + sb.WriteString("[") + for i, u := range k { + sb.WriteString(" '") + sb.WriteString(string(u)) + sb.WriteString("'") + if i < len(k)-1 { + sb.WriteString(",") + } + } + if len(k) > 0 { + sb.WriteString(" ") + } + sb.WriteString("]") + return sb.String() +} + +// Deprecated: use CertificateTemplateValidateAndOverrideKeyUsages instead. +func certificateTemplateOverrideKeyUsages(keyUsage x509.KeyUsage, extKeyUsage []x509.ExtKeyUsage) CertificateTemplateValidatorMutator { + return func(req *x509.CertificateRequest, cert *x509.Certificate) error { + cert.KeyUsage = keyUsage + cert.ExtKeyUsage = extKeyUsage + return nil } } // CertificateTemplateFromCSR will create a x509.Certificate for the // given *x509.CertificateRequest. -// Call OverrideTemplateFromOptions to override the duration, isCA, maxPathLen, keyUsage, and extKeyUsage. -func CertificateTemplateFromCSR(csr *x509.CertificateRequest, mutators ...CertificateTemplateMutator) (*x509.Certificate, error) { +func CertificateTemplateFromCSR(csr *x509.CertificateRequest, validatorMutators ...CertificateTemplateValidatorMutator) (*x509.Certificate, error) { serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) if err != nil { return nil, fmt.Errorf("failed to generate serial number: %s", err.Error()) @@ -145,8 +236,10 @@ func CertificateTemplateFromCSR(csr *x509.CertificateRequest, mutators ...Certif } } - for _, mutator := range mutators { - mutator(cert) + for _, validatorMutator := range validatorMutators { + if err := validatorMutator(csr, cert); err != nil { + return nil, err + } } return cert, nil @@ -154,8 +247,7 @@ func CertificateTemplateFromCSR(csr *x509.CertificateRequest, mutators ...Certif // CertificateTemplateFromCSRPEM will create a x509.Certificate for the // given csrPEM. -// Call OverrideTemplateFromOptions to override the duration, isCA, maxPathLen, keyUsage, and extKeyUsage. -func CertificateTemplateFromCSRPEM(csrPEM []byte, mutators ...CertificateTemplateMutator) (*x509.Certificate, error) { +func CertificateTemplateFromCSRPEM(csrPEM []byte, validatorMutators ...CertificateTemplateValidatorMutator) (*x509.Certificate, error) { csr, err := DecodeX509CertificateRequestBytes(csrPEM) if err != nil { return nil, err @@ -165,7 +257,7 @@ func CertificateTemplateFromCSRPEM(csrPEM []byte, mutators ...CertificateTemplat return nil, err } - return CertificateTemplateFromCSR(csr, mutators...) + return CertificateTemplateFromCSR(csr, validatorMutators...) } // CertificateTemplateFromCertificate will create a x509.Certificate for the given @@ -185,27 +277,44 @@ func CertificateTemplateFromCertificate(crt *v1.Certificate) (*x509.Certificate, return CertificateTemplateFromCSR( csr, CertificateTemplateOverrideDuration(certDuration), - CertificateTemplateOverrideBasicConstraints(crt.Spec.IsCA, nil), - CertificateTemplateOverrideKeyUsages(keyUsage, extKeyUsage), + CertificateTemplateValidateAndOverrideBasicConstraints(crt.Spec.IsCA, nil), + CertificateTemplateValidateAndOverrideKeyUsages(keyUsage, extKeyUsage), ) } +func makeCertificateTemplateFromCertificateRequestFunc(allowInsecureCSRUsageDefinition bool) func(cr *v1.CertificateRequest) (*x509.Certificate, error) { + return func(cr *v1.CertificateRequest) (*x509.Certificate, error) { + certDuration := apiutil.DefaultCertDuration(cr.Spec.Duration) + keyUsage, extKeyUsage, err := KeyUsagesForCertificateOrCertificateRequest(cr.Spec.Usages, cr.Spec.IsCA) + if err != nil { + return nil, err + } + + return CertificateTemplateFromCSRPEM( + cr.Spec.Request, + CertificateTemplateOverrideDuration(certDuration), + CertificateTemplateValidateAndOverrideBasicConstraints(cr.Spec.IsCA, nil), // Override the basic constraints, but make sure they match the constraints in the CSR if present + (func() CertificateTemplateValidatorMutator { + if allowInsecureCSRUsageDefinition && len(cr.Spec.Usages) == 0 { + // If the CertificateRequest does not specify any usages, and the AllowInsecureCSRUsageDefinition + // flag is set, then we allow the usages to be defined solely by the CSR blob, but we still override + // the usages to match the old behavior. + return certificateTemplateOverrideKeyUsages(keyUsage, extKeyUsage) + } + + // Override the key usages, but make sure they match the usages in the CSR if present + return CertificateTemplateValidateAndOverrideKeyUsages(keyUsage, extKeyUsage) + })(), + ) + } +} + // CertificateTemplateFromCertificateRequest will create a x509.Certificate for the given // CertificateRequest resource -func CertificateTemplateFromCertificateRequest(cr *v1.CertificateRequest) (*x509.Certificate, error) { - certDuration := apiutil.DefaultCertDuration(cr.Spec.Duration) - keyUsage, extKeyUsage, err := KeyUsagesForCertificateOrCertificateRequest(cr.Spec.Usages, cr.Spec.IsCA) - if err != nil { - return nil, err - } +var CertificateTemplateFromCertificateRequest = makeCertificateTemplateFromCertificateRequestFunc(false) - return CertificateTemplateFromCSRPEM( - cr.Spec.Request, - CertificateTemplateOverrideDuration(certDuration), - CertificateTemplateOverrideBasicConstraints(cr.Spec.IsCA, nil), - CertificateTemplateOverrideKeyUsages(keyUsage, extKeyUsage), - ) -} +// Deprecated: Use CertificateTemplateFromCertificateRequest instead. +var DeprecatedCertificateTemplateFromCertificateRequestAndAllowInsecureCSRUsageDefinition = makeCertificateTemplateFromCertificateRequestFunc(true) // CertificateTemplateFromCertificateSigningRequest will create a x509.Certificate for the given // CertificateSigningRequest resource @@ -225,8 +334,8 @@ func CertificateTemplateFromCertificateSigningRequest(csr *certificatesv1.Certif return CertificateTemplateFromCSRPEM( csr.Spec.Request, CertificateTemplateOverrideDuration(duration), - CertificateTemplateOverrideBasicConstraints(isCA, nil), - CertificateTemplateOverrideKeyUsages(ku, eku), + CertificateTemplateValidateAndOverrideBasicConstraints(isCA, nil), // Override the basic constraints, but make sure they match the constraints in the CSR if present + CertificateTemplateValidateAndOverrideKeyUsages(ku, eku), // Override the key usages, but make sure they match the usages in the CSR if present ) } @@ -250,8 +359,8 @@ func GenerateTemplateFromCSRPEM(csrPEM []byte, duration time.Duration, isCA bool return CertificateTemplateFromCSRPEM( csrPEM, CertificateTemplateOverrideDuration(duration), - CertificateTemplateOverrideBasicConstraints(isCA, nil), - CertificateTemplateOverrideKeyUsages(0, nil), + CertificateTemplateValidateAndOverrideBasicConstraints(isCA, nil), // Override the basic constraints, but make sure they match the constraints in the CSR if present + certificateTemplateOverrideKeyUsages(0, nil), // Override the key usages to be empty ) } @@ -260,7 +369,7 @@ func GenerateTemplateFromCSRPEMWithUsages(csrPEM []byte, duration time.Duration, return CertificateTemplateFromCSRPEM( csrPEM, CertificateTemplateOverrideDuration(duration), - CertificateTemplateOverrideBasicConstraints(isCA, nil), - CertificateTemplateOverrideKeyUsages(keyUsage, extKeyUsage), + CertificateTemplateValidateAndOverrideBasicConstraints(isCA, nil), // Override the basic constraints, but make sure they match the constraints in the CSR if present + CertificateTemplateValidateAndOverrideKeyUsages(keyUsage, extKeyUsage), // Override the key usages, but make sure they match the usages in the CSR if present ) } diff --git a/test/e2e/LICENSES b/test/e2e/LICENSES index 67eeab174b0..dce69e17dca 100644 --- a/test/e2e/LICENSES +++ b/test/e2e/LICENSES @@ -61,6 +61,7 @@ go.uber.org/atomic,https://github.com/uber-go/atomic/blob/v1.9.0/LICENSE.txt,MIT go.uber.org/multierr,https://github.com/uber-go/multierr/blob/v1.6.0/LICENSE.txt,MIT go.uber.org/zap,https://github.com/uber-go/zap/blob/v1.24.0/LICENSE.txt,MIT golang.org/x/crypto,https://cs.opensource.google/go/x/crypto/+/v0.6.0:LICENSE,BSD-3-Clause +golang.org/x/exp,https://cs.opensource.google/go/x/exp/+/2e198f4a:LICENSE,BSD-3-Clause golang.org/x/net,https://cs.opensource.google/go/x/net/+/v0.10.0:LICENSE,BSD-3-Clause golang.org/x/oauth2,https://cs.opensource.google/go/x/oauth2/+/v0.5.0:LICENSE,BSD-3-Clause golang.org/x/sys/unix,https://cs.opensource.google/go/x/sys/+/v0.8.0:LICENSE,BSD-3-Clause diff --git a/test/e2e/go.mod b/test/e2e/go.mod index 97f41d5382c..78d8ae0fac6 100644 --- a/test/e2e/go.mod +++ b/test/e2e/go.mod @@ -80,6 +80,7 @@ require ( go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.24.0 // indirect golang.org/x/crypto v0.6.0 // indirect + golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/oauth2 v0.5.0 // indirect golang.org/x/sys v0.8.0 // indirect diff --git a/test/e2e/go.sum b/test/e2e/go.sum index f35fe035059..2301c16ebe0 100644 --- a/test/e2e/go.sum +++ b/test/e2e/go.sum @@ -252,6 +252,8 @@ golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0 golang.org/x/crypto v0.6.0 h1:qfktjS5LUO+fFKeJXZ+ikTRijMmljikvG68fpMMruSc= golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= diff --git a/test/e2e/suite/certificatesigningrequests/selfsigned/selfsigned.go b/test/e2e/suite/certificatesigningrequests/selfsigned/selfsigned.go index 81db08c9d1c..7102bdd16dc 100644 --- a/test/e2e/suite/certificatesigningrequests/selfsigned/selfsigned.go +++ b/test/e2e/suite/certificatesigningrequests/selfsigned/selfsigned.go @@ -80,7 +80,7 @@ var _ = framework.CertManagerDescribe("CertificateSigningRequests SelfSigned Sec Spec: certificatesv1.CertificateSigningRequestSpec{ Request: bundle.CSRBytes, SignerName: fmt.Sprintf("issuers.cert-manager.io/%s.%s", f.Namespace.Name, issuer.GetName()), - Usages: []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth, certificatesv1.UsageServerAuth}, + Usages: []certificatesv1.KeyUsage{certificatesv1.UsageKeyEncipherment, certificatesv1.UsageDigitalSignature}, }, }, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -148,7 +148,7 @@ var _ = framework.CertManagerDescribe("CertificateSigningRequests SelfSigned Sec Spec: certificatesv1.CertificateSigningRequestSpec{ Request: bundle.CSRBytes, SignerName: fmt.Sprintf("issuers.cert-manager.io/%s.%s", f.Namespace.Name, issuer.GetName()), - Usages: []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth, certificatesv1.UsageServerAuth}, + Usages: []certificatesv1.KeyUsage{certificatesv1.UsageKeyEncipherment, certificatesv1.UsageDigitalSignature}, }, }, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -206,7 +206,7 @@ var _ = framework.CertManagerDescribe("CertificateSigningRequests SelfSigned Sec Spec: certificatesv1.CertificateSigningRequestSpec{ Request: bundle.CSRBytes, SignerName: fmt.Sprintf("clusterissuers.cert-manager.io/" + issuer.GetName()), - Usages: []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth, certificatesv1.UsageServerAuth}, + Usages: []certificatesv1.KeyUsage{certificatesv1.UsageKeyEncipherment, certificatesv1.UsageDigitalSignature}, }, }, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -274,7 +274,7 @@ var _ = framework.CertManagerDescribe("CertificateSigningRequests SelfSigned Sec Spec: certificatesv1.CertificateSigningRequestSpec{ Request: bundle.CSRBytes, SignerName: fmt.Sprintf("clusterissuers.cert-manager.io/" + issuer.GetName()), - Usages: []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth, certificatesv1.UsageServerAuth}, + Usages: []certificatesv1.KeyUsage{certificatesv1.UsageKeyEncipherment, certificatesv1.UsageDigitalSignature}, }, }, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) diff --git a/test/integration/LICENSES b/test/integration/LICENSES index fc28ae37019..09a929f1a99 100644 --- a/test/integration/LICENSES +++ b/test/integration/LICENSES @@ -133,6 +133,7 @@ go.uber.org/atomic,https://github.com/uber-go/atomic/blob/v1.9.0/LICENSE.txt,MIT go.uber.org/multierr,https://github.com/uber-go/multierr/blob/v1.6.0/LICENSE.txt,MIT go.uber.org/zap,https://github.com/uber-go/zap/blob/v1.24.0/LICENSE.txt,MIT golang.org/x/crypto,https://cs.opensource.google/go/x/crypto/+/v0.6.0:LICENSE,BSD-3-Clause +golang.org/x/exp,https://cs.opensource.google/go/x/exp/+/2e198f4a:LICENSE,BSD-3-Clause golang.org/x/net,https://cs.opensource.google/go/x/net/+/v0.10.0:LICENSE,BSD-3-Clause golang.org/x/oauth2,https://cs.opensource.google/go/x/oauth2/+/v0.5.0:LICENSE,BSD-3-Clause golang.org/x/sync,https://cs.opensource.google/go/x/sync/+/v0.2.0:LICENSE,BSD-3-Clause diff --git a/test/integration/go.mod b/test/integration/go.mod index 677ee15e18d..56f698ac149 100644 --- a/test/integration/go.mod +++ b/test/integration/go.mod @@ -161,6 +161,7 @@ require ( go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.24.0 // indirect + golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect golang.org/x/mod v0.10.0 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/oauth2 v0.5.0 // indirect diff --git a/test/integration/go.sum b/test/integration/go.sum index 8d7191db43f..6e48b8a2be9 100644 --- a/test/integration/go.sum +++ b/test/integration/go.sum @@ -897,6 +897,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=