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

Blowfish maximum key size #407

Open
jpboivin opened this issue Dec 23, 2022 · 6 comments
Open

Blowfish maximum key size #407

jpboivin opened this issue Dec 23, 2022 · 6 comments

Comments

@jpboivin
Copy link

BouncyCastle 1.9 introduced validations of the Blowfish key size (https://github.com/bcgit/bc-csharp/blob/release/v2.0/crypto/src/crypto/engines/BlowfishEngine.cs#L444). More precisely, it must be between 32 and 448 bits, which is indeed the official upper limit. With that said, this is incompatible with OpenSSL's limit which is ((BF_ROUNDS + 2) * 4) or 576 bits (https://github.com/openssl/openssl/blob/openssl-3.0/crypto/bf/bf_skey.c#L31). Due to this difference, BouncyCastle is not interoperable with product/protocols based on OpenSSL, and using non-standard key sizes.

The change would be trivial to make, so the question is whether or not BC should support this non-standard key size.

@cipherboy
Copy link
Collaborator

cipherboy commented Feb 6, 2024

@jpboivin Interesting, Wikipedia currently states:

When asked which Blowfish version is the correct one, Bruce Schneier answered: "The test vectors should be used to determine the one true Blowfish".

However, does not cite a source for this assertion. However, the OpenSSL code has persisted since the initial import, so there hasn't really been a discussion on it as far as I can tell there. NSS lacks a Blowfish implementation, but JDK maintains the original spec's limitation.

So, using these keys will cause broader interoperability problems with other existing implementations.

Do you have a protocol which requires these larger key sizes? Perhaps it is better to move to stronger, modern ciphers like XChaCha20 or AES, if no protocol requires it.

@jpboivin
Copy link
Author

jpboivin commented Feb 6, 2024

NSS lacks a Blowfish implementation, but JDK maintains the original spec's limitation.

Interesting to see that OpenSSL seems to be the black sheep, I'm not sure why they picked the key size limit based on rounds.

Do you have a protocol which requires these larger key sizes? Perhaps it is better to move to stronger, modern ciphers like XChaCha20 or AES, if no protocol requires it.

Yes, but it isn't a common / public protocol (it's the protocol used by an old game for which I wrote a server emulator to keep me up to date with my C#). A change to more modern ciphers is as such out of scope in this context, but I could always re-implement a non-standard Blowfish as I had done before switching to BC.

I would totally understand if BC prefers to stick with the more standard key limit :)

@cipherboy
Copy link
Collaborator

@jpboivin Hmm, if you're interested in proposing a patch, I'll run it by the others and see if there's objections.

It looks like Nettle (underpinning Gnutls) supports completely arbitrary key lengths as well...

@jpboivin
Copy link
Author

jpboivin commented Feb 9, 2024

@cipherboy As discussed, I've created a patch (#519) and added test vectors based on OpenSSL and Nettle.

@pdugre
Copy link

pdugre commented Jun 27, 2024

An example of use with keys over 448 bits is implementing encryption/decryption of SSH keys using a passphrase. It uses what they call brypt_pbkdf which is a modified version of PBKDF2 using a modified version of bcrypt as the primitive. This uses 512 bit keys internally for Blowfish(since they use a SHA512 hash of the password as the key).

This issue is currently blocking for us because of this particular case. Let me know if I can help!

@Recurse-blip
Copy link

Recurse-blip commented Jul 2, 2024

An example of use with keys over 448 bits is implementing encryption/decryption of SSH keys using a passphrase. It uses what they call brypt_pbkdf which is a modified version of PBKDF2 using a modified version of bcrypt as the primitive. This uses 512 bit keys internally for Blowfish(since they use a SHA512 hash of the password as the key).

This issue is currently blocking for us because of this particular case. Let me know if I can help!

I think we should enforce 72 byte size (576 bits) just like the bcrypt Rust crate or make the maximum size configurable, I am also facing an issue using the bouncy castle implementation in a specific use case. I'll be forced to make a fork until this change is merged. #519

See this :
https://docs.rs/bcrypt/latest/src/bcrypt/lib.rs.html#118

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

No branches or pull requests

4 participants