Skip to content

Commit a065784

Browse files
aidangarskeaidangarske
committed
Add Negative testing and Validation for wolfPKCS11
F-2379: Add test for CKA_ENCRYPT/CKA_DECRYPT enforcement F-2380: Add test for CKA_SIGN/CKA_VERIFY enforcement F-2386: Add test for CKA_VERIFY enforcement in C_VerifyRecoverInit F-2375: Fix AES-CBC-PAD output length for block-aligned inputs F-2389: Zero token master encryption key on C_Logout F-2368: Default CKA_SENSITIVE=TRUE and CKA_EXTRACTABLE=FALSE for private keys F-2369: Enforce CKA_PRIVATE access control in WP11_Object_Find F-2381: Add test for HMAC truncated signature rejection F-2382: Add test for CKA_WRAP/CKA_UNWRAP enforcement F-2376: Fix C_Digest single-shot NULL size query F-2390: Clean HMAC/CMAC/Digest crypto state on session close
1 parent d39ecdd commit a065784

File tree

4 files changed

+686
-13
lines changed

4 files changed

+686
-13
lines changed

‎src/crypto.c‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,9 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
514514
ulCount);
515515
break;
516516
case CKO_SECRET_KEY:
517+
if (ret == CKR_OK)
518+
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
519+
ulCount);
517520
if (ret == CKR_OK)
518521
ret = SetIfNotFound(obj, CKA_EXTRACTABLE, trueVal, pTemplate,
519522
ulCount);
@@ -531,9 +534,23 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
531534
ulCount);
532535
break;
533536
case CKO_PRIVATE_KEY:
537+
#ifndef WOLFPKCS11_NSS
538+
if (ret == CKR_OK)
539+
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
540+
ulCount);
541+
if (ret == CKR_OK)
542+
ret = SetIfNotFound(obj, CKA_EXTRACTABLE, falseVal, pTemplate,
543+
ulCount);
544+
#else
545+
/* NSS needs extractable private keys as internal crypto module.
546+
* CKA_SENSITIVE is still set to prevent plaintext key readout. */
547+
if (ret == CKR_OK)
548+
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
549+
ulCount);
534550
if (ret == CKR_OK)
535551
ret = SetIfNotFound(obj, CKA_EXTRACTABLE, trueVal, pTemplate,
536552
ulCount);
553+
#endif
537554
if (ret == CKR_OK)
538555
ret = SetIfNotFound(obj, CKA_DECRYPT, encrypt, pTemplate,
539556
ulCount);
@@ -2277,10 +2294,13 @@ CK_RV C_Encrypt(CK_SESSION_HANDLE hSession, CK_BYTE_PTR pData,
22772294
}
22782295
if (!CK_ULONG_FITS_WORD32(ulDataLen))
22792296
return CKR_DATA_LEN_RANGE;
2297+
/* Ensure padded result fits in word32 */
2298+
if (ulDataLen > (CK_ULONG)(0xFFFFFFFF - AES_BLOCK_SIZE))
2299+
return CKR_DATA_LEN_RANGE;
22802300

2281-
/* PKCS#5 pad makes the output a multiple of 16 */
2282-
encDataLen = (word32)((ulDataLen + AES_BLOCK_SIZE - 1) /
2283-
AES_BLOCK_SIZE) * AES_BLOCK_SIZE;
2301+
/* PKCS#7 padding always adds at least 1 byte */
2302+
encDataLen = (word32)((ulDataLen / AES_BLOCK_SIZE) + 1) *
2303+
AES_BLOCK_SIZE;
22842304
if (pEncryptedData == NULL) {
22852305
*pulEncryptedDataLen = encDataLen;
22862306
return CKR_OK;

‎src/internal.c‎

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

@@ -6949,8 +6973,8 @@ int WP11_Slot_IsLoggedIn(WP11_Slot* slot)
69496973

69506974
void WP11_Slot_Logout(WP11_Slot* slot)
69516975
{
6952-
#ifndef WOLFPKCS11_NO_STORE
69536976
int state;
6977+
#ifndef WOLFPKCS11_NO_STORE
69546978
int ret = 0;
69556979
#endif
69566980

@@ -6965,7 +6989,14 @@ void WP11_Slot_Logout(WP11_Slot* slot)
69656989
object = object->next;
69666990
}
69676991
}
6992+
#else
6993+
state = slot->token.loginState;
69686994
#endif
6995+
/* Zero token key only on user logout — SO logout must preserve it
6996+
* for subsequent object encryption (e.g., empty-PIN flow). */
6997+
if (state == WP11_APP_STATE_RO_USER || state == WP11_APP_STATE_RW_USER) {
6998+
wc_ForceZero(slot->token.key, sizeof(slot->token.key));
6999+
}
69697000
slot->token.loginState = WP11_APP_STATE_RW_PUBLIC;
69707001

69717002
WP11_Lock_UnlockRW(&slot->lock);
@@ -8223,6 +8254,9 @@ static WP11_Object* wp11_Session_FindNext(WP11_Session* session, int onToken,
82238254
}
82248255
#endif
82258256

8257+
/* Note: this CKA_PRIVATE check is intentionally active in NSS mode.
8258+
* NSS accesses private objects by handle (via WP11_Object_Find) rather
8259+
* than discovering them through C_FindObjects enumeration. */
82268260
if ((ret->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE) {
82278261
if (!onToken)
82288262
WP11_Lock_LockRO(&session->slot->token.lock);
@@ -9242,8 +9276,26 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle,
92429276
WP11_Lock_UnlockRO(&session->slot->token.lock);
92439277
}
92449278

9245-
if (obj && (obj->handle == objHandle))
9246-
*object = obj;
9279+
if (ret == 0 && obj != NULL && (obj->handle == objHandle)) {
9280+
#ifndef WOLFPKCS11_NSS
9281+
/* Enforce CKA_PRIVATE: reject private objects from public sessions.
9282+
* Skipped in NSS mode because NSS operates as the internal crypto
9283+
* module without calling C_Login. */
9284+
if ((obj->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE) {
9285+
int loginState;
9286+
WP11_Lock_LockRO(&session->slot->lock);
9287+
loginState = session->slot->token.loginState;
9288+
if (!WP11_Slot_Has_Empty_Pin(session->slot) &&
9289+
(loginState == WP11_APP_STATE_RW_PUBLIC ||
9290+
loginState == WP11_APP_STATE_RO_PUBLIC)) {
9291+
ret = BAD_FUNC_ARG;
9292+
}
9293+
WP11_Lock_UnlockRO(&session->slot->lock);
9294+
}
9295+
#endif
9296+
if (ret == 0)
9297+
*object = obj;
9298+
}
92479299

92489300
return ret;
92499301
}
@@ -11516,7 +11568,7 @@ int WP11_Rsa_Verify_Recover(CK_MECHANISM_TYPE mechanism, unsigned char* sig,
1151611568
}
1151711569
}
1151811570
if (data_out != NULL) {
11519-
*outLen = (out + *outLen) - data_out;
11571+
*outLen = (CK_ULONG)((out + *outLen) - data_out);
1152011572
XMEMMOVE(out, data_out, *outLen);
1152111573
}
1152211574
}
@@ -14304,15 +14356,21 @@ int WP11_Digest_Single(unsigned char* data, word32 dataLen,
1430414356
WP11_Digest* digest = &session->params.digest;
1430514357

1430614358
blockLen = wc_HashGetDigestSize(digest->hashType);
14359+
if (blockLen < 0) {
14360+
wc_HashFree(&digest->hash, digest->hashType);
14361+
session->init = 0;
14362+
return CKR_FUNCTION_FAILED;
14363+
}
1430714364

14308-
if (data == NULL) {
14365+
if (dataOut == NULL) {
1430914366
*dataOutLen = (word32)blockLen;
1431014367
return CKR_OK;
1431114368
}
1431214369
if (*dataOutLen < (word32)blockLen) {
1431314370
return BUFFER_E;
1431414371
}
1431514372
ret = wc_Hash(digest->hashType, data, dataLen, dataOut, *dataOutLen);
14373+
*dataOutLen = (word32)blockLen;
1431614374

1431714375
wc_HashFree(&digest->hash, digest->hashType);
1431814376

‎tests/pkcs11mtt.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ static CK_RV test_attribute(void* args)
491491
{ CKA_CLASS, &privKeyClass, sizeof(privKeyClass) },
492492
{ CKA_KEY_TYPE, &genericKeyType, sizeof(genericKeyType) },
493493
{ CKA_EXTRACTABLE, &ckTrue, sizeof(ckTrue) },
494+
{ CKA_SENSITIVE, &ckFalse, sizeof(ckFalse) },
494495
{ CKA_VALUE, keyData, sizeof(keyData) },
495496
};
496497
CK_ULONG tmplCnt = sizeof(tmpl) / sizeof(*tmpl);
@@ -789,10 +790,12 @@ static CK_RV get_generic_key(CK_SESSION_HANDLE session, unsigned char* data,
789790
CK_OBJECT_HANDLE* key)
790791
{
791792
CK_RV ret;
793+
CK_BBOOL sensitive = (extractable == CK_TRUE) ? CK_FALSE : CK_TRUE;
792794
CK_ATTRIBUTE generic_key[] = {
793795
{ CKA_CLASS, &secretKeyClass, sizeof(secretKeyClass) },
794796
{ CKA_KEY_TYPE, &genericKeyType, sizeof(genericKeyType) },
795797
{ CKA_EXTRACTABLE, &extractable, sizeof(CK_BBOOL) },
798+
{ CKA_SENSITIVE, &sensitive, sizeof(CK_BBOOL) },
796799
{ CKA_SIGN, &ckTrue, sizeof(ckTrue) },
797800
{ CKA_VERIFY, &ckTrue, sizeof(ckTrue) },
798801
{ CKA_VALUE, data, len },
@@ -2053,6 +2056,7 @@ static CK_RV get_rsa_priv_key(CK_SESSION_HANDLE session, unsigned char* privId,
20532056
CK_OBJECT_HANDLE* obj)
20542057
{
20552058
CK_RV ret;
2059+
CK_BBOOL sensitive = (extractable == CK_TRUE) ? CK_FALSE : CK_TRUE;
20562060
CK_ATTRIBUTE rsa_2048_priv_key[] = {
20572061
{ CKA_CLASS, &privKeyClass, sizeof(privKeyClass) },
20582062
{ CKA_KEY_TYPE, &rsaKeyType, sizeof(rsaKeyType) },
@@ -2067,6 +2071,7 @@ static CK_RV get_rsa_priv_key(CK_SESSION_HANDLE session, unsigned char* privId,
20672071
{ CKA_COEFFICIENT, rsa_2048_u, sizeof(rsa_2048_u) },
20682072
{ CKA_PUBLIC_EXPONENT, rsa_2048_pub_exp, sizeof(rsa_2048_pub_exp) },
20692073
{ CKA_EXTRACTABLE, &extractable, sizeof(CK_BBOOL) },
2074+
{ CKA_SENSITIVE, &sensitive, sizeof(CK_BBOOL) },
20702075
{ CKA_TOKEN, &ckTrue, sizeof(ckTrue) },
20712076
{ CKA_ID, privId, privIdLen },
20722077
};
@@ -3415,10 +3420,12 @@ static CK_OBJECT_HANDLE get_ecc_priv_key(CK_SESSION_HANDLE session,
34153420
CK_OBJECT_HANDLE* obj)
34163421
{
34173422
CK_RV ret;
3423+
CK_BBOOL sensitive = (extractable == CK_TRUE) ? CK_FALSE : CK_TRUE;
34183424
CK_ATTRIBUTE ecc_p256_priv_key[] = {
34193425
{ CKA_CLASS, &privKeyClass, sizeof(privKeyClass) },
34203426
{ CKA_KEY_TYPE, &eccKeyType, sizeof(eccKeyType) },
34213427
{ CKA_EXTRACTABLE, &extractable, sizeof(CK_BBOOL) },
3428+
{ CKA_SENSITIVE, &sensitive, sizeof(CK_BBOOL) },
34223429
{ CKA_VERIFY, &ckTrue, sizeof(ckTrue) },
34233430
{ CKA_EC_PARAMS, ecc_p256_params, sizeof(ecc_p256_params) },
34243431
{ CKA_VALUE, ecc_p256_priv, sizeof(ecc_p256_priv) },
@@ -4212,10 +4219,12 @@ static CK_OBJECT_HANDLE get_dh_priv_key(CK_SESSION_HANDLE session,
42124219
CK_OBJECT_HANDLE* obj)
42134220
{
42144221
CK_RV ret;
4222+
CK_BBOOL sensitive = (extractable == CK_TRUE) ? CK_FALSE : CK_TRUE;
42154223
CK_ATTRIBUTE dh_2048_priv_key[] = {
42164224
{ CKA_CLASS, &privKeyClass, sizeof(privKeyClass) },
42174225
{ CKA_KEY_TYPE, &dhKeyType, sizeof(dhKeyType) },
42184226
{ CKA_EXTRACTABLE, &extractable, sizeof(CK_BBOOL) },
4227+
{ CKA_SENSITIVE, &sensitive, sizeof(CK_BBOOL) },
42194228
{ CKA_DERIVE, &ckTrue, sizeof(ckTrue) },
42204229
{ CKA_PRIME, dh_ffdhe2048_p, sizeof(dh_ffdhe2048_p) },
42214230
{ CKA_BASE, dh_ffdhe2048_g, sizeof(dh_ffdhe2048_g) },

0 commit comments

Comments
 (0)