Skip to content

Commit

Permalink
Move VerifyCertChain to common library
Browse files Browse the repository at this point in the history
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
  • Loading branch information
haydentherapper committed Jun 10, 2022
1 parent 963c9f8 commit aa83789
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 129 deletions.
57 changes: 0 additions & 57 deletions pkg/ca/baseca/baseca.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"errors"

ct "github.com/google/certificate-transparency-go"
cttls "github.com/google/certificate-transparency-go/tls"
Expand Down Expand Up @@ -145,59 +144,3 @@ func (bca *BaseCA) Root(ctx context.Context) ([]byte, error) {
certs, _ := bca.GetSignerWithChain()
return cryptoutils.MarshalCertificatesToPEM(certs)
}

func VerifyCertChain(certs []*x509.Certificate, signer crypto.Signer) error {
if len(certs) == 0 {
return errors.New("certificate chain must contain at least one certificate")
}

roots := x509.NewCertPool()
roots.AddCert(certs[len(certs)-1])

intermediates := x509.NewCertPool()
if len(certs) > 1 {
for _, intermediate := range certs[1 : len(certs)-1] {
intermediates.AddCert(intermediate)
}
}

opts := x509.VerifyOptions{
Roots: roots,
Intermediates: intermediates,
KeyUsages: []x509.ExtKeyUsage{
x509.ExtKeyUsageCodeSigning,
},
}
if _, err := certs[0].Verify(opts); err != nil {
return err
}

if !certs[0].IsCA {
return errors.New("certificate is not a CA")
}

// If using an intermediate, verify that code signing extended key
// usage is set to satify extended key usage chainging
if len(certs) > 1 {
var hasExtKeyUsageCodeSigning bool
for _, extKeyUsage := range certs[0].ExtKeyUsage {
if extKeyUsage == x509.ExtKeyUsageCodeSigning {
hasExtKeyUsageCodeSigning = true
break
}
}
if !hasExtKeyUsageCodeSigning {
return errors.New(`certificate must have extended key usage code signing set to sign code signing certificates`)
}
}

if err := cryptoutils.EqualKeys(certs[0].PublicKey, signer.Public()); err != nil {
return err
}

if err := cryptoutils.ValidatePubKey(signer.Public()); err != nil {
return err
}

return nil
}
69 changes: 0 additions & 69 deletions pkg/ca/baseca/baseca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"crypto/x509"
"encoding/asn1"
"reflect"
"strings"
"testing"

ct "github.com/google/certificate-transparency-go"
Expand Down Expand Up @@ -86,74 +85,6 @@ func TestBaseCAGetSignerWithChain(t *testing.T) {
}
}

func TestBaseCAVerifyCertChain(t *testing.T) {
rootCert, rootKey, _ := test.GenerateRootCA()
subCert, subKey, _ := test.GenerateSubordinateCA(rootCert, rootKey)
leafCert, _, _ := test.GenerateLeafCert("subject", "oidc-issuer", subCert, subKey)

err := VerifyCertChain([]*x509.Certificate{subCert, rootCert}, subKey)
if err != nil {
t.Fatalf("unexpected error verifying cert chain: %v", err)
}

// Handles single certifiacte in chain
err = VerifyCertChain([]*x509.Certificate{rootCert}, rootKey)
if err != nil {
t.Fatalf("unexpected error verifying single cert chain: %v", err)
}

// Handles multiple intermediates
subCert2, subKey2, _ := test.GenerateSubordinateCA(subCert, subKey)
err = VerifyCertChain([]*x509.Certificate{subCert2, subCert, rootCert}, subKey2)
if err != nil {
t.Fatalf("unexpected error verifying cert chain: %v", err)
}

// Failure: Certificate is not a CA certificate
err = VerifyCertChain([]*x509.Certificate{leafCert}, nil)
if err == nil || !strings.Contains(err.Error(), "certificate is not a CA") {
t.Fatalf("expected error with non-CA cert: %v", err)
}

// Failure: Certificate missing EKU
// Note that the wrong EKU will be caught by x509.Verify
invalidSubCert, invalidSubKey, _ := test.GenerateSubordinateCAWithoutEKU(rootCert, rootKey)
err = VerifyCertChain([]*x509.Certificate{invalidSubCert, rootCert}, invalidSubKey)
if err == nil || !strings.Contains(err.Error(), "certificate must have extended key usage code signing") {
t.Fatalf("expected error verifying cert chain without EKU: %v", err)
}

// Failure: Invalid chain
rootCert2, _, _ := test.GenerateRootCA()
err = VerifyCertChain([]*x509.Certificate{subCert, rootCert2}, subKey)
if err == nil || !strings.Contains(err.Error(), "certificate signed by unknown authority") {
t.Fatalf("expected error verifying cert chain: %v", err)
}

// Failure: Different signer with different key
signer, _, err := signature.NewDefaultECDSASignerVerifier()
if err != nil {
t.Fatalf("expected error generating signer: %v", err)
}
err = VerifyCertChain([]*x509.Certificate{subCert, rootCert}, signer)
if err == nil || !strings.Contains(err.Error(), "public keys are not equal") {
t.Fatalf("expected error verifying cert with mismatched public keys: %v", err)
}

// Failure: Weak key
weakSubCert, weakSubKey, _ := test.GenerateWeakSubordinateCA(rootCert, rootKey)
err = VerifyCertChain([]*x509.Certificate{weakSubCert, rootCert}, weakSubKey)
if err == nil || !strings.Contains(err.Error(), "unsupported ec curve") {
t.Fatalf("expected error verifying weak cert chain: %v", err)
}

// Failure: Empty chain
err = VerifyCertChain([]*x509.Certificate{}, weakSubKey)
if err == nil || !strings.Contains(err.Error(), "certificate chain must contain at least one certificate") {
t.Fatalf("expected error verifying with empty chain: %v", err)
}
}

type testPrincipal struct{}

func (tp testPrincipal) Name(context.Context) string {
Expand Down
57 changes: 57 additions & 0 deletions pkg/ca/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
"crypto"
"crypto/x509"
"errors"
"time"

"github.com/sigstore/fulcio/pkg/identity"
Expand Down Expand Up @@ -52,3 +53,59 @@ func MakeX509(ctx context.Context, principal identity.Principal, publicKey crypt

return cert, nil
}

func VerifyCertChain(certs []*x509.Certificate, signer crypto.Signer) error {
if len(certs) == 0 {
return errors.New("certificate chain must contain at least one certificate")
}

roots := x509.NewCertPool()
roots.AddCert(certs[len(certs)-1])

intermediates := x509.NewCertPool()
if len(certs) > 1 {
for _, intermediate := range certs[1 : len(certs)-1] {
intermediates.AddCert(intermediate)
}
}

opts := x509.VerifyOptions{
Roots: roots,
Intermediates: intermediates,
KeyUsages: []x509.ExtKeyUsage{
x509.ExtKeyUsageCodeSigning,
},
}
if _, err := certs[0].Verify(opts); err != nil {
return err
}

if !certs[0].IsCA {
return errors.New("certificate is not a CA")
}

// If using an intermediate, verify that code signing extended key
// usage is set to satify extended key usage chainging
if len(certs) > 1 {
var hasExtKeyUsageCodeSigning bool
for _, extKeyUsage := range certs[0].ExtKeyUsage {
if extKeyUsage == x509.ExtKeyUsageCodeSigning {
hasExtKeyUsageCodeSigning = true
break
}
}
if !hasExtKeyUsageCodeSigning {
return errors.New(`certificate must have extended key usage code signing set to sign code signing certificates`)
}
}

if err := cryptoutils.EqualKeys(certs[0].PublicKey, signer.Public()); err != nil {
return err
}

if err := cryptoutils.ValidatePubKey(signer.Public()); err != nil {
return err
}

return nil
}
72 changes: 72 additions & 0 deletions pkg/ca/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@ import (
"crypto/elliptic"
"crypto/rand"
"crypto/x509"
"strings"
"testing"
"time"

"github.com/sigstore/fulcio/pkg/test"
"github.com/sigstore/sigstore/pkg/signature"
)

// cannot use ChallengeResult due to import cycle
Expand Down Expand Up @@ -65,3 +69,71 @@ func TestMakeX509(t *testing.T) {
t.Fatalf("expected email in subject alt name, got %v", cert.EmailAddresses)
}
}

func TestVerifyCertChain(t *testing.T) {
rootCert, rootKey, _ := test.GenerateRootCA()
subCert, subKey, _ := test.GenerateSubordinateCA(rootCert, rootKey)
leafCert, _, _ := test.GenerateLeafCert("subject", "oidc-issuer", subCert, subKey)

err := VerifyCertChain([]*x509.Certificate{subCert, rootCert}, subKey)
if err != nil {
t.Fatalf("unexpected error verifying cert chain: %v", err)
}

// Handles single certifiacte in chain
err = VerifyCertChain([]*x509.Certificate{rootCert}, rootKey)
if err != nil {
t.Fatalf("unexpected error verifying single cert chain: %v", err)
}

// Handles multiple intermediates
subCert2, subKey2, _ := test.GenerateSubordinateCA(subCert, subKey)
err = VerifyCertChain([]*x509.Certificate{subCert2, subCert, rootCert}, subKey2)
if err != nil {
t.Fatalf("unexpected error verifying cert chain: %v", err)
}

// Failure: Certificate is not a CA certificate
err = VerifyCertChain([]*x509.Certificate{leafCert}, nil)
if err == nil || !strings.Contains(err.Error(), "certificate is not a CA") {
t.Fatalf("expected error with non-CA cert: %v", err)
}

// Failure: Certificate missing EKU
// Note that the wrong EKU will be caught by x509.Verify
invalidSubCert, invalidSubKey, _ := test.GenerateSubordinateCAWithoutEKU(rootCert, rootKey)
err = VerifyCertChain([]*x509.Certificate{invalidSubCert, rootCert}, invalidSubKey)
if err == nil || !strings.Contains(err.Error(), "certificate must have extended key usage code signing") {
t.Fatalf("expected error verifying cert chain without EKU: %v", err)
}

// Failure: Invalid chain
rootCert2, _, _ := test.GenerateRootCA()
err = VerifyCertChain([]*x509.Certificate{subCert, rootCert2}, subKey)
if err == nil || !strings.Contains(err.Error(), "certificate signed by unknown authority") {
t.Fatalf("expected error verifying cert chain: %v", err)
}

// Failure: Different signer with different key
signer, _, err := signature.NewDefaultECDSASignerVerifier()
if err != nil {
t.Fatalf("expected error generating signer: %v", err)
}
err = VerifyCertChain([]*x509.Certificate{subCert, rootCert}, signer)
if err == nil || !strings.Contains(err.Error(), "public keys are not equal") {
t.Fatalf("expected error verifying cert with mismatched public keys: %v", err)
}

// Failure: Weak key
weakSubCert, weakSubKey, _ := test.GenerateWeakSubordinateCA(rootCert, rootKey)
err = VerifyCertChain([]*x509.Certificate{weakSubCert, rootCert}, weakSubKey)
if err == nil || !strings.Contains(err.Error(), "unsupported ec curve") {
t.Fatalf("expected error verifying weak cert chain: %v", err)
}

// Failure: Empty chain
err = VerifyCertChain([]*x509.Certificate{}, weakSubKey)
if err == nil || !strings.Contains(err.Error(), "certificate chain must contain at least one certificate") {
t.Fatalf("expected error verifying with empty chain: %v", err)
}
}
3 changes: 1 addition & 2 deletions pkg/ca/fileca/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"path/filepath"

"github.com/sigstore/fulcio/pkg/ca"
"github.com/sigstore/fulcio/pkg/ca/baseca"
"github.com/sigstore/sigstore/pkg/cryptoutils"
"go.step.sm/crypto/pemutil"
)
Expand Down Expand Up @@ -58,7 +57,7 @@ func loadKeyPair(certPath, keyPath, keyPass string) (*ca.SignerCertsMutex, error
}
}

if err := baseca.VerifyCertChain(certs, key); err != nil {
if err := ca.VerifyCertChain(certs, key); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/ca/kmsca/kmsca.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func NewKMSCA(ctx context.Context, kmsKey, certPath string) (ca.CertificateAutho
if err != nil {
return nil, err
}
if err := baseca.VerifyCertChain(sc.Certs, sc.Signer); err != nil {
if err := ca.VerifyCertChain(sc.Certs, sc.Signer); err != nil {
return nil, err
}

Expand Down

0 comments on commit aa83789

Please sign in to comment.