From 8fbf767098c6ed3d34aaf3a584bad077d689a487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Fri, 24 Jul 2026 13:30:02 +0200 Subject: [PATCH 01/10] Fix EC key generation error-path leak The second i2d_ASN1_OBJECT() call can fail after ec_params has been allocated. That error path returned the pooled session but leaked the DER buffer. Route curve lookup, DER encoding, and allocation failures through one cleanup path that releases the session and frees ec_params. --- src/p11_key.c | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/p11_key.c b/src/p11_key.c index 371a4843..37197ef4 100644 --- a/src/p11_key.c +++ b/src/p11_key.c @@ -652,26 +652,18 @@ int pkcs11_ec_keygen(PKCS11_SLOT_private *slot, const char *curve, curve_nid = OBJ_sn2nid(curve); if (curve_nid == NID_undef) curve_nid = OBJ_ln2nid(curve); - if (curve_nid == NID_undef) { - pkcs11_put_session(slot, session); - return -1; - } + if (curve_nid == NID_undef) + goto error; curve_obj = OBJ_nid2obj(curve_nid); - if (!curve_obj) { - pkcs11_put_session(slot, session); - return -1; - } + if (!curve_obj) + goto error; /* convert to DER format and take just the length */ ec_params_len = i2d_ASN1_OBJECT(curve_obj, NULL); - if (ec_params_len < 0) { - pkcs11_put_session(slot, session); - return -1; - } + if (ec_params_len < 0) + goto error; ec_params = OPENSSL_malloc(ec_params_len); - if (!ec_params) { - pkcs11_put_session(slot, session); - return -1; - } + if (!ec_params) + goto error; /** * ec_params points to beginning of DER encoded object. Since we need this * location later and OpenSSL changes it in i2d_ASN1_OBJECT to point to 1 byte @@ -679,10 +671,8 @@ int pkcs11_ec_keygen(PKCS11_SLOT_private *slot, const char *curve, * pointer tmp */ tmp = ec_params; - if (i2d_ASN1_OBJECT(curve_obj, &tmp) < 0) { - pkcs11_put_session(slot, session); - return -1; - } + if (i2d_ASN1_OBJECT(curve_obj, &tmp) < 0) + goto error; /* The following attributes are necessary for ECDSA and ECDH mechanisms */ /* pubkey attributes */ @@ -713,6 +703,11 @@ int pkcs11_ec_keygen(PKCS11_SLOT_private *slot, const char *curve, CRYPTOKI_checkerr(CKR_F_PKCS11_GENERATE_KEY, rv); return 0; + +error: + pkcs11_put_session(slot, session); + OPENSSL_free(ec_params); + return -1; } #endif /* OPENSSL_NO_EC */ From 4a19cb0b49a26f1b532c0c961c2969f02c647545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Fri, 24 Jul 2026 13:30:33 +0200 Subject: [PATCH 02/10] Fix RSA public-key lookup template leak When a private RSA object omits CKA_PUBLIC_EXPONENT, pkcs11_get_rsa() builds a temporary template to find the matching public key. Attributes allocated for that template were never freed. Clear the template immediately after the object lookup, its final use, so every lookup outcome releases the allocated attributes. --- src/p11_rsa.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/p11_rsa.c b/src/p11_rsa.c index 818acbba..ca6f5ad8 100644 --- a/src/p11_rsa.c +++ b/src/p11_rsa.c @@ -188,6 +188,7 @@ static RSA *pkcs11_get_rsa(PKCS11_OBJECT_private *key) pkcs11_addattr_var(&tmpl, CKA_CLASS, class_public_key); pkcs11_addattr_bn(&tmpl, CKA_MODULUS, rsa_n); pubkey = pkcs11_object_from_template(slot, session, &tmpl); + pkcs11_zap_attrs(&tmpl); if (pubkey && !pkcs11_getattr_bn(ctx, session, pubkey->object, CKA_PUBLIC_EXPONENT, &rsa_e)) { pkcs11_object_free(pubkey); From 5ec85c63f18190220002b92e67ed0dccd64a08a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Mon, 27 Jul 2026 14:59:05 +0200 Subject: [PATCH 03/10] Make session-pool mode transitions concurrency-safe Key generation dropped slot->lock before switching to R/W mode and then unlocked it again unconditionally. Mode changes could also call C_CloseAllSessions() while another thread still used a checked-out handle. Serialize mode changes with transition_active under the slot lock. Track checked-out sessions explicitly, block normal acquisition for the entire drain, mode switch, relogin, and key-generation checkout, and close sessions only after every lease is returned. Use condition-variable broadcasts so transition and consumer waiters all recheck their predicates. Acquire and relogin the key-generation session without re-entering the normal gated path. Restore pool accounting on every error, and reject invalid releases defensively. Reset transition and pool accounting during slot reload so a child cannot inherit state owned by a vanished parent thread. Keeping all transition state under the existing slot lock avoids a separate mutex that could be inherited locked. --- src/libp11-int.h | 18 ++- src/p11_key.c | 33 +--- src/p11_pthread.h | 6 + src/p11_slot.c | 378 ++++++++++++++++++++++++++++++++++------------ 4 files changed, 311 insertions(+), 124 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index 8a18121c..d662adad 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -94,6 +94,8 @@ struct pkcs11_slot_private { pthread_mutex_t lock; pthread_cond_t cond; int8_t rw_mode, logged_in; + int transition_active; /* session-pool transition active */ + unsigned int sessions_in_use; /* sessions currently checked out */ CK_SLOT_ID id; CK_SESSION_HANDLE *session_pool; unsigned int session_head, session_tail, session_poolsize; @@ -279,14 +281,20 @@ extern void pkcs11_CTX_unload(PKCS11_CTX *ctx); /* Free a libp11 context */ extern void pkcs11_CTX_free(PKCS11_CTX *ctx); -/* Open a session in RO or RW mode */ +/* Set the R/O or R/W mode of the session pool */ extern int pkcs11_open_session(PKCS11_SLOT_private *, int rw); -/* Acquire a session from the slot specific session pool */ -extern int pkcs11_get_session(PKCS11_SLOT_private *, int rw, CK_SESSION_HANDLE *sessionp); +/* Acquire a session from the slot-specific session pool */ +extern int pkcs11_get_session(PKCS11_SLOT_private *, int rw, + CK_SESSION_HANDLE *sessionp); -/* Return a session the the slot specific session pool */ -extern void pkcs11_put_session(PKCS11_SLOT_private *, CK_SESSION_HANDLE session); +/* Switch to R/W mode, log in again if needed, and acquire a session */ +extern int pkcs11_session_pool_acquire_keygen(PKCS11_SLOT_private *, + CK_SESSION_HANDLE *sessionp); + +/* Release a session back to the slot-specific session pool */ +extern void pkcs11_put_session(PKCS11_SLOT_private *, + CK_SESSION_HANDLE session); /* Get a list of all slots */ extern int pkcs11_enumerate_slots(PKCS11_CTX_private *ctx, diff --git a/src/p11_key.c b/src/p11_key.c index 37197ef4..1528ca17 100644 --- a/src/p11_key.c +++ b/src/p11_key.c @@ -125,7 +125,6 @@ static int pkcs11_find_keys(PKCS11_SLOT_private *, CK_SESSION_HANDLE, unsigned i PKCS11_TEMPLATE *); static int pkcs11_init_key(PKCS11_SLOT_private *, CK_SESSION_HANDLE session, CK_OBJECT_HANDLE o, CK_OBJECT_CLASS type, PKCS11_KEY **); -static int pkcs11_init_keygen(PKCS11_SLOT_private *, CK_SESSION_HANDLE *); static int pkcs11_next_key(PKCS11_CTX_private *ctx, PKCS11_SLOT_private *, CK_SESSION_HANDLE session, CK_OBJECT_CLASS type); static int pkcs11_store_key(PKCS11_SLOT_private *, EVP_PKEY *, CK_OBJECT_CLASS, @@ -586,7 +585,7 @@ int pkcs11_rsa_keygen(PKCS11_SLOT_private *slot, unsigned int bits, CK_OBJECT_HANDLE pub_key_obj, priv_key_obj; int rv; - if (pkcs11_init_keygen(slot, &session)) + if (pkcs11_session_pool_acquire_keygen(slot, &session)) return -1; /* The following attributes are necessary for RSA encryption and DSA */ @@ -644,7 +643,7 @@ int pkcs11_ec_keygen(PKCS11_SLOT_private *slot, const char *curve, ASN1_OBJECT *curve_obj = NULL; int curve_nid = NID_undef; - if (pkcs11_init_keygen(slot, &session)) + if (pkcs11_session_pool_acquire_keygen(slot, &session)) return -1; curve_nid = EC_curve_nist2nid(curve); @@ -730,7 +729,7 @@ int pkcs11_eddsa_keygen(PKCS11_SLOT_private *slot, unsigned char *eddsa_params = NULL; size_t eddsa_params_len = 0; - if (pkcs11_init_keygen(slot, &session)) + if (pkcs11_session_pool_acquire_keygen(slot, &session)) return -1; if (nid == NID_ED25519) { @@ -787,7 +786,7 @@ int pkcs11_xdh_keygen(PKCS11_SLOT_private *slot, unsigned char *xdh_params = NULL; size_t xdh_params_len = 0; - if (pkcs11_init_keygen(slot, &session)) + if (pkcs11_session_pool_acquire_keygen(slot, &session)) return -1; if (nid == NID_X25519) { @@ -846,7 +845,7 @@ int pkcs11_mldsa_keygen(PKCS11_SLOT_private *slot, CK_OBJECT_HANDLE pub_key_obj, priv_key_obj; CK_RV rv; - if (pkcs11_init_keygen(slot, &session)) + if (pkcs11_session_pool_acquire_keygen(slot, &session)) return -1; switch (nid) { @@ -937,7 +936,7 @@ int pkcs11_mlkem_keygen(PKCS11_SLOT_private *slot, int nid, CK_OBJECT_HANDLE pub_key_obj, priv_key_obj; CK_RV rv; - if (pkcs11_init_keygen(slot, &session)) + if (pkcs11_session_pool_acquire_keygen(slot, &session)) return -1; switch (nid) { @@ -1027,7 +1026,7 @@ int pkcs11_slhdsa_keygen(PKCS11_SLOT_private *slot, CK_OBJECT_HANDLE pub_key_obj, priv_key_obj; CK_RV rv; - if (pkcs11_init_keygen(slot, &session)) + if (pkcs11_session_pool_acquire_keygen(slot, &session)) return -1; switch (nid) { @@ -1120,7 +1119,7 @@ int pkcs11_falcon_keygen(PKCS11_SLOT_private *slot, CK_OBJECT_HANDLE pub_key_obj, priv_key_obj; CK_RV rv; - if (pkcs11_init_keygen(slot, &session)) + if (pkcs11_session_pool_acquire_keygen(slot, &session)) return -1; if (nid == NID_FALCON_512) { @@ -1649,22 +1648,6 @@ CK_RSA_PKCS_MGF_TYPE pkcs11_md2ckg(const EVP_MD *md) } } -static int pkcs11_init_keygen(PKCS11_SLOT_private *slot, CK_SESSION_HANDLE *session) -{ - pthread_mutex_lock(&slot->lock); - /* R/W session is mandatory for key generation. */ - if (slot->rw_mode != 1) { - pthread_mutex_unlock(&slot->lock); - if (pkcs11_open_session(slot, 1)) - return -1; - /* open_session will call C_CloseAllSessions which logs everyone out */ - if (pkcs11_login(slot, 0, slot->prev_pin)) - return -1; - } - pthread_mutex_unlock(&slot->lock); - return pkcs11_get_session(slot, 1, session); -} - static void pkcs11_common_pubkey_attr(PKCS11_TEMPLATE *pubtmpl, const char *label, const unsigned char *id, size_t id_len) { diff --git a/src/p11_pthread.h b/src/p11_pthread.h index 139e8ab0..62364634 100644 --- a/src/p11_pthread.h +++ b/src/p11_pthread.h @@ -87,6 +87,12 @@ static int pthread_cond_signal(pthread_cond_t *cond) return 0; } +static int pthread_cond_broadcast(pthread_cond_t *cond) +{ + WakeAllConditionVariable(cond); + return 0; +} + #else #error Locking not supported on this platform. diff --git a/src/p11_slot.c b/src/p11_slot.c index 56a5c4ad..f236826f 100644 --- a/src/p11_slot.c +++ b/src/p11_slot.c @@ -105,108 +105,211 @@ int pkcs11_enumerate_slots(PKCS11_CTX_private *ctx, PKCS11_SLOT **slotp, return 0; } +static void pkcs11_wipe_cache(PKCS11_SLOT_private *slot) +{ + pkcs11_destroy_keys(slot, CKO_PRIVATE_KEY); + pkcs11_destroy_keys(slot, CKO_PUBLIC_KEY); + pkcs11_destroy_certs(slot); +} + +static void pkcs11_session_pool_reset_locked(PKCS11_SLOT_private *slot) +{ + slot->sessions_in_use = 0; + slot->num_sessions = 0; + slot->session_head = 0; + slot->session_tail = 0; +} + /* - * Open a session with this slot + * Keep transition ownership in plain slot state rather than a separate mutex: + * after fork, only the child thread survives, and pkcs11_reload_slot() can + * safely clear this reservation before the child opens new sessions. */ -int pkcs11_open_session(PKCS11_SLOT_private *slot, int rw) +static void pkcs11_session_pool_begin_transition(PKCS11_SLOT_private *slot) { - PKCS11_CTX_private *ctx = slot->ctx; + pthread_mutex_lock(&slot->lock); + while (slot->transition_active) + pthread_cond_wait(&slot->cond, &slot->lock); + slot->transition_active = 1; + while (slot->sessions_in_use != 0) + pthread_cond_wait(&slot->cond, &slot->lock); + pthread_mutex_unlock(&slot->lock); +} +static void pkcs11_session_pool_end_transition(PKCS11_SLOT_private *slot) +{ pthread_mutex_lock(&slot->lock); - /* If different mode requested, flush pool */ - if (rw != slot->rw_mode) { - CRYPTOKI_call(ctx, C_CloseAllSessions(slot->id)); - slot->rw_mode = rw; - slot->logged_in = -1; - } - slot->num_sessions = 0; - slot->session_head = slot->session_tail = 0; + slot->transition_active = 0; + pthread_cond_broadcast(&slot->cond); pthread_mutex_unlock(&slot->lock); +} + +/* The caller must hold slot->lock and own a drained transition. */ +static int pkcs11_session_pool_switch_mode_locked( + PKCS11_SLOT_private *slot, int rw, int *changed) +{ + PKCS11_CTX_private *ctx = slot->ctx; + CK_RV rv; + + if (changed) + *changed = 0; + if (rw == slot->rw_mode) + return 0; + if (slot->sessions_in_use != 0) { + CKRerr(CKR_F_PKCS11_OPEN_SESSION, CKR_GENERAL_ERROR); + return -1; + } + rv = CRYPTOKI_call(ctx, C_CloseAllSessions(slot->id)); + if (rv != CKR_OK) { + CKRerr(CKR_F_PKCS11_OPEN_SESSION, rv); + return -1; + } + pkcs11_session_pool_reset_locked(slot); + slot->rw_mode = rw; + slot->logged_in = -1; + if (changed) + *changed = 1; return 0; } - -static void pkcs11_wipe_cache(PKCS11_SLOT_private *slot) +int pkcs11_open_session(PKCS11_SLOT_private *slot, int rw) { - pkcs11_destroy_keys(slot, CKO_PRIVATE_KEY); - pkcs11_destroy_keys(slot, CKO_PUBLIC_KEY); - pkcs11_destroy_certs(slot); + int rv; + + pkcs11_session_pool_begin_transition(slot); + pthread_mutex_lock(&slot->lock); + rv = pkcs11_session_pool_switch_mode_locked(slot, rw, NULL); + pthread_mutex_unlock(&slot->lock); + pkcs11_session_pool_end_transition(slot); + + return rv; } -int pkcs11_get_session(PKCS11_SLOT_private *slot, int rw, CK_SESSION_HANDLE *sessionp) +enum pkcs11_session_select_result { + PKCS11_SESSION_SELECT_ERROR = -1, + PKCS11_SESSION_SELECT_SUCCESS = 0, + PKCS11_SESSION_SELECT_UNAVAILABLE = 1 +}; + +/* The caller must hold slot->lock. This helper never waits. */ +static int pkcs11_session_pool_select_locked(PKCS11_SLOT_private *slot, + CK_SESSION_HANDLE *sessionp) { PKCS11_CTX_private *ctx = slot->ctx; - int rv = CKR_OK; CK_SESSION_INFO session_info; + CK_RV rv; + + while (slot->session_head != slot->session_tail) { + *sessionp = slot->session_pool[slot->session_head]; + slot->session_head = + (slot->session_head + 1) % slot->session_poolsize; + + /* Discard sessions invalidated by the PKCS#11 module. */ + rv = CRYPTOKI_call(ctx, + C_GetSessionInfo(*sessionp, &session_info)); + if (rv == CKR_OK) + return PKCS11_SESSION_SELECT_SUCCESS; + if (slot->num_sessions == 0) { + slot->session_head = slot->session_tail = 0; + CKRerr(CKR_F_PKCS11_GET_SESSION, CKR_GENERAL_ERROR); + return PKCS11_SESSION_SELECT_ERROR; + } + slot->num_sessions--; + if (slot->num_sessions == 0) { + /* Object handles are valid across sessions, so only clear + * the cache when there are no valid sessions. */ + pkcs11_wipe_cache(slot); + } + } + + if (slot->num_sessions >= slot->max_sessions) + return PKCS11_SESSION_SELECT_UNAVAILABLE; + + rv = CRYPTOKI_call(ctx, + C_OpenSession(slot->id, + CKF_SERIAL_SESSION | + (slot->rw_mode ? CKF_RW_SESSION : 0), + NULL, NULL, sessionp)); + if (rv == CKR_OK) { + slot->num_sessions++; + return PKCS11_SESSION_SELECT_SUCCESS; + } + + /* If the module reports a lower limit than the configured one, + * wait for one of this pool's sessions when that can make progress. */ + if (rv == CKR_SESSION_COUNT && slot->num_sessions > 0) { + slot->max_sessions = slot->num_sessions; + return PKCS11_SESSION_SELECT_UNAVAILABLE; + } + CKRerr(CKR_F_PKCS11_GET_SESSION, rv); + return PKCS11_SESSION_SELECT_ERROR; +} + +int pkcs11_get_session(PKCS11_SLOT_private *slot, int rw, + CK_SESSION_HANDLE *sessionp) +{ + int select_result; if (rw < 0) return -1; pthread_mutex_lock(&slot->lock); - if (slot->rw_mode < 0) - slot->rw_mode = rw; - rw = slot->rw_mode; - do { - /* Get session from the pool */ - if (slot->session_head != slot->session_tail) { - *sessionp = slot->session_pool[slot->session_head]; - slot->session_head = (slot->session_head + 1) % slot->session_poolsize; - - /* Check if session is valid */ - rv = CRYPTOKI_call(ctx, - C_GetSessionInfo(*sessionp, &session_info)); - if (rv == CKR_OK) { - break; - } else { - /* Forget this session */ - slot->num_sessions--; - if (slot->num_sessions == 0) { - /* Object handles are valid across - * sessions, so the cache should only be - * cleared when there are no valid - * sessions.*/ - pkcs11_wipe_cache(slot); - } - continue; - } + for (;;) { + while (slot->transition_active) + pthread_cond_wait(&slot->cond, &slot->lock); + if (slot->rw_mode < 0) + slot->rw_mode = rw; + + select_result = pkcs11_session_pool_select_locked(slot, sessionp); + if (select_result == PKCS11_SESSION_SELECT_SUCCESS) { + slot->sessions_in_use++; + pthread_mutex_unlock(&slot->lock); + return 0; } - - /* Check if new can be instantiated */ - if (slot->num_sessions < slot->max_sessions) { - rv = CRYPTOKI_call(ctx, - C_OpenSession(slot->id, - CKF_SERIAL_SESSION | (rw ? CKF_RW_SESSION : 0), - NULL, NULL, sessionp)); - if (rv == CKR_OK) { - slot->num_sessions++; - break; - } else { - pthread_mutex_unlock(&slot->lock); - return -1; - } - - /* Remember the maximum session count */ - if (rv == CKR_SESSION_COUNT) - slot->max_sessions = slot->num_sessions; + if (select_result == PKCS11_SESSION_SELECT_ERROR) { + pthread_mutex_unlock(&slot->lock); + return -1; } - /* Wait for a session to become available */ + /* The configured maximum is in use. Every wakeup must + * recheck both transition ownership and pool availability. */ pthread_cond_wait(&slot->cond, &slot->lock); - } while (1); - pthread_mutex_unlock(&slot->lock); - - return 0; + } } -void pkcs11_put_session(PKCS11_SLOT_private *slot, CK_SESSION_HANDLE session) +void pkcs11_put_session(PKCS11_SLOT_private *slot, + CK_SESSION_HANDLE session) { - pthread_mutex_lock(&slot->lock); + PKCS11_CTX_private *ctx = slot->ctx; + unsigned int next_tail; + CK_RV rv; - slot->session_pool[slot->session_tail] = session; - slot->session_tail = (slot->session_tail + 1) % slot->session_poolsize; - pthread_cond_signal(&slot->cond); + pthread_mutex_lock(&slot->lock); + if (slot->sessions_in_use == 0 || slot->session_poolsize < 2) { + CKRerr(CKR_F_PKCS11_GET_SESSION, CKR_SESSION_HANDLE_INVALID); + pthread_cond_broadcast(&slot->cond); + pthread_mutex_unlock(&slot->lock); + return; + } + next_tail = (slot->session_tail + 1) % slot->session_poolsize; + slot->sessions_in_use--; + if (next_tail == slot->session_head) { + /* Do not overwrite an available session if accounting was + * corrupted by an invalid or duplicate release. */ + rv = CRYPTOKI_call(ctx, C_CloseSession(session)); + if (slot->num_sessions > 0) + slot->num_sessions--; + if (slot->num_sessions == 0) + pkcs11_wipe_cache(slot); + if (rv != CKR_OK) + CKRerr(CKR_F_PKCS11_GET_SESSION, rv); + } else { + slot->session_pool[slot->session_tail] = session; + slot->session_tail = next_tail; + } + pthread_cond_broadcast(&slot->cond); pthread_mutex_unlock(&slot->lock); } @@ -215,7 +318,47 @@ void pkcs11_put_session(PKCS11_SLOT_private *slot, CK_SESSION_HANDLE session) */ int pkcs11_is_logged_in(PKCS11_SLOT_private *slot, int so, int *res) { + pthread_mutex_lock(&slot->lock); *res = slot->logged_in == so; + pthread_mutex_unlock(&slot->lock); + return 0; +} + +/* Authenticate using a session already checked out from this slot. */ +static int pkcs11_login_on_session(PKCS11_SLOT_private *slot, + CK_SESSION_HANDLE session, int so, const char *pin) +{ + PKCS11_CTX_private *ctx = slot->ctx; + char *pin_copy = NULL; + CK_RV rv; + + if (pin) { + pin_copy = OPENSSL_strdup(pin); + if (!pin_copy) + return -1; + } + + rv = CRYPTOKI_call(ctx, + C_Login(session, so ? CKU_SO : CKU_USER, + (CK_UTF8CHAR *) pin_copy, + pin_copy ? (CK_ULONG)strlen(pin_copy) : 0)); + if (rv != CKR_OK && rv != CKR_USER_ALREADY_LOGGED_IN) { + if (pin_copy) { + OPENSSL_cleanse(pin_copy, strlen(pin_copy)); + OPENSSL_free(pin_copy); + } + CKRerr(CKR_F_PKCS11_LOGIN, rv); + return -1; + } + + pthread_mutex_lock(&slot->lock); + if (slot->prev_pin) { + OPENSSL_cleanse(slot->prev_pin, strlen(slot->prev_pin)); + OPENSSL_free(slot->prev_pin); + } + slot->prev_pin = pin_copy; + slot->logged_in = so; + pthread_mutex_unlock(&slot->lock); return 0; } @@ -224,34 +367,63 @@ int pkcs11_is_logged_in(PKCS11_SLOT_private *slot, int so, int *res) */ int pkcs11_login(PKCS11_SLOT_private *slot, int so, const char *pin) { - PKCS11_CTX_private *ctx = slot->ctx; CK_SESSION_HANDLE session; - int rv; + int logged_in, rv; - if (slot->logged_in >= 0) + pthread_mutex_lock(&slot->lock); + logged_in = slot->logged_in; + pthread_mutex_unlock(&slot->lock); + if (logged_in >= 0) return 0; /* Nothing to do */ - /* SO needs a r/w session, user can be checked with a r/o session. */ + /* SO needs a r/w session, user can use a r/o session. */ if (pkcs11_get_session(slot, so, &session)) return -1; - - rv = CRYPTOKI_call(ctx, - C_Login(session, so ? CKU_SO : CKU_USER, - (CK_UTF8CHAR *) pin, pin ? (unsigned long) strlen(pin) : 0)); + rv = pkcs11_login_on_session(slot, session, so, pin); pkcs11_put_session(slot, session); + return rv; +} + +int pkcs11_session_pool_acquire_keygen(PKCS11_SLOT_private *slot, + CK_SESSION_HANDLE *sessionp) +{ + const char *pin = NULL; + int login_state, mode_changed = 0, select_result; + int session_acquired = 0, rv = -1; - if (rv && rv != CKR_USER_ALREADY_LOGGED_IN) { /* logged in -> OK */ - CRYPTOKI_checkerr(CKR_F_PKCS11_LOGIN, rv); + /* Keep normal acquisition gated until the R/W session has been + * acquired and any login invalidated by the mode switch is restored. */ + pkcs11_session_pool_begin_transition(slot); + pthread_mutex_lock(&slot->lock); + login_state = slot->logged_in; + if (pkcs11_session_pool_switch_mode_locked( + slot, 1, &mode_changed)) { + pthread_mutex_unlock(&slot->lock); + goto out; } - if (slot->prev_pin != pin) { - if (slot->prev_pin) { - OPENSSL_cleanse(slot->prev_pin, strlen(slot->prev_pin)); - OPENSSL_free(slot->prev_pin); - } - slot->prev_pin = OPENSSL_strdup(pin); + + select_result = pkcs11_session_pool_select_locked(slot, sessionp); + if (select_result != PKCS11_SESSION_SELECT_SUCCESS) { + if (select_result == PKCS11_SESSION_SELECT_UNAVAILABLE) + CKRerr(CKR_F_PKCS11_GET_SESSION, CKR_SESSION_COUNT); + pthread_mutex_unlock(&slot->lock); + goto out; } - slot->logged_in = so; - return 0; + slot->sessions_in_use++; + session_acquired = 1; + if (mode_changed && login_state >= 0) + pin = slot->prev_pin; + pthread_mutex_unlock(&slot->lock); + + if (mode_changed && login_state >= 0 && + pkcs11_login_on_session(slot, *sessionp, login_state, pin)) + goto out; + rv = 0; +out: + if (rv != 0 && session_acquired) + pkcs11_put_session(slot, *sessionp); + pkcs11_session_pool_end_transition(slot); + return rv; } /* @@ -261,6 +433,9 @@ int pkcs11_reload_slot(PKCS11_SLOT_private *slot) { int logged_in = slot->logged_in; + /* No transition owner or checked-out session survives fork(). */ + slot->transition_active = 0; + slot->sessions_in_use = 0; slot->num_sessions = 0; slot->session_head = slot->session_tail = 0; if (logged_in >= 0) { @@ -279,18 +454,31 @@ int pkcs11_logout(PKCS11_SLOT_private *slot) { PKCS11_CTX_private *ctx = slot->ctx; CK_SESSION_HANDLE session; - int rv = CKR_OK; + int logged_in, session_acquired = 0, rv = CKR_OK; /* Calling PKCS11_logout invalidates all cached * keys we have */ pkcs11_wipe_cache(slot); - if (pkcs11_get_session(slot, slot->logged_in, &session) == 0) { + pthread_mutex_lock(&slot->lock); + logged_in = slot->logged_in; + pthread_mutex_unlock(&slot->lock); + if (pkcs11_get_session(slot, logged_in, &session) == 0) { + session_acquired = 1; rv = CRYPTOKI_call(ctx, C_Logout(session)); + if (rv == CKR_OK) { + pthread_mutex_lock(&slot->lock); + slot->logged_in = -1; + pthread_mutex_unlock(&slot->lock); + } pkcs11_put_session(slot, session); } CRYPTOKI_checkerr(CKR_F_PKCS11_LOGOUT, rv); - slot->logged_in = -1; + if (!session_acquired) { + pthread_mutex_lock(&slot->lock); + slot->logged_in = -1; + pthread_mutex_unlock(&slot->lock); + } return 0; } @@ -464,6 +652,8 @@ int pkcs11_slot_unref(PKCS11_SLOT_private *slot) if (pkcs11_atomic_add(&slot->refcnt, -1, &slot->lock) != 0) return 0; + /* Destruction also obeys the no-close-while-leased invariant. */ + pkcs11_session_pool_begin_transition(slot); pkcs11_wipe_cache(slot); if (slot->prev_pin) { OPENSSL_cleanse(slot->prev_pin, strlen(slot->prev_pin)); @@ -471,8 +661,8 @@ int pkcs11_slot_unref(PKCS11_SLOT_private *slot) } CRYPTOKI_call(slot->ctx, C_CloseAllSessions(slot->id)); OPENSSL_free(slot->session_pool); - pthread_mutex_destroy(&slot->lock); pthread_cond_destroy(&slot->cond); + pthread_mutex_destroy(&slot->lock); return 1; } From d33f9ec266b14bda83c6c2aec050fc5a79f69c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Mon, 27 Jul 2026 14:59:20 +0200 Subject: [PATCH 04/10] Make internal session-pool operations explicit The get, put, and open helper names obscured that these functions manage libp11's per-slot pool rather than direct Cryptoki session ownership. Rename the helpers and every internal caller to describe pool acquisition, release, and mode changes explicitly. Keep PKCS11_open_session() unchanged to preserve the public API and ABI. --- src/libp11-int.h | 6 ++--- src/p11_cert.c | 8 +++---- src/p11_ckr.c | 4 ++-- src/p11_ec.c | 4 ++-- src/p11_eddsa.c | 16 ++++++------- src/p11_falcon.c | 4 ++-- src/p11_front.c | 2 +- src/p11_key.c | 58 ++++++++++++++++++++++++------------------------ src/p11_mldsa.c | 4 ++-- src/p11_pkey.c | 28 +++++++++++------------ src/p11_rsa.c | 14 ++++++------ src/p11_slhdsa.c | 4 ++-- src/p11_slot.c | 32 +++++++++++++------------- 13 files changed, 92 insertions(+), 92 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index d662adad..288f5204 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -282,10 +282,10 @@ extern void pkcs11_CTX_unload(PKCS11_CTX *ctx); extern void pkcs11_CTX_free(PKCS11_CTX *ctx); /* Set the R/O or R/W mode of the session pool */ -extern int pkcs11_open_session(PKCS11_SLOT_private *, int rw); +extern int pkcs11_session_pool_set_mode(PKCS11_SLOT_private *, int rw); /* Acquire a session from the slot-specific session pool */ -extern int pkcs11_get_session(PKCS11_SLOT_private *, int rw, +extern int pkcs11_session_pool_acquire(PKCS11_SLOT_private *, int rw, CK_SESSION_HANDLE *sessionp); /* Switch to R/W mode, log in again if needed, and acquire a session */ @@ -293,7 +293,7 @@ extern int pkcs11_session_pool_acquire_keygen(PKCS11_SLOT_private *, CK_SESSION_HANDLE *sessionp); /* Release a session back to the slot-specific session pool */ -extern void pkcs11_put_session(PKCS11_SLOT_private *, +extern void pkcs11_session_pool_release(PKCS11_SLOT_private *, CK_SESSION_HANDLE session); /* Get a list of all slots */ diff --git a/src/p11_cert.c b/src/p11_cert.c index 6ece01a1..98675736 100644 --- a/src/p11_cert.c +++ b/src/p11_cert.c @@ -50,11 +50,11 @@ int pkcs11_enumerate_certs(PKCS11_SLOT_private *slot, const PKCS11_CERT *cert_te pkcs11_addattr_s(&tmpl, CKA_LABEL, cert_template->label); } - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; rv = pkcs11_find_certs(slot, &tmpl, session); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (rv < 0) { pkcs11_destroy_certs(slot); return -1; @@ -211,7 +211,7 @@ int pkcs11_store_certificate(PKCS11_SLOT_private *slot, X509 *x509, char *label, CK_MECHANISM_TYPE ckm_md; /* First, make sure we have a session */ - if (pkcs11_get_session(slot, 1, &session)) + if (pkcs11_session_pool_acquire(slot, 1, &session)) return -1; /* Now build the template */ @@ -295,7 +295,7 @@ int pkcs11_store_certificate(PKCS11_SLOT_private *slot, X509 *x509, char *label, if (rv == CKR_OK) { r = pkcs11_init_cert(slot, session, object, ret_cert); } - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_STORE_CERTIFICATE, rv); return r; diff --git a/src/p11_ckr.c b/src/p11_ckr.c index 68440e25..6bf2e679 100644 --- a/src/p11_ckr.c +++ b/src/p11_ckr.c @@ -48,7 +48,7 @@ static ERR_STRING_DATA CKR_str_functs[] = { {ERR_FUNC(CKR_F_PKCS11_LOGOUT), "pkcs11_logout"}, {ERR_FUNC(CKR_F_PKCS11_NEXT_CERT), "pkcs11_next_cert"}, {ERR_FUNC(CKR_F_PKCS11_NEXT_KEY), "pkcs11_next_key"}, - {ERR_FUNC(CKR_F_PKCS11_OPEN_SESSION), "pkcs11_open_session"}, + {ERR_FUNC(CKR_F_PKCS11_OPEN_SESSION), "pkcs11_session_pool_set_mode"}, {ERR_FUNC(CKR_F_PKCS11_PRIVATE_DECRYPT), "pkcs11_private_decrypt"}, {ERR_FUNC(CKR_F_PKCS11_PRIVATE_ENCRYPT), "pkcs11_private_encrypt"}, {ERR_FUNC(CKR_F_PKCS11_RELOAD_KEY), "pkcs11_reload_key"}, @@ -56,7 +56,7 @@ static ERR_STRING_DATA CKR_str_functs[] = { {ERR_FUNC(CKR_F_PKCS11_STORE_CERTIFICATE), "pkcs11_store_certificate"}, {ERR_FUNC(CKR_F_PKCS11_STORE_KEY), "pkcs11_store_key"}, {ERR_FUNC(CKR_F_PKCS11_RELOAD_CERTIFICATE), "pkcs11_reload_certificate"}, - {ERR_FUNC(CKR_F_PKCS11_GET_SESSION), "pkcs11_get_session"}, + {ERR_FUNC(CKR_F_PKCS11_GET_SESSION), "pkcs11_session_pool_acquire"}, {ERR_FUNC(CKR_F_PKCS11_EDDSA_SIGN), "pkcs11_eddsa_sign"}, {0, NULL} }; diff --git a/src/p11_ec.c b/src/p11_ec.c index 2a562a36..1a8e2041 100644 --- a/src/p11_ec.c +++ b/src/p11_ec.c @@ -325,7 +325,7 @@ static EC_KEY *pkcs11_get_ec(PKCS11_OBJECT_private *key) * Continue even if it fails, as the sign operation does not need * it if the PKCS#11 module or the hardware can figure this out */ - if (pkcs11_get_session(slot, 0, &session)) { + if (pkcs11_session_pool_acquire(slot, 0, &session)) { EC_KEY_free(ec); return NULL; } @@ -335,7 +335,7 @@ static EC_KEY *pkcs11_get_ec(PKCS11_OBJECT_private *key) no_point = pkcs11_get_point_associated(ec, key, CKO_PUBLIC_KEY, session); if (no_point && key->object_class == CKO_PRIVATE_KEY) /* Retry with the certificate */ no_point = pkcs11_get_point_associated(ec, key, CKO_CERTIFICATE, session); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (key->object_class == CKO_PRIVATE_KEY && EC_KEY_get0_private_key(ec) == NULL) { BIGNUM *bn = BN_new(); diff --git a/src/p11_eddsa.c b/src/p11_eddsa.c index 17fe827b..a31ebe5b 100644 --- a/src/p11_eddsa.c +++ b/src/p11_eddsa.c @@ -92,10 +92,10 @@ static int pkcs11_eddsa_pmeth_sign(EVP_PKEY_CTX *ctx, unsigned char *sig, if (!slot) return 0; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return 0; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (!pkcs11_evp_pkey_eddsa_sign(key, sig, siglen, tbs, tbslen)) return 0; @@ -136,10 +136,10 @@ static int pkcs11_eddsa_pmeth_digestsign(EVP_MD_CTX *ctx, unsigned char *sig, if (!slot) return -1; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); /* Step 1: caller asks for signature length only */ if (sig == NULL) { @@ -351,10 +351,10 @@ static int pkcs11_xdh_pmeth_derive(EVP_PKEY_CTX *ctx, unsigned char *secret, if (!slot) return -1; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); type = EVP_PKEY_id(pkey); switch (type) { @@ -755,7 +755,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key, slot = key->slot; ctx = slot->ctx; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; obj = pkcs11_choose_public_source(key, session, &obj_needs_free); @@ -775,7 +775,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key, } end: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (!ok) { OPENSSL_free(*raw); diff --git a/src/p11_falcon.c b/src/p11_falcon.c index b425ca80..54cc1aad 100644 --- a/src/p11_falcon.c +++ b/src/p11_falcon.c @@ -217,7 +217,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key, slot = key->slot; ctx = slot->ctx; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; obj = pkcs11_choose_public_source(key, session, &obj_needs_free); @@ -239,7 +239,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key, } end: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (!ok) { OPENSSL_free(*raw); diff --git a/src/p11_front.c b/src/p11_front.c index 7cffe80e..6cd6f235 100644 --- a/src/p11_front.c +++ b/src/p11_front.c @@ -75,7 +75,7 @@ int PKCS11_open_session(PKCS11_SLOT *pslot, int rw) PKCS11_SLOT_private *slot = pslot->_private; if (check_slot_fork(slot) < 0) return -1; - return pkcs11_open_session(slot, rw); + return pkcs11_session_pool_set_mode(slot, rw); } int PKCS11_enumerate_slots(PKCS11_CTX *pctx, diff --git a/src/p11_key.c b/src/p11_key.c index 1528ca17..350ccc32 100644 --- a/src/p11_key.c +++ b/src/p11_key.c @@ -462,7 +462,7 @@ PKCS11_OBJECT_private *pkcs11_object_from_template(PKCS11_SLOT_private *slot, int release = 0; if (session == CK_INVALID_HANDLE) { - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return NULL; release = 1; } @@ -472,7 +472,7 @@ PKCS11_OBJECT_private *pkcs11_object_from_template(PKCS11_SLOT_private *slot, obj = pkcs11_object_from_handle(slot, session, object_handle); if (release) - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return obj; } @@ -547,7 +547,7 @@ int pkcs11_reload_object(PKCS11_OBJECT_private *obj) CK_SESSION_HANDLE session; PKCS11_TEMPLATE tmpl = {0}; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; pkcs11_addattr_var(&tmpl, CKA_CLASS, obj->object_class); @@ -559,7 +559,7 @@ int pkcs11_reload_object(PKCS11_OBJECT_private *obj) obj->object = pkcs11_handle_from_template(slot, session, &tmpl); pkcs11_zap_attrs(&tmpl); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (obj->object == CK_INVALID_HANDLE) CRYPTOKI_checkerr(CKR_F_PKCS11_RELOAD_KEY, CKR_OBJECT_HANDLE_INVALID); @@ -609,7 +609,7 @@ int pkcs11_rsa_keygen(PKCS11_SLOT_private *slot, unsigned int bits, privtmpl.attrs, privtmpl.nattr, &pub_key_obj, &priv_key_obj)); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); /* zap all memory allocated when building the template */ pkcs11_zap_attrs(&privtmpl); @@ -692,7 +692,7 @@ int pkcs11_ec_keygen(PKCS11_SLOT_private *slot, const char *curve, privtmpl.attrs, privtmpl.nattr, &pub_key_obj, &priv_key_obj)); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); /* zap all memory allocated when building the template */ pkcs11_zap_attrs(&privtmpl); @@ -704,7 +704,7 @@ int pkcs11_ec_keygen(PKCS11_SLOT_private *slot, const char *curve, return 0; error: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); OPENSSL_free(ec_params); return -1; } @@ -739,7 +739,7 @@ int pkcs11_eddsa_keygen(PKCS11_SLOT_private *slot, eddsa_params = (unsigned char *)OID_ED448; eddsa_params_len = sizeof(OID_ED448); } else { - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return -1; /* unsupported */ } @@ -760,7 +760,7 @@ int pkcs11_eddsa_keygen(PKCS11_SLOT_private *slot, &pub_key_obj, &priv_key_obj)); /* cleanup */ - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); pkcs11_zap_attrs(&privtmpl); pkcs11_zap_attrs(&pubtmpl); @@ -796,7 +796,7 @@ int pkcs11_xdh_keygen(PKCS11_SLOT_private *slot, xdh_params = (unsigned char *)OID_X448; xdh_params_len = sizeof(OID_X448); } else { - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return -1; /* unsupported */ } @@ -817,7 +817,7 @@ int pkcs11_xdh_keygen(PKCS11_SLOT_private *slot, &pub_key_obj, &priv_key_obj)); /* cleanup */ - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); pkcs11_zap_attrs(&privtmpl); pkcs11_zap_attrs(&pubtmpl); @@ -859,7 +859,7 @@ int pkcs11_mldsa_keygen(PKCS11_SLOT_private *slot, signParamSet = CKP_ML_DSA_87; break; default: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return -1; /* unsupported */ } @@ -908,7 +908,7 @@ int pkcs11_mldsa_keygen(PKCS11_SLOT_private *slot, } /* cleanup */ - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); pkcs11_zap_attrs(&privtmpl); pkcs11_zap_attrs(&pubtmpl); @@ -950,7 +950,7 @@ int pkcs11_mlkem_keygen(PKCS11_SLOT_private *slot, int nid, kemParamSet = CKP_ML_KEM_1024; break; default: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return -1; /* unsupported */ } @@ -999,7 +999,7 @@ int pkcs11_mlkem_keygen(PKCS11_SLOT_private *slot, int nid, } /* cleanup */ - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); pkcs11_zap_attrs(&privtmpl); pkcs11_zap_attrs(&pubtmpl); @@ -1067,7 +1067,7 @@ int pkcs11_slhdsa_keygen(PKCS11_SLOT_private *slot, signParamSet = CKP_SLH_DSA_SHAKE_256F; break; default: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return -1; /* unsupported */ } @@ -1089,7 +1089,7 @@ int pkcs11_slhdsa_keygen(PKCS11_SLOT_private *slot, &pub_key_obj, &priv_key_obj)); /* cleanup */ - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); pkcs11_zap_attrs(&privtmpl); pkcs11_zap_attrs(&pubtmpl); @@ -1127,7 +1127,7 @@ int pkcs11_falcon_keygen(PKCS11_SLOT_private *slot, } else if (nid == NID_FALCON_1024) { signParamSet = CKP_FALCON_1024; } else { - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return -1; /* unsupported */ } @@ -1149,7 +1149,7 @@ int pkcs11_falcon_keygen(PKCS11_SLOT_private *slot, &pub_key_obj, &priv_key_obj)); /* cleanup */ - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); pkcs11_zap_attrs(&privtmpl); pkcs11_zap_attrs(&pubtmpl); @@ -1249,7 +1249,7 @@ static int pkcs11_store_key(PKCS11_SLOT_private *slot, EVP_PKEY *pk, return -1; } - if (pkcs11_get_session(slot, 1, &session)) { + if (pkcs11_session_pool_acquire(slot, 1, &session)) { pkcs11_zap_attrs(&tmpl); return -1; } @@ -1264,7 +1264,7 @@ static int pkcs11_store_key(PKCS11_SLOT_private *slot, EVP_PKEY *pk, /* Gobble the key object */ r = pkcs11_init_key(slot, session, object, type, ret_key); } - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_STORE_KEY, rv); return r; @@ -1454,7 +1454,7 @@ int pkcs11_enumerate_keys(PKCS11_SLOT_private *slot, unsigned int type, const PK if (key_template->label) pkcs11_addattr_s(&tmpl, CKA_LABEL, key_template->label); } - if (pkcs11_get_session(slot, 0, &session)) { + if (pkcs11_session_pool_acquire(slot, 0, &session)) { pkcs11_zap_attrs(&tmpl); return -1; } @@ -1462,7 +1462,7 @@ int pkcs11_enumerate_keys(PKCS11_SLOT_private *slot, unsigned int type, const PK rv = pkcs11_find_keys(slot, session, type, &tmpl); pkcs11_zap_attrs(&tmpl); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (rv < 0) { pkcs11_destroy_keys(slot, type); return -1; @@ -1484,11 +1484,11 @@ int pkcs11_remove_object(PKCS11_OBJECT_private *obj) CK_SESSION_HANDLE session; int rv; - if (pkcs11_get_session(slot, 1, &session)) + if (pkcs11_session_pool_acquire(slot, 1, &session)) return -1; rv = CRYPTOKI_call(ctx, C_DestroyObject(session, obj->object)); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_REMOVE_KEY, rv); return 0; @@ -2027,10 +2027,10 @@ static int pkcs11_try_pkey_rsa_sign(EVP_PKEY_CTX *evp_pkey_ctx, if (!slot) return -1; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); /* retrieve PSS parameters */ if (EVP_PKEY_CTX_get_rsa_padding(evp_pkey_ctx, &padding) <= 0) @@ -2102,10 +2102,10 @@ static int pkcs11_try_pkey_rsa_decrypt(EVP_PKEY_CTX *evp_pkey_ctx, if (!slot) return -1; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); switch (padding) { case RSA_PKCS1_PADDING: diff --git a/src/p11_mldsa.c b/src/p11_mldsa.c index 9a04e390..9bc87d1c 100644 --- a/src/p11_mldsa.c +++ b/src/p11_mldsa.c @@ -231,7 +231,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key, slot = key->slot; ctx = slot->ctx; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; obj = pkcs11_choose_public_source(key, session, &obj_needs_free); @@ -253,7 +253,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key, } end: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (!ok) { OPENSSL_free(*raw); diff --git a/src/p11_pkey.c b/src/p11_pkey.c index 58d4180f..b0a72b12 100644 --- a/src/p11_pkey.c +++ b/src/p11_pkey.c @@ -391,7 +391,7 @@ static int pkcs11_sign_with_mechanism(PKCS11_OBJECT_private *key, ck_siglen = (CK_ULONG)*siglen; ck_tbslen = (CK_ULONG)tbslen; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return CKR_GENERAL_ERROR; rv = CRYPTOKI_call(ctx, C_SignInit(session, mechanism, key->object)); @@ -427,7 +427,7 @@ static int pkcs11_sign_with_mechanism(PKCS11_OBJECT_private *key, *siglen = (size_t)ck_siglen; end: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return rv; } @@ -470,7 +470,7 @@ static int pkcs11_verify_with_mechanism(PKCS11_OBJECT_private *key, ck_siglen = (CK_ULONG)siglen; ck_tbslen = (CK_ULONG)tbslen; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return CKR_GENERAL_ERROR; rv = CRYPTOKI_call(ctx, @@ -492,7 +492,7 @@ static int pkcs11_verify_with_mechanism(PKCS11_OBJECT_private *key, } end: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return rv; } #endif /* OPENSSL_VERSION_NUMBER >= 0x30000000L */ @@ -534,7 +534,7 @@ static int pkcs11_decrypt_with_mechanism(PKCS11_OBJECT_private *key, ck_outlen = (CK_ULONG)*outlen; ck_inlen = (CK_ULONG)inlen; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return CKR_GENERAL_ERROR; rv = CRYPTOKI_call(ctx, C_DecryptInit(session, mechanism, key->object)); @@ -561,7 +561,7 @@ static int pkcs11_decrypt_with_mechanism(PKCS11_OBJECT_private *key, *outlen = (size_t)ck_outlen; end: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return rv; } @@ -617,7 +617,7 @@ static CK_RV pkcs11_derive_with_mechanism(PKCS11_OBJECT_private *key, pkcs11_mechanism_name(mechanism), secret, (unsigned long)*secretlen); #endif - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return CKR_GENERAL_ERROR; if (key->always_authenticate == CK_TRUE) { @@ -659,7 +659,7 @@ static CK_RV pkcs11_derive_with_mechanism(PKCS11_OBJECT_private *key, CRYPTOKI_call(ctx, C_DestroyObject(session, newkey)); OPENSSL_clear_free(value, value_len_alloc); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return rv; } @@ -729,7 +729,7 @@ static CK_RV pkcs11_decapsulate_with_mechanism( newkey_len = (CK_ULONG)len; ck_inlen = (CK_ULONG)inlen; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return CKR_GENERAL_ERROR; if (key->always_authenticate == CK_TRUE) { @@ -785,7 +785,7 @@ static CK_RV pkcs11_decapsulate_with_mechanism( CRYPTOKI_call(ctx, C_DestroyObject(session, newkey)); OPENSSL_clear_free(value, value_len_alloc); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return rv; } #endif /* OPENSSL_VERSION_NUMBER >= 0x30000000L */ @@ -1485,10 +1485,10 @@ static int pkcs11_try_pkey_ec_sign(EVP_PKEY_CTX *evp_pkey_ctx, if (!slot) return -1; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return pkcs11_evp_pkey_ec_sign(key, sig, siglen, tbs, tbslen); } @@ -1506,10 +1506,10 @@ static int pkcs11_eddsa_sign(unsigned char *sig, size_t *siglen, if (!slot) return -1; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return pkcs11_evp_pkey_eddsa_sign(key, sig, siglen, tbs, tbslen); } diff --git a/src/p11_rsa.c b/src/p11_rsa.c index ca6f5ad8..4fe3ee5a 100644 --- a/src/p11_rsa.c +++ b/src/p11_rsa.c @@ -82,10 +82,10 @@ int pkcs11_private_encrypt(int flen, if (!slot) return -1; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); siglen = pkcs11_get_key_size(key); if (pkcs11_evp_pkey_rsa_sign(key, @@ -118,10 +118,10 @@ int pkcs11_private_decrypt(int flen, if (!slot) return -1; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); /* Openssl API for RSA_private_decrypt() allows to use * RSA_PKCS1_OAEP_PADDING only with SHA_1 hash and and MGF1_SHA1 mask @@ -168,7 +168,7 @@ static RSA *pkcs11_get_rsa(PKCS11_OBJECT_private *key) RSA *rsa; BIGNUM *rsa_n = NULL, *rsa_e = NULL; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return NULL; /* Retrieve the modulus */ @@ -202,14 +202,14 @@ static RSA *pkcs11_get_rsa(PKCS11_OBJECT_private *key) goto success; failure: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); /* BN_clear_free() is NULL-safe */ BN_clear_free(rsa_n); BN_clear_free(rsa_e); return NULL; success: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); rsa = RSA_new(); if (!rsa) { BN_clear_free(rsa_n); diff --git a/src/p11_slhdsa.c b/src/p11_slhdsa.c index 4e602e40..07161098 100644 --- a/src/p11_slhdsa.c +++ b/src/p11_slhdsa.c @@ -218,7 +218,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key, slot = key->slot; ctx = slot->ctx; - if (pkcs11_get_session(slot, 0, &session)) + if (pkcs11_session_pool_acquire(slot, 0, &session)) return -1; obj = pkcs11_choose_public_source(key, session, &obj_needs_free); @@ -240,7 +240,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key, } end: - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); if (!ok) { OPENSSL_free(*raw); diff --git a/src/p11_slot.c b/src/p11_slot.c index f236826f..2050cd66 100644 --- a/src/p11_slot.c +++ b/src/p11_slot.c @@ -173,7 +173,7 @@ static int pkcs11_session_pool_switch_mode_locked( return 0; } -int pkcs11_open_session(PKCS11_SLOT_private *slot, int rw) +int pkcs11_session_pool_set_mode(PKCS11_SLOT_private *slot, int rw) { int rv; @@ -246,7 +246,7 @@ static int pkcs11_session_pool_select_locked(PKCS11_SLOT_private *slot, return PKCS11_SESSION_SELECT_ERROR; } -int pkcs11_get_session(PKCS11_SLOT_private *slot, int rw, +int pkcs11_session_pool_acquire(PKCS11_SLOT_private *slot, int rw, CK_SESSION_HANDLE *sessionp) { int select_result; @@ -278,7 +278,7 @@ int pkcs11_get_session(PKCS11_SLOT_private *slot, int rw, } } -void pkcs11_put_session(PKCS11_SLOT_private *slot, +void pkcs11_session_pool_release(PKCS11_SLOT_private *slot, CK_SESSION_HANDLE session) { PKCS11_CTX_private *ctx = slot->ctx; @@ -377,10 +377,10 @@ int pkcs11_login(PKCS11_SLOT_private *slot, int so, const char *pin) return 0; /* Nothing to do */ /* SO needs a r/w session, user can use a r/o session. */ - if (pkcs11_get_session(slot, so, &session)) + if (pkcs11_session_pool_acquire(slot, so, &session)) return -1; rv = pkcs11_login_on_session(slot, session, so, pin); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); return rv; } @@ -421,7 +421,7 @@ int pkcs11_session_pool_acquire_keygen(PKCS11_SLOT_private *slot, rv = 0; out: if (rv != 0 && session_acquired) - pkcs11_put_session(slot, *sessionp); + pkcs11_session_pool_release(slot, *sessionp); pkcs11_session_pool_end_transition(slot); return rv; } @@ -463,7 +463,7 @@ int pkcs11_logout(PKCS11_SLOT_private *slot) pthread_mutex_lock(&slot->lock); logged_in = slot->logged_in; pthread_mutex_unlock(&slot->lock); - if (pkcs11_get_session(slot, logged_in, &session) == 0) { + if (pkcs11_session_pool_acquire(slot, logged_in, &session) == 0) { session_acquired = 1; rv = CRYPTOKI_call(ctx, C_Logout(session)); if (rv == CKR_OK) { @@ -471,7 +471,7 @@ int pkcs11_logout(PKCS11_SLOT_private *slot) slot->logged_in = -1; pthread_mutex_unlock(&slot->lock); } - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); } CRYPTOKI_checkerr(CKR_F_PKCS11_LOGOUT, rv); if (!session_acquired) { @@ -530,14 +530,14 @@ int pkcs11_init_pin(PKCS11_SLOT_private *slot, const char *pin) CK_OBJECT_HANDLE session; int len, rv; - if (pkcs11_get_session(slot, 1, &session)) { + if (pkcs11_session_pool_acquire(slot, 1, &session)) { P11err(P11_F_PKCS11_INIT_PIN, P11_R_NO_SESSION); return -1; } len = pin ? (int) strlen(pin) : 0; rv = CRYPTOKI_call(ctx, C_InitPIN(session, (CK_UTF8CHAR *) pin, len)); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_INIT_PIN, rv); return 0; @@ -553,7 +553,7 @@ int pkcs11_change_pin(PKCS11_SLOT_private *slot, const char *old_pin, CK_SESSION_HANDLE session; int old_len, new_len, rv; - if (pkcs11_get_session(slot, 1, &session)) { + if (pkcs11_session_pool_acquire(slot, 1, &session)) { P11err(P11_F_PKCS11_CHANGE_PIN, P11_R_NO_SESSION); return -1; } @@ -563,7 +563,7 @@ int pkcs11_change_pin(PKCS11_SLOT_private *slot, const char *old_pin, rv = CRYPTOKI_call(ctx, C_SetPIN(session, (CK_UTF8CHAR *) old_pin, old_len, (CK_UTF8CHAR *) new_pin, new_len)); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_CHANGE_PIN, rv); return 0; @@ -579,14 +579,14 @@ int pkcs11_seed_random(PKCS11_SLOT_private *slot, const unsigned char *s, CK_SESSION_HANDLE session; int rv; - if (pkcs11_get_session(slot, 0, &session)) { + if (pkcs11_session_pool_acquire(slot, 0, &session)) { P11err(P11_F_PKCS11_SEED_RANDOM, P11_R_NO_SESSION); return -1; } rv = CRYPTOKI_call(ctx, C_SeedRandom(session, (CK_BYTE_PTR) s, s_len)); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_SEED_RANDOM, rv); return 0; @@ -602,14 +602,14 @@ int pkcs11_generate_random(PKCS11_SLOT_private *slot, unsigned char *r, CK_SESSION_HANDLE session; int rv; - if (pkcs11_get_session(slot, 0, &session)) { + if (pkcs11_session_pool_acquire(slot, 0, &session)) { P11err(P11_F_PKCS11_GENERATE_RANDOM, P11_R_NO_SESSION); return -1; } rv = CRYPTOKI_call(ctx, C_GenerateRandom(session, (CK_BYTE_PTR) r, r_len)); - pkcs11_put_session(slot, session); + pkcs11_session_pool_release(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_GENERATE_RANDOM, rv); From c991e7fd9fd6388e8c6a3740a5dd4834ec103e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Mon, 27 Jul 2026 14:59:28 +0200 Subject: [PATCH 05/10] Add session-pool concurrency regression tests Add deterministic tests around a fake PKCS#11 module for transition waiter wakeups, acquisition gating during key-generation relogin, recovery after a failed relogin, and stale transition state after fork reload. Add a bounded SoftHSM stress test that runs signing, key generation, and pool mode changes concurrently. A watchdog turns deadlocks into explicit test failures. Wire both tests into the Automake test suite and skip the threaded helpers when pthread support is unavailable. --- tests/Makefile.am | 9 +- tests/session-pool-stress.c | 263 +++++++++++++ tests/session-pool-stress.softhsm | 57 +++ tests/session-pool-test.c | 613 ++++++++++++++++++++++++++++++ tests/session-pool-under-test.c | 53 +++ 5 files changed, 993 insertions(+), 2 deletions(-) create mode 100644 tests/session-pool-stress.c create mode 100755 tests/session-pool-stress.softhsm create mode 100644 tests/session-pool-test.c create mode 100644 tests/session-pool-under-test.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 564a38fc..db820e34 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,7 +35,9 @@ check_PROGRAMS = \ ed25519-keygen-prov \ ed448-keygen-prov \ mldsa87-keygen-prov \ - mlkem768-keygen-prov + mlkem768-keygen-prov \ + session-pool-test \ + session-pool-stress dist_check_SCRIPTS = \ rsa-testpkcs11.softhsm \ rsa-testfork.softhsm \ @@ -54,6 +56,7 @@ dist_check_SCRIPTS = \ ec-copy.softhsm \ ec-keygen.softhsm \ ec-derive.softhsm \ + session-pool-stress.softhsm \ ed25519-keygen.softhsm \ ed448-keygen.softhsm \ fork-change-slot.softhsm \ @@ -107,8 +110,10 @@ rsa_pss_sign_prov_SOURCES = rsa-pss-sign-prov.c helpers_prov.c rsa_oaep_prov_SOURCES = rsa-oaep-prov.c helpers_prov.c check_all_prov_SOURCES = check-all-prov.c helpers_prov.c ec_derive_prov_SOURCES = ec-derive-prov.c helpers_prov.c +session_pool_test_SOURCES = session-pool-test.c session-pool-under-test.c +session_pool_test_LDADD = $(OPENSSL_LIBS) -TESTS = $(dist_check_SCRIPTS) +TESTS = session-pool-test$(EXEEXT) $(dist_check_SCRIPTS) TESTS_ENVIRONMENT = \ LC_ALL="C" \ diff --git a/tests/session-pool-stress.c b/tests/session-pool-stress.c new file mode 100644 index 00000000..a78151e2 --- /dev/null +++ b/tests/session-pool-stress.c @@ -0,0 +1,263 @@ +/* + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#define _POSIX_C_SOURCE 200809L +#include "config.h" +#include + +#include +#include +#include +#include + +#ifdef HAVE_PTHREAD +#include + +#define SIGN_SUCCESSES 40 +#define SIGN_ATTEMPTS 400 +#define MODE_CHANGES 40 +#define KEYGEN_ATTEMPTS 40 + +struct stress_state { + pthread_mutex_t lock; + pthread_cond_t cond; + PKCS11_SLOT *slot; + const char *pin; + int start; + int sign_successes; + int keygen_success; + int mode_success; +}; + +static void wait_for_start(struct stress_state *state) +{ + pthread_mutex_lock(&state->lock); + while (!state->start) + pthread_cond_wait(&state->cond, &state->lock); + pthread_mutex_unlock(&state->lock); +} + +static void retry_login(struct stress_state *state) +{ + (void)PKCS11_login(state->slot, 0, state->pin); + ERR_clear_error(); +} + +static void *sign_thread(void *arg) +{ + struct stress_state *state = arg; + PKCS11_KEY *keys, *key; + unsigned char digest[32] = {0}; + unsigned char signature[512]; + unsigned int signature_len, nkeys, i; + int attempts; + + wait_for_start(state); + for (attempts = 0; attempts < SIGN_ATTEMPTS && + state->sign_successes < SIGN_SUCCESSES; attempts++) { + key = NULL; + if (PKCS11_enumerate_keys(state->slot->token, + &keys, &nkeys) == 0) { + for (i = nkeys; i > 0; i--) { + if (keys[i - 1].label && + strcmp(keys[i - 1].label, + "stress-signing-key") == 0) { + key = &keys[i - 1]; + break; + } + } + } + signature_len = sizeof(signature); + if (key && PKCS11_sign(NID_sha256, digest, sizeof(digest), + signature, &signature_len, key) == 1) { + state->sign_successes++; + } else { + retry_login(state); + } + } + if (state->sign_successes != SIGN_SUCCESSES) + ERR_print_errors_fp(stderr); + return NULL; +} + +static void *keygen_thread(void *arg) +{ + struct stress_state *state = arg; + unsigned char id[] = {0x66, 0x65, 0x05}; + int attempts; + + wait_for_start(state); + for (attempts = 0; attempts < KEYGEN_ATTEMPTS; attempts++) { + if (PKCS11_generate_key(state->slot->token, EVP_PKEY_RSA, 1024, + "session-pool-stress-key", id, sizeof(id)) == 0) { + state->keygen_success = 1; + break; + } + retry_login(state); + } + return NULL; +} + +static void *mode_thread(void *arg) +{ + struct stress_state *state = arg; + struct timespec delay = {0, 2000000L}; + int i; + + wait_for_start(state); + for (i = 0; i < MODE_CHANGES; i++) { + if (PKCS11_open_session(state->slot, i & 1) != 0) + break; + retry_login(state); + nanosleep(&delay, NULL); + } + if (i == MODE_CHANGES) + state->mode_success = 1; + return NULL; +} + +static PKCS11_SLOT *find_token(PKCS11_CTX *ctx, PKCS11_SLOT *slots, + unsigned int nslots, const char *label) +{ + PKCS11_SLOT *slot; + + for (slot = PKCS11_find_token(ctx, slots, nslots); slot; + slot = PKCS11_find_next_token(ctx, slots, nslots, slot)) { + if (slot->token && slot->token->label && + strcmp(slot->token->label, label) == 0) + return slot; + } + return NULL; +} + +int main(int argc, char **argv) +{ + struct stress_state state; + PKCS11_CTX *ctx = NULL; + PKCS11_SLOT *slots = NULL, *slot; + PKCS11_KEY *keys = NULL; + pthread_t sign_id, keygen_id, mode_id; + unsigned char initial_digest[32] = {0}; + unsigned char initial_signature[512]; + unsigned int initial_signature_len = sizeof(initial_signature); + unsigned int nslots = 0, nkeys = 0, i; + int sign_created = 0, keygen_created = 0, mode_created = 0; + int result = EXIT_FAILURE; + + if (argc != 4) { + fprintf(stderr, "usage: %s module token-label pin\n", argv[0]); + return EXIT_FAILURE; + } + + ctx = PKCS11_CTX_new(); + if (!ctx || PKCS11_CTX_load(ctx, argv[1]) != 0 || + PKCS11_enumerate_slots(ctx, &slots, &nslots) != 0) { + fprintf(stderr, "could not initialize the PKCS#11 context\n"); + goto out; + } + slot = find_token(ctx, slots, nslots, argv[2]); + if (!slot || PKCS11_open_session(slot, 0) != 0 || + PKCS11_login(slot, 0, argv[3]) != 0 || + PKCS11_enumerate_keys(slot->token, &keys, &nkeys) != 0) { + fprintf(stderr, "could not initialize the test token\n"); + goto out; + } + for (i = 0; i < nkeys; i++) { + if (PKCS11_get_key_type(&keys[i]) == EVP_PKEY_RSA) + break; + } + if (i == nkeys) { + fprintf(stderr, "no RSA private key available\n"); + goto out; + } + if (PKCS11_sign(NID_sha256, initial_digest, sizeof(initial_digest), + initial_signature, &initial_signature_len, &keys[i]) != 1) { + fprintf(stderr, "initial RSA signing operation failed\n"); + ERR_print_errors_fp(stderr); + goto out; + } + + memset(&state, 0, sizeof(state)); + state.slot = slot; + state.pin = argv[3]; + pthread_mutex_init(&state.lock, NULL); + pthread_cond_init(&state.cond, NULL); + if (pthread_create(&sign_id, NULL, sign_thread, &state) != 0) + goto threads_out; + sign_created = 1; + if (pthread_create(&keygen_id, NULL, keygen_thread, &state) != 0) + goto threads_out; + keygen_created = 1; + if (pthread_create(&mode_id, NULL, mode_thread, &state) != 0) + goto threads_out; + mode_created = 1; + + pthread_mutex_lock(&state.lock); + state.start = 1; + pthread_cond_broadcast(&state.cond); + pthread_mutex_unlock(&state.lock); + pthread_join(sign_id, NULL); + sign_created = 0; + pthread_join(keygen_id, NULL); + keygen_created = 0; + pthread_join(mode_id, NULL); + mode_created = 0; + if (state.sign_successes == SIGN_SUCCESSES && + state.keygen_success && state.mode_success) { + printf("session-pool stress test passed\n"); + result = EXIT_SUCCESS; + } else { + fprintf(stderr, + "stress test incomplete: signs=%d keygen=%d modes=%d\n", + state.sign_successes, state.keygen_success, + state.mode_success); + } + +threads_out: + if (sign_created || keygen_created || mode_created) { + pthread_mutex_lock(&state.lock); + state.start = 1; + pthread_cond_broadcast(&state.cond); + pthread_mutex_unlock(&state.lock); + } + if (sign_created) + pthread_join(sign_id, NULL); + if (keygen_created) + pthread_join(keygen_id, NULL); + if (mode_created) + pthread_join(mode_id, NULL); + pthread_cond_destroy(&state.cond); + pthread_mutex_destroy(&state.lock); +out: + if (slots) + PKCS11_release_all_slots(ctx, slots, nslots); + if (ctx) { + PKCS11_CTX_unload(ctx); + PKCS11_CTX_free(ctx); + } + return result; +} + +#else /* HAVE_PTHREAD */ + +int main(void) +{ + fprintf(stderr, "Skipped: pthread support not available\n"); + return 77; +} + +#endif /* HAVE_PTHREAD */ + +/* vim: set noexpandtab: */ diff --git a/tests/session-pool-stress.softhsm b/tests/session-pool-stress.softhsm new file mode 100755 index 00000000..575737b8 --- /dev/null +++ b/tests/session-pool-stress.softhsm @@ -0,0 +1,57 @@ +#!/bin/bash + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +outdir="output.$$" +child="" +watchdog="" + +. "${srcdir}/common.sh" + +cleanup_stress_test() +{ + if [[ -n "${child}" ]]; then + kill "${child}" 2>/dev/null || true + fi + if [[ -n "${watchdog}" ]]; then + kill "${watchdog}" 2>/dev/null || true + fi + cleanup + rm -rf "${outdir}" +} +trap cleanup_stress_test EXIT + +init_db +init_card "session-pool-stress" +generate_rsa_key_pair "stress-signing-key" "session-pool-stress" + +${WRAPPER} ./session-pool-stress \ + "${MODULE}" "session-pool-stress" "${PIN}" & +child=$! +( + sleep 60 + kill -TERM "${child}" 2>/dev/null || true +) & +watchdog=$! + +wait "${child}" +rc=$? +child="" +kill "${watchdog}" 2>/dev/null || true +wait "${watchdog}" 2>/dev/null || true +watchdog="" + +if [[ ${rc} -ne 0 ]]; then + echo "Concurrent session-pool stress test failed or timed out." + exit 1 +fi + +exit 0 diff --git a/tests/session-pool-test.c b/tests/session-pool-test.c new file mode 100644 index 00000000..20d0147b --- /dev/null +++ b/tests/session-pool-test.c @@ -0,0 +1,613 @@ +/* + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#define _POSIX_C_SOURCE 200809L +#include "libp11-int.h" + +#include +#include +#include +#include + +/* p11_slot.c is compiled into this focused unit test. */ +void pkcs11_destroy_keys(PKCS11_SLOT_private *slot, unsigned int type) +{ + (void)slot; + (void)type; +} + +void pkcs11_destroy_certs(PKCS11_SLOT_private *slot) +{ + (void)slot; +} + +char *pkcs11_strdup(char *text, size_t size) +{ + (void)text; + (void)size; + return NULL; +} + +int pkcs11_atomic_add(int *value, int amount, pthread_mutex_t *lock) +{ + int result; + + pthread_mutex_lock(lock); + *value += amount; + result = *value; + pthread_mutex_unlock(lock); + return result; +} + +void ERR_CKR_error(int function, int reason, char *file, int line) +{ + (void)function; + (void)reason; + (void)file; + (void)line; +} + +void ERR_P11_error(int function, int reason, char *file, int line) +{ + (void)function; + (void)reason; + (void)file; + (void)line; +} + +#ifdef HAVE_PTHREAD + +#define TEST_TIMEOUT_SECONDS 5 + +void session_pool_test_delay_transition_unlock(PKCS11_SLOT_private *slot); + +struct fake_module_state { + pthread_mutex_t lock; + pthread_cond_t cond; + PKCS11_SLOT_private *slot; + CK_SESSION_HANDLE next_session; + unsigned int open_sessions; + unsigned int max_sessions; + int logged_in; + int pause_login; + int login_entered; + CK_RV login_result; + int close_while_in_use; + int reload_reset_missing; + int expect_reload_reset; +}; + +struct thread_state { + pthread_mutex_t lock; + pthread_cond_t cond; + PKCS11_SLOT_private *slot; + CK_SESSION_HANDLE session; + int done; + int acquired; + int delay_transition_unlock; + int rv; + int mode; +}; + +static struct fake_module_state fake; + +static CK_RV fake_open_session(CK_SLOT_ID slot_id, CK_FLAGS flags, + CK_VOID_PTR application, CK_NOTIFY notify, + CK_SESSION_HANDLE_PTR session) +{ + (void)slot_id; + (void)flags; + (void)application; + (void)notify; + + pthread_mutex_lock(&fake.lock); + if (fake.expect_reload_reset && + (fake.slot->transition_active != 0 || + fake.slot->sessions_in_use != 0 || + fake.slot->num_sessions != 0 || + fake.slot->session_head != 0 || + fake.slot->session_tail != 0)) + fake.reload_reset_missing = 1; + fake.expect_reload_reset = 0; + if (fake.open_sessions >= fake.max_sessions) { + pthread_mutex_unlock(&fake.lock); + return CKR_SESSION_COUNT; + } + *session = ++fake.next_session; + fake.open_sessions++; + pthread_mutex_unlock(&fake.lock); + return CKR_OK; +} + +static CK_RV fake_close_session(CK_SESSION_HANDLE session) +{ + (void)session; + pthread_mutex_lock(&fake.lock); + if (fake.open_sessions > 0) + fake.open_sessions--; + pthread_mutex_unlock(&fake.lock); + return CKR_OK; +} + +static CK_RV fake_close_all_sessions(CK_SLOT_ID slot_id) +{ + (void)slot_id; + pthread_mutex_lock(&fake.lock); + if (fake.slot->sessions_in_use != 0) + fake.close_while_in_use = 1; + fake.open_sessions = 0; + fake.logged_in = 0; + pthread_mutex_unlock(&fake.lock); + return CKR_OK; +} + +static CK_RV fake_get_session_info(CK_SESSION_HANDLE session, + CK_SESSION_INFO_PTR info) +{ + (void)session; + memset(info, 0, sizeof(*info)); + return CKR_OK; +} + +static CK_RV fake_login(CK_SESSION_HANDLE session, CK_USER_TYPE user_type, + CK_UTF8CHAR_PTR pin, CK_ULONG pin_len) +{ + CK_RV result; + + (void)session; + (void)user_type; + (void)pin; + (void)pin_len; + + pthread_mutex_lock(&fake.lock); + fake.login_entered = 1; + pthread_cond_broadcast(&fake.cond); + while (fake.pause_login) + pthread_cond_wait(&fake.cond, &fake.lock); + result = fake.login_result; + if (result == CKR_OK) + fake.logged_in = 1; + pthread_mutex_unlock(&fake.lock); + return result; +} + +static void deadline_after(struct timespec *deadline, long milliseconds) +{ + clock_gettime(CLOCK_REALTIME, deadline); + deadline->tv_sec += milliseconds / 1000; + deadline->tv_nsec += (milliseconds % 1000) * 1000000L; + if (deadline->tv_nsec >= 1000000000L) { + deadline->tv_sec++; + deadline->tv_nsec -= 1000000000L; + } +} + +static int wait_thread_done(struct thread_state *state, long milliseconds) +{ + struct timespec deadline; + int done; + + deadline_after(&deadline, milliseconds); + pthread_mutex_lock(&state->lock); + while (!state->done) { + if (pthread_cond_timedwait(&state->cond, &state->lock, + &deadline) != 0) + break; + } + done = state->done; + pthread_mutex_unlock(&state->lock); + return done; +} + +static int thread_done(struct thread_state *state) +{ + int done; + + pthread_mutex_lock(&state->lock); + done = state->done; + pthread_mutex_unlock(&state->lock); + return done; +} + +static int wait_login_entered(long milliseconds) +{ + struct timespec deadline; + int entered; + + deadline_after(&deadline, milliseconds); + pthread_mutex_lock(&fake.lock); + while (!fake.login_entered) { + if (pthread_cond_timedwait(&fake.cond, &fake.lock, + &deadline) != 0) + break; + } + entered = fake.login_entered; + pthread_mutex_unlock(&fake.lock); + return entered; +} + +static void thread_state_init(struct thread_state *state, + PKCS11_SLOT_private *slot) +{ + memset(state, 0, sizeof(*state)); + state->slot = slot; + pthread_mutex_init(&state->lock, NULL); + pthread_cond_init(&state->cond, NULL); +} + +static void thread_state_destroy(struct thread_state *state) +{ + pthread_cond_destroy(&state->cond); + pthread_mutex_destroy(&state->lock); +} + +static void thread_complete(struct thread_state *state, int rv, int acquired) +{ + pthread_mutex_lock(&state->lock); + state->rv = rv; + state->acquired = acquired; + state->done = 1; + pthread_cond_broadcast(&state->cond); + pthread_mutex_unlock(&state->lock); +} + +static void *keygen_thread(void *arg) +{ + struct thread_state *state = arg; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + int rv; + + if (state->delay_transition_unlock) + session_pool_test_delay_transition_unlock(state->slot); + rv = pkcs11_session_pool_acquire_keygen(state->slot, &session); + if (rv == 0) + pkcs11_session_pool_release(state->slot, session); + thread_complete(state, rv, rv == 0); + return NULL; +} + +static void *acquire_thread(void *arg) +{ + struct thread_state *state = arg; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + int rv; + + rv = pkcs11_session_pool_acquire(state->slot, 0, &session); + if (rv == 0) + pkcs11_session_pool_release(state->slot, session); + thread_complete(state, rv, rv == 0); + return NULL; +} + +static void *mode_thread(void *arg) +{ + struct thread_state *state = arg; + + thread_complete(state, + pkcs11_session_pool_set_mode(state->slot, state->mode), 0); + return NULL; +} + +static void fake_init(PKCS11_SLOT_private *slot, unsigned int max_sessions) +{ + memset(&fake, 0, sizeof(fake)); + fake.slot = slot; + fake.max_sessions = max_sessions; + pthread_mutex_init(&fake.lock, NULL); + pthread_cond_init(&fake.cond, NULL); +} + +static void fake_destroy(void) +{ + pthread_cond_destroy(&fake.cond); + pthread_mutex_destroy(&fake.lock); +} + +static void slot_init(PKCS11_SLOT_private *slot, PKCS11_CTX_private *ctx, + CK_FUNCTION_LIST_PTR method, CK_SESSION_HANDLE *pool, + unsigned int max_sessions) +{ + memset(ctx, 0, sizeof(*ctx)); + ctx->method = method; + memset(slot, 0, sizeof(*slot)); + slot->ctx = ctx; + slot->rw_mode = -1; + slot->logged_in = -1; + slot->session_pool = pool; + slot->session_poolsize = max_sessions + 1; + slot->max_sessions = max_sessions; + pthread_mutex_init(&slot->lock, NULL); + pthread_cond_init(&slot->cond, NULL); +} + +static void slot_destroy(PKCS11_SLOT_private *slot) +{ + if (slot->prev_pin) { + OPENSSL_cleanse(slot->prev_pin, strlen(slot->prev_pin)); + OPENSSL_free(slot->prev_pin); + } + pthread_cond_destroy(&slot->cond); + pthread_mutex_destroy(&slot->lock); +} + +static int wait_for_transition(PKCS11_SLOT_private *slot, long milliseconds) +{ + struct timespec delay = {0, 1000000L}; + long elapsed; + int active; + + for (elapsed = 0; elapsed < milliseconds; elapsed++) { + pthread_mutex_lock(&slot->lock); + active = slot->transition_active; + pthread_mutex_unlock(&slot->lock); + if (active) + return 1; + nanosleep(&delay, NULL); + } + return 0; +} + +static int transition_waiter_test(CK_FUNCTION_LIST_PTR method) +{ + PKCS11_CTX_private ctx; + PKCS11_SLOT_private slot; + CK_SESSION_HANDLE pool[2], held; + struct thread_state keygen, mode; + pthread_t keygen_id, mode_id; + struct timespec delay = {0, 200000000L}; + int failed = 0; + + slot_init(&slot, &ctx, method, pool, 1); + fake_init(&slot, 1); + slot.rw_mode = 1; + if (pkcs11_session_pool_acquire(&slot, 1, &held) != 0) { + fprintf(stderr, "could not acquire the initial session\n"); + failed = 1; + goto out; + } + + thread_state_init(&keygen, &slot); + thread_state_init(&mode, &slot); + keygen.delay_transition_unlock = 1; + mode.mode = 0; + pthread_create(&keygen_id, NULL, keygen_thread, &keygen); + if (!wait_for_transition(&slot, 1000)) { + fprintf(stderr, "key-generation transition did not start\n"); + failed = 1; + goto threads_out; + } + pthread_create(&mode_id, NULL, mode_thread, &mode); + /* Give the second transition time to wait on the shared condition. */ + nanosleep(&delay, NULL); + pkcs11_session_pool_release(&slot, held); + + if (!wait_thread_done(&keygen, TEST_TIMEOUT_SECONDS * 1000) || + !wait_thread_done(&mode, TEST_TIMEOUT_SECONDS * 1000)) { + fprintf(stderr, "session-pool transition waiters deadlocked\n"); + failed = 1; + goto threads_out; + } + pthread_join(keygen_id, NULL); + pthread_join(mode_id, NULL); + if (keygen.rv != 0 || mode.rv != 0 || fake.close_while_in_use) { + fprintf(stderr, "session-pool transition waiter test failed\n"); + failed = 1; + } + +threads_out: + if (!thread_done(&keygen) || !thread_done(&mode)) + return 1; + thread_state_destroy(&keygen); + thread_state_destroy(&mode); +out: + fake_destroy(); + slot_destroy(&slot); + return failed; +} + +static int relogin_gate_test(CK_FUNCTION_LIST_PTR method) +{ + PKCS11_CTX_private ctx; + PKCS11_SLOT_private slot; + CK_SESSION_HANDLE pool[3]; + struct thread_state keygen, acquire; + pthread_t keygen_id, acquire_id; + unsigned int open_sessions; + int failed = 0; + + slot_init(&slot, &ctx, method, pool, 2); + fake_init(&slot, 2); + slot.rw_mode = 0; + if (pkcs11_login(&slot, 0, "1234") != 0) { + fprintf(stderr, "could not establish the initial login\n"); + failed = 1; + goto out; + } + + thread_state_init(&keygen, &slot); + thread_state_init(&acquire, &slot); + pthread_mutex_lock(&fake.lock); + fake.login_entered = 0; + fake.pause_login = 1; + pthread_mutex_unlock(&fake.lock); + pthread_create(&keygen_id, NULL, keygen_thread, &keygen); + if (!wait_login_entered(1000)) { + fprintf(stderr, "key-generation relogin did not start\n"); + failed = 1; + goto threads_out; + } + + pthread_create(&acquire_id, NULL, acquire_thread, &acquire); + if (wait_thread_done(&acquire, 200)) { + fprintf(stderr, "normal acquisition passed the relogin gate\n"); + failed = 1; + } + pthread_mutex_lock(&fake.lock); + open_sessions = fake.open_sessions; + fake.pause_login = 0; + pthread_cond_broadcast(&fake.cond); + pthread_mutex_unlock(&fake.lock); + if (open_sessions != 1) { + fprintf(stderr, "normal acquisition reached the module during relogin\n"); + failed = 1; + } + + if (!wait_thread_done(&keygen, TEST_TIMEOUT_SECONDS * 1000) || + !wait_thread_done(&acquire, TEST_TIMEOUT_SECONDS * 1000)) { + fprintf(stderr, "relogin gate test deadlocked\n"); + return 1; + } + pthread_join(keygen_id, NULL); + pthread_join(acquire_id, NULL); + if (keygen.rv != 0 || acquire.rv != 0 || + fake.close_while_in_use) { + fprintf(stderr, "relogin gate operations failed\n"); + failed = 1; + } + +threads_out: + if (!thread_done(&keygen) || !thread_done(&acquire)) + return 1; + thread_state_destroy(&keygen); + thread_state_destroy(&acquire); +out: + fake_destroy(); + slot_destroy(&slot); + return failed; +} + +static int transition_error_test(CK_FUNCTION_LIST_PTR method) +{ + PKCS11_CTX_private ctx; + PKCS11_SLOT_private slot; + CK_SESSION_HANDLE pool[3], session; + int failed = 0; + + slot_init(&slot, &ctx, method, pool, 2); + fake_init(&slot, 2); + slot.rw_mode = 0; + if (pkcs11_login(&slot, 0, "1234") != 0) { + fprintf(stderr, "could not establish login for error test\n"); + failed = 1; + goto out; + } + pthread_mutex_lock(&fake.lock); + fake.login_result = CKR_PIN_INCORRECT; + pthread_mutex_unlock(&fake.lock); + if (pkcs11_session_pool_acquire_keygen(&slot, &session) == 0) { + fprintf(stderr, "key-generation relogin unexpectedly succeeded\n"); + pkcs11_session_pool_release(&slot, session); + failed = 1; + } + pthread_mutex_lock(&slot.lock); + if (slot.transition_active != 0 || slot.sessions_in_use != 0) + failed = 1; + pthread_mutex_unlock(&slot.lock); + if (failed) + fprintf(stderr, "failed transition did not restore pool state\n"); + + pthread_mutex_lock(&fake.lock); + fake.login_result = CKR_OK; + pthread_mutex_unlock(&fake.lock); + if (pkcs11_session_pool_acquire(&slot, 1, &session) != 0) { + fprintf(stderr, "pool did not recover after failed relogin\n"); + failed = 1; + } else { + pkcs11_session_pool_release(&slot, session); + } + +out: + fake_destroy(); + slot_destroy(&slot); + return failed; +} + +static int fork_reload_test(CK_FUNCTION_LIST_PTR method) +{ + PKCS11_CTX_private ctx; + PKCS11_SLOT_private slot; + CK_SESSION_HANDLE pool[3]; + int failed = 0; + + slot_init(&slot, &ctx, method, pool, 2); + fake_init(&slot, 2); + slot.rw_mode = 0; + if (pkcs11_login(&slot, 0, "1234") != 0) { + fprintf(stderr, "could not establish login before reload\n"); + failed = 1; + goto out; + } + + /* Model inherited parent state. No parent thread or lease survives. */ + slot.transition_active = 1; + slot.sessions_in_use = 1; + slot.num_sessions = 2; + slot.session_head = 1; + slot.session_tail = 2; + pthread_mutex_lock(&fake.lock); + fake.expect_reload_reset = 1; + pthread_mutex_unlock(&fake.lock); + if (pkcs11_reload_slot(&slot) != 0) { + fprintf(stderr, "slot reload failed\n"); + failed = 1; + } + if (slot.transition_active != 0 || slot.sessions_in_use != 0 || + fake.reload_reset_missing) { + fprintf(stderr, "slot reload inherited stale transition state\n"); + failed = 1; + } + +out: + fake_destroy(); + slot_destroy(&slot); + return failed; +} + +int main(void) +{ + CK_FUNCTION_LIST method; + int failed = 0; + + memset(&method, 0, sizeof(method)); + method.C_OpenSession = fake_open_session; + method.C_CloseSession = fake_close_session; + method.C_CloseAllSessions = fake_close_all_sessions; + method.C_GetSessionInfo = fake_get_session_info; + method.C_Login = fake_login; + + failed = transition_waiter_test(&method); + if (!failed) + failed = relogin_gate_test(&method); + if (!failed) + failed = transition_error_test(&method); + if (!failed) + failed = fork_reload_test(&method); + if (failed) + return EXIT_FAILURE; + printf("session-pool concurrency tests passed\n"); + return EXIT_SUCCESS; +} + +#else /* HAVE_PTHREAD */ + +int main(void) +{ + fprintf(stderr, "Skipped: pthread support not available\n"); + return 77; +} + +#endif /* HAVE_PTHREAD */ + +/* vim: set noexpandtab: */ diff --git a/tests/session-pool-under-test.c b/tests/session-pool-under-test.c new file mode 100644 index 00000000..7d9f70d9 --- /dev/null +++ b/tests/session-pool-under-test.c @@ -0,0 +1,53 @@ +/* + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Build the session-pool implementation with a controllable PKCS#11 method + * table for the focused concurrency test. + */ + +#define _POSIX_C_SOURCE 200809L +#include "libp11-int.h" +#include + +#ifdef HAVE_PTHREAD +static PKCS11_SLOT_private *delayed_slot; +static pthread_t delayed_thread; +static int delay_armed; + +/* Force the transition-owner scheduling window used by the mixed-wakeup + * regression: another transition queues before the owner selects a session. */ +void session_pool_test_delay_transition_unlock(PKCS11_SLOT_private *slot) +{ + delayed_slot = slot; + delayed_thread = pthread_self(); + delay_armed = 1; +} + +static int session_pool_test_mutex_unlock(pthread_mutex_t *mutex) +{ + struct timespec delay = {0, 100000000L}; + int delay_this_unlock, rv; + + delay_this_unlock = delay_armed && delayed_slot && + mutex == &delayed_slot->lock && delayed_slot->transition_active && + pthread_equal(delayed_thread, pthread_self()); + rv = pthread_mutex_unlock(mutex); + if (delay_this_unlock) { + delay_armed = 0; + nanosleep(&delay, NULL); + } + return rv; +} + +#define pthread_mutex_unlock session_pool_test_mutex_unlock +#else /* HAVE_PTHREAD */ +void session_pool_test_delay_transition_unlock(PKCS11_SLOT_private *slot) +{ + (void)slot; +} +#endif /* HAVE_PTHREAD */ + +#include "../src/p11_slot.c" From 47a80f7a9f94d1e6bf6d0248f25519cc708ccb42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Mon, 27 Jul 2026 14:59:45 +0200 Subject: [PATCH 06/10] Fix key-cache use-after-free in EVP_PKEY ex-data Store a referenced PKCS11_OBJECT_private directly in EVP_PKEY ex-data instead of following a PKCS11_KEY back-pointer. The back-pointer becomes stale when key enumeration reallocates the cached key array, causing concurrent signing and key generation to crash. Remove the now-obsolete back-pointer from the private object structure. --- src/libp11-int.h | 1 - src/p11_key.c | 23 +++++++++++------------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index 288f5204..0b7ab81b 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -126,7 +126,6 @@ struct pkcs11_object_private { unsigned int forkid; int refcnt; pthread_mutex_t lock; - PKCS11_KEY *public; /* our current public object */ }; struct pkcs11_object_ops { diff --git a/src/p11_key.c b/src/p11_key.c index 350ccc32..b72a4b40 100644 --- a/src/p11_key.c +++ b/src/p11_key.c @@ -160,7 +160,8 @@ static int EVP_PKEY_is_a(const EVP_PKEY *pkey, const char *name); #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */ #if OPENSSL_VERSION_NUMBER >= 0x30000000L -static void pkcs11_set_ex_data_evp_pkey(EVP_PKEY *pkey, PKCS11_KEY *key); +static void pkcs11_set_ex_data_evp_pkey(EVP_PKEY *pkey, + PKCS11_OBJECT_private *obj); static PKCS11_OBJECT_private *pkcs11_get_ex_data_evp_pkey(const EVP_PKEY *pkey); static void alloc_evp_pkey_ex_index(void); #endif /* OPENSSL_VERSION_NUMBER >= 0x30000000L */ @@ -1363,7 +1364,7 @@ EVP_PKEY *pkcs11_get_key(PKCS11_OBJECT_private *key0, CK_OBJECT_CLASS object_cla /* Store the backing PKCS#11 object in EVP_PKEY ex_data. Public key * ex_data is needed as a workaround for FALCON token-side verify. */ alloc_evp_pkey_ex_index(); - pkcs11_set_ex_data_evp_pkey(ret, key->public); + pkcs11_set_ex_data_evp_pkey(ret, key); #endif /* OPENSSL_VERSION_NUMBER >= 0x30000000L */ err: if (key != key0) @@ -1582,9 +1583,6 @@ static int pkcs11_init_key(PKCS11_SLOT_private *slot, CK_SESSION_HANDLE session, key->label = kpriv->label; key->isPrivate = (type == CKO_PRIVATE_KEY); - /* Link back */ - kpriv->public = key; - if (ret) *ret = key; return 0; @@ -1883,19 +1881,20 @@ void pkcs11_destroy_keys(PKCS11_SLOT_private *slot, unsigned int type) } #if OPENSSL_VERSION_NUMBER >= 0x30000000L -static void pkcs11_set_ex_data_evp_pkey(EVP_PKEY *pkey, PKCS11_KEY *key) +static void pkcs11_set_ex_data_evp_pkey(EVP_PKEY *pkey, + PKCS11_OBJECT_private *obj) { - PKCS11_OBJECT_private *obj; + PKCS11_OBJECT_private *obj_ref; - if (pkey == NULL || key == NULL || key->_private == NULL) + if (pkey == NULL || obj == NULL) return; - obj = pkcs11_object_ref(key->_private); - if (obj == NULL) + obj_ref = pkcs11_object_ref(obj); + if (obj_ref == NULL) return; - if (!EVP_PKEY_set_ex_data(pkey, evp_pkey_ex_index, obj)) - pkcs11_object_free(obj); + if (!EVP_PKEY_set_ex_data(pkey, evp_pkey_ex_index, obj_ref)) + pkcs11_object_free(obj_ref); } static int pkcs11_dup_ex_data_evp_pkey(CRYPTO_EX_DATA *to, From 2cc8326a023f2eb96a39586a7c5d8f93d17b948e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Mon, 27 Jul 2026 16:08:15 +0200 Subject: [PATCH 07/10] Fix cached EdDSA and XDH key object leaks The EVP_PKEY cached on a private EdDSA or XDH object only borrows its legacy ex-data pointer. Returned EVP_PKEY objects acquire their owning object reference through the callback-backed generic ex-data installed by pkcs11_get_key(). Taking another reference while constructing each cached key therefore had no matching release. It kept the object, slot, and associated PIN data alive after ENGINE teardown. Remove those redundant references and document the ownership split. --- src/p11_eddsa.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/p11_eddsa.c b/src/p11_eddsa.c index a31ebe5b..2a65396b 100644 --- a/src/p11_eddsa.c +++ b/src/p11_eddsa.c @@ -813,8 +813,8 @@ static EVP_PKEY *pkcs11_get_evp_key_ed25519(PKCS11_OBJECT_private *key) EVP_PKEY_free(pkey); return NULL; } - /* creates a new EVP_PKEY object which requires its own key object reference */ - key = pkcs11_object_ref(key); + /* The cached EVP_PKEY borrows the object. Returned keys get + * an owning reference in pkcs11_get_key(). */ alloc_pkey_ex_index(); pkcs11_set_ex_data_pkey(pkey, key); atexit(pkcs11_ed25519_method_free); @@ -849,8 +849,8 @@ static EVP_PKEY *pkcs11_get_evp_key_ed448(PKCS11_OBJECT_private *key) EVP_PKEY_free(pkey); return NULL; } - /* create a new EVP_PKEY object which requires its own key object reference */ - key = pkcs11_object_ref(key); + /* The cached EVP_PKEY borrows the object. Returned keys get + * an owning reference in pkcs11_get_key(). */ alloc_pkey_ex_index(); pkcs11_set_ex_data_pkey(pkey, key); atexit(pkcs11_ed25519_method_free); @@ -885,8 +885,8 @@ static EVP_PKEY *pkcs11_get_evp_key_x25519(PKCS11_OBJECT_private *key) EVP_PKEY_free(pkey); return NULL; } - /* creates a new EVP_PKEY object which requires its own key object reference */ - key = pkcs11_object_ref(key); + /* The cached EVP_PKEY borrows the object. Returned keys get + * an owning reference in pkcs11_get_key(). */ alloc_pkey_ex_index(); pkcs11_set_ex_data_pkey(pkey, key); atexit(pkcs11_x25519_method_free); @@ -921,8 +921,8 @@ static EVP_PKEY *pkcs11_get_evp_key_x448(PKCS11_OBJECT_private *key) EVP_PKEY_free(pkey); return NULL; } - /* create a new EVP_PKEY object which requires its own key object reference */ - key = pkcs11_object_ref(key); + /* The cached EVP_PKEY borrows the object. Returned keys get + * an owning reference in pkcs11_get_key(). */ alloc_pkey_ex_index(); pkcs11_set_ex_data_pkey(pkey, key); atexit(pkcs11_x448_method_free); From 1d761b628076d11de370e14f266105e84fe1dba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Mon, 27 Jul 2026 16:08:36 +0200 Subject: [PATCH 08/10] Register the correct Ed448 method cleanup The Ed448 key path registered pkcs11_ed25519_method_free() with atexit(), leaving the Ed448 EVP_PKEY method allocated and invoking cleanup for the wrong method family. Pair pkcs11_ed448_method_new() with pkcs11_ed448_method_free(). --- src/p11_eddsa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/p11_eddsa.c b/src/p11_eddsa.c index 2a65396b..f89c81c5 100644 --- a/src/p11_eddsa.c +++ b/src/p11_eddsa.c @@ -853,7 +853,7 @@ static EVP_PKEY *pkcs11_get_evp_key_ed448(PKCS11_OBJECT_private *key) * an owning reference in pkcs11_get_key(). */ alloc_pkey_ex_index(); pkcs11_set_ex_data_pkey(pkey, key); - atexit(pkcs11_ed25519_method_free); + atexit(pkcs11_ed448_method_free); } } #endif /* OPENSSL_VERSION_NUMBER < 0x40000000L */ From dfef24b018dfb5552ef6fccf89e2c26fc4b8b696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Mon, 27 Jul 2026 16:08:42 +0200 Subject: [PATCH 09/10] Keep ENGINE references alive until test keys are freed PKCS#11-backed EVP_PKEY objects retain engine-managed key and slot state. Several ENGINE tests called ENGINE_finish() before freeing those keys, so teardown could no longer release the object graph. Leak checks consequently reported the objects, slots, and PIN data. Free every EVP_PKEY before dropping the functional ENGINE reference. Centralize cleanup in the check/copy tests and track whether ENGINE_init() succeeded, so error paths release either the functional or structural reference as appropriate. --- tests/check-privkey.c | 21 ++++++++++++--------- tests/dup-key.c | 17 +++++++++++------ tests/ec-keygen.c | 8 ++++++-- tests/ed25519-keygen.c | 8 ++++++-- tests/ed448-keygen.c | 8 ++++++-- tests/evp-sign.c | 5 ++--- tests/rsa-keygen.c | 8 ++++++-- 7 files changed, 49 insertions(+), 26 deletions(-) diff --git a/tests/check-privkey.c b/tests/check-privkey.c index 0f09bf7c..d2ef18d4 100644 --- a/tests/check-privkey.c +++ b/tests/check-privkey.c @@ -64,7 +64,7 @@ int main(int argc, char *argv[]) const char *module, *efile, *certfile, *privkey; - int ret = 0; + int engine_initialized = 0, ret = 0; struct { const char *cert_id; @@ -113,12 +113,14 @@ int main(int argc, char *argv[]) if (!ENGINE_ctrl_cmd_string(engine, "DEBUG_LEVEL", "7", 0)) { display_openssl_errors(__LINE__); - exit(1); + ret = 1; + goto end; } if (!ENGINE_ctrl_cmd_string(engine, "MODULE_PATH", module, 0)) { display_openssl_errors(__LINE__); - exit(1); + ret = 1; + goto end; } if (!ENGINE_init(engine)) { @@ -127,6 +129,7 @@ int main(int argc, char *argv[]) ret = 1; goto end; } + engine_initialized = 1; /* * ENGINE_init() returned a functional reference, so free the structural * reference from ENGINE_by_id(). @@ -168,24 +171,24 @@ int main(int argc, char *argv[]) goto end; } - /* Free the functional reference from ENGINE_init */ - ENGINE_finish(engine); - if (!X509_check_private_key(cert, pkey)) { printf("Could not check private key\n"); display_openssl_errors(__LINE__); - EVP_PKEY_free(pkey); ret = 1; goto end; } - EVP_PKEY_free(pkey); printf("Key and certificate matched\n"); ret = 0; - CONF_modules_unload(1); end: + EVP_PKEY_free(pkey); + if (engine_initialized) + ENGINE_finish(engine); + else if (engine) + ENGINE_free(engine); X509_free(cert); + CONF_modules_unload(1); return ret; } diff --git a/tests/dup-key.c b/tests/dup-key.c index 809acd0d..89a53413 100644 --- a/tests/dup-key.c +++ b/tests/dup-key.c @@ -66,7 +66,7 @@ int main(int argc, char *argv[]) const char *module, *efile, *privkey; - int ret = 0; + int engine_initialized = 0, ret = 0; if (argc < 3){ printf("Too few arguments\n"); @@ -109,12 +109,14 @@ int main(int argc, char *argv[]) if (!ENGINE_ctrl_cmd_string(engine, "DEBUG_LEVEL", "7", 0)) { display_openssl_errors(__LINE__); - exit(1); + ret = 1; + goto end; } if (!ENGINE_ctrl_cmd_string(engine, "MODULE_PATH", module, 0)) { display_openssl_errors(__LINE__); - exit(1); + ret = 1; + goto end; } if (!ENGINE_init(engine)) { @@ -123,6 +125,7 @@ int main(int argc, char *argv[]) ret = 1; goto end; } + engine_initialized = 1; /* * ENGINE_init() returned a functional reference, so free the structural @@ -169,8 +172,6 @@ int main(int argc, char *argv[]) /* Do it one more time */ pkey = ENGINE_load_private_key(engine, privkey, 0, 0); - /* Free the functional reference from ENGINE_init */ - ENGINE_finish(engine); if (pkey == NULL) { printf("Could not load key\n"); display_openssl_errors(__LINE__); @@ -180,9 +181,13 @@ int main(int argc, char *argv[]) ret = 0; - CONF_modules_unload(1); end: EVP_PKEY_free(pkey); + if (engine_initialized) + ENGINE_finish(engine); + else if (engine) + ENGINE_free(engine); + CONF_modules_unload(1); return ret; } diff --git a/tests/ec-keygen.c b/tests/ec-keygen.c index a6611f05..91099ec4 100644 --- a/tests/ec-keygen.c +++ b/tests/ec-keygen.c @@ -116,7 +116,7 @@ static int sign_verify_test(EVP_PKEY *priv, EVP_PKEY *pub) { int main(int argc, char* argv[]) { - int ret = EXIT_FAILURE, res; + int engine_initialized = 0, ret = EXIT_FAILURE, res; ENGINE* engine = NULL; const char *efile, *module; char *key_pass; @@ -196,6 +196,7 @@ int main(int argc, char* argv[]) display_openssl_errors(__LINE__); goto cleanup; } + engine_initialized = 1; /* * ENGINE_init() returned a functional reference, so free the structural * reference from ENGINE_by_id(). @@ -221,9 +222,12 @@ int main(int argc, char* argv[]) ret = 0; cleanup: - ENGINE_finish(engine); EVP_PKEY_free(ecpb); EVP_PKEY_free(ecpr); + if (engine_initialized) + ENGINE_finish(engine); + else if (engine) + ENGINE_free(engine); return ret; } diff --git a/tests/ed25519-keygen.c b/tests/ed25519-keygen.c index 3819e021..6b718e9d 100644 --- a/tests/ed25519-keygen.c +++ b/tests/ed25519-keygen.c @@ -45,7 +45,7 @@ void display_openssl_errors(void) int main(int argc, char *argv[]) { ENGINE *engine = NULL; - int ret = EXIT_FAILURE; + int engine_initialized = 0, ret = EXIT_FAILURE; EVP_PKEY *private_key = NULL, *public_key = NULL; PKCS11_EDDSA_KGEN eddsa = { .nid = NID_ED25519 @@ -109,6 +109,7 @@ int main(int argc, char *argv[]) display_openssl_errors(); goto cleanup; } + engine_initialized = 1; /* * ENGINE_init() returned a functional reference, so free the structural * reference from ENGINE_by_id(). @@ -155,9 +156,12 @@ int main(int argc, char *argv[]) ret = 0; cleanup: - ENGINE_finish(engine); EVP_PKEY_free(private_key); EVP_PKEY_free(public_key); + if (engine_initialized) + ENGINE_finish(engine); + else if (engine) + ENGINE_free(engine); printf("\n"); return ret; } diff --git a/tests/ed448-keygen.c b/tests/ed448-keygen.c index 30ab3f98..33a87a5f 100644 --- a/tests/ed448-keygen.c +++ b/tests/ed448-keygen.c @@ -45,7 +45,7 @@ void display_openssl_errors(void) int main(int argc, char *argv[]) { ENGINE *engine = NULL; - int ret = EXIT_FAILURE; + int engine_initialized = 0, ret = EXIT_FAILURE; EVP_PKEY *private_key = NULL, *public_key = NULL; PKCS11_EDDSA_KGEN eddsa = { .nid = NID_ED448 @@ -109,6 +109,7 @@ int main(int argc, char *argv[]) display_openssl_errors(); goto cleanup; } + engine_initialized = 1; /* * ENGINE_init() returned a functional reference, so free the structural * reference from ENGINE_by_id(). @@ -155,9 +156,12 @@ int main(int argc, char *argv[]) ret = 0; cleanup: - ENGINE_finish(engine); EVP_PKEY_free(private_key); EVP_PKEY_free(public_key); + if (engine_initialized) + ENGINE_finish(engine); + else if (engine) + ENGINE_free(engine); printf("\n"); return ret; } diff --git a/tests/evp-sign.c b/tests/evp-sign.c index 226200bc..a29db73f 100644 --- a/tests/evp-sign.c +++ b/tests/evp-sign.c @@ -260,9 +260,6 @@ int main(int argc, char **argv) exit(1); } - /* Free the functional reference from ENGINE_init */ - ENGINE_finish(e); - digest_algo = EVP_get_digestbyname("sha256"); ctx = EVP_MD_CTX_create(); @@ -314,6 +311,8 @@ int main(int argc, char **argv) printf("Signature verified\n"); + /* Free the functional reference from ENGINE_init */ + ENGINE_finish(e); CONF_modules_unload(1); UI_destroy_method(ui_detect_failed_ctrl); UI_destroy_method(ui_console_with_default); diff --git a/tests/rsa-keygen.c b/tests/rsa-keygen.c index d51cfd89..21bfec6b 100644 --- a/tests/rsa-keygen.c +++ b/tests/rsa-keygen.c @@ -116,7 +116,7 @@ static int sign_verify_test(EVP_PKEY *priv, EVP_PKEY *pub) { int main(int argc, char* argv[]) { - int ret = EXIT_FAILURE, res; + int engine_initialized = 0, ret = EXIT_FAILURE, res; ENGINE* engine = NULL; const char *efile, *module; char *key_pass; @@ -196,6 +196,7 @@ int main(int argc, char* argv[]) display_openssl_errors(__LINE__); goto cleanup; } + engine_initialized = 1; /* * ENGINE_init() returned a functional reference, so free the structural * reference from ENGINE_by_id(). @@ -221,9 +222,12 @@ int main(int argc, char* argv[]) ret = 0; cleanup: - ENGINE_finish(engine); EVP_PKEY_free(rsapb); EVP_PKEY_free(rsapr); + if (engine_initialized) + ENGINE_finish(engine); + else if (engine) + ENGINE_free(engine); return ret; } From 09e064b9eaf47c85cdccffa6f5ff6359c64a5fa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Trojnara?= Date: Mon, 27 Jul 2026 16:11:47 +0200 Subject: [PATCH 10/10] Keep provider EC group name alive while building params OSSL_PARAM_BLD_push_utf8_string() retains the supplied pointer until OSSL_PARAM_BLD_to_param() materializes the parameter array. The EC group-name buffer was scoped to the switch case, so its lifetime ended before the common conversion after the switch. ASan consequently reported a stack-use-after-scope. Move the buffer to function scope so it remains valid through parameter construction. --- src/provider_helpers.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/provider_helpers.c b/src/provider_helpers.c index 41b5a87a..a469912a 100644 --- a/src/provider_helpers.c +++ b/src/provider_helpers.c @@ -2400,6 +2400,10 @@ static OSSL_PARAM *public_params_from_evp_pkey(EVP_PKEY *pkey) OSSL_PARAM_BLD *bld = NULL; BIGNUM *n = NULL, *e = NULL; unsigned char *pub = NULL; +#ifndef OPENSSL_NO_EC + /* OSSL_PARAM_BLD borrows strings until OSSL_PARAM_BLD_to_param(). */ + char group[128]; +#endif int nid; if (pkey == NULL) @@ -2426,7 +2430,6 @@ static OSSL_PARAM *public_params_from_evp_pkey(EVP_PKEY *pkey) #ifndef OPENSSL_NO_EC case EVP_PKEY_EC: { - char group[128]; size_t grouplen = 0; size_t publen = 0;