Skip to content
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

Allow 72 bytes (576 bits) keys for Blowfish #646

Merged
merged 2 commits into from
Sep 14, 2024

Conversation

Xartrick
Copy link
Contributor

@Xartrick Xartrick commented May 7, 2024

Quote from Wikipedia :

Because the P-array is 576 bits long, and the key bytes are XORed through all these 576 bits during the initialization, many implementations support key sizes up to 576 bits. The reason for that is a discrepancy between the original Blowfish description, which uses 448-bit keys, and its reference implementation, which uses 576-bit keys. The test vectors for verifying third-party implementations were also produced with 576-bit keys. When asked which Blowfish version is the correct one, Bruce Schneier answered: "The test vectors should be used to determine the one true Blowfish".

Also, bcrypt uses blowfish with a key length of up to 72 bytes.

@levitte
Copy link
Collaborator

levitte commented Aug 20, 2024

The paragraph directly below what you quoted says this:

Another opinion is that the 448 bits limit is present to ensure that every bit of every subkey depends on every bit of the key,[5] as the last four values of the P-array don't affect every bit of the ciphertext. This point should be taken in consideration for implementations with a different number of rounds, as even though it increases security against an exhaustive attack, it weakens the security guaranteed by the algorithm. And given the slow initialization of the cipher with each change of key, it is granted a natural protection against brute-force attacks, which doesn't really justify key sizes longer than 448 bits.

Also, at least with the current workflows, this PR should get some build problems, 'cause notes/*_tv.txt haven't been updated. For example, notes/cipher_tv.txt contains vectors for blowfish with 8, 32 and 56 byte keys. With this change, I expect to see them replaced with vectors for 8, 40 and 72 byte keys.

@levitte
Copy link
Collaborator

levitte commented Sep 9, 2024

Above comment being said, I don't see the harm with this change

@sjaeckel sjaeckel merged commit e0d90c5 into libtom:develop Sep 14, 2024
75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants