Blob Blame History Raw
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;