From 4935de246254df236caf8487d15028d05ad88d94 Mon Sep 17 00:00:00 2001

From: Per Nilsson <per.nilsson@yubico.com>

Date: Fri, 27 Jan 2023 10:09:11 +0100

Subject: [PATCH] Fix type of id (#312)

---

 pkcs11/util_pkcs11.c    | 17 ++++-------------

 pkcs11/util_pkcs11.h    |  4 ++--

 pkcs11/yubihsm_pkcs11.c | 24 ++++++++----------------

 3 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/pkcs11/util_pkcs11.c b/pkcs11/util_pkcs11.c

index 5834a5bd..db5f83ca 100644

--- a/pkcs11/util_pkcs11.c

+++ b/pkcs11/util_pkcs11.c

@@ -4294,7 +4294,7 @@ CK_RV parse_hmac_template(CK_ATTRIBUTE_PTR pTemplate, CK_ULONG ulCount,

 }

 CK_RV parse_meta_id_template(pkcs11_meta_object *pkcs11meta, bool pubkey,

-                             int *id, uint8_t *value, size_t value_len) {

+                             uint16_t *id, uint8_t *value, size_t value_len) {

   if (value_len != 2) {

     if (pubkey) {

       pkcs11meta->cka_id_pubkey.len = value_len;

@@ -4307,10 +4307,6 @@ CK_RV parse_meta_id_template(pkcs11_meta_object *pkcs11meta, bool pubkey,

   } else {

     if (!pubkey) {

       *id = parse_id_value(value, value_len);

-      if (*id == -1) {

-        DBG_ERR("CKA_ID invalid in template");

-        return CKR_ATTRIBUTE_VALUE_INVALID;

-      }

     }

   }

@@ -4343,7 +4339,6 @@ CK_RV parse_rsa_generate_template(CK_ATTRIBUTE_PTR pPublicKeyTemplate,

   uint8_t *e = NULL;

   CK_RV rv;

-  int id = 0;

   memset(template->label, 0, sizeof(template->label));

   for (CK_ULONG i = 0; i < ulPublicKeyAttributeCount; i++) {

@@ -4482,14 +4477,13 @@ CK_RV parse_rsa_generate_template(CK_ATTRIBUTE_PTR pPublicKeyTemplate,

         break;

       case CKA_ID: {

-        rv = parse_meta_id_template(pkcs11meta, false, &id,

+        rv = parse_meta_id_template(pkcs11meta, false, &template->id,

                                     pPrivateKeyTemplate[i].pValue,

                                     pPrivateKeyTemplate[i].ulValueLen);

         if (rv != CKR_OK) {

           DBG_ERR("Failed to parse CKA_ID in PrivateKeyTemplate");

           return rv;

         }

-        template->id = id;

       } break;

       case CKA_DECRYPT:

@@ -4572,7 +4566,7 @@ CK_RV parse_rsa_generate_template(CK_ATTRIBUTE_PTR pPublicKeyTemplate,

   return CKR_OK;

 }

-int parse_id_value(void *value, CK_ULONG len) {

+uint16_t parse_id_value(void *value, CK_ULONG len) {

   switch (len) {

     case 0:

       return 0;

@@ -4596,7 +4590,6 @@ CK_RV parse_ec_generate_template(CK_ATTRIBUTE_PTR pPublicKeyTemplate,

   uint8_t *ecparams = NULL;

   uint16_t ecparams_len = 0;

   CK_RV rv;

-  int id;

   memset(template->label, 0, sizeof(template->label));

   for (CK_ULONG i = 0; i < ulPublicKeyAttributeCount; i++) {

@@ -4701,15 +4694,13 @@ CK_RV parse_ec_generate_template(CK_ATTRIBUTE_PTR pPublicKeyTemplate,

         break;

       case CKA_ID: {

-        rv = parse_meta_id_template(pkcs11meta, false, &id,

+        rv = parse_meta_id_template(pkcs11meta, false, &template->id,

                                     pPrivateKeyTemplate[i].pValue,

                                     pPrivateKeyTemplate[i].ulValueLen);

         if (rv != CKR_OK) {

           DBG_ERR("Failed to parse CKA_ID in PrivateKeyTemplate");

           return rv;

         }

-        template->id = id;

-

       } break;

       case CKA_SIGN:

diff --git a/pkcs11/util_pkcs11.h b/pkcs11/util_pkcs11.h

index d8026e57..5a91ee34 100644

--- a/pkcs11/util_pkcs11.h

+++ b/pkcs11/util_pkcs11.h

@@ -151,7 +151,7 @@ CK_RV parse_ec_generate_template(CK_ATTRIBUTE_PTR pPublicKeyTemplate,

                                  yubihsm_pkcs11_object_template *template,

                                  pkcs11_meta_object *pkcs11meta);

-int parse_id_value(void *value, CK_ULONG len);

+uint16_t parse_id_value(void *value, CK_ULONG len);

 CK_RV populate_template(int type, void *object, CK_ATTRIBUTE_PTR pTemplate,

                         CK_ULONG ulCount, yubihsm_pkcs11_session *session);

@@ -176,7 +176,7 @@ bool match_meta_attributes(yubihsm_pkcs11_session *session,

 bool is_meta_object(yh_object_descriptor *object);

 CK_RV parse_meta_id_template(pkcs11_meta_object *pkcs11meta, bool public,

-                             int *id, uint8_t *value, size_t value_len);

+                             uint16_t *id, uint8_t *value, size_t value_len);

 void parse_meta_label_template(yubihsm_pkcs11_object_template *template,

                                pkcs11_meta_object *pkcs11meta, bool public,

                                uint8_t *value, size_t value_len);

diff --git a/pkcs11/yubihsm_pkcs11.c b/pkcs11/yubihsm_pkcs11.c

index 48b3bf46..6f715e01 100644

--- a/pkcs11/yubihsm_pkcs11.c

+++ b/pkcs11/yubihsm_pkcs11.c

@@ -1383,10 +1383,6 @@ CK_DEFINE_FUNCTION(CK_RV, C_CreateObject)

             id.d = 0;

           } else {

             id.d = parse_id_value(pTemplate[i].pValue, pTemplate[i].ulValueLen);

-            if (id.d == (CK_ULONG) -1) {

-              rv = CKR_ATTRIBUTE_VALUE_INVALID;

-              goto c_co_out;

-            }

           }

           id.set = true;

         } else {

@@ -2200,7 +2196,7 @@ CK_DEFINE_FUNCTION(CK_RV, C_SetAttributeValue)

             }

           }

         } else {

-          int new_id =

+          uint16_t new_id =

             parse_id_value(pTemplate[i].pValue, pTemplate[i].ulValueLen);

           if (pTemplate[i].ulValueLen != 2 || new_id != object->object.id) {

             if (object->object.type == YH_PUBLIC_KEY) {

@@ -2360,7 +2356,6 @@ CK_DEFINE_FUNCTION(CK_RV, C_FindObjectsInit)

   yh_rc rc = YHR_SUCCESS;

-  int id = 0;

   uint8_t type = 0;

   uint16_t domains = 0;

   yh_capabilities capabilities = {{0}};

@@ -2527,7 +2522,7 @@ CK_DEFINE_FUNCTION(CK_RV, C_FindObjectsInit)

       yh_object_descriptor

         tmp_objects[YH_MAX_ITEMS_COUNT + MAX_ECDH_SESSION_KEYS] = {0};

       size_t tmp_n_objects = YH_MAX_ITEMS_COUNT + MAX_ECDH_SESSION_KEYS;

-      rc = yh_util_list_objects(session->slot->device_session, id, 0, domains,

+      rc = yh_util_list_objects(session->slot->device_session, 0, 0, domains,

                                 &capabilities, algorithm, label, tmp_objects,

                                 &tmp_n_objects);

       if (rc != YHR_SUCCESS) {

@@ -2563,7 +2558,7 @@ CK_DEFINE_FUNCTION(CK_RV, C_FindObjectsInit)

         } else {

           yh_object_descriptor tmp_objects[YH_MAX_ITEMS_COUNT] = {0};

           size_t tmp_n_objects = sizeof(tmp_objects);

-          rc = yh_util_list_objects(session->slot->device_session, id,

+          rc = yh_util_list_objects(session->slot->device_session, 0,

                                     YH_OPAQUE, domains, &capabilities,

                                     YH_ALGO_OPAQUE_X509_CERTIFICATE, label,

                                     tmp_objects, &tmp_n_objects);

@@ -2599,7 +2594,7 @@ CK_DEFINE_FUNCTION(CK_RV, C_FindObjectsInit)

         yh_object_descriptor

           tmp_objects[YH_MAX_ITEMS_COUNT + MAX_ECDH_SESSION_KEYS] = {0};

         size_t tmp_n_objects = YH_MAX_ITEMS_COUNT + MAX_ECDH_SESSION_KEYS;

-        rc = yh_util_list_objects(session->slot->device_session, id, type,

+        rc = yh_util_list_objects(session->slot->device_session, 0, type,

                                   domains, &capabilities, algorithm, label,

                                   tmp_objects, &tmp_n_objects);

@@ -2636,12 +2631,7 @@ CK_DEFINE_FUNCTION(CK_RV, C_FindObjectsInit)

       }

     }

-    id = parse_id_value(template_id, template_id_len);

-    if (id == -1) {

-      DBG_ERR("Failed to parse ID from template");

-      rv = CKR_ATTRIBUTE_VALUE_INVALID;

-      goto c_foi_out;

-    }

+    uint16_t id = parse_id_value(template_id, template_id_len);

     DBG_INFO("id parsed as %x", id);

     if (ulCount == 0 ||

@@ -4948,12 +4938,14 @@ CK_DEFINE_FUNCTION(CK_RV, C_GenerateKey)

       case CKA_ID:

         if (id.set == false) {

-          rv = parse_meta_id_template(&meta_object, FALSE, (int *) &id.d,

+          uint16_t d;

+          rv = parse_meta_id_template(&meta_object, FALSE, &d,

                                       pTemplate[i].pValue,

                                       pTemplate[i].ulValueLen);

           if (rv != CKR_OK) {

             goto c_gk_out;

           }

+          id.d = d;

           id.set = true;

         } else {

           rv = CKR_TEMPLATE_INCONSISTENT;

diff --git a/pkcs11/util_pkcs11.c b/pkcs11/util_pkcs11.c

index db5f83c..caa467f 100644

--- a/pkcs11/util_pkcs11.c

+++ b/pkcs11/util_pkcs11.c

@@ -2720,7 +2720,7 @@ static CK_RV perform_aes_update(yh_session *session,

     return rv;

   }

-  DBG_INFO("Returning %lu bytes (buffered %lu bytes)", size, next);

+  DBG_INFO("Returning %zu bytes (buffered %zu bytes)", size, next);

   *out_len = size;

   return CKR_OK;