Skip to content

Commit

Permalink
rpc: Fix serialization of NULL mechanism pointer
Browse files Browse the repository at this point in the history
A NULL mechanism pointer is valid for C_*Init functions to cancel the
operation.  Since 852ccd8 we encoded it with a CK_MECHANISM_TYPE 0 as
an indicator, though it clashes with CKM_RSA_PKCS_KEY_PAIR_GEN (0).
This patch changes the encoding to use a special value (0xffffffff) to
indicate that and also properly advance the offset when reading.

Signed-off-by: Daiki Ueno <[email protected]>
ueno authored and ZoltanFridrich committed Nov 15, 2023
1 parent 66d6b42 commit 66f1fc7
Showing 4 changed files with 48 additions and 5 deletions.
8 changes: 6 additions & 2 deletions p11-kit/rpc-client.c
Original file line number Diff line number Diff line change
@@ -430,9 +430,13 @@ proto_write_mechanism (p11_rpc_message *msg,
/* Make sure this is in the right order */
assert (!msg->signature || p11_rpc_message_verify_part (msg, "M"));

/* This case is valid for C_*Init () functions to cancel operation */
/*
* The NULL mechanism is used for C_*Init () functions to
* cancel operation. We use a special value 0xffffffff as a
* marker to indicate that.
*/
if (mech == NULL) {
p11_rpc_buffer_add_uint32 (msg->output, 0);
p11_rpc_buffer_add_uint32 (msg->output, 0xffffffff);
return p11_buffer_failed (msg->output) ? CKR_HOST_MEMORY : CKR_OK;
}

10 changes: 8 additions & 2 deletions p11-kit/rpc-message.c
Original file line number Diff line number Diff line change
@@ -2114,8 +2114,14 @@ p11_rpc_buffer_get_mechanism (p11_buffer *buffer,

mech->mechanism = mechanism;

/* special NULL case */
if (mechanism == 0) {
/*
* The NULL mechanism is used for C_*Init () functions to
* cancel operation. We use a special value 0xffffffff as a
* marker to indicate that.
*/
if (mechanism == 0xffffffff) {
mech->ulParameterLen = 0;
mech->pParameter = NULL;
return true;
}

8 changes: 7 additions & 1 deletion p11-kit/rpc-server.c
Original file line number Diff line number Diff line change
@@ -480,8 +480,14 @@ proto_read_mechanism (p11_rpc_message *msg,
return PARSE_ERROR;
}

if (temp.mechanism == 0) {
/*
* The NULL mechanism is used for C_*Init () functions to
* cancel operation. We use a special value 0xffffffff as a
* marker to indicate that.
*/
if (temp.mechanism == 0xffffffff) {
*mech = NULL;
msg->parsed = offset;
return CKR_OK;
}

27 changes: 27 additions & 0 deletions p11-kit/test-rpc.c
Original file line number Diff line number Diff line change
@@ -675,6 +675,31 @@ test_simultaneous_functions (void *module)
p11_mutex_uninit (&delay_mutex);
}

static void
test_mechanism_unsupported (void *module)
{
CK_FUNCTION_LIST_PTR rpc_module;
CK_SESSION_HANDLE session;
CK_MECHANISM mech;
CK_RV rv;

rpc_module = setup_test_rpc_module (&test_normal_vtable,
module, &session);

memset (&mech, 0, sizeof(mech));

/*
* This mechanism is not supported by the remote mock module,
* but it should be able to return an error through RPC.
*/
mech.mechanism = CKM_RSA_PKCS_KEY_PAIR_GEN;

rv = (rpc_module->C_DigestInit) (session, &mech);
assert_num_eq (rv, CKR_MECHANISM_INVALID);

teardown_mock_module (rpc_module);
}

#ifdef OS_UNIX

static void
@@ -759,6 +784,7 @@ main (int argc,
p11_testx (test_get_info_stand_in, &mock_module_no_slots, "/rpc/get-info-stand-in");
p11_testx (test_get_slot_list_no_device, &mock_module_no_slots, "/rpc/get-slot-list-no-device");
p11_testx (test_simultaneous_functions, &mock_module_no_slots, "/rpc/simultaneous-functions");
p11_testx (test_mechanism_unsupported, &mock_module, "/rpc/mechanism-unsupported");

#ifdef OS_UNIX
p11_testx (test_fork_and_reinitialize, &mock_module_no_slots, "/rpc/fork-and-reinitialize");
@@ -778,6 +804,7 @@ main (int argc,
p11_testx (test_get_info_stand_in, &mock_module_v3_no_slots, "/rpc3/get-info-stand-in");
p11_testx (test_get_slot_list_no_device, &mock_module_v3_no_slots, "/rpc3/get-slot-list-no-device");
p11_testx (test_simultaneous_functions, &mock_module_v3_no_slots, "/rpc3/simultaneous-functions");
p11_testx (test_mechanism_unsupported, &mock_module_v3, "/rpc3/mechanism-unsupported");

#ifdef OS_UNIX
p11_testx (test_fork_and_reinitialize, &mock_module_v3_no_slots, "/rpc3/fork-and-reinitialize");

0 comments on commit 66f1fc7

Please sign in to comment.