Consolidate session-state-based access control.

1 files changed, 64 insertions, 51 deletions

diff --git a/pkcs11.c b/pkcs11.c
index a17eec9..de639ab 100644
--- a/pkcs11.c
+++ b/pkcs11.c
@@ -775,6 +775,25 @@ static void *p11_attribute_find_value_in_template(const CK_ATTRIBUTE_TYPE type,
 }
 
 /*
+ * 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,
@@ -1885,25 +1933,6 @@ static CK_RV p11_check_keypair_attributes(const p11_session_t *session,
     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,
@@ -2244,23 +2274,6 @@ static CK_RV p11_check_create_attributes(const p11_session_t *session,
     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.
    */