Skip to content

Commit 25afee1

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 25afee1

File tree

4 files changed

+681
-12
lines changed

4 files changed

+681
-12
lines changed

‎src/crypto.c‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,11 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
514514
ulCount);
515515
break;
516516
case CKO_SECRET_KEY:
517+
#ifndef WOLFPKCS11_NSS
518+
if (ret == CKR_OK)
519+
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
520+
ulCount);
521+
#endif
517522
if (ret == CKR_OK)
518523
ret = SetIfNotFound(obj, CKA_EXTRACTABLE, trueVal, pTemplate,
519524
ulCount);
@@ -531,9 +536,21 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
531536
ulCount);
532537
break;
533538
case CKO_PRIVATE_KEY:
539+
#ifndef WOLFPKCS11_NSS
540+
if (ret == CKR_OK)
541+
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
542+
ulCount);
543+
if (ret == CKR_OK)
544+
ret = SetIfNotFound(obj, CKA_EXTRACTABLE, falseVal, pTemplate,
545+
ulCount);
546+
#else
547+
/* NSS operates as the internal crypto module and needs both
548+
* non-sensitive and extractable private keys to read key material
549+
* directly via C_GetAttributeValue during TLS operations. */
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: 59 additions & 5 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

@@ -6964,6 +6988,9 @@ void WP11_Slot_Logout(WP11_Slot* slot)
69646988
ret = wp11_Object_Encode(object, 1);
69656989
object = object->next;
69666990
}
6991+
/* Zero token key only on user logout — SO logout must preserve it
6992+
* for subsequent object encryption (e.g., empty-PIN flow). */
6993+
wc_ForceZero(slot->token.key, sizeof(slot->token.key));
69676994
}
69686995
#endif
69696996
slot->token.loginState = WP11_APP_STATE_RW_PUBLIC;
@@ -8223,6 +8250,9 @@ static WP11_Object* wp11_Session_FindNext(WP11_Session* session, int onToken,
82238250
}
82248251
#endif
82258252

8253+
/* Note: this CKA_PRIVATE check is intentionally active in NSS mode.
8254+
* NSS accesses private objects by handle (via WP11_Object_Find) rather
8255+
* than discovering them through C_FindObjects enumeration. */
82268256
if ((ret->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE) {
82278257
if (!onToken)
82288258
WP11_Lock_LockRO(&session->slot->token.lock);
@@ -9242,8 +9272,26 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle,
92429272
WP11_Lock_UnlockRO(&session->slot->token.lock);
92439273
}
92449274

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

92489296
return ret;
92499297
}
@@ -11516,7 +11564,7 @@ int WP11_Rsa_Verify_Recover(CK_MECHANISM_TYPE mechanism, unsigned char* sig,
1151611564
}
1151711565
}
1151811566
if (data_out != NULL) {
11519-
*outLen = (out + *outLen) - data_out;
11567+
*outLen = (CK_ULONG)((out + *outLen) - data_out);
1152011568
XMEMMOVE(out, data_out, *outLen);
1152111569
}
1152211570
}
@@ -14304,15 +14352,21 @@ int WP11_Digest_Single(unsigned char* data, word32 dataLen,
1430414352
WP11_Digest* digest = &session->params.digest;
1430514353

1430614354
blockLen = wc_HashGetDigestSize(digest->hashType);
14355+
if (blockLen < 0) {
14356+
wc_HashFree(&digest->hash, digest->hashType);
14357+
session->init = 0;
14358+
return CKR_FUNCTION_FAILED;
14359+
}
1430714360

14308-
if (data == NULL) {
14361+
if (dataOut == NULL) {
1430914362
*dataOutLen = (word32)blockLen;
1431014363
return CKR_OK;
1431114364
}
1431214365
if (*dataOutLen < (word32)blockLen) {
1431314366
return BUFFER_E;
1431414367
}
1431514368
ret = wc_Hash(digest->hashType, data, dataLen, dataOut, *dataOutLen);
14369+
*dataOutLen = (word32)blockLen;
1431614370

1431714371
wc_HashFree(&digest->hash, digest->hashType);
1431814372

‎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)