Add support for Fulcio username identity in SAN

pkg/cosign/certextensions.go

@@ -15,7 +15,13 @@
 
 package cosign
 
-import "crypto/x509"
+import (
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/asn1"
+	"errors"
+	"fmt"
+)
 
 type CertExtensions struct {
 	Cert *x509.Certificate
@@ -30,6 +36,8 @@ var (
 	CertExtensionGithubWorkflowRepository = "1.3.6.1.4.1.57264.1.5"
 	CertExtensionGithubWorkflowRef        = "1.3.6.1.4.1.57264.1.6"
 
+	OIDOtherName = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 7}
+
 	CertExtensionMap = map[string]string{
 		CertExtensionOIDCIssuer:               "oidcIssuer",
 		CertExtensionGithubWorkflowTrigger:    "githubWorkflowTrigger",
@@ -38,6 +46,9 @@ var (
 		CertExtensionGithubWorkflowRepository: "githubWorkflowRepository",
 		CertExtensionGithubWorkflowRef:        "githubWorkflowRef",
 	}
+
+	// OID for Subject Alternative Name
+	SANOID = asn1.ObjectIdentifier{2, 5, 29, 17}
 )
 
 func (ce *CertExtensions) certExtensions() map[string]string {
@@ -82,3 +93,103 @@ func (ce *CertExtensions) GetCertExtensionGithubWorkflowRepository() string {
 func (ce *CertExtensions) GetCertExtensionGithubWorkflowRef() string {
 	return ce.certExtensions()["githubWorkflowRef"]
 }
+
+// TODO: Move (un)marshalling to sigstore/sigstore
+// 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"`
+}
+
+// MarshalOtherNameSAN 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 MarshalOtherNameSAN(name string, critical bool) (*pkix.Extension, error) {
+	o := OtherName{
+		ID:    OIDOtherName,
+		Value: name,
+	}
+	bytes, err := asn1.MarshalWithParams(o, "tag:0")
+	if err != nil {
+		return nil, err
+	}
+
+	sans, err := asn1.Marshal([]asn1.RawValue{{FullBytes: bytes}})
+	if err != nil {
+		return nil, err
+	}
+	return &pkix.Extension{
+		Id:       SANOID,
+		Critical: critical,
+		Value:    sans,
+	}, nil
+}
+
+// UnmarshalOtherNameSAN extracts a UTF-8 string from the OtherName
+// field in the Subject Alternative Name extension.
+func UnmarshalOtherNameSAN(exts []pkix.Extension) (string, error) {
+	var otherNames []string
+
+	for _, e := range exts {
+		if !e.Id.Equal(SANOID) {
+			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 != asn1.TagSequence || seq.Class != asn1.ClassUniversal {
+			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: %w", err)
+			}
+			if !other.ID.Equal(OIDOtherName) {
+				return "", fmt.Errorf("unexpected OID for OtherName, expected %v, got %v", 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/cosign/certextensions_test.go

@@ -18,6 +18,8 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/asn1"
+	"encoding/hex"
+	"strings"
 	"testing"
 )
 
@@ -64,3 +66,166 @@ func TestCertExtensions(t *testing.T) {
 		t.Fatal("CertExtension does not extract field 'githubWorkflowRef' correctly")
 	}
 }
+
+func TestMarshalAndUnmarshalOtherNameSAN(t *testing.T) {
+	otherName := "foo!example.com"
+	critical := true
+
+	ext, err := MarshalOtherNameSAN(otherName, critical)
+	if err != nil {
+		t.Fatalf("unexpected error for MarshalOtherNameSAN: %v", err)
+	}
+	if ext.Critical != critical {
+		t.Fatalf("expected extension to be critical")
+	}
+	if !ext.Id.Equal(SANOID) {
+		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 := UnmarshalOtherNameSAN([]pkix.Extension{*ext})
+	if err != nil {
+		t.Fatalf("unexpected error for UnmarshalOtherNameSAN: %v", err)
+	}
+	if on != otherName {
+		t.Fatalf("unexpected OtherName, expected %s, got %s", otherName, on)
+	}
+}
+
+func TestUnmarshalOtherNameSANFailures(t *testing.T) {
+	var err error
+
+	// failure: no SANs extension
+	ext := &pkix.Extension{
+		Id:       asn1.ObjectIdentifier{},
+		Critical: true,
+		Value:    []byte{},
+	}
+	_, err = UnmarshalOtherNameSAN([]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:       SANOID,
+		Critical: true,
+		Value:    []byte{},
+	}
+	_, err = UnmarshalOtherNameSAN([]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:       SANOID,
+		Critical: true,
+		Value:    b,
+	}
+	_, err = UnmarshalOtherNameSAN([]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:       SANOID,
+		Critical: true,
+		Value:    b,
+	}
+	_, err = UnmarshalOtherNameSAN([]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:       SANOID,
+		Critical: true,
+		Value:    b,
+	}
+	_, err = UnmarshalOtherNameSAN([]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 => 00100010 -> 12)
+	b, _ = hex.DecodeString("1221a01f060a2b0601040183bf300107a0110c0f666f6f216578616d706c652e636f6d")
+	ext = &pkix.Extension{
+		Id:       SANOID,
+		Critical: true,
+		Value:    b,
+	}
+	_, err = UnmarshalOtherNameSAN([]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:       SANOID,
+		Critical: true,
+		Value:    b,
+	}
+	_, err = UnmarshalOtherNameSAN([]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:       SANOID,
+		Critical: true,
+		Value:    b,
+	}
+	_, err = UnmarshalOtherNameSAN([]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:       SANOID,
+		Critical: true,
+		Value:    b,
+	}
+	_, err = UnmarshalOtherNameSAN([]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:       SANOID,
+		Critical: true,
+		Value:    b,
+	}
+	_, err = UnmarshalOtherNameSAN([]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)
+	}
+}

pkg/cosign/verify.go

@@ -21,6 +21,7 @@ import (
 	"crypto/ecdsa"
 	"crypto/sha256"
 	"crypto/x509"
+	"encoding/asn1"
 	"encoding/base64"
 	"encoding/hex"
 	"encoding/json"
@@ -169,6 +170,20 @@ func ValidateAndUnpackCert(cert *x509.Certificate, co *CheckOpts) (signature.Ver
 		return nil, fmt.Errorf("invalid certificate found on signature: %w", err)
 	}
 
+	// Handle certificates where the Subject Alternative Name is not set to a supported
+	// GeneralName (RFC 5280 4.2.1.6). Go only supports DNS, IP addresses, email addresses,
+	// or URIs as SANs. Fulcio can issue a certificate with an OtherName GeneralName, so
+	// remove the unhandled critical SAN extension before verifying.
+	if len(cert.UnhandledCriticalExtensions) > 0 {
+		var unhandledExts []asn1.ObjectIdentifier
+		for _, oid := range cert.UnhandledCriticalExtensions {
+			if !oid.Equal(SANOID) {
+				unhandledExts = append(unhandledExts, oid)
+			}
+		}
+		cert.UnhandledCriticalExtensions = unhandledExts
+	}
+
 	// Now verify the cert, then the signature.
 	chains, err := TrustedCert(cert, co.RootCerts, co.IntermediateCerts)
 	if err != nil {
@@ -332,6 +347,11 @@ func getSubjectAlternateNames(cert *x509.Certificate) []string {
 	for _, uri := range cert.URIs {
 		sans = append(sans, uri.String())
 	}
+	// ignore error if there's no OtherName SAN
+	otherName, _ := UnmarshalOtherNameSAN(cert.Extensions)
+	if len(otherName) > 0 {
+		sans = append(sans, otherName)
+	}
 	return sans
 }