crypto/x509: truncate signed hash before DSA signature verification

According to spec, the hash must be truncated, but crypto/dsa
does not do it. We can't fix it in crypto/dsa, because it would break
verification of previously generated signatures.
In crypto/x509 however, go can't generate DSA certs, only verify them,
so the fix here should be safe.

Fixes #22017

Change-Id: Iee7e20a5d76f45da8901a7ca686063639092949f
GitHub-Last-Rev: 8041cde8d25d3a336b81d86bd52bff5039568246
GitHub-Pull-Request: golang/go#34630
Reviewed-on: https://go-review.googlesource.com/c/go/+/198138
Reviewed-by: Filippo Valsorda <filippo@golang.org>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 9b470339471998ef95ca452bcf11c0ebd27d3022..cc382e52c64d3396611e26f0a526421c129dea24 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -892,6 +892,11 @@ func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey
                if dsaSig.R.Sign() <= 0 || dsaSig.S.Sign() <= 0 {
                        return errors.New("x509: DSA signature contained zero or negative values")
                }
+               // According to FIPS 186-3, section 4.6, the hash must be truncated if it is longer
+               // than the key length, but crypto/dsa doesn't do it automatically.
+               if maxHashLen := pub.Q.BitLen() / 8; maxHashLen < len(signed) {
+                       signed = signed[:maxHashLen]
+               }
                if !dsa.Verify(pub, signed, dsaSig.R, dsaSig.S) {
                        return errors.New("x509: DSA verification failure")
                }

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 1aaf0939373b3be630cb3de3cd4ec2ced2043cf7..d5b168e78ff55704924cdfc692acb205b0ee961d 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -975,6 +975,49 @@ func TestVerifyCertificateWithDSASignature(t *testing.T) {
        }
 }
 
+const dsaCert1024WithSha256 = `-----BEGIN CERTIFICATE-----
+MIIDKzCCAumgAwIBAgIUOXWPK4gTRZVVY7OSXTU00QEWQU8wCwYJYIZIAWUDBAMC
+MEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJ
+bnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwIBcNMTkxMDAxMDYxODUyWhgPMzAxOTAy
+MDEwNjE4NTJaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw
+HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggG4MIIBLAYHKoZIzjgE
+ATCCAR8CgYEAr79m/1ypU1aUbbLX1jikTyX7w2QYP+EkxNtXUiiTuxkC1KBqqxT3
+0Aht2vxFR47ODEK4B79rHO+UevhaqDaAHSH7Z/9umS0h0aS32KLDLb+LI5AneCrn
+eW5YbVhfD03N7uR4kKUCKOnWj5hAk9xiE3y7oFR0bBXzqrrHJF9LMd0CFQCB6lSj
+HSW0rGmNxIZsBl72u7JFLQKBgQCOFd1PGEQmddn0cdFgby5QQfjrqmoD1zNlFZEt
+L0x1EbndFwelLlF1ChNh3NPNUkjwRbla07FDlONs1GMJq6w4vW11ns+pUvAZ2+RM
+EVFjugip8az2ncn3UujGTVdFxnSTLBsRlMP/tFDK3ky//8zn/5ha9SKKw4v1uv6M
+JuoIbwOBhQACgYEAoeKeR90nwrnoPi5MOUPBLQvuzB87slfr+3kL8vFCmgjA6MtB
+7TxQKoBTOo5aVgWDp0lMIMxLd6btzBrm6r3VdRlh/cL8/PtbxkFwBa+Upe4o5NAh
+ISCe2/f2leT1PxtF8xxYjz/fszeUeHsJbVMilE2cuB2SYrR5tMExiqy+QpqjUzBR
+MB0GA1UdDgQWBBQDMIEL8Z3jc1d9wCxWtksUWc8RkjAfBgNVHSMEGDAWgBQDMIEL
+8Z3jc1d9wCxWtksUWc8RkjAPBgNVHRMBAf8EBTADAQH/MAsGCWCGSAFlAwQDAgMv
+ADAsAhQFehZgI4OyKBGpfnXvyJ0Z/0a6nAIUTO265Ane87LfJuQr3FrqvuCI354=
+-----END CERTIFICATE-----
+`
+
+func TestVerifyCertificateWithDSATooLongHash(t *testing.T) {
+       pemBlock, _ := pem.Decode([]byte(dsaCert1024WithSha256))
+       cert, err := ParseCertificate(pemBlock.Bytes)
+       if err != nil {
+               t.Fatalf("Failed to parse certificate: %s", err)
+       }
+
+       // test cert is self-signed
+       if err = cert.CheckSignatureFrom(cert); err != nil {
+               t.Fatalf("DSA Certificate self-signature verification failed: %s", err)
+       }
+
+       signed := []byte("A wild Gopher appears!\n")
+       signature, _ := hex.DecodeString("302c0214417aca7ff458f5b566e43e7b82f994953da84be50214625901e249e33f4e4838f8b5966020c286dd610e")
+
+       // This signature is using SHA256, but only has 1024 DSA key. The hash has to be truncated
+       // in CheckSignature, otherwise it won't pass.
+       if err = cert.CheckSignature(DSAWithSHA256, signed, signature); err != nil {
+               t.Fatalf("DSA signature verification failed: %s", err)
+       }
+}
+
 var rsaPSSSelfSignedPEM = `-----BEGIN CERTIFICATE-----
 MIIGHjCCA9KgAwIBAgIBdjBBBgkqhkiG9w0BAQowNKAPMA0GCWCGSAFlAwQCAQUA
 oRwwGgYJKoZIhvcNAQEIMA0GCWCGSAFlAwQCAQUAogMCASAwbjELMAkGA1UEBhMC