-
Notifications
You must be signed in to change notification settings - Fork 18k
crypto/x509: Truncate DSA signed data's hash before verification #22017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
(Man, I should never have supported DSA in that code.) I'm not sure that it's valid to have the hash mismatch with the DSA key type, as you have here. DSA was originally specified only with SHA-1 and with 1024-bit keys. That's what you have in the certificate, but your signature algorithm is DSAWithSHA256. For that, you should have a 2048/256 key. |
Unfortunately, it's not really my key - I just need to verify the signature is correct. The signature check in Android passes without a problem, so it's "valid" in that way at least. I'm fine with having that extra checkWithTruncatedHash function in my code, I just saw that dsa docs say the hash should be truncated, but it isn't in x509 and thought it might be a bug. |
CC @FiloSottile |
Alright, I would like to let DSA rot in a corner, and using SHA-256 with 1024 bit keys is weird, but as far as I can tell we are indeed doing this wrong. I would take a PR to fix this with a test case that passes OpenSSL verification. |
Change https://golang.org/cl/198138 mentions this issue: |
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 golang#22017
What version of Go are you using (
go version
)?go version go1.9 linux/amd64
Does this issue reproduce with the latest release?
Yes
The x509.Certificate's CheckSignature function does not truncate the signed data's hash (as dsa's docs say should be done), resulting in wrong check result. When I truncated the hash, the check passes, as can be seen below.
I'm not sure whether the hash should be truncated for all x509 certs. This is tested with Android APK signatures, which do require the truncation (the data in code below were extracted from this apk and there are more like it).
I can submit a pull request if you think it is correct to do the truncation.
Raw full source of the code (~4k lines)
The text was updated successfully, but these errors were encountered: