Skip to content

Conversation

@gilles-peskine-arm
Copy link
Contributor

Add support for the SHA-2 family to the PKCS#5 module. This is a very small code change and fixes an interoperability issue, hence the proposal to backport it even though this is technically a new feature.

Straightforward backport of #1219 (I only tweaked the ChangeLog entry)

ordex and others added 6 commits February 14, 2018 11:12
Currently only SHA1 is supported as PRF algorithm for PBKDF2
(PKCS#5 v2.0).
This means that keys encrypted and authenticated using
another algorithm of the SHA family cannot be decrypted.

This deficiency has become particularly incumbent now that
PKIs created with OpenSSL1.1 are encrypting keys using
hmacSHA256 by default (OpenSSL1.0 used PKCS#5 v1.0 by default
and even if v2 was forced, it would still use hmacSHA1).

Enable support for all the digest algorithms of the SHA
family for PKCS#5 v2.0.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Test vectors for SHA224,256,384 and 512 have been
generated using Python's hashlib module by the
following oneliner:

import binascii, hashlib
binascii.hexlify(hashlib.pbkdf2_hmac(ALGO, binascii.unhexlify('PASSWORD'), binascii.unhexlify('SALT'), ITER, KEYLEN)))

where ALGO was 'sha224', 'sha256', 'sha384' and 'sha512'
respectively.

Values for PASSWORD, SALT, ITER and KEYLEN were copied from the
existent test vectors for SHA1.

For SHA256 we also have two test vectors coming from RFC7914 Sec 11.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Some unit tests for pbkdf2_hmac() have results longer than
99bytes when represented in hexadecimal form.

For this reason extend the result array to accommodate
longer strings.

At the same time make memset() parametric to avoid
bugs in the future.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
We now have support for the entire SHA family to be used as
PRF in PKCS#5 v2.0, therefore we need to add new keys to test
these new functionalities.

This patch adds the new keys in `tests/data_files` and
commands to generate them in `tests/data_files/Makefile`.

Note that the pkcs8 command in OpenSSL 1.0 called with
the -v2 argument generates keys using PKCS#5 v2.0 with SHA1
as PRF by default.

(This behaviour has changed in OpenSSL 1.1, where the exact same
command instead uses PKCS#5 v2.0 with SHA256)

The new keys are generated by specifying different PRFs with
-v2prf.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Extend the pkparse test suite with the newly created keys
encrypted using PKCS#8 with PKCS#5 v2.0 with PRF being
SHA224, 256, 384 and 512.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm satisfied this is a faithful backport of the original PR and I agree that the code changes are small enough and well-tested enough for inclusion in the 2.1 branch.

@gilles-peskine-arm
Copy link
Contributor Author

all.sh passed locally.

@hanno-becker
Copy link

@gilles-peskine-arm Do we need a backport to 2.7?

@gilles-peskine-arm
Copy link
Contributor Author

@hanno-arm Thanks for raising this. We haven't decided yet whether we were going to backport this change. #1219 has been merged to development and can be merged to the 2.7 branch as well, so if we decide that we do want a backport, we can use #1219.

@gilles-peskine-arm gilles-peskine-arm removed the needs-review Every commit must be reviewed by at least two team members, label Mar 8, 2018
@simonbutcher simonbutcher added approved for design approved Design and code approved - may be waiting for CI or backports and removed needs-design-approval labels Mar 11, 2018
@simonbutcher
Copy link
Contributor

simonbutcher commented Mar 11, 2018

Agreed and approved that this fix needs back porting to 2.1 branch.

@gilles-peskine-arm gilles-peskine-arm merged commit c0577f3 into Mbed-TLS:mbedtls-2.1 Mar 13, 2018
minosgalanakis added a commit that referenced this pull request Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Design and code approved - may be waiting for CI or backports enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants