diff --git a/README.md b/README.md index c7908a49..db32fb56 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,14 @@ versions may need to update templates or error-handling: (`--enable-nss`) keep the empty-PIN probe accepted on uninitialized tokens because `PK11_InitPin` bootstraps an empty-password NSS database that way; non-empty PINs are still rejected. +- On `--enable-nss` builds `WP11_MIN_PIN_LEN` defaults to `0`, permitting a + zero-length user PIN. An empty user PIN intentionally disables PIN-based + authentication: `C_GetTokenInfo` clears `CKF_LOGIN_REQUIRED` and all + private/token objects are decoded and accessible at token load without + `C_Login`. This is required for NSS tools (`certutil`, `PK11_InitPin`) that + bootstrap empty-password databases. Integrators who require an enforced + minimum can opt in at build time with `C_EXTRA_FLAGS="-DWP11_MIN_PIN_LEN=N"` + (`N>0`); non-NSS builds already default to `4`. #### Analog Devices, Inc. MAXQ10xx Secure Elements ([MAXQ1065](https://www.analog.com/en/products/maxq1065.html)/MAXQ1080) diff --git a/src/crypto.c b/src/crypto.c index b4ce9c9e..a01e18af 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -1028,6 +1028,17 @@ static CK_RV SetAttributeValue(WP11_Session* session, WP11_Object* obj, *(CK_BBOOL*)attr->pValue == CK_TRUE) return CKR_ATTRIBUTE_READ_ONLY; } + /* PKCS#11 v2.40 sec 4.5: only an SO session may set CKA_TRUSTED to + * CK_TRUE. A regular-user session must not forge trust and bypass the + * CKA_WRAP_WITH_TRUSTED export gate enforced by C_WrapKey. Not + * qualified with !newObject so it also stops C_CreateObject / + * C_GenerateKey from minting a trusted key. CheckAttributes above has + * already validated CKA_TRUSTED as a well-formed CK_BBOOL. */ + if (attr->type == CKA_TRUSTED && + *(CK_BBOOL*)attr->pValue == CK_TRUE && + WP11_Session_GetState(session) != WP11_APP_STATE_RW_SO) { + return CKR_ATTRIBUTE_READ_ONLY; + } /* These class/identity and generated-state attributes are read-only * once the object exists; reject a change. Setting the current value * is a no-op. */ @@ -1583,6 +1594,18 @@ CK_RV C_CopyObject(CK_SESSION_HANDLE hSession, CK_OBJECT_HANDLE hObject, return rv; } + /* Creating a private object requires an authenticated user session. The + * copy template can set CKA_PRIVATE=CK_TRUE, so gate it like + * C_CreateObject. The copy's default CKA_PRIVATE is inherited from the + * source object, whose own login requirement was already enforced by + * WP11_Object_Find above, so only an explicit template override is checked + * here (WP11_NO_IMPLICIT_CLASS = template inspection only). */ + rv = CheckPrivateLogin(session, pTemplate, ulCount, WP11_NO_IMPLICIT_CLASS); + if (rv != CKR_OK) { + WOLFPKCS11_LEAVE("C_CopyObject", rv); + return rv; + } + keyType = WP11_Object_GetType(obj); /* The copy inherits the source's CKA_TOKEN (PKCS#11 v2.40 4.6.2) unless diff --git a/src/internal.c b/src/internal.c index dad45296..d66074c5 100644 --- a/src/internal.c +++ b/src/internal.c @@ -641,6 +641,20 @@ static wolfSSL_Mutex libraryInitLock #define WP11_HAVE_LIBRARY_INIT_LOCK #endif +#if !defined(SINGLE_THREADED) && defined(WOLFSSL_MUTEX_INITIALIZER) && \ + defined(WOLFSSL_MUTEX_INITIALIZER_CLAUSE) && \ + !defined(WOLFPKCS11_TPM_STORE) && defined(WOLFPKCS11_NSS) +/* Permanently-live leaf mutex serializing the module-global storeDir, which + * is set at C_Initialize (before globalLock exists), read in + * wolfPKCS11_Store_Name, and freed in WP11_Library_Final after globalLock is + * released. Without it the free races the set/read (Fenrir F-5868, F-5150). + * Static init mirrors libraryInitLock. It is always acquired as a leaf (no + * other lock is taken while it is held), so it cannot invert any ordering. */ +static wolfSSL_Mutex storeDirLock + WOLFSSL_MUTEX_INITIALIZER_CLAUSE(storeDirLock); +#define WP11_HAVE_STORE_DIR_LOCK +#endif + #ifndef SINGLE_THREADED /** @@ -1149,17 +1163,30 @@ static char* storeDir = NULL; int WP11_SetStoreDir(const char *dir, size_t dirSz) { + int ret = 0; +#ifdef WP11_HAVE_STORE_DIR_LOCK + /* Serialize against the free in WP11_Library_Final and any concurrent set + * so the XFREE/XMALLOC/XMEMCPY sequence is not raced (F-5868). */ + if (wc_LockMutex(&storeDirLock) != 0) + return BAD_MUTEX_E; +#endif if (storeDir != NULL) XFREE(storeDir, NULL, DYNAMIC_TYPE_TMP_BUFFER); storeDir = NULL; if (dir != NULL) { storeDir = (char*) XMALLOC(dirSz + 1, NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (storeDir == NULL) - return MEMORY_E; - XMEMCPY(storeDir, dir, dirSz); - storeDir[dirSz] = '\0'; /* Ensure null termination */ + if (storeDir == NULL) { + ret = MEMORY_E; + } + else { + XMEMCPY(storeDir, dir, dirSz); + storeDir[dirSz] = '\0'; /* Ensure null termination */ + } } - return 0; +#ifdef WP11_HAVE_STORE_DIR_LOCK + wc_UnLockMutex(&storeDirLock); +#endif + return ret; } #endif @@ -1352,6 +1379,9 @@ static int wolfPKCS11_Store_Name(int type, CK_ULONG id1, CK_ULONG id2, char* nam */ enum { WP11_STORE_SUFFIX_RESERVE = 48 }; char homePath[256]; +#ifdef WP11_HAVE_STORE_DIR_LOCK + char storeDirCopy[WP11_STORE_MAX_PATH]; +#endif /* Path order: * 1. Environment variable WOLFPKCS11_TOKEN_PATH @@ -1365,9 +1395,28 @@ static int wolfPKCS11_Store_Name(int type, CK_ULONG id1, CK_ULONG id2, char* nam #endif #ifdef WOLFPKCS11_NSS - if (str == NULL) + if (str == NULL) { +#ifdef WP11_HAVE_STORE_DIR_LOCK + /* Copy storeDir into a local under storeDirLock so a concurrent + * WP11_Library_Final free cannot leave str dangling while we format + * the path below (F-5150). The lock is released before use. */ + if (wc_LockMutex(&storeDirLock) != 0) + return -1; + if (storeDir != NULL) { + size_t sdLen = XSTRLEN(storeDir); + if (sdLen >= sizeof(storeDirCopy)) { + wc_UnLockMutex(&storeDirLock); + return -1; + } + XMEMCPY(storeDirCopy, storeDir, sdLen + 1); + str = storeDirCopy; + } + wc_UnLockMutex(&storeDirLock); +#else str = storeDir; #endif + } +#endif if (str == NULL) { const char* homeDir = NULL; @@ -3290,8 +3339,11 @@ static int wp11_Object_Load_Data(WP11_Object* object, int tokenId, int objId) #ifdef WOLFSSL_MAXQ10XX_CRYPTO #ifdef MAXQ10XX_PRODUCTION_KEY #include "maxq10xx_key.h" -#else -/* TEST KEY. This must be changed for production environments!! */ +#elif defined(WOLFPKCS11_MAXQ10XX_TEST_KEY) +/* INSECURE TEST KEY. The private scalar (last 32 bytes) is public in the + * source, so anyone can forge a valid MXQ_ImportRootCert provisioning + * signature for an arbitrary root certificate. For development against the + * MAXQ10xx evaluation kit only - never ship this. */ static mxq_u1 KeyPairImport[] = { 0xd0,0x97,0x31,0xc7,0x63,0xc0,0x9e,0xe3,0x9a,0xb4,0xd0,0xce,0xa7,0x89,0xab, 0x52,0xc8,0x80,0x3a,0x91,0x77,0x29,0xc3,0xa0,0x79,0x2e,0xe6,0x61,0x8b,0x2d, @@ -3301,6 +3353,8 @@ static mxq_u1 KeyPairImport[] = { 0x72,0x5e,0x88,0xaf,0xc2,0xee,0x8b,0x6f,0xe5,0x36,0xe3,0x60,0x7c,0xf8,0x2c, 0xea,0x3a,0x4f,0xe3,0x6d,0x73 }; +#else +#error "MAXQ10xx root-cert provisioning key is undefined. Define MAXQ10XX_PRODUCTION_KEY (with a real maxq10xx_key.h) for production, or WOLFPKCS11_MAXQ10XX_TEST_KEY to explicitly opt into the built-in INSECURE test key for evaluation-kit development." #endif /* MAXQ10XX_PRODUCTION_KEY */ static int crypto_sha256(const byte *buf, word32 len, byte *hash, @@ -6784,8 +6838,22 @@ void WP11_Library_Final(void) (void)ret; /* store failure cannot be returned, so log and ignore */ } #if !defined (WOLFPKCS11_CUSTOM_STORE) && defined(WOLFPKCS11_NSS) - XFREE(storeDir, NULL, DYNAMIC_TYPE_TMP_BUFFER); - storeDir = NULL; + /* Serialize the free against a concurrent WP11_SetStoreDir / + * wolfPKCS11_Store_Name (F-5868, F-5150). Nested inside libraryInitLock + * (held here); storeDirLock is a leaf so the fixed order + * libraryInitLock -> storeDirLock cannot deadlock. */ + { +#ifdef WP11_HAVE_STORE_DIR_LOCK + int storeDirLocked = (wc_LockMutex(&storeDirLock) == 0); +#endif + XFREE(storeDir, NULL, DYNAMIC_TYPE_TMP_BUFFER); + storeDir = NULL; +#ifdef WP11_HAVE_STORE_DIR_LOCK + /* Only unlock if the lock was actually acquired. */ + if (storeDirLocked) + wc_UnLockMutex(&storeDirLock); +#endif + } #endif #endif /* Cleanup the slots. */ @@ -7050,6 +7118,12 @@ void WP11_Slot_CloseSessions(WP11_Slot* slot) for (curr = slot->session; curr != NULL; curr = curr->next) wp11_Session_Final(curr); WP11_Lock_UnlockRW(&slot->lock); + + /* PKCS#11: closing an application's last session with a token logs the + * application out. Mirror the single-session close path and reset the + * token login state (outside the slot lock, as WP11_Slot_Logout takes it). + */ + WP11_Slot_Logout(slot); } /** @@ -7118,6 +7192,21 @@ static int HashPIN(char* pin, int pinLen, byte* seed, int seedLen, byte* hash, WP11_HASH_PIN_COST, WP11_HASH_PIN_BLOCKSIZE, WP11_HASH_PIN_PARALLEL, hashLen); #elif !defined(NO_SHA256) + /* Fallback: unsalted single-pass SHA-256 of the PIN. This provides no + * salt (the per-token seed is discarded) and no key stretching, so an + * attacker with the token-store file can brute-force a weak PIN offline + * and recover the token storage key. Configure WOLFPKCS11_PBKDF2 or + * HAVE_SCRYPT for a salted, stretched KDF. The selection is compile-time + * and otherwise silent, so warn integrators unless they opt out (F-6232). + * Note: changing this derivation would invalidate existing token stores, + * so hardening it in place is left as a deliberate maintainer decision. */ +#ifndef WOLFPKCS11_ALLOW_WEAK_PIN_KDF + #if defined(_MSC_VER) + #pragma message("wolfPKCS11: no PBKDF2/scrypt - PIN hashing and token key derivation use unsalted SHA-256; define WOLFPKCS11_PBKDF2 or HAVE_SCRYPT for a strong KDF, or WOLFPKCS11_ALLOW_WEAK_PIN_KDF to silence") + #elif defined(__GNUC__) || defined(__clang__) + #warning "wolfPKCS11: no PBKDF2/scrypt - PIN hashing and token key derivation use unsalted SHA-256; define WOLFPKCS11_PBKDF2 or HAVE_SCRYPT for a strong KDF, or WOLFPKCS11_ALLOW_WEAK_PIN_KDF to silence" + #endif +#endif /* fallback to simple SHA2-256 hash of pin */ (void)seed; (void)seedLen; @@ -8517,6 +8606,13 @@ int WP11_Session_SetCbcParams(WP11_Session* session, unsigned char* iv, WP11_CbcParams* cbc = &session->params.cbc; WP11_Data* key; + /* The session params union is shared by every mechanism and is only zeroed + * at allocation, so a prior operation can leave stale multi-part streaming + * state here. Reset it before use (as the other Set*Params routines do) so + * a fresh CBC operation cannot inherit a bogus partial-block count. */ + cbc->partialSz = 0; + XMEMSET(cbc->partial, 0, sizeof(cbc->partial)); + /* AES object on session. */ ret = wc_AesInit(&cbc->aes, NULL, object->devId); #ifdef WOLFSSL_STM32U5_DHUK @@ -14431,6 +14527,13 @@ int WP11_AesCbc_EncryptUpdate(unsigned char* plain, word32 plainSz, int sz = 0; int outSz = 0; + /* Serialize the read-modify-write of cbc->partial/partialSz. Without this + * two threads sharing one session handle can both read partialSz, both + * copy into cbc->partial and both add, driving partialSz past + * AES_BLOCK_SIZE so the next call computes a negative sz and overflows the + * 16-byte partial buffer (F-5764). No caller holds slot->lock here. */ + WP11_Lock_LockRW(&session->slot->lock); + if (cbc->partialSz > 0) { sz = AES_BLOCK_SIZE - cbc->partialSz; if (sz > (int)plainSz) @@ -14464,6 +14567,7 @@ int WP11_AesCbc_EncryptUpdate(unsigned char* plain, word32 plainSz, if (ret == 0) *encSz = outSz; + WP11_Lock_UnlockRW(&session->slot->lock); return ret; } @@ -14538,6 +14642,10 @@ int WP11_AesCbc_DecryptUpdate(unsigned char* enc, word32 encSz, int sz = 0; int outSz = 0; + /* Serialize the partial-block read-modify-write against a concurrent + * update on the same session (F-5764); see WP11_AesCbc_EncryptUpdate. */ + WP11_Lock_LockRW(&session->slot->lock); + if (cbc->partialSz > 0) { sz = AES_BLOCK_SIZE - cbc->partialSz; if (sz > (int)encSz) @@ -14570,6 +14678,7 @@ int WP11_AesCbc_DecryptUpdate(unsigned char* enc, word32 encSz, if (ret == 0) *decSz = outSz; + WP11_Lock_UnlockRW(&session->slot->lock); return ret; } @@ -14742,6 +14851,10 @@ int WP11_AesCbcPad_DecryptUpdate(unsigned char* enc, word32 encSz, int sz = 0; int outSz = 0; + /* Serialize the partial-block read-modify-write against a concurrent + * update on the same session (F-5764); see WP11_AesCbc_EncryptUpdate. */ + WP11_Lock_LockRW(&session->slot->lock); + if (cbc->partialSz > 0) { sz = AES_BLOCK_SIZE - cbc->partialSz; if (sz > (int)encSz) @@ -14755,6 +14868,7 @@ int WP11_AesCbcPad_DecryptUpdate(unsigned char* enc, word32 encSz, * far and leave the operation active (CKR_BUFFER_TOO_SMALL). */ if ((word32)(outSz + AES_BLOCK_SIZE) > bufSz) { *decSz = (word32)outSz + AES_BLOCK_SIZE; + WP11_Lock_UnlockRW(&session->slot->lock); return BUFFER_E; } ret = wc_AesCbcDecrypt(&cbc->aes, dec, cbc->partial, @@ -14770,6 +14884,7 @@ int WP11_AesCbcPad_DecryptUpdate(unsigned char* enc, word32 encSz, sz -= AES_BLOCK_SIZE; if ((word32)(outSz + sz) > bufSz) { *decSz = (word32)(outSz + sz); + WP11_Lock_UnlockRW(&session->slot->lock); return BUFFER_E; } ret = wc_AesCbcDecrypt(&cbc->aes, dec, enc, sz); @@ -14784,6 +14899,7 @@ int WP11_AesCbcPad_DecryptUpdate(unsigned char* enc, word32 encSz, if (ret == 0) *decSz = outSz; + WP11_Lock_UnlockRW(&session->slot->lock); return ret; } diff --git a/src/slot.c b/src/slot.c index 4764eef4..6d1d792e 100644 --- a/src/slot.c +++ b/src/slot.c @@ -384,7 +384,7 @@ static CK_MECHANISM_TYPE mechanismList[] = { #endif #ifndef NO_AES CKM_AES_KEY_GEN, -#ifdef HAVE_AES_KEY_WRAP +#ifdef HAVE_AES_KEYWRAP CKM_AES_KEY_WRAP, CKM_AES_KEY_WRAP_PAD, #endif @@ -539,6 +539,9 @@ CK_RV C_GetMechanismList(CK_SLOT_ID slotID, return rv; } else if (*pulCount < (CK_ULONG)mechanismCnt) { + /* PKCS#11: on CKR_BUFFER_TOO_SMALL *pulCount must be set to the + * required count so the two-call (size-query then fetch) idiom works. */ + *pulCount = mechanismCnt; rv = CKR_BUFFER_TOO_SMALL; WOLFPKCS11_LEAVE("C_GetMechanismList", rv); return rv; @@ -716,7 +719,7 @@ static CK_MECHANISM_INFO tlsMacMechInfo = { static CK_MECHANISM_INFO aesKeyGenMechInfo = { 16, 32, CKF_GENERATE }; -#ifdef HAVE_AES_KEY_WRAP +#ifdef HAVE_AES_KEYWRAP static CK_MECHANISM_INFO aesKeyWrapMechInfo = { 16, 32, CKF_ENCRYPT | CKF_DECRYPT | CKF_WRAP | CKF_UNWRAP }; @@ -1029,7 +1032,7 @@ CK_RV C_GetMechanismInfo(CK_SLOT_ID slotID, CK_MECHANISM_TYPE type, case CKM_AES_KEY_GEN: XMEMCPY(pInfo, &aesKeyGenMechInfo, sizeof(CK_MECHANISM_INFO)); break; -#ifdef HAVE_AES_KEY_WRAP +#ifdef HAVE_AES_KEYWRAP case CKM_AES_KEY_WRAP: case CKM_AES_KEY_WRAP_PAD: XMEMCPY(pInfo, &aesKeyWrapMechInfo, sizeof(CK_MECHANISM_INFO)); @@ -1283,12 +1286,16 @@ CK_RV C_InitToken(CK_SLOT_ID slotID, CK_UTF8CHAR_PTR pPin, return rv; } + /* PKCS#11: an open session on the token must fail C_InitToken with + * CKR_SESSION_EXISTS regardless of whether the token is already + * initialized. Check this unconditionally before any token-reset logic. */ + if (WP11_Slot_HasSession(slot)) { + rv = CKR_SESSION_EXISTS; + WOLFPKCS11_LEAVE("C_InitToken", rv); + return rv; + } + if (WP11_Slot_IsTokenInitialized(slot)) { - if (WP11_Slot_HasSession(slot)) { - rv = CKR_SESSION_EXISTS; - WOLFPKCS11_LEAVE("C_InitToken", rv); - return rv; - } if (WP11_Slot_SOPin_IsSet(slot)) { /* Verify the SO PIN with the failed-login lockout applied, so this * path cannot be used to brute-force the SO PIN (Fenrir F-4632). diff --git a/tests/pkcs11test.c b/tests/pkcs11test.c index e23c3f49..71a9355f 100644 --- a/tests/pkcs11test.c +++ b/tests/pkcs11test.c @@ -6538,12 +6538,16 @@ static CK_RV test_wrap_key_wrap_with_trusted(void* args) }; CK_ULONG untrustedWrapTmplCnt = sizeof(untrustedWrapTmpl) / sizeof(*untrustedWrapTmpl); + /* CKA_TRUSTED may only be set by the SO, so the trusted wrapping key is + * provisioned as a public token object under an SO session. */ CK_ATTRIBUTE trustedWrapTmpl[] = { { CKA_CLASS, &secretKeyClass, sizeof(secretKeyClass) }, { CKA_KEY_TYPE, &aesKeyType, sizeof(aesKeyType) }, { CKA_VALUE, aes_128_key, sizeof(aes_128_key) }, { CKA_WRAP, &ckTrue, sizeof(ckTrue) }, { CKA_TRUSTED, &ckTrusted, sizeof(CK_BBOOL) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, }; CK_ULONG trustedWrapTmplCnt = sizeof(trustedWrapTmpl) / sizeof(*trustedWrapTmpl); @@ -6558,13 +6562,44 @@ static CK_RV test_wrap_key_wrap_with_trusted(void* args) memset(keyData, 0x55, sizeof(keyData)); - ret = funcList->C_CreateObject(session, untrustedWrapTmpl, - untrustedWrapTmplCnt, &untrustedKey); - CHECK_CKR(ret, "Create untrusted AES wrapping key"); + /* Provision the trusted wrapping key as the SO. The token has a single + * login state, so log the harness user out, log in as SO to create the + * public token key, then restore the user session. */ + ret = funcList->C_Logout(session); + CHECK_CKR(ret, "Logout user before SO provisioning"); + if (ret == CKR_OK) { + ret = funcList->C_Login(session, CKU_SO, soPin, soPinLen); + CHECK_CKR(ret, "Login SO to create trusted key"); + } if (ret == CKR_OK) { ret = funcList->C_CreateObject(session, trustedWrapTmpl, trustedWrapTmplCnt, &trustedKey); - CHECK_CKR(ret, "Create trusted AES wrapping key"); + CHECK_CKR(ret, "Create trusted AES wrapping key (SO)"); + } + if (ret == CKR_OK) { + ret = funcList->C_Logout(session); + CHECK_CKR(ret, "Logout SO after provisioning"); + } + if (ret == CKR_OK) { + ret = funcList->C_Login(session, CKU_USER, userPin, userPinLen); + CHECK_CKR(ret, "Re-login user after SO provisioning"); + } + + /* A user session must not be able to forge CKA_TRUSTED (F-5867). */ + if (ret == CKR_OK) { + CK_OBJECT_HANDLE forgedKey = CK_INVALID_HANDLE; + ret = funcList->C_CreateObject(session, trustedWrapTmpl, + trustedWrapTmplCnt, &forgedKey); + if (ret == CKR_OK) + funcList->C_DestroyObject(session, forgedKey); + CHECK_CKR_FAIL(ret, CKR_ATTRIBUTE_READ_ONLY, + "User session forging CKA_TRUSTED must be rejected"); + } + + if (ret == CKR_OK) { + ret = funcList->C_CreateObject(session, untrustedWrapTmpl, + untrustedWrapTmplCnt, &untrustedKey); + CHECK_CKR(ret, "Create untrusted AES wrapping key"); } if (ret == CKR_OK) { ret = funcList->C_CreateObject(session, wwtTmpl, wwtTmplCnt, &key); diff --git a/wolfpkcs11/internal.h b/wolfpkcs11/internal.h index 0e72be16..29f31184 100644 --- a/wolfpkcs11/internal.h +++ b/wolfpkcs11/internal.h @@ -311,7 +311,12 @@ C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT" #define WP11_HASH_PIN_PARALLEL 1 #endif -/* PIN length constraints. */ +/* PIN length constraints. The NSS default is 0: an empty user PIN is allowed + * and intentionally disables login (C_GetTokenInfo clears CKF_LOGIN_REQUIRED + * and token objects are decoded at load without C_Login) for NSS tool + * compatibility (certutil / PK11_InitPin bootstrap empty-password databases). + * Integrators who require an enforced minimum can override with + * -DWP11_MIN_PIN_LEN=N (N>0). */ #ifndef WP11_MIN_PIN_LEN #ifdef WOLFPKCS11_NSS #define WP11_MIN_PIN_LEN 0