Merge pull request #7514 from valeriosetti/issue7513

PK: move the opaque context to a new member of the mbedtls_pk_context structure

include/mbedtls/pk.h

@@ -40,7 +40,7 @@
 #include "mbedtls/ecdsa.h"
 #endif
 
-#if defined(MBEDTLS_USE_PSA_CRYPTO)
+#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_PSA_CRYPTO_C)
 #include "psa/crypto.h"
 #endif
 
@@ -234,10 +234,17 @@ typedef struct mbedtls_pk_info_t mbedtls_pk_info_t;
 
 /**
  * \brief           Public key container
+ *
+ * \note            The priv_id is guarded by MBEDTLS_PSA_CRYPTO_C and not
+ *                  by MBEDTLS_USE_PSA_CRYPTO because it can be used also
+ *                  in mbedtls_pk_sign_ext for RSA keys.
  */
 typedef struct mbedtls_pk_context {
     const mbedtls_pk_info_t *MBEDTLS_PRIVATE(pk_info);    /**< Public key information         */
     void *MBEDTLS_PRIVATE(pk_ctx);                        /**< Underlying public key context  */
+#if defined(MBEDTLS_PSA_CRYPTO_C)
+    mbedtls_svc_key_id_t MBEDTLS_PRIVATE(priv_id);      /**< Key ID for opaque keys */
+#endif /* MBEDTLS_PSA_CRYPTO_C */
 } mbedtls_pk_context;
 
 #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE)

library/pk.c

@@ -60,6 +60,9 @@ void mbedtls_pk_init(mbedtls_pk_context *ctx)
 {
     ctx->pk_info = NULL;
     ctx->pk_ctx = NULL;
+#if defined(MBEDTLS_PSA_CRYPTO_C)
+    ctx->priv_id = MBEDTLS_SVC_KEY_ID_INIT;
+#endif /* MBEDTLS_PSA_CRYPTO_C */
 }
 
 /*
@@ -71,7 +74,7 @@ void mbedtls_pk_free(mbedtls_pk_context *ctx)
         return;
     }
 
-    if (ctx->pk_info != NULL) {
+    if ((ctx->pk_info != NULL) && (ctx->pk_info->ctx_free_func != NULL)) {
         ctx->pk_info->ctx_free_func(ctx->pk_ctx);
     }
 
@@ -140,7 +143,8 @@ int mbedtls_pk_setup(mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info)
         return MBEDTLS_ERR_PK_BAD_INPUT_DATA;
     }
 
-    if ((ctx->pk_ctx = info->ctx_alloc_func()) == NULL) {
+    if ((info->ctx_alloc_func == NULL) ||
+        ((ctx->pk_ctx = info->ctx_alloc_func()) == NULL)) {
         return MBEDTLS_ERR_PK_ALLOC_FAILED;
     }
 
@@ -158,7 +162,6 @@ int mbedtls_pk_setup_opaque(mbedtls_pk_context *ctx,
 {
     const mbedtls_pk_info_t *info = NULL;
     psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
-    mbedtls_svc_key_id_t *pk_ctx;
     psa_key_type_t type;
 
     if (ctx == NULL || ctx->pk_info != NULL) {
@@ -179,14 +182,8 @@ int mbedtls_pk_setup_opaque(mbedtls_pk_context *ctx,
         return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE;
     }
 
-    if ((ctx->pk_ctx = info->ctx_alloc_func()) == NULL) {
-        return MBEDTLS_ERR_PK_ALLOC_FAILED;
-    }
-
     ctx->pk_info = info;
-
-    pk_ctx = (mbedtls_svc_key_id_t *) ctx->pk_ctx;
-    *pk_ctx = key;
+    ctx->priv_id = key;
 
     return 0;
 }
@@ -315,12 +312,11 @@ int mbedtls_pk_can_do_ext(const mbedtls_pk_context *ctx, psa_algorithm_t alg,
         return (key_usage & usage) == usage;
     }
 
-    const mbedtls_svc_key_id_t *key = (const mbedtls_svc_key_id_t *) ctx->pk_ctx;
     psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
     psa_algorithm_t key_alg, key_alg2;
     psa_status_t status;
 
-    status = psa_get_key_attributes(*key, &attributes);
+    status = psa_get_key_attributes(ctx->priv_id, &attributes);
     if (status != PSA_SUCCESS) {
         return 0;
     }
@@ -701,10 +697,9 @@ int mbedtls_pk_sign_ext(mbedtls_pk_type_t pk_type,
     }
 
     if (mbedtls_pk_get_type(ctx) == MBEDTLS_PK_OPAQUE) {
-        const mbedtls_svc_key_id_t *key = (const mbedtls_svc_key_id_t *) ctx->pk_ctx;
         psa_status_t status;
 
-        status = psa_sign_hash(*key, PSA_ALG_RSA_PSS(psa_md_alg),
+        status = psa_sign_hash(ctx->priv_id, PSA_ALG_RSA_PSS(psa_md_alg),
                                hash, hash_len,
                                sig, sig_size, sig_len);
         return PSA_PK_RSA_TO_MBEDTLS_ERR(status);

library/pk_wrap.c

@@ -1503,29 +1503,12 @@ const mbedtls_pk_info_t mbedtls_rsa_alt_info = {
 #endif /* MBEDTLS_PK_RSA_ALT_SUPPORT */
 
 #if defined(MBEDTLS_USE_PSA_CRYPTO)
-
-static void *pk_opaque_alloc_wrap(void)
-{
-    void *ctx = mbedtls_calloc(1, sizeof(mbedtls_svc_key_id_t));
-
-    /* no _init() function to call, as calloc() already zeroized */
-
-    return ctx;
-}
-
-static void pk_opaque_free_wrap(void *ctx)
-{
-    mbedtls_platform_zeroize(ctx, sizeof(mbedtls_svc_key_id_t));
-    mbedtls_free(ctx);
-}
-
 static size_t pk_opaque_get_bitlen(mbedtls_pk_context *pk)
 {
-    const mbedtls_svc_key_id_t *key = pk->pk_ctx;
     size_t bits;
     psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
 
-    if (PSA_SUCCESS != psa_get_key_attributes(*key, &attributes)) {
+    if (PSA_SUCCESS != psa_get_key_attributes(pk->priv_id, &attributes)) {
         return 0;
     }
 
@@ -1563,7 +1546,6 @@ static int pk_opaque_sign_wrap(mbedtls_pk_context *pk, mbedtls_md_type_t md_alg,
     ((void) p_rng);
     return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE;
 #else /* !MBEDTLS_PK_CAN_ECDSA_SIGN && !MBEDTLS_RSA_C */
-    const mbedtls_svc_key_id_t *key = pk->pk_ctx;
     psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
     psa_algorithm_t alg;
     psa_key_type_t type;
@@ -1573,7 +1555,7 @@ static int pk_opaque_sign_wrap(mbedtls_pk_context *pk, mbedtls_md_type_t md_alg,
     (void) f_rng;
     (void) p_rng;
 
-    status = psa_get_key_attributes(*key, &attributes);
+    status = psa_get_key_attributes(pk->priv_id, &attributes);
     if (status != PSA_SUCCESS) {
         return PSA_PK_TO_MBEDTLS_ERR(status);
     }
@@ -1594,7 +1576,7 @@ static int pk_opaque_sign_wrap(mbedtls_pk_context *pk, mbedtls_md_type_t md_alg,
     return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE;
 
     /* make the signature */
-    status = psa_sign_hash(*key, alg, hash, hash_len,
+    status = psa_sign_hash(pk->priv_id, alg, hash, hash_len,
                            sig, sig_size, sig_len);
     if (status != PSA_SUCCESS) {
 #if defined(MBEDTLS_PK_CAN_ECDSA_SIGN)
@@ -1635,8 +1617,8 @@ const mbedtls_pk_info_t mbedtls_pk_ecdsa_opaque_info = {
     NULL, /* decrypt - not relevant */
     NULL, /* encrypt - not relevant */
     NULL, /* check_pair - could be done later or left NULL */
-    pk_opaque_alloc_wrap,
-    pk_opaque_free_wrap,
+    NULL, /* alloc - no need to allocate new data dynamically */
+    NULL, /* free - as for the alloc, there is no data to free */
 #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE)
     NULL, /* restart alloc - not relevant */
     NULL, /* restart free - not relevant */
@@ -1650,14 +1632,13 @@ static int pk_opaque_rsa_decrypt(mbedtls_pk_context *pk,
                                  unsigned char *output, size_t *olen, size_t osize,
                                  int (*f_rng)(void *, unsigned char *, size_t), void *p_rng)
 {
-    const mbedtls_svc_key_id_t *key = pk->pk_ctx;
     psa_status_t status;
 
     /* PSA has its own RNG */
     (void) f_rng;
     (void) p_rng;
 
-    status = psa_asymmetric_decrypt(*key, PSA_ALG_RSA_PKCS1V15_CRYPT,
+    status = psa_asymmetric_decrypt(pk->priv_id, PSA_ALG_RSA_PKCS1V15_CRYPT,
                                     input, ilen,
                                     NULL, 0,
                                     output, osize, olen);
@@ -1687,8 +1668,8 @@ const mbedtls_pk_info_t mbedtls_pk_rsa_opaque_info = {
 #endif /* PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY */
     NULL, /* encrypt - will be done later */
     NULL, /* check_pair - could be done later or left NULL */
-    pk_opaque_alloc_wrap,
-    pk_opaque_free_wrap,
+    NULL, /* alloc - no need to allocate new data dynamically */
+    NULL, /* free - as for the alloc, there is no data to free */
 #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE)
     NULL, /* restart alloc - not relevant */
     NULL, /* restart free - not relevant */

library/pkwrite.c

@@ -188,14 +188,13 @@ int mbedtls_pk_write_pubkey(unsigned char **p, unsigned char *start,
 #if defined(MBEDTLS_USE_PSA_CRYPTO)
     if (mbedtls_pk_get_type(key) == MBEDTLS_PK_OPAQUE) {
         size_t buffer_size;
-        mbedtls_svc_key_id_t *key_id = (mbedtls_svc_key_id_t *) key->pk_ctx;
 
         if (*p < start) {
             return MBEDTLS_ERR_PK_BAD_INPUT_DATA;
         }
 
         buffer_size = (size_t) (*p - start);
-        if (psa_export_public_key(*key_id, start, buffer_size, &len)
+        if (psa_export_public_key(key->priv_id, start, buffer_size, &len)
             != PSA_SUCCESS) {
             return MBEDTLS_ERR_PK_BAD_INPUT_DATA;
         } else {
@@ -254,9 +253,9 @@ int mbedtls_pk_write_pubkey_der(const mbedtls_pk_context *key, unsigned char *bu
     if (pk_type == MBEDTLS_PK_OPAQUE) {
         psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
         psa_key_type_t key_type;
-        mbedtls_svc_key_id_t key_id;
-        key_id = *((mbedtls_svc_key_id_t *) key->pk_ctx);
-        if (PSA_SUCCESS != psa_get_key_attributes(key_id, &attributes)) {
+
+        if (PSA_SUCCESS != psa_get_key_attributes(key->priv_id,
+                                                  &attributes)) {
             return MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED;
         }
         key_type = psa_get_key_type(&attributes);

library/ssl_tls12_server.c

@@ -2614,8 +2614,7 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl)
                 return MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH;
             }
 
-            ssl->handshake->ecdh_psa_privkey =
-                *((mbedtls_svc_key_id_t *) pk->pk_ctx);
+            ssl->handshake->ecdh_psa_privkey = pk->priv_id;
 
             /* Key should not be destroyed in the TLS library */
             ssl->handshake->ecdh_psa_privkey_is_external = 1;

tests/src/test_helpers/ssl_helpers.c

@@ -595,8 +595,7 @@ static void test_ssl_endpoint_certificate_free(mbedtls_test_ssl_endpoint *ep)
         if (cert->pkey != NULL) {
 #if defined(MBEDTLS_USE_PSA_CRYPTO)
             if (mbedtls_pk_get_type(cert->pkey) == MBEDTLS_PK_OPAQUE) {
-                mbedtls_svc_key_id_t *key_slot = cert->pkey->pk_ctx;
-                psa_destroy_key(*key_slot);
+                psa_destroy_key(cert->pkey->priv_id);
             }
 #endif
             mbedtls_pk_free(cert->pkey);