From 7332b6c2b9586d02a87a1950124a3b447f6aec22 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Fri, 10 Mar 2017 17:37:10 +0100 Subject: pkcs11: re-open private key session inside a locked section This prevents clashes when the same operation is carried in other threads. Signed-off-by: Nikos Mavrogiannopoulos --- lib/pkcs11.c | 69 ++++++++++++++++++++++++++++++++++------------------ lib/pkcs11_int.h | 10 +++++--- lib/pkcs11_privkey.c | 22 ++++++++++++----- 3 files changed, 67 insertions(+), 34 deletions(-) diff --git a/lib/pkcs11.c b/lib/pkcs11.c index a7b2c2b2f6..ed843aa3ed 100644 --- a/lib/pkcs11.c +++ b/lib/pkcs11.c @@ -3,7 +3,7 @@ * Copyright (C) 2010-2014 Free Software Foundation, Inc. * Copyright (C) 2008 Joe Orton * Copyright (C) 2013 Nikos Mavrogiannopoulos - * Copyright (C) 2014 Red Hat + * Copyright (C) 2014-2017 Red Hat * * Authors: Nikos Mavrogiannopoulos, Stef Walter * @@ -109,6 +109,8 @@ static unsigned int active_providers = 0; static unsigned int providers_initialized = 0; static unsigned int pkcs11_forkid = 0; +static int _gnutls_pkcs11_reinit(void); + gnutls_pkcs11_token_callback_t _gnutls_token_func; void *_gnutls_token_data; @@ -251,9 +253,12 @@ pkcs11_add_module(const char* name, struct ck_function_list *module, const char /* Returns: * - negative error code on error, * - 0 on success - * - 1 on success (and a fork was detected) - */ -int _gnutls_pkcs11_check_init(void) + * - 1 on success (and a fork was detected - cb was run) + * + * The output value of the callback will be returned if it is + * a negative one (indicating failure). +*/ +int _gnutls_pkcs11_check_init(void *priv, pkcs11_reinit_function cb) { int ret; @@ -266,9 +271,16 @@ int _gnutls_pkcs11_check_init(void) if (_gnutls_detect_fork(pkcs11_forkid)) { /* if we are initialized but a fork is detected */ - ret = gnutls_pkcs11_reinit(); - if (ret == 0) + ret = _gnutls_pkcs11_reinit(); + if (ret == 0) { ret = 1; + if (cb) { + int ret2 = cb(priv); + if (ret2 < 0) + ret = ret2; + } + pkcs11_forkid = _gnutls_get_forkid(); + } } gnutls_mutex_unlock(&_gnutls_pkcs11_mutex); @@ -794,6 +806,30 @@ gnutls_pkcs11_init(unsigned int flags, const char *deprecated_config_file) return 0; } +static int _gnutls_pkcs11_reinit(void) +{ + unsigned i; + ck_rv_t rv; + + for (i = 0; i < active_providers; i++) { + if (providers[i].module != NULL) { + rv = p11_kit_module_initialize(providers + [i].module); + if (rv == CKR_OK || rv == CKR_CRYPTOKI_ALREADY_INITIALIZED) { + providers[i].active = 1; + } else { + providers[i].active = 0; + _gnutls_debug_log + ("Cannot re-initialize registered module '%.*s': %s\n", + (int)32, providers[i].info.library_description, + p11_kit_strerror(rv)); + } + } + } + + return 0; +} + /** * gnutls_pkcs11_reinit: * @@ -811,32 +847,17 @@ gnutls_pkcs11_init(unsigned int flags, const char *deprecated_config_file) **/ int gnutls_pkcs11_reinit(void) { - unsigned i; - ck_rv_t rv; + int ret; /* make sure that we don't call more than once after a fork */ if (_gnutls_detect_fork(pkcs11_forkid) == 0) return 0; - for (i = 0; i < active_providers; i++) { - if (providers[i].module != NULL) { - rv = p11_kit_module_initialize(providers - [i].module); - if (rv == CKR_OK || rv == CKR_CRYPTOKI_ALREADY_INITIALIZED) { - providers[i].active = 1; - } else { - providers[i].active = 0; - _gnutls_debug_log - ("Cannot re-initialize registered module '%.*s': %s\n", - (int)32, providers[i].info.library_description, - p11_kit_strerror(rv)); - } - } - } + ret = _gnutls_pkcs11_reinit(); pkcs11_forkid = _gnutls_get_forkid(); - return 0; + return ret; } /** diff --git a/lib/pkcs11_int.h b/lib/pkcs11_int.h index f012773e69..bcde4d8ce8 100644 --- a/lib/pkcs11_int.h +++ b/lib/pkcs11_int.h @@ -62,8 +62,10 @@ struct gnutls_pkcs11_obj_st { }; /* This must be called on every function that uses a PKCS #11 function - * directly */ -int _gnutls_pkcs11_check_init(void); + * directly. It can be provided a callback function to run when a reinitialization + * occurs. */ +typedef int (*pkcs11_reinit_function)(void *priv); +int _gnutls_pkcs11_check_init(void *priv, pkcs11_reinit_function cb); #define FIX_KEY_USAGE(pk, usage) \ if (usage == 0) { \ @@ -74,12 +76,12 @@ int _gnutls_pkcs11_check_init(void); } #define PKCS11_CHECK_INIT \ - ret = _gnutls_pkcs11_check_init(); \ + ret = _gnutls_pkcs11_check_init(NULL, NULL); \ if (ret < 0) \ return gnutls_assert_val(ret) #define PKCS11_CHECK_INIT_RET(x) \ - ret = _gnutls_pkcs11_check_init(); \ + ret = _gnutls_pkcs11_check_init(NULL, NULL); \ if (ret < 0) \ return gnutls_assert_val(x) diff --git a/lib/pkcs11_privkey.c b/lib/pkcs11_privkey.c index 220d718ddc..9cfce7e322 100644 --- a/lib/pkcs11_privkey.c +++ b/lib/pkcs11_privkey.c @@ -36,13 +36,9 @@ /* In case of a fork, it will invalidate the open session * in the privkey and start another */ #define PKCS11_CHECK_INIT_PRIVKEY(k) \ - ret = _gnutls_pkcs11_check_init(); \ + ret = _gnutls_pkcs11_check_init(k, reopen_privkey_session); \ if (ret < 0) \ - return gnutls_assert_val(ret); \ - if (ret == 1) { \ - memset(&k->sinfo, 0, sizeof(k->sinfo)); \ - FIND_OBJECT(k); \ - } + return gnutls_assert_val(ret) #define FIND_OBJECT(key) \ do { \ @@ -240,7 +236,21 @@ find_object(struct pkcs11_session_info *sinfo, return ret; } +/* callback function to be passed in _gnutls_pkcs11_check_init(). + * It is run, only when a fork has been detected, and data have + * been re-initialized. In that case we reset the session and re-open + * the object. */ +static int reopen_privkey_session(void * _privkey) +{ + int ret; + gnutls_pkcs11_privkey_t privkey = _privkey; + + memset(&privkey->sinfo, 0, sizeof(privkey->sinfo)); + privkey->ref = 0; + FIND_OBJECT(privkey); + return 0; +} /*- * _gnutls_pkcs11_privkey_sign_hash: -- cgit v1.2.1