More access control cleanup.

1 files changed, 78 insertions, 103 deletions

diff --git a/pkcs11.c b/pkcs11.c
index de96a79..0be7513 100644
--- a/pkcs11.c
+++ b/pkcs11.c
@@ -946,6 +946,78 @@ static void p11_attribute_apply_keyusage(hal_key_flags_t *keyusage, const CK_ATT
 
 
 /*
+ * Access rights.
+ */
+
+static CK_RV p11_check_read_access(const p11_session_t *session,
+                                   const CK_BBOOL cka_private,
+                                   const CK_BBOOL cka_token)
+{
+  if (session == NULL)
+    return CKR_SESSION_HANDLE_INVALID;
+
+  switch (session->state) {
+
+  case CKS_RO_PUBLIC_SESSION:
+    /* RO access to public token objects, RW access to public session objects */
+    return (cka_private) ? CKR_OBJECT_HANDLE_INVALID : CKR_OK;
+
+  case CKS_RO_USER_FUNCTIONS:
+    /* RO access to all token objects, RW access to all session objects */
+    return CKR_OK;
+
+  case CKS_RW_PUBLIC_SESSION:
+    /* RW access all public objects */
+    return (cka_private) ? CKR_OBJECT_HANDLE_INVALID : CKR_OK;
+
+  case CKS_RW_USER_FUNCTIONS:
+    /* RW acess to all objects */
+    return CKR_OK;
+
+  case CKS_RW_SO_FUNCTIONS:
+    /* RW access to public token objects only */
+    return (cka_private || ! cka_token) ? CKR_OBJECT_HANDLE_INVALID : CKR_OK;
+  }
+
+  return CKR_SESSION_HANDLE_INVALID;
+}
+
+static CK_RV p11_check_write_access(const p11_session_t *session,
+                                    const CK_BBOOL cka_private,
+                                    const CK_BBOOL cka_token)
+{
+  if (session == NULL)
+    return CKR_SESSION_HANDLE_INVALID;
+
+  switch (session->state) {
+
+  case CKS_RO_PUBLIC_SESSION:
+    /* RO access to public token objects, RW access to public session objects */
+    return (cka_private || cka_token) ? CKR_USER_NOT_LOGGED_IN : CKR_OK;
+
+  case CKS_RO_USER_FUNCTIONS:
+    /* RO access to all token objects, RW access to all session objects */
+    return (cka_token) ? CKR_SESSION_READ_ONLY : CKR_OK;
+
+  case CKS_RW_PUBLIC_SESSION:
+    /* RW access all public objects */
+    return (cka_private) ? CKR_USER_NOT_LOGGED_IN : CKR_OK;
+
+  case CKS_RW_USER_FUNCTIONS:
+    /* RW acess to all objects */
+    return CKR_OK;
+
+  case CKS_RW_SO_FUNCTIONS:
+    /* RW access to public token objects only */
+    return (cka_private || ! cka_token) ? CKR_USER_NOT_LOGGED_IN : CKR_OK;
+  }
+
+  return CKR_SESSION_HANDLE_INVALID;
+}
+
+
+
+/*
  * Object methods.
  */
 
@@ -1131,66 +1203,6 @@ static int p11_object_pkey_open(const p11_session_t *session,
 }
 
 /*
- * Check access rights for an object.
- */
-
-typedef enum { p11_object_access_read, p11_object_access_write } p11_object_access_t;
-
-static CK_RV p11_object_check_rights(const p11_session_t *session,
-                                     const CK_OBJECT_HANDLE object_handle,
-                                     const p11_object_access_t rights,
-                                     const CK_BBOOL cka_private,
-                                     const CK_BBOOL cka_token)
-{
-  CK_RV rv;
-
-  if (session == NULL)
-    lose(CKR_SESSION_HANDLE_INVALID);
-
-  /*
-   * Read-only sessions are, um, read-only.  Well, except, in PKCS #11,
-   * read-only only sort of means what you might expect.
-   */
-
-  if (rights == p11_object_access_write) {
-    switch (session->state) {
-    case CKS_RO_PUBLIC_SESSION:
-      if (cka_private)
-        lose(CKR_SESSION_READ_ONLY);
-      /* Fall through */
-    case CKS_RO_USER_FUNCTIONS:
-      if (cka_token)
-        lose(CKR_SESSION_READ_ONLY);
-      /* Fall through */
-    }
-  }
-
-  /*
-   * Private objects don't exist for sessions in the wrong state.
-   */
-
-  switch (session->state) {
-  case CKS_RO_PUBLIC_SESSION:
-  case CKS_RW_PUBLIC_SESSION:
-  case CKS_RW_SO_FUNCTIONS:
-    if (cka_private)
-      lose(CKR_OBJECT_HANDLE_INVALID);
-  }
-
-#warning Want session-object visibility sanity check here?
-  /*
-   * At this point the old code checked for objects which are not
-   * supposed to be visible to this session.  In theory, the keystore
-   * now handles that, but we might want a sanity check here too.
-   */
-
-  rv = CKR_OK;
-
- fail:
-  return rv;
-}
-
-/*
  * Create pkeys to go with PKCS #11 key objects loaded by C_CreateObject().
  */
 
@@ -1747,37 +1759,8 @@ static CK_RV p11_template_check_2(const p11_session_t *session,
    * 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:
-    /* RW access to public token objects only */
-    if (*cka_private || ! *cka_token)
-      lose(CKR_USER_NOT_LOGGED_IN);
-    break;
-
-  }
+  if ((rv = p11_check_write_access(session, *cka_private, *cka_token)) != CKR_OK)
+    goto fail;
 
   for (int i = 0; i < descriptor->n_attributes; i++) {
     const p11_attribute_descriptor_t * const atd = &descriptor->attributes[i];
@@ -3174,9 +3157,7 @@ CK_RV C_DestroyObject(CK_SESSION_HANDLE hSession,
   cka_private  = *(CK_BBOOL*) attributes[0].value;
   cka_token    = *(CK_BBOOL*) attributes[1].value;
 
-  rv = p11_object_check_rights(session, hObject, p11_object_access_write, cka_private, cka_token);
-
-  if (rv != CKR_OK)
+  if ((rv = p11_check_write_access(session, cka_private, cka_token)) != CKR_OK)
     goto fail;
 
   if (!hal_check(hal_rpc_pkey_delete(pkey)))
@@ -3237,9 +3218,7 @@ CK_RV C_GetAttributeValue(CK_SESSION_HANDLE hSession,
     cka_token    = *(CK_BBOOL*)        attributes[2].value;
     cka_key_type = *(CK_KEY_TYPE*)     attributes[3].value;
 
-    rv = p11_object_check_rights(session, hObject, p11_object_access_read, cka_private, cka_token);
-
-    if (rv != CKR_OK)
+    if ((rv = p11_check_read_access(session, cka_private, cka_token)) != CKR_OK)
       goto fail;
 
     descriptor = p11_descriptor_from_key_type(cka_class, cka_key_type);
@@ -3735,9 +3714,7 @@ CK_RV C_SignInit(CK_SESSION_HANDLE hSession,
   cka_private  = *(CK_BBOOL*)    attributes[2].value;
   cka_token    = *(CK_BBOOL*)    attributes[3].value;
 
-  rv = p11_object_check_rights(session, hKey, p11_object_access_read, cka_private, cka_token);
-
-  if (rv != CKR_OK)
+  if ((rv = p11_check_read_access(session, cka_private, cka_token)) != CKR_OK)
     goto fail;
 
   if (!cka_sign)
@@ -3972,9 +3949,7 @@ CK_RV C_VerifyInit(CK_SESSION_HANDLE hSession,
   cka_private  = *(CK_BBOOL*)    attributes[2].value;
   cka_token    = *(CK_BBOOL*)    attributes[3].value;
 
-  rv = p11_object_check_rights(session, hKey, p11_object_access_read, cka_private, cka_token);
-
-  if (rv != CKR_OK)
+  if ((rv = p11_check_read_access(session, cka_private, cka_token)) != CKR_OK)
     goto fail;
 
   if (!cka_verify)