From 849b6bda9e4907d8a4f1de04d62f463c43b9ae36 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Tue, 12 Dec 2023 15:57:07 +0100 Subject: [PATCH] add tests & final cleanup Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- pkg/util/pki/certificatetemplate.go | 2 +- pkg/util/pki/nameconstraints.go | 27 +++++++++---------- pkg/util/pki/nameconstraints_test.go | 39 ++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 16 deletions(-) diff --git a/pkg/util/pki/certificatetemplate.go b/pkg/util/pki/certificatetemplate.go index fe27efad2..b3bda76e3 100644 --- a/pkg/util/pki/certificatetemplate.go +++ b/pkg/util/pki/certificatetemplate.go @@ -194,7 +194,7 @@ func CertificateTemplateFromCSR(csr *x509.CertificateRequest, validatorMutators } if val.Id.Equal(OIDExtensionNameConstraints) { - nameConstraints, err := UnmarshalNameConstraints(val) + nameConstraints, err := UnmarshalNameConstraints(val.Value) if err != nil { return err } diff --git a/pkg/util/pki/nameconstraints.go b/pkg/util/pki/nameconstraints.go index 490cc4afa..b5d0a6817 100644 --- a/pkg/util/pki/nameconstraints.go +++ b/pkg/util/pki/nameconstraints.go @@ -182,8 +182,7 @@ func parseCIDRs(cidrs []string) ([]*net.IPNet, error) { } // Adapted from crypto/x509/parser.go -func UnmarshalNameConstraints(e pkix.Extension) (*NameConstraints, error) { - out := &NameConstraints{} +func UnmarshalNameConstraints(value []byte) (*NameConstraints, error) { // RFC 5280, 4.2.1.10 // NameConstraints ::= SEQUENCE { @@ -199,7 +198,7 @@ func UnmarshalNameConstraints(e pkix.Extension) (*NameConstraints, error) { // // BaseDistance ::= INTEGER (0..MAX) - outer := cryptobyte.String(e.Value) + outer := cryptobyte.String(value) var toplevel, permitted, excluded cryptobyte.String var havePermitted, haveExcluded bool if !outer.ReadASN1(&toplevel, cryptobyte_asn1.SEQUENCE) || @@ -207,7 +206,7 @@ func UnmarshalNameConstraints(e pkix.Extension) (*NameConstraints, error) { !toplevel.ReadOptionalASN1(&permitted, &havePermitted, cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) || !toplevel.ReadOptionalASN1(&excluded, &haveExcluded, cryptobyte_asn1.Tag(1).ContextSpecific().Constructed()) || !toplevel.Empty() { - return out, errors.New("x509: invalid NameConstraints extension") + return nil, errors.New("x509: invalid NameConstraints extension") } if !havePermitted && !haveExcluded || len(permitted) == 0 && len(excluded) == 0 { @@ -215,7 +214,7 @@ func UnmarshalNameConstraints(e pkix.Extension) (*NameConstraints, error) { // “either the permittedSubtrees field // or the excludedSubtrees MUST be // present” - return out, errors.New("x509: empty name constraints extension") + return nil, errors.New("x509: empty name constraints extension") } getValues := func(subtrees cryptobyte.String) (dnsNames []string, ips []*net.IPNet, emails, uriDomains []string, err error) { @@ -248,13 +247,13 @@ func UnmarshalNameConstraints(e pkix.Extension) (*NameConstraints, error) { var ip, mask []byte switch l { - case 8: - ip = value[:4] - mask = value[4:] + case 2 * net.IPv4len: + ip = value[:net.IPv4len] + mask = value[net.IPv4len:] - case 32: - ip = value[:16] - mask = value[16:] + case 2 * net.IPv6len: + ip = value[:net.IPv6len] + mask = value[net.IPv6len:] default: return nil, nil, nil, nil, fmt.Errorf("x509: IP constraint contained value of length %d", l) @@ -287,12 +286,14 @@ func UnmarshalNameConstraints(e pkix.Extension) (*NameConstraints, error) { return dnsNames, ips, emails, uriDomains, nil } + out := &NameConstraints{} + var err error if out.PermittedDNSDomains, out.PermittedIPRanges, out.PermittedEmailAddresses, out.PermittedURIDomains, err = getValues(permitted); err != nil { - return out, err + return nil, err } if out.ExcludedDNSDomains, out.ExcludedIPRanges, out.ExcludedEmailAddresses, out.ExcludedURIDomains, err = getValues(excluded); err != nil { - return out, err + return nil, err } return out, nil diff --git a/pkg/util/pki/nameconstraints_test.go b/pkg/util/pki/nameconstraints_test.go index c1645dd6e..309a5ec46 100644 --- a/pkg/util/pki/nameconstraints_test.go +++ b/pkg/util/pki/nameconstraints_test.go @@ -17,6 +17,7 @@ limitations under the License. package pki import ( + "bytes" "crypto/x509" "crypto/x509/pkix" "encoding/pem" @@ -41,7 +42,7 @@ import ( // [req_ext] // 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 -func TestMarshalNameConstraints(t *testing.T) { +func TestMarshalUnmarshalNameConstraints(t *testing.T) { // Test data testCases := []struct { name string @@ -141,8 +142,22 @@ mYfy24EOPhpvyIyYS+lbkc9wdYT4BSIjQCFNAjcBD+/04SkHgtbFLy0i8xsKcfOy }, } + compareIPArrays := func(a, b []*net.IPNet) bool { + if len(a) != len(b) { + return false + } + + for i, ipNet := range a { + if !ipNet.IP.Equal(b[i].IP) || !bytes.Equal(ipNet.Mask, b[i].Mask) { + return false + } + } + + return true + } + for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { + t.Run(tc.name+"_marshal", func(t *testing.T) { expectedResult, err := getExtensionFromPem(tc.expectedPEM) assert.NoError(t, err) result, err := MarshalNameConstraints(tc.input, expectedResult.Critical) @@ -156,6 +171,26 @@ mYfy24EOPhpvyIyYS+lbkc9wdYT4BSIjQCFNAjcBD+/04SkHgtbFLy0i8xsKcfOy assert.Equal(t, expectedResult.Value, result.Value) } }) + + t.Run(tc.name+"_unmarshal", func(t *testing.T) { + expectedResult, err := getExtensionFromPem(tc.expectedPEM) + assert.NoError(t, err) + constraints, err := UnmarshalNameConstraints(expectedResult.Value) + if tc.expectedErr != nil { + assert.Error(t, err) + assert.EqualError(t, err, tc.expectedErr.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, constraints.ExcludedDNSDomains, tc.input.ExcludedDNSDomains) + assert.Equal(t, constraints.ExcludedEmailAddresses, tc.input.ExcludedEmailAddresses) + assert.True(t, compareIPArrays(constraints.ExcludedIPRanges, tc.input.ExcludedIPRanges)) + assert.Equal(t, constraints.ExcludedURIDomains, tc.input.ExcludedURIDomains) + assert.Equal(t, constraints.PermittedDNSDomains, tc.input.PermittedDNSDomains) + assert.Equal(t, constraints.PermittedEmailAddresses, tc.input.PermittedEmailAddresses) + assert.True(t, compareIPArrays(constraints.PermittedIPRanges, tc.input.PermittedIPRanges)) + assert.Equal(t, constraints.PermittedURIDomains, tc.input.PermittedURIDomains) + } + }) } }