diff --git a/x509/codesigning_cert_validations.go b/x509/codesigning_cert_validations.go index e074cef8..52db95f4 100644 --- a/x509/codesigning_cert_validations.go +++ b/x509/codesigning_cert_validations.go @@ -14,6 +14,7 @@ package x509 import ( + "bytes" "crypto/x509" "errors" "fmt" @@ -34,12 +35,17 @@ func ValidateCodeSigningCertChain(certChain []*x509.Certificate, signingTime *ti // For self-signed signing certificate (not a CA) if len(certChain) == 1 { cert := certChain[0] - if signedTimeError := validateSigningTime(cert, signingTime); signedTimeError != nil { - return signedTimeError - } + // check self-signed if err := cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature); err != nil { return fmt.Errorf("invalid self-signed certificate. subject: %q. Error: %w", cert.Subject, err) } + // check self-issued + if !bytes.Equal(cert.RawSubject, cert.RawIssuer) { + return fmt.Errorf("invalid self-signed certificate. subject: %q. Error: issuer(%s) and subject(%s) are not the same", cert.Subject, cert.Issuer, cert.Subject) + } + if signedTimeError := validateSigningTime(cert, signingTime); signedTimeError != nil { + return signedTimeError + } if err := validateCodeSigningLeafCertificate(cert); err != nil { return fmt.Errorf("invalid self-signed certificate. Error: %w", err) } diff --git a/x509/codesigning_cert_validations_test.go b/x509/codesigning_cert_validations_test.go index dcb2e230..31dfa8e0 100644 --- a/x509/codesigning_cert_validations_test.go +++ b/x509/codesigning_cert_validations_test.go @@ -19,6 +19,7 @@ import ( _ "embed" "errors" "os" + "strings" "testing" "time" @@ -200,6 +201,35 @@ func TestFailEmptyChain(t *testing.T) { assertErrorEqual("certificate chain must contain at least one certificate", err, t) } +func TestFailNonSelfSignedLeafCert(t *testing.T) { + signingTime := time.Now() + err := ValidateCodeSigningCertChain([]*x509.Certificate{codeSigningCert}, &signingTime) + + assertErrorEqual("invalid self-signed certificate. subject: \"CN=CodeSigningLeaf\". Error: crypto/rsa: verification error", err, t) +} + +func TestFailSelfIssuedCodeSigningCert(t *testing.T) { + chainTuple := testhelper.GetRevokableRSATimestampChain(2) + // the leaf certiifcate and the root certificate share the same private key + // so the leaf is also self-signed but issuer and subject are different + chain := []*x509.Certificate{chainTuple[0].Cert} + signingTime := time.Now() + err := ValidateCodeSigningCertChain(chain, &signingTime) + assertErrorEqual("invalid self-signed certificate. subject: \"CN=Notation Test Revokable RSA Chain Cert 2,O=Notary,L=Seattle,ST=WA,C=US\". Error: 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, t) +} + +func TestInvalidCodeSigningCertSigningTime(t *testing.T) { + chainTuple := testhelper.GetRevokableRSATimestampChain(2) + chain := []*x509.Certificate{chainTuple[1].Cert} + signingTime := time.Date(2021, 7, 7, 20, 48, 42, 0, time.UTC) + + expectPrefix := "certificate with subject \"CN=Notation Test Revokable RSA Chain Cert Root,O=Notary,L=Seattle,ST=WA,C=US\" was invalid at signing time of 2021-07-07 20:48:42 +0000 UTC" + err := ValidateCodeSigningCertChain(chain, &signingTime) + if !strings.HasPrefix(err.Error(), expectPrefix) { + t.Errorf("expected error to start with %q, got %q", expectPrefix, err) + } +} + func TestFailInvalidSigningTime(t *testing.T) { certChain := []*x509.Certificate{codeSigningCert, intermediateCert2, intermediateCert1, rootCert} diff --git a/x509/timestamp_cert_validations.go b/x509/timestamp_cert_validations.go index adb46e60..07c0d5eb 100644 --- a/x509/timestamp_cert_validations.go +++ b/x509/timestamp_cert_validations.go @@ -14,6 +14,7 @@ package x509 import ( + "bytes" "crypto/x509" "errors" "fmt" @@ -33,9 +34,14 @@ func ValidateTimestampingCertChain(certChain []*x509.Certificate) error { // For self-signed signing certificate (not a CA) if len(certChain) == 1 { cert := certChain[0] + // check self-signed if err := cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature); err != nil { return fmt.Errorf("invalid self-signed certificate. subject: %q. Error: %w", cert.Subject, err) } + // check self-issued + if !bytes.Equal(cert.RawSubject, cert.RawIssuer) { + return fmt.Errorf("invalid self-signed certificate. subject: %q. Error: issuer (%s) and subject (%s) are not the same", cert.Subject, cert.Issuer, cert.Subject) + } if err := validateTimestampingLeafCertificate(cert); err != nil { return fmt.Errorf("invalid self-signed certificate. Error: %w", err) } diff --git a/x509/timestamp_cert_validations_test.go b/x509/timestamp_cert_validations_test.go index cfede2da..ce042839 100644 --- a/x509/timestamp_cert_validations_test.go +++ b/x509/timestamp_cert_validations_test.go @@ -14,9 +14,18 @@ package x509 import ( + "crypto/rand" + "crypto/rsa" "crypto/x509" "crypto/x509/pkix" + "encoding/asn1" + "math/big" + "strings" "testing" + "time" + + "github.com/notaryproject/notation-core-go/internal/oid" + "github.com/notaryproject/notation-core-go/testhelper" ) func TestValidTimestampingChain(t *testing.T) { @@ -39,6 +48,42 @@ func TestValidTimestampingChain(t *testing.T) { } } +func TestFailSelfIssuedTimestampingCert(t *testing.T) { + chainTuple := testhelper.GetRevokableRSATimestampChain(2) + // the leaf certiifcate and the root certificate share the same private key + // so the leaf is also self-signed but issuer and subject are different + chain := []*x509.Certificate{chainTuple[0].Cert} + err := ValidateTimestampingCertChain(chain) + assertErrorEqual("invalid self-signed certificate. subject: \"CN=Notation Test Revokable RSA Chain Cert 2,O=Notary,L=Seattle,ST=WA,C=US\". Error: 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, t) +} + +func TestInvalidTimestampSelfSignedCert(t *testing.T) { + cert, err := createSelfSignedCert("valid cert", "valid cert", false) + if err != nil { + t.Error(err) + } + certChain := []*x509.Certificate{cert} + + expectPrefix := "invalid self-signed certificate. Error: timestamp signing certificate with subject \"CN=valid cert\" must have and only have Timestamping as extended key usage" + err = ValidateTimestampingCertChain(certChain) + if !strings.HasPrefix(err.Error(), expectPrefix) { + t.Errorf("expected error to start with %q, got %q", expectPrefix, err) + } +} + +func TestValidTimestampSelfSignedCert(t *testing.T) { + cert, err := createSelfSignedCert("valid cert", "valid cert", true) + if err != nil { + t.Error(err) + } + certChain := []*x509.Certificate{cert} + + err = ValidateTimestampingCertChain(certChain) + if err != nil { + t.Error(err) + } +} + func TestInvalidTimestampingChain(t *testing.T) { timestamp_leaf, err := readSingleCertificate("testdata/timestamp_leaf.crt") if err != nil { @@ -215,3 +260,47 @@ func TestEkuToString(t *testing.T) { t.Fatalf("expected 7") } } + +func createSelfSignedCert(subject string, issuer string, isTimestamp bool) (*x509.Certificate, error) { + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, err + } + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: subject}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + + if isTimestamp { + oids := []asn1.ObjectIdentifier{{1, 3, 6, 1, 5, 5, 7, 3, 8}} + value, err := asn1.Marshal(oids) + if err != nil { + return nil, err + } + template.ExtraExtensions = []pkix.Extension{{ + Id: oid.ExtKeyUsage, + Critical: true, + Value: value, + }} + template.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageTimeStamping} + } + + parentTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(2), + Subject: pkix.Name{CommonName: issuer}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageCertSign, + } + + certDER, err := x509.CreateCertificate(rand.Reader, template, parentTemplate, &priv.PublicKey, priv) + if err != nil { + return nil, err + } + + return x509.ParseCertificate(certDER) +}