Change PBKDF2 hash from SHA512 to SHA256 (according to specification) #176
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have been looking more deeply into the Java version of Crypt4GH created by Dmytro Titov (@dtitov) many years ago,
and especially the KDF functions that stretch the password used to encrypt the private key file.
I compared the Java version to the Python implementation and this Go implementation to see if the KDFs produce the same hash values and verify that private key files created with the different versions are compatible with each other.
The good news is that all three versions produce the same hash for SCrypt, which is the preferred and default KDF. For the BCrypt alternative, the Python and Go implementations are compatible, but the Java version is not. I am looking into that.
For PBKDF2, the Java and Python versions are compatible but the Go version is not. PBKDF2 combines the input with a salt using a hash function and repeats this process multiple times. And from the Crypt4GH documentation, it seems clear that PBKDF2 should use SHA256 as its hash function. It is even part of the name for the KDF:
pbkdf2_hmac_sha256
.But for some reason, the Go implementation is using SHA512, even if it still retains the KDF name pbkdf2_hmac_sha256.
I don't know the reason for this. Perhaps Dmytro decided to use SHA512 because it is more secure, but this should not really matter much anyway, since PBKDF2 should not really be used in the first place, and it is only an alternative in case the first two options, SCrypt and BCrypt, are not available.
My PR changes the hash function back to SHA256 (which is the correct choice according to the Crypt4GH specification) and makes it compatible with the other two implementations of Crypt4GH.
The new hash value in the kdf_test.go file is the one that is returned by the Python implementation using SHA256.