Refactor x509 extension embedding logic

Adds a new extensions structure that documents the global set of
extensions added to our certificates and a method to render that data
into extensions.

Signed-off-by: Nathan Smith <nathan@chainguard.dev>

pkg/ca/x509ca/common.go

@@ -20,8 +20,6 @@ import (
 	"crypto"
 	"crypto/rand"
 	"crypto/x509"
-	"crypto/x509/pkix"
-	"encoding/asn1"
 	"encoding/pem"
 	"math/big"
 	"net/url"
@@ -88,7 +86,39 @@ func MakeX509(subject *challenges.ChallengeResult) (*x509.Certificate, error) {
 	case challenges.UsernameValue:
 		cert.EmailAddresses = []string{subject.Value}
 	}
-	cert.ExtraExtensions = append(IssuerExtension(subject.Issuer), AdditionalExtensions(subject)...)
+
+	exts := Extensions{
+		Issuer: subject.Issuer,
+	}
+	if subject.TypeVal == challenges.GithubWorkflowValue {
+		var ok bool
+		exts.GithubWorkflowTrigger, ok = subject.AdditionalInfo[challenges.GithubWorkflowTrigger]
+		if !ok {
+			return nil, errors.New("x509ca: github workflow missing trigger claim")
+		}
+		exts.GithubWorkflowSHA, ok = subject.AdditionalInfo[challenges.GithubWorkflowSha]
+		if !ok {
+			return nil, errors.New("x509ca: github workflow missing SHA claim")
+		}
+		exts.GithubWorkflowName, ok = subject.AdditionalInfo[challenges.GithubWorkflowName]
+		if !ok {
+			return nil, errors.New("x509ca: github workflow missing workflow name claim")
+		}
+		exts.GithubWorkflowRepository, ok = subject.AdditionalInfo[challenges.GithubWorkflowRepository]
+		if !ok {
+			return nil, errors.New("x509ca: github workflow missing repository claim")
+		}
+		exts.GithubWorkflowRef, ok = subject.AdditionalInfo[challenges.GithubWorkflowRef]
+		if !ok {
+			return nil, errors.New("x509ca: github workflow missing ref claim")
+		}
+	}
+
+	cert.ExtraExtensions, err = exts.Render()
+	if err != nil {
+		return nil, err
+	}
+
 	return cert, nil
 }
 
@@ -113,58 +143,6 @@ func (x *X509CA) CreateCertificate(_ context.Context, subject *challenges.Challe
 	return ca.CreateCSCFromDER(subject, finalCertBytes, []*x509.Certificate{x.RootCA})
 }
 
-func AdditionalExtensions(subject *challenges.ChallengeResult) []pkix.Extension {
-	res := []pkix.Extension{}
-	if subject.TypeVal == challenges.GithubWorkflowValue {
-		if trigger, ok := subject.AdditionalInfo[challenges.GithubWorkflowTrigger]; ok {
-			res = append(res, pkix.Extension{
-				Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 2},
-				Value: []byte(trigger),
-			})
-		}
-
-		if sha, ok := subject.AdditionalInfo[challenges.GithubWorkflowSha]; ok {
-			res = append(res, pkix.Extension{
-				Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 3},
-				Value: []byte(sha),
-			})
-		}
-
-		if name, ok := subject.AdditionalInfo[challenges.GithubWorkflowName]; ok {
-			res = append(res, pkix.Extension{
-				Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 4},
-				Value: []byte(name),
-			})
-		}
-
-		if repo, ok := subject.AdditionalInfo[challenges.GithubWorkflowRepository]; ok {
-			res = append(res, pkix.Extension{
-				Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 5},
-				Value: []byte(repo),
-			})
-		}
-
-		if ref, ok := subject.AdditionalInfo[challenges.GithubWorkflowRef]; ok {
-			res = append(res, pkix.Extension{
-				Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 6},
-				Value: []byte(ref),
-			})
-		}
-	}
-	return res
-}
-
-func IssuerExtension(issuer string) []pkix.Extension {
-	if issuer == "" {
-		return nil
-	}
-
-	return []pkix.Extension{{
-		Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 1},
-		Value: []byte(issuer),
-	}}
-}
-
 // GenerateSerialNumber creates a compliant serial number as per RFC 5280 4.1.2.2.
 // Serial numbers must be positive, and can be no longer than 20 bytes.
 // The serial number is generated with 159 bits, so that the first bit will always

pkg/ca/x509ca/common_test.go

@@ -15,8 +15,19 @@
 package x509ca
 
 import (
+	"bytes"
+	"crypto"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/x509"
+	"encoding/asn1"
+	"fmt"
 	"math/big"
 	"testing"
+
+	"github.com/pkg/errors"
+	"github.com/sigstore/fulcio/pkg/challenges"
 )
 
 func TestGenerateSerialNumber(t *testing.T) {
@@ -36,3 +47,102 @@ func TestGenerateSerialNumber(t *testing.T) {
 		t.Fatalf("serial number is too large: %v", serialNumber)
 	}
 }
+
+func mustNewTestPublicKey() crypto.PublicKey {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		panic(err)
+	}
+	return priv.Public()
+}
+
+func TestMakeCert(t *testing.T) {
+	tests := map[string]struct {
+		Challenge challenges.ChallengeResult
+		WantErr   bool
+		WantFacts map[string]func(x509.Certificate) error
+	}{
+		`Github workflow challenge should have all Github workflow extensions set`: {
+			Challenge: challenges.ChallengeResult{
+				Issuer:    `https://token.actions.githubusercontent.com`,
+				TypeVal:   challenges.GithubWorkflowValue,
+				PublicKey: mustNewTestPublicKey(),
+				Value:     `https://github.com/foo/bar/`,
+				AdditionalInfo: map[challenges.AdditionalInfo]string{
+					challenges.GithubWorkflowSha:        "sha",
+					challenges.GithubWorkflowTrigger:    "trigger",
+					challenges.GithubWorkflowName:       "workflowname",
+					challenges.GithubWorkflowRepository: "repository",
+					challenges.GithubWorkflowRef:        "ref",
+				},
+			},
+			WantErr: false,
+			WantFacts: map[string]func(x509.Certificate) error{
+				`Certifificate should have correct issuer`:     factIssuerIs(`https://token.actions.githubusercontent.com`),
+				`Certificate has correct trigger extension`:    factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 2}, "trigger"),
+				`Certificate has correct SHA extension`:        factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 3}, "sha"),
+				`Certificate has correct workflow extension`:   factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 4}, "workflowname"),
+				`Certificate has correct repository extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 5}, "repository"),
+				`Certificate has correct ref extension`:        factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 6}, "ref"),
+			},
+		},
+		`Email challenges should set issuer extension and email subject`: {
+			Challenge: challenges.ChallengeResult{
+				Issuer:    `example.com`,
+				TypeVal:   challenges.EmailValue,
+				PublicKey: mustNewTestPublicKey(),
+				Value:     `alice@example.com`,
+			},
+			WantErr: false,
+			WantFacts: map[string]func(x509.Certificate) error{
+				`Certificate should have alice@example.com email subject`: func(cert x509.Certificate) error {
+					if len(cert.EmailAddresses) != 1 {
+						return errors.New("no email SAN set for email challenge")
+					}
+					if cert.EmailAddresses[0] != `alice@example.com` {
+						return errors.New("bad email. expected alice@example.com")
+					}
+					return nil
+				},
+				`Certificate should have issuer extension set`: factIssuerIs("example.com"),
+			},
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			cert, err := MakeX509(&test.Challenge)
+			if err != nil {
+				if !test.WantErr {
+					t.Error(err)
+				}
+				return
+			}
+			for factName, fact := range test.WantFacts {
+				t.Run(factName, func(t *testing.T) {
+					if err := fact(*cert); err != nil {
+						t.Error(err)
+					}
+				})
+			}
+		})
+	}
+}
+
+func factIssuerIs(issuer string) func(x509.Certificate) error {
+	return factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 1}, issuer)
+}
+
+func factExtensionIs(oid asn1.ObjectIdentifier, value string) func(x509.Certificate) error {
+	return func(cert x509.Certificate) error {
+		for _, ext := range cert.ExtraExtensions {
+			if ext.Id.Equal(oid) {
+				if !bytes.Equal(ext.Value, []byte(value)) {
+					return fmt.Errorf("expected oid %v to be %s, but got %s", oid, value, ext.Value)
+				}
+				return nil
+			}
+		}
+		return errors.New("extension not set")
+	}
+}

pkg/ca/x509ca/extensions.go

@@ -0,0 +1,96 @@
+// 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 x509ca
+
+import (
+	"crypto/x509/pkix"
+	"encoding/asn1"
+	"errors"
+)
+
+// Extensions contains all custom x509 extensions defined by Fulcio
+type Extensions struct {
+	// NB: New extensions must be added here and documented
+	// at docs/oidc-info.md
+
+	// The OIDC issuer. Should match `iss` claim of ID token or, in the case
+	// of a federated login like Dex it should match the issuer URL of the upstream
+	// issuer
+	Issuer string // OID 1.3.6.1.4.1.57264.1.1
+
+	// Triggering event of the Github Workflow. Matches the `event_name` claim of ID
+	// tokens from Github Actions
+	GithubWorkflowTrigger string // OID 1.3.6.1.4.1.57264.1.2
+
+	// SHA of git commit being built in Github Actions. Matches the `sha` claim of ID
+	// tokens from Github Actions
+	GithubWorkflowSHA string // OID 1.3.6.1.4.1.57264.1.3
+
+	// Name of Github Actions Workflow. Matches the `workflow` claim of the ID
+	// tokens from Github Actions
+	GithubWorkflowName string // OID 1.3.6.1.4.1.57264.1.4
+
+	// Repository of the Github Actions Workflow. Matches the `repository` claim of the ID
+	// tokens from Github Actions
+	GithubWorkflowRepository string // OID 1.3.6.1.4.1.57264.1.5
+
+	// Git Ref of the Github Actions Workflow. Matches the `ref` claim of the ID tokens
+	// from Github Actions
+	GithubWorkflowRef string // 1.3.6.1.4.1.57264.1.6
+}
+
+func (e Extensions) Render() ([]pkix.Extension, error) {
+	var exts []pkix.Extension
+
+	if e.Issuer != "" {
+		exts = append(exts, pkix.Extension{
+			Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 1},
+			Value: []byte(e.Issuer),
+		})
+	} else {
+		return nil, errors.New("x509ca: extensions must have a non-empty issuer url")
+	}
+	if e.GithubWorkflowTrigger != "" {
+		exts = append(exts, pkix.Extension{
+			Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 2},
+			Value: []byte(e.GithubWorkflowTrigger),
+		})
+	}
+	if e.GithubWorkflowSHA != "" {
+		exts = append(exts, pkix.Extension{
+			Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 3},
+			Value: []byte(e.GithubWorkflowSHA),
+		})
+	}
+	if e.GithubWorkflowName != "" {
+		exts = append(exts, pkix.Extension{
+			Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 4},
+			Value: []byte(e.GithubWorkflowName),
+		})
+	}
+	if e.GithubWorkflowRepository != "" {
+		exts = append(exts, pkix.Extension{
+			Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 5},
+			Value: []byte(e.GithubWorkflowRepository),
+		})
+	}
+	if e.GithubWorkflowRef != "" {
+		exts = append(exts, pkix.Extension{
+			Id:    asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 6},
+			Value: []byte(e.GithubWorkflowRef),
+		})
+	}
+	return exts, nil
+}