Skip to content

Commit

Permalink
fix: added tsa trust store root cert validation (notaryproject#471)
Browse files Browse the repository at this point in the history
This PR adds tsa trust store root cert validation while getting
certificates from trust store. This is to fail fast if cert in TSA trust
store is not a root CA certificate.

Resolves notaryproject#470

---------

Signed-off-by: Patrick Zheng <[email protected]>
  • Loading branch information
Two-Hearts committed Dec 9, 2024
1 parent 3c33022 commit d66c7ab
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 0 deletions.
Binary file not shown.
Binary file not shown.
20 changes: 20 additions & 0 deletions verifier/truststore/truststore.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package truststore

import (
"bytes"
"context"
"crypto/x509"
"errors"
Expand Down Expand Up @@ -106,6 +107,14 @@ func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType
if err := ValidateCertificates(certs); err != nil {
return nil, CertificateError{InnerError: err, Msg: fmt.Sprintf("failed to validate the trusted certificate %s in trust store %s of type %s", certFileName, namedStore, storeType)}
}
// we require TSA certificates in trust store to be root CA certificates
if storeType == TypeTSA {
for _, cert := range certs {
if err := isRootCACertificate(cert); err != nil {
return nil, CertificateError{InnerError: err, Msg: fmt.Sprintf("trusted certificate %s in trust store %s of type %s is invalid: %v", certFileName, namedStore, storeType, err.Error())}
}
}
}
certificates = append(certificates, certs...)
}
if len(certificates) < 1 {
Expand Down Expand Up @@ -137,3 +146,14 @@ func ValidateCertificates(certs []*x509.Certificate) error {
func isValidStoreType(storeType Type) bool {
return slices.Contains(Types, storeType)
}

// isRootCACertificate returns nil if cert is a root CA certificate
func isRootCACertificate(cert *x509.Certificate) error {
if err := cert.CheckSignatureFrom(cert); err != nil {
return fmt.Errorf("certificate with subject %q is not a root CA certificate: %w", cert.Subject, err)
}
if !bytes.Equal(cert.RawSubject, cert.RawIssuer) {
return fmt.Errorf("certificate with subject %q is not a root CA certificate: issuer (%s) and subject (%s) are not the same", cert.Subject, cert.Issuer, cert.Subject)
}
return nil
}
28 changes: 28 additions & 0 deletions verifier/truststore/truststore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,31 @@ func TestValidateCertsWithLeafCert(t *testing.T) {
t.Fatalf("leaf cert in a trust store should return error %q, got: %v", expectedErr, err)
}
}

func TestGetCertFromValidTsaTrustStore(t *testing.T) {
// testing ../testdata/truststore/x509/tsa/test-nonCA/globalsignRoot.cer
_, err := trustStore.GetCertificates(context.Background(), "tsa", "test-timestamp")
if err != nil {
t.Fatalf("expected nil error, but got %s", err)
}
}

func TestGetCertFromInvalidTsaTrustStore(t *testing.T) {
t.Run("non CA certificate", func(t *testing.T) {
// testing ../testdata/truststore/x509/tsa/test-nonCA/wabbit-networks.io
expectedErrMsg := `trusted certificate wabbit-networks.io.crt in trust store test-nonCA of type tsa is invalid: certificate with subject "CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US" is not a root CA certificate: x509: invalid signature: parent certificate cannot sign this kind of certificate`
_, err := trustStore.GetCertificates(context.Background(), "tsa", "test-nonCA")
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("expected error: %s, but got %s", expectedErrMsg, err)
}
})

t.Run("not self-issued", func(t *testing.T) {
//testing ../testdata/truststore/x509/tsa/test-nonSelfIssued/nonSelfIssued.crt
expectedErrMsg := `trusted certificate nonSelfIssued.crt in trust store test-nonSelfIssued of type tsa is invalid: certificate with subject "CN=Notation Test Revokable RSA Chain Cert 2,O=Notary,L=Seattle,ST=WA,C=US" is not a root CA certificate: issuer (CN=Notation Test Revokable RSA Chain Cert Root,O=Notary,L=Seattle,ST=WA,C=US) and subject (CN=Notation Test Revokable RSA Chain Cert 2,O=Notary,L=Seattle,ST=WA,C=US) are not the same`
_, err := trustStore.GetCertificates(context.Background(), "tsa", "test-nonSelfIssued")
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("expected error: %s, but got %s", expectedErrMsg, err)
}
})
}

0 comments on commit d66c7ab

Please sign in to comment.