From 9d1c959a1eaf7d649e9fcfe27410b3870b9b174d Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Fri, 10 May 2024 09:27:50 +0200 Subject: [PATCH 1/2] LiteralSubject: add support for literal oid type values Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- pkg/util/pki/subject.go | 13 ++++++++++++- pkg/util/pki/subject_test.go | 25 ++++++++----------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/pkg/util/pki/subject.go b/pkg/util/pki/subject.go index d7b14625f..87ebe7f06 100644 --- a/pkg/util/pki/subject.go +++ b/pkg/util/pki/subject.go @@ -79,8 +79,19 @@ func UnmarshalSubjectStringToRDNSequence(subject string) (pkix.RDNSequence, erro atvs := make([]pkix.AttributeTypeAndValue, 0, len(ldapRelativeDN.Attributes)) for _, ldapATV := range ldapRelativeDN.Attributes { + oid, ok := attributeTypeNames[ldapATV.Type] + if !ok { + // If the attribute type is not known, we try to parse it as an OID. + // If it is not an OID, we set Type=nil + + oid, err = ParseObjectIdentifier(ldapATV.Type) + if err != nil { + oid = nil + } + } + atvs = append(atvs, pkix.AttributeTypeAndValue{ - Type: attributeTypeNames[ldapATV.Type], + Type: oid, Value: ldapATV.Value, }) } diff --git a/pkg/util/pki/subject_test.go b/pkg/util/pki/subject_test.go index 10d3f387e..1a701741c 100644 --- a/pkg/util/pki/subject_test.go +++ b/pkg/util/pki/subject_test.go @@ -18,13 +18,14 @@ package pki import ( "crypto/x509/pkix" + "encoding/asn1" "testing" "github.com/stretchr/testify/assert" ) func TestMustParseRDN(t *testing.T) { - subject := "SERIALNUMBER=42, L=some-locality, ST=some-state-or-province, STREET=some-street, CN=foo-long.com, OU=FooLong, OU=Barq, OU=Baz, OU=Dept., O=Corp., C=US" + subject := "SERIALNUMBER=42, L=some-locality, ST=some-state-or-province, STREET=some-street, CN=foo-long.com, OU=FooLong, OU=Barq, OU=Baz, OU=Dept., O=Corp., C=US+123.544.555= A Test Value " rdnSeq, err := UnmarshalSubjectStringToRDNSequence(subject) if err != nil { t.Fatal(err) @@ -34,6 +35,7 @@ func TestMustParseRDN(t *testing.T) { pkix.RDNSequence{ []pkix.AttributeTypeAndValue{ {Type: OIDConstants.Country, Value: "US"}, + {Type: asn1.ObjectIdentifier{123, 544, 555}, Value: "A Test Value"}, }, []pkix.AttributeTypeAndValue{ {Type: OIDConstants.Organization, Value: "Corp."}, @@ -139,7 +141,7 @@ func TestRoundTripRDNSequence(t *testing.T) { }, { []pkix.AttributeTypeAndValue{ - {Type: OIDConstants.CommonName, Value: "fo\x00o-long.com"}, + {Type: asn1.ObjectIdentifier{0, 5, 80, 99, 58962185}, Value: "fo\x00o-long.com"}, }, }, } @@ -170,18 +172,6 @@ func FuzzRoundTripRDNSequence(f *testing.F) { t.Skip() } - // See pkix.go for the list of known attribute types - var knownMarshalTypes = map[string]bool{ - "2.5.4.6": true, - "2.5.4.10": true, - "2.5.4.11": true, - "2.5.4.3": true, - "2.5.4.5": true, - "2.5.4.7": true, - "2.5.4.8": true, - "2.5.4.9": true, - "2.5.4.17": true, - } hasSpecialChar := func(s string) bool { for _, char := range s { if char < ' ' || char > '~' { @@ -192,9 +182,10 @@ func FuzzRoundTripRDNSequence(f *testing.F) { } for _, rdn := range rdnSeq { for _, tv := range rdn { - // Skip if the String() function will return a literal OID type, as we - // do not yet support parsing these. - if _, ok := knownMarshalTypes[tv.Type.String()]; !ok { + // Skip if the Type was not recognized. The String() output will be + // an invalid type, value pair with empty type, which will give a "DN ended with + // an incomplete type, value pair" error when parsing. + if tv.Type.String() == "" { t.Skip() } From 0a452989710971700b7a29a14cb100e18c4cd415 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Fri, 10 May 2024 20:43:54 +0200 Subject: [PATCH 2/2] improve tests based on review Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- pkg/util/pki/subject_test.go | 61 +++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/pkg/util/pki/subject_test.go b/pkg/util/pki/subject_test.go index 1a701741c..33e0c8b19 100644 --- a/pkg/util/pki/subject_test.go +++ b/pkg/util/pki/subject_test.go @@ -120,39 +120,56 @@ func TestShouldFailForHexDER(t *testing.T) { // TestRoundTripRDNSequence tests a set of RDNSequences to ensure that they are // the same after a round trip through String() and UnmarshalSubjectStringToRDNSequence(). func TestRoundTripRDNSequence(t *testing.T) { - rdnSequences := []pkix.RDNSequence{ + type testCase struct { + name string + rdn pkix.RDNSequence + } + rdnSequences := []testCase{ { - []pkix.AttributeTypeAndValue{ - {Type: OIDConstants.Organization, Value: "Corp."}, - {Type: OIDConstants.OrganizationalUnit, Value: "FooLong"}, + name: "Simple RDNSequence", + rdn: pkix.RDNSequence{ + []pkix.AttributeTypeAndValue{ + {Type: OIDConstants.Organization, Value: "Corp."}, + {Type: OIDConstants.OrganizationalUnit, Value: "FooLong"}, + }, }, }, { - []pkix.AttributeTypeAndValue{ - {Type: OIDConstants.CommonName, Value: "foo-lon❤️\\g.com "}, - {Type: OIDConstants.OrganizationalUnit, Value: "Foo===Long"}, - {Type: OIDConstants.OrganizationalUnit, Value: "Ba rq"}, - {Type: OIDConstants.OrganizationalUnit, Value: "Baz"}, - }, - []pkix.AttributeTypeAndValue{ - {Type: OIDConstants.Organization, Value: "C; orp."}, - {Type: OIDConstants.Country, Value: "US"}, + name: "Character Escaping", + rdn: pkix.RDNSequence{ + []pkix.AttributeTypeAndValue{ + {Type: OIDConstants.CommonName, Value: "foo-lon❤️\\g.com "}, + {Type: OIDConstants.OrganizationalUnit, Value: "Foo===Long"}, + {Type: OIDConstants.OrganizationalUnit, Value: "Ba rq"}, + {Type: OIDConstants.OrganizationalUnit, Value: "Baz"}, + {Type: OIDConstants.Country, Value: "fo\x00o-long.com"}, + }, + []pkix.AttributeTypeAndValue{ + {Type: OIDConstants.Organization, Value: "C; orp."}, + {Type: OIDConstants.Country, Value: "US"}, + }, }, }, { - []pkix.AttributeTypeAndValue{ - {Type: asn1.ObjectIdentifier{0, 5, 80, 99, 58962185}, Value: "fo\x00o-long.com"}, + name: "Numeric OID", + rdn: pkix.RDNSequence{ + []pkix.AttributeTypeAndValue{ + {Type: asn1.ObjectIdentifier{0, 5, 80, 99, 58962185}, Value: "String Value"}, + }, }, }, } - for _, rdnSeq := range rdnSequences { - newRDNSeq, err := UnmarshalSubjectStringToRDNSequence(rdnSeq.String()) - if err != nil { - t.Fatal(err) - } + for _, tc := range rdnSequences { + tc := tc + t.Run(tc.name, func(t *testing.T) { + newRDNSeq, err := UnmarshalSubjectStringToRDNSequence(tc.rdn.String()) + if err != nil { + t.Fatal(err) + } - assert.Equal(t, rdnSeq, newRDNSeq) + assert.Equal(t, tc.rdn, newRDNSeq) + }) } } @@ -164,6 +181,8 @@ func FuzzRoundTripRDNSequence(f *testing.F) { f.Add("CN=foo-long.com,OU=FooLong,OU=Barq,OU=Baz,OU=Dept.,O=Corp.,C=US") f.Add("CN=foo-lon❤️\\,g.com,OU=Foo===Long,OU=Ba # rq,OU=Baz,O=C\\; orp.,C=US") f.Add("CN=fo\x00o-long.com,OU=\x04FooLong") + f.Add("1.2.3.4=String Value") + f.Add("1.3.6.1.4.1.1466.0=#04024869") f.Fuzz(func(t *testing.T, subjectString string) { t.Parallel()