[Cryptech-Commits] [sw/libhal] 02/02: Make previous_uuid an input-only argument to hal_rpc_pkey_match().

git at cryptech.is git at cryptech.is
Mon Oct 24 19:19:13 UTC 2016


This is an automated email from the git hooks/post-receive script.

sra at hactrn.net pushed a commit to branch ksng
in repository sw/libhal.

commit dcf3c671314b36285277073c0a3d3a09bf4d93e6
Author: Rob Austein <sra at hactrn.net>
AuthorDate: Mon Oct 24 15:15:51 2016 -0400

    Make previous_uuid an input-only argument to hal_rpc_pkey_match().
    
    In retrospect it's obvious that this never needed to be an
    input/output argument, as its value will always be the same as the
    last value in the returned array.  Doh.  So simplify the RPC and call
    sequence slightly by removing the unnecessary output value.
---
 hal.h          |  2 +-
 hal_internal.h |  6 +++---
 ks_flash.c     |  4 ++--
 ks_volatile.c  |  4 ++--
 libhal.py      |  7 ++-----
 rpc_api.c      |  2 +-
 rpc_client.c   | 17 ++++++++++++-----
 rpc_pkey.c     |  2 +-
 rpc_server.c   | 10 +++-------
 9 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/hal.h b/hal.h
index 21104c3..194948a 100644
--- a/hal.h
+++ b/hal.h
@@ -779,7 +779,7 @@ extern hal_error_t hal_rpc_pkey_match(const hal_client_handle_t client,
                                       hal_uuid_t *result,
                                       unsigned *result_len,
                                       const unsigned result_max,
-                                      hal_uuid_t *previous_uuid);
+                                      const hal_uuid_t * const previous_uuid);
 
 extern hal_error_t hal_rpc_pkey_set_attribute(const hal_pkey_handle_t pkey,
                                               const uint32_t type,
diff --git a/hal_internal.h b/hal_internal.h
index 3e6cf29..44deaf6 100644
--- a/hal_internal.h
+++ b/hal_internal.h
@@ -252,7 +252,7 @@ typedef struct {
                        hal_uuid_t *result,
                        unsigned *result_len,
                        const unsigned result_max,
-                       hal_uuid_t *previous_uuid);
+                       const hal_uuid_t * const previous_uuid);
 
   hal_error_t (*set_attribute)(const hal_pkey_handle_t pkey,
                                const uint32_t type,
@@ -499,7 +499,7 @@ struct hal_ks_driver {
                        hal_uuid_t *result,
                        unsigned *result_len,
                        const unsigned result_max,
-                       hal_uuid_t *previous_uuid);
+                       const hal_uuid_t * const previous_uuid);
 
   hal_error_t (*set_attribute)(hal_ks_t *ks,
                                hal_pkey_slot_t *slot,
@@ -624,7 +624,7 @@ static inline hal_error_t hal_ks_match(hal_ks_t *ks,
                                        hal_uuid_t *result,
                                        unsigned *result_len,
                                        const unsigned result_max,
-                                       hal_uuid_t *previous_uuid)
+                                       const hal_uuid_t * const previous_uuid)
 {
   if (ks == NULL || ks->driver == NULL || ks->driver->match == NULL)
     return HAL_ERROR_BAD_ARGUMENTS;
diff --git a/ks_flash.c b/ks_flash.c
index 7c96f90..10df54b 100644
--- a/ks_flash.c
+++ b/ks_flash.c
@@ -1156,7 +1156,7 @@ static hal_error_t ks_match(hal_ks_t *ks,
                             hal_uuid_t *result,
                             unsigned *result_len,
                             const unsigned result_max,
-                            hal_uuid_t *previous_uuid)
+                            const hal_uuid_t * const previous_uuid)
 {
   if (ks == NULL || attributes == NULL ||
       result == NULL || result_len == NULL || previous_uuid == NULL)
@@ -1240,7 +1240,7 @@ static hal_error_t ks_match(hal_ks_t *ks,
     if (attributes_len > 0 && memchr(need_attr, 1, sizeof(need_attr)) != NULL)
       continue;
 
-    *previous_uuid = result[*result_len] = db.ksi.names[b].name;
+    result[*result_len] = db.ksi.names[b].name;
     ++*result_len;
     possible = 0;
   }
diff --git a/ks_volatile.c b/ks_volatile.c
index 2018adc..e88b871 100644
--- a/ks_volatile.c
+++ b/ks_volatile.c
@@ -380,7 +380,7 @@ static hal_error_t ks_match(hal_ks_t *ks,
                             hal_uuid_t *result,
                             unsigned *result_len,
                             const unsigned result_max,
-                            hal_uuid_t *previous_uuid)
+                            const hal_uuid_t * const previous_uuid)
 {
   if (ks == NULL || attributes == NULL ||
       result == NULL || result_len == NULL || previous_uuid == NULL)
@@ -451,7 +451,7 @@ static hal_error_t ks_match(hal_ks_t *ks,
         continue;
     }
 
-    *previous_uuid = result[*result_len] = ksv->db->ksi.names[b].name;
+    result[*result_len] = ksv->db->ksi.names[b].name;
     ++*result_len;
   }
 
diff --git a/libhal.py b/libhal.py
index 911ad7c..8334f12 100644
--- a/libhal.py
+++ b/libhal.py
@@ -571,11 +571,8 @@ class HSM(object):
                    previous_uuid = UUID(int = 0), length = 512, client = 0, session = 0):
         with self.rpc(RPC_FUNC_PKEY_MATCH, session, type, curve, flags,
                       attributes, length, previous_uuid, client = client) as r:
-            x = tuple(UUID(bytes = r.unpack_bytes())
-                      for i in xrange(r.unpack_uint()))
-            y = UUID(bytes = r.unpack_bytes())
-            assert len(x) == 0 or y == x[-1]
-            return x
+            return tuple(UUID(bytes = r.unpack_bytes())
+                         for i in xrange(r.unpack_uint()))
 
     def pkey_set_attribute(self, pkey, attr_type, attr_value = None):
         if attr_value is None and isinstance(attr_type, Attribute):
diff --git a/rpc_api.c b/rpc_api.c
index 3ff814a..28bc73e 100644
--- a/rpc_api.c
+++ b/rpc_api.c
@@ -348,7 +348,7 @@ hal_error_t hal_rpc_pkey_match(const hal_client_handle_t client,
                                hal_uuid_t *result,
                                unsigned *result_len,
                                const unsigned result_max,
-                               hal_uuid_t *previous_uuid)
+                               const hal_uuid_t * const previous_uuid)
 {
   if ((attributes == NULL && attributes_len > 0) || previous_uuid == NULL ||
       result == NULL || result_len == NULL || result_max == 0)
diff --git a/rpc_client.c b/rpc_client.c
index e7e5567..959e26a 100644
--- a/rpc_client.c
+++ b/rpc_client.c
@@ -101,6 +101,16 @@ static hal_error_t read_matching_packet(const rpc_func_num_t expected_func,
 
 /*
  * RPC calls.
+ *
+ * In reading these, it helps to know that every call takes a minimum
+ * of two arguments (function code and client handle, even if the
+ * latter is just a dummy), and that every call returns a minimum of
+ * three values (function code, client handle, and return status).
+ * This may seem a bit redundant, but There Are Reasons:
+ * read_matching_packet() wants to make sure the result we're getting
+ * is from the function we thought we called, and having the client
+ * handle always present in a known place vastly simplifies the task
+ * of the client-side MUX daemon.
  */
 
 static hal_error_t get_version(uint32_t *version)
@@ -800,7 +810,7 @@ static hal_error_t pkey_remote_match(const hal_client_handle_t client,
                                      hal_uuid_t *result,
                                      unsigned *result_len,
                                      const unsigned result_max,
-                                     hal_uuid_t *previous_uuid)
+                                     const hal_uuid_t * const previous_uuid)
 {
   size_t attributes_buffer_len = 0;
   if (attributes != NULL)
@@ -809,7 +819,7 @@ static hal_error_t pkey_remote_match(const hal_client_handle_t client,
 
   uint8_t outbuf[nargs(9 + attributes_len * 2) + pad(attributes_buffer_len) + pad(sizeof(hal_uuid_t))];
   uint8_t *optr = outbuf, *olimit = outbuf + sizeof(outbuf);
-  uint8_t inbuf[nargs(5) + pad(result_max * sizeof(hal_uuid_t)) + pad(sizeof(hal_uuid_t))];
+  uint8_t inbuf[nargs(4) + pad(result_max * sizeof(hal_uuid_t))];
   const uint8_t *iptr = inbuf, *ilimit = inbuf + sizeof(inbuf);
   hal_error_t rpc_ret;
 
@@ -842,9 +852,6 @@ static hal_error_t pkey_remote_match(const hal_client_handle_t client,
       if (uuid_len != sizeof(result[i].uuid))
         return HAL_ERROR_KEY_NAME_TOO_LONG;
     }
-    check(hal_xdr_decode_buffer(&iptr, ilimit, previous_uuid->uuid, &uuid_len));
-    if (uuid_len != sizeof(previous_uuid->uuid))
-      return HAL_ERROR_KEY_NAME_TOO_LONG;
     *result_len = array_len;
   }
   return rpc_ret;
diff --git a/rpc_pkey.c b/rpc_pkey.c
index 0a53221..053c8b4 100644
--- a/rpc_pkey.c
+++ b/rpc_pkey.c
@@ -878,7 +878,7 @@ static hal_error_t pkey_local_match(const hal_client_handle_t client,
                                     hal_uuid_t *result,
                                     unsigned *result_len,
                                     const unsigned result_max,
-                                    hal_uuid_t *previous_uuid)
+                                    const hal_uuid_t * const previous_uuid)
 {
   hal_ks_t *ks = NULL;
   hal_error_t err;
diff --git a/rpc_server.c b/rpc_server.c
index 18f6823..6ed4959 100644
--- a/rpc_server.c
+++ b/rpc_server.c
@@ -672,7 +672,6 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit
     hal_session_handle_t session;
     uint32_t type, curve, attributes_len, result_max, previous_uuid_len;
     const uint8_t *previous_uuid_ptr;
-    hal_uuid_t previous_uuid;
     hal_key_flags_t flags;
     hal_error_t ret;
 
@@ -696,10 +695,10 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit
     check(hal_xdr_decode_int(iptr, ilimit, &result_max));
     check(hal_xdr_decode_buffer_in_place(iptr, ilimit, &previous_uuid_ptr, &previous_uuid_len));
 
-    if (previous_uuid_len != sizeof(previous_uuid.uuid))
+    if (previous_uuid_len != sizeof(hal_uuid_t))
         return HAL_ERROR_KEY_NAME_TOO_LONG;
 
-    memcpy(previous_uuid.uuid, previous_uuid_ptr, sizeof(previous_uuid.uuid));
+    const hal_uuid_t * const previous_uuid = (const void *) previous_uuid_ptr;
 
     hal_uuid_t result[result_max];
     unsigned result_len;
@@ -707,7 +706,7 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit
     ret = hal_rpc_local_pkey_dispatch.match(client, session, type, curve, flags,
                                             attributes, attributes_len,
                                             result, &result_len, result_max,
-                                            &previous_uuid);
+                                            previous_uuid);
 
     if (ret == HAL_OK) {
         uint8_t *optr_orig = *optr;
@@ -715,9 +714,6 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit
         for (int i = 0; ret == HAL_OK && i < result_len; ++i)
             ret = hal_xdr_encode_buffer(optr, olimit, result[i].uuid,
                                         sizeof(result[i].uuid));
-        if (ret == HAL_OK)
-            ret = hal_xdr_encode_buffer(optr, olimit, previous_uuid.uuid,
-                                        sizeof(previous_uuid.uuid));
         if (ret != HAL_OK)
             *optr = optr_orig;
     }



More information about the Commits mailing list