Merge pull request #472 from hectorj2f/cert_chain_validation

chore: add TSA cert chain validation

pkg/apis/config/sigstore_keys.go

@@ -92,7 +92,7 @@ type CertificateAuthority struct {
 // SigstoreKeys.
 type SigstoreKeys struct {
 	// Trusted certificate authorities (e.g Fulcio).
-	CertificateAuthorities []CertificateAuthority `json:"certificateAuthorities"`
+	CertificateAuthorities []CertificateAuthority `json:"certificateAuthorities,omitempty"`
 	// Rekor log specifications
 	TLogs []TransparencyLogInstance `json:"tLogs,omitempty"`
 	// Certificate Transparency Log

pkg/apis/policy/v1alpha1/trustroot_validation.go

@@ -15,10 +15,13 @@
 package v1alpha1
 
 import (
+	"bytes"
 	"context"
+	"crypto/x509"
 	"encoding/json"
 
 	"github.com/sigstore/policy-controller/pkg/tuf"
+	"github.com/sigstore/sigstore/pkg/cryptoutils"
 
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/logging"
@@ -110,7 +113,7 @@ func (sigstoreKeys *SigstoreKeys) Validate(ctx context.Context) (errors *apis.Fi
 	// These are optionals, so we just validate them if they are there and do
 	// not report them as missing.
 	for i, tsa := range sigstoreKeys.TimeStampAuthorities {
-		errors = ValidateCertificateAuthority(ctx, tsa).ViaFieldIndex("timestampAuthorities", i)
+		errors = ValidateTimeStampAuthority(ctx, tsa).ViaFieldIndex("timestampAuthorities", i)
 	}
 	for i, ctl := range sigstoreKeys.CTLogs {
 		errors = ValidateTransparencyLogInstance(ctx, ctl).ViaFieldIndex("ctLogs", i)
@@ -142,9 +145,26 @@ func ValidateCertificateAuthority(ctx context.Context, ca CertificateAuthority)
 		errors = errors.Also(apis.ErrMissingField("uri"))
 	}
 	if len(ca.CertChain) == 0 {
-		errors = errors.Also(apis.ErrMissingField("certChain"))
+		errors = errors.Also(apis.ErrMissingField("certchain"))
+	}
+	return
+}
+
+func ValidateTimeStampAuthority(ctx context.Context, ca CertificateAuthority) (errors *apis.FieldError) {
+	errors = errors.Also(ValidateDistinguishedName(ctx, ca.Subject)).ViaField("subject")
+	if ca.URI.String() == "" {
+		errors = errors.Also(apis.ErrMissingField("uri"))
+	}
+	if len(ca.CertChain) == 0 {
+		errors = errors.Also(apis.ErrMissingField("certchain"))
+	}
+	leaves, _, _, err := SplitPEMCertificateChain(ca.CertChain)
+	if err != nil {
+		errors = errors.Also(apis.ErrInvalidValue("error splitting the certificates", "certChain", err.Error()))
+	}
+	if len(leaves) > 1 {
+		errors = errors.Also(apis.ErrInvalidValue("certificate chain must contain at most one TSA certificate", "certChain"))
 	}
-	// TODO: Validate the certchain more thorougly.
 	return
 }
 
@@ -170,3 +190,27 @@ func ValidateTransparencyLogInstance(ctx context.Context, tli TransparencyLogIns
 	}
 	return
 }
+
+// SplitPEMCertificateChain returns a list of leaf (non-CA) certificates, a certificate pool for
+// intermediate CA certificates, and a certificate pool for root CA certificates
+func SplitPEMCertificateChain(pem []byte) (leaves, intermediates, roots []*x509.Certificate, err error) {
+	certs, err := cryptoutils.UnmarshalCertificatesFromPEM(pem)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+
+	for _, cert := range certs {
+		if !cert.IsCA {
+			leaves = append(leaves, cert)
+		} else {
+			// root certificates are self-signed
+			if bytes.Equal(cert.RawSubject, cert.RawIssuer) {
+				roots = append(roots, cert)
+			} else {
+				intermediates = append(intermediates, cert)
+			}
+		}
+	}
+
+	return leaves, intermediates, roots, nil
+}

pkg/apis/policy/v1alpha1/trustroot_validation_test.go

@@ -16,8 +16,13 @@ package v1alpha1
 
 import (
 	"context"
+	"crypto/x509"
 	"encoding/base64"
 	"testing"
+
+	"github.com/sigstore/policy-controller/test"
+	"github.com/sigstore/sigstore/pkg/cryptoutils"
+	"knative.dev/pkg/apis"
 )
 
 // validRepository is a TUF repository that's been tarred, gzipped and base64
@@ -112,3 +117,66 @@ func TestTrustRootValidation(t *testing.T) {
 		})
 	}
 }
+
+func TestTimeStampAuthorityValidation(t *testing.T) {
+	rootCert, rootKey, _ := test.GenerateRootCa()
+	subCert, subKey, _ := test.GenerateSubordinateCa(rootCert, rootKey)
+	leafCert, _, _ := test.GenerateLeafCert("subject", "oidc-issuer", subCert, subKey)
+	rootCert2, rootKey2, _ := test.GenerateRootCa()
+	subCert2, subKey2, _ := test.GenerateSubordinateCa(rootCert2, rootKey2)
+	leafCert2, _, _ := test.GenerateLeafCert("subject", "oidc-issuer", subCert2, subKey2)
+
+	pem, err := cryptoutils.MarshalCertificatesToPEM([]*x509.Certificate{rootCert, subCert, leafCert})
+	if err != nil {
+		t.Fatalf("unexpected error marshalling certificates to PEM: %v", err)
+	}
+	tooManyLeavesPem, err := cryptoutils.MarshalCertificatesToPEM([]*x509.Certificate{rootCert, subCert, leafCert, leafCert2})
+	if err != nil {
+		t.Fatalf("unexpected error marshalling certificates to PEM: %v", err)
+	}
+
+	tests := []struct {
+		name        string
+		tsa         CertificateAuthority
+		errorString string
+	}{{
+		name: "Should work with a valid repository",
+		tsa: CertificateAuthority{
+			Subject: DistinguishedName{
+				Organization: "fulcio-organization",
+				CommonName:   "fulcio-common-name",
+			},
+			URI:       *apis.HTTPS("fulcio.example.com"),
+			CertChain: pem,
+		},
+	}, {
+		name:        "Should fail splitting the certificates of the certChain",
+		errorString: "invalid value: error splitting the certificates: certChain\nerror during PEM decoding",
+		tsa: CertificateAuthority{
+			Subject: DistinguishedName{
+				Organization: "fulcio-organization",
+				CommonName:   "fulcio-common-name",
+			},
+			URI:       *apis.HTTPS("fulcio.example.com"),
+			CertChain: []byte("INVALID"),
+		},
+	}, {
+		name:        "Should fail with a must contain at most one TSA certificate",
+		errorString: "invalid value: certificate chain must contain at most one TSA certificate: certChain",
+		tsa: CertificateAuthority{
+			Subject: DistinguishedName{
+				Organization: "fulcio-organization",
+				CommonName:   "fulcio-common-name",
+			},
+			URI:       *apis.HTTPS("fulcio.example.com"),
+			CertChain: tooManyLeavesPem,
+		},
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := ValidateTimeStampAuthority(context.TODO(), test.tsa)
+			validateError(t, test.errorString, "", err)
+		})
+	}
+}