1
0
mirror of https://github.com/golang/go synced 2024-11-17 18:14:46 -07:00

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: 8041cde8d2
GitHub-Pull-Request: golang/go#34630
Reviewed-on: https://go-review.googlesource.com/c/go/+/198138
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This commit is contained in:
Vojtech Bocek 2019-10-04 05:24:55 +00:00 committed by Filippo Valsorda
parent 6145a80608
commit 72dc3a0919
2 changed files with 48 additions and 0 deletions

View File

@ -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")
}

View File

@ -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