diff --git a/docs/oid-info.md b/docs/oid-info.md index 6e441a9ae..42ef9b5f4 100644 --- a/docs/oid-info.md +++ b/docs/oid-info.md @@ -52,6 +52,11 @@ This contains the `ref` claim from the GitHub OIDC Identity token that contains the git ref that the workflow run was based upon. [(docs)][github-oidc-doc] +### 1.3.6.1.4.1.57264.1.7 | OtherName SAN + +This specifies the username identity in the OtherName Subject Alternative Name, as +defined by [RFC5280 4.2.1.6](https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6). + [github-oidc-doc]: https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect#understanding-the-oidc-token [oid-link]: http://oid-info.com/get/1.3.6.1.4.1.57264 diff --git a/docs/oidc.md b/docs/oidc.md index f205cf61f..9075d45e5 100644 --- a/docs/oidc.md +++ b/docs/oidc.md @@ -163,4 +163,4 @@ Additionally, the configuration must include `SubjectDomain`, for example `examp * The issuer in the configuration must partially match the domain in the configuration. The top level domain and second level domain must match. The user who updates the Fulcio configuration must also have control over both the issuer and domain configuration fields (Verified either manually or through an ACME-style challenge). -`SubjectDomain` is appended to `sub` to form an email, `sub@SubjectDomain`, and included as a SAN email address. +`SubjectDomain` is appended to `sub` to form an identity, `sub!SubjectDomain`, and included as an OtherName SAN. diff --git a/go.mod b/go.mod index f46401f05..10998df76 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( cloud.google.com/go/security v1.7.0 github.com/PaesslerAG/jsonpath v0.1.1 github.com/ThalesIgnite/crypto11 v1.2.5 + github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d github.com/coreos/go-oidc/v3 v3.4.0 github.com/fsnotify/fsnotify v1.5.4 github.com/goadesign/goa v2.2.5+incompatible diff --git a/go.sum b/go.sum index f9f7fabb8..314855a58 100644 --- a/go.sum +++ b/go.sum @@ -183,6 +183,8 @@ github.com/armon/go-radix v1.0.0 h1:F4z6KzEeeQIMeLFa97iZU6vupzoecKdU5TX24SNppXI= github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/aryann/difflib v0.0.0-20170710044230-e206f873d14a/go.mod h1:DAHtR1m6lCRdSC2Tm3DSWRPvIPr6xNKyeHdqDQSQT+A= +github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ= +github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/aws/aws-lambda-go v1.13.3/go.mod h1:4UKl9IzQMoD+QF79YdCuzCwp8VbmG4VAQwij/eHl5CU= github.com/aws/aws-sdk-go v1.15.27/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0= github.com/aws/aws-sdk-go v1.19.18/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= diff --git a/pkg/certificate/extensions.go b/pkg/certificate/extensions.go index 78d91df20..b58a7bc93 100644 --- a/pkg/certificate/extensions.go +++ b/pkg/certificate/extensions.go @@ -27,6 +27,7 @@ var ( OIDGitHubWorkflowName = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 4} OIDGitHubWorkflowRepository = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 5} OIDGitHubWorkflowRef = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 6} + OIDOtherName = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 7} ) // Extensions contains all custom x509 extensions defined by Fulcio diff --git a/pkg/identity/email/principal.go b/pkg/identity/email/principal.go index b17532f12..2a3568aa5 100644 --- a/pkg/identity/email/principal.go +++ b/pkg/identity/email/principal.go @@ -18,7 +18,9 @@ import ( "context" "crypto/x509" "errors" + "fmt" + "github.com/asaskevich/govalidator" "github.com/coreos/go-oidc/v3/oidc" "github.com/sigstore/fulcio/pkg/certificate" "github.com/sigstore/fulcio/pkg/config" @@ -40,6 +42,10 @@ func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Pr return nil, errors.New("email_verified claim was false") } + if !govalidator.IsEmail(emailAddress) { + return nil, fmt.Errorf("email address is not valid") + } + cfg, ok := config.FromContext(ctx).GetIssuer(token.Issuer) if !ok { return nil, errors.New("invalid configuration for OIDC ID Token issuer") diff --git a/pkg/identity/email/principal_test.go b/pkg/identity/email/principal_test.go index 829230498..980ef3879 100644 --- a/pkg/identity/email/principal_test.go +++ b/pkg/identity/email/principal_test.go @@ -144,6 +144,25 @@ func TestPrincipalFromIDToken(t *testing.T) { }, WantErr: true, }, + `Invalid email address should error`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "iss": "https://iss.example.com", + "sub": "doesntmatter", + "email": "foo.com", + "email_verified": true, + }, + Config: config.FulcioConfig{ + OIDCIssuers: map[string]config.OIDCIssuer{ + "https://iss.example.com": { + IssuerURL: "https://iss.example.com", + Type: config.IssuerTypeEmail, + ClientID: "sigstore", + }, + }, + }, + WantErr: true, + }, `No issuer configured for token`: { Claims: map[string]interface{}{ "aud": "sigstore", diff --git a/pkg/identity/uri/principal.go b/pkg/identity/uri/principal.go index 0e1dcfdb5..227e5b477 100644 --- a/pkg/identity/uri/principal.go +++ b/pkg/identity/uri/principal.go @@ -21,6 +21,7 @@ import ( "fmt" "net/url" + "github.com/asaskevich/govalidator" "github.com/coreos/go-oidc/v3/oidc" "github.com/sigstore/fulcio/pkg/certificate" "github.com/sigstore/fulcio/pkg/config" @@ -40,6 +41,10 @@ func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Pr return nil, errors.New("invalid configuration for OIDC ID Token issuer") } + if govalidator.IsEmail(uriWithSubject) { + return nil, fmt.Errorf("uri subject should not be an email address") + } + // The subject hostname must exactly match the subject domain from the configuration uSubject, err := url.Parse(uriWithSubject) if err != nil { diff --git a/pkg/identity/uri/principal_test.go b/pkg/identity/uri/principal_test.go index c72451d16..135ff2e43 100644 --- a/pkg/identity/uri/principal_test.go +++ b/pkg/identity/uri/principal_test.go @@ -47,6 +47,10 @@ func TestPrincipalFromIDToken(t *testing.T) { Token: &oidc.IDToken{Issuer: "https://notaccounts.example.com", Subject: "https://example.com/users/1"}, WantErr: true, }, + `Subject as an email address should error`: { + Token: &oidc.IDToken{Issuer: "https://accounts.example.com", Subject: "user@example.com"}, + WantErr: true, + }, `Incorrect subject domain hostname should error`: { Token: &oidc.IDToken{Issuer: "https://accounts.example.com", Subject: "https://notexample.com/users/1"}, WantErr: true, diff --git a/pkg/identity/username/othername.go b/pkg/identity/username/othername.go new file mode 100644 index 000000000..726fce9e7 --- /dev/null +++ b/pkg/identity/username/othername.go @@ -0,0 +1,125 @@ +// Copyright 2022 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package username + +import ( + "crypto/x509/pkix" + "encoding/asn1" + "errors" + "fmt" + + "github.com/sigstore/fulcio/pkg/certificate" +) + +// OtherName describes a name related to a certificate which is not in one +// of the standard name formats. RFC 5280, 4.2.1.6: +// +// OtherName ::= SEQUENCE { +// type-id OBJECT IDENTIFIER, +// value [0] EXPLICIT ANY DEFINED BY type-id } +// +// OtherName for Fulcio-issued certificates only supports UTF-8 strings as values. +type OtherName struct { + ID asn1.ObjectIdentifier + Value string `asn1:"utf8,explicit,tag:0"` +} + +// MarshalSANS creates a Subject Alternative Name extension +// with an OtherName sequence. RFC 5280, 4.2.1.6: +// +// SubjectAltName ::= GeneralNames +// GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName +// GeneralName ::= CHOICE { +// +// otherName [0] OtherName, +// ... } +func MarshalSANS(name string, critical bool) (*pkix.Extension, error) { + var rawValues []asn1.RawValue + o := OtherName{ + ID: certificate.OIDOtherName, + Value: name, + } + bytes, err := asn1.MarshalWithParams(o, "tag:0") + if err != nil { + return nil, err + } + rawValues = append(rawValues, asn1.RawValue{FullBytes: bytes}) + + sans, err := asn1.Marshal(rawValues) + if err != nil { + return nil, err + } + return &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: critical, + Value: sans, + }, nil +} + +// UnmarshalSANs extracts a UTF-8 string from the OtherName +// field in the Subject Alternative Name extension. +func UnmarshalSANS(exts []pkix.Extension) (string, error) { + var otherNames []string + + for _, e := range exts { + if !e.Id.Equal(asn1.ObjectIdentifier{2, 5, 29, 17}) { + continue + } + + var seq asn1.RawValue + rest, err := asn1.Unmarshal(e.Value, &seq) + if err != nil { + return "", err + } else if len(rest) != 0 { + return "", fmt.Errorf("trailing data after X.509 extension") + } + if !seq.IsCompound || seq.Tag != 16 || seq.Class != 0 { + return "", asn1.StructuralError{Msg: "bad SAN sequence"} + } + + rest = seq.Bytes + for len(rest) > 0 { + var v asn1.RawValue + rest, err = asn1.Unmarshal(rest, &v) + if err != nil { + return "", err + } + + // skip all GeneralName fields except OtherName + if v.Tag != 0 { + continue + } + + var other OtherName + _, err := asn1.UnmarshalWithParams(v.FullBytes, &other, "tag:0") + if err != nil { + return "", fmt.Errorf("could not parse requested OtherName SAN: %v", err) + } + if !other.ID.Equal(certificate.OIDOtherName) { + return "", fmt.Errorf("unexpected OID for OtherName, expected %v, got %v", certificate.OIDOtherName, other.ID) + } + otherNames = append(otherNames, other.Value) + } + } + + if len(otherNames) == 0 { + return "", errors.New("no OtherName found") + } + if len(otherNames) != 1 { + return "", errors.New("expected only one OtherName") + } + + return otherNames[0], nil +} diff --git a/pkg/identity/username/othername_test.go b/pkg/identity/username/othername_test.go new file mode 100644 index 000000000..f84f6b4f2 --- /dev/null +++ b/pkg/identity/username/othername_test.go @@ -0,0 +1,186 @@ +// Copyright 2022 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package username + +import ( + "crypto/x509/pkix" + "encoding/asn1" + "encoding/hex" + "strings" + "testing" +) + +func TestMarshalAndUnmarshalSANS(t *testing.T) { + otherName := "foo!example.com" + critical := true + + ext, err := MarshalSANS(otherName, critical) + if err != nil { + t.Fatalf("unexpected error for MarshalSANs: %v", err) + } + if ext.Critical != critical { + t.Fatalf("expected extension to be critical") + } + if !ext.Id.Equal(asn1.ObjectIdentifier{2, 5, 29, 17}) { + t.Fatalf("expected extension's OID to be SANs OID") + } + // https://lapo.it/asn1js/#MCGgHwYKKwYBBAGDvzABB6ARDA9mb28hZXhhbXBsZS5jb20 + // 30 - Constructed sequence + // 21 - length of sequence + // A0 - Context-specific (class 2) (bits 8,7) with Constructed bit (bit 6) and 0 tag + // 1F - length of context-specific field (OID) + // 06 - OID tag + // 0A - length of OID + // 2B 06 01 04 01 83 BF 30 01 07 - OID + // A0 - Context-specific (class 2) with Constructed bit and 0 tag + // (needed for EXPLICIT encoding, which wraps field in outer encoding) + // 11 - length of context-specific field (string) + // 0C - UTF8String tag + // 0F - length of string + // 66 6F 6F 21 65 78 61 6D 70 6C 65 2E 63 6F 6D - string + if hex.EncodeToString(ext.Value) != "3021a01f060a2b0601040183bf300107a0110c0f666f6f216578616d706c652e636f6d" { + t.Fatalf("unexpected ASN.1 encoding") + } + + on, err := UnmarshalSANS([]pkix.Extension{*ext}) + if err != nil { + t.Fatalf("unexpected error for UnmarshalSANs: %v", err) + } + if on != otherName { + t.Fatalf("unexpected OtherName, expected %s, got %s", otherName, on) + } +} + +func TestUnmarshalSANsFailures(t *testing.T) { + var err error + + // failure: no SANs extension + ext := &pkix.Extension{ + Id: asn1.ObjectIdentifier{}, + Critical: true, + Value: []byte{}, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "no OtherName found") { + t.Fatalf("expected error finding no OtherName, got %v", err) + } + + // failure: bad sequence + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: []byte{}, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "sequence truncated") { + t.Fatalf("expected error with invalid ASN.1, got %v", err) + } + + // failure: extra data after valid sequence + b, _ := hex.DecodeString("3021a01f060a2b0601040183bf300107a0110c0f666f6f216578616d706c652e636f6d" + "30") + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: b, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "trailing data after X.509 extension") { + t.Fatalf("expected error with extra data, got %v", err) + } + + // failure: non-universal class (Change last two bits: 30 = 00110000 => 10110000 -> B0) + b, _ = hex.DecodeString("B021a01f060a2b0601040183bf300107a0110c0f666f6f216578616d706c652e636f6d") + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: b, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "bad SAN sequence") { + t.Fatalf("expected error with non-universal class, got %v", err) + } + + // failure: not compound sequence (Change 6th bit: 30 = 00110000 => 00010000 -> 10) + b, _ = hex.DecodeString("1021a01f060a2b0601040183bf300107a0110c0f666f6f216578616d706c652e636f6d") + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: b, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "bad SAN sequence") { + t.Fatalf("expected error with non-compound sequence, got %v", err) + } + + // failure: non-sequence tag (Change lower 5 bits: 30 = 00110000 => 00000010 -> 02) + b, _ = hex.DecodeString("0221a01f060a2b0601040183bf300107a0110c0f666f6f216578616d706c652e636f6d") + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: b, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "bad SAN sequence") { + t.Fatalf("expected error with non-sequence tag, got %v", err) + } + + // failure: no GeneralName with tag=0 (Change lower 5 bits of first sequence field: 3021a01f -> 3021a11f) + b, _ = hex.DecodeString("3021a11f060a2b0601040183bf300108a0110c0f666f6f216578616d706c652e636f6d") + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: b, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "no OtherName found") { + t.Fatalf("expected error with no GeneralName, got %v", err) + } + + // failure: invalid OtherName (Change tag of UTF8String field to 1: a0110c0f -> a1110c0f) + b, _ = hex.DecodeString("3021a01f060a2b0601040183bf300108a1110c0f666f6f216578616d706c652e636f6d") + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: b, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "could not parse requested OtherName SAN") { + t.Fatalf("expected error with invalid OtherName, got %v", err) + } + + // failure: OtherName has wrong OID (2b0601040183bf300107 -> 2b0601040183bf300108) + b, _ = hex.DecodeString("3021a01f060a2b0601040183bf300108a0110c0f666f6f216578616d706c652e636f6d") + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: b, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "unexpected OID for OtherName") { + t.Fatalf("expected error with wrong OID, got %v", err) + } + + // failure: multiple OtherName fields (Increase sequence size from 0x21 -> 0x42, duplicate OtherName) + b, _ = hex.DecodeString("3042a01f060a2b0601040183bf300107a0110c0f666f6f216578616d706c652e636f6da01f060a2b0601040183bf300107a0110c0f666f6f216578616d706c652e636f6d") + ext = &pkix.Extension{ + Id: asn1.ObjectIdentifier{2, 5, 29, 17}, + Critical: true, + Value: b, + } + _, err = UnmarshalSANS([]pkix.Extension{*ext}) + if err == nil || !strings.Contains(err.Error(), "expected only one OtherName") { + t.Fatalf("expected error with multiple OtherName fields, got %v", err) + } +} diff --git a/pkg/identity/username/principal.go b/pkg/identity/username/principal.go index 569971b65..2b952031d 100644 --- a/pkg/identity/username/principal.go +++ b/pkg/identity/username/principal.go @@ -17,10 +17,12 @@ package username import ( "context" "crypto/x509" + "crypto/x509/pkix" "errors" "fmt" "strings" + "github.com/asaskevich/govalidator" "github.com/coreos/go-oidc/v3/oidc" "github.com/sigstore/fulcio/pkg/certificate" "github.com/sigstore/fulcio/pkg/config" @@ -28,16 +30,20 @@ import ( ) type principal struct { - issuer string - username string - emailAddress string + issuer string + username string + unIdentity string } func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Principal, error) { username := token.Subject - if strings.Contains(username, "@") { - return nil, errors.New("username cannot contain @ character") + if strings.Contains(username, "!") { + return nil, errors.New("username cannot contain ! character") + } + + if govalidator.IsEmail(username) { + return nil, fmt.Errorf("uri subject should not be an email address") } cfg, ok := config.FromContext(ctx).GetIssuer(token.Issuer) @@ -45,12 +51,12 @@ func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Pr return nil, errors.New("invalid configuration for OIDC ID Token issuer") } - emailSubject := fmt.Sprintf("%s@%s", username, cfg.SubjectDomain) + unIdentity := fmt.Sprintf("%s!%s", username, cfg.SubjectDomain) return principal{ - issuer: token.Issuer, - username: username, - emailAddress: emailSubject, + issuer: token.Issuer, + username: username, + unIdentity: unIdentity, }, nil } @@ -59,15 +65,22 @@ func (p principal) Name(context.Context) string { } func (p principal) Embed(ctx context.Context, cert *x509.Certificate) error { - cert.EmailAddresses = []string{p.emailAddress} + var exts []pkix.Extension + + ext, err := MarshalSANS(p.unIdentity, true /*critical*/) + if err != nil { + return err + } + exts = append(exts, *ext) - var err error - cert.ExtraExtensions, err = certificate.Extensions{ + issuerExt, err := certificate.Extensions{ Issuer: p.issuer, }.Render() if err != nil { return err } + exts = append(exts, issuerExt...) + cert.ExtraExtensions = exts return nil } diff --git a/pkg/identity/username/principal_test.go b/pkg/identity/username/principal_test.go index 158373f20..cc5252b7f 100644 --- a/pkg/identity/username/principal_test.go +++ b/pkg/identity/username/principal_test.go @@ -37,13 +37,17 @@ func TestPrincipalFromIDToken(t *testing.T) { `Valid token authenticates with correct claims`: { Token: &oidc.IDToken{Issuer: "https://accounts.example.com", Subject: "alice"}, Principal: principal{ - issuer: "https://accounts.example.com", - username: "alice", - emailAddress: "alice@example.com", + issuer: "https://accounts.example.com", + username: "alice", + unIdentity: "alice!example.com", }, WantErr: false, }, - `username cannot contain @`: { + `username with ! character should error`: { + Token: &oidc.IDToken{Issuer: "https://accounts.example.com", Subject: "alice!"}, + WantErr: true, + }, + `username as an email address should error`: { Token: &oidc.IDToken{Issuer: "https://accounts.example.com", Subject: "alice@example.com"}, WantErr: true, }, @@ -135,18 +139,22 @@ func TestEmbed(t *testing.T) { }{ `Valid uri challenge`: { Principal: principal{ - issuer: `https://accounts.example.com`, - username: "alice", - emailAddress: "alice@example.com", + issuer: `https://accounts.example.com`, + username: "alice", + unIdentity: "alice!example.com", }, WantErr: false, WantFacts: map[string]func(x509.Certificate) error{ `Issuer is example.com`: factIssuerIs(`https://accounts.example.com`), - `SAN is alice@example.com`: func(cert x509.Certificate) error { - if len(cert.EmailAddresses) != 1 { - return errors.New("no URI SAN set") + `SAN is alice!example.com`: func(cert x509.Certificate) error { + otherName, err := UnmarshalSANS(cert.ExtraExtensions) + if err != nil { + return err + } + if len(cert.EmailAddresses) != 0 { + return errors.New("unexpected email address SAN") } - if diff := cmp.Diff(cert.EmailAddresses[0], "alice@example.com"); diff != "" { + if diff := cmp.Diff(otherName, "alice!example.com"); diff != "" { return errors.New(diff) } return nil @@ -155,9 +163,9 @@ func TestEmbed(t *testing.T) { }, `Empty issuer url should fail to render extensions`: { Principal: principal{ - issuer: "", - emailAddress: "alice@example.com", - username: "alice", + issuer: "", + unIdentity: "alice!example.com", + username: "alice", }, WantErr: true, }, diff --git a/pkg/server/grpc_server_test.go b/pkg/server/grpc_server_test.go index 0b7f94c76..94be1ec3b 100644 --- a/pkg/server/grpc_server_test.go +++ b/pkg/server/grpc_server_test.go @@ -46,6 +46,7 @@ import ( "github.com/sigstore/fulcio/pkg/config" "github.com/sigstore/fulcio/pkg/generated/protobuf" "github.com/sigstore/fulcio/pkg/identity" + "github.com/sigstore/fulcio/pkg/identity/username" "github.com/sigstore/sigstore/pkg/cryptoutils" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -312,9 +313,90 @@ type customClaims struct { OtherIssuer string `json:"other_issuer"` } -// Tests API for email and username subject types +// Tests API for email subject types func TestAPIWithEmail(t *testing.T) { emailSigner, emailIssuer := newOIDCIssuer(t) + + // Create a FulcioConfig that supports these issuers. + cfg, err := config.Read([]byte(fmt.Sprintf(`{ + "OIDCIssuers": { + %q: { + "IssuerURL": %q, + "ClientID": "sigstore", + "Type": "email" + } + } + }`, emailIssuer, emailIssuer))) + if err != nil { + t.Fatalf("config.Read() = %v", err) + } + + emailSubject := "foo@example.com" + + tests := []oidcTestContainer{ + { + Signer: emailSigner, Issuer: emailIssuer, Subject: emailSubject, ExpectedSubject: emailSubject, + }, + } + for _, c := range tests { + // Create an OIDC token using this issuer's signer. + tok, err := jwt.Signed(c.Signer).Claims(jwt.Claims{ + Issuer: c.Issuer, + IssuedAt: jwt.NewNumericDate(time.Now()), + Expiry: jwt.NewNumericDate(time.Now().Add(30 * time.Minute)), + Subject: c.Subject, + Audience: jwt.Audience{"sigstore"}, + }).Claims(customClaims{Email: c.Subject, EmailVerified: true}).CompactSerialize() + if err != nil { + t.Fatalf("CompactSerialize() = %v", err) + } + + ctClient, eca := createCA(cfg, t) + ctx := context.Background() + server, conn := setupGRPCForTest(ctx, t, cfg, ctClient, eca) + defer func() { + server.Stop() + conn.Close() + }() + + client := protobuf.NewCAClient(conn) + + pubBytes, proof := generateKeyAndProof(c.Subject, t) + + // Hit the API to have it sign our certificate. + resp, err := client.CreateSigningCertificate(ctx, &protobuf.CreateSigningCertificateRequest{ + Credentials: &protobuf.Credentials{ + Credentials: &protobuf.Credentials_OidcIdentityToken{ + OidcIdentityToken: tok, + }, + }, + Key: &protobuf.CreateSigningCertificateRequest_PublicKeyRequest{ + PublicKeyRequest: &protobuf.PublicKeyRequest{ + PublicKey: &protobuf.PublicKey{ + Content: pubBytes, + }, + ProofOfPossession: proof, + }, + }, + }) + if err != nil { + t.Fatalf("SigningCert() = %v", err) + } + + leafCert := verifyResponse(resp, eca, c.Issuer, t) + + // Expect email subject + if len(leafCert.EmailAddresses) != 1 { + t.Fatalf("unexpected length of leaf certificate URIs, expected 1, got %d", len(leafCert.URIs)) + } + if leafCert.EmailAddresses[0] != c.ExpectedSubject { + t.Fatalf("subjects do not match: Expected %v, got %v", c.ExpectedSubject, leafCert.EmailAddresses[0]) + } + } +} + +// Tests API for username subject types +func TestAPIWithUsername(t *testing.T) { usernameSigner, usernameIssuer := newOIDCIssuer(t) issuerDomain, err := url.Parse(usernameIssuer) @@ -325,11 +407,6 @@ func TestAPIWithEmail(t *testing.T) { // Create a FulcioConfig that supports these issuers. cfg, err := config.Read([]byte(fmt.Sprintf(`{ "OIDCIssuers": { - %q: { - "IssuerURL": %q, - "ClientID": "sigstore", - "Type": "email" - }, %q: { "IssuerURL": %q, "ClientID": "sigstore", @@ -337,19 +414,15 @@ func TestAPIWithEmail(t *testing.T) { "Type": "username" } } - }`, emailIssuer, emailIssuer, usernameIssuer, usernameIssuer, issuerDomain.Hostname()))) + }`, usernameIssuer, usernameIssuer, issuerDomain.Hostname()))) if err != nil { t.Fatalf("config.Read() = %v", err) } - emailSubject := "foo@example.com" usernameSubject := "foo" - expectedUsernamedSubject := fmt.Sprintf("%s@%s", usernameSubject, issuerDomain.Hostname()) + expectedUsernamedSubject := fmt.Sprintf("%s!%s", usernameSubject, issuerDomain.Hostname()) tests := []oidcTestContainer{ - { - Signer: emailSigner, Issuer: emailIssuer, Subject: emailSubject, ExpectedSubject: emailSubject, - }, { Signer: usernameSigner, Issuer: usernameIssuer, Subject: usernameSubject, ExpectedSubject: expectedUsernamedSubject, }, @@ -401,12 +474,16 @@ func TestAPIWithEmail(t *testing.T) { leafCert := verifyResponse(resp, eca, c.Issuer, t) - // Expect email subject - if len(leafCert.EmailAddresses) != 1 { - t.Fatalf("unexpected length of leaf certificate URIs, expected 1, got %d", len(leafCert.URIs)) + // Expect no email subject + if len(leafCert.EmailAddresses) != 0 { + t.Fatalf("unexpected length of leaf certificate URIs, expected 0, got %d", len(leafCert.URIs)) } - if leafCert.EmailAddresses[0] != c.ExpectedSubject { - t.Fatalf("subjects do not match: Expected %v, got %v", c.ExpectedSubject, leafCert.EmailAddresses[0]) + otherName, err := username.UnmarshalSANS(leafCert.Extensions) + if err != nil { + t.Fatalf("error unmarshalling SANs: %v", err) + } + if otherName != c.ExpectedSubject { + t.Fatalf("subjects do not match: Expected %v, got %v", c.ExpectedSubject, otherName) } } }