commit 6d839f05bf1404c8d424bdf9a858e7d521f262f3
author Steven Cooreman <steven.cooreman@silabs.com> Thu Jul 30 11:36:45 2020 +0200
committer Steven Cooreman <steven.cooreman@silabs.com> Thu Jul 30 15:21:41 2020 +0200
tree 1a4efa364f51998a270d9926189e387f74546992
parent a2371e53e4146828de28855929b144fe89d47ed1

Cleanup

* No null-check before calling free
* Close memory leak
* No need for double check of privkey validity

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>

library/psa_crypto.c

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 15a612b..b6a34df 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -691,8 +691,7 @@
 exit:
     /* Always free the RSA object */
     mbedtls_rsa_free( rsa );
-    if( rsa != NULL )
-        mbedtls_free( rsa );
+    mbedtls_free( rsa );
 
     /* Free the allocated buffer only on error. */
     if( status != PSA_SUCCESS )
@@ -726,10 +725,7 @@
     mbedtls_ecp_group_id grp_id = MBEDTLS_ECP_DP_NONE;
     size_t data_length = slot->data.key.bytes;
     psa_status_t status;
-    mbedtls_ecp_keypair *ecp = mbedtls_calloc(1, sizeof(mbedtls_ecp_keypair));
-
-    if( ecp == NULL )
-        return PSA_ERROR_INSUFFICIENT_MEMORY;
+    mbedtls_ecp_keypair *ecp = NULL;
 
     if( PSA_KEY_TYPE_IS_PUBLIC_KEY( slot->attr.type ) )
     {
@@ -744,19 +740,27 @@
         data_length = slot->data.key.bytes / 2;
     }
 
+    /* Allocate and initialize a key representation. */
+    ecp = mbedtls_calloc(1, sizeof(mbedtls_ecp_keypair));
+    if( ecp == NULL )
+        return PSA_ERROR_INSUFFICIENT_MEMORY;
     mbedtls_ecp_keypair_init( ecp );
 
     /* Load the group. */
-    grp_id = mbedtls_ecc_group_of_psa( PSA_KEY_TYPE_ECC_GET_FAMILY( slot->attr.type),
+    grp_id = mbedtls_ecc_group_of_psa( PSA_KEY_TYPE_ECC_GET_FAMILY( slot->attr.type ),
                                        data_length );
     if( grp_id == MBEDTLS_ECP_DP_NONE )
-        return( PSA_ERROR_INVALID_ARGUMENT );
+    {
+        status = PSA_ERROR_INVALID_ARGUMENT;
+        goto exit;
+    }
+
     status = mbedtls_to_psa_error(
                 mbedtls_ecp_group_load( &ecp->grp, grp_id ) );
     if( status != PSA_SUCCESS )
-        return( status );
+        goto exit;
 
-    /* Load the key material */
+    /* Load the key material. */
     if( PSA_KEY_TYPE_IS_PUBLIC_KEY( slot->attr.type ) )
     {
         /* Load the public value. */
@@ -775,7 +779,7 @@
     }
     else
     {
-        /* Load the secret value. */
+        /* Load and validate the secret value. */
         status = mbedtls_to_psa_error(
             mbedtls_ecp_read_key( ecp->grp.id,
                                   ecp,
@@ -784,11 +788,6 @@
 
         if( status != PSA_SUCCESS )
             goto exit;
-        /* Validate the private key. */
-        status = mbedtls_to_psa_error(
-            mbedtls_ecp_check_privkey( &ecp->grp, &ecp->d ) );
-        if( status != PSA_SUCCESS )
-            goto exit;
     }
 
     *p_ecp = ecp;
@@ -892,8 +891,7 @@
 exit:
     /* Always free the PK object (will also free contained ECP context) */
     mbedtls_ecp_keypair_free( ecp );
-    if( ecp != NULL )
-        mbedtls_free( ecp );
+    mbedtls_free( ecp );
 
     /* Free the allocated buffer only on error. */
     if( status != PSA_SUCCESS )
@@ -2003,12 +2001,12 @@
         {
             mbedtls_rsa_context *rsa = NULL;
             mbedtls_mpi actual, required;
+            int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
 
             psa_status_t status = psa_load_rsa_representation( slot, &rsa );
             if( status != PSA_SUCCESS )
                 return status;
 
-            int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
             mbedtls_mpi_init( &actual );
             mbedtls_mpi_init( &required );
             ret = mbedtls_rsa_export( rsa,
@@ -5592,8 +5590,7 @@
         mbedtls_platform_zeroize( shared_secret, shared_secret_size );
     mbedtls_ecdh_free( &ecdh );
     mbedtls_ecp_keypair_free( their_key );
-    if( their_key != NULL)
-        mbedtls_free( their_key );
+    mbedtls_free( their_key );
 
     return( status );
 }