Merge pull request wolfSSL#83 from bigbrett/curve25519-fixes

Curve25519 fixes

.github/workflows/build-and-test.yml

@@ -34,12 +34,10 @@ jobs:
       run: cd test && make clean && make WOLFSSL_DIR=../wolfssl run
 
     # Build and test ASAN build, with wolfCrypt tests enabled.
-    # since wolfCrypt tests aren't all passing yet - this is just a status indicator
-    - name: Build and test ASAN TESTWOLFCRYPT (wolfCrypt tests OK to fail)
+    - name: Build and test ASAN TESTWOLFCRYPT
       run: cd test && make clean && make ASAN=1 TESTWOLFCRYPT=1 WOLFSSL_DIR=../wolfssl run
 
     # Build and test ASAN build, with wolfCrypt tests enabled and using the DMA devId.
-    # since wolfCrypt tests aren't all passing yet - this is just a status indicator
     - name: Build and test ASAN TESTWOLFCRYPT TESTWOLFCRYPT_DMA
       run: cd test && make clean && make ASAN=1 TESTWOLFCRYPT=1 TESTWOLFCRYPT_DMA=1 WOLFSSL_DIR=../wolfssl run
 
@@ -54,8 +52,3 @@ jobs:
     # Build and test debug build with SHE
     - name: Build and test SHE
       run: cd test && make clean && make SHE=1 WOLFSSL_DIR=../wolfssl run
-
-    ## Test structure padding
-    #- name: Check structure padding
-    #  run: cd test && make clean && make checkpadding
-

src/wh_client_crypto.c

@@ -1185,8 +1185,8 @@ int wh_Client_Curve25519ImportKey(whClientContext* ctx, curve25519_key* key,
         key_id = *inout_keyId;
     }
 
-    ret = wh_Crypto_Curve25519SerializeKey(key, sizeof(buffer),buffer,
-            &buffer_len);
+    buffer_len = sizeof(buffer);
+    ret        = wh_Crypto_Curve25519SerializeKey(key, buffer, &buffer_len);
     if (ret == 0) {
         /* Cache the key and get the keyID */
         ret = wh_Client_KeyCache(ctx,
@@ -1195,6 +1195,10 @@ int wh_Client_Curve25519ImportKey(whClientContext* ctx, curve25519_key* key,
         if (inout_keyId != NULL) {
             *inout_keyId = key_id;
         }
+#if defined(DEBUG_CRYPTOCB) && defined(DEBUG_CRYPTOCB_VERBOSE)
+        printf("[client] importKey: cached keyid=%u\n", key_id);
+        wh_Utils_Hexdump("[client] importKey: key=", buffer, buffer_len);
+#endif
     }
     return ret;
 }
@@ -1220,8 +1224,7 @@ int wh_Client_Curve25519ExportKey(whClientContext* ctx, whKeyId keyId,
             buffer, &buffer_len);
     if (ret == 0) {
         /* Update the key structure */
-        ret = wh_Crypto_Curve25519DeserializeKey(
-                buffer_len, buffer, key);
+        ret = wh_Crypto_Curve25519DeserializeKey(buffer, buffer_len, key);
     }
 
     return ret;
@@ -1311,8 +1314,7 @@ static int _Curve25519MakeKey(whClientContext* ctx,
 
                 if (flags & WH_NVM_FLAGS_EPHEMERAL) {
                     /* Response has the exported key */
-                    ret = wh_Crypto_Curve25519DeserializeKey(
-                            der_size, key_der, key);
+                    ret = wh_Crypto_Curve25519DeserializeKey(key_der, der_size, key);
 #ifdef DEBUG_CRYPTOCB_VERBOSE
                     wh_Utils_Hexdump("[client] KeyGen export:", key_der, der_size);
 #endif

src/wh_crypto.c

@@ -207,49 +207,42 @@ int wh_Crypto_EccUpdatePrivateOnlyKeyDer(ecc_key* key, uint16_t pub_size,
 #endif /* HAVE_ECC */
 
 #ifdef HAVE_CURVE25519
- int wh_Crypto_Curve25519SerializeKey(curve25519_key* key,
-        uint16_t max_size, uint8_t* buffer, uint16_t *out_size)
+
+/* Store a curve25519_key to a byte sequence in DER format */
+int wh_Crypto_Curve25519SerializeKey(curve25519_key* key, uint8_t* buffer,
+                                     uint16_t* derSize)
 {
     int ret = 0;
-    word32 privSz = CURVE25519_KEYSIZE;
-    word32 pubSz = CURVE25519_KEYSIZE;
+    /* We must include the algorithm identifier in the DER encoding, or we will
+     * not be able to deserialize it properly in the public key only case*/
+    const int WITH_ALG_ENABLE_SUBJECT_PUBLIC_KEY_INFO = 1;
 
-    if (    (key == NULL) ||
-            (buffer == NULL)) {
+    if ((key == NULL) || (buffer == NULL) || (derSize == NULL)) {
         return WH_ERROR_BADARGS;
     }
 
-    ret = wc_curve25519_export_key_raw(key,
-            buffer + CURVE25519_KEYSIZE, &privSz,
-            buffer, &pubSz);
-    if (    (ret == 0) &&
-            (out_size != NULL)) {
-        *out_size = CURVE25519_KEYSIZE * 2;
+    ret = wc_Curve25519KeyToDer(key, buffer, *derSize,
+                                WITH_ALG_ENABLE_SUBJECT_PUBLIC_KEY_INFO);
+
+    /* ASN.1 functions return the size of the DER encoded key on success */
+    if (ret > 0) {
+        *derSize = ret;
+        ret      = WH_ERROR_OK;
     }
     return ret;
 }
 
-int wh_Crypto_Curve25519DeserializeKey(uint16_t size,
-        const uint8_t* buffer, curve25519_key* key)
+/* Restore a curve25519_key from a byte sequence in DER format */
+int wh_Crypto_Curve25519DeserializeKey(const uint8_t* derBuffer,
+                                       uint16_t derSize, curve25519_key* key)
 {
-    int ret = 0;
-    word32 privSz = CURVE25519_KEYSIZE;
-    word32 pubSz = CURVE25519_KEYSIZE;
+    word32 idx = 0;
 
-    if (    (size < (CURVE25519_KEYSIZE * 2)) ||
-            (buffer == NULL) ||
-            (key == NULL)) {
+    if ((derBuffer == NULL) || (key == NULL)) {
         return WH_ERROR_BADARGS;
     }
 
-    /* decode the key */
-    if (ret == 0) {
-        ret = wc_curve25519_import_private_raw(
-                buffer + CURVE25519_KEYSIZE, privSz,
-                buffer, pubSz,
-                key);
-    }
-    return ret;
+    return wc_Curve25519KeyDecode(derBuffer, &idx, key, derSize);
 }
 #endif /* HAVE_CURVE25519 */
 

src/wh_server_crypto.c

@@ -430,38 +430,42 @@ int wh_Server_EccKeyCacheExport(whServerContext* ctx, whKeyId keyId,
 
 #ifdef HAVE_CURVE25519
 int wh_Server_CacheImportCurve25519Key(whServerContext* server,
-        curve25519_key* key,
-        whKeyId keyId, whNvmFlags flags, uint16_t label_len, uint8_t* label)
+                                       curve25519_key* key, whKeyId keyId,
+                                       whNvmFlags flags, uint16_t label_len,
+                                       uint8_t* label)
 {
-    uint8_t* cacheBuf;
+    uint8_t*       cacheBuf;
     whNvmMetadata* cacheMeta;
-    int ret;
-    const uint16_t keySz = CURVE25519_KEYSIZE * 2;
-    uint16_t size = 0;
+    int            ret;
+    /* Max size of a DER encoded curve25519 keypair with SubjectPublicKeyInfo
+     * included. Determined by experiment */
+    const uint16_t MAX_DER_SIZE = 128;
+    uint16_t       keySz        = keySz;
 
-    if (    (server == NULL) ||
-            (key == NULL) ||
-            (WH_KEYID_ISERASED(keyId)) ||
-            ((label != NULL) && (label_len > sizeof(cacheMeta->label)))) {
+    uint8_t der_buf[MAX_DER_SIZE];
+
+
+    if ((server == NULL) || (key == NULL) || (WH_KEYID_ISERASED(keyId)) ||
+        ((label != NULL) && (label_len > sizeof(cacheMeta->label)))) {
         return WH_ERROR_BADARGS;
     }
 
-    /* get a free slot */
-    ret = hsmCacheFindSlotAndZero(server, keySz, &cacheBuf, &cacheMeta);
-    if (ret == 0) {
-        ret = wh_Crypto_Curve25519SerializeKey(key, keySz, cacheBuf, &size);
-    }
+    /* Serialize the key into the temporary buffer so we can get the size */
+    ret = wh_Crypto_Curve25519SerializeKey(key, der_buf, &keySz);
 
+    /* if successful, find a free cache slot and copy in the key data */
     if (ret == 0) {
-        /* set meta */
-        cacheMeta->id = keyId;
-        cacheMeta->len = size;
-        cacheMeta->flags = flags;
-        cacheMeta->access = WH_NVM_ACCESS_ANY;
-
-        if (    (label != NULL) &&
-                (label_len > 0) ) {
-            memcpy(cacheMeta->label, label, label_len);
+        ret = hsmCacheFindSlotAndZero(server, keySz, &cacheBuf, &cacheMeta);
+        if (ret == 0) {
+            memcpy(cacheBuf, der_buf, keySz);
+            /* Update metadata to cache the key */
+            cacheMeta->id     = keyId;
+            cacheMeta->len    = keySz;
+            cacheMeta->flags  = flags;
+            cacheMeta->access = WH_NVM_ACCESS_ANY;
+            if ((label != NULL) && (label_len > 0)) {
+                memcpy(cacheMeta->label, label, label_len);
+            }
         }
     }
     return ret;
@@ -483,7 +487,7 @@ int wh_Server_CacheExportCurve25519Key(whServerContext* server, whKeyId keyId,
     ret = hsmFreshenKey(server, keyId, &cacheBuf, &cacheMeta);
 
     if (ret == 0) {
-        ret = wh_Crypto_Curve25519DeserializeKey(cacheMeta->len, cacheBuf, key);
+        ret = wh_Crypto_Curve25519DeserializeKey(cacheBuf, cacheMeta->len, key);
 #ifdef DEBUG_CRYPTOCB_VERBOSE
         printf("[server] Export25519Key id:%u ret:%d\n", keyId, ret);
         wh_Utils_Hexdump("[server] export key:", cacheBuf, cacheMeta->len);
@@ -795,9 +799,9 @@ static int _HandleCurve25519KeyGen(whServerContext* server, whPacket* packet,
 
     /* Response Message */
     uint8_t* out        = (uint8_t*)(res + 1);
-    uint16_t max_size   = (word32)(WOLFHSM_CFG_COMM_DATA_LEN -
+    /* Initialize the key size to the max size of the buffer */
+    uint16_t ser_size   = (word32)(WOLFHSM_CFG_COMM_DATA_LEN -
                             (out - (uint8_t*)packet));
-    uint16_t res_size   = 0;
 
     /* init key */
     ret = wc_curve25519_init_ex(key, NULL, server->crypto->devId);
@@ -809,11 +813,10 @@ static int _HandleCurve25519KeyGen(whServerContext* server, whPacket* packet,
             if (flags & WH_NVM_FLAGS_EPHEMERAL) {
                 /* Must serialize the key into the response packet */
                 key_id = WH_KEYID_ERASED;
-                ret = wh_Crypto_Curve25519SerializeKey(key, max_size,
-                        out, &res_size);
+                ret = wh_Crypto_Curve25519SerializeKey(key, out, &ser_size);
             } else {
+                ser_size = 0;
                 /* Must import the key into the cache and return keyid */
-                res_size = 0;
                 if (WH_KEYID_ISERASED(key_id)) {
                     /* Generate a new id */
                     ret = hsmGetUniqueId(server, &key_id);
@@ -836,8 +839,8 @@ static int _HandleCurve25519KeyGen(whServerContext* server, whPacket* packet,
 
     if (ret == 0) {
         res->keyId  = WH_KEYID_ID(key_id);
-        res->len    = res_size;
-        *out_size   = WH_PACKET_STUB_SIZE + sizeof(*res) + res_size;
+        res->len    = ser_size;
+        *out_size   = WH_PACKET_STUB_SIZE + sizeof(*res) + ser_size;
     }
     return ret;
 }
@@ -1585,7 +1588,7 @@ static int _HandleSha256Dma(whServerContext* server, whPacket* packet,
         if (ret == WH_ERROR_OK) {
 #ifdef DEBUG_CRYPTOCB_VERBOSE
             printf("[server]   wc_Sha256Update: inAddr=%p, sz=%llu\n", inAddr,
-                   req->input.sz);
+                   (long long unsigned int)req->input.sz);
 #endif
             ret = wc_Sha256Update(sha256, inAddr, req->input.sz);
         }

src/wh_server_keystore.c

@@ -220,6 +220,10 @@ int hsmCacheKey(whServerContext* server, whNvmMetadata* meta, uint8_t* in)
             } else {
                 server->cache[foundIndex].commited = 1;
             }
+#if defined(DEBUG_CRYPTOCB) && defined(DEBUG_CRYPTOCB_VERBOSE)
+            printf("[server] cacheKey: caching keyid=%u\n", meta->id);
+            wh_Utils_Hexdump("[server] cacheKey: key=", in, meta->len);
+#endif
         }
     } else {
         /* try big key cache, don't put small keys into big cache if full */

wolfhsm/wh_crypto.h

@@ -77,11 +77,11 @@ int wh_Crypto_EccUpdatePrivateOnlyKeyDer(ecc_key* key, uint16_t pub_size,
 
 #ifdef HAVE_CURVE25519
 /* Store a curve25519_key to a byte sequence */
-int wh_Crypto_Curve25519SerializeKey(curve25519_key* key,
-        uint16_t max_size, uint8_t* buffer, uint16_t *out_size);
+int wh_Crypto_Curve25519SerializeKey(curve25519_key* key, uint8_t* buffer,
+                                     uint16_t* out_size);
 /* Restore a curve25519_key from a byte sequence */
-int wh_Crypto_Curve25519DeserializeKey(uint16_t size,
-        const uint8_t* buffer, curve25519_key* key);
+int wh_Crypto_Curve25519DeserializeKey(const uint8_t* derBuffer,
+                                       uint16_t derSize, curve25519_key* key);
 #endif /* HAVE_CURVE25519 */
 
 #endif  /* !WOLFHSM_CFG_NO_CRYPTO */