From 095eff3e552c3d314f3af6a430b9b1c3acb99bd6 Mon Sep 17 00:00:00 2001
From: Fredrik Thulin <fredrik@thulin.net>
Date: Mon, 7 Jan 2013 12:11:26 +0100
Subject: [PATCH] proto_read_byte_buffer: Support undefined buffer size.
A number of PKCS#11 functions take a pointer to a buffer size as
argument. To be a transparent proxy of PKCS#11 calls, it is necessary to
support invoking these functions with a NULL pointer. pkcs11-proxy used
to send the buffer size as an integer and create a pointer to the integer
on the server side, but this is different to the backend PKCS#11 module
in some cases.
E.g. the C_Encrypt call is specified to have side effectes (finalizing)
when called with a NULL encrypted data length. The softhsm test suite
exposed that these side effects never occured because the NULL data length
pointer was conveyed as a valid pointer to the integer zero.
Since an additional uint8_t was added to "byte buffers", this is an
backwards incompatible change. As such, the version number in the protocol
greeting was increased (GCK_RPC_HANDSHAKE).
---
gck-rpc-dispatch.c | 121 ++++++++++++++++++++++++---------------------
gck-rpc-message.c | 16 +++++-
gck-rpc-module.c | 15 ++++--
gck-rpc-private.h | 7 ++-
4 files changed, 96 insertions(+), 63 deletions(-)
diff --git a/gck-rpc-dispatch.c b/gck-rpc-dispatch.c
index 67ea0aa..a0152f3 100644
--- a/gck-rpc-dispatch.c
+++ b/gck-rpc-dispatch.c
@@ -201,9 +201,10 @@ static void call_uninit(CallState * cs)
static CK_RV
proto_read_byte_buffer(CallState * cs, CK_BYTE_PTR * buffer,
- CK_ULONG * n_buffer)
+ CK_ULONG_PTR * n_buffer)
{
GckRpcMessage *msg;
+ uint8_t flags;
uint32_t length;
assert(cs);
@@ -215,23 +216,27 @@ proto_read_byte_buffer(CallState * cs, CK_BYTE_PTR * buffer,
/* Check that we're supposed to be reading this at this point */
assert(!msg->signature || gck_rpc_message_verify_part(msg, "fy"));
+ if (!egg_buffer_get_byte
+ (&msg->buffer, msg->parsed, &msg->parsed, &flags))
+ return PARSE_ERROR;
+
/* The number of ulongs there's room for on the other end */
if (!egg_buffer_get_uint32
(&msg->buffer, msg->parsed, &msg->parsed, &length))
return PARSE_ERROR;
- *n_buffer = length;
- *buffer = NULL;
- /* We go ahead and allocate a buffer even if length is zero. The code used
- * to just return CKR_OK without allocating a buffer, but that breaks a
- * test case in pkcs11-tool for C_GenerateRandom of 0 bytes. Best to be as
- * transparent as possible and let the p11 module decide how to handle it.
- */
+ **n_buffer = length;
+ *buffer = NULL_PTR;
- *buffer = call_alloc(cs, length * sizeof(CK_BYTE));
- if (!*buffer)
- return CKR_DEVICE_MEMORY;
+ if ((flags & GCK_RPC_BYTE_BUFFER_NULL_COUNT))
+ *n_buffer = NULL_PTR;
+
+ if (! (flags & GCK_RPC_BYTE_BUFFER_NULL_DATA)) {
+ *buffer = call_alloc(cs, length * sizeof(CK_BYTE));
+ if (!*buffer)
+ return CKR_DEVICE_MEMORY;
+ }
return CKR_OK;
}
@@ -278,7 +283,7 @@ proto_read_byte_array(CallState * cs, CK_BYTE_PTR * array, CK_ULONG * n_array)
}
static CK_RV
-proto_write_byte_array(CallState * cs, CK_BYTE_PTR array, CK_ULONG len,
+proto_write_byte_array(CallState * cs, CK_BYTE_PTR array, CK_ULONG_PTR len,
CK_RV ret)
{
assert(cs);
@@ -300,7 +305,7 @@ proto_write_byte_array(CallState * cs, CK_BYTE_PTR array, CK_ULONG len,
return ret;
};
- if (!gck_rpc_message_write_byte_array(cs->resp, array, len))
+ if (!gck_rpc_message_write_byte_array(cs->resp, array, len ? *len : 0))
return PREP_ERROR;
return CKR_OK;
@@ -716,6 +721,12 @@ static CK_RV proto_write_session_info(CallState * cs, CK_SESSION_INFO_PTR info)
* CALL MACROS
*/
+#define DECLARE_CK_ULONG_PTR(ck_ulong_ptr_name) \
+ CK_ULONG ck_ulong_ptr_name ## _v ; \
+ CK_ULONG_PTR ck_ulong_ptr_name ; \
+ ck_ulong_ptr_name ## _v = 0; \
+ ck_ulong_ptr_name = &ck_ulong_ptr_name ## _v ;
+
#define BEGIN_CALL(call_id) \
debug ((#call_id ": enter")); \
assert (cs); \
@@ -747,8 +758,8 @@ static CK_RV proto_write_session_info(CallState * cs, CK_SESSION_INFO_PTR info)
_ret = proto_read_null_string (cs, &val); \
if (_ret != CKR_OK) goto _cleanup;
-#define IN_BYTE_BUFFER(buffer, buffer_len) \
- _ret = proto_read_byte_buffer (cs, &buffer, &buffer_len); \
+#define IN_BYTE_BUFFER(buffer, buffer_len_ptr) \
+ _ret = proto_read_byte_buffer (cs, &buffer, &buffer_len_ptr); \
if (_ret != CKR_OK) goto _cleanup;
#define IN_BYTE_ARRAY(buffer, buffer_len) \
@@ -775,9 +786,9 @@ static CK_RV proto_write_session_info(CallState * cs, CK_SESSION_INFO_PTR info)
if (_ret == CKR_OK && !gck_rpc_message_write_ulong (cs->resp, val)) \
_ret = PREP_ERROR;
-#define OUT_BYTE_ARRAY(array, len) \
+#define OUT_BYTE_ARRAY(array, len_ptr) \
/* Note how we filter return codes */ \
- _ret = proto_write_byte_array (cs, array, len, _ret);
+ _ret = proto_write_byte_array (cs, array, len_ptr, _ret);
#define OUT_ULONG_ARRAY(array, len) \
/* Note how we filter return codes */ \
@@ -1136,12 +1147,12 @@ static CK_RV rpc_C_GetOperationState(CallState * cs)
{
CK_SESSION_HANDLE session;
CK_BYTE_PTR operation_state;
- CK_ULONG operation_state_len;
+ DECLARE_CK_ULONG_PTR(operation_state_len);
BEGIN_CALL(C_GetOperationState);
IN_ULONG(session);
IN_BYTE_BUFFER(operation_state, operation_state_len);
- PROCESS_CALL((session, operation_state, &operation_state_len));
+ PROCESS_CALL((session, operation_state, operation_state_len));
OUT_BYTE_ARRAY(operation_state, operation_state_len);
END_CALL;
}
@@ -1341,14 +1352,14 @@ static CK_RV rpc_C_Encrypt(CallState * cs)
CK_BYTE_PTR data;
CK_ULONG data_len;
CK_BYTE_PTR encrypted_data;
- CK_ULONG encrypted_data_len;
+ DECLARE_CK_ULONG_PTR(encrypted_data_len);
BEGIN_CALL(C_Encrypt);
IN_ULONG(session);
IN_BYTE_ARRAY(data, data_len);
IN_BYTE_BUFFER(encrypted_data, encrypted_data_len);
PROCESS_CALL((session, data, data_len, encrypted_data,
- &encrypted_data_len));
+ encrypted_data_len));
OUT_BYTE_ARRAY(encrypted_data, encrypted_data_len);
END_CALL;
}
@@ -1359,14 +1370,14 @@ static CK_RV rpc_C_EncryptUpdate(CallState * cs)
CK_BYTE_PTR part;
CK_ULONG part_len;
CK_BYTE_PTR encrypted_part;
- CK_ULONG encrypted_part_len;
+ DECLARE_CK_ULONG_PTR(encrypted_part_len);
BEGIN_CALL(C_EncryptUpdate);
IN_ULONG(session);
IN_BYTE_ARRAY(part, part_len);
IN_BYTE_BUFFER(encrypted_part, encrypted_part_len);
PROCESS_CALL((session, part, part_len, encrypted_part,
- &encrypted_part_len));
+ encrypted_part_len));
OUT_BYTE_ARRAY(encrypted_part, encrypted_part_len);
END_CALL;
}
@@ -1375,12 +1386,12 @@ static CK_RV rpc_C_EncryptFinal(CallState * cs)
{
CK_SESSION_HANDLE session;
CK_BYTE_PTR last_encrypted_part;
- CK_ULONG last_encrypted_part_len;
+ DECLARE_CK_ULONG_PTR(last_encrypted_part_len);
BEGIN_CALL(C_EncryptFinal);
IN_ULONG(session);
IN_BYTE_BUFFER(last_encrypted_part, last_encrypted_part_len);
- PROCESS_CALL((session, last_encrypted_part, &last_encrypted_part_len));
+ PROCESS_CALL((session, last_encrypted_part, last_encrypted_part_len));
OUT_BYTE_ARRAY(last_encrypted_part, last_encrypted_part_len);
END_CALL;
}
@@ -1405,14 +1416,14 @@ static CK_RV rpc_C_Decrypt(CallState * cs)
CK_BYTE_PTR encrypted_data;
CK_ULONG encrypted_data_len;
CK_BYTE_PTR data;
- CK_ULONG data_len;
+ DECLARE_CK_ULONG_PTR(data_len);
BEGIN_CALL(C_Decrypt);
IN_ULONG(session);
IN_BYTE_ARRAY(encrypted_data, encrypted_data_len);
IN_BYTE_BUFFER(data, data_len);
PROCESS_CALL((session, encrypted_data, encrypted_data_len, data,
- &data_len));
+ data_len));
OUT_BYTE_ARRAY(data, data_len);
END_CALL;
}
@@ -1423,14 +1434,14 @@ static CK_RV rpc_C_DecryptUpdate(CallState * cs)
CK_BYTE_PTR encrypted_part;
CK_ULONG encrypted_part_len;
CK_BYTE_PTR part;
- CK_ULONG part_len;
+ DECLARE_CK_ULONG_PTR(part_len);
BEGIN_CALL(C_DecryptUpdate);
IN_ULONG(session);
IN_BYTE_ARRAY(encrypted_part, encrypted_part_len);
IN_BYTE_BUFFER(part, part_len);
PROCESS_CALL((session, encrypted_part, encrypted_part_len, part,
- &part_len));
+ part_len));
OUT_BYTE_ARRAY(part, part_len);
END_CALL;
}
@@ -1439,12 +1450,12 @@ static CK_RV rpc_C_DecryptFinal(CallState * cs)
{
CK_SESSION_HANDLE session;
CK_BYTE_PTR last_part;
- CK_ULONG last_part_len;
+ DECLARE_CK_ULONG_PTR(last_part_len);
BEGIN_CALL(C_DecryptFinal);
IN_ULONG(session);
IN_BYTE_BUFFER(last_part, last_part_len);
- PROCESS_CALL((session, last_part, &last_part_len));
+ PROCESS_CALL((session, last_part, last_part_len));
OUT_BYTE_ARRAY(last_part, last_part_len);
END_CALL;
}
@@ -1467,13 +1478,13 @@ static CK_RV rpc_C_Digest(CallState * cs)
CK_BYTE_PTR data;
CK_ULONG data_len;
CK_BYTE_PTR digest;
- CK_ULONG digest_len;
+ DECLARE_CK_ULONG_PTR(digest_len);
BEGIN_CALL(C_Digest);
IN_ULONG(session);
IN_BYTE_ARRAY(data, data_len);
IN_BYTE_BUFFER(digest, digest_len);
- PROCESS_CALL((session, data, data_len, digest, &digest_len));
+ PROCESS_CALL((session, data, data_len, digest, digest_len));
OUT_BYTE_ARRAY(digest, digest_len);
END_CALL;
}
@@ -1507,12 +1518,12 @@ static CK_RV rpc_C_DigestFinal(CallState * cs)
{
CK_SESSION_HANDLE session;
CK_BYTE_PTR digest;
- CK_ULONG digest_len;
+ DECLARE_CK_ULONG_PTR(digest_len);
BEGIN_CALL(C_DigestFinal);
IN_ULONG(session);
IN_BYTE_BUFFER(digest, digest_len);
- PROCESS_CALL((session, digest, &digest_len));
+ PROCESS_CALL((session, digest, digest_len));
OUT_BYTE_ARRAY(digest, digest_len);
END_CALL;
}
@@ -1537,13 +1548,13 @@ static CK_RV rpc_C_Sign(CallState * cs)
CK_BYTE_PTR part;
CK_ULONG part_len;
CK_BYTE_PTR signature;
- CK_ULONG signature_len;
+ DECLARE_CK_ULONG_PTR(signature_len);
BEGIN_CALL(C_Sign);
IN_ULONG(session);
IN_BYTE_ARRAY(part, part_len);
IN_BYTE_BUFFER(signature, signature_len);
- PROCESS_CALL((session, part, part_len, signature, &signature_len));
+ PROCESS_CALL((session, part, part_len, signature, signature_len));
OUT_BYTE_ARRAY(signature, signature_len);
END_CALL;
@@ -1566,12 +1577,12 @@ static CK_RV rpc_C_SignFinal(CallState * cs)
{
CK_SESSION_HANDLE session;
CK_BYTE_PTR signature;
- CK_ULONG signature_len;
+ DECLARE_CK_ULONG_PTR(signature_len);
BEGIN_CALL(C_SignFinal);
IN_ULONG(session);
IN_BYTE_BUFFER(signature, signature_len);
- PROCESS_CALL((session, signature, &signature_len));
+ PROCESS_CALL((session, signature, signature_len));
OUT_BYTE_ARRAY(signature, signature_len);
END_CALL;
}
@@ -1596,13 +1607,13 @@ static CK_RV rpc_C_SignRecover(CallState * cs)
CK_BYTE_PTR data;
CK_ULONG data_len;
CK_BYTE_PTR signature;
- CK_ULONG signature_len;
+ DECLARE_CK_ULONG_PTR(signature_len);
BEGIN_CALL(C_SignRecover);
IN_ULONG(session);
IN_BYTE_ARRAY(data, data_len);
IN_BYTE_BUFFER(signature, signature_len);
- PROCESS_CALL((session, data, data_len, signature, &signature_len));
+ PROCESS_CALL((session, data, data_len, signature, signature_len));
OUT_BYTE_ARRAY(signature, signature_len);
END_CALL;
}
@@ -1683,13 +1694,13 @@ static CK_RV rpc_C_VerifyRecover(CallState * cs)
CK_BYTE_PTR signature;
CK_ULONG signature_len;
CK_BYTE_PTR data;
- CK_ULONG data_len;
+ DECLARE_CK_ULONG_PTR(data_len);
BEGIN_CALL(C_VerifyRecover);
IN_ULONG(session);
IN_BYTE_ARRAY(signature, signature_len);
IN_BYTE_BUFFER(data, data_len);
- PROCESS_CALL((session, signature, signature_len, data, &data_len));
+ PROCESS_CALL((session, signature, signature_len, data, data_len));
OUT_BYTE_ARRAY(data, data_len);
END_CALL;
}
@@ -1700,14 +1711,14 @@ static CK_RV rpc_C_DigestEncryptUpdate(CallState * cs)
CK_BYTE_PTR part;
CK_ULONG part_len;
CK_BYTE_PTR encrypted_part;
- CK_ULONG encrypted_part_len;
+ DECLARE_CK_ULONG_PTR(encrypted_part_len);
BEGIN_CALL(C_DigestEncryptUpdate);
IN_ULONG(session);
IN_BYTE_ARRAY(part, part_len);
IN_BYTE_BUFFER(encrypted_part, encrypted_part_len);
PROCESS_CALL((session, part, part_len, encrypted_part,
- &encrypted_part_len));
+ encrypted_part_len));
OUT_BYTE_ARRAY(encrypted_part, encrypted_part_len);
END_CALL;
}
@@ -1718,14 +1729,14 @@ static CK_RV rpc_C_DecryptDigestUpdate(CallState * cs)
CK_BYTE_PTR encrypted_part;
CK_ULONG encrypted_part_len;
CK_BYTE_PTR part;
- CK_ULONG part_len;
+ DECLARE_CK_ULONG_PTR(part_len);
BEGIN_CALL(C_DecryptDigestUpdate);
IN_ULONG(session);
IN_BYTE_ARRAY(encrypted_part, encrypted_part_len);
IN_BYTE_BUFFER(part, part_len);
PROCESS_CALL((session, encrypted_part, encrypted_part_len, part,
- &part_len));
+ part_len));
OUT_BYTE_ARRAY(part, part_len);
END_CALL;
}
@@ -1736,14 +1747,14 @@ static CK_RV rpc_C_SignEncryptUpdate(CallState * cs)
CK_BYTE_PTR part;
CK_ULONG part_len;
CK_BYTE_PTR encrypted_part;
- CK_ULONG encrypted_part_len;
+ DECLARE_CK_ULONG_PTR(encrypted_part_len);
BEGIN_CALL(C_SignEncryptUpdate);
IN_ULONG(session);
IN_BYTE_ARRAY(part, part_len);
IN_BYTE_BUFFER(encrypted_part, encrypted_part_len);
PROCESS_CALL((session, part, part_len, encrypted_part,
- &encrypted_part_len));
+ encrypted_part_len));
OUT_BYTE_ARRAY(encrypted_part, encrypted_part_len);
END_CALL;
}
@@ -1754,14 +1765,14 @@ static CK_RV rpc_C_DecryptVerifyUpdate(CallState * cs)
CK_BYTE_PTR encrypted_part;
CK_ULONG encrypted_part_len;
CK_BYTE_PTR part;
- CK_ULONG part_len;
+ DECLARE_CK_ULONG_PTR(part_len);
BEGIN_CALL(C_DecryptVerifyUpdate);
IN_ULONG(session);
IN_BYTE_ARRAY(encrypted_part, encrypted_part_len);
IN_BYTE_BUFFER(part, part_len);
PROCESS_CALL((session, encrypted_part, encrypted_part_len, part,
- &part_len));
+ part_len));
OUT_BYTE_ARRAY(part, part_len);
END_CALL;
}
@@ -1819,7 +1830,7 @@ static CK_RV rpc_C_WrapKey(CallState * cs)
CK_OBJECT_HANDLE wrapping_key;
CK_OBJECT_HANDLE key;
CK_BYTE_PTR wrapped_key;
- CK_ULONG wrapped_key_len;
+ DECLARE_CK_ULONG_PTR(wrapped_key_len);
BEGIN_CALL(C_WrapKey);
IN_ULONG(session);
@@ -1828,7 +1839,7 @@ static CK_RV rpc_C_WrapKey(CallState * cs)
IN_ULONG(key);
IN_BYTE_BUFFER(wrapped_key, wrapped_key_len);
PROCESS_CALL((session, &mechanism, wrapping_key, key, wrapped_key,
- &wrapped_key_len));
+ wrapped_key_len));
OUT_BYTE_ARRAY(wrapped_key, wrapped_key_len);
END_CALL;
}
@@ -1893,12 +1904,12 @@ static CK_RV rpc_C_GenerateRandom(CallState * cs)
{
CK_SESSION_HANDLE session;
CK_BYTE_PTR random_data;
- CK_ULONG random_len;
+ DECLARE_CK_ULONG_PTR(random_len);
BEGIN_CALL(C_GenerateRandom);
IN_ULONG(session);
IN_BYTE_BUFFER(random_data, random_len);
- PROCESS_CALL((session, random_data, random_len));
+ PROCESS_CALL((session, random_data, *random_len));
OUT_BYTE_ARRAY(random_data, random_len);
END_CALL;
}
diff --git a/gck-rpc-message.c b/gck-rpc-message.c
index 079b21f..440d88d 100644
--- a/gck-rpc-message.c
+++ b/gck-rpc-message.c
@@ -334,13 +334,25 @@ int gck_rpc_message_write_ulong(GckRpcMessage * msg, CK_ULONG val)
return egg_buffer_add_uint64(&msg->buffer, val);
}
-int gck_rpc_message_write_byte_buffer(GckRpcMessage * msg, CK_ULONG count)
+int gck_rpc_message_write_byte_buffer(GckRpcMessage * msg, CK_BYTE_PTR arr, CK_ULONG *count_ptr)
{
+ uint8_t flags;
assert(msg);
/* Make sure this is in the right order */
assert(!msg->signature || gck_rpc_message_verify_part(msg, "fy"));
- return egg_buffer_add_uint32(&msg->buffer, count);
+
+ flags = 0;
+ if (! arr)
+ flags |= GCK_RPC_BYTE_BUFFER_NULL_DATA;
+ if (! count_ptr)
+ flags |= GCK_RPC_BYTE_BUFFER_NULL_COUNT;
+
+ egg_buffer_add_byte(&msg->buffer, flags);
+
+ egg_buffer_add_uint32(&msg->buffer, count_ptr ? *count_ptr : 0x0);
+
+ return !egg_buffer_has_error(&msg->buffer);
}
int
diff --git a/gck-rpc-module.c b/gck-rpc-module.c
index 27b4145..88313a1 100644
--- a/gck-rpc-module.c
+++ b/gck-rpc-module.c
@@ -1073,9 +1073,7 @@ proto_read_sesssion_info(GckRpcMessage * msg, CK_SESSION_INFO_PTR info)
{ _ret = CKR_HOST_MEMORY; goto _cleanup; }
#define IN_BYTE_BUFFER(arr, len) \
- if (len == NULL) \
- { _ret = CKR_ARGUMENTS_BAD; goto _cleanup; } \
- if (!gck_rpc_message_write_byte_buffer (_cs->req, arr ? *len : 0)) \
+ if (!gck_rpc_message_write_byte_buffer (_cs->req, arr, len)) \
{ _ret = CKR_HOST_MEMORY; goto _cleanup; }
#define IN_BYTE_ARRAY(arr, len) \
@@ -1720,7 +1718,16 @@ static CK_RV
rpc_C_Encrypt(CK_SESSION_HANDLE session, CK_BYTE_PTR data, CK_ULONG data_len,
CK_BYTE_PTR encrypted_data, CK_ULONG_PTR encrypted_data_len)
{
- return_val_if_fail(encrypted_data_len, CKR_ARGUMENTS_BAD);
+ return_val_if_fail(pkcs11_initialized, CKR_CRYPTOKI_NOT_INITIALIZED);
+ /* From PKCS#11 v2.01 :
+ * A call to C_Encrypt always terminates the active encryption operation
+ * unless it returns CKR_BUFFER_TOO_SMALL or is a successful call (i.e.,
+ * one which returns CKR_OK) to determine the length of the buffer
+ * needed to hold the ciphertext.
+ *
+ * Thus, we can't reject for example NULL encrypted_data_len, since then
+ * the encryption operation won't be terminated in the real PKCS#11 module.
+ */
BEGIN_CALL(C_Encrypt);
IN_ULONG(session);
diff --git a/gck-rpc-private.h b/gck-rpc-private.h
index 22f897a..02c1e9d 100644
--- a/gck-rpc-private.h
+++ b/gck-rpc-private.h
@@ -216,7 +216,7 @@ static const GckRpcCall gck_rpc_calls[] = {
#endif
#define GCK_RPC_HANDSHAKE \
- "PRIVATE-GNOME-KEYRING-PKCS11-PROTOCOL-V-1"
+ "PRIVATE-GNOME-KEYRING-PKCS11-PROTOCOL-V-2"
#define GCK_RPC_HANDSHAKE_LEN \
(sizeof (GCK_RPC_HANDSHAKE) - 1)
@@ -237,6 +237,9 @@ typedef struct _GckRpcMessage {
const char *sigverify;
} GckRpcMessage;
+#define GCK_RPC_BYTE_BUFFER_NULL_DATA 1
+#define GCK_RPC_BYTE_BUFFER_NULL_COUNT 2
+
GckRpcMessage *gck_rpc_message_new(EggBufferAllocator allocator);
void gck_rpc_message_free(GckRpcMessage * msg);
@@ -266,7 +269,7 @@ int gck_rpc_message_write_zero_string(GckRpcMessage * msg,
int gck_rpc_message_write_space_string(GckRpcMessage * msg,
CK_UTF8CHAR * buffer, CK_ULONG length);
-int gck_rpc_message_write_byte_buffer(GckRpcMessage * msg, CK_ULONG count);
+int gck_rpc_message_write_byte_buffer(GckRpcMessage * msg, CK_BYTE_PTR arr, CK_ULONG *count_ptr);
int gck_rpc_message_write_byte_array(GckRpcMessage * msg,
CK_BYTE_PTR arr, CK_ULONG num);
--
GitLab