proxy: Refresh slot list on every C_GetSlotList call

Previously, the proxy module calculated the slot list only once at the
C_Initialize() call.  That was causing a usability limitation when the
user attaches HSM after starting an application.

Suggested by David Ward.

4 files changed, 192 insertions, 48 deletions

diff --git a/p11-kit/Makefile.am b/p11-kit/Makefile.am
index b6dffde..0155f7c 100644
--- a/p11-kit/Makefile.am
+++ b/p11-kit/Makefile.am
@@ -408,7 +408,8 @@ check_LTLIBRARIES += \
 	mock-three.la \
 	mock-four.la \
 	mock-five.la \
-	mock-seven.la
+	mock-seven.la \
+	mock-eight.la
 
 mock_one_la_SOURCES = p11-kit/mock-module-ep.c
 mock_one_la_LIBADD = libp11-test.la libp11-common.la
@@ -443,6 +444,10 @@ mock_seven_la_SOURCES = p11-kit/mock-module-ep5.c
 mock_seven_la_LDFLAGS = $(mock_one_la_LDFLAGS)
 mock_seven_la_LIBADD = $(mock_one_la_LIBADD)
 
+mock_eight_la_SOURCES = p11-kit/mock-module-ep6.c
+mock_eight_la_LDFLAGS = $(mock_one_la_LDFLAGS)
+mock_eight_la_LIBADD = $(mock_one_la_LIBADD)
+
 EXTRA_DIST += \
 	p11-kit/fixtures \
 	p11-kit/test-mock.c \
diff --git a/p11-kit/mock-module-ep6.c b/p11-kit/mock-module-ep6.c
new file mode 100644
index 0000000..9821b3d
--- /dev/null
+++ b/p11-kit/mock-module-ep6.c
@@ -0,0 +1,76 @@
+/*
+ * Copyright (c) 2012 Stefan Walter
+ * Copyright (c) 2019 Red Hat, Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *     * Redistributions of source code must retain the above
+ *       copyright notice, this list of conditions and the
+ *       following disclaimer.
+ *     * Redistributions in binary form must reproduce the
+ *       above copyright notice, this list of conditions and
+ *       the following disclaimer in the documentation and/or
+ *       other materials provided with the distribution.
+ *     * The names of contributors to this software may not be
+ *       used to endorse or promote products derived from this
+ *       software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ * Author: Stef Walter <stef@thewalter.net>, Daiki Ueno
+ */
+
+#include "config.h"
+
+#define CRYPTOKI_EXPORTS 1
+#include "pkcs11.h"
+
+#include "mock.h"
+#include "test.h"
+
+static size_t called = 0;
+
+static CK_RV
+override_get_slot_list (CK_BBOOL token_present,
+			CK_SLOT_ID_PTR slot_list,
+			CK_ULONG_PTR count)
+{
+	/* Return 0 on the first attempt to retrieve slot list.  Note
+	 * that this function is typically called twice: first to get
+	 * the number of slots, and then to retrieve the slot list.
+	 */
+	if (called++ < 2) {
+		*count = 0;
+		return CKR_OK;
+	}
+
+	return mock_C_GetSlotList (token_present, slot_list, count);
+}
+
+#ifdef OS_WIN32
+__declspec(dllexport)
+#endif
+CK_RV
+C_GetFunctionList (CK_FUNCTION_LIST_PTR_PTR list)
+{
+	mock_module_init ();
+	mock_module.C_GetFunctionList = C_GetFunctionList;
+	if (list == NULL)
+		return CKR_ARGUMENTS_BAD;
+	mock_module.C_GetSlotList = override_get_slot_list;
+	*list = &mock_module;
+	return CKR_OK;
+}
diff --git a/p11-kit/proxy.c b/p11-kit/proxy.c
index 8eaf205..ae88318 100644
--- a/p11-kit/proxy.c
+++ b/p11-kit/proxy.c
@@ -250,8 +250,7 @@ modules_dup (CK_FUNCTION_LIST **modules)
 }
 
 static CK_RV
-proxy_create (Proxy **res, CK_FUNCTION_LIST **loaded,
-	      Mapping *mappings, unsigned int n_mappings)
+proxy_list_slots (Proxy *py, Mapping *mappings, unsigned int n_mappings)
 {
 	CK_FUNCTION_LIST_PTR *f;
 	CK_FUNCTION_LIST_PTR funcs;
@@ -259,6 +258,58 @@ proxy_create (Proxy **res, CK_FUNCTION_LIST **loaded,
 	CK_ULONG i, count;
 	unsigned int j;
 	CK_RV rv = CKR_OK;
+
+	for (f = py->inited; *f; ++f) {
+		funcs = *f;
+		assert (funcs != NULL);
+		slots = NULL;
+
+		/* Ask module for its slots */
+		rv = (funcs->C_GetSlotList) (FALSE, NULL, &count);
+		if (rv == CKR_OK && count) {
+			slots = calloc (sizeof (CK_SLOT_ID), count);
+			rv = (funcs->C_GetSlotList) (FALSE, slots, &count);
+		}
+
+		if (rv != CKR_OK) {
+			free (slots);
+			break;
+		}
+
+		return_val_if_fail (count == 0 || slots != NULL, CKR_GENERAL_ERROR);
+
+		if (count > 0) {
+			py->mappings = realloc (py->mappings, sizeof (Mapping) * (py->n_mappings + count));
+			return_val_if_fail (py->mappings != NULL, CKR_HOST_MEMORY);
+
+			/* And now add a mapping for each of those slots */
+			for (i = 0; i < count; ++i) {
+				/* Reuse the existing mapping if any */
+				for (j = 0; j < n_mappings; ++j) {
+					if (mappings[j].funcs == funcs &&
+					    mappings[j].real_slot == slots[i])
+						break;
+				}
+				py->mappings[py->n_mappings].funcs = funcs;
+				py->mappings[py->n_mappings].wrap_slot =
+					(n_mappings == 0 || j == n_mappings) ?
+					py->n_mappings + MAPPING_OFFSET :
+					mappings[j].wrap_slot;
+				py->mappings[py->n_mappings].real_slot = slots[i];
+				++py->n_mappings;
+			}
+		}
+
+		free (slots);
+	}
+	return rv;
+}
+
+static CK_RV
+proxy_create (Proxy **res, CK_FUNCTION_LIST **loaded,
+	      Mapping *mappings, unsigned int n_mappings)
+{
+	CK_RV rv = CKR_OK;
 	Proxy *py;
 
 	py = calloc (1, sizeof (Proxy));
@@ -275,49 +326,7 @@ proxy_create (Proxy **res, CK_FUNCTION_LIST **loaded,
 	rv = p11_kit_modules_initialize (py->inited, NULL);
 
 	if (rv == CKR_OK) {
-		for (f = py->inited; *f; ++f) {
-			funcs = *f;
-			assert (funcs != NULL);
-			slots = NULL;
-
-			/* Ask module for its slots */
-			rv = (funcs->C_GetSlotList) (FALSE, NULL, &count);
-			if (rv == CKR_OK && count) {
-				slots = calloc (sizeof (CK_SLOT_ID), count);
-				rv = (funcs->C_GetSlotList) (FALSE, slots, &count);
-			}
-
-			if (rv != CKR_OK) {
-				free (slots);
-				break;
-			}
-
-			return_val_if_fail (count == 0 || slots != NULL, CKR_GENERAL_ERROR);
-
-			if (count > 0) {
-				py->mappings = realloc (py->mappings, sizeof (Mapping) * (py->n_mappings + count));
-				return_val_if_fail (py->mappings != NULL, CKR_HOST_MEMORY);
-
-				/* And now add a mapping for each of those slots */
-				for (i = 0; i < count; ++i) {
-					/* Reuse the existing mapping if any */
-					for (j = 0; j < n_mappings; ++j) {
-						if (mappings[j].funcs == funcs &&
-						    mappings[j].real_slot == slots[i])
-							break;
-					}
-					py->mappings[py->n_mappings].funcs = funcs;
-					py->mappings[py->n_mappings].wrap_slot =
-						(n_mappings == 0 || j == n_mappings) ?
-						py->n_mappings + MAPPING_OFFSET :
-						mappings[j].wrap_slot;
-					py->mappings[py->n_mappings].real_slot = slots[i];
-					++py->n_mappings;
-				}
-			}
-
-			free (slots);
-		}
+		rv = proxy_list_slots (py, mappings, n_mappings);
 	}
 
 	if (rv != CKR_OK) {
@@ -454,7 +463,29 @@ proxy_C_GetSlotList (CK_X_FUNCTION_LIST *self,
 
 		if (!PROXY_VALID (state->px)) {
 			rv = CKR_CRYPTOKI_NOT_INITIALIZED;
-		} else {
+		}
+
+		if (rv == CKR_OK) {
+			Mapping *mappings = NULL;
+			unsigned int n_mappings = 0;
+
+			if (state->px->mappings) {
+				mappings = state->px->mappings;
+				n_mappings = state->px->n_mappings;
+				state->px->mappings = NULL;
+				state->px->n_mappings = 0;
+			}
+			rv = proxy_list_slots (state->px, mappings, n_mappings);
+			if (rv == CKR_OK) {
+				free (mappings);
+			} else {
+				p11_debug ("failed to list slots: %lu", rv);
+				state->px->mappings = mappings;
+				state->px->n_mappings = n_mappings;
+			}
+		}
+
+		if (rv == CKR_OK) {
 			index = 0;
 
 			/* Go through and build up a map */
diff --git a/p11-kit/test-proxy.c b/p11-kit/test-proxy.c
index bf894f8..f61f37e 100644
--- a/p11-kit/test-proxy.c
+++ b/p11-kit/test-proxy.c
@@ -233,6 +233,7 @@ teardown (void *unused)
 #define TWO_MODULE "module: mock-two" SHLEXT "\n"
 #define ENABLED "enable-in: test-proxy, p11-kit-proxy\n"
 #define DISABLED "disable-in: p11-kit-proxy\n"
+#define EIGHT_MODULE "module: mock-eight" SHLEXT "\n"
 
 static CK_ULONG
 load_modules_and_count_slots (void)
@@ -311,6 +312,37 @@ test_disable (void)
 	assert_num_cmp (disabled, <, count);
 }
 
+static void
+test_slot_appear (void)
+{
+	CK_FUNCTION_LIST_PTR proxy;
+	CK_ULONG count;
+	CK_RV rv;
+
+	p11_test_file_write (test.directory, "eight.module", EIGHT_MODULE, strlen (EIGHT_MODULE));
+
+	rv = C_GetFunctionList (&proxy);
+	assert (rv == CKR_OK);
+
+	assert (p11_proxy_module_check (proxy));
+
+	rv = proxy->C_Initialize (NULL);
+	assert (rv == CKR_OK);
+
+	rv = proxy->C_GetSlotList (CK_TRUE, NULL, &count);
+	assert (rv == CKR_OK);
+	assert_num_eq (count, 0);
+
+	rv = proxy->C_GetSlotList (CK_TRUE, NULL, &count);
+	assert (rv == CKR_OK);
+	assert_num_eq (count, 1);
+
+	rv = proxy->C_Finalize (NULL);
+	assert_num_eq (rv, CKR_OK);
+
+	p11_proxy_module_cleanup ();
+}
+
 static CK_FUNCTION_LIST_PTR
 setup_mock_module (CK_SESSION_HANDLE *session)
 {
@@ -388,7 +420,6 @@ main (int argc,
 {
 	p11_library_init ();
 	p11_kit_be_quiet ();
-
 	p11_test (test_initialize_finalize, "/proxy/initialize-finalize");
 	p11_test (test_initialize_multiple, "/proxy/initialize-multiple");
 #ifndef _WIN32
@@ -398,6 +429,7 @@ main (int argc,
 	p11_fixture (setup, teardown);
 	p11_test (test_disable, "/proxy/disable");
 	p11_test (test_no_slot, "/proxy/no-slot");
+	p11_test (test_slot_appear, "/proxy/slot-appear");
 
 	test_mock_add_tests ("/proxy");