proxy: Do not assign duplicate slot IDs

p11-kit/Makefile.am

@@ -432,7 +432,8 @@ check_LTLIBRARIES += \
 	mock-five.la \
 	mock-seven.la \
 	mock-eight.la \
-	mock-nine.la
+	mock-nine.la \
+	mock-ten.la
 
 mock_one_la_SOURCES = p11-kit/mock-module-ep.c
 mock_one_la_LIBADD = libp11-test.la libp11-common.la
@@ -475,6 +476,10 @@ mock_nine_la_SOURCES = p11-kit/mock-module-ep7.c
 mock_nine_la_LDFLAGS = $(mock_one_la_LDFLAGS)
 mock_nine_la_LIBADD = $(mock_one_la_LIBADD)
 
+mock_ten_la_SOURCES = p11-kit/mock-module-ep8.c
+mock_ten_la_LDFLAGS = $(mock_one_la_LDFLAGS)
+mock_ten_la_LIBADD = $(mock_one_la_LIBADD)
+
 EXTRA_DIST += \
 	p11-kit/fixtures \
 	p11-kit/test-mock.c \

p11-kit/meson.build

@@ -275,7 +275,8 @@ mock_sources = {
   'mock-five': ['mock-module-ep3.c'],
   'mock-seven': ['mock-module-ep5.c'],
   'mock-eight': ['mock-module-ep6.c'],
-  'mock-nine': ['mock-module-ep7.c']
+  'mock-nine': ['mock-module-ep7.c'],
+  'mock-ten': ['mock-module-ep8.c']
 }
 
 if host_system != 'windows'

p11-kit/mock-module-ep8.c

@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2012 Stefan Walter
+ * Copyright (c) 2020 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, Anderson Sasaki
+ */
+
+#include "config.h"
+
+#define CRYPTOKI_EXPORTS 1
+#include "pkcs11.h"
+
+#include "mock.h"
+#include "test.h"
+
+#define MOCK_SLOT_THREE_ID 792
+
+/* Update mock-module.h URIs when updating this */
+
+static int called = 0;
+static CK_SLOT_ID last_id = 1;
+
+static CK_RV
+override_get_slot_list (CK_BBOOL token_present,
+                    CK_SLOT_ID_PTR slot_list,
+                    CK_ULONG_PTR count)
+{
+	if (count == NULL)
+		return CKR_ARGUMENTS_BAD;
+
+	/* For odd numbered calls, the module will return 1 slot with a slot ID
+	 * returned previously.
+	 *
+	 * For even numbered calls, the module will return 2 slots, being the new
+	 * slot put first in the list */
+	if (called % 2) {
+		if (slot_list == NULL) {
+			*count = 1;
+			return CKR_OK;
+		}
+		if (*count < 1) {
+			return CKR_BUFFER_TOO_SMALL;
+		}
+
+		slot_list[0] = last_id;
+		*count = 1;
+	} else {
+		if (slot_list == NULL) {
+			*count = 2;
+			return CKR_OK;
+		}
+
+		if (*count < 2) {
+			return CKR_BUFFER_TOO_SMALL;
+		}
+
+		slot_list[1] = last_id;
+		slot_list[0] = ++last_id;
+
+		*count = 2;
+	}
+
+	++called;
+
+	return CKR_OK;
+}
+
+#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;
+}
+

p11-kit/proxy.c

@@ -83,6 +83,7 @@ typedef struct {
 	p11_dict *sessions;
 	CK_FUNCTION_LIST **inited;
 	unsigned int forkid;
+	CK_SLOT_ID last_id;
 } Proxy;
 
 typedef struct _State {
@@ -113,22 +114,20 @@ map_slot_unlocked (Proxy *px,
                    CK_SLOT_ID slot,
                    Mapping *mapping)
 {
+	unsigned int i;
+
 	assert (px != NULL);
 	assert (mapping != NULL);
 
-	if (slot < MAPPING_OFFSET)
-		return CKR_SLOT_ID_INVALID;
-	slot -= MAPPING_OFFSET;
-
-	if (slot > px->n_mappings) {
-		return CKR_SLOT_ID_INVALID;
-	} else if (px->n_mappings == 0) {
-		return CKR_SLOT_ID_INVALID;
-	} else {
-		assert (px->mappings);
-		memcpy (mapping, &px->mappings[slot], sizeof (Mapping));
-		return CKR_OK;
+	for (i = 0; i < px->n_mappings; i++) {
+		assert (px->mappings != NULL);
+		if (px->mappings[i].wrap_slot == slot) {
+			memcpy (mapping, &px->mappings[i], sizeof(Mapping));
+			return CKR_OK;
+		}
 	}
+
+	return CKR_SLOT_ID_INVALID;
 }
 
 static CK_RV
@@ -279,29 +278,47 @@ proxy_list_slots (Proxy *py, Mapping *mappings, unsigned int n_mappings)
 
 		if (count > 0) {
 			Mapping *new_mappings;
+			CK_SLOT_ID_PTR new_slots;
+			int new_slots_count = 0;
 
+			new_slots = calloc (count, sizeof(CK_SLOT_ID));
+			return_val_if_fail (new_slots != NULL, CKR_HOST_MEMORY);
 			new_mappings = realloc (py->mappings, sizeof (Mapping) * (py->n_mappings + count));
 			return_val_if_fail (new_mappings != NULL, CKR_HOST_MEMORY);
 			py->mappings = new_mappings;
 
-			/* And now add a mapping for each of those slots */
+			/* Reuse the existing mapping if any */
 			for (i = 0; i < count; ++i) {
-				/* Reuse the existing mapping if any */
 				for (j = 0; j < n_mappings; ++j) {
 					/* cppcheck-suppress nullPointer symbolName=mappings */
 					/* false-positive: https://trac.cppcheck.net/ticket/9573 */
 					if (mappings[j].funcs == funcs &&
-					    mappings[j].real_slot == slots[i])
+					    mappings[j].real_slot == slots[i]) {
+						py->mappings[py->n_mappings].funcs = funcs;
+						py->mappings[py->n_mappings].real_slot = slots[i];
+						py->mappings[py->n_mappings].wrap_slot =
+							mappings[j].wrap_slot;
+						++py->n_mappings;
 						break;
+					}
+				}
+				if (n_mappings == 0 || j == n_mappings) {
+					new_slots[new_slots_count] = slots[i];
+					++new_slots_count;
 				}
+			}
+
+			/* And now add a mapping for each new slot */
+			for (i = 0; i < new_slots_count; ++i) {
+				++py->last_id;
 				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->last_id + MAPPING_OFFSET;
+				py->mappings[py->n_mappings].real_slot = new_slots[i];
 				++py->n_mappings;
 			}
+
+			free(new_slots);
 		}
 
 		free (slots);
@@ -320,6 +337,7 @@ proxy_create (Proxy **res, CK_FUNCTION_LIST **loaded,
 	return_val_if_fail (py != NULL, CKR_HOST_MEMORY);
 
 	py->forkid = p11_forkid;
+	py->last_id = 0;
 
 	py->inited = modules_dup (loaded);
 	if (py->inited == NULL) {

p11-kit/test-proxy.c

@@ -236,6 +236,7 @@ teardown (void *unused)
 #define ENABLED_PREFIX "enable-in: test-proxy-suffix, p11-kit-proxy-suffix, test-proxy, p11-kit-proxy\n"
 #define EIGHT_MODULE "module: mock-eight" SHLEXT "\n"
 #define NINE_MODULE "module: mock-nine" SHLEXT "\n"
+#define TEN_MODULE "module: mock-ten" SHLEXT "\n"
 
 static CK_ULONG
 load_modules_and_count_slots (void)
@@ -393,6 +394,43 @@ test_slot_event (void)
 	p11_proxy_module_cleanup ();
 }
 
+static void
+test_reuse_slots (void)
+{
+	CK_FUNCTION_LIST_PTR proxy;
+	CK_SLOT_ID slots[32];
+	CK_ULONG count = 32;
+	CK_RV rv;
+
+	p11_test_file_write (test.directory, "ten.module", TEN_MODULE, strlen (TEN_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_FALSE, slots, &count);
+	assert (rv == CKR_OK);
+	assert_num_eq (count, 1);
+
+	count = 32;
+
+	rv = proxy->C_GetSlotList (CK_FALSE, slots, &count);
+	assert (rv == CKR_OK);
+	assert_num_eq (count, 2);
+
+	/* Make sure the assigned slot IDs are different */
+	assert_num_cmp (slots[0], !=, slots[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)
 {
@@ -481,6 +519,7 @@ main (int argc,
 	p11_test (test_no_slot, "/proxy/no-slot");
 	p11_test (test_slot_appear, "/proxy/slot-appear");
 	p11_test (test_slot_event, "/proxy/slot-event");
+	p11_test (test_reuse_slots, "/proxy/reuse-slots");
 
 	test_mock_add_tests ("/proxy");