Skip to content

Commit 6178663

Browse files
authored
Merge pull request #179 from aidangarske/fenrir-fixes-2
Add Negative testing and Validation for wolfPKCS11
2 parents 39eb4d6 + 25afee1 commit 6178663

4 files changed

Lines changed: 681 additions & 12 deletions

File tree

src/crypto.c

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,11 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
540540
#endif
541541
break;
542542
case CKO_SECRET_KEY:
543+
#ifndef WOLFPKCS11_NSS
544+
if (ret == CKR_OK)
545+
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
546+
ulCount);
547+
#endif
543548
if (ret == CKR_OK)
544549
ret = SetIfNotFound(obj, CKA_EXTRACTABLE, trueVal, pTemplate,
545550
ulCount);
@@ -557,9 +562,21 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
557562
ulCount);
558563
break;
559564
case CKO_PRIVATE_KEY:
565+
#ifndef WOLFPKCS11_NSS
566+
if (ret == CKR_OK)
567+
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
568+
ulCount);
569+
if (ret == CKR_OK)
570+
ret = SetIfNotFound(obj, CKA_EXTRACTABLE, falseVal, pTemplate,
571+
ulCount);
572+
#else
573+
/* NSS operates as the internal crypto module and needs both
574+
* non-sensitive and extractable private keys to read key material
575+
* directly via C_GetAttributeValue during TLS operations. */
560576
if (ret == CKR_OK)
561577
ret = SetIfNotFound(obj, CKA_EXTRACTABLE, trueVal, pTemplate,
562578
ulCount);
579+
#endif
563580
if (ret == CKR_OK)
564581
ret = SetIfNotFound(obj, CKA_DECRYPT, encrypt, pTemplate,
565582
ulCount);
@@ -2320,10 +2337,13 @@ CK_RV C_Encrypt(CK_SESSION_HANDLE hSession, CK_BYTE_PTR pData,
23202337
}
23212338
if (!CK_ULONG_FITS_WORD32(ulDataLen))
23222339
return CKR_DATA_LEN_RANGE;
2340+
/* Ensure padded result fits in word32 */
2341+
if (ulDataLen > (CK_ULONG)(0xFFFFFFFF - AES_BLOCK_SIZE))
2342+
return CKR_DATA_LEN_RANGE;
23232343

2324-
/* PKCS#5 pad makes the output a multiple of 16 */
2325-
encDataLen = (word32)((ulDataLen + AES_BLOCK_SIZE - 1) /
2326-
AES_BLOCK_SIZE) * AES_BLOCK_SIZE;
2344+
/* PKCS#7 padding always adds at least 1 byte */
2345+
encDataLen = (word32)((ulDataLen / AES_BLOCK_SIZE) + 1) *
2346+
AES_BLOCK_SIZE;
23272347
if (pEncryptedData == NULL) {
23282348
*pulEncryptedDataLen = encDataLen;
23292349
return CKR_OK;

src/internal.c

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,31 @@ static void wp11_Session_Final(WP11_Session* session)
959959
}
960960
#endif
961961
#endif
962-
/* Clear any remaining active operation state not handled above. */
962+
#ifndef NO_HMAC
963+
if ((session->init & ~WP11_INIT_DIGEST_MASK) == WP11_INIT_HMAC_SIGN ||
964+
(session->init & ~WP11_INIT_DIGEST_MASK) == WP11_INIT_HMAC_VERIFY) {
965+
wc_HmacFree(&session->params.hmac.hmac);
966+
session->init &= WP11_INIT_DIGEST_MASK;
967+
}
968+
#endif
969+
#ifdef HAVE_AESCMAC
970+
if ((session->init & ~WP11_INIT_DIGEST_MASK) == WP11_INIT_AES_CMAC_SIGN ||
971+
(session->init & ~WP11_INIT_DIGEST_MASK) == WP11_INIT_AES_CMAC_VERIFY) {
972+
#if (!defined(HAVE_FIPS) || FIPS_VERSION_GE(5, 3))
973+
(void)wc_CmacFree(&session->params.cmac.cmac);
974+
#else
975+
wc_ForceZero(&session->params.cmac.cmac,
976+
sizeof(session->params.cmac.cmac));
977+
#endif
978+
session->init &= WP11_INIT_DIGEST_MASK;
979+
}
980+
#endif
981+
if ((session->init & ~WP11_INIT_DIGEST_MASK) == WP11_INIT_DIGEST) {
982+
wc_HashFree(&session->params.digest.hash,
983+
session->params.digest.hashType);
984+
session->init &= ~WP11_INIT_DIGEST_MASK;
985+
}
986+
/* Ensure no stale bits remain after all cleanup. */
963987
session->init = 0;
964988
}
965989

@@ -7383,6 +7407,9 @@ void WP11_Slot_Logout(WP11_Slot* slot)
73837407
ret = wp11_Object_Encode(object, 1);
73847408
object = object->next;
73857409
}
7410+
/* Zero token key only on user logout — SO logout must preserve it
7411+
* for subsequent object encryption (e.g., empty-PIN flow). */
7412+
wc_ForceZero(slot->token.key, sizeof(slot->token.key));
73867413
}
73877414
#endif
73887415
slot->token.loginState = WP11_APP_STATE_RW_PUBLIC;
@@ -8626,6 +8653,9 @@ static WP11_Object* wp11_Session_FindNext(WP11_Session* session, int onToken,
86268653
}
86278654
#endif
86288655

8656+
/* Note: this CKA_PRIVATE check is intentionally active in NSS mode.
8657+
* NSS accesses private objects by handle (via WP11_Object_Find) rather
8658+
* than discovering them through C_FindObjects enumeration. */
86298659
if ((ret->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE) {
86308660
if (!onToken)
86318661
WP11_Lock_LockRO(&session->slot->token.lock);
@@ -9767,8 +9797,26 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle,
97679797
WP11_Lock_UnlockRO(&session->slot->token.lock);
97689798
}
97699799

9770-
if (obj && (obj->handle == objHandle))
9771-
*object = obj;
9800+
if (ret == 0 && obj != NULL && (obj->handle == objHandle)) {
9801+
#ifndef WOLFPKCS11_NSS
9802+
/* Enforce CKA_PRIVATE: reject private objects from public sessions.
9803+
* Skipped in NSS mode because NSS operates as the internal crypto
9804+
* module without calling C_Login. */
9805+
if ((obj->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE) {
9806+
int loginState;
9807+
WP11_Lock_LockRO(&session->slot->lock);
9808+
loginState = session->slot->token.loginState;
9809+
if (!WP11_Slot_Has_Empty_Pin(session->slot) &&
9810+
(loginState == WP11_APP_STATE_RW_PUBLIC ||
9811+
loginState == WP11_APP_STATE_RO_PUBLIC)) {
9812+
ret = BAD_FUNC_ARG;
9813+
}
9814+
WP11_Lock_UnlockRO(&session->slot->lock);
9815+
}
9816+
#endif
9817+
if (ret == 0)
9818+
*object = obj;
9819+
}
97729820

97739821
return ret;
97749822
}
@@ -12248,7 +12296,7 @@ int WP11_Rsa_Verify_Recover(CK_MECHANISM_TYPE mechanism, unsigned char* sig,
1224812296
}
1224912297
}
1225012298
if (data_out != NULL) {
12251-
*outLen = (out + *outLen) - data_out;
12299+
*outLen = (CK_ULONG)((out + *outLen) - data_out);
1225212300
XMEMMOVE(out, data_out, *outLen);
1225312301
}
1225412302
}
@@ -15224,15 +15272,21 @@ int WP11_Digest_Single(unsigned char* data, word32 dataLen,
1522415272
WP11_Digest* digest = &session->params.digest;
1522515273

1522615274
blockLen = wc_HashGetDigestSize(digest->hashType);
15275+
if (blockLen < 0) {
15276+
wc_HashFree(&digest->hash, digest->hashType);
15277+
session->init = 0;
15278+
return CKR_FUNCTION_FAILED;
15279+
}
1522715280

15228-
if (data == NULL) {
15281+
if (dataOut == NULL) {
1522915282
*dataOutLen = (word32)blockLen;
1523015283
return CKR_OK;
1523115284
}
1523215285
if (*dataOutLen < (word32)blockLen) {
1523315286
return BUFFER_E;
1523415287
}
1523515288
ret = wc_Hash(digest->hashType, data, dataLen, dataOut, *dataOutLen);
15289+
*dataOutLen = (word32)blockLen;
1523615290

1523715291
wc_HashFree(&digest->hash, digest->hashType);
1523815292

tests/pkcs11mtt.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,7 @@ static CK_RV test_attribute(void* args)
498498
{ CKA_CLASS, &privKeyClass, sizeof(privKeyClass) },
499499
{ CKA_KEY_TYPE, &genericKeyType, sizeof(genericKeyType) },
500500
{ CKA_EXTRACTABLE, &ckTrue, sizeof(ckTrue) },
501+
{ CKA_SENSITIVE, &ckFalse, sizeof(ckFalse) },
501502
{ CKA_VALUE, keyData, sizeof(keyData) },
502503
};
503504
CK_ULONG tmplCnt = sizeof(tmpl) / sizeof(*tmpl);
@@ -796,10 +797,12 @@ static CK_RV get_generic_key(CK_SESSION_HANDLE session, unsigned char* data,
796797
CK_OBJECT_HANDLE* key)
797798
{
798799
CK_RV ret;
800+
CK_BBOOL sensitive = (extractable == CK_TRUE) ? CK_FALSE : CK_TRUE;
799801
CK_ATTRIBUTE generic_key[] = {
800802
{ CKA_CLASS, &secretKeyClass, sizeof(secretKeyClass) },
801803
{ CKA_KEY_TYPE, &genericKeyType, sizeof(genericKeyType) },
802804
{ CKA_EXTRACTABLE, &extractable, sizeof(CK_BBOOL) },
805+
{ CKA_SENSITIVE, &sensitive, sizeof(CK_BBOOL) },
803806
{ CKA_SIGN, &ckTrue, sizeof(ckTrue) },
804807
{ CKA_VERIFY, &ckTrue, sizeof(ckTrue) },
805808
{ CKA_VALUE, data, len },
@@ -2060,6 +2063,7 @@ static CK_RV get_rsa_priv_key(CK_SESSION_HANDLE session, unsigned char* privId,
20602063
CK_OBJECT_HANDLE* obj)
20612064
{
20622065
CK_RV ret;
2066+
CK_BBOOL sensitive = (extractable == CK_TRUE) ? CK_FALSE : CK_TRUE;
20632067
CK_ATTRIBUTE rsa_2048_priv_key[] = {
20642068
{ CKA_CLASS, &privKeyClass, sizeof(privKeyClass) },
20652069
{ CKA_KEY_TYPE, &rsaKeyType, sizeof(rsaKeyType) },
@@ -2074,6 +2078,7 @@ static CK_RV get_rsa_priv_key(CK_SESSION_HANDLE session, unsigned char* privId,
20742078
{ CKA_COEFFICIENT, rsa_2048_u, sizeof(rsa_2048_u) },
20752079
{ CKA_PUBLIC_EXPONENT, rsa_2048_pub_exp, sizeof(rsa_2048_pub_exp) },
20762080
{ CKA_EXTRACTABLE, &extractable, sizeof(CK_BBOOL) },
2081+
{ CKA_SENSITIVE, &sensitive, sizeof(CK_BBOOL) },
20772082
{ CKA_TOKEN, &ckTrue, sizeof(ckTrue) },
20782083
{ CKA_ID, privId, privIdLen },
20792084
};
@@ -3422,10 +3427,12 @@ static CK_OBJECT_HANDLE get_ecc_priv_key(CK_SESSION_HANDLE session,
34223427
CK_OBJECT_HANDLE* obj)
34233428
{
34243429
CK_RV ret;
3430+
CK_BBOOL sensitive = (extractable == CK_TRUE) ? CK_FALSE : CK_TRUE;
34253431
CK_ATTRIBUTE ecc_p256_priv_key[] = {
34263432
{ CKA_CLASS, &privKeyClass, sizeof(privKeyClass) },
34273433
{ CKA_KEY_TYPE, &eccKeyType, sizeof(eccKeyType) },
34283434
{ CKA_EXTRACTABLE, &extractable, sizeof(CK_BBOOL) },
3435+
{ CKA_SENSITIVE, &sensitive, sizeof(CK_BBOOL) },
34293436
{ CKA_VERIFY, &ckTrue, sizeof(ckTrue) },
34303437
{ CKA_EC_PARAMS, ecc_p256_params, sizeof(ecc_p256_params) },
34313438
{ CKA_VALUE, ecc_p256_priv, sizeof(ecc_p256_priv) },
@@ -4219,10 +4226,12 @@ static CK_OBJECT_HANDLE get_dh_priv_key(CK_SESSION_HANDLE session,
42194226
CK_OBJECT_HANDLE* obj)
42204227
{
42214228
CK_RV ret;
4229+
CK_BBOOL sensitive = (extractable == CK_TRUE) ? CK_FALSE : CK_TRUE;
42224230
CK_ATTRIBUTE dh_2048_priv_key[] = {
42234231
{ CKA_CLASS, &privKeyClass, sizeof(privKeyClass) },
42244232
{ CKA_KEY_TYPE, &dhKeyType, sizeof(dhKeyType) },
42254233
{ CKA_EXTRACTABLE, &extractable, sizeof(CK_BBOOL) },
4234+
{ CKA_SENSITIVE, &sensitive, sizeof(CK_BBOOL) },
42264235
{ CKA_DERIVE, &ckTrue, sizeof(ckTrue) },
42274236
{ CKA_PRIME, dh_ffdhe2048_p, sizeof(dh_ffdhe2048_p) },
42284237
{ CKA_BASE, dh_ffdhe2048_g, sizeof(dh_ffdhe2048_g) },

0 commit comments

Comments
 (0)