From a6e480d7f7edd31b52b37945b98edc30874a29d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sat, 20 Mar 2021 08:37:32 +0200 Subject: [PATCH 01/12] Simplify pkcs11_try_pkey_rsa_sign() to not hold lock over calls The long term lock keeping was originally added in commit e81e3355 "NULL sig support #178" to support querying the size of the signature with sig=NULL. However, this commit was immediately followed up by 7a1fca41 "EVP_PKEY_FLAG_AUTOARGLEN for EVP_PKEY_meth_new()" which refers to same issue too. The EVP_PKEY_FLAG_AUTOARGLEN makes OpenSSL core handle sig=NULL case before calling the algorithm specific sign function. Thus we never get the sig=NULL call in the current code. Thus the original hack is unneeded. This effectively reverts e81e3355 and adds an error handling if sig=NULL would happen. --- src/p11_pkey.c | 63 +++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/p11_pkey.c b/src/p11_pkey.c index 50eef56..a92874f 100644 --- a/src/p11_pkey.c +++ b/src/p11_pkey.c @@ -298,7 +298,7 @@ static int pkcs11_try_pkey_rsa_sign(EVP_PKEY_CTX *evp_pkey_ctx, EVP_PKEY *pkey; RSA *rsa; PKCS11_KEY *key; - int rv = 0; + int rv = 0, padding; CK_ULONG size = *siglen; PKCS11_SLOT *slot; PKCS11_CTX *ctx; @@ -306,12 +306,19 @@ static int pkcs11_try_pkey_rsa_sign(EVP_PKEY_CTX *evp_pkey_ctx, PKCS11_SLOT_private *spriv; PKCS11_CTX_private *cpriv; const EVP_MD *sig_md; + CK_MECHANISM mechanism; + CK_RSA_PKCS_PSS_PARAMS pss_params; #ifdef DEBUG fprintf(stderr, "%s:%d pkcs11_try_pkey_rsa_sign() " "sig=%p *siglen=%lu tbs=%p tbslen=%lu\n", __FILE__, __LINE__, sig, *siglen, tbs, tbslen); #endif + /* RSA method has EVP_PKEY_FLAG_AUTOARGLEN set. OpenSSL core will handle + * the size inquiry internally. */ + if (!sig) + return -1; + pkey = EVP_PKEY_CTX_get0_pkey(evp_pkey_ctx); if (!pkey) return -1; @@ -334,45 +341,37 @@ static int pkcs11_try_pkey_rsa_sign(EVP_PKEY_CTX *evp_pkey_ctx, if (tbslen != (size_t)EVP_MD_size(sig_md)) return -1; - if (!cpriv->sign_initialized) { - int padding; - CK_MECHANISM mechanism; - CK_RSA_PKCS_PSS_PARAMS pss_params; - - memset(&mechanism, 0, sizeof mechanism); - EVP_PKEY_CTX_get_rsa_padding(evp_pkey_ctx, &padding); - switch (padding) { - case RSA_PKCS1_PSS_PADDING: + memset(&mechanism, 0, sizeof mechanism); + EVP_PKEY_CTX_get_rsa_padding(evp_pkey_ctx, &padding); + switch (padding) { + case RSA_PKCS1_PSS_PADDING: #ifdef DEBUG - fprintf(stderr, "%s:%d padding=RSA_PKCS1_PSS_PADDING\n", - __FILE__, __LINE__); + fprintf(stderr, "%s:%d padding=RSA_PKCS1_PSS_PADDING\n", + __FILE__, __LINE__); #endif - if (pkcs11_params_pss(&pss_params, evp_pkey_ctx) < 0) - return -1; - mechanism.mechanism = CKM_RSA_PKCS_PSS; - mechanism.pParameter = &pss_params; - mechanism.ulParameterLen = sizeof pss_params; - break; - default: + if (pkcs11_params_pss(&pss_params, evp_pkey_ctx) < 0) + return -1; + mechanism.mechanism = CKM_RSA_PKCS_PSS; + mechanism.pParameter = &pss_params; + mechanism.ulParameterLen = sizeof pss_params; + break; + default: #ifdef DEBUG - fprintf(stderr, "%s:%d unsupported padding: %d\n", - __FILE__, __LINE__, padding); + fprintf(stderr, "%s:%d unsupported padding: %d\n", + __FILE__, __LINE__, padding); #endif - return -1; - } /* end switch(padding) */ + return -1; + } /* end switch(padding) */ - CRYPTO_THREAD_write_lock(cpriv->rwlock); - rv = CRYPTOKI_call(ctx, - C_SignInit(spriv->session, &mechanism, kpriv->object)); - if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); - } + CRYPTO_THREAD_write_lock(cpriv->rwlock); + rv = CRYPTOKI_call(ctx, + C_SignInit(spriv->session, &mechanism, kpriv->object)); + if (!rv && kpriv->always_authenticate == CK_TRUE) + rv = pkcs11_authenticate(key); if (!rv) rv = CRYPTOKI_call(ctx, C_Sign(spriv->session, (CK_BYTE_PTR)tbs, tbslen, sig, &size)); - cpriv->sign_initialized = !rv && sig == NULL; - if (!cpriv->sign_initialized) - CRYPTO_THREAD_unlock(cpriv->rwlock); + CRYPTO_THREAD_unlock(cpriv->rwlock); #ifdef DEBUG fprintf(stderr, "%s:%d C_SignInit or C_Sign rv=%d\n", __FILE__, __LINE__, rv); From e4090f6071043a4213126133b6f6a4787014a4c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sat, 20 Mar 2021 08:52:40 +0200 Subject: [PATCH 02/12] Simplify pkcs11_try_pkey_rsa_decrypt() to not hold lock over calls This was originally added in commit 8356d568 "Add support for RSA-OAEP and RSA-PKCS encryption for PIV and HSM tokens" which just cloned the same pattern from pkcs11_try_pkey_rsa_sign(). Remove it as unneeded for the same reason: OpenSSL core handles the special case for us. --- src/libp11-int.h | 1 - src/p11_load.c | 1 - src/p11_pkey.c | 75 ++++++++++++++++++++++++------------------------ 3 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index 5c2b295..ab0893a 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -49,7 +49,6 @@ typedef struct pkcs11_ctx_private { unsigned int forkid; PKCS11_RWLOCK rwlock; int sign_initialized; - int decrypt_initialized; } PKCS11_CTX_private; #define PRIVCTX(ctx) ((PKCS11_CTX_private *) ((ctx)->_private)) diff --git a/src/p11_load.c b/src/p11_load.c index 2208861..9f478d9 100644 --- a/src/p11_load.c +++ b/src/p11_load.c @@ -42,7 +42,6 @@ PKCS11_CTX *pkcs11_CTX_new(void) cpriv->forkid = get_forkid(); cpriv->rwlock = CRYPTO_THREAD_lock_new(); cpriv->sign_initialized = 0; - cpriv->decrypt_initialized = 0; return ctx; fail: diff --git a/src/p11_pkey.c b/src/p11_pkey.c index a92874f..c378e86 100644 --- a/src/p11_pkey.c +++ b/src/p11_pkey.c @@ -402,19 +402,26 @@ static int pkcs11_try_pkey_rsa_decrypt(EVP_PKEY_CTX *evp_pkey_ctx, EVP_PKEY *pkey; RSA *rsa; PKCS11_KEY *key; - int rv = 0; + int rv = 0, padding; CK_ULONG size = *outlen; PKCS11_SLOT *slot; PKCS11_CTX *ctx; PKCS11_KEY_private *kpriv; PKCS11_SLOT_private *spriv; PKCS11_CTX_private *cpriv; + CK_MECHANISM mechanism; + CK_RSA_PKCS_OAEP_PARAMS oaep_params; #ifdef DEBUG fprintf(stderr, "%s:%d pkcs11_try_pkey_rsa_decrypt() " "out=%p *outlen=%lu in=%p inlen=%lu\n", __FILE__, __LINE__, out, *outlen, in, inlen); #endif + /* RSA method has EVP_PKEY_FLAG_AUTOARGLEN set. OpenSSL core will handle + * the size inquiry internally. */ + if (!out) + return -1; + pkey = EVP_PKEY_CTX_get0_pkey(evp_pkey_ctx); if (!pkey) return -1; @@ -433,53 +440,45 @@ static int pkcs11_try_pkey_rsa_decrypt(EVP_PKEY_CTX *evp_pkey_ctx, if (!evp_pkey_ctx) return -1; - if (!cpriv->decrypt_initialized) { - int padding; - CK_MECHANISM mechanism; - CK_RSA_PKCS_OAEP_PARAMS oaep_params; - - memset(&mechanism, 0, sizeof mechanism); - EVP_PKEY_CTX_get_rsa_padding(evp_pkey_ctx, &padding); - switch (padding) { - case RSA_PKCS1_OAEP_PADDING: + memset(&mechanism, 0, sizeof mechanism); + EVP_PKEY_CTX_get_rsa_padding(evp_pkey_ctx, &padding); + switch (padding) { + case RSA_PKCS1_OAEP_PADDING: #ifdef DEBUG - fprintf(stderr, "%s:%d padding=RSA_PKCS1_OAEP_PADDING\n", - __FILE__, __LINE__); + fprintf(stderr, "%s:%d padding=RSA_PKCS1_OAEP_PADDING\n", + __FILE__, __LINE__); #endif - if (pkcs11_params_oaep(&oaep_params, evp_pkey_ctx) < 0) - return -1; - mechanism.mechanism = CKM_RSA_PKCS_OAEP; - mechanism.pParameter = &oaep_params; - mechanism.ulParameterLen = sizeof oaep_params; - break; - case CKM_RSA_PKCS: + if (pkcs11_params_oaep(&oaep_params, evp_pkey_ctx) < 0) + return -1; + mechanism.mechanism = CKM_RSA_PKCS_OAEP; + mechanism.pParameter = &oaep_params; + mechanism.ulParameterLen = sizeof oaep_params; + break; + case CKM_RSA_PKCS: #ifdef DEBUG - fprintf(stderr, "%s:%d padding=CKM_RSA_PKCS\n", - __FILE__, __LINE__); + fprintf(stderr, "%s:%d padding=CKM_RSA_PKCS\n", + __FILE__, __LINE__); #endif - mechanism.pParameter = NULL; - mechanism.ulParameterLen = 0; - break; - default: + mechanism.pParameter = NULL; + mechanism.ulParameterLen = 0; + break; + default: #ifdef DEBUG - fprintf(stderr, "%s:%d unsupported padding: %d\n", - __FILE__, __LINE__, padding); + fprintf(stderr, "%s:%d unsupported padding: %d\n", + __FILE__, __LINE__, padding); #endif - return -1; - } /* end switch(padding) */ + return -1; + } /* end switch(padding) */ - CRYPTO_THREAD_write_lock(cpriv->rwlock); - rv = CRYPTOKI_call(ctx, - C_DecryptInit(spriv->session, &mechanism, kpriv->object)); - if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); - } + CRYPTO_THREAD_write_lock(cpriv->rwlock); + rv = CRYPTOKI_call(ctx, + C_DecryptInit(spriv->session, &mechanism, kpriv->object)); + if (!rv && kpriv->always_authenticate == CK_TRUE) + rv = pkcs11_authenticate(key); if (!rv) rv = CRYPTOKI_call(ctx, C_Decrypt(spriv->session, (CK_BYTE_PTR)in, inlen, out, &size)); - cpriv->decrypt_initialized = !rv && out == NULL; - if (!cpriv->decrypt_initialized) - CRYPTO_THREAD_unlock(cpriv->rwlock); + CRYPTO_THREAD_unlock(cpriv->rwlock); #ifdef DEBUG fprintf(stderr, "%s:%d C_DecryptInit or C_Decrypt rv=%d\n", __FILE__, __LINE__, rv); From 26e7e2616fbf46dd1f1c0b4c9991c560980129ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sat, 20 Mar 2021 08:59:25 +0200 Subject: [PATCH 03/12] Simplify pkcs11_try_pkey_ec_sign() to not hold lock over calls This was added in 592b71ae "Add EC signing through EVP api" and it just seems to have followed the pattern set by pkcs11_try_pkey_rsa_sign(). In fact here the code never worked correctly, because the *siglen is tested early with "if (*siglen < (size_t)ECDSA_size(eckey))" which breaks the size inquiry. Often *siglen would be uninitialized, or initialized to zero causing failure. This adds the proper code used by OpenSSL core to do the size inquiry, and removes the stateful handling of lock. --- src/libp11-int.h | 1 - src/p11_load.c | 1 - src/p11_pkey.c | 29 +++++++++++++++-------------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index ab0893a..aa19295 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -48,7 +48,6 @@ typedef struct pkcs11_ctx_private { void *ui_user_data; unsigned int forkid; PKCS11_RWLOCK rwlock; - int sign_initialized; } PKCS11_CTX_private; #define PRIVCTX(ctx) ((PKCS11_CTX_private *) ((ctx)->_private)) diff --git a/src/p11_load.c b/src/p11_load.c index 9f478d9..0704271 100644 --- a/src/p11_load.c +++ b/src/p11_load.c @@ -41,7 +41,6 @@ PKCS11_CTX *pkcs11_CTX_new(void) ctx->_private = cpriv; cpriv->forkid = get_forkid(); cpriv->rwlock = CRYPTO_THREAD_lock_new(); - cpriv->sign_initialized = 0; return ctx; fail: diff --git a/src/p11_pkey.c b/src/p11_pkey.c index c378e86..810ef91 100644 --- a/src/p11_pkey.c +++ b/src/p11_pkey.c @@ -549,6 +549,7 @@ static int pkcs11_try_pkey_ec_sign(EVP_PKEY_CTX *evp_pkey_ctx, PKCS11_CTX_private *cpriv; const EVP_MD *sig_md; ECDSA_SIG *ossl_sig; + CK_MECHANISM mechanism; #ifdef DEBUG fprintf(stderr, "%s:%d pkcs11_try_pkey_ec_sign() " @@ -568,6 +569,12 @@ static int pkcs11_try_pkey_ec_sign(EVP_PKEY_CTX *evp_pkey_ctx, if (!eckey) goto error; + if (!sig) { + *siglen = (size_t)ECDSA_size(eckey); + rv = CKR_OK; + goto error; + } + if (*siglen < (size_t)ECDSA_size(eckey)) goto error; @@ -591,25 +598,19 @@ static int pkcs11_try_pkey_ec_sign(EVP_PKEY_CTX *evp_pkey_ctx, goto error; rv = 0; - if (!cpriv->sign_initialized) { - CK_MECHANISM mechanism; - memset(&mechanism, 0, sizeof mechanism); - - mechanism.mechanism = CKM_ECDSA; + memset(&mechanism, 0, sizeof mechanism); + mechanism.mechanism = CKM_ECDSA; - CRYPTO_THREAD_write_lock(cpriv->rwlock); - rv = CRYPTOKI_call(ctx, - C_SignInit(spriv->session, &mechanism, kpriv->object)); - if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); - } + CRYPTO_THREAD_write_lock(cpriv->rwlock); + rv = CRYPTOKI_call(ctx, + C_SignInit(spriv->session, &mechanism, kpriv->object)); + if (!rv && kpriv->always_authenticate == CK_TRUE) + rv = pkcs11_authenticate(key); if (!rv) rv = CRYPTOKI_call(ctx, C_Sign(spriv->session, (CK_BYTE_PTR)tbs, tbslen, sig, &size)); + CRYPTO_THREAD_unlock(cpriv->rwlock); - cpriv->sign_initialized = !rv && sig == NULL; - if (!cpriv->sign_initialized) - CRYPTO_THREAD_unlock(cpriv->rwlock); #ifdef DEBUG fprintf(stderr, "%s:%d C_SignInit or C_Sign rv=%d\n", __FILE__, __LINE__, rv); From be8d3bf70e19068093f7df6ae66dafa53a3f28f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sun, 21 Mar 2021 20:41:20 +0200 Subject: [PATCH 04/12] Remove legacy cruft from atfork code This removes the __sun cruft which is never used. The #pragma init(lib_init) makes a function named "lib_init" an initializer, but we don't have such function. This is likely copy-paste cruft from where this code was taken from. Remove also the usage of "inline" and related checks. They add no value in .c file as the defined functions are used. Normally "inline" is used in header inline function definitions to remove compiler warning of unused function (as the function might not be used in all the C files including the header). --- src/p11_atfork.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/p11_atfork.c b/src/p11_atfork.c index fcbd3f4..537d10a 100644 --- a/src/p11_atfork.c +++ b/src/p11_atfork.c @@ -25,31 +25,16 @@ #ifndef _WIN32 #include -#ifndef __STDC_VERSION__ -/* older than C90 */ -#define inline -#endif /* __STDC_VERSION__ */ - #ifdef HAVE___REGISTER_ATFORK -#ifdef __sun -#pragma fini(lib_deinit) -#pragma init(lib_init) -#define _CONSTRUCTOR -#define _DESTRUCTOR -#else /* __sun */ -#define _CONSTRUCTOR __attribute__((constructor)) -#define _DESTRUCTOR __attribute__((destructor)) -#endif /* __sun */ - static unsigned int P11_forkid = 0; -inline static unsigned int _P11_get_forkid(void) +static unsigned int _P11_get_forkid(void) { return P11_forkid; } -inline static int _P11_detect_fork(unsigned int forkid) +static int _P11_detect_fork(unsigned int forkid) { if (forkid == P11_forkid) return 0; @@ -64,7 +49,7 @@ static void fork_handler(void) extern int __register_atfork(void (*)(void), void(*)(void), void (*)(void), void *); extern void *__dso_handle; -_CONSTRUCTOR +__attribute__((constructor)) int _P11_register_fork_handler(void) { if (__register_atfork(0, 0, fork_handler, __dso_handle) != 0) @@ -74,12 +59,12 @@ int _P11_register_fork_handler(void) #else /* HAVE___REGISTER_ATFORK */ -inline static unsigned int _P11_get_forkid(void) +static unsigned int _P11_get_forkid(void) { return getpid(); } -inline static int _P11_detect_fork(unsigned int forkid) +static int _P11_detect_fork(unsigned int forkid) { if (getpid() == forkid) return 0; From 03fbac537de0c20ef29b070146f99bf49fe9ce34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sat, 20 Mar 2021 20:14:23 +0200 Subject: [PATCH 05/12] Use pthread_atfork instead of __register_atfork Modify build system to detect pthreads, and use pthread_atfork for fork handling if available. It is conforming to POSIX.1-2001 and available widely instead of the non-standard __register_atfork. This is especially useful on musl c-library which does not ship the non-standard variant. Using the atfork callbacks is prefered as the last resort alternative adds a syscall, getpid(), to many fast path places. --- configure.ac | 6 +- m4/ax_pthread.m4 | 522 +++++++++++++++++++++++++++++++++++++++++++++++ src/p11_atfork.c | 18 +- 3 files changed, 536 insertions(+), 10 deletions(-) create mode 100644 m4/ax_pthread.m4 diff --git a/configure.ac b/configure.ac index 6e15443..d3b9b32 100644 --- a/configure.ac +++ b/configure.ac @@ -12,6 +12,7 @@ AC_INIT([libp11],[PACKAGE_VERSION_MAJOR.PACKAGE_VERSION_MINOR.PACKAGE_VERSION_FI AC_CONFIG_AUX_DIR([.]) AC_CONFIG_HEADERS([src/config.h]) AC_CONFIG_MACRO_DIR([m4]) +AC_CANONICAL_TARGET AM_INIT_AUTOMAKE([subdir-objects]) LIBP11_VERSION_MAJOR="PACKAGE_VERSION_MAJOR" @@ -193,7 +194,10 @@ if test "${WIN32}" != "yes"; then , [AC_MSG_ERROR([dlopen required])] ) - AC_CHECK_FUNCS([__register_atfork],,) + AX_PTHREAD + LIBS="$PTHREAD_LIBS $LIBS" + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + CC="$PTHREAD_CC" fi PKG_CHECK_MODULES( diff --git a/m4/ax_pthread.m4 b/m4/ax_pthread.m4 new file mode 100644 index 0000000..e5858e5 --- /dev/null +++ b/m4/ax_pthread.m4 @@ -0,0 +1,522 @@ +# =========================================================================== +# https://www.gnu.org/software/autoconf-archive/ax_pthread.html +# =========================================================================== +# +# SYNOPSIS +# +# AX_PTHREAD([ACTION-IF-FOUND[, ACTION-IF-NOT-FOUND]]) +# +# DESCRIPTION +# +# This macro figures out how to build C programs using POSIX threads. It +# sets the PTHREAD_LIBS output variable to the threads library and linker +# flags, and the PTHREAD_CFLAGS output variable to any special C compiler +# flags that are needed. (The user can also force certain compiler +# flags/libs to be tested by setting these environment variables.) +# +# Also sets PTHREAD_CC and PTHREAD_CXX to any special C compiler that is +# needed for multi-threaded programs (defaults to the value of CC +# respectively CXX otherwise). (This is necessary on e.g. AIX to use the +# special cc_r/CC_r compiler alias.) +# +# NOTE: You are assumed to not only compile your program with these flags, +# but also to link with them as well. For example, you might link with +# $PTHREAD_CC $CFLAGS $PTHREAD_CFLAGS $LDFLAGS ... $PTHREAD_LIBS $LIBS +# $PTHREAD_CXX $CXXFLAGS $PTHREAD_CFLAGS $LDFLAGS ... $PTHREAD_LIBS $LIBS +# +# If you are only building threaded programs, you may wish to use these +# variables in your default LIBS, CFLAGS, and CC: +# +# LIBS="$PTHREAD_LIBS $LIBS" +# CFLAGS="$CFLAGS $PTHREAD_CFLAGS" +# CXXFLAGS="$CXXFLAGS $PTHREAD_CFLAGS" +# CC="$PTHREAD_CC" +# CXX="$PTHREAD_CXX" +# +# In addition, if the PTHREAD_CREATE_JOINABLE thread-attribute constant +# has a nonstandard name, this macro defines PTHREAD_CREATE_JOINABLE to +# that name (e.g. PTHREAD_CREATE_UNDETACHED on AIX). +# +# Also HAVE_PTHREAD_PRIO_INHERIT is defined if pthread is found and the +# PTHREAD_PRIO_INHERIT symbol is defined when compiling with +# PTHREAD_CFLAGS. +# +# ACTION-IF-FOUND is a list of shell commands to run if a threads library +# is found, and ACTION-IF-NOT-FOUND is a list of commands to run it if it +# is not found. If ACTION-IF-FOUND is not specified, the default action +# will define HAVE_PTHREAD. +# +# Please let the authors know if this macro fails on any platform, or if +# you have any other suggestions or comments. This macro was based on work +# by SGJ on autoconf scripts for FFTW (http://www.fftw.org/) (with help +# from M. Frigo), as well as ac_pthread and hb_pthread macros posted by +# Alejandro Forero Cuervo to the autoconf macro repository. We are also +# grateful for the helpful feedback of numerous users. +# +# Updated for Autoconf 2.68 by Daniel Richard G. +# +# LICENSE +# +# Copyright (c) 2008 Steven G. Johnson +# Copyright (c) 2011 Daniel Richard G. +# Copyright (c) 2019 Marc Stevens +# +# 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 . +# +# As a special exception, the respective Autoconf Macro's copyright owner +# gives unlimited permission to copy, distribute and modify the configure +# scripts that are the output of Autoconf when processing the Macro. You +# need not follow the terms of the GNU General Public License when using +# or distributing such scripts, even though portions of the text of the +# Macro appear in them. The GNU General Public License (GPL) does govern +# all other use of the material that constitutes the Autoconf Macro. +# +# This special exception to the GPL applies to versions of the Autoconf +# Macro released by the Autoconf Archive. When you make and distribute a +# modified version of the Autoconf Macro, you may extend this special +# exception to the GPL to apply to your modified version as well. + +#serial 30 + +AU_ALIAS([ACX_PTHREAD], [AX_PTHREAD]) +AC_DEFUN([AX_PTHREAD], [ +AC_REQUIRE([AC_CANONICAL_TARGET]) +AC_REQUIRE([AC_PROG_CC]) +AC_REQUIRE([AC_PROG_SED]) +AC_LANG_PUSH([C]) +ax_pthread_ok=no + +# We used to check for pthread.h first, but this fails if pthread.h +# requires special compiler flags (e.g. on Tru64 or Sequent). +# It gets checked for in the link test anyway. + +# First of all, check if the user has set any of the PTHREAD_LIBS, +# etcetera environment variables, and if threads linking works using +# them: +if test "x$PTHREAD_CFLAGS$PTHREAD_LIBS" != "x"; then + ax_pthread_save_CC="$CC" + ax_pthread_save_CFLAGS="$CFLAGS" + ax_pthread_save_LIBS="$LIBS" + AS_IF([test "x$PTHREAD_CC" != "x"], [CC="$PTHREAD_CC"]) + AS_IF([test "x$PTHREAD_CXX" != "x"], [CXX="$PTHREAD_CXX"]) + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + LIBS="$PTHREAD_LIBS $LIBS" + AC_MSG_CHECKING([for pthread_join using $CC $PTHREAD_CFLAGS $PTHREAD_LIBS]) + AC_LINK_IFELSE([AC_LANG_CALL([], [pthread_join])], [ax_pthread_ok=yes]) + AC_MSG_RESULT([$ax_pthread_ok]) + if test "x$ax_pthread_ok" = "xno"; then + PTHREAD_LIBS="" + PTHREAD_CFLAGS="" + fi + CC="$ax_pthread_save_CC" + CFLAGS="$ax_pthread_save_CFLAGS" + LIBS="$ax_pthread_save_LIBS" +fi + +# We must check for the threads library under a number of different +# names; the ordering is very important because some systems +# (e.g. DEC) have both -lpthread and -lpthreads, where one of the +# libraries is broken (non-POSIX). + +# Create a list of thread flags to try. Items with a "," contain both +# C compiler flags (before ",") and linker flags (after ","). Other items +# starting with a "-" are C compiler flags, and remaining items are +# library names, except for "none" which indicates that we try without +# any flags at all, and "pthread-config" which is a program returning +# the flags for the Pth emulation library. + +ax_pthread_flags="pthreads none -Kthread -pthread -pthreads -mthreads pthread --thread-safe -mt pthread-config" + +# The ordering *is* (sometimes) important. Some notes on the +# individual items follow: + +# pthreads: AIX (must check this before -lpthread) +# none: in case threads are in libc; should be tried before -Kthread and +# other compiler flags to prevent continual compiler warnings +# -Kthread: Sequent (threads in libc, but -Kthread needed for pthread.h) +# -pthread: Linux/gcc (kernel threads), BSD/gcc (userland threads), Tru64 +# (Note: HP C rejects this with "bad form for `-t' option") +# -pthreads: Solaris/gcc (Note: HP C also rejects) +# -mt: Sun Workshop C (may only link SunOS threads [-lthread], but it +# doesn't hurt to check since this sometimes defines pthreads and +# -D_REENTRANT too), HP C (must be checked before -lpthread, which +# is present but should not be used directly; and before -mthreads, +# because the compiler interprets this as "-mt" + "-hreads") +# -mthreads: Mingw32/gcc, Lynx/gcc +# pthread: Linux, etcetera +# --thread-safe: KAI C++ +# pthread-config: use pthread-config program (for GNU Pth library) + +case $target_os in + + freebsd*) + + # -kthread: FreeBSD kernel threads (preferred to -pthread since SMP-able) + # lthread: LinuxThreads port on FreeBSD (also preferred to -pthread) + + ax_pthread_flags="-kthread lthread $ax_pthread_flags" + ;; + + hpux*) + + # From the cc(1) man page: "[-mt] Sets various -D flags to enable + # multi-threading and also sets -lpthread." + + ax_pthread_flags="-mt -pthread pthread $ax_pthread_flags" + ;; + + openedition*) + + # IBM z/OS requires a feature-test macro to be defined in order to + # enable POSIX threads at all, so give the user a hint if this is + # not set. (We don't define these ourselves, as they can affect + # other portions of the system API in unpredictable ways.) + + AC_EGREP_CPP([AX_PTHREAD_ZOS_MISSING], + [ +# if !defined(_OPEN_THREADS) && !defined(_UNIX03_THREADS) + AX_PTHREAD_ZOS_MISSING +# endif + ], + [AC_MSG_WARN([IBM z/OS requires -D_OPEN_THREADS or -D_UNIX03_THREADS to enable pthreads support.])]) + ;; + + solaris*) + + # On Solaris (at least, for some versions), libc contains stubbed + # (non-functional) versions of the pthreads routines, so link-based + # tests will erroneously succeed. (N.B.: The stubs are missing + # pthread_cleanup_push, or rather a function called by this macro, + # so we could check for that, but who knows whether they'll stub + # that too in a future libc.) So we'll check first for the + # standard Solaris way of linking pthreads (-mt -lpthread). + + ax_pthread_flags="-mt,-lpthread pthread $ax_pthread_flags" + ;; +esac + +# Are we compiling with Clang? + +AC_CACHE_CHECK([whether $CC is Clang], + [ax_cv_PTHREAD_CLANG], + [ax_cv_PTHREAD_CLANG=no + # Note that Autoconf sets GCC=yes for Clang as well as GCC + if test "x$GCC" = "xyes"; then + AC_EGREP_CPP([AX_PTHREAD_CC_IS_CLANG], + [/* Note: Clang 2.7 lacks __clang_[a-z]+__ */ +# if defined(__clang__) && defined(__llvm__) + AX_PTHREAD_CC_IS_CLANG +# endif + ], + [ax_cv_PTHREAD_CLANG=yes]) + fi + ]) +ax_pthread_clang="$ax_cv_PTHREAD_CLANG" + + +# GCC generally uses -pthread, or -pthreads on some platforms (e.g. SPARC) + +# Note that for GCC and Clang -pthread generally implies -lpthread, +# except when -nostdlib is passed. +# This is problematic using libtool to build C++ shared libraries with pthread: +# [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25460 +# [2] https://bugzilla.redhat.com/show_bug.cgi?id=661333 +# [3] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=468555 +# To solve this, first try -pthread together with -lpthread for GCC + +AS_IF([test "x$GCC" = "xyes"], + [ax_pthread_flags="-pthread,-lpthread -pthread -pthreads $ax_pthread_flags"]) + +# Clang takes -pthread (never supported any other flag), but we'll try with -lpthread first + +AS_IF([test "x$ax_pthread_clang" = "xyes"], + [ax_pthread_flags="-pthread,-lpthread -pthread"]) + + +# The presence of a feature test macro requesting re-entrant function +# definitions is, on some systems, a strong hint that pthreads support is +# correctly enabled + +case $target_os in + darwin* | hpux* | linux* | osf* | solaris*) + ax_pthread_check_macro="_REENTRANT" + ;; + + aix*) + ax_pthread_check_macro="_THREAD_SAFE" + ;; + + *) + ax_pthread_check_macro="--" + ;; +esac +AS_IF([test "x$ax_pthread_check_macro" = "x--"], + [ax_pthread_check_cond=0], + [ax_pthread_check_cond="!defined($ax_pthread_check_macro)"]) + + +if test "x$ax_pthread_ok" = "xno"; then +for ax_pthread_try_flag in $ax_pthread_flags; do + + case $ax_pthread_try_flag in + none) + AC_MSG_CHECKING([whether pthreads work without any flags]) + ;; + + *,*) + PTHREAD_CFLAGS=`echo $ax_pthread_try_flag | sed "s/^\(.*\),\(.*\)$/\1/"` + PTHREAD_LIBS=`echo $ax_pthread_try_flag | sed "s/^\(.*\),\(.*\)$/\2/"` + AC_MSG_CHECKING([whether pthreads work with "$PTHREAD_CFLAGS" and "$PTHREAD_LIBS"]) + ;; + + -*) + AC_MSG_CHECKING([whether pthreads work with $ax_pthread_try_flag]) + PTHREAD_CFLAGS="$ax_pthread_try_flag" + ;; + + pthread-config) + AC_CHECK_PROG([ax_pthread_config], [pthread-config], [yes], [no]) + AS_IF([test "x$ax_pthread_config" = "xno"], [continue]) + PTHREAD_CFLAGS="`pthread-config --cflags`" + PTHREAD_LIBS="`pthread-config --ldflags` `pthread-config --libs`" + ;; + + *) + AC_MSG_CHECKING([for the pthreads library -l$ax_pthread_try_flag]) + PTHREAD_LIBS="-l$ax_pthread_try_flag" + ;; + esac + + ax_pthread_save_CFLAGS="$CFLAGS" + ax_pthread_save_LIBS="$LIBS" + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + LIBS="$PTHREAD_LIBS $LIBS" + + # Check for various functions. We must include pthread.h, + # since some functions may be macros. (On the Sequent, we + # need a special flag -Kthread to make this header compile.) + # We check for pthread_join because it is in -lpthread on IRIX + # while pthread_create is in libc. We check for pthread_attr_init + # due to DEC craziness with -lpthreads. We check for + # pthread_cleanup_push because it is one of the few pthread + # functions on Solaris that doesn't have a non-functional libc stub. + # We try pthread_create on general principles. + + AC_LINK_IFELSE([AC_LANG_PROGRAM([#include +# if $ax_pthread_check_cond +# error "$ax_pthread_check_macro must be defined" +# endif + static void *some_global = NULL; + static void routine(void *a) + { + /* To avoid any unused-parameter or + unused-but-set-parameter warning. */ + some_global = a; + } + static void *start_routine(void *a) { return a; }], + [pthread_t th; pthread_attr_t attr; + pthread_create(&th, 0, start_routine, 0); + pthread_join(th, 0); + pthread_attr_init(&attr); + pthread_cleanup_push(routine, 0); + pthread_cleanup_pop(0) /* ; */])], + [ax_pthread_ok=yes], + []) + + CFLAGS="$ax_pthread_save_CFLAGS" + LIBS="$ax_pthread_save_LIBS" + + AC_MSG_RESULT([$ax_pthread_ok]) + AS_IF([test "x$ax_pthread_ok" = "xyes"], [break]) + + PTHREAD_LIBS="" + PTHREAD_CFLAGS="" +done +fi + + +# Clang needs special handling, because older versions handle the -pthread +# option in a rather... idiosyncratic way + +if test "x$ax_pthread_clang" = "xyes"; then + + # Clang takes -pthread; it has never supported any other flag + + # (Note 1: This will need to be revisited if a system that Clang + # supports has POSIX threads in a separate library. This tends not + # to be the way of modern systems, but it's conceivable.) + + # (Note 2: On some systems, notably Darwin, -pthread is not needed + # to get POSIX threads support; the API is always present and + # active. We could reasonably leave PTHREAD_CFLAGS empty. But + # -pthread does define _REENTRANT, and while the Darwin headers + # ignore this macro, third-party headers might not.) + + # However, older versions of Clang make a point of warning the user + # that, in an invocation where only linking and no compilation is + # taking place, the -pthread option has no effect ("argument unused + # during compilation"). They expect -pthread to be passed in only + # when source code is being compiled. + # + # Problem is, this is at odds with the way Automake and most other + # C build frameworks function, which is that the same flags used in + # compilation (CFLAGS) are also used in linking. Many systems + # supported by AX_PTHREAD require exactly this for POSIX threads + # support, and in fact it is often not straightforward to specify a + # flag that is used only in the compilation phase and not in + # linking. Such a scenario is extremely rare in practice. + # + # Even though use of the -pthread flag in linking would only print + # a warning, this can be a nuisance for well-run software projects + # that build with -Werror. So if the active version of Clang has + # this misfeature, we search for an option to squash it. + + AC_CACHE_CHECK([whether Clang needs flag to prevent "argument unused" warning when linking with -pthread], + [ax_cv_PTHREAD_CLANG_NO_WARN_FLAG], + [ax_cv_PTHREAD_CLANG_NO_WARN_FLAG=unknown + # Create an alternate version of $ac_link that compiles and + # links in two steps (.c -> .o, .o -> exe) instead of one + # (.c -> exe), because the warning occurs only in the second + # step + ax_pthread_save_ac_link="$ac_link" + ax_pthread_sed='s/conftest\.\$ac_ext/conftest.$ac_objext/g' + ax_pthread_link_step=`AS_ECHO(["$ac_link"]) | sed "$ax_pthread_sed"` + ax_pthread_2step_ac_link="($ac_compile) && (echo ==== >&5) && ($ax_pthread_link_step)" + ax_pthread_save_CFLAGS="$CFLAGS" + for ax_pthread_try in '' -Qunused-arguments -Wno-unused-command-line-argument unknown; do + AS_IF([test "x$ax_pthread_try" = "xunknown"], [break]) + CFLAGS="-Werror -Wunknown-warning-option $ax_pthread_try -pthread $ax_pthread_save_CFLAGS" + ac_link="$ax_pthread_save_ac_link" + AC_LINK_IFELSE([AC_LANG_SOURCE([[int main(void){return 0;}]])], + [ac_link="$ax_pthread_2step_ac_link" + AC_LINK_IFELSE([AC_LANG_SOURCE([[int main(void){return 0;}]])], + [break]) + ]) + done + ac_link="$ax_pthread_save_ac_link" + CFLAGS="$ax_pthread_save_CFLAGS" + AS_IF([test "x$ax_pthread_try" = "x"], [ax_pthread_try=no]) + ax_cv_PTHREAD_CLANG_NO_WARN_FLAG="$ax_pthread_try" + ]) + + case "$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG" in + no | unknown) ;; + *) PTHREAD_CFLAGS="$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG $PTHREAD_CFLAGS" ;; + esac + +fi # $ax_pthread_clang = yes + + + +# Various other checks: +if test "x$ax_pthread_ok" = "xyes"; then + ax_pthread_save_CFLAGS="$CFLAGS" + ax_pthread_save_LIBS="$LIBS" + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + LIBS="$PTHREAD_LIBS $LIBS" + + # Detect AIX lossage: JOINABLE attribute is called UNDETACHED. + AC_CACHE_CHECK([for joinable pthread attribute], + [ax_cv_PTHREAD_JOINABLE_ATTR], + [ax_cv_PTHREAD_JOINABLE_ATTR=unknown + for ax_pthread_attr in PTHREAD_CREATE_JOINABLE PTHREAD_CREATE_UNDETACHED; do + AC_LINK_IFELSE([AC_LANG_PROGRAM([#include ], + [int attr = $ax_pthread_attr; return attr /* ; */])], + [ax_cv_PTHREAD_JOINABLE_ATTR=$ax_pthread_attr; break], + []) + done + ]) + AS_IF([test "x$ax_cv_PTHREAD_JOINABLE_ATTR" != "xunknown" && \ + test "x$ax_cv_PTHREAD_JOINABLE_ATTR" != "xPTHREAD_CREATE_JOINABLE" && \ + test "x$ax_pthread_joinable_attr_defined" != "xyes"], + [AC_DEFINE_UNQUOTED([PTHREAD_CREATE_JOINABLE], + [$ax_cv_PTHREAD_JOINABLE_ATTR], + [Define to necessary symbol if this constant + uses a non-standard name on your system.]) + ax_pthread_joinable_attr_defined=yes + ]) + + AC_CACHE_CHECK([whether more special flags are required for pthreads], + [ax_cv_PTHREAD_SPECIAL_FLAGS], + [ax_cv_PTHREAD_SPECIAL_FLAGS=no + case $target_os in + solaris*) + ax_cv_PTHREAD_SPECIAL_FLAGS="-D_POSIX_PTHREAD_SEMANTICS" + ;; + esac + ]) + AS_IF([test "x$ax_cv_PTHREAD_SPECIAL_FLAGS" != "xno" && \ + test "x$ax_pthread_special_flags_added" != "xyes"], + [PTHREAD_CFLAGS="$ax_cv_PTHREAD_SPECIAL_FLAGS $PTHREAD_CFLAGS" + ax_pthread_special_flags_added=yes]) + + AC_CACHE_CHECK([for PTHREAD_PRIO_INHERIT], + [ax_cv_PTHREAD_PRIO_INHERIT], + [AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include ]], + [[int i = PTHREAD_PRIO_INHERIT; + return i;]])], + [ax_cv_PTHREAD_PRIO_INHERIT=yes], + [ax_cv_PTHREAD_PRIO_INHERIT=no]) + ]) + AS_IF([test "x$ax_cv_PTHREAD_PRIO_INHERIT" = "xyes" && \ + test "x$ax_pthread_prio_inherit_defined" != "xyes"], + [AC_DEFINE([HAVE_PTHREAD_PRIO_INHERIT], [1], [Have PTHREAD_PRIO_INHERIT.]) + ax_pthread_prio_inherit_defined=yes + ]) + + CFLAGS="$ax_pthread_save_CFLAGS" + LIBS="$ax_pthread_save_LIBS" + + # More AIX lossage: compile with *_r variant + if test "x$GCC" != "xyes"; then + case $target_os in + aix*) + AS_CASE(["x/$CC"], + [x*/c89|x*/c89_128|x*/c99|x*/c99_128|x*/cc|x*/cc128|x*/xlc|x*/xlc_v6|x*/xlc128|x*/xlc128_v6], + [#handle absolute path differently from PATH based program lookup + AS_CASE(["x$CC"], + [x/*], + [ + AS_IF([AS_EXECUTABLE_P([${CC}_r])],[PTHREAD_CC="${CC}_r"]) + AS_IF([test "x${CXX}" != "x"], [AS_IF([AS_EXECUTABLE_P([${CXX}_r])],[PTHREAD_CXX="${CXX}_r"])]) + ], + [ + AC_CHECK_PROGS([PTHREAD_CC],[${CC}_r],[$CC]) + AS_IF([test "x${CXX}" != "x"], [AC_CHECK_PROGS([PTHREAD_CXX],[${CXX}_r],[$CXX])]) + ] + ) + ]) + ;; + esac + fi +fi + +test -n "$PTHREAD_CC" || PTHREAD_CC="$CC" +test -n "$PTHREAD_CXX" || PTHREAD_CXX="$CXX" + +AC_SUBST([PTHREAD_LIBS]) +AC_SUBST([PTHREAD_CFLAGS]) +AC_SUBST([PTHREAD_CC]) +AC_SUBST([PTHREAD_CXX]) + +# Finally, execute ACTION-IF-FOUND/ACTION-IF-NOT-FOUND: +if test "x$ax_pthread_ok" = "xyes"; then + ifelse([$1],,[AC_DEFINE([HAVE_PTHREAD],[1],[Define if you have POSIX threads libraries and header files.])],[$1]) + : +else + ax_pthread_ok=no + $2 +fi +AC_LANG_POP +])dnl AX_PTHREAD diff --git a/src/p11_atfork.c b/src/p11_atfork.c index 537d10a..6e4b37e 100644 --- a/src/p11_atfork.c +++ b/src/p11_atfork.c @@ -23,9 +23,10 @@ #include "libp11-int.h" #ifndef _WIN32 -#include -#ifdef HAVE___REGISTER_ATFORK +#ifdef HAVE_PTHREAD + +#include static unsigned int P11_forkid = 0; @@ -41,23 +42,22 @@ static int _P11_detect_fork(unsigned int forkid) return 1; } -static void fork_handler(void) +static void _P11_atfork_child(void) { P11_forkid++; } -extern int __register_atfork(void (*)(void), void(*)(void), void (*)(void), void *); -extern void *__dso_handle; - __attribute__((constructor)) int _P11_register_fork_handler(void) { - if (__register_atfork(0, 0, fork_handler, __dso_handle) != 0) + if (pthread_atfork(0, 0, _P11_atfork_child) != 0) return -1; return 0; } -#else /* HAVE___REGISTER_ATFORK */ +#else /* HAVE_PTHREAD */ + +#include static unsigned int _P11_get_forkid(void) { @@ -71,7 +71,7 @@ static int _P11_detect_fork(unsigned int forkid) return 1; } -#endif /* HAVE___REGISTER_ATFORK */ +#endif /* HAVE_PTHREAD */ #else /* !_WIN32 */ From f109cf277336b29f8cf0673094c427d0126e2999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sun, 21 Mar 2021 20:57:15 +0200 Subject: [PATCH 06/12] Refactor atfork logic to elide locking on fast path The atfork infrastructure is modified so that on Windows it optimizes practically to nothing. Otherwise the forkid is kept in a global variable, which is updated exactly once per check round (to minimize syscalls). A fast path to avoid locking is added. When the atfork check is done, it is guaranteed that the P11_forkid does not change (it can change only for the after-fork child process). Only if a fork is detected, the locks are taken. Each object's forkid is again checked in each of the check_*_int calls, so they get updated only once even if the child is multithreaded. This already improves performance measurably, but is even more important to avoid lock contention after session pooling support is added. --- src/p11_atfork.c | 86 +++++++++++++++++++----------------------------- 1 file changed, 34 insertions(+), 52 deletions(-) diff --git a/src/p11_atfork.c b/src/p11_atfork.c index 6e4b37e..134c402 100644 --- a/src/p11_atfork.c +++ b/src/p11_atfork.c @@ -24,23 +24,11 @@ #ifndef _WIN32 -#ifdef HAVE_PTHREAD - -#include - static unsigned int P11_forkid = 0; -static unsigned int _P11_get_forkid(void) -{ - return P11_forkid; -} +#ifdef HAVE_PTHREAD -static int _P11_detect_fork(unsigned int forkid) -{ - if (forkid == P11_forkid) - return 0; - return 1; -} +#include static void _P11_atfork_child(void) { @@ -55,34 +43,47 @@ int _P11_register_fork_handler(void) return 0; } +static unsigned int _P11_update_forkid(void) +{ + return P11_forkid; +} + #else /* HAVE_PTHREAD */ #include -static unsigned int _P11_get_forkid(void) -{ - return getpid(); -} - -static int _P11_detect_fork(unsigned int forkid) +static unsigned int _P11_update_forkid(void) { - if (getpid() == forkid) - return 0; - return 1; + P11_forkid = (unsigned int)getpid(); + return P11_forkid; } #endif /* HAVE_PTHREAD */ +#define CHECK_FORKID(ctx, forkid, function_call) \ + do { \ + int rv = 0; \ + _P11_update_forkid(); \ + if (forkid != P11_forkid) { \ + CRYPTO_THREAD_write_lock(PRIVCTX(ctx)->rwlock); \ + function_call; \ + CRYPTO_THREAD_unlock(PRIVCTX(ctx)->rwlock); \ + } \ + return rv; \ + } while (0) + #else /* !_WIN32 */ -#define _P11_get_forkid() 0 -#define _P11_detect_fork(x) 0 +#define P11_forkid 0 +#define _P11_update_forkid() 0 +#define CHECK_FORKID(ctx, forkid, function_call) return 0 #endif /* !_WIN32 */ unsigned int get_forkid() { - return _P11_get_forkid(); + _P11_update_forkid(); + return P11_forkid; } /* @@ -94,10 +95,10 @@ static int check_fork_int(PKCS11_CTX *ctx) { PKCS11_CTX_private *cpriv = PRIVCTX(ctx); - if (_P11_detect_fork(cpriv->forkid)) { + if (cpriv->forkid != P11_forkid) { if (pkcs11_CTX_reload(ctx) < 0) return -1; - cpriv->forkid = _P11_get_forkid(); + cpriv->forkid = P11_forkid; } return 0; } @@ -157,16 +158,9 @@ static int check_key_fork_int(PKCS11_KEY *key) */ int check_fork(PKCS11_CTX *ctx) { - PKCS11_CTX_private *cpriv; - int rv; - if (!ctx) return -1; - cpriv = PRIVCTX(ctx); - CRYPTO_THREAD_write_lock(cpriv->rwlock); - rv = check_fork_int(ctx); - CRYPTO_THREAD_unlock(cpriv->rwlock); - return rv; + CHECK_FORKID(ctx, PRIVCTX(ctx)->forkid, check_fork_int(ctx)); } /* @@ -174,16 +168,10 @@ int check_fork(PKCS11_CTX *ctx) */ int check_slot_fork(PKCS11_SLOT *slot) { - PKCS11_CTX_private *cpriv; - int rv; - if (!slot) return -1; - cpriv = PRIVCTX(SLOT2CTX(slot)); - CRYPTO_THREAD_write_lock(cpriv->rwlock); - rv = check_slot_fork_int(slot); - CRYPTO_THREAD_unlock(cpriv->rwlock); - return rv; + CHECK_FORKID(SLOT2CTX(slot), PRIVSLOT(slot)->forkid, + check_slot_fork_int(slot)); } /* @@ -201,16 +189,10 @@ int check_token_fork(PKCS11_TOKEN *token) */ int check_key_fork(PKCS11_KEY *key) { - PKCS11_CTX_private *cpriv; - int rv; - if (!key) return -1; - cpriv = PRIVCTX(KEY2CTX(key)); - CRYPTO_THREAD_write_lock(cpriv->rwlock); - rv = check_key_fork_int(key); - CRYPTO_THREAD_unlock(cpriv->rwlock); - return rv; + CHECK_FORKID(KEY2CTX(key), PRIVKEY(key)->forkid, + check_key_fork_int(key)); } /* From 22a19c19a009c0b467e0ac602b0899b202045014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sun, 21 Mar 2021 21:08:23 +0200 Subject: [PATCH 07/12] Remove complexity from slot reinitialization after fork After fork the context handler will reset the PKCS#11 module by calling C_Initialize. After this all handles from the module should be considered invalidated. This combines the session opening and logging after fork to single function, and removes the relogin flag from functions where it's not needed. The new pkcs11_reload_slot() properly update the state so the normal functions operate as expected. This also fixes a memory leak after fork: if the slot was in logged-in state, a session was leaked from check_slot_fork_int because: 1. the "if loggedIn" clears state, and calls pkcs11_relogin() which also implicitly opens a session 2. the next "if haveSession" block fires also, and clears state, and calls pkcs11_reopen_session. This function will explicitly call C_OpenSession overwriting and leaking the session from step #1 --- src/libp11-int.h | 7 +++---- src/libp11.h | 2 -- src/p11_atfork.c | 18 ++++------------- src/p11_ckr.c | 1 - src/p11_front.c | 4 ++-- src/p11_slot.c | 52 ++++++++++++++++++++---------------------------- 6 files changed, 31 insertions(+), 53 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index aa19295..9676f2c 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -169,8 +169,7 @@ extern CK_RV C_UnloadModule(void *module); extern void pkcs11_destroy_keys(PKCS11_TOKEN *, unsigned int); extern void pkcs11_destroy_certs(PKCS11_TOKEN *); extern int pkcs11_reload_key(PKCS11_KEY *); -extern int pkcs11_reopen_session(PKCS11_SLOT * slot); -extern int pkcs11_relogin(PKCS11_SLOT * slot); +extern int pkcs11_reload_slot(PKCS11_SLOT * slot); /* Managing object attributes */ extern int pkcs11_getattr_var(PKCS11_TOKEN *, CK_OBJECT_HANDLE, @@ -232,7 +231,7 @@ extern void pkcs11_CTX_unload(PKCS11_CTX * ctx); extern void pkcs11_CTX_free(PKCS11_CTX * ctx); /* Open a session in RO or RW mode */ -extern int pkcs11_open_session(PKCS11_SLOT * slot, int rw, int relogin); +extern int pkcs11_open_session(PKCS11_SLOT * slot, int rw); /* Get a list of all slots */ extern int pkcs11_enumerate_slots(PKCS11_CTX * ctx, @@ -258,7 +257,7 @@ extern PKCS11_SLOT *pkcs11_find_next_token(PKCS11_CTX * ctx, extern int pkcs11_is_logged_in(PKCS11_SLOT * slot, int so, int * res); /* Authenticate to the card */ -extern int pkcs11_login(PKCS11_SLOT * slot, int so, const char *pin, int relogin); +extern int pkcs11_login(PKCS11_SLOT * slot, int so, const char *pin); /* De-authenticate from the card */ extern int pkcs11_logout(PKCS11_SLOT * slot); diff --git a/src/libp11.h b/src/libp11.h index e3da418..37cb432 100644 --- a/src/libp11.h +++ b/src/libp11.h @@ -511,7 +511,6 @@ P11_DEPRECATED_FUNC extern int PKCS11_private_decrypt( # define CKR_F_PKCS11_PRIVATE_DECRYPT 121 # define CKR_F_PKCS11_PRIVATE_ENCRYPT 122 # define CKR_F_PKCS11_RELOAD_KEY 123 -# define CKR_F_PKCS11_REOPEN_SESSION 124 # define CKR_F_PKCS11_SEED_RANDOM 125 # define CKR_F_PKCS11_STORE_CERTIFICATE 126 # define CKR_F_PKCS11_STORE_KEY 127 @@ -544,7 +543,6 @@ P11_DEPRECATED_FUNC extern int PKCS11_private_decrypt( #define PKCS11_F_PKCS11_PRIVATE_DECRYPT CKR_F_PKCS11_PRIVATE_DECRYPT #define PKCS11_F_PKCS11_PRIVATE_ENCRYPT CKR_F_PKCS11_PRIVATE_ENCRYPT #define PKCS11_F_PKCS11_RELOAD_KEY CKR_F_PKCS11_RELOAD_KEY -#define PKCS11_F_PKCS11_REOPEN_SESSION CKR_F_PKCS11_REOPEN_SESSION #define PKCS11_F_PKCS11_SEED_RANDOM CKR_F_PKCS11_SEED_RANDOM #define PKCS11_F_PKCS11_STORE_CERTIFICATE CKR_F_PKCS11_STORE_CERTIFICATE #define PKCS11_F_PKCS11_STORE_KEY CKR_F_PKCS11_STORE_KEY diff --git a/src/p11_atfork.c b/src/p11_atfork.c index 134c402..9313ba3 100644 --- a/src/p11_atfork.c +++ b/src/p11_atfork.c @@ -116,19 +116,8 @@ static int check_slot_fork_int(PKCS11_SLOT *slot) if (check_fork_int(SLOT2CTX(slot)) < 0) return -1; if (spriv->forkid != cpriv->forkid) { - if (spriv->loggedIn) { - int saved = spriv->haveSession; - spriv->haveSession = 0; - spriv->loggedIn = 0; - if (pkcs11_relogin(slot) < 0) - return -1; - spriv->haveSession = saved; - } - if (spriv->haveSession) { - spriv->haveSession = 0; - if (pkcs11_reopen_session(slot) < 0) - return -1; - } + if (pkcs11_reload_slot(slot) < 0) + return -1; spriv->forkid = cpriv->forkid; } return 0; @@ -147,7 +136,8 @@ static int check_key_fork_int(PKCS11_KEY *key) if (check_slot_fork_int(slot) < 0) return -1; if (spriv->forkid != kpriv->forkid) { - pkcs11_reload_key(key); + if (pkcs11_reload_key(key) < 0) + return -1; kpriv->forkid = spriv->forkid; } return 0; diff --git a/src/p11_ckr.c b/src/p11_ckr.c index 228edcd..280a722 100644 --- a/src/p11_ckr.c +++ b/src/p11_ckr.c @@ -52,7 +52,6 @@ static ERR_STRING_DATA CKR_str_functs[] = { {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"}, - {ERR_FUNC(CKR_F_PKCS11_REOPEN_SESSION), "pkcs11_reopen_session"}, {ERR_FUNC(CKR_F_PKCS11_SEED_RANDOM), "pkcs11_seed_random"}, {ERR_FUNC(CKR_F_PKCS11_STORE_CERTIFICATE), "pkcs11_store_certificate"}, {ERR_FUNC(CKR_F_PKCS11_STORE_KEY), "pkcs11_store_key"}, diff --git a/src/p11_front.c b/src/p11_front.c index 3a7bcd5..2bf7a7f 100644 --- a/src/p11_front.c +++ b/src/p11_front.c @@ -64,7 +64,7 @@ int PKCS11_open_session(PKCS11_SLOT *slot, int rw) { if (check_slot_fork(slot) < 0) return -1; - return pkcs11_open_session(slot, rw, 0); + return pkcs11_open_session(slot, rw); } int PKCS11_enumerate_slots(PKCS11_CTX *ctx, @@ -118,7 +118,7 @@ int PKCS11_login(PKCS11_SLOT *slot, int so, const char *pin) { if (check_slot_fork(slot) < 0) return -1; - return pkcs11_login(slot, so, pin, 0); + return pkcs11_login(slot, so, pin); } int PKCS11_logout(PKCS11_SLOT *slot) diff --git a/src/p11_slot.c b/src/p11_slot.c index 8934474..6f08269 100644 --- a/src/p11_slot.c +++ b/src/p11_slot.c @@ -147,17 +147,15 @@ PKCS11_SLOT *pkcs11_find_next_token(PKCS11_CTX *ctx, PKCS11_SLOT *slots, /* * Open a session with this slot */ -int pkcs11_open_session(PKCS11_SLOT *slot, int rw, int relogin) +int pkcs11_open_session(PKCS11_SLOT *slot, int rw) { PKCS11_SLOT_private *spriv = PRIVSLOT(slot); PKCS11_CTX *ctx = SLOT2CTX(slot); int rv; - if (relogin == 0) { - if (spriv->haveSession) { - CRYPTOKI_call(ctx, C_CloseSession(spriv->session)); - spriv->haveSession = 0; - } + if (spriv->haveSession) { + CRYPTOKI_call(ctx, C_CloseSession(spriv->session)); + spriv->haveSession = 0; } rv = CRYPTOKI_call(ctx, C_OpenSession(spriv->id, @@ -170,22 +168,6 @@ int pkcs11_open_session(PKCS11_SLOT *slot, int rw, int relogin) return 0; } -int pkcs11_reopen_session(PKCS11_SLOT *slot) -{ - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - PKCS11_CTX *ctx = SLOT2CTX(slot); - int rv; - - rv = CRYPTOKI_call(ctx, - C_OpenSession(spriv->id, - CKF_SERIAL_SESSION | (spriv->prev_rw ? CKF_RW_SESSION : 0), - NULL, NULL, &spriv->session)); - CRYPTOKI_checkerr(CKR_F_PKCS11_REOPEN_SESSION, rv); - spriv->haveSession = 1; - - return 0; -} - /* * Determines if user is authenticated with token */ @@ -219,22 +201,21 @@ int pkcs11_is_logged_in(PKCS11_SLOT *slot, int so, int *res) } /* - * Authenticate with the card. relogin should be set if we automatically - * relogin after a fork. + * Authenticate with the card. */ -int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin, int relogin) +int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin) { PKCS11_CTX *ctx = SLOT2CTX(slot); PKCS11_SLOT_private *spriv = PRIVSLOT(slot); int rv; - if (!relogin && spriv->loggedIn) + if (spriv->loggedIn) return 0; /* Nothing to do */ if (!spriv->haveSession) { /* SO gets a r/w session by default, * user gets a r/o session by default. */ - if (pkcs11_open_session(slot, so, relogin)) + if (pkcs11_open_session(slot, so)) return -1; } @@ -257,13 +238,24 @@ int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin, int relogin) } /* - * Authenticate with the card + * Reopens the slot by creating a session and logging in if needed. */ -int pkcs11_relogin(PKCS11_SLOT *slot) +int pkcs11_reload_slot(PKCS11_SLOT *slot) { PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - return pkcs11_login(slot, spriv->prev_so, spriv->prev_pin, 1); + if (spriv->haveSession) { + spriv->haveSession = 0; + if (pkcs11_open_session(slot, spriv->prev_rw)) + return -1; + } + if (spriv->loggedIn) { + spriv->loggedIn = 0; + if (pkcs11_login(slot, spriv->prev_so, spriv->prev_pin)) + return -1; + } + + return 0; } /* From be88be24c3c43716ca55703235e7dcf1565b62ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sun, 21 Mar 2021 21:46:46 +0200 Subject: [PATCH 08/12] Implement atfork handling for certificates This removes a search operation from pkcs11_remove_certificate() and simplifies it greatly. This makes also the handling of certificates similar to other objects, as only this was lacking the fork handling. Also fixed the memory leaks on error paths of pkcs11_remove_certificate() in case the certificate search failed. The equivalent code in pkcs11_reload_certificate() was refactored to always free allocated resources. --- src/libp11-int.h | 2 ++ src/libp11.h | 1 + src/p11_atfork.c | 26 ++++++++++++++-- src/p11_cert.c | 79 ++++++++++++++++++++++++++++-------------------- src/p11_ckr.c | 1 + 5 files changed, 75 insertions(+), 34 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index 9676f2c..13f7af4 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -106,6 +106,7 @@ typedef struct pkcs11_cert_private { CK_OBJECT_HANDLE object; unsigned char id[255]; size_t id_len; + unsigned int forkid; } PKCS11_CERT_private; #define PRIVCERT(cert) ((PKCS11_CERT_private *) (cert)->_private) #define CERT2SLOT(cert) TOKEN2SLOT(CERT2TOKEN(cert)) @@ -169,6 +170,7 @@ extern CK_RV C_UnloadModule(void *module); extern void pkcs11_destroy_keys(PKCS11_TOKEN *, unsigned int); extern void pkcs11_destroy_certs(PKCS11_TOKEN *); extern int pkcs11_reload_key(PKCS11_KEY *); +extern int pkcs11_reload_certificate(PKCS11_CERT *cert); extern int pkcs11_reload_slot(PKCS11_SLOT * slot); /* Managing object attributes */ diff --git a/src/libp11.h b/src/libp11.h index 37cb432..76027b4 100644 --- a/src/libp11.h +++ b/src/libp11.h @@ -517,6 +517,7 @@ P11_DEPRECATED_FUNC extern int PKCS11_private_decrypt( # define CKR_F_PKCS11_REMOVE_KEY 128 # define CKR_F_PKCS11_REMOVE_CERTIFICATE 129 # define CKR_F_PKCS11_GENERATE_KEY 130 +# define CKR_F_PKCS11_RELOAD_CERTIFICATE 131 /* Backward compatibility of error function codes */ #define PKCS11_F_PKCS11_CHANGE_PIN CKR_F_PKCS11_CHANGE_PIN diff --git a/src/p11_atfork.c b/src/p11_atfork.c index 9313ba3..af6e709 100644 --- a/src/p11_atfork.c +++ b/src/p11_atfork.c @@ -143,6 +143,27 @@ static int check_key_fork_int(PKCS11_KEY *key) return 0; } +/* + * PKCS#11 reinitialization after fork + * Also reloads the key + */ +static int check_cert_fork_int(PKCS11_CERT *cert) +{ + PKCS11_SLOT *slot = CERT2SLOT(cert); + PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + PKCS11_CERT_private *cpriv = PRIVCERT(cert); + + if (check_slot_fork_int(slot) < 0) + return -1; + + if (spriv->forkid != cpriv->forkid) { + if (pkcs11_reload_certificate(cert) < 0) + return -1; + cpriv->forkid = spriv->forkid; + } + return 0; +} + /* * Locking interface to check_fork_int() */ @@ -186,13 +207,14 @@ int check_key_fork(PKCS11_KEY *key) } /* - * Reinitialize cert (just its token) + * Locking interface to check_cert_fork_int() */ int check_cert_fork(PKCS11_CERT *cert) { if (!cert) return -1; - return check_token_fork(CERT2TOKEN(cert)); + CHECK_FORKID(CERT2CTX(cert), PRIVCERT(cert)->forkid, + check_cert_fork_int(cert)); } /* vim: set noexpandtab: */ diff --git a/src/p11_cert.c b/src/p11_cert.c index 19b9208..865a671 100644 --- a/src/p11_cert.c +++ b/src/p11_cert.c @@ -66,47 +66,20 @@ int pkcs11_enumerate_certs(PKCS11_TOKEN *token, /** * Remove a certificate from the associated token */ -int pkcs11_remove_certificate(PKCS11_CERT *cert){ +int pkcs11_remove_certificate(PKCS11_CERT *cert) +{ PKCS11_SLOT *slot = CERT2SLOT(cert); PKCS11_CTX *ctx = CERT2CTX(cert); PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - CK_OBJECT_HANDLE obj; - CK_ULONG count; - CK_ATTRIBUTE search_parameters[32]; - unsigned int n = 0; + PKCS11_CERT_private *cpriv = PRIVCERT(cert); int rv; /* First, make sure we have a session */ - if (!spriv->haveSession && PKCS11_open_session(slot, 1)){ + if (!spriv->haveSession && PKCS11_open_session(slot, 1)) return -1; - } - + - pkcs11_addattr_int(search_parameters + n++, CKA_CLASS, CKO_CERTIFICATE); - if (cert->id && cert->id_len){ - pkcs11_addattr(search_parameters + n++, CKA_ID, cert->id, cert->id_len); - } - if (cert->label){ - pkcs11_addattr_s(search_parameters + n++, CKA_LABEL, cert->label); - } - - rv = CRYPTOKI_call(ctx, - C_FindObjectsInit(spriv->session, search_parameters, n)); + rv = CRYPTOKI_call(ctx, C_DestroyObject(spriv->session, cpriv->object)); CRYPTOKI_checkerr(CKR_F_PKCS11_REMOVE_CERTIFICATE, rv); - - rv = CRYPTOKI_call(ctx, C_FindObjects(spriv->session, &obj, 1, &count)); - CRYPTOKI_checkerr(CKR_F_PKCS11_REMOVE_CERTIFICATE, rv); - - CRYPTOKI_call(ctx, C_FindObjectsFinal(spriv->session)); - if (count!=1){ - pkcs11_zap_attrs(search_parameters, n); - return -1; - } - rv = CRYPTOKI_call(ctx, C_DestroyObject(spriv->session, obj)); - if (rv != CKR_OK){ - pkcs11_zap_attrs(search_parameters, n); - return -1; - } - pkcs11_zap_attrs(search_parameters, n); return 0; } @@ -247,6 +220,48 @@ static int pkcs11_init_cert(PKCS11_CTX *ctx, PKCS11_TOKEN *token, return 0; } +/* + * Reload certificate object handle + */ +int pkcs11_reload_certificate(PKCS11_CERT *cert) +{ + PKCS11_SLOT *slot = CERT2SLOT(cert); + PKCS11_CTX *ctx = CERT2CTX(cert); + PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + PKCS11_CERT_private *cpriv = PRIVCERT(cert); + CK_ULONG count = 0; + CK_ATTRIBUTE search_parameters[32]; + unsigned int n = 0; + int rv; + + /* First, make sure we have a session */ + if (!spriv->haveSession && PKCS11_open_session(slot, 0)) { + return -1; + } + + pkcs11_addattr_int(search_parameters + n++, CKA_CLASS, CKO_CERTIFICATE); + if (cert->id && cert->id_len) { + pkcs11_addattr(search_parameters + n++, CKA_ID, cert->id, cert->id_len); + } + if (cert->label) { + pkcs11_addattr_s(search_parameters + n++, CKA_LABEL, cert->label); + } + + rv = CRYPTOKI_call(ctx, + C_FindObjectsInit(spriv->session, search_parameters, n)); + if (rv == CKR_OK) { + rv = CRYPTOKI_call(ctx, + C_FindObjects(spriv->session, &cpriv->object, 1, &count)); + CRYPTOKI_call(ctx, C_FindObjectsFinal(spriv->session)); + } + pkcs11_zap_attrs(search_parameters, n); + CRYPTOKI_checkerr(CKR_F_PKCS11_RELOAD_CERTIFICATE, rv); + + if (count != 1) + return -1; + return 0; +} + /* * Destroy all certs */ diff --git a/src/p11_ckr.c b/src/p11_ckr.c index 280a722..fcd651b 100644 --- a/src/p11_ckr.c +++ b/src/p11_ckr.c @@ -55,6 +55,7 @@ static ERR_STRING_DATA CKR_str_functs[] = { {ERR_FUNC(CKR_F_PKCS11_SEED_RANDOM), "pkcs11_seed_random"}, {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"}, {0, NULL} }; From 8ab6ca5add6b2514be57e597932ed845a24f1f0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Sun, 21 Mar 2021 21:46:46 +0200 Subject: [PATCH 09/12] Refactor internal code to use session pooling This gets rid of locking in the crypto operation paths, and allows concurrent use of multiple threads by making sure that each session is not used by any two threads. Additionally this fixes cases where it was possible that the PKCS#11 was called from different threads with the one per-slot session. The default session pool size is selected as 16, but it is dynamically shrunk if the maximum supported session count is reached. Further knobs to tune the session pool size can be added later. The pool is implemented with mutex+condition mechanism, and thus a simple wrapper to implement pthreads API on Windows Vista and later is added. This bumps the Windows requirement to a bit higher, but considering Vista is already EOL, this should be acceptable. A FIFO style queue was chosen to support even load-balancing between sessions. This is helps pkcs#11 libraries which in turn are load-balancing sessions to different units in a cluster. --- README.md | 13 ++-- make.rules.mak | 2 +- src/libp11-int.h | 20 +++-- src/libp11.h | 1 + src/p11_attr.c | 18 +++-- src/p11_cert.c | 63 ++++++++------- src/p11_ckr.c | 1 + src/p11_ec.c | 37 +++++---- src/p11_key.c | 121 +++++++++++------------------ src/p11_pkey.c | 56 ++++++------- src/p11_pthread.h | 94 ++++++++++++++++++++++ src/p11_rsa.c | 34 ++++---- src/p11_slot.c | 194 ++++++++++++++++++++++++++++------------------ 13 files changed, 400 insertions(+), 254 deletions(-) create mode 100644 src/p11_pthread.h diff --git a/README.md b/README.md index d155084..67b4d43 100644 --- a/README.md +++ b/README.md @@ -184,11 +184,14 @@ defaults to loading the p11-kit proxy module. ## Thread safety in libp11 -Thread-safety requires dynamic callbacks to be registered by the calling -application with the following OpenSSL functions: -* CRYPTO_set_dynlock_create_callback -* CRYPTO_set_dynlock_destroy_callback -* CRYPTO_set_dynlock_lock_callback +libp11 internally uses OS locking, and configures the PKCS#11 module to do +the same. + +Access to the the PKCS#11 tokens and objects is via a pool of PKCS#11 sessions. +This allows concurrent usage of crypto operations in thread safe manner. + +However, many of the main PKCS11_* API functions are currently not fully thread +safe. Work to fix this is pending. ## Submitting pull requests diff --git a/make.rules.mak b/make.rules.mak index 46e6854..8453cc0 100644 --- a/make.rules.mak +++ b/make.rules.mak @@ -43,6 +43,6 @@ OPENSSL_LIB = $(OPENSSL_DIR)\lib\VC\static\libcryptoMT$(DEBUG_SUFFIX).lib LIBS = "$(OPENSSL_LIB)" ws2_32.lib user32.lib advapi32.lib crypt32.lib gdi32.lib -CFLAGS = /nologo /GS /W3 /D_CRT_SECURE_NO_DEPRECATE /MT$(DEBUG_SUFFIX) $(OPENSSL_INC) /D_WIN32_WINNT=0x0502 /DWIN32_LEAN_AND_MEAN $(DEBUG_COMPILE) +CFLAGS = /nologo /GS /W3 /D_CRT_SECURE_NO_DEPRECATE /MT$(DEBUG_SUFFIX) $(OPENSSL_INC) /D_WIN32_WINNT=0x0600 /DWIN32_LEAN_AND_MEAN $(DEBUG_COMPILE) LINKFLAGS = /NOLOGO /INCREMENTAL:NO $(MACHINE) /MANIFEST:NO /NXCOMPAT /DYNAMICBASE $(DEBUG_LINK) diff --git a/src/libp11-int.h b/src/libp11-int.h index 13f7af4..2eb7703 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -29,6 +29,8 @@ #define CRYPTOKI_EXPORTS #include "pkcs11.h" +#include "p11_pthread.h" + #if OPENSSL_VERSION_NUMBER < 0x10100004L || defined(LIBRESSL_VERSION_NUMBER) typedef int PKCS11_RWLOCK; #else @@ -53,15 +55,17 @@ typedef struct pkcs11_ctx_private { typedef struct pkcs11_slot_private { PKCS11_CTX *parent; - unsigned char haveSession, loggedIn; + pthread_mutex_t lock; + pthread_cond_t cond; + int8_t rw_mode, logged_in; CK_SLOT_ID id; - CK_SESSION_HANDLE session; + CK_SESSION_HANDLE *session_pool; + unsigned int session_head, session_tail, session_poolsize; + unsigned int num_sessions, max_sessions; unsigned int forkid; - int prev_rw; /* the rw status the session was open */ /* options used in last PKCS11_login */ char *prev_pin; - int prev_so; } PKCS11_SLOT_private; #define PRIVSLOT(slot) ((PKCS11_SLOT_private *) ((slot)->_private)) #define SLOT2CTX(slot) (PRIVSLOT(slot)->parent) @@ -235,6 +239,12 @@ extern void pkcs11_CTX_free(PKCS11_CTX * ctx); /* Open a session in RO or RW mode */ extern int pkcs11_open_session(PKCS11_SLOT * slot, int rw); +/* Acquire a session from the slot specific session pool */ +extern int pkcs11_get_session(PKCS11_SLOT * slot, int rw, CK_SESSION_HANDLE *sessionp); + +/* Return a session the the slot specific session pool */ +extern void pkcs11_put_session(PKCS11_SLOT * slot, CK_SESSION_HANDLE session); + /* Get a list of all slots */ extern int pkcs11_enumerate_slots(PKCS11_CTX * ctx, PKCS11_SLOT **slotsp, unsigned int *nslotsp); @@ -265,7 +275,7 @@ extern int pkcs11_login(PKCS11_SLOT * slot, int so, const char *pin); extern int pkcs11_logout(PKCS11_SLOT * slot); /* Authenticate a private the key operation if needed */ -int pkcs11_authenticate(PKCS11_KEY *key); +int pkcs11_authenticate(PKCS11_KEY *key, CK_SESSION_HANDLE session); /* Get a list of keys associated with this token */ extern int pkcs11_enumerate_keys(PKCS11_TOKEN *token, unsigned int type, diff --git a/src/libp11.h b/src/libp11.h index 76027b4..734971b 100644 --- a/src/libp11.h +++ b/src/libp11.h @@ -518,6 +518,7 @@ P11_DEPRECATED_FUNC extern int PKCS11_private_decrypt( # define CKR_F_PKCS11_REMOVE_CERTIFICATE 129 # define CKR_F_PKCS11_GENERATE_KEY 130 # define CKR_F_PKCS11_RELOAD_CERTIFICATE 131 +# define CKR_F_PKCS11_GET_SESSION 132 /* Backward compatibility of error function codes */ #define PKCS11_F_PKCS11_CHANGE_PIN CKR_F_PKCS11_CHANGE_PIN diff --git a/src/p11_attr.c b/src/p11_attr.c index 76eac0a..d261c04 100644 --- a/src/p11_attr.c +++ b/src/p11_attr.c @@ -33,20 +33,26 @@ /* * Query pkcs11 attributes */ -static int pkcs11_getattr_int(PKCS11_CTX *ctx, CK_SESSION_HANDLE session, - CK_OBJECT_HANDLE o, CK_ATTRIBUTE_TYPE type, CK_BYTE *value, - size_t *size) +static int pkcs11_getattr_int(PKCS11_TOKEN *token, CK_OBJECT_HANDLE o, + CK_ATTRIBUTE_TYPE type, CK_BYTE *value, size_t *size) { + PKCS11_SLOT *slot = TOKEN2SLOT(token); + PKCS11_CTX *ctx = SLOT2CTX(slot); CK_ATTRIBUTE templ; + CK_SESSION_HANDLE session; int rv; templ.type = type; templ.pValue = value; templ.ulValueLen = *size; + if (pkcs11_get_session(slot, 0, &session)) + return -1; + rv = CRYPTOKI_call(ctx, C_GetAttributeValue(session, o, &templ, 1)); - CRYPTOKI_checkerr(CKR_F_PKCS11_GETATTR_INT, rv); + pkcs11_put_session(slot, session); + CRYPTOKI_checkerr(CKR_F_PKCS11_GETATTR_INT, rv); *size = templ.ulValueLen; return 0; } @@ -54,9 +60,7 @@ static int pkcs11_getattr_int(PKCS11_CTX *ctx, CK_SESSION_HANDLE session, int pkcs11_getattr_var(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object, unsigned int type, CK_BYTE *value, size_t *size) { - return pkcs11_getattr_int(TOKEN2CTX(token), - PRIVSLOT(TOKEN2SLOT(token))->session, - object, type, value, size); + return pkcs11_getattr_int(token, object, type, value, size); } int pkcs11_getattr_val(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object, diff --git a/src/p11_cert.c b/src/p11_cert.c index 865a671..024c03f 100644 --- a/src/p11_cert.c +++ b/src/p11_cert.c @@ -26,7 +26,7 @@ #include "libp11-int.h" #include -static int pkcs11_find_certs(PKCS11_TOKEN *); +static int pkcs11_find_certs(PKCS11_TOKEN *, CK_SESSION_HANDLE); static int pkcs11_next_cert(PKCS11_CTX *, PKCS11_TOKEN *, CK_SESSION_HANDLE); static int pkcs11_init_cert(PKCS11_CTX *ctx, PKCS11_TOKEN *token, CK_SESSION_HANDLE session, CK_OBJECT_HANDLE o, PKCS11_CERT **); @@ -38,19 +38,15 @@ int pkcs11_enumerate_certs(PKCS11_TOKEN *token, PKCS11_CERT **certp, unsigned int *countp) { PKCS11_SLOT *slot = TOKEN2SLOT(token); - PKCS11_CTX *ctx = SLOT2CTX(slot); PKCS11_TOKEN_private *tpriv = PRIVTOKEN(token); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - PKCS11_CTX_private *cpriv = PRIVCTX(ctx); + CK_SESSION_HANDLE session; int rv; - /* Make sure we have a session */ - if (!spriv->haveSession && PKCS11_open_session(slot, 0)) + if (pkcs11_get_session(slot, 0, &session)) return -1; - CRYPTO_THREAD_write_lock(cpriv->rwlock); - rv = pkcs11_find_certs(token); - CRYPTO_THREAD_unlock(cpriv->rwlock); + rv = pkcs11_find_certs(token, session); + pkcs11_put_session(slot, session); if (rv < 0) { pkcs11_destroy_certs(token); return -1; @@ -70,15 +66,16 @@ int pkcs11_remove_certificate(PKCS11_CERT *cert) { PKCS11_SLOT *slot = CERT2SLOT(cert); PKCS11_CTX *ctx = CERT2CTX(cert); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); PKCS11_CERT_private *cpriv = PRIVCERT(cert); + CK_SESSION_HANDLE session; int rv; - /* First, make sure we have a session */ - if (!spriv->haveSession && PKCS11_open_session(slot, 1)) + if (pkcs11_get_session(slot, 1, &session)) return -1; - rv = CRYPTOKI_call(ctx, C_DestroyObject(spriv->session, cpriv->object)); + rv = CRYPTOKI_call(ctx, C_DestroyObject(session, cpriv->object)); + pkcs11_put_session(slot, session); + CRYPTOKI_checkerr(CKR_F_PKCS11_REMOVE_CERTIFICATE, rv); return 0; } @@ -108,11 +105,10 @@ PKCS11_CERT *pkcs11_find_certificate(PKCS11_KEY *key) /* * Find all certs of a given type (public or private) */ -static int pkcs11_find_certs(PKCS11_TOKEN *token) +static int pkcs11_find_certs(PKCS11_TOKEN *token, CK_SESSION_HANDLE session) { PKCS11_SLOT *slot = TOKEN2SLOT(token); PKCS11_CTX *ctx = SLOT2CTX(slot); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); CK_OBJECT_CLASS cert_search_class; CK_ATTRIBUTE cert_search_attrs[] = { {CKA_CLASS, &cert_search_class, sizeof(cert_search_class)}, @@ -121,14 +117,14 @@ static int pkcs11_find_certs(PKCS11_TOKEN *token) /* Tell the PKCS11 lib to enumerate all matching objects */ cert_search_class = CKO_CERTIFICATE; - rv = CRYPTOKI_call(ctx, C_FindObjectsInit(spriv->session, cert_search_attrs, 1)); + rv = CRYPTOKI_call(ctx, C_FindObjectsInit(session, cert_search_attrs, 1)); CRYPTOKI_checkerr(CKR_F_PKCS11_FIND_CERTS, rv); do { - res = pkcs11_next_cert(ctx, token, spriv->session); + res = pkcs11_next_cert(ctx, token, session); } while (res == 0); - CRYPTOKI_call(ctx, C_FindObjectsFinal(spriv->session)); + CRYPTOKI_call(ctx, C_FindObjectsFinal(session)); return (res < 0) ? -1 : 0; } @@ -227,17 +223,15 @@ int pkcs11_reload_certificate(PKCS11_CERT *cert) { PKCS11_SLOT *slot = CERT2SLOT(cert); PKCS11_CTX *ctx = CERT2CTX(cert); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); PKCS11_CERT_private *cpriv = PRIVCERT(cert); CK_ULONG count = 0; CK_ATTRIBUTE search_parameters[32]; + CK_SESSION_HANDLE session; unsigned int n = 0; int rv; - /* First, make sure we have a session */ - if (!spriv->haveSession && PKCS11_open_session(slot, 0)) { + if (pkcs11_get_session(slot, 0, &session)) return -1; - } pkcs11_addattr_int(search_parameters + n++, CKA_CLASS, CKO_CERTIFICATE); if (cert->id && cert->id_len) { @@ -248,12 +242,13 @@ int pkcs11_reload_certificate(PKCS11_CERT *cert) } rv = CRYPTOKI_call(ctx, - C_FindObjectsInit(spriv->session, search_parameters, n)); + C_FindObjectsInit(session, search_parameters, n)); if (rv == CKR_OK) { rv = CRYPTOKI_call(ctx, - C_FindObjects(spriv->session, &cpriv->object, 1, &count)); - CRYPTOKI_call(ctx, C_FindObjectsFinal(spriv->session)); + C_FindObjects(session, &cpriv->object, 1, &count)); + CRYPTOKI_call(ctx, C_FindObjectsFinal(session)); } + pkcs11_put_session(slot, session); pkcs11_zap_attrs(search_parameters, n); CRYPTOKI_checkerr(CKR_F_PKCS11_RELOAD_CERTIFICATE, rv); @@ -294,10 +289,10 @@ int pkcs11_store_certificate(PKCS11_TOKEN *token, X509 *x509, char *label, { PKCS11_SLOT *slot = TOKEN2SLOT(token); PKCS11_CTX *ctx = SLOT2CTX(slot); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; CK_OBJECT_HANDLE object; CK_ATTRIBUTE attrs[32]; - unsigned int n = 0; + unsigned int n = 0, r = -1; int rv; int signature_nid; const EVP_MD* evp_md; @@ -307,7 +302,7 @@ int pkcs11_store_certificate(PKCS11_TOKEN *token, X509 *x509, char *label, unsigned int md_len; /* First, make sure we have a session */ - if (!PRIVSLOT(slot)->haveSession && PKCS11_open_session(slot, 1)) + if (pkcs11_get_session(slot, 1, &session)) return -1; /* Now build the template */ @@ -361,15 +356,19 @@ int pkcs11_store_certificate(PKCS11_TOKEN *token, X509 *x509, char *label, pkcs11_addattr(attrs + n++, CKA_ID, id, id_len); /* Now call the pkcs11 module to create the object */ - rv = CRYPTOKI_call(ctx, C_CreateObject(spriv->session, attrs, n, &object)); + rv = CRYPTOKI_call(ctx, C_CreateObject(session, attrs, n, &object)); /* Zap all memory allocated when building the template */ pkcs11_zap_attrs(attrs, n); - CRYPTOKI_checkerr(CKR_F_PKCS11_STORE_CERTIFICATE, rv); - /* Gobble the key object */ - return pkcs11_init_cert(ctx, token, spriv->session, object, ret_cert); + if (rv == CKR_OK) { + r = pkcs11_init_cert(ctx, token, session, object, ret_cert); + } + pkcs11_put_session(slot, session); + + CRYPTOKI_checkerr(CKR_F_PKCS11_STORE_CERTIFICATE, rv); + return r; } /* vim: set noexpandtab: */ diff --git a/src/p11_ckr.c b/src/p11_ckr.c index fcd651b..53e90ed 100644 --- a/src/p11_ckr.c +++ b/src/p11_ckr.c @@ -56,6 +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"}, {0, NULL} }; diff --git a/src/p11_ec.c b/src/p11_ec.c index 7e4b676..0fddff7 100644 --- a/src/p11_ec.c +++ b/src/p11_ec.c @@ -385,7 +385,7 @@ static int pkcs11_ecdsa_sign(const unsigned char *msg, unsigned int msg_len, PKCS11_SLOT *slot = KEY2SLOT(key); PKCS11_CTX *ctx = KEY2CTX(key); PKCS11_KEY_private *kpriv = PRIVKEY(key); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; CK_MECHANISM mechanism; CK_ULONG ck_sigsize; @@ -394,15 +394,17 @@ static int pkcs11_ecdsa_sign(const unsigned char *msg, unsigned int msg_len, memset(&mechanism, 0, sizeof(mechanism)); mechanism.mechanism = CKM_ECDSA; - CRYPTO_THREAD_write_lock(PRIVCTX(ctx)->rwlock); + if (pkcs11_get_session(slot, 0, &session)) + return -1; + rv = CRYPTOKI_call(ctx, - C_SignInit(spriv->session, &mechanism, kpriv->object)); + C_SignInit(session, &mechanism, kpriv->object)); if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); + rv = pkcs11_authenticate(key, session); if (!rv) rv = CRYPTOKI_call(ctx, - C_Sign(spriv->session, (CK_BYTE *)msg, msg_len, sigret, &ck_sigsize)); - CRYPTO_THREAD_unlock(PRIVCTX(ctx)->rwlock); + C_Sign(session, (CK_BYTE *)msg, msg_len, sigret, &ck_sigsize)); + pkcs11_put_session(slot, session); if (rv) { CKRerr(CKR_F_PKCS11_ECDSA_SIGN, rv); @@ -544,7 +546,7 @@ static int pkcs11_ecdh_derive(unsigned char **out, size_t *outlen, PKCS11_CTX *ctx = KEY2CTX(key); PKCS11_TOKEN *token = KEY2TOKEN(key); PKCS11_KEY_private *kpriv = PRIVKEY(key); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; CK_MECHANISM mechanism; int rv; @@ -585,24 +587,33 @@ static int pkcs11_ecdh_derive(unsigned char **out, size_t *outlen, return -1; } - rv = CRYPTOKI_call(ctx, C_DeriveKey(spriv->session, &mechanism, kpriv->object, + if (pkcs11_get_session(slot, 0, &session)) + return -1; + + rv = CRYPTOKI_call(ctx, C_DeriveKey(session, &mechanism, kpriv->object, newkey_template, sizeof(newkey_template)/sizeof(*newkey_template), &newkey)); - CRYPTOKI_checkerr(CKR_F_PKCS11_ECDH_DERIVE, rv); + if (rv != CKR_OK) + goto error; /* Return the value of the secret key and/or the object handle of the secret key */ if (out && outlen) { /* pkcs11_ec_ckey only asks for the value */ if (pkcs11_getattr_alloc(token, newkey, CKA_VALUE, out, outlen)) { - CKRerr(CKR_F_PKCS11_ECDH_DERIVE, CKR_ATTRIBUTE_VALUE_INVALID); - CRYPTOKI_call(ctx, C_DestroyObject(spriv->session, newkey)); - return -1; + CRYPTOKI_call(ctx, C_DestroyObject(session, newkey)); + goto error; } } if (tmpnewkey) /* For future use (not used by pkcs11_ec_ckey) */ *tmpnewkey = newkey; else /* Destroy the temporary key */ - CRYPTOKI_call(ctx, C_DestroyObject(spriv->session, newkey)); + CRYPTOKI_call(ctx, C_DestroyObject(session, newkey)); + + pkcs11_put_session(slot, session); return 0; +error: + pkcs11_put_session(slot, session); + CKRerr(CKR_F_PKCS11_ECDH_DERIVE, rv); + return -1; } static int pkcs11_ecdh_compute_key(unsigned char **buf, size_t *buflen, diff --git a/src/p11_key.c b/src/p11_key.c index e2ffe3b..d42386b 100644 --- a/src/p11_key.c +++ b/src/p11_key.c @@ -29,7 +29,7 @@ /* The maximum length of PIN */ #define MAX_PIN_LENGTH 32 -static int pkcs11_find_keys(PKCS11_TOKEN *, unsigned int); +static int pkcs11_find_keys(PKCS11_TOKEN *, CK_SESSION_HANDLE, unsigned int); static int pkcs11_next_key(PKCS11_CTX *ctx, PKCS11_TOKEN *token, CK_SESSION_HANDLE session, CK_OBJECT_CLASS type); static int pkcs11_init_key(PKCS11_CTX *ctx, PKCS11_TOKEN *token, @@ -98,8 +98,8 @@ int pkcs11_reload_key(PKCS11_KEY *key) { PKCS11_KEY_private *kpriv = PRIVKEY(key); PKCS11_SLOT *slot = KEY2SLOT(key); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); PKCS11_CTX *ctx = SLOT2CTX(slot); + CK_SESSION_HANDLE session; CK_OBJECT_CLASS key_search_class = key->isPrivate ? CKO_PRIVATE_KEY : CKO_PUBLIC_KEY; CK_ATTRIBUTE key_search_attrs[2] = { @@ -109,18 +109,18 @@ int pkcs11_reload_key(PKCS11_KEY *key) CK_ULONG count; int rv; - /* this is already covered with a per-ctx lock */ - - rv = CRYPTOKI_call(ctx, - C_FindObjectsInit(spriv->session, key_search_attrs, 2)); - CRYPTOKI_checkerr(CKR_F_PKCS11_RELOAD_KEY, rv); + if (pkcs11_get_session(slot, 0, &session)) + return -1; rv = CRYPTOKI_call(ctx, - C_FindObjects(spriv->session, &kpriv->object, 1, &count)); + C_FindObjectsInit(session, key_search_attrs, 2)); + if (rv == CKR_OK) { + rv = CRYPTOKI_call(ctx, + C_FindObjects(session, &kpriv->object, 1, &count)); + CRYPTOKI_call(ctx, C_FindObjectsFinal(session)); + } CRYPTOKI_checkerr(CKR_F_PKCS11_RELOAD_KEY, rv); - CRYPTOKI_call(ctx, C_FindObjectsFinal(spriv->session)); - return 0; } @@ -132,8 +132,7 @@ int pkcs11_generate_key(PKCS11_TOKEN *token, int algorithm, unsigned int bits, PKCS11_SLOT *slot = TOKEN2SLOT(token); PKCS11_CTX *ctx = TOKEN2CTX(token); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - + CK_SESSION_HANDLE session; CK_ATTRIBUTE pubkey_attrs[32]; CK_ATTRIBUTE privkey_attrs[32]; unsigned int n_pub = 0, n_priv = 0; @@ -146,8 +145,7 @@ int pkcs11_generate_key(PKCS11_TOKEN *token, int algorithm, unsigned int bits, (void)algorithm; /* squash the unused parameter warning */ - /* make sure we have a session */ - if (!spriv->haveSession && PKCS11_open_session(slot, 1)) + if (pkcs11_get_session(slot, 1, &session)) return -1; /* pubkey attributes */ @@ -175,7 +173,7 @@ int pkcs11_generate_key(PKCS11_TOKEN *token, int algorithm, unsigned int bits, /* call the pkcs11 module to create the key pair */ rv = CRYPTOKI_call(ctx, C_GenerateKeyPair( - spriv->session, + session, &mechanism, pubkey_attrs, n_pub, @@ -184,6 +182,7 @@ int pkcs11_generate_key(PKCS11_TOKEN *token, int algorithm, unsigned int bits, &pub_key_obj, &priv_key_obj )); + pkcs11_put_session(slot, session); /* zap all memory allocated when building the template */ pkcs11_zap_attrs(privkey_attrs, n_priv); @@ -222,17 +221,13 @@ static int pkcs11_store_key(PKCS11_TOKEN *token, EVP_PKEY *pk, { PKCS11_SLOT *slot = TOKEN2SLOT(token); PKCS11_CTX *ctx = TOKEN2CTX(token); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; CK_OBJECT_HANDLE object; CK_ATTRIBUTE attrs[32]; unsigned int n = 0; - int rv; + int rv, r = -1; const BIGNUM *rsa_n, *rsa_e, *rsa_d, *rsa_p, *rsa_q, *rsa_dmp1, *rsa_dmq1, *rsa_iqmp; - /* First, make sure we have a session */ - if (!spriv->haveSession && PKCS11_open_session(slot, 1)) - return -1; - /* Now build the key attrs */ pkcs11_addattr_int(attrs + n++, CKA_CLASS, type); if (label) @@ -291,16 +286,26 @@ static int pkcs11_store_key(PKCS11_TOKEN *token, EVP_PKEY *pk, return -1; } + if (pkcs11_get_session(slot, 1, &session)) { + pkcs11_zap_attrs(attrs, n); + return -1; + } + /* Now call the pkcs11 module to create the object */ - rv = CRYPTOKI_call(ctx, C_CreateObject(spriv->session, attrs, n, &object)); + rv = CRYPTOKI_call(ctx, C_CreateObject(session, attrs, n, &object)); /* Zap all memory allocated when building the template */ pkcs11_zap_attrs(attrs, n); + if (rv == CKR_OK) { + /* Gobble the key object */ + r = pkcs11_init_key(ctx, token, session, object, type, ret_key); + } + pkcs11_put_session(slot, session); + CRYPTOKI_checkerr(CKR_F_PKCS11_STORE_KEY, rv); + return r; - /* Gobble the key object */ - return pkcs11_init_key(ctx, token, spriv->session, object, type, ret_key); } /* @@ -348,11 +353,10 @@ EVP_PKEY *pkcs11_get_key(PKCS11_KEY *key, int isPrivate) * Authenticate a private the key operation if needed * This function *only* handles CKU_CONTEXT_SPECIFIC logins. */ -int pkcs11_authenticate(PKCS11_KEY *key) +int pkcs11_authenticate(PKCS11_KEY *key, CK_SESSION_HANDLE session) { PKCS11_TOKEN *token = KEY2TOKEN(key); PKCS11_SLOT *slot = TOKEN2SLOT(token); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); PKCS11_CTX *ctx = SLOT2CTX(slot); PKCS11_CTX_private *cpriv = PRIVCTX(ctx); char pin[MAX_PIN_LENGTH+1]; @@ -363,7 +367,7 @@ int pkcs11_authenticate(PKCS11_KEY *key) /* Handle CKF_PROTECTED_AUTHENTICATION_PATH */ if (token->secureLogin) { rv = CRYPTOKI_call(ctx, - C_Login(spriv->session, CKU_CONTEXT_SPECIFIC, NULL, 0)); + C_Login(session, CKU_CONTEXT_SPECIFIC, NULL, 0)); return rv == CKR_USER_ALREADY_LOGGED_IN ? 0 : rv; } @@ -394,7 +398,7 @@ int pkcs11_authenticate(PKCS11_KEY *key) /* Login with the PIN */ rv = CRYPTOKI_call(ctx, - C_Login(spriv->session, CKU_CONTEXT_SPECIFIC, + C_Login(session, CKU_CONTEXT_SPECIFIC, (CK_UTF8CHAR *)pin, strlen(pin))); OPENSSL_cleanse(pin, MAX_PIN_LENGTH+1); return rv == CKR_USER_ALREADY_LOGGED_IN ? 0 : rv; @@ -408,22 +412,18 @@ int pkcs11_enumerate_keys(PKCS11_TOKEN *token, unsigned int type, PKCS11_KEY ** keyp, unsigned int *countp) { PKCS11_SLOT *slot = TOKEN2SLOT(token); - PKCS11_CTX *ctx = TOKEN2CTX(token); PKCS11_TOKEN_private *tpriv = PRIVTOKEN(token); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - PKCS11_CTX_private *cpriv = PRIVCTX(ctx); PKCS11_keys *keys = (type == CKO_PRIVATE_KEY) ? &tpriv->prv : &tpriv->pub; PKCS11_KEY *first_key_prev = keys->keys; + CK_SESSION_HANDLE session; int rv; int i; - /* Make sure we have a session */ - if (!spriv->haveSession && PKCS11_open_session(slot, 0)) + if (pkcs11_get_session(slot, 0, &session)) return -1; - CRYPTO_THREAD_write_lock(cpriv->rwlock); - rv = pkcs11_find_keys(token, type); - CRYPTO_THREAD_unlock(cpriv->rwlock); + rv = pkcs11_find_keys(token, session, type); + pkcs11_put_session(slot, session); if (rv < 0) { pkcs11_destroy_keys(token, type); return -1; @@ -448,57 +448,30 @@ int pkcs11_enumerate_keys(PKCS11_TOKEN *token, unsigned int type, /** * Remove a key from the associated token */ -int pkcs11_remove_key(PKCS11_KEY *key) { +int pkcs11_remove_key(PKCS11_KEY *key) +{ PKCS11_SLOT *slot = KEY2SLOT(key); PKCS11_CTX *ctx = KEY2CTX(key); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - CK_OBJECT_HANDLE obj; - CK_ULONG count; - CK_ATTRIBUTE search_parameters[32]; - unsigned int n = 0; + CK_SESSION_HANDLE session; + PKCS11_KEY_private *kpriv = PRIVKEY(key); int rv; - /* First, make sure we have a session */ - if (!spriv->haveSession && PKCS11_open_session(slot, 1)) + if (pkcs11_get_session(slot, 1, &session)) return -1; - if (key->isPrivate) - pkcs11_addattr_int(search_parameters + n++, CKA_CLASS, CKO_PRIVATE_KEY); - else - pkcs11_addattr_int(search_parameters + n++, CKA_CLASS, CKO_PUBLIC_KEY); - if (key->id && key->id_len) - pkcs11_addattr(search_parameters + n++, CKA_ID, key->id, key->id_len); - if (key->label) - pkcs11_addattr_s(search_parameters + n++, CKA_LABEL, key->label); - rv = CRYPTOKI_call(ctx, - C_FindObjectsInit(spriv->session, search_parameters, n)); - CRYPTOKI_checkerr(CKR_F_PKCS11_REMOVE_KEY, rv); - - rv = CRYPTOKI_call(ctx, C_FindObjects(spriv->session, &obj, 1, &count)); + rv = CRYPTOKI_call(ctx, C_DestroyObject(session, kpriv->object)); + pkcs11_put_session(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_REMOVE_KEY, rv); - CRYPTOKI_call(ctx, C_FindObjectsFinal(spriv->session)); - if (count!=1) { - pkcs11_zap_attrs(search_parameters, n); - return -1; - } - rv = CRYPTOKI_call(ctx, C_DestroyObject(spriv->session, obj)); - if (rv != CKR_OK) { - pkcs11_zap_attrs(search_parameters, n); - return -1; - } - pkcs11_zap_attrs(search_parameters, n); return 0; } /* * Find all keys of a given type (public or private) */ -static int pkcs11_find_keys(PKCS11_TOKEN *token, unsigned int type) +static int pkcs11_find_keys(PKCS11_TOKEN *token, CK_SESSION_HANDLE session, unsigned int type) { - PKCS11_SLOT *slot = TOKEN2SLOT(token); PKCS11_CTX *ctx = TOKEN2CTX(token); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); CK_OBJECT_CLASS key_search_class; CK_ATTRIBUTE key_search_attrs[1] = { {CKA_CLASS, &key_search_class, sizeof(key_search_class)}, @@ -508,14 +481,14 @@ static int pkcs11_find_keys(PKCS11_TOKEN *token, unsigned int type) /* Tell the PKCS11 lib to enumerate all matching objects */ key_search_class = type; rv = CRYPTOKI_call(ctx, - C_FindObjectsInit(spriv->session, key_search_attrs, 1)); + C_FindObjectsInit(session, key_search_attrs, 1)); CRYPTOKI_checkerr(CKR_F_PKCS11_FIND_KEYS, rv); do { - res = pkcs11_next_key(ctx, token, spriv->session, type); + res = pkcs11_next_key(ctx, token, session, type); } while (res == 0); - CRYPTOKI_call(ctx, C_FindObjectsFinal(spriv->session)); + CRYPTOKI_call(ctx, C_FindObjectsFinal(session)); return (res < 0) ? -1 : 0; } diff --git a/src/p11_pkey.c b/src/p11_pkey.c index 810ef91..cf14a36 100644 --- a/src/p11_pkey.c +++ b/src/p11_pkey.c @@ -303,9 +303,8 @@ static int pkcs11_try_pkey_rsa_sign(EVP_PKEY_CTX *evp_pkey_ctx, PKCS11_SLOT *slot; PKCS11_CTX *ctx; PKCS11_KEY_private *kpriv; - PKCS11_SLOT_private *spriv; - PKCS11_CTX_private *cpriv; const EVP_MD *sig_md; + CK_SESSION_HANDLE session; CK_MECHANISM mechanism; CK_RSA_PKCS_PSS_PARAMS pss_params; @@ -331,8 +330,6 @@ static int pkcs11_try_pkey_rsa_sign(EVP_PKEY_CTX *evp_pkey_ctx, slot = KEY2SLOT(key); ctx = KEY2CTX(key); kpriv = PRIVKEY(key); - spriv = PRIVSLOT(slot); - cpriv = PRIVCTX(ctx); if (!evp_pkey_ctx) return -1; @@ -363,15 +360,17 @@ static int pkcs11_try_pkey_rsa_sign(EVP_PKEY_CTX *evp_pkey_ctx, return -1; } /* end switch(padding) */ - CRYPTO_THREAD_write_lock(cpriv->rwlock); + if (pkcs11_get_session(slot, 0, &session)) + return -1; + rv = CRYPTOKI_call(ctx, - C_SignInit(spriv->session, &mechanism, kpriv->object)); + C_SignInit(session, &mechanism, kpriv->object)); if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); + rv = pkcs11_authenticate(key, session); if (!rv) rv = CRYPTOKI_call(ctx, - C_Sign(spriv->session, (CK_BYTE_PTR)tbs, tbslen, sig, &size)); - CRYPTO_THREAD_unlock(cpriv->rwlock); + C_Sign(session, (CK_BYTE_PTR)tbs, tbslen, sig, &size)); + pkcs11_put_session(slot, session); #ifdef DEBUG fprintf(stderr, "%s:%d C_SignInit or C_Sign rv=%d\n", __FILE__, __LINE__, rv); @@ -407,8 +406,7 @@ static int pkcs11_try_pkey_rsa_decrypt(EVP_PKEY_CTX *evp_pkey_ctx, PKCS11_SLOT *slot; PKCS11_CTX *ctx; PKCS11_KEY_private *kpriv; - PKCS11_SLOT_private *spriv; - PKCS11_CTX_private *cpriv; + CK_SESSION_HANDLE session; CK_MECHANISM mechanism; CK_RSA_PKCS_OAEP_PARAMS oaep_params; @@ -434,8 +432,6 @@ static int pkcs11_try_pkey_rsa_decrypt(EVP_PKEY_CTX *evp_pkey_ctx, slot = KEY2SLOT(key); ctx = KEY2CTX(key); kpriv = PRIVKEY(key); - spriv = PRIVSLOT(slot); - cpriv = PRIVCTX(ctx); if (!evp_pkey_ctx) return -1; @@ -470,15 +466,17 @@ static int pkcs11_try_pkey_rsa_decrypt(EVP_PKEY_CTX *evp_pkey_ctx, return -1; } /* end switch(padding) */ - CRYPTO_THREAD_write_lock(cpriv->rwlock); + if (pkcs11_get_session(slot, 0, &session)) + return -1; + rv = CRYPTOKI_call(ctx, - C_DecryptInit(spriv->session, &mechanism, kpriv->object)); + C_DecryptInit(session, &mechanism, kpriv->object)); if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); + rv = pkcs11_authenticate(key, session); if (!rv) rv = CRYPTOKI_call(ctx, - C_Decrypt(spriv->session, (CK_BYTE_PTR)in, inlen, out, &size)); - CRYPTO_THREAD_unlock(cpriv->rwlock); + C_Decrypt(session, (CK_BYTE_PTR)in, inlen, out, &size)); + pkcs11_put_session(slot, session); #ifdef DEBUG fprintf(stderr, "%s:%d C_DecryptInit or C_Decrypt rv=%d\n", __FILE__, __LINE__, rv); @@ -545,8 +543,7 @@ static int pkcs11_try_pkey_ec_sign(EVP_PKEY_CTX *evp_pkey_ctx, PKCS11_SLOT *slot; PKCS11_CTX *ctx; PKCS11_KEY_private *kpriv; - PKCS11_SLOT_private *spriv; - PKCS11_CTX_private *cpriv; + CK_SESSION_HANDLE session; const EVP_MD *sig_md; ECDSA_SIG *ossl_sig; CK_MECHANISM mechanism; @@ -575,6 +572,12 @@ static int pkcs11_try_pkey_ec_sign(EVP_PKEY_CTX *evp_pkey_ctx, goto error; } + if (sig == NULL) { + *siglen = (size_t)ECDSA_size(eckey); + rv = CKR_OK; + goto error; + } + if (*siglen < (size_t)ECDSA_size(eckey)) goto error; @@ -585,8 +588,6 @@ static int pkcs11_try_pkey_ec_sign(EVP_PKEY_CTX *evp_pkey_ctx, slot = KEY2SLOT(key); ctx = KEY2CTX(key); kpriv = PRIVKEY(key); - spriv = PRIVSLOT(slot); - cpriv = PRIVCTX(ctx); if (!evp_pkey_ctx) goto error; @@ -601,15 +602,16 @@ static int pkcs11_try_pkey_ec_sign(EVP_PKEY_CTX *evp_pkey_ctx, memset(&mechanism, 0, sizeof mechanism); mechanism.mechanism = CKM_ECDSA; - CRYPTO_THREAD_write_lock(cpriv->rwlock); + if (pkcs11_get_session(slot, 0, &session)) + return -1; rv = CRYPTOKI_call(ctx, - C_SignInit(spriv->session, &mechanism, kpriv->object)); + C_SignInit(session, &mechanism, kpriv->object)); if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); + rv = pkcs11_authenticate(key, session); if (!rv) rv = CRYPTOKI_call(ctx, - C_Sign(spriv->session, (CK_BYTE_PTR)tbs, tbslen, sig, &size)); - CRYPTO_THREAD_unlock(cpriv->rwlock); + C_Sign(session, (CK_BYTE_PTR)tbs, tbslen, sig, &size)); + pkcs11_put_session(slot, session); #ifdef DEBUG fprintf(stderr, "%s:%d C_SignInit or C_Sign rv=%d\n", diff --git a/src/p11_pthread.h b/src/p11_pthread.h new file mode 100644 index 0000000..625ca96 --- /dev/null +++ b/src/p11_pthread.h @@ -0,0 +1,94 @@ +/* libp11, a simple layer on to of PKCS#11 API + * Copyright (C) 2017 Douglas E. Engert + * Copyright (C) 2017-2018 MichaƂ Trojnara + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#if defined(HAVE_PTHREAD) + +#include + +#elif defined( _WIN32) + +/* Simple wrappers for used pthread API using Windows Vista+ APIs. */ +#if _WIN32_WINNT < 0x0600 +#error Windows Vista (or Server 2008) or later required. +#endif + +#include + +typedef CRITICAL_SECTION pthread_mutex_t; +typedef void pthread_mutexattr_t; + +static int pthread_mutex_init(pthread_mutex_t *mutex, pthread_mutexattr_t *attr) +{ + (void)attr; + InitializeCriticalSection(mutex); + return 0; +} + +static int pthread_mutex_destroy(pthread_mutex_t *mutex) +{ + DeleteCriticalSection(mutex); + return 0; +} + +static int pthread_mutex_lock(pthread_mutex_t *mutex) +{ + EnterCriticalSection(mutex); + return 0; +} + +static int pthread_mutex_unlock(pthread_mutex_t *mutex) +{ + LeaveCriticalSection(mutex); + return 0; +} + +typedef CONDITION_VARIABLE pthread_cond_t; +typedef void pthread_condattr_t; + +static int pthread_cond_init(pthread_cond_t *cond, pthread_condattr_t *attr) +{ + (void)attr; + InitializeConditionVariable(cond); + return 0; +} + +static int pthread_cond_destroy(pthread_cond_t *cond) +{ + (void)cond; + return 0; +} + +static int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex) +{ + if (!SleepConditionVariableCS(cond, mutex, INFINITE)) + return 1; + return 0; +} + +static int pthread_cond_signal(pthread_cond_t *cond) +{ + WakeConditionVariable(cond); + return 0; +} + +#else + +#error Locking not supported on this platform. + +#endif diff --git a/src/p11_rsa.c b/src/p11_rsa.c index ff12ed7..83d79fd 100644 --- a/src/p11_rsa.c +++ b/src/p11_rsa.c @@ -81,9 +81,9 @@ int pkcs11_private_encrypt(int flen, PKCS11_SLOT *slot = KEY2SLOT(key); PKCS11_CTX *ctx = KEY2CTX(key); PKCS11_KEY_private *kpriv = PRIVKEY(key); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); CK_MECHANISM mechanism; CK_ULONG size; + CK_SESSION_HANDLE session; int rv; size = pkcs11_get_key_size(key); @@ -91,26 +91,28 @@ int pkcs11_private_encrypt(int flen, if (pkcs11_mechanism(&mechanism, padding) < 0) return -1; - CRYPTO_THREAD_write_lock(PRIVCTX(ctx)->rwlock); + if (pkcs11_get_session(slot, 0, &session)) + return -1; + /* Try signing first, as applications are more likely to use it */ rv = CRYPTOKI_call(ctx, - C_SignInit(spriv->session, &mechanism, kpriv->object)); + C_SignInit(session, &mechanism, kpriv->object)); if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); + rv = pkcs11_authenticate(key, session); if (!rv) rv = CRYPTOKI_call(ctx, - C_Sign(spriv->session, (CK_BYTE *)from, flen, to, &size)); + C_Sign(session, (CK_BYTE *)from, flen, to, &size)); if (rv == CKR_KEY_FUNCTION_NOT_PERMITTED) { /* OpenSSL may use it for encryption rather than signing */ rv = CRYPTOKI_call(ctx, - C_EncryptInit(spriv->session, &mechanism, kpriv->object)); + C_EncryptInit(session, &mechanism, kpriv->object)); if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); + rv = pkcs11_authenticate(key, session); if (!rv) rv = CRYPTOKI_call(ctx, - C_Encrypt(spriv->session, (CK_BYTE *)from, flen, to, &size)); + C_Encrypt(session, (CK_BYTE *)from, flen, to, &size)); } - CRYPTO_THREAD_unlock(PRIVCTX(ctx)->rwlock); + pkcs11_put_session(slot, session); if (rv) { CKRerr(CKR_F_PKCS11_PRIVATE_ENCRYPT, rv); @@ -127,7 +129,7 @@ int pkcs11_private_decrypt(int flen, const unsigned char *from, unsigned char *t PKCS11_SLOT *slot = KEY2SLOT(key); PKCS11_CTX *ctx = KEY2CTX(key); PKCS11_KEY_private *kpriv = PRIVKEY(key); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; CK_MECHANISM mechanism; CK_ULONG size = flen; CK_RV rv; @@ -135,16 +137,18 @@ int pkcs11_private_decrypt(int flen, const unsigned char *from, unsigned char *t if (pkcs11_mechanism(&mechanism, padding) < 0) return -1; - CRYPTO_THREAD_write_lock(PRIVCTX(ctx)->rwlock); + if (pkcs11_get_session(slot, 0, &session)) + return -1; + rv = CRYPTOKI_call(ctx, - C_DecryptInit(spriv->session, &mechanism, kpriv->object)); + C_DecryptInit(session, &mechanism, kpriv->object)); if (!rv && kpriv->always_authenticate == CK_TRUE) - rv = pkcs11_authenticate(key); + rv = pkcs11_authenticate(key, session); if (!rv) rv = CRYPTOKI_call(ctx, - C_Decrypt(spriv->session, (CK_BYTE *)from, size, + C_Decrypt(session, (CK_BYTE *)from, size, (CK_BYTE_PTR)to, &size)); - CRYPTO_THREAD_unlock(PRIVCTX(ctx)->rwlock); + pkcs11_put_session(slot, session); if (rv) { CKRerr(CKR_F_PKCS11_PRIVATE_DECRYPT, rv); diff --git a/src/p11_slot.c b/src/p11_slot.c index 6f08269..fe04ac3 100644 --- a/src/p11_slot.c +++ b/src/p11_slot.c @@ -149,54 +149,88 @@ PKCS11_SLOT *pkcs11_find_next_token(PKCS11_CTX *ctx, PKCS11_SLOT *slots, */ int pkcs11_open_session(PKCS11_SLOT *slot, int rw) { - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); PKCS11_CTX *ctx = SLOT2CTX(slot); - int rv; + PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - if (spriv->haveSession) { - CRYPTOKI_call(ctx, C_CloseSession(spriv->session)); - spriv->haveSession = 0; + pthread_mutex_lock(&spriv->lock); + /* If different mode requested, flush pool */ + if (rw != spriv->rw_mode) { + CRYPTOKI_call(ctx, C_CloseAllSessions(spriv->id)); + spriv->rw_mode = rw; } - rv = CRYPTOKI_call(ctx, - C_OpenSession(spriv->id, - CKF_SERIAL_SESSION | (rw ? CKF_RW_SESSION : 0), - NULL, NULL, &spriv->session)); - CRYPTOKI_checkerr(CKR_F_PKCS11_OPEN_SESSION, rv); - spriv->haveSession = 1; - spriv->prev_rw = rw; + spriv->num_sessions = 0; + spriv->session_head = spriv->session_tail = 0; + pthread_mutex_unlock(&spriv->lock); + + return 0; +} + +int pkcs11_get_session(PKCS11_SLOT * slot, int rw, CK_SESSION_HANDLE *sessionp) +{ + PKCS11_CTX *ctx = SLOT2CTX(slot); + PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + int rv = CKR_OK; + + if (rw < 0) + return -1; + + pthread_mutex_lock(&spriv->lock); + if (spriv->rw_mode < 0) + spriv->rw_mode = rw; + rw = spriv->rw_mode; + do { + /* Get session from the pool */ + if (spriv->session_head != spriv->session_tail) { + *sessionp = spriv->session_pool[spriv->session_head]; + spriv->session_head = (spriv->session_head + 1) % spriv->session_poolsize; + break; + } + + /* Check if new can be instantiated */ + if (spriv->num_sessions < spriv->max_sessions) { + rv = CRYPTOKI_call(ctx, + C_OpenSession(spriv->id, + CKF_SERIAL_SESSION | (rw ? CKF_RW_SESSION : 0), + NULL, NULL, sessionp)); + if (rv == CKR_OK) { + spriv->num_sessions++; + break; + } + + /* Remember the maximum session count */ + if (rv == CKR_SESSION_COUNT) + spriv->max_sessions = spriv->num_sessions; + } + + /* Wait for a session to become available */ + pthread_cond_wait(&spriv->cond, &spriv->lock); + } while (1); + pthread_mutex_unlock(&spriv->lock); return 0; } +void pkcs11_put_session(PKCS11_SLOT * slot, CK_SESSION_HANDLE session) +{ + PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + + pthread_mutex_lock(&spriv->lock); + + spriv->session_pool[spriv->session_tail] = session; + spriv->session_tail = (spriv->session_tail + 1) % spriv->session_poolsize; + pthread_cond_signal(&spriv->cond); + + pthread_mutex_unlock(&spriv->lock); +} + /* * Determines if user is authenticated with token */ int pkcs11_is_logged_in(PKCS11_SLOT *slot, int so, int *res) { - PKCS11_CTX *ctx = SLOT2CTX(slot); PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - CK_SESSION_INFO session_info; - int rv; - - if (spriv->loggedIn) { - *res = 1; - return 0; - } - if (!spriv->haveSession) { - /* SO gets a r/w session by default, - * user gets a r/o session by default. */ - if (PKCS11_open_session(slot, so)) - return -1; - } - rv = CRYPTOKI_call(ctx, C_GetSessionInfo(spriv->session, &session_info)); - CRYPTOKI_checkerr(CKR_F_PKCS11_IS_LOGGED_IN, rv); - if (so) { - *res = session_info.state == CKS_RW_SO_FUNCTIONS; - } else { - *res = session_info.state == CKS_RO_USER_FUNCTIONS || - session_info.state == CKS_RW_USER_FUNCTIONS; - } + *res = spriv->logged_in == so; return 0; } @@ -207,25 +241,24 @@ int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin) { PKCS11_CTX *ctx = SLOT2CTX(slot); PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; int rv; - if (spriv->loggedIn) + if (spriv->logged_in >= 0) return 0; /* Nothing to do */ - if (!spriv->haveSession) { - /* SO gets a r/w session by default, - * user gets a r/o session by default. */ - if (pkcs11_open_session(slot, so)) - return -1; - } + /* SO needs a r/w session, user can be checked with a r/o session. */ + if (pkcs11_get_session(slot, so, &session)) + return -1; rv = CRYPTOKI_call(ctx, - C_Login(spriv->session, so ? CKU_SO : CKU_USER, + C_Login(session, so ? CKU_SO : CKU_USER, (CK_UTF8CHAR *) pin, pin ? (unsigned long) strlen(pin) : 0)); - if (rv && rv != CKR_USER_ALREADY_LOGGED_IN) /* logged in -> OK */ - CRYPTOKI_checkerr(CKR_F_PKCS11_LOGIN, rv); - spriv->loggedIn = 1; + pkcs11_put_session(slot, session); + if (rv && rv != CKR_USER_ALREADY_LOGGED_IN) { /* logged in -> OK */ + CRYPTOKI_checkerr(CKR_F_PKCS11_LOGIN, rv); + } if (spriv->prev_pin != pin) { if (spriv->prev_pin) { OPENSSL_cleanse(spriv->prev_pin, strlen(spriv->prev_pin)); @@ -233,7 +266,7 @@ int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin) } spriv->prev_pin = OPENSSL_strdup(pin); } - spriv->prev_so = so; + spriv->logged_in = so; return 0; } @@ -243,15 +276,13 @@ int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin) int pkcs11_reload_slot(PKCS11_SLOT *slot) { PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + int logged_in = spriv->logged_in; - if (spriv->haveSession) { - spriv->haveSession = 0; - if (pkcs11_open_session(slot, spriv->prev_rw)) - return -1; - } - if (spriv->loggedIn) { - spriv->loggedIn = 0; - if (pkcs11_login(slot, spriv->prev_so, spriv->prev_pin)) + spriv->num_sessions = 0; + spriv->session_head = spriv->session_tail = 0; + if (logged_in >= 0) { + spriv->logged_in = -1; + if (pkcs11_login(slot, logged_in, spriv->prev_pin)) return -1; } @@ -265,7 +296,8 @@ int pkcs11_logout(PKCS11_SLOT *slot) { PKCS11_CTX *ctx = SLOT2CTX(slot); PKCS11_SLOT_private *spriv = PRIVSLOT(slot); - int rv; + CK_SESSION_HANDLE session; + int rv = CKR_OK; /* Calling PKCS11_logout invalidates all cached * keys we have */ @@ -274,14 +306,13 @@ int pkcs11_logout(PKCS11_SLOT *slot) pkcs11_destroy_keys(slot->token, CKO_PUBLIC_KEY); pkcs11_destroy_certs(slot->token); } - if (!spriv->haveSession) { - P11err(P11_F_PKCS11_LOGOUT, P11_R_NO_SESSION); - return -1; - } - rv = CRYPTOKI_call(ctx, C_Logout(spriv->session)); + if (pkcs11_get_session(slot, spriv->logged_in, &session) == 0) { + rv = CRYPTOKI_call(ctx, C_Logout(session)); + pkcs11_put_session(slot, session); + } CRYPTOKI_checkerr(CKR_F_PKCS11_LOGOUT, rv); - spriv->loggedIn = 0; + spriv->logged_in = -1; return 0; } @@ -323,16 +354,17 @@ int pkcs11_init_pin(PKCS11_TOKEN *token, const char *pin) { PKCS11_SLOT *slot = TOKEN2SLOT(token); PKCS11_CTX *ctx = SLOT2CTX(slot); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_OBJECT_HANDLE session; int len, rv; - if (!spriv->haveSession) { + if (pkcs11_get_session(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(spriv->session, (CK_UTF8CHAR *) pin, len)); + rv = CRYPTOKI_call(ctx, C_InitPIN(session, (CK_UTF8CHAR *) pin, len)); + pkcs11_put_session(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_INIT_PIN, rv); return pkcs11_check_token(ctx, TOKEN2SLOT(token)); @@ -345,10 +377,10 @@ int pkcs11_change_pin(PKCS11_SLOT *slot, const char *old_pin, const char *new_pin) { PKCS11_CTX *ctx = SLOT2CTX(slot); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; int old_len, new_len, rv; - if (!spriv->haveSession) { + if (pkcs11_get_session(slot, 1, &session)) { P11err(P11_F_PKCS11_CHANGE_PIN, P11_R_NO_SESSION); return -1; } @@ -356,8 +388,9 @@ int pkcs11_change_pin(PKCS11_SLOT *slot, const char *old_pin, old_len = old_pin ? (int) strlen(old_pin) : 0; new_len = new_pin ? (int) strlen(new_pin) : 0; rv = CRYPTOKI_call(ctx, - C_SetPIN(spriv->session, (CK_UTF8CHAR *) old_pin, old_len, + C_SetPIN(session, (CK_UTF8CHAR *) old_pin, old_len, (CK_UTF8CHAR *) new_pin, new_len)); + pkcs11_put_session(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_CHANGE_PIN, rv); return pkcs11_check_token(ctx, slot); @@ -370,16 +403,17 @@ int pkcs11_seed_random(PKCS11_SLOT *slot, const unsigned char *s, unsigned int s_len) { PKCS11_CTX *ctx = SLOT2CTX(slot); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; int rv; - if (!spriv->haveSession && PKCS11_open_session(slot, 0)) { + if (pkcs11_get_session(slot, 0, &session)) { P11err(P11_F_PKCS11_SEED_RANDOM, P11_R_NO_SESSION); return -1; } rv = CRYPTOKI_call(ctx, - C_SeedRandom(spriv->session, (CK_BYTE_PTR) s, s_len)); + C_SeedRandom(session, (CK_BYTE_PTR) s, s_len)); + pkcs11_put_session(slot, session); CRYPTOKI_checkerr(CKR_F_PKCS11_SEED_RANDOM, rv); return pkcs11_check_token(ctx, slot); @@ -392,16 +426,18 @@ int pkcs11_generate_random(PKCS11_SLOT *slot, unsigned char *r, unsigned int r_len) { PKCS11_CTX *ctx = SLOT2CTX(slot); - PKCS11_SLOT_private *spriv = PRIVSLOT(slot); + CK_SESSION_HANDLE session; int rv; - if (!spriv->haveSession && PKCS11_open_session(slot, 0)) { + if (pkcs11_get_session(slot, 0, &session)) { P11err(P11_F_PKCS11_GENERATE_RANDOM, P11_R_NO_SESSION); return -1; } rv = CRYPTOKI_call(ctx, - C_GenerateRandom(spriv->session, (CK_BYTE_PTR) r, r_len)); + C_GenerateRandom(session, (CK_BYTE_PTR) r, r_len)); + pkcs11_put_session(slot, session); + CRYPTOKI_checkerr(CKR_F_PKCS11_GENERATE_RANDOM, rv); return pkcs11_check_token(ctx, slot); @@ -427,9 +463,14 @@ static int pkcs11_init_slot(PKCS11_CTX *ctx, PKCS11_SLOT *slot, CK_SLOT_ID id) spriv->parent = ctx; spriv->id = id; spriv->forkid = PRIVCTX(ctx)->forkid; - spriv->prev_rw = 0; spriv->prev_pin = NULL; - spriv->prev_so = 0; + spriv->logged_in = -1; + spriv->rw_mode = -1; + spriv->max_sessions = 16; + spriv->session_poolsize = spriv->max_sessions + 1; + spriv->session_pool = OPENSSL_malloc(spriv->session_poolsize * sizeof(CK_SESSION_HANDLE)); + pthread_mutex_init(&spriv->lock, 0); + pthread_cond_init(&spriv->cond, 0); slot->description = PKCS11_DUP(info.slotDescription); slot->manufacturer = PKCS11_DUP(info.manufacturerID); @@ -462,6 +503,9 @@ static void pkcs11_release_slot(PKCS11_CTX *ctx, PKCS11_SLOT *slot) OPENSSL_free(spriv->prev_pin); } CRYPTOKI_call(ctx, C_CloseAllSessions(spriv->id)); + OPENSSL_free(spriv->session_pool); + pthread_mutex_destroy(&spriv->lock); + pthread_cond_destroy(&spriv->cond); } OPENSSL_free(slot->_private); OPENSSL_free(slot->description); From c20f2b719df52cf472cd3709a60943909ffa65ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Mon, 22 Mar 2021 09:55:13 +0200 Subject: [PATCH 10/12] Replace the context rwlock with a mutex The only remaining user for this is the after-fork code, so rename the lock also as fork_lock. --- src/libp11-int.h | 2 +- src/p11_atfork.c | 4 ++-- src/p11_load.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index 2eb7703..a70c248 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -49,7 +49,7 @@ typedef struct pkcs11_ctx_private { UI_METHOD *ui_method; /* UI_METHOD for CKU_CONTEXT_SPECIFIC PINs */ void *ui_user_data; unsigned int forkid; - PKCS11_RWLOCK rwlock; + pthread_mutex_t fork_lock; } PKCS11_CTX_private; #define PRIVCTX(ctx) ((PKCS11_CTX_private *) ((ctx)->_private)) diff --git a/src/p11_atfork.c b/src/p11_atfork.c index af6e709..224b189 100644 --- a/src/p11_atfork.c +++ b/src/p11_atfork.c @@ -65,9 +65,9 @@ static unsigned int _P11_update_forkid(void) int rv = 0; \ _P11_update_forkid(); \ if (forkid != P11_forkid) { \ - CRYPTO_THREAD_write_lock(PRIVCTX(ctx)->rwlock); \ + pthread_mutex_lock(&PRIVCTX(ctx)->fork_lock); \ function_call; \ - CRYPTO_THREAD_unlock(PRIVCTX(ctx)->rwlock); \ + pthread_mutex_unlock(&PRIVCTX(ctx)->fork_lock); \ } \ return rv; \ } while (0) diff --git a/src/p11_load.c b/src/p11_load.c index 0704271..97b10f2 100644 --- a/src/p11_load.c +++ b/src/p11_load.c @@ -40,7 +40,7 @@ PKCS11_CTX *pkcs11_CTX_new(void) memset(ctx, 0, sizeof(PKCS11_CTX)); ctx->_private = cpriv; cpriv->forkid = get_forkid(); - cpriv->rwlock = CRYPTO_THREAD_lock_new(); + pthread_mutex_init(&cpriv->fork_lock, 0); return ctx; fail: @@ -170,7 +170,7 @@ void pkcs11_CTX_free(PKCS11_CTX *ctx) if (cpriv->handle) { OPENSSL_free(cpriv->handle); } - CRYPTO_THREAD_lock_free(cpriv->rwlock); + pthread_mutex_destroy(&cpriv->fork_lock); OPENSSL_free(ctx->manufacturer); OPENSSL_free(ctx->description); OPENSSL_free(ctx->_private); From 5b1c61ba713ce0ceca3e89674697f5890513d584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Mon, 22 Mar 2021 10:31:38 +0200 Subject: [PATCH 11/12] Convert and fix engine context locking Optimize also ctx_init_libp11 to elide locking if the one time intialization is completed already. Basic locking to key and certificate loading is added, because the underlying functions they use are not thread safe. Also remove the old compat functions which are no longer needed. add simple locking --- src/eng_back.c | 84 ++++++++++++++++++------------------------------ src/libp11-int.h | 20 ------------ src/p11_misc.c | 28 ---------------- 3 files changed, 31 insertions(+), 101 deletions(-) diff --git a/src/eng_back.c b/src/eng_back.c index 37ee076..b4004f0 100644 --- a/src/eng_back.c +++ b/src/eng_back.c @@ -27,6 +27,7 @@ */ #include "engine.h" +#include "p11_pthread.h" #include #include @@ -54,13 +55,7 @@ struct st_engine_ctx { UI_METHOD *ui_method; void *callback_data; int force_login; - - /* Engine initialization mutex */ -#if OPENSSL_VERSION_NUMBER >= 0x10100004L && !defined(LIBRESSL_VERSION_NUMBER) - CRYPTO_RWLOCK *rwlock; -#else - int rwlock; -#endif + pthread_mutex_t lock; /* Current operations */ PKCS11_CTX *pkcs11_ctx; @@ -224,6 +219,7 @@ ENGINE_CTX *ctx_new() if (!ctx) return NULL; memset(ctx, 0, sizeof(ENGINE_CTX)); + pthread_mutex_init(&ctx->lock, 0); mod = getenv("PKCS11_MODULE_PATH"); if (mod) { @@ -236,13 +232,6 @@ ENGINE_CTX *ctx_new() #endif } -#if OPENSSL_VERSION_NUMBER >= 0x10100004L && !defined(LIBRESSL_VERSION_NUMBER) - ctx->rwlock = CRYPTO_THREAD_lock_new(); -#else - ctx->rwlock = CRYPTO_get_dynlock_create_callback() ? - CRYPTO_get_new_dynlockid() : 0; -#endif - return ctx; } @@ -253,12 +242,7 @@ int ctx_destroy(ENGINE_CTX *ctx) ctx_destroy_pin(ctx); OPENSSL_free(ctx->module); OPENSSL_free(ctx->init_args); -#if OPENSSL_VERSION_NUMBER >= 0x10100004L && !defined(LIBRESSL_VERSION_NUMBER) - CRYPTO_THREAD_lock_free(ctx->rwlock); -#else - if (ctx->rwlock) - CRYPTO_destroy_dynlockid(ctx->rwlock); -#endif + pthread_mutex_destroy(&ctx->lock); OPENSSL_free(ctx); } return 1; @@ -302,20 +286,14 @@ static void ctx_init_libp11_unlocked(ENGINE_CTX *ctx) static int ctx_init_libp11(ENGINE_CTX *ctx) { -#if OPENSSL_VERSION_NUMBER >= 0x10100004L && !defined(LIBRESSL_VERSION_NUMBER) - CRYPTO_THREAD_write_lock(ctx->rwlock); -#else - if (ctx->rwlock) - CRYPTO_w_lock(ctx->rwlock); -#endif - if (!ctx->pkcs11_ctx|| !ctx->slot_list) + if (ctx->pkcs11_ctx && ctx->slot_list) + return 0; + + pthread_mutex_lock(&ctx->lock); + if (!ctx->pkcs11_ctx || !ctx->slot_list) ctx_init_libp11_unlocked(ctx); -#if OPENSSL_VERSION_NUMBER >= 0x10100004L && !defined(LIBRESSL_VERSION_NUMBER) - CRYPTO_THREAD_unlock(ctx->rwlock); -#else - if (ctx->rwlock) - CRYPTO_w_unlock(ctx->rwlock); -#endif + pthread_mutex_unlock(&ctx->lock); + return ctx->pkcs11_ctx && ctx->slot_list ? 0 : -1; } @@ -327,21 +305,7 @@ int ctx_init(ENGINE_CTX *ctx) * Double-locking a non-recursive rwlock causes the application to * crash or hang, depending on the locking library implementation. */ - /* Only attempt initialization when dynamic locks are unavailable. - * This likely also indicates a single-threaded application, - * so temporarily unlocking CRYPTO_LOCK_ENGINE should be safe. */ -#if OPENSSL_VERSION_NUMBER < 0x10100004L && !defined(LIBRESSL_VERSION_NUMBER) - if (CRYPTO_get_dynlock_create_callback() == NULL || - CRYPTO_get_dynlock_lock_callback() == NULL || - CRYPTO_get_dynlock_destroy_callback() == NULL) { - CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE); - ctx_init_libp11_unlocked(ctx); - CRYPTO_w_lock(CRYPTO_LOCK_ENGINE); - return ctx->pkcs11_ctx && ctx->slot_list ? 1 : 0; - } -#else (void)ctx; /* squash the unused parameter warning */ -#endif return 1; } @@ -389,9 +353,6 @@ static X509 *ctx_load_cert(ENGINE_CTX *ctx, const char *s_slot_cert_id, char flags[64]; size_t matched_count = 0; - if (ctx_init_libp11(ctx)) /* Delayed libp11 initialization */ - return NULL; - if (s_slot_cert_id && *s_slot_cert_id) { if (!strncasecmp(s_slot_cert_id, "pkcs11:", 7)) { n = parse_pkcs11_uri(ctx, s_slot_cert_id, &match_tok, @@ -640,13 +601,21 @@ static int ctx_ctrl_load_cert(ENGINE_CTX *ctx, void *p) ENGerr(ENG_F_CTX_CTRL_LOAD_CERT, ENG_R_INVALID_PARAMETER); return 0; } + + if (ctx_init_libp11(ctx)) { /* Delayed libp11 initialization */ + ENGerr(ENG_F_CTX_CTRL_LOAD_CERT, ENG_R_INVALID_PARAMETER); + return 0; + } + ERR_clear_error(); + pthread_mutex_lock(&ctx->lock); if (!ctx->force_login) parms->cert = ctx_load_cert(ctx, parms->s_slot_cert_id, 0); if (!parms->cert) { /* Try again with login */ ERR_clear_error(); parms->cert = ctx_load_cert(ctx, parms->s_slot_cert_id, 1); } + pthread_mutex_unlock(&ctx->lock); if (!parms->cert) { if (!ERR_peek_last_error()) ENGerr(ENG_F_CTX_CTRL_LOAD_CERT, ENG_R_OBJECT_NOT_FOUND); @@ -678,9 +647,6 @@ static EVP_PKEY *ctx_load_key(ENGINE_CTX *ctx, const char *s_slot_key_id, char flags[64]; size_t matched_count = 0; - if (ctx_init_libp11(ctx)) /* Delayed libp11 initialization */ - goto error; - ctx_log(ctx, 1, "Loading %s key \"%s\"\n", (char *)(isPrivate ? "private" : "public"), s_slot_key_id); @@ -941,12 +907,18 @@ EVP_PKEY *ctx_load_pubkey(ENGINE_CTX *ctx, const char *s_key_id, EVP_PKEY *pk = NULL; ERR_clear_error(); + + if (ctx_init_libp11(ctx)) /* Delayed libp11 initialization */ + return NULL; + + pthread_mutex_lock(&ctx->lock); if (!ctx->force_login) pk = ctx_load_key(ctx, s_key_id, ui_method, callback_data, 0, 0); if (!pk) { /* Try again with login */ ERR_clear_error(); pk = ctx_load_key(ctx, s_key_id, ui_method, callback_data, 0, 1); } + pthread_mutex_unlock(&ctx->lock); if (!pk) { ctx_log(ctx, 0, "PKCS11_load_public_key returned NULL\n"); if (!ERR_peek_last_error()) @@ -962,12 +934,18 @@ EVP_PKEY *ctx_load_privkey(ENGINE_CTX *ctx, const char *s_key_id, EVP_PKEY *pk = NULL; ERR_clear_error(); + + if (ctx_init_libp11(ctx)) /* Delayed libp11 initialization */ + return NULL; + + pthread_mutex_lock(&ctx->lock); if (!ctx->force_login) pk = ctx_load_key(ctx, s_key_id, ui_method, callback_data, 1, 0); if (!pk) { /* Try again with login */ ERR_clear_error(); pk = ctx_load_key(ctx, s_key_id, ui_method, callback_data, 1, 1); } + pthread_mutex_unlock(&ctx->lock); if (!pk) { ctx_log(ctx, 0, "PKCS11_get_private_key returned NULL\n"); if (!ERR_peek_last_error()) diff --git a/src/libp11-int.h b/src/libp11-int.h index a70c248..d05665c 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -31,12 +31,6 @@ #include "p11_pthread.h" -#if OPENSSL_VERSION_NUMBER < 0x10100004L || defined(LIBRESSL_VERSION_NUMBER) -typedef int PKCS11_RWLOCK; -#else -typedef CRYPTO_RWLOCK *PKCS11_RWLOCK; -#endif - /* get private implementations of PKCS11 structures */ /* @@ -140,20 +134,6 @@ extern int ERR_load_CKR_strings(void); pkcs11_strdup((char *) s, sizeof(s)) extern char *pkcs11_strdup(char *, size_t); -/* Emulate the OpenSSL 1.1 locking API for older OpenSSL versions */ -#if OPENSSL_VERSION_NUMBER < 0x10100004L || defined(LIBRESSL_VERSION_NUMBER) -int CRYPTO_THREAD_lock_new(); -void CRYPTO_THREAD_lock_free(int); -#define CRYPTO_THREAD_write_lock(type) \ - if(type) CRYPTO_lock(CRYPTO_LOCK|CRYPTO_WRITE,type,__FILE__,__LINE__) -#define CRYPTO_THREAD_unlock(type) \ - if(type) CRYPTO_lock(CRYPTO_UNLOCK|CRYPTO_WRITE,type,__FILE__,__LINE__) -#define CRYPTO_THREAD_read_lock(type) \ - if(type) CRYPTO_lock(CRYPTO_LOCK|CRYPTO_READ,type,__FILE__,__LINE__) -#define CRYPTO_THREAD_read_unlock(type) \ - if(type) CRYPTO_lock(CRYPTO_UNLOCK|CRYPTO_READ,type,__FILE__,__LINE__) -#endif - /* Emulate the OpenSSL 1.1 getters */ #if OPENSSL_VERSION_NUMBER < 0x10100003L || defined(LIBRESSL_VERSION_NUMBER) #define EVP_PKEY_get0_RSA(key) ((key)->pkey.rsa) diff --git a/src/p11_misc.c b/src/p11_misc.c index d871f9c..7458d74 100644 --- a/src/p11_misc.c +++ b/src/p11_misc.c @@ -39,32 +39,4 @@ char *pkcs11_strdup(char *mem, size_t size) return res; } -/* - * CRYPTO dynlock wrappers: 0 is an invalid dynamic lock ID - */ - -#if OPENSSL_VERSION_NUMBER < 0x10100004L || defined(LIBRESSL_VERSION_NUMBER) - -int CRYPTO_THREAD_lock_new() -{ - int i; - - if (CRYPTO_get_dynlock_create_callback() == NULL || - CRYPTO_get_dynlock_lock_callback() == NULL || - CRYPTO_get_dynlock_destroy_callback() == NULL) - return 0; /* Dynamic callbacks not set */ - i = CRYPTO_get_new_dynlockid(); - if (i == 0) - ERR_clear_error(); /* Dynamic locks are optional -> ignore */ - return i; -} - -void CRYPTO_THREAD_lock_free(int i) -{ - if (i) - CRYPTO_destroy_dynlockid(i); -} - -#endif - /* vim: set noexpandtab: */ From b750e5fa91bce4286bd87d0654f9256a1d761178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Mon, 22 Mar 2021 10:50:58 +0200 Subject: [PATCH 12/12] Refactor attribute getting to include session handle This avoids one thread to get two sessions from the pool, and speeds up operation as locking is not needed to get the attribute. --- src/libp11-int.h | 32 ++++++++------------------------ src/p11_attr.c | 44 ++++++++++++++++---------------------------- src/p11_cert.c | 10 +++++----- src/p11_ec.c | 26 +++++++++++++++++--------- src/p11_key.c | 22 +++++++++++----------- src/p11_rsa.c | 23 +++++++++++++++++------ 6 files changed, 74 insertions(+), 83 deletions(-) diff --git a/src/libp11-int.h b/src/libp11-int.h index d05665c..f743eb9 100644 --- a/src/libp11-int.h +++ b/src/libp11-int.h @@ -158,34 +158,18 @@ extern int pkcs11_reload_certificate(PKCS11_CERT *cert); extern int pkcs11_reload_slot(PKCS11_SLOT * slot); /* Managing object attributes */ -extern int pkcs11_getattr_var(PKCS11_TOKEN *, CK_OBJECT_HANDLE, - unsigned int, CK_BYTE *, size_t *); -extern int pkcs11_getattr_val(PKCS11_TOKEN *, CK_OBJECT_HANDLE, - unsigned int, void *, size_t); -extern int pkcs11_getattr_alloc(PKCS11_TOKEN *, CK_OBJECT_HANDLE, - unsigned int, CK_BYTE **, size_t *); +extern int pkcs11_getattr_var(PKCS11_CTX *, CK_SESSION_HANDLE, CK_OBJECT_HANDLE, + CK_ATTRIBUTE_TYPE, CK_BYTE *, size_t *); +extern int pkcs11_getattr_val(PKCS11_CTX *, CK_SESSION_HANDLE, CK_OBJECT_HANDLE, + CK_ATTRIBUTE_TYPE, void *, size_t); +extern int pkcs11_getattr_alloc(PKCS11_CTX *, CK_SESSION_HANDLE, CK_OBJECT_HANDLE, + CK_ATTRIBUTE_TYPE, CK_BYTE **, size_t *); /* * Caution: the BIGNUM ** shall reference either a NULL pointer or a * pointer to a valid BIGNUM. */ -extern int pkcs11_getattr_bn(PKCS11_TOKEN *, CK_OBJECT_HANDLE, - unsigned int, BIGNUM **); - -#define key_getattr_var(key, t, p, s) \ - pkcs11_getattr_var(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (p), (s)) - -#define key_getattr_val(key, t, p, s) \ - pkcs11_getattr_val(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (p), (s)) - -#define key_getattr_alloc(key, t, p, s) \ - pkcs11_getattr_alloc(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (p), (s)) - -/* - * Caution: bn shall reference either a NULL pointer or a pointer to - * a valid BIGNUM. - */ -#define key_getattr_bn(key, t, bn) \ - pkcs11_getattr_bn(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (bn)) +extern int pkcs11_getattr_bn(PKCS11_CTX *, CK_SESSION_HANDLE, CK_OBJECT_HANDLE, + CK_ATTRIBUTE_TYPE, BIGNUM **); typedef int (*pkcs11_i2d_fn) (void *, unsigned char **); extern void pkcs11_addattr(CK_ATTRIBUTE_PTR, int, const void *, size_t); diff --git a/src/p11_attr.c b/src/p11_attr.c index d261c04..7c0fa93 100644 --- a/src/p11_attr.c +++ b/src/p11_attr.c @@ -33,49 +33,37 @@ /* * Query pkcs11 attributes */ -static int pkcs11_getattr_int(PKCS11_TOKEN *token, CK_OBJECT_HANDLE o, - CK_ATTRIBUTE_TYPE type, CK_BYTE *value, size_t *size) +int pkcs11_getattr_var(PKCS11_CTX *ctx, CK_SESSION_HANDLE session, + CK_OBJECT_HANDLE object, CK_ATTRIBUTE_TYPE type, + CK_BYTE *value, size_t *size) { - PKCS11_SLOT *slot = TOKEN2SLOT(token); - PKCS11_CTX *ctx = SLOT2CTX(slot); CK_ATTRIBUTE templ; - CK_SESSION_HANDLE session; int rv; templ.type = type; templ.pValue = value; templ.ulValueLen = *size; - - if (pkcs11_get_session(slot, 0, &session)) - return -1; - - rv = CRYPTOKI_call(ctx, C_GetAttributeValue(session, o, &templ, 1)); - pkcs11_put_session(slot, session); - + rv = CRYPTOKI_call(ctx, C_GetAttributeValue(session, object, &templ, 1)); CRYPTOKI_checkerr(CKR_F_PKCS11_GETATTR_INT, rv); *size = templ.ulValueLen; return 0; } -int pkcs11_getattr_var(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object, - unsigned int type, CK_BYTE *value, size_t *size) -{ - return pkcs11_getattr_int(token, object, type, value, size); -} - -int pkcs11_getattr_val(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object, - unsigned int type, void *value, size_t size) +int pkcs11_getattr_val(PKCS11_CTX *ctx, CK_SESSION_HANDLE session, + CK_OBJECT_HANDLE object, CK_ATTRIBUTE_TYPE type, + void *value, size_t size) { - return pkcs11_getattr_var(token, object, type, value, &size); + return pkcs11_getattr_var(ctx, session, object, type, value, &size); } -int pkcs11_getattr_alloc(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object, - unsigned int type, CK_BYTE **value, size_t *size) +int pkcs11_getattr_alloc(PKCS11_CTX *ctx, CK_SESSION_HANDLE session, + CK_OBJECT_HANDLE object, CK_ATTRIBUTE_TYPE type, + CK_BYTE **value, size_t *size) { CK_BYTE *data; size_t len = 0; - if (pkcs11_getattr_var(token, object, type, NULL, &len)) + if (pkcs11_getattr_var(ctx, session, object, type, NULL, &len)) return -1; data = OPENSSL_malloc(len+1); if (!data) { @@ -83,7 +71,7 @@ int pkcs11_getattr_alloc(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object, return -1; } memset(data, 0, len+1); /* also null-terminate the allocated data */ - if (pkcs11_getattr_var(token, object, type, data, &len)) { + if (pkcs11_getattr_var(ctx, session, object, type, data, &len)) { OPENSSL_free(data); return -1; } @@ -94,14 +82,14 @@ int pkcs11_getattr_alloc(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object, return 0; } -int pkcs11_getattr_bn(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object, - unsigned int type, BIGNUM **bn) +int pkcs11_getattr_bn(PKCS11_CTX *ctx, CK_SESSION_HANDLE session, + CK_OBJECT_HANDLE object, CK_ATTRIBUTE_TYPE type, BIGNUM **bn) { CK_BYTE *binary; size_t size; size = 0; - if (pkcs11_getattr_alloc(token, object, type, &binary, &size)) + if (pkcs11_getattr_alloc(ctx, session, object, type, &binary, &size)) return -1; /* * @ALON: invalid object, diff --git a/src/p11_cert.c b/src/p11_cert.c index 024c03f..5cc5333 100644 --- a/src/p11_cert.c +++ b/src/p11_cert.c @@ -165,7 +165,7 @@ static int pkcs11_init_cert(PKCS11_CTX *ctx, PKCS11_TOKEN *token, /* Ignore unknown certificate types */ size = sizeof(CK_CERTIFICATE_TYPE); - if (pkcs11_getattr_var(token, obj, CKA_CERTIFICATE_TYPE, (CK_BYTE *)&cert_type, &size)) + if (pkcs11_getattr_var(ctx, session, obj, CKA_CERTIFICATE_TYPE, (CK_BYTE *)&cert_type, &size)) return -1; if (cert_type != CKC_X_509) return 0; @@ -192,23 +192,23 @@ static int pkcs11_init_cert(PKCS11_CTX *ctx, PKCS11_TOKEN *token, memset(cert, 0, sizeof(PKCS11_CERT)); /* Fill public properties */ - pkcs11_getattr_alloc(token, obj, CKA_LABEL, (CK_BYTE **)&cert->label, NULL); + pkcs11_getattr_alloc(ctx, session, obj, CKA_LABEL, (CK_BYTE **)&cert->label, NULL); size = 0; - if (!pkcs11_getattr_alloc(token, obj, CKA_VALUE, &data, &size)) { + if (!pkcs11_getattr_alloc(ctx, session, obj, CKA_VALUE, &data, &size)) { const unsigned char *p = data; cert->x509 = d2i_X509(NULL, &p, (long)size); OPENSSL_free(data); } cert->id_len = 0; - pkcs11_getattr_alloc(token, obj, CKA_ID, &cert->id, &cert->id_len); + pkcs11_getattr_alloc(ctx, session, obj, CKA_ID, &cert->id, &cert->id_len); /* Fill private properties */ cert->_private = cpriv; cpriv->object = obj; cpriv->parent = token; cpriv->id_len = sizeof cpriv->id; - if (pkcs11_getattr_var(token, obj, CKA_ID, cpriv->id, &cpriv->id_len)) + if (pkcs11_getattr_var(ctx, session, obj, CKA_ID, cpriv->id, &cpriv->id_len)) cpriv->id_len = 0; if (ret) diff --git a/src/p11_ec.c b/src/p11_ec.c index 0fddff7..294cbad 100644 --- a/src/p11_ec.c +++ b/src/p11_ec.c @@ -187,14 +187,15 @@ static void free_ec_ex_index() /* Retrieve EC parameters from key into ec * return nonzero on error */ -static int pkcs11_get_params(EC_KEY *ec, PKCS11_KEY *key) +static int pkcs11_get_params(EC_KEY *ec, PKCS11_KEY *key, CK_SESSION_HANDLE session) { CK_BYTE *params; size_t params_len = 0; const unsigned char *a; int rv; - if (key_getattr_alloc(key, CKA_EC_PARAMS, ¶ms, ¶ms_len)) + if (pkcs11_getattr_alloc(KEY2CTX(key), session, PRIVKEY(key)->object, + CKA_EC_PARAMS, ¶ms, ¶ms_len)) return -1; a = params; @@ -205,7 +206,7 @@ static int pkcs11_get_params(EC_KEY *ec, PKCS11_KEY *key) /* Retrieve EC point from key into ec * return nonzero on error */ -static int pkcs11_get_point_key(EC_KEY *ec, PKCS11_KEY *key) +static int pkcs11_get_point_key(EC_KEY *ec, PKCS11_KEY *key, CK_SESSION_HANDLE session) { CK_BYTE *point; size_t point_len = 0; @@ -213,7 +214,8 @@ static int pkcs11_get_point_key(EC_KEY *ec, PKCS11_KEY *key) ASN1_OCTET_STRING *os; int rv = -1; - if (!key || key_getattr_alloc(key, CKA_EC_POINT, &point, &point_len)) + if (!key || pkcs11_getattr_alloc(KEY2CTX(key), session, PRIVKEY(key)->object, + CKA_EC_POINT, &point, &point_len)) return -1; /* PKCS#11-compliant modules should return ASN1_OCTET_STRING */ @@ -268,6 +270,8 @@ static int pkcs11_get_point_cert(EC_KEY *ec, PKCS11_CERT *cert) static EC_KEY *pkcs11_get_ec(PKCS11_KEY *key) { + PKCS11_SLOT *slot = KEY2SLOT(key); + CK_SESSION_HANDLE session; EC_KEY *ec; int no_params, no_point; @@ -280,12 +284,17 @@ static EC_KEY *pkcs11_get_ec(PKCS11_KEY *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 */ - no_params = pkcs11_get_params(ec, key); - no_point = pkcs11_get_point_key(ec, key); + if (pkcs11_get_session(slot, 0, &session)) { + EC_KEY_free(ec); + return NULL; + } + no_params = pkcs11_get_params(ec, key, session); + no_point = pkcs11_get_point_key(ec, key, session); if (no_point && key->isPrivate) /* Retry with the public key */ - no_point = pkcs11_get_point_key(ec, pkcs11_find_key_from_key(key)); + no_point = pkcs11_get_point_key(ec, pkcs11_find_key_from_key(key), session); if (no_point && key->isPrivate) /* Retry with the certificate */ no_point = pkcs11_get_point_cert(ec, pkcs11_find_certificate(key)); + pkcs11_put_session(slot, session); if (key->isPrivate && EC_KEY_get0_private_key(ec) == NULL) { BIGNUM *bn = BN_new(); @@ -544,7 +553,6 @@ static int pkcs11_ecdh_derive(unsigned char **out, size_t *outlen, { PKCS11_SLOT *slot = KEY2SLOT(key); PKCS11_CTX *ctx = KEY2CTX(key); - PKCS11_TOKEN *token = KEY2TOKEN(key); PKCS11_KEY_private *kpriv = PRIVKEY(key); CK_SESSION_HANDLE session; CK_MECHANISM mechanism; @@ -597,7 +605,7 @@ static int pkcs11_ecdh_derive(unsigned char **out, size_t *outlen, /* Return the value of the secret key and/or the object handle of the secret key */ if (out && outlen) { /* pkcs11_ec_ckey only asks for the value */ - if (pkcs11_getattr_alloc(token, newkey, CKA_VALUE, out, outlen)) { + if (pkcs11_getattr_alloc(ctx, session, newkey, CKA_VALUE, out, outlen)) { CRYPTOKI_call(ctx, C_DestroyObject(session, newkey)); goto error; } diff --git a/src/p11_key.c b/src/p11_key.c index d42386b..494520f 100644 --- a/src/p11_key.c +++ b/src/p11_key.c @@ -333,13 +333,6 @@ EVP_PKEY *pkcs11_get_key(PKCS11_KEY *key, int isPrivate) key->evp_key = kpriv->ops->get_evp_key(key); if (!key->evp_key) return NULL; - kpriv->always_authenticate = CK_FALSE; - if (isPrivate && key_getattr_val(key, CKA_ALWAYS_AUTHENTICATE, - &kpriv->always_authenticate, sizeof(CK_BBOOL))) { -#ifdef DEBUG - fprintf(stderr, "Missing CKA_ALWAYS_AUTHENTICATE attribute\n"); -#endif - } } #if OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER) EVP_PKEY_up_ref(key->evp_key); @@ -531,7 +524,7 @@ static int pkcs11_init_key(PKCS11_CTX *ctx, PKCS11_TOKEN *token, /* Ignore unknown key types */ size = sizeof(CK_KEY_TYPE); - if (pkcs11_getattr_var(token, obj, CKA_KEY_TYPE, (CK_BYTE *)&key_type, &size)) + if (pkcs11_getattr_var(ctx, session, obj, CKA_KEY_TYPE, (CK_BYTE *)&key_type, &size)) return -1; switch (key_type) { case CKK_RSA: @@ -567,17 +560,24 @@ static int pkcs11_init_key(PKCS11_CTX *ctx, PKCS11_TOKEN *token, memset(key, 0, sizeof(PKCS11_KEY)); /* Fill public properties */ - pkcs11_getattr_alloc(token, obj, CKA_LABEL, (CK_BYTE **)&key->label, NULL); + pkcs11_getattr_alloc(ctx, session, obj, CKA_LABEL, (CK_BYTE **)&key->label, NULL); key->id_len = 0; - pkcs11_getattr_alloc(token, obj, CKA_ID, &key->id, &key->id_len); + pkcs11_getattr_alloc(ctx, session, obj, CKA_ID, &key->id, &key->id_len); key->isPrivate = (type == CKO_PRIVATE_KEY); + if (key->isPrivate && pkcs11_getattr_val(ctx, session, obj, + CKA_ALWAYS_AUTHENTICATE, + &kpriv->always_authenticate, sizeof(CK_BBOOL))) { +#ifdef DEBUG + fprintf(stderr, "Missing CKA_ALWAYS_AUTHENTICATE attribute\n"); +#endif + } /* Fill private properties */ key->_private = kpriv; kpriv->object = obj; kpriv->parent = token; kpriv->id_len = sizeof kpriv->id; - if (pkcs11_getattr_var(token, obj, CKA_ID, kpriv->id, &kpriv->id_len)) + if (pkcs11_getattr_var(ctx, session, obj, CKA_ID, kpriv->id, &kpriv->id_len)) kpriv->id_len = 0; kpriv->ops = ops; kpriv->forkid = get_forkid(); diff --git a/src/p11_rsa.c b/src/p11_rsa.c index 83d79fd..f2f3eb3 100644 --- a/src/p11_rsa.c +++ b/src/p11_rsa.c @@ -179,17 +179,24 @@ int pkcs11_verify(int type, const unsigned char *m, unsigned int m_len, */ static RSA *pkcs11_get_rsa(PKCS11_KEY *key) { - RSA *rsa; + PKCS11_CTX *ctx = KEY2CTX(key); + PKCS11_SLOT *slot = KEY2SLOT(key); PKCS11_KEY *keys; + CK_OBJECT_HANDLE object = PRIVKEY(key)->object; + CK_SESSION_HANDLE session; + RSA *rsa; unsigned int i, count; BIGNUM *rsa_n = NULL, *rsa_e = NULL; - /* Retrieve the modulus */ - if (key_getattr_bn(key, CKA_MODULUS, &rsa_n)) + if (pkcs11_get_session(slot, 0, &session)) return NULL; + /* Retrieve the modulus */ + if (pkcs11_getattr_bn(ctx, session, object, CKA_MODULUS, &rsa_n)) + goto failure; + /* Retrieve the public exponent */ - if (!key_getattr_bn(key, CKA_PUBLIC_EXPONENT, &rsa_e)) { + if (!pkcs11_getattr_bn(ctx, session, object, CKA_PUBLIC_EXPONENT, &rsa_e)) { if (!BN_is_zero(rsa_e)) /* A valid public exponent */ goto success; BN_clear_free(rsa_e); @@ -201,10 +208,12 @@ static RSA *pkcs11_get_rsa(PKCS11_KEY *key) if (!PKCS11_enumerate_public_keys(KEY2TOKEN(key), &keys, &count)) { for (i = 0; i < count; i++) { BIGNUM *pubmod = NULL; - if (!key_getattr_bn(&keys[i], CKA_MODULUS, &pubmod)) { + if (!pkcs11_getattr_bn(ctx, session, PRIVKEY(&keys[i])->object, + CKA_MODULUS, &pubmod)) { int found = BN_cmp(rsa_n, pubmod) == 0; BN_clear_free(pubmod); - if (found && !key_getattr_bn(&keys[i], + if (found && !pkcs11_getattr_bn(ctx, session, + PRIVKEY(&keys[i])->object, CKA_PUBLIC_EXPONENT, &rsa_e)) goto success; } @@ -217,6 +226,7 @@ static RSA *pkcs11_get_rsa(PKCS11_KEY *key) goto success; failure: + pkcs11_put_session(slot, session); if (rsa_n) BN_clear_free(rsa_n); if (rsa_e) @@ -224,6 +234,7 @@ static RSA *pkcs11_get_rsa(PKCS11_KEY *key) return NULL; success: + pkcs11_put_session(slot, session); rsa = RSA_new(); if (!rsa) goto failure;