Skip to content

Commit

Permalink
wolcrypt: NIST_SP_800_56C address reviewer's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@rizlik
rizlik committed May 30, 2024
1 parent 8d41e68 commit 1744564
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 53 deletions.
6 changes: 3 additions & 3 deletions doc/dox_comments/header_files/kdf.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,14 @@ int wc_SRTP_KDF_kdr_to_idx(word32 kdr);
unsigned char output[32];
int ret;
ret = wc_SP80056C_KDF_single(z, sizeof(z), fixedInfo, sizeof(fixedInfo),
ret = wc_KDA_KDF_onestep(z, sizeof(z), fixedInfo, sizeof(fixedInfo),
sizeof(output), WC_HASH_TYPE_SHA256, output, sizeof(output));
if (ret != 0) {
WOLFSSL_MSG("wc_SP80056C_KDF_single failed");
WOLFSSL_MSG("wc_KDA_KDF_onestep failed");
}
\endcode
*/
int wc_SP80056C_KDF_single(const byte* z, word32 zSz,
int wc_KDA_KDF_onestep(const byte* z, word32 zSz,
const byte* fixedInfo, word32 fixedInfoSz, word32 derivedSecretSz,
enum wc_HashType hashType, byte* output, word32 outputSz);

71 changes: 26 additions & 45 deletions wolfcrypt/src/kdf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1390,9 +1390,9 @@ int wc_SRTP_KDF_kdr_to_idx(word32 kdr)
#endif /* WC_SRTP_KDF */

#ifdef WC_KDF_NIST_SP_800_56C
static int wc_SP80056C_KDF_iteration(const byte* z, word32 zSz,
word32 counter, const byte* fixedInfo, word32 fixedInfoSz,
enum wc_HashType hashType, byte* output)
static int wc_KDA_KDF_iteration(const byte* z, word32 zSz, word32 counter,
const byte* fixedInfo, word32 fixedInfoSz, enum wc_HashType hashType,
byte* output)
{
byte counterBuf[4];
wc_HashAlg hash;
Expand Down Expand Up @@ -1433,17 +1433,14 @@ static int wc_SP80056C_KDF_iteration(const byte* z, word32 zSz,
* \return BAD_FUNC_ARG if the input parameters are invalid.
* \return negative error code if the KDF operation fails.
*/
int wc_SP80056C_KDF_single(const byte* z, word32 zSz,
const byte* fixedInfo, word32 fixedInfoSz, word32 derivedSecretSz,
enum wc_HashType hashType, byte* output, word32 outputSz)
int wc_KDA_KDF_onestep(const byte* z, word32 zSz, const byte* fixedInfo,
word32 fixedInfoSz, word32 derivedSecretSz, enum wc_HashType hashType,
byte* output, word32 outputSz)
{
byte hashTempBuf[WC_MAX_DIGEST_SIZE];
int ret = BAD_FUNC_ARG;
word32 counter, outIdx;
word32 inputSz;
byte* hashOut;
int hashOutSz;
word32 reps;
int ret;

if (output == NULL || outputSz < derivedSecretSz)
return BAD_FUNC_ARG;
Expand All @@ -1456,50 +1453,34 @@ int wc_SP80056C_KDF_single(const byte* z, word32 zSz,
if (hashOutSz == HASH_TYPE_E)
return BAD_FUNC_ARG;

/* According to SP800_56C reps shall not be greater than 2**32-1. This is
* not possible using word32 integers. The code checks for overflow. */
reps = derivedSecretSz / hashOutSz;
if (derivedSecretSz % hashOutSz) {
if (reps + 1 < reps)
return BAD_FUNC_ARG;
reps++;
}

/* According to SP800_56C, table 1, the max input size (max_H_inputBits)
* depends on the HASH algo. The smaller value in the table is (2**64-1)/8.
* This is larger than the possible length using word32 integers. The code
* checks for overflow. */
inputSz = zSz;
if (inputSz + 4 < inputSz)
return BAD_FUNC_ARG;
inputSz += 4;
if (inputSz + fixedInfoSz < inputSz)
return BAD_FUNC_ARG;
* This is larger than the possible length using word32 integers. */

counter = 1;
outIdx = 0;
for (counter = 1; counter <= reps; counter++) {
/* If the user provided a buffer output size bigger than the
* derivedSecretSz then the copy in hashTempBuf can be avoided.
* Nevertheless, the code conservatively does the copy anyway as the
* data is sensitive and the user may forget zeroing outputsz bytes
* instead of derivedSecretsz bytes. */
if (outIdx + hashOutSz <= derivedSecretSz) {
hashOut = output + outIdx;
}
else {
hashOut = hashTempBuf;
}
ret = wc_SP80056C_KDF_iteration(z, zSz, counter,
fixedInfo, fixedInfoSz, hashType, hashOut);
if (hashOut == hashTempBuf) {
XMEMCPY(output + outIdx, hashTempBuf, derivedSecretSz - outIdx);
ForceZero(hashTempBuf, sizeof(hashTempBuf));
}
ret = 0;

/* According to SP800_56C the number of iterations shall not be greater than
* 2**32-1. This is not possible using word32 integers.*/
while (outIdx + hashOutSz <= derivedSecretSz) {
ret = wc_KDA_KDF_iteration(z, zSz, counter, fixedInfo, fixedInfoSz,
hashType, output + outIdx);
if (ret != 0)
break;
counter++;
outIdx += hashOutSz;
}

if (ret == 0 && outIdx < derivedSecretSz) {
ret = wc_KDA_KDF_iteration(z, zSz, counter, fixedInfo, fixedInfoSz,
hashType, hashTempBuf);
if (ret == 0) {
XMEMCPY(output + outIdx, hashTempBuf, derivedSecretSz - outIdx);
}
ForceZero(hashTempBuf, hashOutSz);
}

if (ret != 0) {
ForceZero(output, derivedSecretSz);
}
Expand Down
23 changes: 19 additions & 4 deletions wolfcrypt/test/test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1226,11 +1226,26 @@ static WOLFSSL_TEST_SUBROUTINE wc_test_ret_t nist_sp80056c_kdf_test(void)
"\x1b\x17\xe1\x67\xfc\x43\x7f\x84\x86\x9d\x85\x49\x53\x7b\x33\x38",
WC_HASH_TYPE_SHA512),
#endif
INIT_SP80056C_TEST_VECTOR(
"\x00\xcd\xea\x89\x62\x1c\xfa\x46\xb1\x32\xf9\xe4\xcf\xe2\x26\x1c"
"\xde\x2d\x43\x68\xeb\x56\x56\x63\x4c\x7c\xc9\x8c\x7a\x00\xcd\xe5"
"\x4e\xd1\x86\x6a\x0d\xd3\xe6\x12\x6c\x9d\x2f\x84\x5d\xaf\xf8\x2c"
"\xeb\x1d\xa0\x8f\x5d\x87\x52\x1b\xb0\xeb\xec\xa7\x79\x11\x16\x9c"
"\x20\xcc\x01\x38\xa6\x72\xb6\x95\x8b\xd7\x84\xe5\xd7\xfa\x83\x73"
"\x8a\xc6\x8f\x9b\x34\x23\xb4\x83\xf9\xbf\x53\x9e\x71\x14\x1e\x45"
"\xdb\xfb\x7a\xfe\xd1\x8b\x11\xc0\x02\x8b\x13\xf1\xf8\x60\xef\x43"
"\xc4\x80\xf4\xda\xcd\xa2\x08\x10\x59\xd3\x97\x8c\x99\x9d\x5d\x1a"
"\xde\x34\x54\xe4",
"\x12\x34\x56\x78\x9a\xbc\xde\xf0\x41\x4c\x49\x43\x45\x31\x32\x33"
"\x42\x4f\x42\x42\x59\x34\x35\x36",
"\x2d\x4a",
WC_HASH_TYPE_SHA512),

};

for (i = 0; i < sizeof(vctors) / sizeof(vctors[0]); i++) {
v = &vctors[i];
ret = wc_SP80056C_KDF_single(v->z, v->zSz, v->fixedInfo, v->fixedInfoSz,
ret = wc_KDA_KDF_onestep(v->z, v->zSz, v->fixedInfo, v->fixedInfoSz,
v->derivedKeySz, v->hashType, output,
/* use derivedKeySz to force the function to use a temporary buff
for the last block */
Expand All @@ -1242,17 +1257,17 @@ static WOLFSSL_TEST_SUBROUTINE wc_test_ret_t nist_sp80056c_kdf_test(void)
}

/* negative tests */
ret = wc_SP80056C_KDF_single(NULL, 0, (byte*)"fixed_info",
ret = wc_KDA_KDF_onestep(NULL, 0, (byte*)"fixed_info",
sizeof("fixed_info"), 16, WC_HASH_TYPE_SHA256, output, 16);
if (ret != BAD_FUNC_ARG)
return WC_TEST_RET_ENC_NC;
ret = wc_SP80056C_KDF_single((byte*)"secret", sizeof("secret"), NULL, 1, 16,
ret = wc_KDA_KDF_onestep((byte*)"secret", sizeof("secret"), NULL, 1, 16,
WC_HASH_TYPE_SHA256, output, 16);
if (ret != BAD_FUNC_ARG)
return WC_TEST_RET_ENC_NC;

/* allow empty FixedInfo */
ret = wc_SP80056C_KDF_single((byte*)"secret", sizeof("secret"), NULL, 0, 16,
ret = wc_KDA_KDF_onestep((byte*)"secret", sizeof("secret"), NULL, 0, 16,
WC_HASH_TYPE_SHA256, output, 16);
if (ret != 0)
return WC_TEST_RET_ENC_EC(ret);
Expand Down
2 changes: 1 addition & 1 deletion wolfssl/wolfcrypt/kdf.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ WOLFSSL_API int wc_SRTP_KDF_kdr_to_idx(word32 kdr);
#endif /* WC_SRTP_KDF */

#ifdef WC_KDF_NIST_SP_800_56C
WOLFSSL_API int wc_SP80056C_KDF_single(const byte* z, word32 zSz,
WOLFSSL_API int wc_KDA_KDF_onestep(const byte* z, word32 zSz,
const byte* fixedInfo, word32 fixedInfoSz, word32 derivedSecretSz,
enum wc_HashType hashType, byte* output, word32 outputSz);
#endif
Expand Down

0 comments on commit 1744564

Please sign in to comment.