pkcs5_keyivgen: continue to work with increased default salt length

[xnox]

In the PKCS5 RFC salt length is specified of a default value of 8. But
it can be longer or shorter.

In OpenSSL PKCS5_SALT_LEN constant is specified, but it is a internal
implementation detail, a fallback value, and a default suggestion. One
can pick lower and higher salt values.

Recently, many OpenSSL-like implementations are increasing this
fallback default salt length from 8 bytes to 16 bytes. With such
implementations ruby-openssl starts to fail (in error). Afterall the
8-byte salt is passed in, and is still acceptable to be used. Update
the guard to keep 8-bytes as the lower value, without relying on the
PKCS5_SALT_LEN which is going to be different based on a given system
library implementation.

References:

• openssl/openssl@995e948
• https://marc.info/?l=openbsd-tech&m=174689919725477&w=2
• https://marc.info/?l=openbsd-tech&m=174690438827143&w=2
• Increase default salt from 8 to 16 bytes for PKCS#8 & PKCS#12 aws/aws-lc#2409
• https://boringssl-review.googlesource.com/c/boringssl/+/79267

This was cought by aws-lc integration tests.

[rhenium]

PKCS#5 v2 specifies two different schemes: PBES1 (for PKCS#5 v1 compatibility) and PBES2. PBES1 uses a fixed 8 bytes salt. PBES2 doesn't specify the salt length.

https://www.rfc-editor.org/rfc/rfc8018#section-6.1.1

OpenSSL::Cipher#pkcs5_keyivgen uses EVP_BytesToKey(), which is based on (a generalized in a non-standard way, AFAIK) PBES1, so it requires the salt to be exactly 8 bytes.

PKCS5_SALT_LEN is not well documented, like many stuff from the OpenSSL 0.9.x era, but looking at OpenSSL's code, it seems to be intended for this use.

https://github.com/openssl/openssl/blob/b87f4407c72bea7044ef86f6ef7a3eb9bd746606/crypto/evp/evp_key.c#L108-L109
https://github.com/openssl/openssl/blob/b87f4407c72bea7044ef86f6ef7a3eb9bd746606/apps/enc.c#L348-L349

[xnox]

PKCS#5 v2 specifies two different schemes: PBES1 (for PKCS#5 v1 compatibility) and PBES2. PBES1 uses a fixed 8 bytes salt. PBES2 doesn't specify the salt length.

https://www.rfc-editor.org/rfc/rfc8018#section-6.1.1

OpenSSL::Cipher#pkcs5_keyivgen uses EVP_BytesToKey(), which is based on (a generalized in a non-standard way, AFAIK) PBES1, so it requires the salt to be exactly 8 bytes.

PKCS5_SALT_LEN is not well documented, like many stuff from the OpenSSL 0.9.x era, but looking at OpenSSL's code, it seems to be intended for this use.

https://github.com/openssl/openssl/blob/b87f4407c72bea7044ef86f6ef7a3eb9bd746606/crypto/evp/evp_key.c#L108-L109 https://github.com/openssl/openssl/blob/b87f4407c72bea7044ef86f6ef7a3eb9bd746606/apps/enc.c#L348-L349

Correct, and in all implementations being changed the EVP_BytesToKey() remains accepting "salt[8]". Depending on an implementation the defines are declared as private (accidentially public), or unused, or have duplicate defines/meaning, or have separate meaning.

Do you me in this patch instead to keep the check as strictly equals 8, rather than more that 8? (note not compared to PKCS5_SALT_LEN as depending on the implementation it may or may not be the define that configures the EVP_BytesToKey() api).

My primary concern here is to stop relying on an ambiguous PKCS5_SALT_LEN define.

Or maybe I should do something else entirely in OpenSSL? i.e. keep that stray define as 8, and start using an different constant at 16, which possibly would work better -> as in this code continious to work as is (and any other similar case)

[xnox]

Let me ponder this.

[xnox]

I agree that touching and changing legacy APIs is not useful, so will reimplement the proposed change in aws-lc in a different way.