Support PKCS11 3.0 (Yubico#183)

* Use PKCS11 3.0 headers

* Added files to be installed

* Basic PKCS11 3.0 support

* Make function lists static

* Make function lists const

* Refactored C_Login and C_LoginUser

* Rebase fixes

* Rebase changes

* Support ed25519

* Fix attributes for ed25519 keys

* ed25519 keys are considered 255 bits. Refactor getting CKA_EC_POINT slightly

* pkc11 3.1

* fix buffer_length check for EdDSA in util_pkcs11.c (Yubico#390)

Having problems signing with EdDSA on YubiHSM2 via PKCS11.
Getting an 
pkcs11:p11prov_Sign:The size of plaintext input data to a cryptographic operation is invalid (Out of range):interface.gen.c:679:Error returned by C_Sign
error

As I understand the PKCS11 v3.0 spec, the 1024 bit limit (note by "adma" in line 2228) applies only to "ECDSA without hashing" (CKM_ECDSA) as it only processes a hash value.

see: https://docs.oasis-open.org/pkcs11/pkcs11-curr/v3.0/os/pkcs11-curr-v3.0-os.html#_Toc30061189

EdDSA does not have this limit, so the size of "op_info->buffer" should be the limiting factor

see: https://docs.oasis-open.org/pkcs11/pkcs11-curr/v3.0/os/pkcs11-curr-v3.0-os.html#_Toc30061191

* PKCS11: Rebase on current master

* PKCS11: Fix include for older OSs. Githubactions: Fix OpenSSL installation on MacOS runners

* PKCS11: Compile ED25519 code only when OpenSSL is higher than 1.0. Build: Fix Redhat and MacOS builds

* PKCS11: Fix header inclusion in test

* PKCS11: Update tests to use PKCS11_3 functionlist. Add test for PKCS11 interfaces

* PKCS11: Remove unnecessary KDF definitions

* PKCS11: Fix PKCS11 interfaces test

* PKCS11_3: return CKR_PIN_INCORRECT when logging in with wrong password

* Only C_GetInfo from 3.x function list returns 3.x cryptoki version

* PKCS11_3: Use snprintf instead of sprintf

* PKCS11_3: Fix allocation of arrays

* More usage of snprintf

---------

Co-authored-by: marcwillert <[email protected]>
Co-authored-by: Aveen Ismail <[email protected]>

.github/workflows/build_and_test_windows.yml

@@ -67,8 +67,6 @@ jobs:
           Set-PSDebug -Trace 1
           $YHSHELL_SRC_DIR="$env:GITHUB_WORKSPACE\yubihsm-shell-"
           $MERGEDPATH = Get-ChildItem "C:\Program Files*\Microsoft Visual Studio\*\Enterprise\VC\Redist\MSVC\v14*\MergeModules\Microsoft_VC*_CRT_$env:ARCH.msm"
-          
-          echo "MERGEDPATH = $MERGEDPATH"
 
           cd $YHSHELL_SRC_DIR/resources/release/win
           ./make_release_binaries.ps1 $env:ARCH_CMAKE C:/vcpkg

.github/workflows/release.yml

@@ -109,6 +109,12 @@ jobs:
           set -x
           tar xf yubihsm-shell-$VERSION.tar.gz
 
+      - name: Install dependecies
+        run: |
+          brew update
+          brew install cmake pkg-config gengetopt help2man
+          brew reinstall openssl@3
+
       - name: Build and make MSI installer
         env:
           ARCH: ${{ matrix.arch }}

cmake/SecurityFlags.cmake

@@ -7,7 +7,7 @@ if (CMAKE_C_COMPILER_ID STREQUAL "Clang" OR
     add_compile_options (-Wall -Wextra -Werror)
     add_compile_options (-Wformat -Wformat-nonliteral -Wformat-security)
     add_compile_options (-Wshadow)
-    add_compile_options (-Wcast-qual)
+    #add_compile_options (-Wcast-qual)
     add_compile_options (-Wmissing-prototypes)
     add_compile_options (-Wbad-function-cast)
     add_compile_options (-pedantic -pedantic-errors)

examples/p11_generate_rsa.c

@@ -23,7 +23,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include <pkcs11.h>
+#include <pkcs11y.h>
 
 int main(int argc, char *argv[]) {
   if (argc != 2) {

lib/internal.h

@@ -37,13 +37,13 @@ struct yh_connector {
   void *backend;
   struct backend_functions *bf;
   yh_backend *connection;
-  char *status_url;
-  char *api_url;
+  char status_url[256];
+  char api_url[256];
   bool has_device;
   uint8_t version_major;
   uint8_t version_minor;
   uint8_t version_patch;
-  uint8_t address[32];
+  char address[32];
   uint32_t port;
   uint32_t pid;
 };

lib/lib_util.c

@@ -115,7 +115,7 @@ void parse_status_data(char *data, yh_connector *connector) {
 
       connector->pid = pid;
     } else if (strncmp(str, ADDRESS_STR, strlen(ADDRESS_STR)) == 0) {
-      strncpy((char *) connector->address, str + strlen(ADDRESS_STR),
+      strncpy(connector->address, str + strlen(ADDRESS_STR),
               sizeof(connector->address) - 1);
     } else if (strncmp(str, PORT_STR, strlen(PORT_STR)) == 0) {
       char *endptr;

lib/tests/test_parsing.c

@@ -97,7 +97,7 @@ static void test_capabilities2(void) {
   size_t len = 0;
   for (size_t i = 0;
        i < sizeof(capabilities_list) / sizeof(capabilities_list[0]); i++) {
-    sprintf(capabilities_string + len, "%s:", capabilities_list[i]);
+    snprintf(capabilities_string + len, sizeof(capabilities_string) - len, "%s:", capabilities_list[i]);
     len += strlen(capabilities_list[i]) + 1;
   }
   capabilities_string[len - 1] = '\0';

lib/tests/test_util.c

@@ -35,21 +35,21 @@ static void test_status(void) {
     yh_connector c;
   } tests[] = {
     {"status=OK\nversion=1.2.3\n",
-     {NULL, NULL, NULL, NULL, NULL, true, 1, 2, 3, "", 0, 0}},
-    {"", {NULL, NULL, NULL, NULL, NULL, false, 0, 0, 0, "", 0, 0}},
-    {"foobar", {NULL, NULL, NULL, NULL, NULL, false, 0, 0, 0, "", 0, 0}},
-    {"\n\n\n\n\n\n", {NULL, NULL, NULL, NULL, NULL, false, 0, 0, 0, "", 0, 0}},
+     {NULL, NULL, NULL, {0}, {0}, true, 1, 2, 3, "", 0, 0}},
+    {"", {NULL, NULL, NULL, {0}, {0}, false, 0, 0, 0, "", 0, 0}},
+    {"foobar", {NULL, NULL, NULL, {0}, {0}, false, 0, 0, 0, "", 0, 0}},
+    {"\n\n\n\n\n\n", {NULL, NULL, NULL, {0}, {0}, false, 0, 0, 0, "", 0, 0}},
     {"status=NO_DEVICE\nserial=*\nversion=1.0.2\npid=412\naddress=\nport=12345",
-     {NULL, NULL, NULL, NULL, NULL, false, 1, 0, 2, "", 12345, 412}},
-    {"version=1.2", {NULL, NULL, NULL, NULL, NULL, false, 1, 2, 0, "", 0, 0}},
+     {NULL, NULL, NULL, {0}, {0}, false, 1, 0, 2, "", 12345, 412}},
+    {"version=1.2", {NULL, NULL, NULL, {0}, {0}, false, 1, 2, 0, "", 0, 0}},
     {"version=foobar",
-     {NULL, NULL, NULL, NULL, NULL, false, 0, 0, 0, "", 0, 0}},
+     {NULL, NULL, NULL, {0}, {0}, false, 0, 0, 0, "", 0, 0}},
     {"version=2..\nstatus=OK",
-     {NULL, NULL, NULL, NULL, NULL, true, 2, 0, 0, "", 0, 0}},
+     {NULL, NULL, NULL, {0}, {0}, true, 2, 0, 0, "", 0, 0}},
   };
 
   for (size_t i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
-    yh_connector c = {NULL, NULL, NULL, NULL, NULL, false, 0, 0, 0, "", 0, 0};
+    yh_connector c = {NULL, NULL, NULL, {0}, {0}, false, 0, 0, 0, "", 0, 0};
     char *data = strdup(tests[i].data);
 
     parse_status_data(data, &c);

lib/yubihsm.c

@@ -1346,7 +1346,7 @@ yh_rc yh_get_connector_address(yh_connector *connector, char **const address) {
     return YHR_INVALID_PARAMETERS;
   }
 
-  *address = (char *) connector->address;
+  *address = connector->address;
 
   return YHR_SUCCESS;
 }
@@ -4305,31 +4305,11 @@ static yh_rc create_connector(yh_connector **connector, const char *url,
   yh_rc rc = YHR_SUCCESS;
 
   if (strncmp(url, YH_USB_URL_SCHEME, strlen(YH_USB_URL_SCHEME)) == 0) {
-    (*connector)->status_url = strdup(url);
-    if ((*connector)->status_url == NULL) {
-      rc = YHR_MEMORY_ERROR;
-      goto cc_failure;
-    }
-    (*connector)->api_url = strdup(url);
-    if ((*connector)->api_url == NULL) {
-      rc = YHR_MEMORY_ERROR;
-      goto cc_failure;
-    }
+    snprintf((*connector)->status_url, sizeof((*connector)->status_url), "%s", url);
+    snprintf((*connector)->api_url, sizeof((*connector)->api_url), "%s", url);
   } else {
-    (*connector)->status_url =
-      calloc(1, strlen(url) + strlen(STATUS_ENDPOINT) + 1);
-    if ((*connector)->status_url == NULL) {
-      rc = YHR_MEMORY_ERROR;
-      goto cc_failure;
-    }
-    sprintf((*connector)->status_url, "%s%s", url, STATUS_ENDPOINT);
-
-    (*connector)->api_url = calloc(1, strlen(url) + strlen(API_ENDPOINT) + 1);
-    if ((*connector)->api_url == NULL) {
-      rc = YHR_MEMORY_ERROR;
-      goto cc_failure;
-    }
-    sprintf((*connector)->api_url, "%s%s", url, API_ENDPOINT);
+    snprintf((*connector)->status_url, sizeof((*connector)->status_url), "%s%s", url, STATUS_ENDPOINT);
+    snprintf((*connector)->api_url, sizeof((*connector)->api_url), "%s%s", url, API_ENDPOINT);
   }
 
   (*connector)->connection = bf->backend_create();
@@ -4344,15 +4324,6 @@ static yh_rc create_connector(yh_connector **connector, const char *url,
   return YHR_SUCCESS;
 
 cc_failure:
-  if ((*connector)->status_url) {
-    free((*connector)->status_url);
-    (*connector)->status_url = NULL;
-  }
-
-  if ((*connector)->api_url) {
-    free((*connector)->api_url);
-    (*connector)->api_url = NULL;
-  }
 
   if (*connector) {
     free(*connector);
@@ -4373,15 +4344,8 @@ static void destroy_connector(yh_connector *connector) {
     connector->connection = NULL;
   }
 
-  if (connector->status_url != NULL) {
-    free(connector->status_url);
-    connector->status_url = NULL;
-  }
-
-  if (connector->api_url != NULL) {
-    free(connector->api_url);
-    connector->api_url = NULL;
-  }
+  memset(connector->status_url, 0, sizeof(connector->status_url));
+  memset(connector->api_url, 0, sizeof(connector->api_url));
 
   if (connector->bf) {
     connector->bf->backend_cleanup();
@@ -4746,7 +4710,7 @@ yh_rc yh_get_key_bitlength(yh_algorithm algorithm, size_t *result) {
       break;
 
     case YH_ALGO_EC_ED25519:
-      *result = 256;
+      *result = 255;
       break;
 
     case YH_ALGO_HMAC_SHA1:

pkcs11/CMakeLists.txt

@@ -95,6 +95,8 @@ install(
   LIBRARY DESTINATION "${YUBIHSM_INSTALL_LIB_DIR}/pkcs11"
   RUNTIME DESTINATION "${YUBIHSM_INSTALL_BIN_DIR}/pkcs11")
 install(FILES pkcs11.h DESTINATION "${YUBIHSM_INSTALL_INC_DIR}/pkcs11")
+install(FILES pkcs11t.h DESTINATION "${YUBIHSM_INSTALL_INC_DIR}/pkcs11")
+install(FILES pkcs11f.h DESTINATION "${YUBIHSM_INSTALL_INC_DIR}/pkcs11")
 install(FILES pkcs11y.h DESTINATION "${YUBIHSM_INSTALL_INC_DIR}/pkcs11")
 
 add_subdirectory (tests)