crypto/x509: add signature verification to CreateCertificate
This changes checks the signature generated during CreateCertificate
and returns an error if the verification fails. A benchmark is also
added. For RSA keys the delta looks to be insignificant, but for
ECDSA keys it introduces a much larger delta which is not ideal.
name old time/op new time/op delta
RSA_2048-8 1.38ms ± 6% 1.41ms ± 2% ~ (p=0.182 n=10)
ECDSA_P256-8 42.6µs ± 4% 116.8µs ± 4% +174.00% (p=0.000 n=1
Fixes #40458
Change-Id: I22827795bb9bb6868b4fa47391927db1d3bc19a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/259697
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
Trust: Roland Shoemaker <roland@golang.org>
diff --git a/doc/go1.16.html b/doc/go1.16.html
index 509956f..2eb6166 100644
--- a/doc/go1.16.html
+++ b/doc/go1.16.html
@@ -201,6 +201,13 @@
contain strings with characters within the ASCII range.
</p>
+<p><!-- CL 259697 -->
+ <a href="/pkg/crypto/x509/#CreateCertificate">CreateCertificate</a> now
+ verifies the generated certificate's signature using the signer's
+ public key. If the signature is invalid, an error is returned, instead
+ of a malformed certificate.
+</p>
+
<h3 id="net"><a href="/pkg/net/">net</a></h3>
<p><!-- CL 250357 -->
diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 58c4aa3..bcef54d 100644
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -2145,12 +2145,22 @@
return
}
- return asn1.Marshal(certificate{
+ signedCert, err := asn1.Marshal(certificate{
nil,
c,
signatureAlgorithm,
asn1.BitString{Bytes: signature, BitLength: len(signature) * 8},
})
+ if err != nil {
+ return nil, err
+ }
+
+ // Check the signature to ensure the crypto.Signer behaved correctly.
+ if err := checkSignature(getSignatureAlgorithmFromAI(signatureAlgorithm), c.Raw, signature, key.Public()); err != nil {
+ return nil, fmt.Errorf("x509: signature over certificate returned by signer is invalid: %w", err)
+ }
+
+ return signedCert, nil
}
// pemCRLPrefix is the magic string that indicates that we have a PEM encoded
diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 2d9ace4..5a39e61 100644
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -22,6 +22,7 @@
"encoding/pem"
"fmt"
"internal/testenv"
+ "io"
"math/big"
"net"
"net/url"
@@ -2820,3 +2821,78 @@
}
}
}
+
+func BenchmarkCreateCertificate(b *testing.B) {
+ template := &Certificate{
+ SerialNumber: big.NewInt(10),
+ DNSNames: []string{"example.com"},
+ }
+ tests := []struct {
+ name string
+ gen func() crypto.Signer
+ }{
+ {
+ name: "RSA 2048",
+ gen: func() crypto.Signer {
+ k, err := rsa.GenerateKey(rand.Reader, 2048)
+ if err != nil {
+ b.Fatalf("failed to generate test key: %s", err)
+ }
+ return k
+ },
+ },
+ {
+ name: "ECDSA P256",
+ gen: func() crypto.Signer {
+ k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ if err != nil {
+ b.Fatalf("failed to generate test key: %s", err)
+ }
+ return k
+ },
+ },
+ }
+
+ for _, tc := range tests {
+ k := tc.gen()
+ b.ResetTimer()
+ b.Run(tc.name, func(b *testing.B) {
+ for i := 0; i < b.N; i++ {
+ _, err := CreateCertificate(rand.Reader, template, template, k.Public(), k)
+ if err != nil {
+ b.Fatalf("failed to create certificate: %s", err)
+ }
+ }
+ })
+ }
+}
+
+type brokenSigner struct {
+ pub crypto.PublicKey
+}
+
+func (bs *brokenSigner) Public() crypto.PublicKey {
+ return bs.pub
+}
+
+func (bs *brokenSigner) Sign(_ io.Reader, _ []byte, _ crypto.SignerOpts) ([]byte, error) {
+ return []byte{1, 2, 3}, nil
+}
+
+func TestCreateCertificateBrokenSigner(t *testing.T) {
+ template := &Certificate{
+ SerialNumber: big.NewInt(10),
+ DNSNames: []string{"example.com"},
+ }
+ k, err := rsa.GenerateKey(rand.Reader, 1024)
+ if err != nil {
+ t.Fatalf("failed to generate test key: %s", err)
+ }
+ expectedErr := "x509: signature over certificate returned by signer is invalid: crypto/rsa: verification error"
+ _, err = CreateCertificate(rand.Reader, template, template, k.Public(), &brokenSigner{k.Public()})
+ if err == nil {
+ t.Fatal("expected CreateCertificate to fail with a broken signer")
+ } else if err.Error() != expectedErr {
+ t.Fatalf("CreateCertificate returned an unexpected error: got %q, want %q", err, expectedErr)
+ }
+}