diff --git a/internal/apis/certmanager/types_certificate.go b/internal/apis/certmanager/types_certificate.go index ac7f9b0e4..0e6872c79 100644 --- a/internal/apis/certmanager/types_certificate.go +++ b/internal/apis/certmanager/types_certificate.go @@ -544,8 +544,8 @@ type NameConstraints struct { // // +optional Permitted *NameConstraintItem - // Excluded contains the constraints which must be disallowed. Any name matching a - // restriction in the excluded field is invalid regardless + // Excluded contains the constraints which must be disallowed. Any name matching a + // restriction in the excluded field is invalid regardless // of information appearing in the permitted // // +optional diff --git a/internal/apis/certmanager/v1alpha2/types_certificate.go b/internal/apis/certmanager/v1alpha2/types_certificate.go index 8ef7ce266..7721ad796 100644 --- a/internal/apis/certmanager/v1alpha2/types_certificate.go +++ b/internal/apis/certmanager/v1alpha2/types_certificate.go @@ -513,9 +513,9 @@ type NameConstraints struct { // Permitted contains the constraints in which the names must be located. // // +optional - Permitted *NameConstraintItem`json:"permitted,omitempty"` - // Excluded contains the constraints which must be disallowed. Any name matching a - // restriction in the excluded field is invalid regardless + Permitted *NameConstraintItem `json:"permitted,omitempty"` + // Excluded contains the constraints which must be disallowed. Any name matching a + // restriction in the excluded field is invalid regardless // of information appearing in the permitted // // +optional diff --git a/internal/apis/certmanager/v1alpha3/types_certificate.go b/internal/apis/certmanager/v1alpha3/types_certificate.go index e0f3ba602..86cd012a4 100644 --- a/internal/apis/certmanager/v1alpha3/types_certificate.go +++ b/internal/apis/certmanager/v1alpha3/types_certificate.go @@ -520,9 +520,9 @@ type NameConstraints struct { // Permitted contains the constraints in which the names must be located. // // +optional - Permitted *NameConstraintItem`json:"permitted,omitempty"` - // Excluded contains the constraints which must be disallowed. Any name matching a - // restriction in the excluded field is invalid regardless + Permitted *NameConstraintItem `json:"permitted,omitempty"` + // Excluded contains the constraints which must be disallowed. Any name matching a + // restriction in the excluded field is invalid regardless // of information appearing in the permitted // // +optional diff --git a/internal/apis/certmanager/v1beta1/types_certificate.go b/internal/apis/certmanager/v1beta1/types_certificate.go index 48b8bf6f3..bf3c8c9c6 100644 --- a/internal/apis/certmanager/v1beta1/types_certificate.go +++ b/internal/apis/certmanager/v1beta1/types_certificate.go @@ -518,9 +518,9 @@ type NameConstraints struct { // Permitted contains the constraints in which the names must be located. // // +optional - Permitted *NameConstraintItem`json:"permitted,omitempty"` - // Excluded contains the constraints which must be disallowed. Any name matching a - // restriction in the excluded field is invalid regardless + Permitted *NameConstraintItem `json:"permitted,omitempty"` + // Excluded contains the constraints which must be disallowed. Any name matching a + // restriction in the excluded field is invalid regardless // of information appearing in the permitted // // +optional diff --git a/internal/apis/certmanager/validation/certificate_test.go b/internal/apis/certmanager/validation/certificate_test.go index 103de4ec5..0e951ac3a 100644 --- a/internal/apis/certmanager/validation/certificate_test.go +++ b/internal/apis/certmanager/validation/certificate_test.go @@ -684,7 +684,7 @@ func TestValidateCertificate(t *testing.T) { Spec: internalcmapi.CertificateSpec{ CommonName: "testcn", SecretName: "abc", - IsCA: true, + IsCA: true, NameConstraints: &internalcmapi.NameConstraints{ Permitted: &internalcmapi.NameConstraintItem{ DNSDomains: []string{"example.com"}, @@ -700,9 +700,9 @@ func TestValidateCertificate(t *testing.T) { "invalid with name constraints": { cfg: &internalcmapi.Certificate{ Spec: internalcmapi.CertificateSpec{ - CommonName: "testcn", - SecretName: "abc", - IsCA: true, + CommonName: "testcn", + SecretName: "abc", + IsCA: true, NameConstraints: &internalcmapi.NameConstraints{}, IssuerRef: cmmeta.ObjectReference{ Name: "valid", diff --git a/internal/controller/feature/features.go b/internal/controller/feature/features.go index 41ee3bfcd..48840e972 100644 --- a/internal/controller/feature/features.go +++ b/internal/controller/feature/features.go @@ -99,7 +99,7 @@ const ( UseCertificateRequestBasicConstraints featuregate.Feature = "UseCertificateRequestBasicConstraints" // Owner: @tanujd11 - // Alpha: v1.13 + // Alpha: v1.14 // // UseCertificateRequestNameConstraints will add Name Constraints section in the Extension Request of the Certificate Signing Request // This feature will add NameConstraints section in CSR with CA field as true @@ -147,5 +147,5 @@ var defaultCertManagerFeatureGates = map[featuregate.Feature]featuregate.Feature ServerSideApply: {Default: false, PreRelease: featuregate.Alpha}, LiteralCertificateSubject: {Default: false, PreRelease: featuregate.Alpha}, UseCertificateRequestBasicConstraints: {Default: false, PreRelease: featuregate.Alpha}, - UseCertificateRequestNameConstraints: {Default: false, PreRelease: featuregate.Alpha}, + UseCertificateRequestNameConstraints: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/pkg/apis/certmanager/v1/types_certificate.go b/pkg/apis/certmanager/v1/types_certificate.go index 0a597f87b..94159179c 100644 --- a/pkg/apis/certmanager/v1/types_certificate.go +++ b/pkg/apis/certmanager/v1/types_certificate.go @@ -609,8 +609,8 @@ type NameConstraints struct { // // +optional Permitted *NameConstraintItem `json:"permitted,omitempty"` - // Excluded contains the constraints which must be disallowed. Any name matching a - // restriction in the excluded field is invalid regardless + // Excluded contains the constraints which must be disallowed. Any name matching a + // restriction in the excluded field is invalid regardless // of information appearing in the permitted // // +optional diff --git a/pkg/util/pki/certificatetemplate.go b/pkg/util/pki/certificatetemplate.go index 01ecbc115..c69a85f9d 100644 --- a/pkg/util/pki/certificatetemplate.go +++ b/pkg/util/pki/certificatetemplate.go @@ -198,16 +198,16 @@ func CertificateTemplateFromCSR(csr *x509.CertificateRequest, validatorMutators if err != nil { return err } - + template.PermittedDNSDomainsCritical = nameConstraints.PermittedDNSDomainsCritical template.PermittedDNSDomains = nameConstraints.PermittedDNSDomains - template.ExcludedDNSDomains = nameConstraints.ExcludedDNSDomains + template.ExcludedDNSDomains = nameConstraints.ExcludedDNSDomains template.PermittedIPRanges = ConvertIPNeSliceToIPNetPointerSlice(nameConstraints.PermittedIPRanges) - template.ExcludedIPRanges = ConvertIPNeSliceToIPNetPointerSlice(nameConstraints.ExcludedIPRanges) + template.ExcludedIPRanges = ConvertIPNeSliceToIPNetPointerSlice(nameConstraints.ExcludedIPRanges) template.PermittedEmailAddresses = nameConstraints.PermittedEmailAddresses template.ExcludedEmailAddresses = nameConstraints.ExcludedEmailAddresses template.PermittedURIDomains = nameConstraints.PermittedURIDomains - template.ExcludedURIDomains = nameConstraints.ExcludedEmailAddresses + template.ExcludedURIDomains = nameConstraints.ExcludedEmailAddresses } // RFC 5280, 4.2.1.3 diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index da6b54254..975f6d5a6 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -207,7 +207,6 @@ func WithEncodeNameConstraintsInRequest(encode bool) GenerateCSROption { } } - func WithUseLiteralSubject(useLiteralSubject bool) GenerateCSROption { return func(o *generateCSROptions) { o.UseLiteralSubject = useLiteralSubject @@ -382,6 +381,10 @@ func SignCSRTemplate(caCerts []*x509.Certificate, caKey crypto.Signer, template } issuingCACert := caCerts[0] + err := validateNameConstraints(issuingCACert, template) + if err != nil { + return PEMBundle{}, fmt.Errorf("cert not present in the namesConstraints specified by cacert: %s", err.Error()) + } _, cert, err := SignCertificate(template, issuingCACert, template.PublicKey, caKey) if err != nil { diff --git a/pkg/util/pki/csr_test.go b/pkg/util/pki/csr_test.go index 109967a99..60c95bcbe 100644 --- a/pkg/util/pki/csr_test.go +++ b/pkg/util/pki/csr_test.go @@ -359,9 +359,9 @@ func TestGenerateCSR(t *testing.T) { nameConstraints := NameConstraints{ PermittedDNSDomainsCritical: true, PermittedDNSDomains: []string{"example.org"}, - PermittedIPRanges: []net.IPNet{*permittedIPNet}, - PermittedEmailAddresses: []string{"email@email.org"}, - ExcludedIPRanges: []net.IPNet{*excludedIPNet}, + PermittedIPRanges: []net.IPNet{*permittedIPNet}, + PermittedEmailAddresses: []string{"email@email.org"}, + ExcludedIPRanges: []net.IPNet{*excludedIPNet}, } asn1NameConstraints, err := asn1.Marshal(nameConstraints) if err != nil { @@ -685,9 +685,13 @@ func TestSignCSRTemplate(t *testing.T) { // for that, we construct a chain of four certificates: // a root CA, two intermediate CA, and a leaf certificate. - mustCreatePair := func(issuerCert *x509.Certificate, issuerPK crypto.Signer, name string, isCA bool) ([]byte, *x509.Certificate, *x509.Certificate, crypto.Signer) { + mustCreatePair := func(issuerCert *x509.Certificate, issuerPK crypto.Signer, name string, isCA bool, nameConstraints *NameConstraints) ([]byte, *x509.Certificate, *x509.Certificate, crypto.Signer) { pk, err := GenerateECPrivateKey(256) require.NoError(t, err) + var permittedIPRanges []*net.IPNet + if nameConstraints != nil { + permittedIPRanges = ConvertIPNeSliceToIPNetPointerSlice(nameConstraints.PermittedIPRanges) + } tmpl := &x509.Certificate{ Version: 3, BasicConstraintsValid: true, @@ -701,6 +705,7 @@ func TestSignCSRTemplate(t *testing.T) { KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, PublicKey: pk.Public(), IsCA: isCA, + PermittedIPRanges: permittedIPRanges, } if isCA { @@ -719,10 +724,16 @@ func TestSignCSRTemplate(t *testing.T) { return pem, cert, tmpl, pk } - rootPEM, rootCert, rootTmpl, rootPK := mustCreatePair(nil, nil, "root", true) - int1PEM, int1Cert, int1Tmpl, int1PK := mustCreatePair(rootCert, rootPK, "int1", true) - int2PEM, int2Cert, int2Tmpl, int2PK := mustCreatePair(int1Cert, int1PK, "int2", true) - leafPEM, _, leafTmpl, _ := mustCreatePair(int2Cert, int2PK, "leaf", false) + rootPEM, rootCert, rootTmpl, rootPK := mustCreatePair(nil, nil, "root", true, nil) + int1PEM, int1Cert, int1Tmpl, int1PK := mustCreatePair(rootCert, rootPK, "int1", true, nil) + int2PEM, int2Cert, int2Tmpl, int2PK := mustCreatePair(int1Cert, int1PK, "int2", true, nil) + leafPEM, _, leafTmpl, _ := mustCreatePair(int2Cert, int2PK, "leaf", false, nil) + + // vars for testing name constraints + _, permittedIPNet, _ := net.ParseCIDR("10.10.0.0/16") + _, ncRootCert, _, ncRootPK := mustCreatePair(nil, nil, "ncroot", true, &NameConstraints{PermittedIPRanges: []net.IPNet{*permittedIPNet}}) + _, _, ncLeafTmpl, _ := mustCreatePair(ncRootCert, ncRootPK, "ncleaf", false, nil) + ncLeafTmpl.IPAddresses = []net.IP{net.ParseIP("10.20.0.5")} tests := map[string]struct { caCerts []*x509.Certificate @@ -769,6 +780,12 @@ func TestSignCSRTemplate(t *testing.T) { template: rootTmpl, wantErr: true, }, + "Error: Non-compliance with NameConstraints in the leaf certificate.": { + caCerts: []*x509.Certificate{ncRootCert}, + caKey: ncRootPK, + template: ncLeafTmpl, + wantErr: true, + }, } for name, test := range tests { diff --git a/pkg/util/pki/nameconstraints.go b/pkg/util/pki/nameconstraints.go index 6e680f0e7..44131c5c9 100644 --- a/pkg/util/pki/nameconstraints.go +++ b/pkg/util/pki/nameconstraints.go @@ -32,15 +32,15 @@ var ( // NameConstraints represents the NameConstraints extension. type NameConstraints struct { - PermittedDNSDomainsCritical bool `asn1:"optional,explicit,tag:0"` - PermittedDNSDomains []string `asn1:"optional,explicit,tag:1"` - ExcludedDNSDomains []string `asn1:"optional,explicit,tag:2"` + PermittedDNSDomainsCritical bool `asn1:"optional,explicit,tag:0"` + PermittedDNSDomains []string `asn1:"optional,explicit,tag:1"` + ExcludedDNSDomains []string `asn1:"optional,explicit,tag:2"` PermittedIPRanges []net.IPNet `asn1:"optional,explicit,tag:3"` ExcludedIPRanges []net.IPNet `asn1:"optional,explicit,tag:4"` - PermittedEmailAddresses []string `asn1:"optional,explicit,tag:5"` - ExcludedEmailAddresses []string `asn1:"optional,explicit,tag:6"` - PermittedURIDomains []string `asn1:"optional,explicit,tag:7"` - ExcludedURIDomains []string `asn1:"optional,explicit,tag:8"` + PermittedEmailAddresses []string `asn1:"optional,explicit,tag:5"` + ExcludedEmailAddresses []string `asn1:"optional,explicit,tag:6"` + PermittedURIDomains []string `asn1:"optional,explicit,tag:7"` + ExcludedURIDomains []string `asn1:"optional,explicit,tag:8"` } // Adapted from x509.go @@ -54,10 +54,10 @@ func MarshalNameConstraints(nameConstraints *v1.NameConstraints) (pkix.Extension } nameConstraintsForMarshalling = NameConstraints{ PermittedDNSDomainsCritical: nameConstraints.Permitted.Critical, - PermittedDNSDomains: nameConstraints.Permitted.DNSDomains, - PermittedIPRanges: permittedIPRanges, - PermittedEmailAddresses: nameConstraints.Permitted.EmailAddresses, - PermittedURIDomains: nameConstraints.Permitted.URIDomains, + PermittedDNSDomains: nameConstraints.Permitted.DNSDomains, + PermittedIPRanges: permittedIPRanges, + PermittedEmailAddresses: nameConstraints.Permitted.EmailAddresses, + PermittedURIDomains: nameConstraints.Permitted.URIDomains, } } @@ -78,7 +78,7 @@ func MarshalNameConstraints(nameConstraints *v1.NameConstraints) (pkix.Extension func parseCIDRs(cidrs []string) ([]net.IPNet, error) { ipRanges := []net.IPNet{} - for _, cidr := range(cidrs) { + for _, cidr := range cidrs { _, ipNet, err := net.ParseCIDR(cidr) ipRanges = append(ipRanges, net.IPNet{ IP: ipNet.IP, @@ -105,7 +105,10 @@ func UnmarshalNameConstraints(value []byte) (NameConstraints, error) { } // ConvertIPNeSliceToIPNetPointerSlice converts []net.IPNet to []*net.IPNet. -func ConvertIPNeSliceToIPNetPointerSlice(ipNetPointerSlice []net.IPNet) ([]*net.IPNet) { +func ConvertIPNeSliceToIPNetPointerSlice(ipNetPointerSlice []net.IPNet) []*net.IPNet { + if ipNetPointerSlice == nil { + return nil + } var ipNets []*net.IPNet for _, ipNet := range ipNetPointerSlice { ipNets = append(ipNets, &ipNet)