From 15f62d67542a94a3bee6406001a51c388d83f4a6 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 5 Aug 2022 14:57:05 -0700 Subject: [PATCH 01/13] Add initial support for permanent identifier SAN Fixes #53 --- x509util/extensions.go | 116 +++++++++++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 38 deletions(-) diff --git a/x509util/extensions.go b/x509util/extensions.go index 3a78a10f..0b6a8a87 100644 --- a/x509util/extensions.go +++ b/x509util/extensions.go @@ -53,15 +53,16 @@ var ( // Names used and SubjectAlternativeNames types. const ( - AutoType = "auto" - EmailType = "email" // also known as 'rfc822Name' in RFC 5280 - DNSType = "dns" - X400AddressType = "x400Address" - DirectoryNameType = "dn" - EDIPartyNameType = "ediPartyName" - URIType = "uri" - IPType = "ip" - RegisteredIDType = "registeredID" + AutoType = "auto" + EmailType = "email" // also known as 'rfc822Name' in RFC 5280 + DNSType = "dns" + X400AddressType = "x400Address" + DirectoryNameType = "dn" + EDIPartyNameType = "ediPartyName" + URIType = "uri" + IPType = "ip" + RegisteredIDType = "registeredID" + PermanentIdentifierType = "permanentIdentifier" ) // These type ids are defined in RFC 5280 page 36 @@ -82,17 +83,33 @@ const ( // provided. const sanTypeSeparator = ":" -// OtherNameValue is a simple struct to ensure the ASN1 marshaller creates an -// EXPLICIT asn1 type when creating the otherName -type OtherNameValue struct { - V interface{} +// RFC 4043 - https://datatracker.ietf.org/doc/html/rfc4043 +var oidPermanentIdentifier = []int{1, 3, 6, 1, 5, 5, 7, 8, 3} + +// OtherName ::= SEQUENCE { +// type-id OBJECT IDENTIFIER, +// value [0] EXPLICIT ANY DEFINED BY type-id } +type otherName struct { + TypeID asn1.ObjectIdentifier + Value asn1.RawValue } -// OtherName holds a SubjectAlternativeName type otherName as defined in RFC -// 5280. -type OtherName struct { - OID asn1.ObjectIdentifier - Value OtherNameValue `asn1:"tag:0"` +// PermanentIdentifier is defined in RFC 4043 as an optional feature that +// may be used by a CA to indicate that two or more certificates relate to the +// same entity. +// +// In device attestation this SAN will contain the UDID (Unique Device +// IDentifier) or serial number of the device. +// +// See https://tools.ietf.org/html/rfc4043 +// +// PermanentIdentifier ::= SEQUENCE { +// identifierValue UTF8String OPTIONAL, +// assigner OBJECT IDENTIFIER OPTIONAL +// } +type PermanentIdentifier struct { + IdentifierValue string `asn1:"utf8,optional"` + Assigner asn1.ObjectIdentifier `asn1:"optional"` } // Extension is the JSON representation of a raw X.509 extensions. @@ -263,9 +280,17 @@ func (s SubjectAlternativeName) RawValue() (asn1.RawValue, error) { } rawBytes, err := asn1.MarshalWithParams(oid, fmt.Sprintf("tag:%d", nameTypeRegisteredID)) if err != nil { - return zero, errors.Wrap(err, "unable to Marshal RegisteredID SAN") + return zero, errors.Wrap(err, "error marshaling RegisteredID SAN") } return asn1.RawValue{FullBytes: rawBytes}, nil + case PermanentIdentifierType: + otherName, err := marshalOtherName(oidPermanentIdentifier, PermanentIdentifier{ + IdentifierValue: s.Value, + }) + if err != nil { + return zero, errors.Wrap(err, "error marshaling PermanentIdentifier SAN") + } + return otherName, nil case X400AddressType, DirectoryNameType, EDIPartyNameType: return zero, fmt.Errorf("unimplemented SAN type %s", s.Type) default: @@ -283,21 +308,18 @@ func (s SubjectAlternativeName) RawValue() (asn1.RawValue, error) { value = value[len(params)+1:] } - rawBytes, err := marshalOtherName(value, params) + rawBytes, err := marshalExplicitValue(value, params) if err != nil { return zero, errors.Wrapf(err, "error marshaling ASN1 value %q", s.Value) } - // OtherName SANs are an ASN1 sequence containing OID and Value - otherName := OtherName{ - OID: oid, - Value: OtherNameValue{ - V: asn1.RawValue{FullBytes: rawBytes}, //load in the raw OtherName value - }, + otherName := otherName{ + TypeID: oid, + Value: asn1.RawValue{FullBytes: rawBytes}, } // use MarshalWithParams so we can set the context-specific tag - in this case 0 - otherNameBytes, err := asn1.MarshalWithParams(otherName, fmt.Sprintf("tag:%d", nameTypeOtherName)) + otherNameBytes, err := asn1.MarshalWithParams(otherName, "tag:0") if err != nil { return zero, errors.Wrap(err, "unable to Marshal otherName SAN") } @@ -305,28 +327,46 @@ func (s SubjectAlternativeName) RawValue() (asn1.RawValue, error) { } } -// marshalOtherName marshals the given value with given type and returns the raw -// bytes to use. +// marshalOtherName marshals an otherName field with the given oid and value and +// returns the raw bytes to use. +func marshalOtherName(oid asn1.ObjectIdentifier, value interface{}) (asn1.RawValue, error) { + valueBytes, err := asn1.MarshalWithParams(value, "explicit,tag:0") + if err != nil { + return asn1.RawValue{}, err + } + otherName := otherName{ + TypeID: oid, + Value: asn1.RawValue{FullBytes: valueBytes}, + } + bytes, err := asn1.MarshalWithParams(otherName, "tag:0") + if err != nil { + return asn1.RawValue{}, err + } + return asn1.RawValue{FullBytes: bytes}, nil +} + +// marshalExplicitValue marshals the given value with given type and returns the +// raw bytes to use. // -// The OtherName value can be any type depending on the OID ASN supports a great +// The return value value can be any type depending on the OID ASN supports a great // number of formats, but Golang's ans1 package supports much fewer -- for now // support anything the golang asn1 marshaller supports. // // See https://www.openssl.org/docs/man1.0.2/man3/ASN1_generate_nconf.html -func marshalOtherName(value, typ string) ([]byte, error) { +func marshalExplicitValue(value, typ string) ([]byte, error) { switch typ { case "int": i, err := strconv.Atoi(value) if err != nil { return nil, errors.Wrap(err, "invalid int value") } - return asn1.Marshal(i) + return asn1.MarshalWithParams(i, "explicit") case "oid": oid, err := parseObjectIdentifier(value) if err != nil { return nil, errors.Wrap(err, "invalid oid value") } - return asn1.Marshal(oid) + return asn1.MarshalWithParams(oid, "explicit") case "raw": // the raw type accepts a base64 encoded byte array which is passed unaltered into the ASN // marshaller. By using this type users can add ASN1 data types manually into templates @@ -336,29 +376,29 @@ func marshalOtherName(value, typ string) ([]byte, error) { if !isUTF8String(value) { return nil, fmt.Errorf("invalid utf8 value") } - return asn1.MarshalWithParams(value, typ) + return asn1.MarshalWithParams(value, "explicit,utf8") case "ia5": if !isIA5String(value) { return nil, fmt.Errorf("invalid ia5 value") } - return asn1.MarshalWithParams(value, typ) + return asn1.MarshalWithParams(value, "explicit,ia5") case "numeric": if !isNumericString(value) { return nil, fmt.Errorf("invalid numeric value") } - return asn1.MarshalWithParams(value, typ) + return asn1.MarshalWithParams(value, "explicit,numeric") case "printable": if !isPrintableString(value, true, true) { return nil, fmt.Errorf("invalid printable value") } - return asn1.MarshalWithParams(value, typ) + return asn1.MarshalWithParams(value, "explicit,printable") default: // if it's an unknown type, default to printable - but use the entire // value specified in case there is a semicolon in the value if !isPrintableString(value, true, true) { return nil, fmt.Errorf("invalid printable value") } - return asn1.MarshalWithParams(value, "printable") + return asn1.MarshalWithParams(value, "explicit,printable") } } From 521c8389bdfa2472b2566ebd9f27c28c442c572f Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 5 Aug 2022 16:32:01 -0700 Subject: [PATCH 02/13] Add unit tests for SubjectAlternativeName.RawValue() --- x509util/extensions_test.go | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/x509util/extensions_test.go b/x509util/extensions_test.go index 0b106c69..a8594eb6 100644 --- a/x509util/extensions_test.go +++ b/x509util/extensions_test.go @@ -239,6 +239,74 @@ func TestSubjectAlternativeName_Set(t *testing.T) { } } +func TestSubjectAlternativeName_RawValue(t *testing.T) { + type fields struct { + Type string + Value string + } + tests := []struct { + name string + fields fields + want asn1.RawValue + wantErr bool + }{ + {"ip", fields{"auto", "1.1.1.1"}, asn1.RawValue{Class: 2, Tag: 7, Bytes: []byte{1, 1, 1, 1}}, false}, + {"uri", fields{"auto", "urn:smallstep:1234"}, asn1.RawValue{Class: 2, Tag: 6, Bytes: []byte("urn:smallstep:1234")}, false}, + {"email", fields{"auto", "foo@bar.com"}, asn1.RawValue{Class: 2, Tag: 1, Bytes: []byte("foo@bar.com")}, false}, + {"dns", fields{"auto", "bar.com"}, asn1.RawValue{Class: 2, Tag: 2, Bytes: []byte("bar.com")}, false}, + {"registeredID", fields{"registeredID", "1.2.3.4"}, asn1.RawValue{ + // Class 2, Type: 8 + FullBytes: []byte{(2 << 6) | 8, 3, 0x20 | 1<<3 | 2, 3, 4}, + }, false}, + {"permanentIdentifier", fields{"permanentIdentifier", "0123456789"}, asn1.RawValue{ + FullBytes: append(append( + []byte{160, 26, 6, 8, 43, 6, 1, 5, 5, 7, 8, 3}, + []byte{160, 14, 48, 12, 12, 10}...), + []byte("0123456789")...), + }, false}, + {"otherName int", fields{"1.2.3.4", "int:1024"}, asn1.RawValue{ + FullBytes: []byte{160, 11, 6, 3, 42, 3, 4, 160, 4, 2, 2, 4, 0}, + }, false}, + {"otherName oid", fields{"1.2.3.4", "oid:1.2.3.4"}, asn1.RawValue{ + FullBytes: []byte{160, 12, 6, 3, 42, 3, 4, 160, 5, 6, 3, 42, 3, 4}, + }, false}, + {"otherName raw", fields{"1.2.3.4", "raw:MTIzNA=="}, asn1.RawValue{ + FullBytes: append([]byte{160, 9, 6, 3, 42, 3, 4}, []byte("1234")...), + }, false}, + {"otherName utf8", fields{"1.2.3.4", "utf8:á∫ç1234"}, asn1.RawValue{ + FullBytes: append([]byte{160, 20, 6, 3, 42, 3, 4, 160, 13, 12, 11}, []byte("á∫ç1234")...), + }, false}, + {"otherName ia5", fields{"1.2.3.4", "ia5:abc1234"}, asn1.RawValue{ + FullBytes: append([]byte{160, 16, 6, 3, 42, 3, 4, 160, 9, 22, 7}, []byte("abc1234")...), + }, false}, + {"otherName numeric", fields{"1.2.3.4", "numeric:1024"}, asn1.RawValue{ + FullBytes: append([]byte{160, 13, 6, 3, 42, 3, 4, 160, 6, 18, 4}, []byte("1024")...), + }, false}, + {"otherName printable", fields{"1.2.3.4", "printable:abc1234"}, asn1.RawValue{ + FullBytes: append([]byte{160, 16, 6, 3, 42, 3, 4, 160, 9, 19, 7}, []byte("abc1234")...), + }, false}, + {"otherName default", fields{"1.2.3.4", "foo:abc1234"}, asn1.RawValue{ + FullBytes: append([]byte{160, 16, 6, 3, 42, 3, 4, 160, 9, 19, 7}, []byte("abc1234")...), + }, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := SubjectAlternativeName{ + Type: tt.fields.Type, + Value: tt.fields.Value, + } + got, err := s.RawValue() + if (err != nil) != tt.wantErr { + t.Errorf("SubjectAlternativeName.RawValue() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("SubjectAlternativeName.RawValue() = %v, want %v", got, tt.want) + } + }) + } +} + func TestKeyUsage_Set(t *testing.T) { type args struct { c *x509.Certificate From 9201589e06ab7b6745461a9734e3f77644c112b9 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 8 Aug 2022 11:53:02 -0700 Subject: [PATCH 03/13] Copy TestSanitizeName to x509util --- x509util/utils_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/x509util/utils_test.go b/x509util/utils_test.go index 6167e650..e3403808 100644 --- a/x509util/utils_test.go +++ b/x509util/utils_test.go @@ -227,3 +227,32 @@ func Test_generateSubjectKeyID(t *testing.T) { }) } } + +func TestSanitizeName(t *testing.T) { + type args struct { + domain string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + {"ok", args{"smallstep.com"}, "smallstep.com", false}, + {"ok ascii", args{"bücher.example.com"}, "xn--bcher-kva.example.com", false}, + {"fail", args{"xn--bücher.example.com"}, "", true}, + {"fail empty", args{""}, "", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := SanitizeName(tt.args.domain) + if (err != nil) != tt.wantErr { + t.Errorf("SanitizeName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("SanitizeName() = %v, want %v", got, tt.want) + } + }) + } +} From 523cb9e2fa25db8ec8447ec1928ebcaec72ae2d2 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 8 Aug 2022 12:20:35 -0700 Subject: [PATCH 04/13] Add fail unit tests for RawValue --- x509util/extensions.go | 2 +- x509util/extensions_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/x509util/extensions.go b/x509util/extensions.go index 0b6a8a87..5b13d5f1 100644 --- a/x509util/extensions.go +++ b/x509util/extensions.go @@ -278,7 +278,7 @@ func (s SubjectAlternativeName) RawValue() (asn1.RawValue, error) { if err != nil { return zero, errors.Wrap(err, "error parsing OID for RegisteredID SAN") } - rawBytes, err := asn1.MarshalWithParams(oid, fmt.Sprintf("tag:%d", nameTypeRegisteredID)) + rawBytes, err := asn1.MarshalWithParams(oid, "tag:8") if err != nil { return zero, errors.Wrap(err, "error marshaling RegisteredID SAN") } diff --git a/x509util/extensions_test.go b/x509util/extensions_test.go index a8594eb6..36f4e502 100644 --- a/x509util/extensions_test.go +++ b/x509util/extensions_test.go @@ -251,6 +251,7 @@ func TestSubjectAlternativeName_RawValue(t *testing.T) { wantErr bool }{ {"ip", fields{"auto", "1.1.1.1"}, asn1.RawValue{Class: 2, Tag: 7, Bytes: []byte{1, 1, 1, 1}}, false}, + {"ipv6", fields{"auto", "2001:0db8:0000:0000:0000:ff00:0042:8329"}, asn1.RawValue{Class: 2, Tag: 7, Bytes: []byte{0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0xff, 0, 0, 0x42, 0x83, 0x29}}, false}, {"uri", fields{"auto", "urn:smallstep:1234"}, asn1.RawValue{Class: 2, Tag: 6, Bytes: []byte("urn:smallstep:1234")}, false}, {"email", fields{"auto", "foo@bar.com"}, asn1.RawValue{Class: 2, Tag: 1, Bytes: []byte("foo@bar.com")}, false}, {"dns", fields{"auto", "bar.com"}, asn1.RawValue{Class: 2, Tag: 2, Bytes: []byte("bar.com")}, false}, @@ -288,6 +289,32 @@ func TestSubjectAlternativeName_RawValue(t *testing.T) { {"otherName default", fields{"1.2.3.4", "foo:abc1234"}, asn1.RawValue{ FullBytes: append([]byte{160, 16, 6, 3, 42, 3, 4, 160, 9, 19, 7}, []byte("abc1234")...), }, false}, + {"otherName no type", fields{"1.2.3.4", "abc1234"}, asn1.RawValue{ + FullBytes: append([]byte{160, 16, 6, 3, 42, 3, 4, 160, 9, 19, 7}, []byte("abc1234")...), + }, false}, + {"fail dn", fields{"dn", "1234"}, asn1.RawValue{}, true}, + {"fail x400Address", fields{"x400Address", "1234"}, asn1.RawValue{}, true}, + {"fail ediPartyName", fields{"ediPartyName", "1234"}, asn1.RawValue{}, true}, + {"fail email", fields{"email", "nöt@ia5.com"}, asn1.RawValue{}, true}, + {"fail dns", fields{"dns", "xn--bücher.example.com"}, asn1.RawValue{}, true}, + {"fail dns empty", fields{"dns", ""}, asn1.RawValue{}, true}, + {"fail uri", fields{"uri", "urn:nöt:ia5"}, asn1.RawValue{}, true}, + {"fail ip", fields{"ip", "1.2.3.4.5"}, asn1.RawValue{}, true}, + {"fail registeredID", fields{"registeredID", "4.3.2.1"}, asn1.RawValue{}, true}, + {"fail registeredID empty", fields{"registeredID", ""}, asn1.RawValue{}, true}, + {"fail registeredID parse", fields{"registeredID", "a.b.c.d"}, asn1.RawValue{}, true}, + {"fail otherName parse", fields{"a.b.c.d", "foo"}, asn1.RawValue{}, true}, + {"fail otherName marshal", fields{"1", "foo"}, asn1.RawValue{}, true}, + {"fail otherName int", fields{"1.2.3.4", "int:abc"}, asn1.RawValue{}, true}, + {"fail otherName oid", fields{"1.2.3.4", "oid:4.3.2.1"}, asn1.RawValue{}, true}, + {"fail otherName oid parse", fields{"1.2.3.4", "oid:a.b.c.d"}, asn1.RawValue{}, true}, + {"fail otherName raw", fields{"1.2.3.4", "raw:abc"}, asn1.RawValue{}, true}, + {"fail otherName utf8", fields{"1.2.3.4", "utf8:\xff"}, asn1.RawValue{}, true}, + {"fail otherName ia5", fields{"1.2.3.4", "ia5:nötia5"}, asn1.RawValue{}, true}, + {"fail otherName numeric", fields{"1.2.3.4", "numeric:abc"}, asn1.RawValue{}, true}, + {"fail otherName printable", fields{"1.2.3.4", "printable:nötprintable"}, asn1.RawValue{}, true}, + {"fail otherName default", fields{"1.2.3.4", "foo:nötprintable"}, asn1.RawValue{}, true}, + {"fail otherName no type", fields{"1.2.3.4", "nötprintable"}, asn1.RawValue{}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 9d763da8156567e21026533ae3511fa84a67650c Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 8 Aug 2022 12:28:58 -0700 Subject: [PATCH 05/13] Use Go 1.19 and test with 1.18 and 1.19 --- .github/workflows/release.yml | 4 ++-- .github/workflows/test.yml | 6 +++--- go.mod | 2 +- go.sum | 4 ---- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b8cfcdee..c4ac70da 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - go: [ '1.17', '1.18' ] + go: [ '1.18', '1.19' ] steps: - name: Install dependencies run: sudo apt update && sudo apt install -y libpcsclite-dev @@ -25,7 +25,7 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v2 with: - version: 'v1.46.2' + version: 'v1.48' args: --timeout=30m - name: Test run: V=1 make ci diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f3d91a23..2d7c2f29 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - go: [ '1.17', '1.18' ] + go: [ '1.18', '1.19' ] steps: - name: Install dependencies run: sudo apt update && sudo apt install -y libpcsclite-dev @@ -27,13 +27,13 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v2 with: - version: 'v1.46.2' + version: 'v1.48' args: --timeout=30m - name: Test run: V=1 make ci - name: Codecov uses: codecov/codecov-action@v1.2.1 - if: matrix.go == '1.18' + if: matrix.go == '1.19' with: file: ./coverage.out name: codecov-umbrella diff --git a/go.mod b/go.mod index 81e34cec..15ff905a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module go.step.sm/crypto -go 1.17 +go 1.18 require ( cloud.google.com/go/kms v1.4.0 diff --git a/go.sum b/go.sum index 4c1642f7..23426d8c 100644 --- a/go.sum +++ b/go.sum @@ -306,7 +306,6 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= -github.com/yuin/goldmark v1.4.0/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= @@ -399,7 +398,6 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLdyRGr576XBO4/greRjx4P4O3yc= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= @@ -488,7 +486,6 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210806184541-e5e7981a1069/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210908233432-aa78b53d3365/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211031064116-611d5d643895/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -572,7 +569,6 @@ golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.1.7/go.mod h1:LGqMHiF4EqQNHR1JncWGqT5BVaXmza+X+BDGol+dOxo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From e3fb60f857ee365cbaf89394159d86dd7e9542c7 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 8 Aug 2022 12:35:21 -0700 Subject: [PATCH 06/13] Fix linter errors --- internal/bcrypt_pbkdf/bcrypt_pbkdf.go | 1 + internal/bcrypt_pbkdf/bcrypt_pbkdf_test.go | 1 + internal/utils/io_test.go | 3 +- jose/generate_test.go | 5 +- jose/types.go | 2 + tlsutil/renewer.go | 1 + x25519/x25519.go | 64 +++++++++++----------- x509util/extensions.go | 42 +++++++------- 8 files changed, 61 insertions(+), 58 deletions(-) diff --git a/internal/bcrypt_pbkdf/bcrypt_pbkdf.go b/internal/bcrypt_pbkdf/bcrypt_pbkdf.go index 3d2dda7a..f7067622 100644 --- a/internal/bcrypt_pbkdf/bcrypt_pbkdf.go +++ b/internal/bcrypt_pbkdf/bcrypt_pbkdf.go @@ -4,6 +4,7 @@ // Package bcrypt_pbkdf implements password-based key derivation function based // on bcrypt compatible with bcrypt_pbkdf(3) from OpenBSD. +// //nolint:revive // ignore underscore in package package bcrypt_pbkdf diff --git a/internal/bcrypt_pbkdf/bcrypt_pbkdf_test.go b/internal/bcrypt_pbkdf/bcrypt_pbkdf_test.go index d7f5e6e6..0c2d9f53 100644 --- a/internal/bcrypt_pbkdf/bcrypt_pbkdf_test.go +++ b/internal/bcrypt_pbkdf/bcrypt_pbkdf_test.go @@ -1,6 +1,7 @@ // Copyright 2014 Dmitry Chestnykh. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +// //nolint:revive // ignore underscore in package package bcrypt_pbkdf diff --git a/internal/utils/io_test.go b/internal/utils/io_test.go index 20062a35..c22bbc6d 100644 --- a/internal/utils/io_test.go +++ b/internal/utils/io_test.go @@ -2,7 +2,6 @@ package utils import ( "fmt" - "io/ioutil" "os" "path/filepath" "reflect" @@ -67,7 +66,7 @@ func TestReadPasswordFromFile(t *testing.T) { } func TestWriteFile(t *testing.T) { - tmpDir, err := ioutil.TempDir(os.TempDir(), "go-tests") + tmpDir, err := os.MkdirTemp(os.TempDir(), "go-tests") if err != nil { t.Fatal(err) } diff --git a/jose/generate_test.go b/jose/generate_test.go index cc32b574..9f069769 100644 --- a/jose/generate_test.go +++ b/jose/generate_test.go @@ -9,7 +9,6 @@ import ( "crypto/x509" "encoding/json" "encoding/pem" - "io/ioutil" "math/big" "os" "reflect" @@ -357,8 +356,8 @@ func newCert(t *testing.T, keyUsage x509.KeyUsage) []byte { return cert } -func tempFile(t *testing.T) (_ *os.File, cleanup func()) { - f, err := ioutil.TempFile("" /* use default tmp dir */, "jose-generate-test") +func tempFile(t *testing.T) (*os.File, func()) { + f, err := os.CreateTemp("", "jose-generate-test") assert.NoError(t, err) return f, func() { f.Close() diff --git a/jose/types.go b/jose/types.go index 07f6b96d..bb141561 100644 --- a/jose/types.go +++ b/jose/types.go @@ -122,6 +122,7 @@ var ErrInvalidID = jwt.ErrInvalidID var ErrIssuedInTheFuture = jwt.ErrIssuedInTheFuture // Key management algorithms +// //nolint:revive // use standard names in upper-case const ( RSA1_5 = KeyAlgorithm("RSA1_5") // RSA-PKCS1v1.5 @@ -162,6 +163,7 @@ const ( ) // Content encryption algorithms +// //nolint:revive // use standard names in upper-case const ( A128CBC_HS256 = ContentEncryption("A128CBC-HS256") // AES-CBC + HMAC-SHA256 (128) diff --git a/tlsutil/renewer.go b/tlsutil/renewer.go index 5690df5f..2617f8fb 100644 --- a/tlsutil/renewer.go +++ b/tlsutil/renewer.go @@ -17,6 +17,7 @@ type RenewFunc func() (*tls.Certificate, *tls.Config, error) var MinCertDuration = time.Minute // Renewer automatically renews a tls certificate using a RenewFunc. +// //nolint:gocritic // ignore exposedSyncMutex type Renewer struct { sync.RWMutex diff --git a/x25519/x25519.go b/x25519/x25519.go index 5e3d2b60..452cd31e 100644 --- a/x25519/x25519.go +++ b/x25519/x25519.go @@ -110,13 +110,13 @@ func (p PrivateKey) Sign(rand io.Reader, message []byte, opts crypto.SignerOpts) // It implements the XEdDSA sign method defined in // https://signal.org/docs/specifications/xeddsa/#xeddsa // -// xeddsa_sign(k, M, Z): -// A, a = calculate_key_pair(k) -// r = hash1(a || M || Z) (mod q) -// R = rB -// h = hash(R || A || M) (mod q) -// s = r + ha (mod q) -// return R || s +// xeddsa_sign(k, M, Z): +// A, a = calculate_key_pair(k) +// r = hash1(a || M || Z) (mod q) +// R = rB +// h = hash(R || A || M) (mod q) +// s = r + ha (mod q) +// return R || s func Sign(rand io.Reader, p PrivateKey, message []byte) (signature []byte, err error) { if l := len(p); l != PrivateKeySize { panic("x25519: bad private key length: " + strconv.Itoa(l)) @@ -184,17 +184,17 @@ func Sign(rand io.Reader, p PrivateKey, message []byte) (signature []byte, err e // It implements the XEdDSA verify method defined in // https://signal.org/docs/specifications/xeddsa/#xeddsa // -// xeddsa_verify(u, M, (R || s)): -// if u >= p or R.y >= 2|p| or s >= 2|q|: -// return false -// A = convert_mont(u) -// if not on_curve(A): -// return false -// h = hash(R || A || M) (mod q) -// Rcheck = sB - hA -// if bytes_equal(R, Rcheck): -// return true -// return false +// xeddsa_verify(u, M, (R || s)): +// if u >= p or R.y >= 2|p| or s >= 2|q|: +// return false +// A = convert_mont(u) +// if not on_curve(A): +// return false +// h = hash(R || A || M) (mod q) +// Rcheck = sB - hA +// if bytes_equal(R, Rcheck): +// return true +// return false func Verify(publicKey PublicKey, message, sig []byte) bool { // The following code should be equivalent to: // @@ -242,15 +242,15 @@ func Verify(publicKey PublicKey, message, sig []byte) bool { // public key and private key (A, a) as defined in // https://signal.org/docs/specifications/xeddsa/#elliptic-curve-conversions // -// calculate_key_pair(k): -// E = kB -// A.y = E.y -// A.s = 0 -// if E.s == 1: -// a = -k (mod q) -// else: -// a = k (mod q) -// return A, a +// calculate_key_pair(k): +// E = kB +// A.y = E.y +// A.s = 0 +// if E.s == 1: +// a = -k (mod q) +// else: +// a = k (mod q) +// return A, a func (p PrivateKey) calculateKeyPair() ([]byte, *edwards25519.Scalar, error) { var pA edwards25519.Point var sa edwards25519.Scalar @@ -278,11 +278,11 @@ func (p PrivateKey) calculateKeyPair() ([]byte, *edwards25519.Scalar, error) { // point P, according to // https://signal.org/docs/specifications/xeddsa/#elliptic-curve-conversions // -// convert_mont(u): -// umasked = u (mod 2|p|) -// P.y = u_to_y(umasked) -// P.s = 0 -// return P +// convert_mont(u): +// umasked = u (mod 2|p|) +// P.y = u_to_y(umasked) +// P.s = 0 +// return P func convertMont(u PublicKey) (*edwards25519.Point, error) { um, err := (&field.Element{}).SetBytes(u) if err != nil { diff --git a/x509util/extensions.go b/x509util/extensions.go index 5b13d5f1..2ce19855 100644 --- a/x509util/extensions.go +++ b/x509util/extensions.go @@ -66,13 +66,14 @@ const ( ) // These type ids are defined in RFC 5280 page 36 +// nolint:deadcode // ignore const ( - nameTypeOtherName = 0 - nameTypeEmail = 1 - nameTypeDNS = 2 - //nameTypeX400 = 3 - //nameTypeDirectory = 4 - //nameTypeEDI = 5 + nameTypeOtherName = 0 + nameTypeEmail = 1 + nameTypeDNS = 2 + nameTypeX400 = 3 + nameTypeDirectory = 4 + nameTypeEDI = 5 nameTypeURI = 6 nameTypeIP = 7 nameTypeRegisteredID = 8 @@ -86,9 +87,11 @@ const sanTypeSeparator = ":" // RFC 4043 - https://datatracker.ietf.org/doc/html/rfc4043 var oidPermanentIdentifier = []int{1, 3, 6, 1, 5, 5, 7, 8, 3} -// OtherName ::= SEQUENCE { -// type-id OBJECT IDENTIFIER, -// value [0] EXPLICIT ANY DEFINED BY type-id } +// RFC 5280 - https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6 +// +// OtherName ::= SEQUENCE { +// type-id OBJECT IDENTIFIER, +// value [0] EXPLICIT ANY DEFINED BY type-id } type otherName struct { TypeID asn1.ObjectIdentifier Value asn1.RawValue @@ -103,10 +106,10 @@ type otherName struct { // // See https://tools.ietf.org/html/rfc4043 // -// PermanentIdentifier ::= SEQUENCE { -// identifierValue UTF8String OPTIONAL, -// assigner OBJECT IDENTIFIER OPTIONAL -// } +// PermanentIdentifier ::= SEQUENCE { +// identifierValue UTF8String OPTIONAL, +// assigner OBJECT IDENTIFIER OPTIONAL +// } type PermanentIdentifier struct { IdentifierValue string `asn1:"utf8,optional"` Assigner asn1.ObjectIdentifier `asn1:"optional"` @@ -313,13 +316,11 @@ func (s SubjectAlternativeName) RawValue() (asn1.RawValue, error) { return zero, errors.Wrapf(err, "error marshaling ASN1 value %q", s.Value) } - otherName := otherName{ + // use MarshalWithParams so we can set the context-specific tag - in this case 0 + otherNameBytes, err := asn1.MarshalWithParams(otherName{ TypeID: oid, Value: asn1.RawValue{FullBytes: rawBytes}, - } - - // use MarshalWithParams so we can set the context-specific tag - in this case 0 - otherNameBytes, err := asn1.MarshalWithParams(otherName, "tag:0") + }, "tag:0") if err != nil { return zero, errors.Wrap(err, "unable to Marshal otherName SAN") } @@ -334,11 +335,10 @@ func marshalOtherName(oid asn1.ObjectIdentifier, value interface{}) (asn1.RawVal if err != nil { return asn1.RawValue{}, err } - otherName := otherName{ + bytes, err := asn1.MarshalWithParams(otherName{ TypeID: oid, Value: asn1.RawValue{FullBytes: valueBytes}, - } - bytes, err := asn1.MarshalWithParams(otherName, "tag:0") + }, "tag:0") if err != nil { return asn1.RawValue{}, err } From eaf28a000b4750be2b2bf820904cebf3bf083c79 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 9 Aug 2022 17:32:43 -0700 Subject: [PATCH 07/13] Add method to return if subject is empty --- x509util/name.go | 51 ++++++++++++++++++++++------------------ x509util/name_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 23 deletions(-) diff --git a/x509util/name.go b/x509util/name.go index e0cde298..8f7c87af 100644 --- a/x509util/name.go +++ b/x509util/name.go @@ -1,6 +1,7 @@ package x509util import ( + "bytes" "crypto/x509" "crypto/x509/pkix" "encoding/asn1" @@ -57,6 +58,22 @@ func newName(n pkix.Name) Name { } } +// goValue converts Subject to the Go representation. +func (n Name) goValue() pkix.Name { + return pkix.Name{ + Country: n.Country, + Organization: n.Organization, + OrganizationalUnit: n.OrganizationalUnit, + Locality: n.Locality, + Province: n.Province, + StreetAddress: n.StreetAddress, + PostalCode: n.PostalCode, + SerialNumber: n.SerialNumber, + CommonName: n.CommonName, + ExtraNames: fromDistinguishedNames(n.ExtraNames), + } +} + // UnmarshalJSON implements the json.Unmarshal interface and unmarshals a JSON // object in the Name struct or a string as just the subject common name. func (n *Name) UnmarshalJSON(data []byte) error { @@ -94,18 +111,17 @@ func (s *Subject) UnmarshalJSON(data []byte) error { // Set sets the subject in the given certificate. func (s Subject) Set(c *x509.Certificate) { - c.Subject = pkix.Name{ - Country: s.Country, - Organization: s.Organization, - OrganizationalUnit: s.OrganizationalUnit, - Locality: s.Locality, - Province: s.Province, - StreetAddress: s.StreetAddress, - PostalCode: s.PostalCode, - SerialNumber: s.SerialNumber, - CommonName: s.CommonName, - ExtraNames: fromDistinguishedNames(s.ExtraNames), + c.Subject = Name(s).goValue() +} + +// IsEmpty returns if the subject is empty. Certificates with an empty subject +// must have the subjectAltName extension mark as critical. +func (s Subject) IsEmpty() bool { + subject := Name(s).goValue() + if asn1Subject, err := asn1.Marshal(subject.ToRDNSequence()); err == nil { + return bytes.Equal(asn1Subject, emptyASN1Subject) } + return false } // Issuer is the JSON representation of the X.509 issuer field. @@ -128,18 +144,7 @@ func (i *Issuer) UnmarshalJSON(data []byte) error { // Set sets the issuer in the given certificate. func (i Issuer) Set(c *x509.Certificate) { - c.Issuer = pkix.Name{ - Country: i.Country, - Organization: i.Organization, - OrganizationalUnit: i.OrganizationalUnit, - Locality: i.Locality, - Province: i.Province, - StreetAddress: i.StreetAddress, - PostalCode: i.PostalCode, - SerialNumber: i.SerialNumber, - CommonName: i.CommonName, - ExtraNames: fromDistinguishedNames(i.ExtraNames), - } + c.Issuer = Name(i).goValue() } // DistinguishedName mirrors the ASN.1 structure AttributeTypeAndValue in RFC diff --git a/x509util/name_test.go b/x509util/name_test.go index 6c017b6e..31e25ad7 100644 --- a/x509util/name_test.go +++ b/x509util/name_test.go @@ -288,6 +288,60 @@ func TestSubject_Set(t *testing.T) { } } +func TestSubject_IsEmpty(t *testing.T) { + type fields struct { + Country MultiString + Organization MultiString + OrganizationalUnit MultiString + Locality MultiString + Province MultiString + StreetAddress MultiString + PostalCode MultiString + SerialNumber string + CommonName string + ExtraNames []DistinguishedName + } + tests := []struct { + name string + fields fields + want bool + }{ + {"ok", fields{}, true}, + {"country", fields{Country: []string{"The country"}}, false}, + {"commonName", fields{CommonName: "The commonName"}, false}, + {"all fields", fields{ + Country: []string{"The country"}, + Organization: []string{"The organization"}, + OrganizationalUnit: []string{"The organizationalUnit 1", "The organizationalUnit 2"}, + Locality: []string{"The locality 1", "The locality 2"}, + Province: []string{"The province"}, + StreetAddress: []string{"The streetAddress"}, + PostalCode: []string{"The postalCode"}, + SerialNumber: "The serialNumber", + CommonName: "The commonName", + }, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := Subject{ + Country: tt.fields.Country, + Organization: tt.fields.Organization, + OrganizationalUnit: tt.fields.OrganizationalUnit, + Locality: tt.fields.Locality, + Province: tt.fields.Province, + StreetAddress: tt.fields.StreetAddress, + PostalCode: tt.fields.PostalCode, + SerialNumber: tt.fields.SerialNumber, + CommonName: tt.fields.CommonName, + ExtraNames: tt.fields.ExtraNames, + } + if got := s.IsEmpty(); got != tt.want { + t.Errorf("Subject.IsEmpty() = %v, want %v", got, tt.want) + } + }) + } +} + func Test_newIssuer(t *testing.T) { type args struct { n pkix.Name From 890392ceb3e7b9bca28e93eb842e0cffa737062a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 9 Aug 2022 18:13:20 -0700 Subject: [PATCH 08/13] Avoid panic creating SAN extension on constructor --- x509util/certificate.go | 24 +++++++++++++++--------- x509util/certificate_test.go | 32 +++++++++++++++++++++++++++++++- x509util/extensions.go | 22 +++++++++++----------- 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/x509util/certificate.go b/x509util/certificate.go index 9336b77d..09b17d8c 100644 --- a/x509util/certificate.go +++ b/x509util/certificate.go @@ -68,6 +68,18 @@ func NewCertificate(cr *x509.CertificateRequest, opts ...Option) (*Certificate, cert.PublicKey = cr.PublicKey cert.PublicKeyAlgorithm = cr.PublicKeyAlgorithm + // Generate the subjectAltName extension if has SANs not directly supported + // by Go. + if cert.hasExtendedSANs() && !cert.hasExtension(oidExtensionSubjectAltName) { + ext, err := createSubjectAltNameExtension(&cert, cert.Subject.IsEmpty()) + if err != nil { + return nil, err + } + // Prepend extension to achieve a certificate as similar as possible to + // the one generated by the standard Go library. + cert.Extensions = append([]Extension{ext}, cert.Extensions...) + } + return &cert, nil } @@ -83,15 +95,9 @@ func (c *Certificate) GetCertificate() *x509.Certificate { // Subject c.Subject.Set(cert) - if c.hasExtendedSANs() && !c.hasExtension(oidExtensionSubjectAltName) { - subjectAltNameExtension, err := createSubjectAltNameExtension(c, subjectIsEmpty(cert.Subject)) - if err != nil { - panic(err) - } - subjectAltNameExtension.Set(cert) - } else { - // When we have no extended SANs, use the golang x509 lib to create the - // extension instead + // When we have no extended SANs, use the golang x509 lib to create the + // extension instead + if !c.hasExtension(oidExtensionSubjectAltName) { cert.DNSNames = c.DNSNames cert.EmailAddresses = c.EmailAddresses cert.IPAddresses = c.IPAddresses diff --git a/x509util/certificate_test.go b/x509util/certificate_test.go index 5ec1bf21..6d41a647 100644 --- a/x509util/certificate_test.go +++ b/x509util/certificate_test.go @@ -110,6 +110,16 @@ func TestNewCertificate(t *testing.T) { crBadSignateure, _ := createCertificateRequest(t, "fail", []string{"foo.com"}) crBadSignateure.PublicKey = priv.Public() + customSANsData := CreateTemplateData("commonName", nil) + customSANsData.Set(SANsKey, []SubjectAlternativeName{ + {Type: PermanentIdentifierType, Value: "123456"}, + {Type: "1.2.3.4", Value: "utf8:otherName"}, + }) + badCustomSANsData := CreateTemplateData("commonName", nil) + badCustomSANsData.Set(SANsKey, []SubjectAlternativeName{ + {Type: "1.2.3.4", Value: "int:not-an-int"}, + }) + ipNet := func(s string) *net.IPNet { _, ipNet, err := net.ParseCIDR(s) if err != nil { @@ -153,6 +163,25 @@ func TestNewCertificate(t *testing.T) { PublicKey: priv.Public(), PublicKeyAlgorithm: x509.Ed25519, }, false}, + {"okCustomSANs", args{cr, []Option{WithTemplate(DefaultLeafTemplate, customSANsData)}}, &Certificate{ + Subject: Subject{CommonName: "commonName"}, + SANs: []SubjectAlternativeName{ + {Type: PermanentIdentifierType, Value: "123456"}, + {Type: "1.2.3.4", Value: "utf8:otherName"}, + }, + Extensions: []Extension{{ + ID: ObjectIdentifier{2, 5, 29, 17}, + Critical: false, + Value: []byte{48, 44, 160, 22, 6, 8, 43, 6, 1, 5, 5, 7, 8, 3, 160, 10, 48, 8, 12, 6, 49, 50, 51, 52, 53, 54, 160, 18, 6, 3, 42, 3, 4, 160, 11, 12, 9, 111, 116, 104, 101, 114, 78, 97, 109, 101}, + }}, + KeyUsage: KeyUsage(x509.KeyUsageDigitalSignature), + ExtKeyUsage: ExtKeyUsage([]x509.ExtKeyUsage{ + x509.ExtKeyUsageServerAuth, + x509.ExtKeyUsageClientAuth, + }), + PublicKey: priv.Public(), + PublicKeyAlgorithm: x509.Ed25519, + }, false}, {"okExample", args{cr, []Option{WithTemplateFile("./testdata/example.tpl", TemplateData{ SANsKey: []SubjectAlternativeName{ {Type: "dns", Value: "foo.com"}, @@ -241,12 +270,13 @@ func TestNewCertificate(t *testing.T) { {"failTemplate", args{cr, []Option{WithTemplate(`{{ fail "fatal error }}`, CreateTemplateData("commonName", []string{"foo.com"}))}}, nil, true}, {"missingTemplate", args{cr, []Option{WithTemplateFile("./testdata/missing.tpl", CreateTemplateData("commonName", []string{"foo.com"}))}}, nil, true}, {"badJson", args{cr, []Option{WithTemplate(`"this is not a json object"`, CreateTemplateData("commonName", []string{"foo.com"}))}}, nil, true}, + {"failCustomSANs", args{cr, []Option{WithTemplate(DefaultLeafTemplate, badCustomSANsData)}}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := NewCertificate(tt.args.cr, tt.args.opts...) if (err != nil) != tt.wantErr { - t.Errorf("NewCertificate() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("NewCertificate() error = %+v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { diff --git a/x509util/extensions.go b/x509util/extensions.go index 2ce19855..14ce779b 100644 --- a/x509util/extensions.go +++ b/x509util/extensions.go @@ -778,14 +778,16 @@ func MarshalSubjectAlternativeName(sans ...SubjectAlternativeName) (pkix.Extensi // // TODO(mariano,unreality): X400Address, DirectoryName, and EDIPartyName types // are defined in RFC5280 but are currently unimplemented -func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (*Extension, error) { +func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (Extension, error) { + var zero Extension + var rawValues []asn1.RawValue for _, dnsName := range c.DNSNames { rawValue, err := SubjectAlternativeName{ Type: DNSType, Value: dnsName, }.RawValue() if err != nil { - return nil, err + return zero, err } rawValues = append(rawValues, rawValue) @@ -796,7 +798,7 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (*Extens Type: EmailType, Value: emailAddress, }.RawValue() if err != nil { - return nil, err + return zero, err } rawValues = append(rawValues, rawValue) @@ -807,7 +809,7 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (*Extens Type: URIType, Value: uri.String(), }.RawValue() if err != nil { - return nil, err + return zero, err } rawValues = append(rawValues, rawValue) @@ -818,7 +820,7 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (*Extens Type: IPType, Value: ip.String(), }.RawValue() if err != nil { - return nil, err + return zero, err } rawValues = append(rawValues, rawValue) @@ -827,7 +829,7 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (*Extens for _, san := range c.SANs { rawValue, err := san.RawValue() if err != nil { - return nil, err + return zero, err } rawValues = append(rawValues, rawValue) @@ -836,14 +838,12 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (*Extens // Now marshal the rawValues into the ASN1 sequence, and create an Extension object to hold the extension rawBytes, err := asn1.Marshal(rawValues) if err != nil { - return nil, errors.Wrap(err, "error marshaling SubjectAlternativeName extension to ASN1") + return zero, errors.Wrap(err, "error marshaling SubjectAlternativeName extension to ASN1") } - subjectAltNameExtension := Extension{ + return Extension{ ID: oidExtensionSubjectAltName, Critical: subjectIsEmpty, Value: rawBytes, - } - - return &subjectAltNameExtension, nil + }, nil } From c257830f9dd4bfc7891eaf9dee5451fd67afa42b Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 9 Aug 2022 19:22:08 -0700 Subject: [PATCH 09/13] Add test for createSubjectAltNameExtension --- x509util/extensions_test.go | 115 ++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/x509util/extensions_test.go b/x509util/extensions_test.go index 36f4e502..1b182b45 100644 --- a/x509util/extensions_test.go +++ b/x509util/extensions_test.go @@ -1,6 +1,7 @@ package x509util import ( + "bytes" "crypto/x509" "crypto/x509/pkix" "encoding/asn1" @@ -1063,3 +1064,117 @@ func TestSerialNumber_UnmarshalJSON(t *testing.T) { }) } } + +func Test_createSubjectAltNameExtension(t *testing.T) { + type args struct { + c *Certificate + subjectIsEmpty bool + } + tests := []struct { + name string + args args + want Extension + wantErr bool + }{ + {"ok dns", args{&Certificate{ + DNSNames: []string{"foo.com"}, + }, false}, Extension{ + ID: oidExtensionSubjectAltName, + Critical: false, + Value: append([]byte{0x30, 9, 0x80 | nameTypeDNS, 7}, []byte("foo.com")...), + }, false}, + {"ok dns critical", args{&Certificate{ + DNSNames: []string{"foo.com"}, + }, true}, Extension{ + ID: oidExtensionSubjectAltName, + Critical: true, + Value: append([]byte{0x30, 9, 0x80 | nameTypeDNS, 7}, []byte("foo.com")...), + }, false}, + {"ok email", args{&Certificate{ + EmailAddresses: []string{"bar@foo.com"}, + }, false}, Extension{ + ID: oidExtensionSubjectAltName, + Critical: false, + Value: append([]byte{0x30, 13, 0x80 | nameTypeEmail, 11}, []byte("bar@foo.com")...), + }, false}, + {"ok uri", args{&Certificate{ + URIs: []*url.URL{{Scheme: "urn", Opaque: "foo:bar"}}, + }, false}, Extension{ + ID: oidExtensionSubjectAltName, + Critical: false, + Value: append([]byte{0x30, 13, 0x80 | nameTypeURI, 11}, []byte("urn:foo:bar")...), + }, false}, + {"ok ip", args{&Certificate{ + IPAddresses: []net.IP{net.ParseIP("1.2.3.4")}, + }, false}, Extension{ + ID: oidExtensionSubjectAltName, + Critical: false, + Value: []byte{0x30, 6, 0x80 | nameTypeIP, 4, 1, 2, 3, 4}, + }, false}, + {"ok sans", args{&Certificate{ + SANs: []SubjectAlternativeName{ + {Type: "dns", Value: "foo.com"}, + {Type: "email", Value: "bar@foo.com"}, + {Type: "uri", Value: "urn:foo:bar"}, + {Type: "ip", Value: "1.2.3.4"}, + }, + }, false}, Extension{ + ID: oidExtensionSubjectAltName, + Critical: false, + Value: bytes.Join([][]byte{ + {0x30, (2 + 7) + (2 + 11) + (2 + 11) + (2 + 4)}, + {0x80 | nameTypeDNS, 7}, []byte("foo.com"), + {0x80 | nameTypeEmail, 11}, []byte("bar@foo.com"), + {0x80 | nameTypeURI, 11}, []byte("urn:foo:bar"), + {0x80 | nameTypeIP, 4, 1, 2, 3, 4}, + }, nil), + }, false}, + {"ok otherName", args{&Certificate{ + SANs: []SubjectAlternativeName{ + {Type: "dns", Value: "foo.com"}, + {Type: "1.2.3.4", Value: "utf8:bar@foo.com"}, + }, + }, false}, Extension{ + ID: oidExtensionSubjectAltName, + Critical: false, + Value: bytes.Join([][]byte{ + {0x30, (2 + 7) + (2 + 20)}, + {0x80 | nameTypeDNS, 7}, []byte("foo.com"), + {0xA0, 20, asn1.TagOID, 3, 0x20 | 0x0A, 3, 4}, + {0xA0, 13, asn1.TagUTF8String, 11}, []byte("bar@foo.com"), + }, nil), + }, false}, + {"fail dns", args{&Certificate{ + DNSNames: []string{""}, + }, false}, Extension{}, true}, + {"fail email", args{&Certificate{ + EmailAddresses: []string{"nöt@ia5.com"}, + }, false}, Extension{}, true}, + {"fail uri", args{&Certificate{ + URIs: []*url.URL{{Scheme: "urn", Opaque: "nöt:ia5"}}, + }, false}, Extension{}, true}, + {"fail ip", args{&Certificate{ + IPAddresses: []net.IP{{1, 2, 3}}, + }, false}, Extension{}, true}, + {"fail otherName", args{&Certificate{ + SANs: []SubjectAlternativeName{ + {Type: "1.2.3.4", Value: "int:bar@foo.com"}, + }, + }, false}, Extension{}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.name == "fail dns" { + t.Log("foo") + } + got, err := createSubjectAltNameExtension(tt.args.c, tt.args.subjectIsEmpty) + if (err != nil) != tt.wantErr { + t.Errorf("createSubjectAltNameExtension() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("createSubjectAltNameExtension() = \n%v, want \n%v", got, tt.want) + } + }) + } +} From f30f4950903252ed695586b8456f8a017c0abc2a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 9 Aug 2022 19:23:36 -0700 Subject: [PATCH 10/13] Remove unnecessary method --- x509util/extensions.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/x509util/extensions.go b/x509util/extensions.go index 14ce779b..72b7c0f5 100644 --- a/x509util/extensions.go +++ b/x509util/extensions.go @@ -745,30 +745,6 @@ func (s *SerialNumber) UnmarshalJSON(data []byte) error { return nil } -// MarshalSubjectAlternativeName marshals to a pkix.Extension the given SANs. -// This method does not set the Critical option that must be set if the subject -// is empty. -func MarshalSubjectAlternativeName(sans ...SubjectAlternativeName) (pkix.Extension, error) { - var rawValues []asn1.RawValue - for _, san := range sans { - rawValue, err := san.RawValue() - if err != nil { - return pkix.Extension{}, err - } - rawValues = append(rawValues, rawValue) - } - - rawBytes, err := asn1.Marshal(rawValues) - if err != nil { - return pkix.Extension{}, fmt.Errorf("error marshaling SubjectAlternativeName extension to ASN1: %w", err) - } - - return pkix.Extension{ - Id: oidExtensionSubjectAltName, - Value: rawBytes, - }, nil -} - // createSubjectAltNameExtension will construct an Extension containing all // SubjectAlternativeNames held in a Certificate. It implements more types than // the golang x509 library, so it is used whenever OtherName or RegisteredID From 8de4757686bf1fa291d577c438aa1b30be8e7c94 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 10 Aug 2022 16:07:43 -0700 Subject: [PATCH 11/13] Apply suggestions from code review Co-authored-by: Herman Slatman --- x509util/certificate.go | 6 +++--- x509util/name.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x509util/certificate.go b/x509util/certificate.go index 09b17d8c..bef19105 100644 --- a/x509util/certificate.go +++ b/x509util/certificate.go @@ -68,15 +68,15 @@ func NewCertificate(cr *x509.CertificateRequest, opts ...Option) (*Certificate, cert.PublicKey = cr.PublicKey cert.PublicKeyAlgorithm = cr.PublicKeyAlgorithm - // Generate the subjectAltName extension if has SANs not directly supported - // by Go. + // Generate the subjectAltName extension if the certificate contains + // SANs that are not supported in the Go standard library. if cert.hasExtendedSANs() && !cert.hasExtension(oidExtensionSubjectAltName) { ext, err := createSubjectAltNameExtension(&cert, cert.Subject.IsEmpty()) if err != nil { return nil, err } // Prepend extension to achieve a certificate as similar as possible to - // the one generated by the standard Go library. + // the one generated by the Go standard library. cert.Extensions = append([]Extension{ext}, cert.Extensions...) } diff --git a/x509util/name.go b/x509util/name.go index 8f7c87af..1d0ad12b 100644 --- a/x509util/name.go +++ b/x509util/name.go @@ -58,7 +58,7 @@ func newName(n pkix.Name) Name { } } -// goValue converts Subject to the Go representation. +// goValue converts Name to its Go representation. func (n Name) goValue() pkix.Name { return pkix.Name{ Country: n.Country, From 33046b2672579e76caf6ed1449a48951ab677228 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 10 Aug 2022 16:34:07 -0700 Subject: [PATCH 12/13] Remove debug format options in tests --- sshutil/certificate_test.go | 2 +- x509util/certificate_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sshutil/certificate_test.go b/sshutil/certificate_test.go index 90bcf14e..6bb09e6f 100644 --- a/sshutil/certificate_test.go +++ b/sshutil/certificate_test.go @@ -204,7 +204,7 @@ func TestNewCertificate(t *testing.T) { return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("NewCertificate() = \n%+v, want \n%+v", got, tt.want) + t.Errorf("NewCertificate() = %v, want %v", got, tt.want) } }) } diff --git a/x509util/certificate_test.go b/x509util/certificate_test.go index 6d41a647..3eddb8c7 100644 --- a/x509util/certificate_test.go +++ b/x509util/certificate_test.go @@ -276,7 +276,7 @@ func TestNewCertificate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := NewCertificate(tt.args.cr, tt.args.opts...) if (err != nil) != tt.wantErr { - t.Errorf("NewCertificate() error = %+v, wantErr %v", err, tt.wantErr) + t.Errorf("NewCertificate() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { @@ -412,7 +412,7 @@ func TestCertificate_GetCertificate(t *testing.T) { PublicKey: tt.fields.PublicKey, } if got := c.GetCertificate(); !reflect.DeepEqual(got, tt.want) { - t.Errorf("Certificate.GetCertificate() = \n%+v, want \n%+v", got, tt.want) + t.Errorf("Certificate.GetCertificate() = %v, want %v", got, tt.want) } }) } From 022ae3d77b4874a29b606e7a39cd6cd3f55d16c3 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 10 Aug 2022 16:34:34 -0700 Subject: [PATCH 13/13] Fix comment format --- x509util/certificate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x509util/certificate.go b/x509util/certificate.go index bef19105..a3c1956c 100644 --- a/x509util/certificate.go +++ b/x509util/certificate.go @@ -68,8 +68,8 @@ func NewCertificate(cr *x509.CertificateRequest, opts ...Option) (*Certificate, cert.PublicKey = cr.PublicKey cert.PublicKeyAlgorithm = cr.PublicKeyAlgorithm - // Generate the subjectAltName extension if the certificate contains - // SANs that are not supported in the Go standard library. + // Generate the subjectAltName extension if the certificate contains SANs + // that are not supported in the Go standard library. if cert.hasExtendedSANs() && !cert.hasExtension(oidExtensionSubjectAltName) { ext, err := createSubjectAltNameExtension(&cert, cert.Subject.IsEmpty()) if err != nil {