_gnutls_pkcs11_check_init: improved transition between states

The init_level_t for PKCS#11 modules, was incorrectly handled as a
linear state transition, causing few cases in the transition to be
incorrectly handled. Define precisely the state transitions and
enforce them in _gnutls_pkcs11_check_init.

That addresses a regression introduced by the previous state handling
addition, which made impossible to switch from the trusted state to
the all modules.

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

3 files changed, 59 insertions, 20 deletions

diff --git a/lib/pkcs11.c b/lib/pkcs11.c
index e6e37c60cf..d36935b84c 100644
--- a/lib/pkcs11.c
+++ b/lib/pkcs11.c
@@ -267,20 +267,20 @@ pkcs11_add_module(const char* name, struct ck_function_list *module, unsigned cu
 */
 int _gnutls_pkcs11_check_init(init_level_t req_level, void *priv, pkcs11_reinit_function cb)
 {
-	int ret;
+	int ret, sret = 0;
 
 	ret = gnutls_mutex_lock(&_gnutls_pkcs11_mutex);
 	if (ret != 0)
 		return gnutls_assert_val(GNUTLS_E_LOCKING_ERROR);
 
-	if (providers_initialized >= req_level) {
+	if (providers_initialized > PROV_UNINITIALIZED) {
 		ret = 0;
 
 		if (_gnutls_detect_fork(pkcs11_forkid)) {
 			/* if we are initialized but a fork is detected */
 			ret = _gnutls_pkcs11_reinit();
 			if (ret == 0) {
-				ret = 1;
+				sret = 1;
 				if (cb) {
 					int ret2 = cb(priv);
 					if (ret2 < 0)
@@ -290,25 +290,60 @@ int _gnutls_pkcs11_check_init(init_level_t req_level, void *priv, pkcs11_reinit_
 			}
 		}
 
-		gnutls_mutex_unlock(&_gnutls_pkcs11_mutex);
-		return ret;
-	} else if (providers_initialized < req_level &&
-		   (req_level == PROV_INIT_TRUSTED)) {
-		_gnutls_debug_log("Initializing needed PKCS #11 modules\n");
-		ret = auto_load(1);
+		if (ret < 0) {
+			gnutls_assert();
+			goto cleanup;
+		}
+	}
 
-		providers_initialized = PROV_INIT_TRUSTED;
-	} else {
-		_gnutls_debug_log("Initializing all PKCS #11 modules\n");
-		ret = gnutls_pkcs11_init(GNUTLS_PKCS11_FLAG_AUTO, NULL);
+	/* Possible Transitions: PROV_UNINITIALIZED -> PROV_INIT_MANUAL -> PROV_INIT_MANUAL_TRUSTED
+	 * PROV_UNINITIALIZED -> PROV_INIT_TRUSTED -> PROV_INIT_ALL
+	 *
+	 * request for PROV_INIT_TRUSTED may result to PROV_INIT_MANUAL_TRUSTED
+	 * request for PROV_INIT_ALL may result to PROV_INIT_MANUAL or PROV_INIT_MANUAL_TRUSTED
+	 */
+	switch(req_level) {
+		case PROV_UNINITIALIZED:
+		case PROV_INIT_MANUAL:
+			break;
+		case PROV_INIT_TRUSTED:
+		case PROV_INIT_MANUAL_TRUSTED:
+			if (providers_initialized < PROV_INIT_MANUAL_TRUSTED) {
+				_gnutls_debug_log("Initializing needed PKCS #11 modules\n");
+				ret = auto_load(1);
+				if (ret < 0) {
+					gnutls_assert();
+				}
+
+				if (providers_initialized == PROV_INIT_MANUAL)
+					providers_initialized = PROV_INIT_MANUAL_TRUSTED;
+				else
+					providers_initialized = PROV_INIT_TRUSTED;
+
+				goto cleanup;
+			}
+			break;
+		case PROV_INIT_ALL:
+			if (providers_initialized == PROV_INIT_TRUSTED ||
+			    providers_initialized == PROV_UNINITIALIZED) {
+				_gnutls_debug_log("Initializing all PKCS #11 modules\n");
+				ret = gnutls_pkcs11_init(GNUTLS_PKCS11_FLAG_AUTO, NULL);
+				if (ret < 0) {
+					gnutls_assert();
+				}
+
+				providers_initialized = PROV_INIT_ALL;
+				goto cleanup;
+			}
+			break;
 	}
 
-	gnutls_mutex_unlock(&_gnutls_pkcs11_mutex);
+	ret = sret;
 
-	if (ret < 0)
-		return gnutls_assert_val(ret);
+ cleanup:
+	gnutls_mutex_unlock(&_gnutls_pkcs11_mutex);
 
-	return 0;
+	return ret;
 }
 
 
diff --git a/lib/pkcs11_int.h b/lib/pkcs11_int.h
index de9afbdee5..9ce7294b9d 100644
--- a/lib/pkcs11_int.h
+++ b/lib/pkcs11_int.h
@@ -86,10 +86,14 @@ typedef int (*pkcs11_reinit_function)(void *priv);
 typedef enum init_level_t {
 	PROV_UNINITIALIZED = 0,
 	PROV_INIT_MANUAL,
+	PROV_INIT_MANUAL_TRUSTED,
 	PROV_INIT_TRUSTED,
 	PROV_INIT_ALL
 } init_level_t;
 
+/* See _gnutls_pkcs11_check_init() for possible Transitions.
+ */
+
 int _gnutls_pkcs11_check_init(init_level_t req_level, void *priv, pkcs11_reinit_function cb);
 
 #define FIX_KEY_USAGE(pk, usage) \
@@ -101,7 +105,7 @@ int _gnutls_pkcs11_check_init(init_level_t req_level, void *priv, pkcs11_reinit_
 	}
 
 #define PKCS11_CHECK_INIT \
-	ret = _gnutls_pkcs11_check_init(PROV_INIT_MANUAL, NULL, NULL); \
+	ret = _gnutls_pkcs11_check_init(PROV_INIT_ALL, NULL, NULL); \
 	if (ret < 0) \
 		return gnutls_assert_val(ret)
 
@@ -111,7 +115,7 @@ int _gnutls_pkcs11_check_init(init_level_t req_level, void *priv, pkcs11_reinit_
 		return gnutls_assert_val(ret)
 
 #define PKCS11_CHECK_INIT_RET(x) \
-	ret = _gnutls_pkcs11_check_init(PROV_INIT_MANUAL, NULL, NULL); \
+	ret = _gnutls_pkcs11_check_init(PROV_INIT_ALL, NULL, NULL); \
 	if (ret < 0) \
 		return gnutls_assert_val(x)
 
diff --git a/lib/pkcs11_privkey.c b/lib/pkcs11_privkey.c
index afe831ee9b..fd1997c8cc 100644
--- a/lib/pkcs11_privkey.c
+++ b/lib/pkcs11_privkey.c
@@ -36,7 +36,7 @@
 /* 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(PROV_INIT_MANUAL, k, reopen_privkey_session); \
+	ret = _gnutls_pkcs11_check_init(PROV_INIT_ALL, k, reopen_privkey_session); \
 	if (ret < 0) \
 		return gnutls_assert_val(ret)