diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index 9c1af3d4b..1d3708f1c 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -320,7 +320,7 @@ func GenerateCSR(crt *v1.Certificate, optFuncs ...GenerateCSROption) (*x509.Cert if opts.EncodeNameConstraintsInRequest && crt.Spec.NameConstraints != nil { nameConstraints := &NameConstraints{} - nameConstraints.PermittedDNSDomainsCritical = crt.Spec.NameConstraints.Critical + if crt.Spec.NameConstraints.Permitted != nil { nameConstraints.PermittedDNSDomains = crt.Spec.NameConstraints.Permitted.DNSDomains nameConstraints.PermittedIPRanges, err = parseCIDRs(crt.Spec.NameConstraints.Permitted.IPRanges) @@ -340,11 +340,15 @@ func GenerateCSR(crt *v1.Certificate, optFuncs ...GenerateCSROption) (*x509.Cert nameConstraints.ExcludedEmailAddresses = crt.Spec.NameConstraints.Excluded.EmailAddresses nameConstraints.ExcludedURIDomains = crt.Spec.NameConstraints.Excluded.URIDomains } - extension, err := MarshalNameConstraints(nameConstraints) - if err != nil { - return nil, err + + if !nameConstraints.IsEmpty() { + extension, err := MarshalNameConstraints(nameConstraints, crt.Spec.NameConstraints.Critical) + if err != nil { + return nil, err + } + + extraExtensions = append(extraExtensions, extension) } - extraExtensions = append(extraExtensions, extension) } cr := &x509.CertificateRequest{ diff --git a/pkg/util/pki/nameconstraints.go b/pkg/util/pki/nameconstraints.go index a3bb3d563..490cc4afa 100644 --- a/pkg/util/pki/nameconstraints.go +++ b/pkg/util/pki/nameconstraints.go @@ -34,128 +34,125 @@ var ( // NameConstraints represents the NameConstraints extension. type NameConstraints struct { - PermittedDNSDomainsCritical bool - PermittedDNSDomains []string - ExcludedDNSDomains []string - PermittedIPRanges []*net.IPNet - ExcludedIPRanges []*net.IPNet - PermittedEmailAddresses []string - ExcludedEmailAddresses []string - PermittedURIDomains []string - ExcludedURIDomains []string + PermittedDNSDomains []string + ExcludedDNSDomains []string + PermittedIPRanges []*net.IPNet + ExcludedIPRanges []*net.IPNet + PermittedEmailAddresses []string + ExcludedEmailAddresses []string + PermittedURIDomains []string + ExcludedURIDomains []string +} + +func (nc NameConstraints) IsEmpty() bool { + return len(nc.PermittedDNSDomains) == 0 && + len(nc.PermittedIPRanges) == 0 && + len(nc.PermittedEmailAddresses) == 0 && + len(nc.PermittedURIDomains) == 0 && + len(nc.ExcludedDNSDomains) == 0 && + len(nc.ExcludedIPRanges) == 0 && + len(nc.ExcludedEmailAddresses) == 0 && + len(nc.ExcludedURIDomains) == 0 } // Adapted from x509.go -func MarshalNameConstraints(nameConstraints *NameConstraints) (pkix.Extension, error) { - ext := pkix.Extension{} - if doMarshalNameConstraints(nameConstraints) { - ext.Id = OIDExtensionNameConstraints - ext.Critical = nameConstraints.PermittedDNSDomainsCritical - - ipAndMask := func(ipNet *net.IPNet) []byte { - maskedIP := ipNet.IP.Mask(ipNet.Mask) - ipAndMask := make([]byte, 0, len(maskedIP)+len(ipNet.Mask)) - ipAndMask = append(ipAndMask, maskedIP...) - ipAndMask = append(ipAndMask, ipNet.Mask...) - return ipAndMask - } - - serialiseConstraints := func(dns []string, ips []*net.IPNet, emails []string, uriDomains []string) (der []byte, err error) { - var b cryptobyte.Builder - - for _, name := range dns { - if err = isIA5String(name); err != nil { - return nil, err - } - - b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { - b.AddASN1(cryptobyte_asn1.Tag(2).ContextSpecific(), func(b *cryptobyte.Builder) { - b.AddBytes([]byte(name)) - }) - }) - } - - for _, ipNet := range ips { - b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { - b.AddASN1(cryptobyte_asn1.Tag(7).ContextSpecific(), func(b *cryptobyte.Builder) { - b.AddBytes(ipAndMask(ipNet)) - }) - }) - } - - for _, email := range emails { - if err = isIA5String(email); err != nil { - return nil, err - } - - b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { - b.AddASN1(cryptobyte_asn1.Tag(1).ContextSpecific(), func(b *cryptobyte.Builder) { - b.AddBytes([]byte(email)) - }) - }) - } - - for _, uriDomain := range uriDomains { - if err = isIA5String(uriDomain); err != nil { - return nil, err - } - - b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { - b.AddASN1(cryptobyte_asn1.Tag(6).ContextSpecific(), func(b *cryptobyte.Builder) { - b.AddBytes([]byte(uriDomain)) - }) - }) - } - - return b.Bytes() - } - - var permitted []byte - var err error - permitted, err = serialiseConstraints(nameConstraints.PermittedDNSDomains, nameConstraints.PermittedIPRanges, nameConstraints.PermittedEmailAddresses, nameConstraints.PermittedURIDomains) - if err != nil { - return pkix.Extension{}, err - } - - var excluded []byte - excluded, err = serialiseConstraints(nameConstraints.ExcludedDNSDomains, nameConstraints.ExcludedIPRanges, nameConstraints.ExcludedEmailAddresses, nameConstraints.ExcludedURIDomains) - if err != nil { - return pkix.Extension{}, err - } - - var b cryptobyte.Builder - b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { - if len(permitted) > 0 { - b.AddASN1(cryptobyte_asn1.Tag(0).ContextSpecific().Constructed(), func(b *cryptobyte.Builder) { - b.AddBytes(permitted) - }) - } - - if len(excluded) > 0 { - b.AddASN1(cryptobyte_asn1.Tag(1).ContextSpecific().Constructed(), func(b *cryptobyte.Builder) { - b.AddBytes(excluded) - }) - } - }) - - ext.Value, err = b.Bytes() - if err != nil { - return pkix.Extension{}, err - } +func MarshalNameConstraints(nameConstraints *NameConstraints, critical bool) (pkix.Extension, error) { + ipAndMask := func(ipNet *net.IPNet) []byte { + maskedIP := ipNet.IP.Mask(ipNet.Mask) + ipAndMask := make([]byte, 0, len(maskedIP)+len(ipNet.Mask)) + ipAndMask = append(ipAndMask, maskedIP...) + ipAndMask = append(ipAndMask, ipNet.Mask...) + return ipAndMask } - return ext, nil -} -func doMarshalNameConstraints(nameConstraints *NameConstraints) bool { - return nameConstraints != nil && - (len(nameConstraints.PermittedDNSDomains) > 0 || - len(nameConstraints.PermittedIPRanges) > 0 || - len(nameConstraints.PermittedEmailAddresses) > 0 || - len(nameConstraints.PermittedURIDomains) > 0 || - len(nameConstraints.ExcludedDNSDomains) > 0 || - len(nameConstraints.ExcludedIPRanges) > 0 || - len(nameConstraints.ExcludedEmailAddresses) > 0 || - len(nameConstraints.ExcludedURIDomains) > 0) + serialiseConstraints := func(dns []string, ips []*net.IPNet, emails []string, uriDomains []string) (der []byte, err error) { + var b cryptobyte.Builder + + for _, name := range dns { + if err = isIA5String(name); err != nil { + return nil, err + } + + b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { + b.AddASN1(cryptobyte_asn1.Tag(2).ContextSpecific(), func(b *cryptobyte.Builder) { + b.AddBytes([]byte(name)) + }) + }) + } + + for _, ipNet := range ips { + b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { + b.AddASN1(cryptobyte_asn1.Tag(7).ContextSpecific(), func(b *cryptobyte.Builder) { + b.AddBytes(ipAndMask(ipNet)) + }) + }) + } + + for _, email := range emails { + if err = isIA5String(email); err != nil { + return nil, err + } + + b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { + b.AddASN1(cryptobyte_asn1.Tag(1).ContextSpecific(), func(b *cryptobyte.Builder) { + b.AddBytes([]byte(email)) + }) + }) + } + + for _, uriDomain := range uriDomains { + if err = isIA5String(uriDomain); err != nil { + return nil, err + } + + b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { + b.AddASN1(cryptobyte_asn1.Tag(6).ContextSpecific(), func(b *cryptobyte.Builder) { + b.AddBytes([]byte(uriDomain)) + }) + }) + } + + return b.Bytes() + } + + var permitted []byte + var err error + permitted, err = serialiseConstraints(nameConstraints.PermittedDNSDomains, nameConstraints.PermittedIPRanges, nameConstraints.PermittedEmailAddresses, nameConstraints.PermittedURIDomains) + if err != nil { + return pkix.Extension{}, err + } + + var excluded []byte + excluded, err = serialiseConstraints(nameConstraints.ExcludedDNSDomains, nameConstraints.ExcludedIPRanges, nameConstraints.ExcludedEmailAddresses, nameConstraints.ExcludedURIDomains) + if err != nil { + return pkix.Extension{}, err + } + + var b cryptobyte.Builder + b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { + if len(permitted) > 0 { + b.AddASN1(cryptobyte_asn1.Tag(0).ContextSpecific().Constructed(), func(b *cryptobyte.Builder) { + b.AddBytes(permitted) + }) + } + + if len(excluded) > 0 { + b.AddASN1(cryptobyte_asn1.Tag(1).ContextSpecific().Constructed(), func(b *cryptobyte.Builder) { + b.AddBytes(excluded) + }) + } + }) + + bytes, err := b.Bytes() + if err != nil { + return pkix.Extension{}, err + } + + return pkix.Extension{ + Id: OIDExtensionNameConstraints, + Critical: critical, + Value: bytes, + }, nil } func isIA5String(s string) error { @@ -297,7 +294,6 @@ func UnmarshalNameConstraints(e pkix.Extension) (*NameConstraints, error) { if out.ExcludedDNSDomains, out.ExcludedIPRanges, out.ExcludedEmailAddresses, out.ExcludedURIDomains, err = getValues(excluded); err != nil { return out, err } - out.PermittedDNSDomainsCritical = e.Critical return out, nil } diff --git a/pkg/util/pki/nameconstraints_test.go b/pkg/util/pki/nameconstraints_test.go index ed8e53a21..c1645dd6e 100644 --- a/pkg/util/pki/nameconstraints_test.go +++ b/pkg/util/pki/nameconstraints_test.go @@ -52,11 +52,10 @@ func TestMarshalNameConstraints(t *testing.T) { { name: "Permitted constraints", input: &NameConstraints{ - PermittedDNSDomainsCritical: true, - PermittedDNSDomains: []string{"example.com"}, - PermittedIPRanges: []*net.IPNet{{IP: net.IPv4(192, 168, 1, 0), Mask: net.IPv4Mask(255, 255, 255, 0)}}, - PermittedEmailAddresses: []string{"user@example.com"}, - PermittedURIDomains: []string{"https://example.com"}, + PermittedDNSDomains: []string{"example.com"}, + PermittedIPRanges: []*net.IPNet{{IP: net.IPv4(192, 168, 1, 0), Mask: net.IPv4Mask(255, 255, 255, 0)}}, + PermittedEmailAddresses: []string{"user@example.com"}, + PermittedURIDomains: []string{"https://example.com"}, }, expectedErr: nil, // nameConstraints = critical,permitted;DNS:example.com,permitted;IP:192.168.1.0/255.255.255.0,permitted;email:user@example.com,permitted;URI:https://example.com @@ -81,15 +80,14 @@ Nu6OGP4KFgW0HWyeGeNBzioGUeyIHFKILLvj2n94WJMqXNyT5eE= { name: "Mixed constraints", input: &NameConstraints{ - PermittedDNSDomainsCritical: true, - PermittedDNSDomains: []string{"example.com"}, - PermittedIPRanges: []*net.IPNet{{IP: net.IPv4(192, 168, 1, 0), Mask: net.IPv4Mask(255, 255, 255, 0)}}, - PermittedEmailAddresses: []string{"user@example.com"}, - PermittedURIDomains: []string{"https://example.com"}, - ExcludedDNSDomains: []string{"excluded.com"}, - ExcludedIPRanges: []*net.IPNet{{IP: net.IPv4(192, 168, 0, 0), Mask: net.IPv4Mask(255, 255, 255, 0)}}, - ExcludedEmailAddresses: []string{"user@excluded.com"}, - ExcludedURIDomains: []string{"https://excluded.com"}, + PermittedDNSDomains: []string{"example.com"}, + PermittedIPRanges: []*net.IPNet{{IP: net.IPv4(192, 168, 1, 0), Mask: net.IPv4Mask(255, 255, 255, 0)}}, + PermittedEmailAddresses: []string{"user@example.com"}, + PermittedURIDomains: []string{"https://example.com"}, + ExcludedDNSDomains: []string{"excluded.com"}, + ExcludedIPRanges: []*net.IPNet{{IP: net.IPv4(192, 168, 0, 0), Mask: net.IPv4Mask(255, 255, 255, 0)}}, + ExcludedEmailAddresses: []string{"user@excluded.com"}, + ExcludedURIDomains: []string{"https://excluded.com"}, }, expectedErr: nil, // nameConstraints = critical,permitted;DNS:example.com,permitted;IP:192.168.1.0/255.255.255.0,permitted;email:user@example.com,permitted;URI:https://example.com,excluded;DNS:excluded.com,excluded;IP:192.168.0.0/255.255.255.0,excluded;email:user@excluded.com,excluded;URI:https://excluded.com @@ -113,20 +111,13 @@ AHpUq+yDI0oaIz6BIfn2Vs7jUSXCZIoQBwajALg9kGqh3O6+ds617+AzxGXk0LBQ 0GsHVWCimOgcqgU5Qg4K6iMUtlDU2WAW -----END CERTIFICATE REQUEST-----`, }, - { - name: "Empty constraints", - input: &NameConstraints{}, - expectedErr: nil, - expectedPEM: "", - }, { name: "Excluded constraints", input: &NameConstraints{ - PermittedDNSDomainsCritical: true, - ExcludedDNSDomains: []string{"excluded.com"}, - ExcludedIPRanges: []*net.IPNet{{IP: net.IPv4(192, 168, 0, 0), Mask: net.IPv4Mask(255, 255, 255, 0)}}, - ExcludedEmailAddresses: []string{"user@excluded.com"}, - ExcludedURIDomains: []string{"https://excluded.com"}, + ExcludedDNSDomains: []string{"excluded.com"}, + ExcludedIPRanges: []*net.IPNet{{IP: net.IPv4(192, 168, 0, 0), Mask: net.IPv4Mask(255, 255, 255, 0)}}, + ExcludedEmailAddresses: []string{"user@excluded.com"}, + ExcludedURIDomains: []string{"https://excluded.com"}, }, expectedErr: nil, // nameConstraints = critical,excluded;DNS:excluded.com,excluded;IP:192.168.0.0/255.255.255.0,excluded;email:user@excluded.com,excluded;URI:https://excluded.com @@ -154,7 +145,7 @@ mYfy24EOPhpvyIyYS+lbkc9wdYT4BSIjQCFNAjcBD+/04SkHgtbFLy0i8xsKcfOy t.Run(tc.name, func(t *testing.T) { expectedResult, err := getExtensionFromPem(tc.expectedPEM) assert.NoError(t, err) - result, err := MarshalNameConstraints(tc.input) + result, err := MarshalNameConstraints(tc.input, expectedResult.Critical) if tc.expectedErr != nil { assert.Error(t, err) assert.EqualError(t, err, tc.expectedErr.Error())