Skip to content

Commit 341feab

Browse files
committed
Fix static code compliance findings
- F-3637: pin SecretObject_GetAttr CKA_SENSITIVE guard with a test exercising CKA_SENSITIVE=TRUE + CKA_EXTRACTABLE=TRUE. - F-2776: RsaObject_GetAttr / EcObject_GetAttr / DhObject_GetAttr / MldsaObject_GetAttr / MlKemObject_GetAttr now return CKR_ATTRIBUTE_SENSITIVE alongside the existing CK_UNAVAILABLE_INFORMATION sentinel. Updated test_attributes_rsa / ecc / dh in pkcs11test.c and pkcs11mtt.c to expect the new return. - F-1343: introduced WP11_FLAG_NOT_MODIFIABLE (inverted, mirroring NOT_COPYABLE / NOT_DESTROYABLE) and WP11_Object_IsModifiable; C_SetAttributeValue returns CKR_ACTION_PROHIBITED for CKA_MODIFIABLE=FALSE objects. Gate lives in the public entry point only so C_CopyObject (governed by CKA_COPYABLE) is unaffected. - F-3144: new CheckPrivateLogin helper in crypto.c wired into C_CreateObject, C_GenerateKey, C_GenerateKeyPair (both templates), and C_UnwrapKey. Returns CKR_USER_NOT_LOGGED_IN for CKA_PRIVATE=TRUE on a public session; empty-PIN tokens bypassed to match the find-time filter in WP11_Session_Find. - F-2370: SetAttributeDefaults seeds CKA_PRIVATE=CK_TRUE for CKO_PRIVATE_KEY / CKO_SECRET_KEY and CK_FALSE for CKO_PUBLIC_KEY, gated by WOLFPKCS11_LEGACY_PRIVATE_FALSE_DEFAULT. - F-2774: CKA_WRAP / CKA_UNWRAP default flipped to CK_FALSE in SetAttributeDefaults, gated by WOLFPKCS11_LEGACY_WRAP_TRUE_DEFAULT. pkcs11test.c / pkcs11mtt.c get_aes_128_key, get_generic_key, get_rsa_pub_key, get_rsa_priv_key set CKA_WRAP/CKA_UNWRAP=TRUE explicitly where wrap/unwrap is exercised. - F-3407: wp11_Token_Init no longer eager-marks state INITIALIZED; WP11_Slot_TokenReset sets it on C_InitToken. Redundant state-vs-INITIALIZED checks dropped from WP11_Slot_CheckSOPin / CheckUserPin so the NSS empty-PIN bootstrap probe still passes on a fresh slot.
1 parent 4bac443 commit 341feab

6 files changed

Lines changed: 1229 additions & 33 deletions

File tree

src/crypto.c

Lines changed: 170 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,82 @@ static CK_RV FindValidAttributeType(CK_ATTRIBUTE* pTemplate, CK_ULONG ulCount,
324324
return CKR_OK;
325325
}
326326

327+
/* Sentinel for CheckPrivateLogin: callers that legitimately have no implicit
328+
* class (C_CreateObject, C_UnwrapKey - both require CKA_CLASS in template per
329+
* spec) pass this so the helper falls back to template inspection only. */
330+
#define WP11_NO_IMPLICIT_CLASS ((CK_OBJECT_CLASS)~(CK_OBJECT_CLASS)0)
331+
332+
/* Per PKCS#11 v3.0 sec 5.1 Table 16: creating, generating, or unwrapping an
333+
* object whose effective CKA_PRIVATE is CK_TRUE on a session that is not
334+
* logged in as the user must return CKR_USER_NOT_LOGGED_IN. Empty-PIN tokens
335+
* treat public sessions as logged-in (matches the find-time filter in
336+
* WP11_Session_Find) so this gate must mirror that.
337+
*
338+
* `implicitClass` lets callers whose object class is implied by the API
339+
* itself (C_GenerateKey always creates a CKO_SECRET_KEY, C_GenerateKeyPair
340+
* produces CKO_PUBLIC_KEY/CKO_PRIVATE_KEY) supply that class even when the
341+
* template omits CKA_CLASS. Without this, the spec-default
342+
* CKA_PRIVATE=CK_TRUE would not be inferred for templates like
343+
* `{CKA_VALUE_LEN, CKA_KEY_TYPE}` and a public session could silently
344+
* generate a private key. An explicit CKA_PRIVATE attribute in the template
345+
* still wins over the class-derived default.
346+
*
347+
* Gated by WOLFPKCS11_LEGACY_PRIVATE_FALSE_DEFAULT so the class-based
348+
* inference matches the legacy default in SetAttributeDefaults. */
349+
static CK_RV CheckPrivateLogin(WP11_Session* session,
350+
CK_ATTRIBUTE_PTR pTemplate, CK_ULONG ulCount,
351+
CK_OBJECT_CLASS implicitClass)
352+
{
353+
CK_ATTRIBUTE* attr = NULL;
354+
CK_BBOOL isPrivate = CK_FALSE;
355+
WP11_Slot* slot;
356+
357+
if (session == NULL)
358+
return CKR_OK;
359+
360+
if (pTemplate != NULL && ulCount > 0)
361+
FindAttributeType(pTemplate, ulCount, CKA_PRIVATE, &attr);
362+
363+
if (attr != NULL && attr->pValue != NULL &&
364+
attr->ulValueLen == sizeof(CK_BBOOL)) {
365+
isPrivate = *(CK_BBOOL*)attr->pValue;
366+
}
367+
else {
368+
#ifndef WOLFPKCS11_LEGACY_PRIVATE_FALSE_DEFAULT
369+
CK_OBJECT_CLASS objClass = implicitClass;
370+
CK_ATTRIBUTE* classAttr = NULL;
371+
if (pTemplate != NULL && ulCount > 0)
372+
FindAttributeType(pTemplate, ulCount, CKA_CLASS, &classAttr);
373+
if (classAttr != NULL && classAttr->pValue != NULL &&
374+
classAttr->ulValueLen == sizeof(CK_OBJECT_CLASS)) {
375+
objClass = *(CK_OBJECT_CLASS*)classAttr->pValue;
376+
}
377+
if (objClass == CKO_PRIVATE_KEY || objClass == CKO_SECRET_KEY)
378+
isPrivate = CK_TRUE;
379+
#endif
380+
}
381+
382+
if (!isPrivate)
383+
return CKR_OK;
384+
385+
slot = WP11_Session_GetSlot(session);
386+
if (slot == NULL)
387+
return CKR_OK;
388+
/* Fresh / unprovisioned tokens (no user PIN initialized) have nothing to
389+
* log in as, and the existing find-time private-object filter
390+
* (src/internal.c) likewise bypasses on Has_Empty_Pin. Mirror both:
391+
* an unset user PIN is treated as "no protection required" so callers
392+
* that pre-date C_InitToken / C_InitPIN keep working. After the token
393+
* is provisioned with a non-empty user PIN, the gate engages. */
394+
if (!WP11_Slot_IsTokenUserPinInitialized(slot))
395+
return CKR_OK;
396+
if (WP11_Slot_Has_Empty_Pin(slot))
397+
return CKR_OK;
398+
if (!WP11_Slot_IsLoggedIn(slot))
399+
return CKR_USER_NOT_LOGGED_IN;
400+
return CKR_OK;
401+
}
402+
327403
/**
328404
* Check the value and length are valid for the data type of the attributes in
329405
* the template.
@@ -468,7 +544,14 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
468544
CK_BBOOL falseVal = CK_FALSE;
469545
CK_BBOOL encrypt = CK_TRUE;
470546
CK_BBOOL recover = CK_TRUE;
547+
/* PKCS#11 v2.40 sec 4.4.1: CKA_WRAP and CKA_UNWRAP default to CK_FALSE.
548+
* The pre-fix default (CK_TRUE) silently made every secret/RSA key a
549+
* wrapping key, violating least-privilege (Fenrir 2774). */
550+
#ifdef WOLFPKCS11_LEGACY_WRAP_TRUE_DEFAULT
471551
CK_BBOOL wrap = CK_TRUE;
552+
#else
553+
CK_BBOOL wrap = CK_FALSE;
554+
#endif
472555
CK_BBOOL derive = (keyType == CKO_PUBLIC_KEY ? CK_FALSE : CK_TRUE);
473556
CK_BBOOL verify = CK_TRUE;
474557
CK_BBOOL sign = CK_FALSE;
@@ -525,6 +608,15 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
525608
/* Defaults if not set */
526609
switch (keyType) {
527610
case CKO_PUBLIC_KEY:
611+
#ifndef WOLFPKCS11_LEGACY_PRIVATE_FALSE_DEFAULT
612+
/* Spec default: public keys are CKA_PRIVATE=FALSE (this is also
613+
* what the historical implementation produced, but make it
614+
* explicit so the default is stable when the macro toggles
615+
* private/secret keys below). */
616+
if (ret == CKR_OK)
617+
ret = SetIfNotFound(obj, CKA_PRIVATE, falseVal, pTemplate,
618+
ulCount);
619+
#endif
528620
if (ret == CKR_OK)
529621
ret = SetIfNotFound(obj, CKA_ENCRYPT, encrypt, pTemplate,
530622
ulCount);
@@ -546,6 +638,13 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
546638
#endif
547639
break;
548640
case CKO_SECRET_KEY:
641+
#ifndef WOLFPKCS11_LEGACY_PRIVATE_FALSE_DEFAULT
642+
/* PKCS#11 v2.40 sec 4.4.1: secret keys default to
643+
* CKA_PRIVATE=CK_TRUE (Fenrir 2370). */
644+
if (ret == CKR_OK)
645+
ret = SetIfNotFound(obj, CKA_PRIVATE, trueVal, pTemplate,
646+
ulCount);
647+
#endif
549648
#ifndef WOLFPKCS11_NSS
550649
if (ret == CKR_OK)
551650
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
@@ -561,13 +660,22 @@ static CK_RV SetAttributeDefaults(WP11_Object* obj, CK_OBJECT_CLASS keyType,
561660
ret = SetIfNotFound(obj, CKA_DECRYPT, trueVal, pTemplate,
562661
ulCount);
563662
/* CKA_SIGN / CKA_VERIFY default false */
663+
/* CKA_WRAP / CKA_UNWRAP defaults follow the macro-gated `wrap'
664+
* (CK_TRUE under the legacy macro, CK_FALSE per spec). */
564665
if (ret == CKR_OK)
565-
ret = SetIfNotFound(obj, CKA_WRAP, trueVal, pTemplate, ulCount);
666+
ret = SetIfNotFound(obj, CKA_WRAP, wrap, pTemplate, ulCount);
566667
if (ret == CKR_OK)
567-
ret = SetIfNotFound(obj, CKA_UNWRAP, trueVal, pTemplate,
668+
ret = SetIfNotFound(obj, CKA_UNWRAP, wrap, pTemplate,
568669
ulCount);
569670
break;
570671
case CKO_PRIVATE_KEY:
672+
#ifndef WOLFPKCS11_LEGACY_PRIVATE_FALSE_DEFAULT
673+
/* PKCS#11 v2.40 sec 4.4.1: private keys default to
674+
* CKA_PRIVATE=CK_TRUE (Fenrir 2370). */
675+
if (ret == CKR_OK)
676+
ret = SetIfNotFound(obj, CKA_PRIVATE, trueVal, pTemplate,
677+
ulCount);
678+
#endif
571679
#ifndef WOLFPKCS11_NSS
572680
if (ret == CKR_OK)
573681
ret = SetIfNotFound(obj, CKA_SENSITIVE, trueVal, pTemplate,
@@ -659,6 +767,13 @@ static CK_RV SetAttributeValue(WP11_Session* session, WP11_Object* obj,
659767
if (!WP11_Session_IsRW(session) && WP11_Object_OnToken(obj))
660768
return CKR_SESSION_READ_ONLY;
661769

770+
/* CKA_MODIFIABLE enforcement (Fenrir 1343) lives in C_SetAttributeValue,
771+
* not this shared helper - C_CopyObject also funnels through here with
772+
* newObject=FALSE after the new object has just inherited the source
773+
* object's modifiable flag via WP11_Object_Copy, and copy semantics are
774+
* governed by CKA_COPYABLE (already checked separately), not
775+
* CKA_MODIFIABLE. */
776+
662777
rv = CheckAttributes(pTemplate, ulCount, 1);
663778
if (rv != CKR_OK)
664779
return rv;
@@ -1257,6 +1372,15 @@ CK_RV C_CreateObject(CK_SESSION_HANDLE hSession, CK_ATTRIBUTE_PTR pTemplate,
12571372
}
12581373
}
12591374

1375+
/* C_CreateObject requires CKA_CLASS in the template; no API-implied
1376+
* class to fall back on. */
1377+
rv = CheckPrivateLogin(session, pTemplate, ulCount,
1378+
WP11_NO_IMPLICIT_CLASS);
1379+
if (rv != CKR_OK) {
1380+
WOLFPKCS11_LEAVE("C_CreateObject", rv);
1381+
return rv;
1382+
}
1383+
12601384
rv = CreateObject(session, pTemplate, ulCount, &object);
12611385
if (rv != CKR_OK) {
12621386
WOLFPKCS11_LEAVE("C_CreateObject", rv);
@@ -1736,6 +1860,16 @@ CK_RV C_SetAttributeValue(CK_SESSION_HANDLE hSession,
17361860
return rv;
17371861
}
17381862

1863+
/* PKCS#11 v2.40 sec 4.4.1: an object with CKA_MODIFIABLE=CK_FALSE must
1864+
* reject all attribute changes (Fenrir 1343). Gate only the public
1865+
* entry point - C_CopyObject also uses SetAttributeValue with
1866+
* newObject=FALSE but is governed by CKA_COPYABLE, not CKA_MODIFIABLE. */
1867+
if (!WP11_Object_IsModifiable(obj)) {
1868+
rv = CKR_ACTION_PROHIBITED;
1869+
WOLFPKCS11_LEAVE("C_SetAttributeValue", rv);
1870+
return rv;
1871+
}
1872+
17391873
rv = SetAttributeValue(session, obj, pTemplate, ulCount, CK_FALSE);
17401874
WOLFPKCS11_LEAVE("C_SetAttributeValue", rv);
17411875
return rv;
@@ -6889,6 +7023,15 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,
68897023
}
68907024
}
68917025

7026+
/* C_GenerateKey always produces a secret key, even when the template
7027+
* omits CKA_CLASS - pass the implied class so the spec-default
7028+
* CKA_PRIVATE=CK_TRUE still gates login. */
7029+
rv = CheckPrivateLogin(session, pTemplate, ulCount, CKO_SECRET_KEY);
7030+
if (rv != CKR_OK) {
7031+
WOLFPKCS11_LEAVE("C_GenerateKey", rv);
7032+
return rv;
7033+
}
7034+
68927035
switch (pMechanism->mechanism) {
68937036
#ifndef NO_AES
68947037
case CKM_AES_KEY_GEN:
@@ -7332,6 +7475,22 @@ CK_RV C_GenerateKeyPair(CK_SESSION_HANDLE hSession,
73327475
}
73337476
}
73347477

7478+
/* Each template's API-implied class lets the helper detect the
7479+
* spec-default CKA_PRIVATE=CK_TRUE for the private arm even when
7480+
* the template omits CKA_CLASS. An explicit CKA_PRIVATE=TRUE in the
7481+
* public template is still flagged because the helper consults the
7482+
* template attribute first. */
7483+
rv = CheckPrivateLogin(session, pPublicKeyTemplate,
7484+
ulPublicKeyAttributeCount, CKO_PUBLIC_KEY);
7485+
if (rv == CKR_OK) {
7486+
rv = CheckPrivateLogin(session, pPrivateKeyTemplate,
7487+
ulPrivateKeyAttributeCount, CKO_PRIVATE_KEY);
7488+
}
7489+
if (rv != CKR_OK) {
7490+
WOLFPKCS11_LEAVE("C_GenerateKeyPair", rv);
7491+
return rv;
7492+
}
7493+
73357494
switch (pMechanism->mechanism) {
73367495
#if !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN)
73377496
case CKM_RSA_PKCS_KEY_PAIR_GEN:
@@ -7923,6 +8082,15 @@ CK_RV C_UnwrapKey(CK_SESSION_HANDLE hSession,
79238082
}
79248083
}
79258084

8085+
/* C_UnwrapKey requires CKA_CLASS in the unwrapped key template per
8086+
* spec; no API-implied class to fall back on. */
8087+
rv = CheckPrivateLogin(session, pTemplate, ulAttributeCount,
8088+
WP11_NO_IMPLICIT_CLASS);
8089+
if (rv != CKR_OK) {
8090+
WOLFPKCS11_LEAVE("C_UnwrapKey", rv);
8091+
return rv;
8092+
}
8093+
79268094
*phKey = CK_INVALID_HANDLE;
79278095

79288096
ret = WP11_Object_Find(session, hUnwrappingKey, &unwrappingKey);

0 commit comments

Comments
 (0)