Fix PBKDF2 with empty salt segment on platforms where malloc(0)=NULL
"Fix PBKDF2 with empty salt on platforms where malloc(0)=NULL" took care of
making an empty salt work. But it didn't fix the case of an empty salt
segment followed by a non-empty salt segment, which still invoked memcpy
with a potentially null pointer as the source. This commit fixes that case,
and also simplifies the logic in the function a little.
Test data obtained with:
```
pip3 install cryptodome
python3 -c 'import sys; from Crypto.Hash import SHA256; from Crypto.Protocol.KDF import PBKDF2; cost = int(sys.argv[1], 0); salt = bytes.fromhex(sys.argv[2]); password = bytes.fromhex(sys.argv[3]); n = int(sys.argv[4], 0); print(PBKDF2(password=password, salt=salt, dkLen=n, count=cost, hmac_hash_module=SHA256).hex())' 1 "" "706173737764" 64
```
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index c4a48f8..35d71e7 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -6684,22 +6684,17 @@
const uint8_t *data,
size_t data_length)
{
- if (pbkdf2->state != PSA_PBKDF2_STATE_INPUT_COST_SET &&
- pbkdf2->state != PSA_PBKDF2_STATE_SALT_SET) {
+ if (pbkdf2->state == PSA_PBKDF2_STATE_INPUT_COST_SET) {
+ pbkdf2->state = PSA_PBKDF2_STATE_SALT_SET;
+ } else if (pbkdf2->state == PSA_PBKDF2_STATE_SALT_SET) {
+ /* Appending to existing salt. No state change. */
+ } else {
return PSA_ERROR_BAD_STATE;
}
if (data_length == 0) {
- /* Nothing to do */
- } else if (pbkdf2->state == PSA_PBKDF2_STATE_INPUT_COST_SET) {
- pbkdf2->salt = mbedtls_calloc(1, data_length);
- if (pbkdf2->salt == NULL) {
- return PSA_ERROR_INSUFFICIENT_MEMORY;
- }
-
- memcpy(pbkdf2->salt, data, data_length);
- pbkdf2->salt_length = data_length;
- } else if (pbkdf2->state == PSA_PBKDF2_STATE_SALT_SET) {
+ /* Appending an empty string, nothing to do. */
+ } else {
uint8_t *next_salt;
next_salt = mbedtls_calloc(1, data_length + pbkdf2->salt_length);
@@ -6707,15 +6702,14 @@
return PSA_ERROR_INSUFFICIENT_MEMORY;
}
- memcpy(next_salt, pbkdf2->salt, pbkdf2->salt_length);
+ if (pbkdf2->salt_length != 0) {
+ memcpy(next_salt, pbkdf2->salt, pbkdf2->salt_length);
+ }
memcpy(next_salt + pbkdf2->salt_length, data, data_length);
pbkdf2->salt_length += data_length;
mbedtls_free(pbkdf2->salt);
pbkdf2->salt = next_salt;
}
-
- pbkdf2->state = PSA_PBKDF2_STATE_SALT_SET;
-
return PSA_SUCCESS;
}
diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data
index 410ae64..2237a41 100644
--- a/tests/suites/test_suite_psa_crypto.data
+++ b/tests/suites/test_suite_psa_crypto.data
@@ -6377,10 +6377,22 @@
depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_1
derive_output:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_1):PSA_KEY_DERIVATION_INPUT_COST:"1000":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"7361006c74":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_PASSWORD:"7061737300776f7264":PSA_SUCCESS:0:"":PSA_SUCCESS:"":16:"56fa6aa75548099dcc37d7f03425e0c3":"":0:1:0
-PSA key derivation: PBKDF2-HMAC(SHA-256), RFC7914 #1, salt in two step
+PSA key derivation: PBKDF2-HMAC(SHA-256), RFC7914 #1, salt=2+2
depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_256
derive_output:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_COST:"01":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"7361":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"6c74":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_PASSWORD:"706173737764":PSA_SUCCESS:"":64:"55ac046e56e3089fec1691c22544b605f94185216dde0465e68b9d57c20dacbc49ca9cccf179b645991664b39d77ef317c71b845b1e30bd509112041d3a19783":"":0:1:0
+PSA key derivation: PBKDF2-HMAC(SHA-256), RFC7914 #1, salt=0+4
+depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_256
+derive_output:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_COST:"01":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"73616c74":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_PASSWORD:"706173737764":PSA_SUCCESS:"":64:"55ac046e56e3089fec1691c22544b605f94185216dde0465e68b9d57c20dacbc49ca9cccf179b645991664b39d77ef317c71b845b1e30bd509112041d3a19783":"":0:1:0
+
+PSA key derivation: PBKDF2-HMAC(SHA-256), RFC7914 #1, salt=4+0
+depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_256
+derive_output:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_COST:"01":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"73616c74":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_PASSWORD:"706173737764":PSA_SUCCESS:"":64:"55ac046e56e3089fec1691c22544b605f94185216dde0465e68b9d57c20dacbc49ca9cccf179b645991664b39d77ef317c71b845b1e30bd509112041d3a19783":"":0:1:0
+
+PSA key derivation: PBKDF2-HMAC(SHA-256), salt=0+0
+depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_256
+derive_output:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_COST:"01":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_PASSWORD:"706173737764":PSA_SUCCESS:"":64:"b03ada2451aa1084ce14cf51c93eeea9d2bd435db3f93a70031b2de39fdef45d2ccb1fe2078e79773c148311d3e6ec5dec9da7f30d78584ec21c94de839671b2":"":0:1:0
+
PSA key derivation: PBKDF2-HMAC(SHA-256), RFC7914 #1, password as key, derive key
depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_256
derive_output:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_COST:"01":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SALT:"73616c74":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_PASSWORD:"706173737764":PSA_SUCCESS:0:"":PSA_SUCCESS:"":64:"55ac046e56e3089fec1691c22544b605f94185216dde0465e68b9d57c20dacbc49ca9cccf179b645991664b39d77ef317c71b845b1e30bd509112041d3a19783":"":0:1:1