Skip to content

Commit

Permalink
ta: sks: fix precedence of invalid session handle over other errors
Browse files Browse the repository at this point in the history 
As per PKCS#11 specification, an invalid session handle argument should
be reported with CKR_SESSION_HANDLE_INVALID over any other error
found when executing the requested command. This change complies
with this by introducing helper serialargs_get_session() that
gets the session handle value from the input argument buffer and
finds related session instance. This allows to factorize early
reporting of invalid session handles when parsing client input
arguments over the several TA commands that gets a session handle
argument.

Signed-off-by: Etienne Carriere <[email protected]>
  • Loading branch information
@etienne-lms
etienne-lms committed Apr 6, 2020
1 parent 301ea48 commit 6a1e6d9
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 127 deletions.
45 changes: 8 additions & 37 deletions ta/pkcs11/src/object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,6 @@ uint32_t entry_destroy_object(struct pkcs11_client *client,
TEE_PARAM_TYPE_NONE);
TEE_Param *ctrl = &params[0];
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
uint32_t object_handle = 0;
struct pkcs11_session *session = NULL;
struct pkcs11_object *object = NULL;
Expand All @@ -282,7 +281,7 @@ uint32_t entry_destroy_object(struct pkcs11_client *client,

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

Expand All @@ -293,10 +292,6 @@ uint32_t entry_destroy_object(struct pkcs11_client *client,
if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

if (session_is_active(session))
return PKCS11_CKR_OPERATION_ACTIVE;

Expand All @@ -308,7 +303,7 @@ uint32_t entry_destroy_object(struct pkcs11_client *client,
handle_put(&session->object_handle_db, object_handle);

DMSG("PKCS11 session %"PRIu32": destroy object %#"PRIx32,
session_handle, object_handle);
session->handle, object_handle);

return rv;
}
Expand Down Expand Up @@ -424,7 +419,6 @@ uint32_t entry_find_objects_init(struct pkcs11_client *client,
TEE_Param *ctrl = &params[0];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;
struct pkcs11_object_head *template = NULL;
struct pkcs11_attrs_head *req_attrs = NULL;
Expand All @@ -436,7 +430,7 @@ uint32_t entry_find_objects_init(struct pkcs11_client *client,

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

Expand All @@ -449,12 +443,6 @@ uint32_t entry_find_objects_init(struct pkcs11_client *client,
goto bail;
}

session = pkcs11_handle2session(session_handle, client);
if (!session) {
rv = PKCS11_CKR_SESSION_HANDLE_INVALID;
goto bail;
}

/* Search objects only if no operation is on-going */
if (session_is_active(session)) {
rv = PKCS11_CKR_OPERATION_ACTIVE;
Expand Down Expand Up @@ -606,7 +594,6 @@ uint32_t entry_find_objects(struct pkcs11_client *client,
TEE_Param *out = &params[2];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;
struct pkcs11_find_objects *ctx = NULL;
char *out_handles = NULL;
Expand All @@ -622,17 +609,13 @@ uint32_t entry_find_objects(struct pkcs11_client *client,

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

ctx = session->find_ctx;

/*
Expand Down Expand Up @@ -665,7 +648,7 @@ uint32_t entry_find_objects(struct pkcs11_client *client,
/* Update output buffer according the number of handles provided */
out->memref.size = count * sizeof(uint32_t);

DMSG("PKCS11 session %"PRIu32": finding objects", session_handle);
DMSG("PKCS11 session %"PRIu32": finding objects", session->handle);

return PKCS11_CKR_OK;
}
Expand All @@ -686,25 +669,20 @@ uint32_t entry_find_objects_final(struct pkcs11_client *client,
TEE_Param *ctrl = &params[0];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 9;
struct pkcs11_session *session = NULL;

if (!client || ptypes != exp_pt)
return PKCS11_CKR_ARGUMENTS_BAD;

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

if (!session->find_ctx)
return PKCS11_CKR_OPERATION_NOT_INITIALIZED;

Expand All @@ -724,7 +702,6 @@ uint32_t entry_get_attribute_value(struct pkcs11_client *client,
TEE_Param *out = &params[2];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;
struct pkcs11_object_head *template = NULL;
struct pkcs11_object *obj = NULL;
Expand All @@ -741,7 +718,7 @@ uint32_t entry_get_attribute_value(struct pkcs11_client *client,

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

Expand All @@ -758,12 +735,6 @@ uint32_t entry_get_attribute_value(struct pkcs11_client *client,
goto bail;
}

session = pkcs11_handle2session(session_handle, client);
if (!session) {
rv = PKCS11_CKR_SESSION_HANDLE_INVALID;
goto bail;
}

obj = pkcs11_handle2object(object_handle, session);
if (!obj) {
rv = PKCS11_CKR_ARGUMENTS_BAD;
Expand Down Expand Up @@ -864,7 +835,7 @@ uint32_t entry_get_attribute_value(struct pkcs11_client *client,
TEE_MemMove(out->memref.buffer, template, out->memref.size);

DMSG("PKCS11 session %"PRIu32": get attributes %#"PRIx32,
session_handle, object_handle);
session->handle, object_handle);

bail:
TEE_Free(template);
Expand Down
50 changes: 10 additions & 40 deletions ta/pkcs11/src/pkcs11_token.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -947,25 +947,20 @@ uint32_t entry_ck_close_session(struct pkcs11_client *client,
TEE_Param *ctrl = &params[0];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;

if (!client || ptypes != exp_pt)
return PKCS11_CKR_ARGUMENTS_BAD;

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

close_ck_session(session);

return PKCS11_CKR_OK;
Expand Down Expand Up @@ -1022,7 +1017,6 @@ uint32_t entry_ck_session_info(struct pkcs11_client *client,
TEE_Param *out = &params[2];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;
struct pkcs11_session_info info = {
.flags = PKCS11_CKFSS_SERIAL_SESSION,
Expand All @@ -1033,17 +1027,13 @@ uint32_t entry_ck_session_info(struct pkcs11_client *client,

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

info.slot_id = get_token_id(session->token);
info.state = session->state;
if (pkcs11_session_is_read_write(session))
Expand Down Expand Up @@ -1143,7 +1133,6 @@ uint32_t entry_init_pin(struct pkcs11_client *client,
TEE_Param *ctrl = &params[0];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;
uint32_t pin_size = 0;
void *pin = NULL;
Expand All @@ -1153,7 +1142,7 @@ uint32_t entry_init_pin(struct pkcs11_client *client,

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

Expand All @@ -1168,16 +1157,12 @@ uint32_t entry_init_pin(struct pkcs11_client *client,
if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

if (!pkcs11_session_is_so(session))
return PKCS11_CKR_USER_NOT_LOGGED_IN;

assert(session->token->db_main->flags & PKCS11_CKFT_TOKEN_INITIALIZED);

DMSG("PKCS11 session %"PRIu32": init PIN", session_handle);
DMSG("PKCS11 session %"PRIu32": init PIN", session->handle);

return set_pin(session, pin, pin_size, PKCS11_CKU_USER);
}
Expand Down Expand Up @@ -1357,7 +1342,6 @@ uint32_t entry_set_pin(struct pkcs11_client *client,
TEE_Param *ctrl = &params[0];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;
uint32_t old_pin_size = 0;
uint32_t pin_size = 0;
Expand All @@ -1369,7 +1353,7 @@ uint32_t entry_set_pin(struct pkcs11_client *client,

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

Expand All @@ -1392,10 +1376,6 @@ uint32_t entry_set_pin(struct pkcs11_client *client,
if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

if (!pkcs11_session_is_read_write(session))
return PKCS11_CKR_SESSION_READ_ONLY;

Expand All @@ -1419,7 +1399,7 @@ uint32_t entry_set_pin(struct pkcs11_client *client,
if (rv)
return rv;

DMSG("PKCS11 session %"PRIu32": set PIN", session_handle);
DMSG("PKCS11 session %"PRIu32": set PIN", session->handle);

return set_pin(session, pin, pin_size, PKCS11_CKU_USER);
}
Expand All @@ -1434,7 +1414,6 @@ uint32_t entry_login(struct pkcs11_client *client,
TEE_Param *ctrl = &params[0];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;
struct pkcs11_session *sess = NULL;
uint32_t user_type = 0;
Expand All @@ -1446,7 +1425,7 @@ uint32_t entry_login(struct pkcs11_client *client,

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

Expand All @@ -1465,10 +1444,6 @@ uint32_t entry_login(struct pkcs11_client *client,
if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

switch ((enum pkcs11_user_type)user_type) {
case PKCS11_CKU_SO:
if (pkcs11_session_is_so(session))
Expand Down Expand Up @@ -1540,7 +1515,7 @@ uint32_t entry_login(struct pkcs11_client *client,
}

if (!rv)
DMSG("PKCS11 session %"PRIu32": login", session_handle);
DMSG("PKCS11 session %"PRIu32": login", session->handle);

return rv;
}
Expand All @@ -1555,31 +1530,26 @@ uint32_t entry_logout(struct pkcs11_client *client,
TEE_Param *ctrl = &params[0];
uint32_t rv = 0;
struct serialargs ctrlargs = { };
uint32_t session_handle = 0;
struct pkcs11_session *session = NULL;

if (!client || ptypes != exp_pt)
return PKCS11_CKR_ARGUMENTS_BAD;

serialargs_init(&ctrlargs, ctrl->memref.buffer, ctrl->memref.size);

rv = serialargs_get(&ctrlargs, &session_handle, sizeof(uint32_t));
rv = serialargs_get_session(&ctrlargs, client, &session);
if (rv)
return rv;

if (serialargs_remaining_bytes(&ctrlargs))
return PKCS11_CKR_ARGUMENTS_BAD;

session = pkcs11_handle2session(session_handle, client);
if (!session)
return PKCS11_CKR_SESSION_HANDLE_INVALID;

if (pkcs11_session_is_public(session))
return PKCS11_CKR_USER_NOT_LOGGED_IN;

session_logout(session);

DMSG("PKCS11 session %"PRIu32": logout", session_handle);
DMSG("PKCS11 session %"PRIu32": logout", session->handle);

return PKCS11_CKR_OK;
}
Loading

0 comments on commit 6a1e6d9

Please sign in to comment.