From be5a83543764d4d40e20dfd72ea9c70e82842700 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Fri, 19 May 2017 00:43:12 -0400 Subject: Consolidate session-state-based access control. --- pkcs11.c | 115 +++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 64 insertions(+), 51 deletions(-) diff --git a/pkcs11.c b/pkcs11.c index a17eec9..de639ab 100644 --- a/pkcs11.c +++ b/pkcs11.c @@ -774,6 +774,25 @@ static void *p11_attribute_find_value_in_template(const CK_ATTRIBUTE_TYPE type, return i < 0 ? NULL : template[i].pValue; } +/* + * Idiom for combination of p11_attribute_find_value_in_template() and + * p11_find_attribute_in_descriptor(). + */ + +static const void * +p11_attribute_find_value_in_template_or_descriptor(const p11_descriptor_t *descriptor, + const CK_ATTRIBUTE_TYPE type, + const CK_ATTRIBUTE_PTR template, + const CK_ULONG length) +{ + const int i = p11_attribute_find_in_template(type, template, length); + if (i >= 0) + return template[i].pValue; + const p11_attribute_descriptor_t * const atd = p11_find_attribute_in_descriptor(descriptor, type); + assert(atd != NULL); + return atd->value; +} + /* * Set attributes for a newly-created or newly-uploaded HSM key. */ @@ -1701,7 +1720,8 @@ static CK_RV p11_template_check_1(const CK_ATTRIBUTE_TYPE type, /* * Second pass: called once per template to check that each attribute - * required for that template has been specified exactly once. + * required for that template has been specified exactly once and that + * the session's current login state allows access with this template. */ static CK_RV p11_template_check_2(const p11_session_t *session, @@ -1711,28 +1731,55 @@ static CK_RV p11_template_check_2(const p11_session_t *session, unsigned long required_flag, unsigned long forbidden_flag) { - const CK_BBOOL *object_is_private; + const CK_BBOOL * const cka_private + = p11_attribute_find_value_in_template_or_descriptor(descriptor, CKA_PRIVATE, + template, template_length); + const CK_BBOOL * const cka_token + = p11_attribute_find_value_in_template_or_descriptor(descriptor, CKA_TOKEN, + template, template_length); CK_RV rv; - int i, j; + + assert(cka_private != NULL && cka_token != NULL); /* - * Some session states aren't allowed to play with private objects. + * Morass of session-state-specific restrictions on which objects we + * can even see, much less modify. Callers of this function need RW + * acecss to the object in question, which simplifies this a bit. */ switch (session->state) { + case CKS_RO_PUBLIC_SESSION: + /* RO access to public token objects, RW access to public session objects */ + if (*cka_private || *cka_token) + lose(CKR_USER_NOT_LOGGED_IN); + break; + + case CKS_RO_USER_FUNCTIONS: + /* RO access to all token objects, RW access to all session objects */ + if (*cka_token) + lose(CKR_SESSION_READ_ONLY); + break; + case CKS_RW_PUBLIC_SESSION: + /* RW access all public objects */ + if (*cka_private) + lose(CKR_USER_NOT_LOGGED_IN); + break; + + case CKS_RW_USER_FUNCTIONS: + /* RW acess to all objects */ + break; + case CKS_RW_SO_FUNCTIONS: - if ((object_is_private = p11_attribute_find_value_in_template(CKA_PRIVATE, template, template_length)) == NULL) { - const p11_attribute_descriptor_t * const atd = p11_find_attribute_in_descriptor(descriptor, CKA_PRIVATE); - assert(atd != NULL && atd->value != NULL); - object_is_private = atd->value; - } - if (*object_is_private) - lose(CKR_TEMPLATE_INCONSISTENT); + /* RW access to public token objects only */ + if (*cka_private || ! *cka_token) + lose(CKR_USER_NOT_LOGGED_IN); + break; + } - for (i = 0; i < descriptor->n_attributes; i++) { + for (int i = 0; i < descriptor->n_attributes; i++) { const p11_attribute_descriptor_t * const atd = &descriptor->attributes[i]; const int required_by_api = (atd->flags & required_flag) != 0; const int forbidden_by_api = (atd->flags & forbidden_flag) != 0; @@ -1741,7 +1788,7 @@ static CK_RV p11_template_check_2(const p11_session_t *session, /* Multiple entries for same attribute */ if (pos_in_template >= 0) - for (j = pos_in_template + 1; j < template_length; j++) + for (int j = pos_in_template + 1; j < template_length; j++) if (template[j].type == atd->type) lose(CKR_TEMPLATE_INCONSISTENT); @@ -1867,7 +1914,8 @@ static CK_RV p11_check_keypair_attributes(const p11_session_t *session, *private_flags |= HAL_KEY_FLAG_EXPORTABLE; /* - * Check that all required attributes have been specified. + * Check that all required attributes have been specified, + * and that our current session state allows this access. */ if ((rv = p11_template_check_2(session, @@ -1884,25 +1932,6 @@ static CK_RV p11_check_keypair_attributes(const p11_session_t *session, P11_DESCRIPTOR_FORBIDDEN_BY_GENERATE)) != CKR_OK) goto fail; - /* - * Read-only sessions can't create objects, doh. - * Well, except when they can, thanks, PKCS #11. - */ - - switch (session->state) { - case CKS_RO_PUBLIC_SESSION: - if ((public_cka_private == NULL || *public_cka_private) || - (private_cka_private == NULL || *private_cka_private)) - lose(CKR_SESSION_READ_ONLY); - /* Fall through */ - - case CKS_RO_USER_FUNCTIONS: - if ((public_cka_token != NULL && *public_cka_token) || - (private_cka_token != NULL && *private_cka_token)) - lose(CKR_SESSION_READ_ONLY); - /* Fall through */ - } - /* * If we get this far, we're happy. Maybe. */ @@ -2235,7 +2264,8 @@ static CK_RV p11_check_create_attributes(const p11_session_t *session, } /* - * Check that all required attributes have been specified. + * Check that all required attributes have been specified, + * and that our current session state allows this access. */ if ((rv = p11_template_check_2(session, descriptor, pTemplate, ulCount, @@ -2243,23 +2273,6 @@ static CK_RV p11_check_create_attributes(const p11_session_t *session, P11_DESCRIPTOR_FORBIDDEN_BY_CREATEOBJECT)) != CKR_OK) goto fail; - /* - * Read-only sessions can't create objects, doh. - * Well, except when they can, thanks, PKCS #11. - */ - - switch (session->state) { - case CKS_RO_PUBLIC_SESSION: - if (cka_private == NULL || *cka_private) - lose(CKR_SESSION_READ_ONLY); - /* Fall through */ - - case CKS_RO_USER_FUNCTIONS: - if (cka_token != NULL && *cka_token) - lose(CKR_SESSION_READ_ONLY); - /* Fall through */ - } - /* * If we get this far, we're happy. Maybe. */ -- cgit v1.2.3