From 8f07adc542fed2f6e5f26258f385ed9883cb7934 Mon Sep 17 00:00:00 2001 From: Hayden Blauzvern Date: Mon, 26 Sep 2022 21:24:15 +0000 Subject: [PATCH] Change username format, enforce identity format This updates the username type to avoid the username subject format looking like an email. Fulcio will now specify the subject in the OtherName SAN, and the format will use a ! instead of @. This required some custom ASN.1 marshalling and unmarshalling, since crypto/x509 does not support the OtherName SAN. This also adds enforcement that email subjects match a basic email regex format, and that other types do not look like emails. Fixes #716 Signed-off-by: Hayden Blauzvern --- pkg/certificate/extensions.go | 1 + pkg/identity/email/principal.go | 7 + pkg/identity/email/principal_test.go | 19 +++ pkg/identity/uri/principal.go | 6 + pkg/identity/uri/principal_test.go | 4 + pkg/identity/username/othername.go | 109 +++++++++++++++ pkg/identity/username/othername_test.go | 172 ++++++++++++++++++++++++ pkg/identity/username/principal.go | 38 ++++-- pkg/identity/username/principal_test.go | 36 +++-- pkg/server/grpc_server_test.go | 111 ++++++++++++--- 10 files changed, 460 insertions(+), 43 deletions(-) create mode 100644 pkg/identity/username/othername.go create mode 100644 pkg/identity/username/othername_test.go 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..472e057ca 100644 --- a/pkg/identity/email/principal.go +++ b/pkg/identity/email/principal.go @@ -18,6 +18,8 @@ import ( "context" "crypto/x509" "errors" + "fmt" + "regexp" "github.com/coreos/go-oidc/v3/oidc" "github.com/sigstore/fulcio/pkg/certificate" @@ -40,6 +42,11 @@ func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Pr return nil, errors.New("email_verified claim was false") } + emailRegex := regexp.MustCompile(`^.+@.+\..+$`) + if !emailRegex.MatchString(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..7a4611766 100644 --- a/pkg/identity/uri/principal.go +++ b/pkg/identity/uri/principal.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "net/url" + "regexp" "github.com/coreos/go-oidc/v3/oidc" "github.com/sigstore/fulcio/pkg/certificate" @@ -40,6 +41,11 @@ func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Pr return nil, errors.New("invalid configuration for OIDC ID Token issuer") } + emailRegex := regexp.MustCompile(`^.+@.+\..+$`) + if emailRegex.MatchString(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..5a37b2b34 --- /dev/null +++ b/pkg/identity/username/othername.go @@ -0,0 +1,109 @@ +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 +} + +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..c23bbfcf8 --- /dev/null +++ b/pkg/identity/username/othername_test.go @@ -0,0 +1,172 @@ +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..2776235c6 100644 --- a/pkg/identity/username/principal.go +++ b/pkg/identity/username/principal.go @@ -17,8 +17,10 @@ package username import ( "context" "crypto/x509" + "crypto/x509/pkix" "errors" "fmt" + "regexp" "strings" "github.com/coreos/go-oidc/v3/oidc" @@ -28,16 +30,21 @@ 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") + } + + emailRegex := regexp.MustCompile(`^.+@.+\..+$`) + if emailRegex.MatchString(username) { + return nil, fmt.Errorf("uri subject should not be an email address") } cfg, ok := config.FromContext(ctx).GetIssuer(token.Issuer) @@ -45,12 +52,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 +66,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) } } }