aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--hal.h1
-rw-r--r--ks_volatile.c21
-rw-r--r--libhal.py17
-rw-r--r--rpc_pkey.c86
4 files changed, 112 insertions, 13 deletions
diff --git a/hal.h b/hal.h
index 194948a..db4038d 100644
--- a/hal.h
+++ b/hal.h
@@ -692,6 +692,7 @@ typedef uint32_t hal_key_flags_t;
#define HAL_KEY_FLAG_USAGE_KEYENCIPHERMENT (1 << 1)
#define HAL_KEY_FLAG_USAGE_DATAENCIPHERMENT (1 << 2)
#define HAL_KEY_FLAG_TOKEN (1 << 3)
+#define HAL_KEY_FLAG_PUBLIC (1 << 4)
extern hal_error_t hal_rpc_pkey_load(const hal_client_handle_t client,
const hal_session_handle_t session,
diff --git a/ks_volatile.c b/ks_volatile.c
index e88b871..0f53c11 100644
--- a/ks_volatile.c
+++ b/ks_volatile.c
@@ -106,14 +106,29 @@ static inline ks_t *ks_to_ksv(hal_ks_t *ks)
return (ks_t *) ks;
}
+/*
+ * Check whether the current session can see a particular key. One
+ * might expect this to be based on whether the session matches, and
+ * indeed it would be in a sane world, but in the world of PKCS #11,
+ * keys belong to sessions, are visible to other sessions, and may
+ * even be modifiable by other sessions, but softly and silently
+ * vanish away when the original creating session is destroyed.
+ *
+ * In our terms, this means that visibility of session objects is
+ * determined only by the client handle, so taking the session handle
+ * as an argument here isn't really necessary, but we've flipflopped
+ * on that enough times that at least for now I'd prefer to leave the
+ * session handle here and not have to revise all the RPC calls again.
+ * Remove it at some later date and redo the RPC calls if we manage to
+ * avoid revising this yet again.
+ */
+
static inline int key_visible_to_session(const ks_t * const ksv,
const hal_client_handle_t client,
const hal_session_handle_t session,
const ks_key_t * const k)
{
- return (!ksv->per_session || client.handle == HAL_HANDLE_NONE ||
- (k->client.handle == client.handle &&
- k->session.handle == session.handle));
+ return !ksv->per_session || client.handle == HAL_HANDLE_NONE || k->client.handle == client.handle;
}
static inline void *gnaw(uint8_t **mem, size_t *len, const size_t size)
diff --git a/libhal.py b/libhal.py
index 8334f12..c35a51a 100644
--- a/libhal.py
+++ b/libhal.py
@@ -224,7 +224,7 @@ HAL_KEY_FLAG_USAGE_DIGITALSIGNATURE = (1 << 0)
HAL_KEY_FLAG_USAGE_KEYENCIPHERMENT = (1 << 1)
HAL_KEY_FLAG_USAGE_DATAENCIPHERMENT = (1 << 2)
HAL_KEY_FLAG_TOKEN = (1 << 3)
-
+HAL_KEY_FLAG_PUBLIC = (1 << 4)
class Attribute(object):
@@ -590,10 +590,17 @@ class HSM(object):
if __name__ == "__main__":
+ import argparse
+
def hexstr(s):
return "".join("{:02x}".format(ord(c)) for c in s)
- hsm = HSM()
+ parser = argparse.ArgumentParser()
+ parser.add_argument("--device", default = os.getenv("CRYPTECH_RPC_CLIENT_SERIAL_DEVICE", "/dev/ttyUSB0"))
+ parser.add_argument("--pin", default = "fnord")
+ args = parser.parse_args()
+
+ hsm = HSM(device = args.device)
print "Version:", hex(hsm.get_version())
@@ -607,6 +614,10 @@ if __name__ == "__main__":
h.update("Hi, Dad")
print "HMAC:", hexstr(h.finalize())
+ print "Logging in"
+ hsm.login(HAL_USER_NORMAL, args.pin)
+
+ print "Generating key"
k = hsm.pkey_generate_ec(HAL_CURVE_P256)
print "PKey: {0.uuid} {0.key_type} {0.key_flags} {1}".format(k, hexstr(k.public_key))
hsm.pkey_close(k)
@@ -621,3 +632,5 @@ if __name__ == "__main__":
k = hsm.pkey_find(k.uuid)
hsm.pkey_delete(k)
+
+ hsm.logout()
diff --git a/rpc_pkey.c b/rpc_pkey.c
index 053c8b4..493dec8 100644
--- a/rpc_pkey.c
+++ b/rpc_pkey.c
@@ -101,18 +101,61 @@ static inline hal_pkey_slot_t *find_handle(const hal_pkey_handle_t handle)
return NULL;
}
-#warning Still need access control on pkey objects based on current login state
/*
- * This would be simple, except for PKCS #11 non-token objects (CKA_TOKEN = CK_FALSE).
- * Need to check detailed PKCS #11 rules, but, from memory, we may be supposed to allow
- * access to non-token objects even when not logged in. Maybe. Rules are complex.
+ * Access rules are a bit complicated, mostly due to PKCS #11.
*
- * I think the libhal translation of this resolves around HAL_KEY_FLAG_TOKEN.
- * For token objects, we insist on being logged in properly; for non-token
- * objects, we do whatever silly thing PKCS #11 wants us to do, probably
- * defaulting to requiring login if PKCS #11 gives us a choice.
+ * The simple, obvious rule would be that one must be logged in as
+ * HAL_USER_NORMAL to create, see, use, or delete a key, full stop.
+ *
+ * That's almost the rule that PKCS #11 follows for so-called
+ * "private" objects (CKA_PRIVATE = CK_TRUE), but PKCS #11 has a more
+ * model which not only allows wider visibility to "public" objects
+ * (CKA_PRIVATE = CK_FALSE) but also allows write access to "public
+ * session" (CKA_PRIVATE = CK_FALSE, CKA_TOKEN = CK_FALSE) objects
+ * regardless of login state.
+ *
+ * PKCS #11 also has a concept of read-only sessions, which we don't
+ * bother to implement at all on the HSM, since the PIN is required to
+ * be the same as for the corresponding read-write session, so this
+ * would just be additional compexity without adding any security on
+ * the HSM; the PKCS #11 library still has to support read-only
+ * sessions, but that's not our problem here.
+ *
+ * In general, non-PKCS #11 users of this API should probably never
+ * set HAL_KEY_FLAG_PUBLIC, in which case they'll get the simple rule.
+ *
+ * Note that keystore drivers may need to implement additional
+ * additional checks, eg, ks_volatile needs to enforce the rule that
+ * session objects are only visible to the client which created them
+ * (not the session, that would be too simple, thanks PKCS #11). In
+ * practice, this should not be a serious problem, since such checks
+ * will likely only apply to existing objects. The thing we really
+ * want to avoid is doing all the work to create a large key only to
+ * have the keystore driver reject access at the end, but since, by
+ * definition, that only occurs when creating new objects, the access
+ * decision doesn't depend on preexisting data, so the rules here
+ * should suffice. That's the theory, anyway, if this is wrong we may
+ * need to refactor.
*/
+static inline hal_error_t check_readable(const hal_client_handle_t client,
+ const hal_key_flags_t flags)
+{
+ if ((flags & HAL_KEY_FLAG_PUBLIC) != 0)
+ return HAL_OK;
+
+ return hal_rpc_is_logged_in(client, HAL_USER_NORMAL);
+}
+
+static inline hal_error_t check_writable(const hal_client_handle_t client,
+ const hal_key_flags_t flags)
+{
+ if ((flags & (HAL_KEY_FLAG_TOKEN | HAL_KEY_FLAG_PUBLIC)) == HAL_KEY_FLAG_PUBLIC)
+ return HAL_OK;
+
+ return hal_rpc_is_logged_in(client, HAL_USER_NORMAL);
+}
+
/*
* Pad an octet string with PKCS #1.5 padding for use with RSA.
*
@@ -186,6 +229,9 @@ static hal_error_t pkey_local_load(const hal_client_handle_t client,
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_writable(client, flags)) != HAL_OK)
+ return err;
+
if ((slot = alloc_slot(flags)) == NULL)
return HAL_ERROR_NO_KEY_SLOTS_AVAILABLE;
@@ -230,6 +276,9 @@ static hal_error_t pkey_local_find(const hal_client_handle_t client,
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_readable(client, flags)) != HAL_OK)
+ return err;
+
if ((slot = alloc_slot(flags)) == NULL)
return HAL_ERROR_NO_KEY_SLOTS_AVAILABLE;
@@ -272,6 +321,9 @@ static hal_error_t pkey_local_generate_rsa(const hal_client_handle_t client,
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_writable(client, flags)) != HAL_OK)
+ return err;
+
if ((slot = alloc_slot(flags)) == NULL)
return HAL_ERROR_NO_KEY_SLOTS_AVAILABLE;
@@ -333,6 +385,9 @@ static hal_error_t pkey_local_generate_ec(const hal_client_handle_t client,
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_writable(client, flags)) != HAL_OK)
+ return err;
+
if ((slot = alloc_slot(flags)) == NULL)
return HAL_ERROR_NO_KEY_SLOTS_AVAILABLE;
@@ -403,6 +458,9 @@ static hal_error_t pkey_local_delete(const hal_pkey_handle_t pkey)
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_writable(slot->client_handle, slot->flags)) != HAL_OK)
+ return err;
+
if ((err = ks_open_from_flags(&ks, slot->flags)) == HAL_OK &&
(err = hal_ks_delete(ks, slot)) == HAL_OK)
err = hal_ks_close(ks);
@@ -859,6 +917,9 @@ static hal_error_t pkey_local_list(const hal_client_handle_t client,
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_readable(client, flags)) != HAL_OK)
+ return err;
+
if ((err = ks_open_from_flags(&ks, flags)) == HAL_OK &&
(err = hal_ks_list(ks, client, session, result, result_len, result_max)) == HAL_OK)
err = hal_ks_close(ks);
@@ -883,6 +944,9 @@ static hal_error_t pkey_local_match(const hal_client_handle_t client,
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_readable(client, flags)) != HAL_OK)
+ return err;
+
if ((err = ks_open_from_flags(&ks, flags)) == HAL_OK &&
(err = hal_ks_match(ks, client, session, type, curve, flags, attributes, attributes_len,
result, result_len, result_max, previous_uuid)) == HAL_OK)
@@ -906,6 +970,9 @@ static hal_error_t pkey_local_set_attribute(const hal_pkey_handle_t pkey,
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_writable(slot->client_handle, slot->flags)) != HAL_OK)
+ return err;
+
if ((err = ks_open_from_flags(&ks, slot->flags)) == HAL_OK &&
(err = hal_ks_set_attribute(ks, slot, type, value, value_len)) == HAL_OK)
err = hal_ks_close(ks);
@@ -949,6 +1016,9 @@ static hal_error_t pkey_local_delete_attribute(const hal_pkey_handle_t pkey,
hal_ks_t *ks = NULL;
hal_error_t err;
+ if ((err = check_writable(slot->client_handle, slot->flags)) != HAL_OK)
+ return err;
+
if ((err = ks_open_from_flags(&ks, slot->flags)) == HAL_OK &&
(err = hal_ks_delete_attribute(ks, slot, type)) == HAL_OK)
err = hal_ks_close(ks);