Fixed pkcs1.5 padding

lib/ykpiv.c

@@ -534,39 +534,55 @@ static ykpiv_rc _general_authenticate(ykpiv_state *state,
   ykpiv_rc res;
 
   switch(algorithm) {
-    case YKPIV_ALGO_RSA1024:
+  case YKPIV_ALGO_RSA1024:
-      pad_len = 128;
+    pad_len = 128;
-    case YKPIV_ALGO_RSA2048:
+
-      if(pad_len == 0) {
+  case YKPIV_ALGO_RSA2048:
-        pad_len = 256;
+    if(pad_len == 0) {
+      pad_len = 256;
+    }
+    if(!decipher) {
+      // Signature
+      if (padding) {
+        // Padding required
+        if(in_len + RSA_PKCS1_PADDING_SIZE > pad_len) {
+          return YKPIV_SIZE_ERROR;
+        }
+
+        // Add padding and copy data
+        RSA_padding_add_PKCS1_type_1(sign_in, pad_len, raw_in, in_len);
+        in_len = pad_len;
       }
-      if(!decipher) {
+      else {
-  if(in_len + RSA_PKCS1_PADDING_SIZE > pad_len) {
+        // No padding required
-    return YKPIV_SIZE_ERROR;
+        if (in_len != pad_len)
-  }
+          return YKPIV_SIZE_ERROR;
-  if (padding) {
+
-    RSA_padding_add_PKCS1_type_1(sign_in, pad_len, raw_in, in_len);
+        // Just copy data
-    in_len = pad_len;
+        memcpy(sign_in, raw_in, in_len);
-  }
-  else if (in_len != pad_len)
-    return YKPIV_SIZE_ERROR;
-      } else {
-  if(in_len != pad_len) {
-    return YKPIV_SIZE_ERROR;
-  }
-  memcpy(sign_in, raw_in, in_len);
       }
-      break;
+    }
-    case YKPIV_ALGO_ECCP256:
+    else {
-      if(!decipher && in_len > 32) {
+      // Decryption
-  return YKPIV_SIZE_ERROR;
+      if(in_len != pad_len) {
-      } else if(decipher && in_len != 65) {
+        return YKPIV_SIZE_ERROR;
-  return YKPIV_SIZE_ERROR;
       }
+
+      // Just copy data
       memcpy(sign_in, raw_in, in_len);
-      break;
+    }
-    default:
+    break;
-      return YKPIV_ALGORITHM_ERROR;
+
+  case YKPIV_ALGO_ECCP256:
+    if(!decipher && in_len > 32) {
+      return YKPIV_SIZE_ERROR;
+    } else if(decipher && in_len != 65) {
+      return YKPIV_SIZE_ERROR;
+    }
+    memcpy(sign_in, raw_in, in_len);
+    break;
+  default:
+    return YKPIV_ALGORITHM_ERROR;
   }
 
   if(in_len < 0x80) {

ykcs11/debug.h

@@ -1,8 +1,8 @@
 #ifndef DEBUG_H
 #define DEBUG_H
 
-#define YKCS11_DBG    0  // General debug, must be either 1 or 0
+#define YKCS11_DBG    1  // General debug, must be either 1 or 0
-#define YKCS11_DINOUT 0  // Function in/out debug, must be either 1 or 0
+#define YKCS11_DINOUT 1  // Function in/out debug, must be either 1 or 0
 
 #define D(x) do {                                                     \
     printf ("debug: %s:%d (%s): ", __FILE__, __LINE__, __FUNCTION__); \

ykcs11/mechanisms.c

@@ -1,4 +1,5 @@
 #include "mechanisms.h"
+#include "debug.h"
 
 #define F4 "\x01\x00\x01"
 #define PRIME256V1 "\x06\x08\x2a\x86\x48\xce\x3d\x03\x01\x07"
@@ -267,8 +268,6 @@ CK_RV apply_sign_mechanism_finalize(op_info_t *op_info) {
     rv = do_md_finalize(op_info->op.sign.md_ctx, op_info->buf, &op_info->buf_len, &nid);
     if (rv != CKR_OK)
       return CKR_FUNCTION_FAILED;
-//    DBG(("The hashed value is %lu long and looks like\n", op_info->buf_len));
-    dump_hex(op_info->buf, op_info->buf_len, stderr, CK_TRUE);
 
   case CKM_RSA_PKCS:
     // Add digest info if needed
@@ -276,9 +275,6 @@ CK_RV apply_sign_mechanism_finalize(op_info_t *op_info) {
       rv = do_pkcs_1_digest_info(op_info->buf, op_info->buf_len, nid, op_info->buf, &op_info->buf_len);
       if (rv != CKR_OK)
         return CKR_FUNCTION_FAILED;
-
-//      DBG(("After adding digestinfo is %lu long and looks like\n", op_info->buf_len));
-      dump_hex(op_info->buf, op_info->buf_len, stderr, CK_TRUE);
     }
 
     // Compute padding for all PKCS1 variants
@@ -356,8 +352,10 @@ CK_RV check_pubkey_template(op_info_t *op_info, CK_ATTRIBUTE_PTR templ, CK_ULONG
         return CKR_ATTRIBUTE_VALUE_INVALID;
 
       // Only support F4
-      if (templ[i].ulValueLen != 3 || memcmp((CK_BYTE_PTR)templ[i].pValue, F4, 3) != 0)
+      if (templ[i].ulValueLen != 3 || memcmp((CK_BYTE_PTR)templ[i].pValue, F4, 3) != 0) {
+        DBG(("Unsupported public exponent"));
         return CKR_ATTRIBUTE_VALUE_INVALID;
+      }
 
       break;
 
@@ -366,8 +364,10 @@ CK_RV check_pubkey_template(op_info_t *op_info, CK_ATTRIBUTE_PTR templ, CK_ULONG
         return CKR_ATTRIBUTE_VALUE_INVALID;
 
       if (*((CK_ULONG_PTR)templ[i].pValue) != 1024 &&
-          *((CK_ULONG_PTR) templ[i].pValue) != 2048) // TODO: make define?
+          *((CK_ULONG_PTR) templ[i].pValue) != 2048) { // TODO: make define?
+        DBG(("Unsupported MODULUS_BITS (key length)"));
         return CKR_ATTRIBUTE_VALUE_INVALID;
+      }
 
       op_info->op.gen.key_len = *((CK_ULONG_PTR) templ[i].pValue);
       break;
@@ -395,6 +395,7 @@ CK_RV check_pubkey_template(op_info_t *op_info, CK_ATTRIBUTE_PTR templ, CK_ULONG
       break;
       
     default:
+      DBG(("Invalid attribute %lx in public key template", templ[i].type));
       return CKR_ATTRIBUTE_VALUE_INVALID;
     }
   }
@@ -459,6 +460,7 @@ CK_RV check_pvtkey_template(op_info_t *op_info, CK_ATTRIBUTE_PTR templ, CK_ULONG
       break;
 
     default:
+      DBG(("Invalid attribute %lx in private key template", templ[i].type));
       return CKR_ATTRIBUTE_VALUE_INVALID;
     }
   }

ykcs11/openssl_utils.c

@@ -2,6 +2,7 @@
 #include <stdbool.h>
 #include "../tool/util.h" // TODO: share this better?
 #include "debug.h"
+#include <string.h>
 
 CK_RV do_store_cert(CK_BYTE_PTR data, CK_ULONG len, X509 **cert) {
 
@@ -367,6 +368,8 @@ CK_RV free_key(EVP_PKEY *key) {
 }
 
 CK_RV do_pkcs_1_t1(CK_BYTE_PTR in, CK_ULONG in_len, CK_BYTE_PTR out, CK_ULONG_PTR out_len, CK_ULONG key_len) {
+  unsigned char buffer[512];
+  
   key_len /= 8;
   DBG(("Apply padding to %lu bytes and get %lu\n", in_len, key_len));
 
@@ -374,9 +377,10 @@ CK_RV do_pkcs_1_t1(CK_BYTE_PTR in, CK_ULONG in_len, CK_BYTE_PTR out, CK_ULONG_PT
   if (*out_len < key_len)
     CKR_BUFFER_TOO_SMALL;
 
-  if (RSA_padding_add_PKCS1_type_1(out, key_len, in, in_len) == 0)
+  if (RSA_padding_add_PKCS1_type_1(buffer, key_len, in, in_len) == 0)
     return CKR_FUNCTION_FAILED;
 
+  memcpy(out, buffer, key_len);
   *out_len = key_len;
 
   return CKR_OK;

ykcs11/ykcs11.c

@@ -559,10 +559,10 @@ CK_DEFINE_FUNCTION(CK_RV, C_CloseSession)(
     DBG(("Unknown session %lu", hSession));
     return CKR_SESSION_HANDLE_INVALID;
   }
-  DBG(("HI"));
+
   free(session.slot->token->objects); // TODO: make objects survive a session so there is no need to get them again?
   session.slot->token->objects = NULL;
-  DBG(("HI2"));
+
   memset(&session, 0, sizeof(ykcs11_session_t));
   session.handle = CK_INVALID_HANDLE;
 
@@ -1351,24 +1351,37 @@ CK_DEFINE_FUNCTION(CK_RV, C_Sign)(
   // TODO: check other conditions
   ykpiv_rc r; // TODO: delete this
 
+  if (pSignature == NULL_PTR) {
+    // Just return the size of the signature
+    *pulSignatureLen = op_info.op.sign.key_len / 8;
+    DBG(("The size of the signature will be %lu", *pulSignatureLen));
+
+    DOUT;
+    return CKR_OK;
+  }
+    
+
   DBG(("Sending %lu bytes to sign", ulDataLen));
   dump_hex(pData, ulDataLen, stderr, CK_TRUE);
 
   if (is_hashed_mechanism(op_info.mechanism.mechanism) == CK_TRUE) {
-      if (apply_sign_mechanism_update(&op_info, pData, ulDataLen) != CKR_OK) {
+    if (apply_sign_mechanism_update(&op_info, pData, ulDataLen) != CKR_OK) {
-        DBG(("Unable to perform signing operation step"));
+      DBG(("Unable to perform signing operation step"));
-        return CKR_FUNCTION_FAILED;
+      return CKR_FUNCTION_FAILED; // TODO: every error in here must stop and clear the signing operation
-      }
     }
+  }
 
   if (apply_sign_mechanism_finalize(&op_info) != CKR_OK) {
     DBG(("Unable to finalize signing operation"));
     return CKR_FUNCTION_FAILED;
   }
-  //dump_hex(buf, 256, stderr, CK_TRUE);
+
-  //*pulSignatureLen = 256;
   DBG(("Using key %lx", op_info.op.sign.key_id)); // TODO: test what happens if there is no key on the card
-  if ((r = ykpiv_sign_data(piv_state, op_info.buf, ulDataLen, pSignature, pulSignatureLen, op_info.op.sign.algo, op_info.op.sign.key_id)) != YKPIV_OK) {
+  DBG(("After padding and transformation there are %lu bytes", op_info.buf_len));
+  dump_hex(op_info.buf, op_info.buf_len, stderr, CK_TRUE);
+
+  *pulSignatureLen = sizeof(op_info.buf);
+  if ((r = ykpiv_sign_data2(piv_state, op_info.buf, op_info.buf_len, pSignature, pulSignatureLen, op_info.op.sign.algo, op_info.op.sign.key_id, 0)) != YKPIV_OK) {
       DBG(("Sign error, %s", ykpiv_strerror(r)));
     return CKR_FUNCTION_FAILED;
   }
@@ -1665,7 +1678,7 @@ CK_DEFINE_FUNCTION(CK_RV, C_GenerateKeyPair)(
   }
 
   if (op_info.op.gen.key_id == 0) {
-    DBG(("Key id not specified, using default value"));
+    DBG(("Key id not specified"));
     return CKR_TEMPLATE_INCOMPLETE;
   }