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 sigstore#716

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

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

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")

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",

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 {

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,

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
+}

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)
+	}
+}