Don't panic when padding is wrong in asymmetric decryption 91/201991/2
authorKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Thu, 21 Mar 2019 10:36:28 +0000 (11:36 +0100)
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Fri, 17 May 2019 12:25:28 +0000 (14:25 +0200)
According to GP Internal API 1.1.2 TEE_AsymmetricDecrypt() should not panic but
return TEE_BAD_PARAMETERS in case of incorrect ciphertext padding.

Return TEE_BAD_PARAMETERS if crypto_internal_final() fails with
CRYPTO_INVALID_ARGUMENT instead of panicking.

Update related code.

Change-Id: I576c1699cf284d501e13d7367f936c708d924ec5

include/include/tee_internal_api.h
ssflib/src/ssf_crypto.cpp

index b43b487b0b928902109105ea0b6ca6d142126f09..de7090629b534e8dff011e4130239783d0d756ae 100644 (file)
@@ -650,7 +650,7 @@ TEE_Result TEE_GetNextProperty(TEE_PropSetHandle enumerator);
  * @param[in] panicCode An informative panic code defined by the TA. May be
  * displayed in traces if traces are available.
  */
-void TEE_Panic(TEE_Result panicCode);
+void TEE_Panic(TEE_Result panicCode) __attribute__((noreturn));
 
 /******************************************************************************
  *  4.9 Internal Client API
index 684435071699a81d80a7f7ee86cd73015c97c116..e1c89146c425d100151934ce06dcae901d66d457 100644 (file)
@@ -455,7 +455,7 @@ static int sw_crypto_ioctl_init(crypto_internal_operation *operation, crypto_int
                case TEE_ALG_GENERATE_SECRET_KEY:
                        rc = handle->PRNG_get(handle, key->secret.size, key->secret.buffer);
                        /* Ignore return value to avoid CRYPTO_PANIC. Only SDRM_X931_ConditionalTest() can return TEE_ERROR.*/
-                       rc = TEE_SUCCESS;
+                       rc = 0;
                        break;
 
                case TEE_ALG_GENERATE_RSA_KEY:
@@ -1138,7 +1138,7 @@ int crypto_internal_final(crypto_internal_operation *operation, unsigned char *s
                                {
                                        processing_len = out_size;
                                        if (crypto_update_engine(operation, operation->data, operation->data_len, out_data, &processing_len)) {
-                                               goto error;
+                                               return -1;
                                        }
                                        total_processing_len += processing_len;
                                        out_size -= processing_len;
@@ -1167,7 +1167,7 @@ int crypto_internal_final(crypto_internal_operation *operation, unsigned char *s
                        {
                                processing_len = out_size-total_processing_len;
                                if (crypto_update_engine(operation, in_data, should_be_processed_of_bytes, out_data, &processing_len)) {
-                                       goto error;
+                                       return -1;
                                }
                                total_processing_len += processing_len;
                                in_size -= processing_len;
@@ -1195,7 +1195,7 @@ int crypto_internal_final(crypto_internal_operation *operation, unsigned char *s
                                operation->data_len += remaining_number_of_bytes;
 
                                if (dst_len && *dst_len < total_processing_len+operation->block_len) {
-                                       return TEE_ERROR_SHORT_BUFFER;
+                                       return -2;
                                }
 
                                pad_byte = operation->block_len - remaining_number_of_bytes;
@@ -1232,7 +1232,7 @@ int crypto_internal_final(crypto_internal_operation *operation, unsigned char *s
                                }
 
                                if (crypto_final_engine(operation, operation->data, should_be_processed_of_pad_bytes, out_data, &processing_len)) {
-                                       goto error;
+                                       return -1;
                                }
 
                                total_processing_len += processing_len;
@@ -1304,12 +1304,12 @@ int crypto_internal_final(crypto_internal_operation *operation, unsigned char *s
                                        operation->data_len += remaining_number_of_bytes;
 
                                        if (crypto_final_engine(operation, operation->data, remaining_number_of_bytes, out_data, &processing_len)) {
-                                               goto error;
+                                               return -1;
                                        }
                                        total_processing_len += remaining_number_of_bytes;
                                }
                        } else {
-                               goto error;
+                               return -1;
                        }
                }
        } else if (operation->info.operationClass == TEE_OPERATION_MAC || operation->info.operationClass == TEE_OPERATION_DIGEST) {
@@ -1331,7 +1331,7 @@ int crypto_internal_final(crypto_internal_operation *operation, unsigned char *s
                                /* accumulated data is full */
                                if (operation->data_len == operation->block_len) {
                                        if (crypto_update_engine(operation, operation->data, operation->data_len, NULL, NULL)) {
-                                               goto error;
+                                               return -1;
                                        }
                                        operation->data_len = 0;
                                }
@@ -1347,13 +1347,13 @@ int crypto_internal_final(crypto_internal_operation *operation, unsigned char *s
 
                if (in_size != 0) {
                        if (crypto_final_engine(operation, in_data, in_size, out_data, &out_size)) {
-                               goto error;
+                               return -1;
                        }
                        total_processing_len += in_size;
                }
        } else {
                if (crypto_final_engine(operation, in_data, in_size, out_data, &out_size)) {
-                       goto error;
+                       return -1;
                }
                total_processing_len += in_size;
        }
@@ -1374,10 +1374,6 @@ exit:
                *dst_len = out_size;
        }
        return 0;
-error:
-       LOGE(MODULE_SSF_LIB, "THIS HERE!!!");
-       CRYPTO_INTERNAL_LOG("--------------------------------------------------------------");
-       return -1;
 }
 
 
@@ -2065,10 +2061,15 @@ TEE_Result TEE_DigestDoFinal(TEE_OperationHandle operation, const void* chunk, s
        if (!(op->info.handleState & TEE_HANDLE_FLAG_INITIALIZED)) {
                TEE_DigestInit(operation);
        }
-       if(crypto_internal_final(op, (unsigned char*)chunk, chunkLen, (unsigned char*)hash, hashLen)) {
+       int ret = crypto_internal_final(op, (unsigned char*)chunk, chunkLen, (unsigned char*)hash, hashLen);
+       switch (ret) {
+       case 0:
+               return TEE_SUCCESS;
+       case -1:
+       case -2:
+       default:
                CRYPTO_PANIC;
        }
-       return TEE_SUCCESS;
 }
 
 // Symmetric Cipher Functions
@@ -2128,19 +2129,22 @@ TEE_Result TEE_CipherDoFinal(TEE_OperationHandle operation, const void* srcData,
        PERMISSION_CHECK(PERM_CRYPTO);
        crypto_internal_operation * op = (crypto_internal_operation*) operation;
 
-       if (*destLen < srcLen) {
-               return TEE_ERROR_SHORT_BUFFER;
-       }
        if (op->info.operationClass != TEE_OPERATION_CIPHER) {
                CRYPTO_PANIC;
        }
        if (!(op->info.handleState & TEE_HANDLE_FLAG_INITIALIZED)) {
                CRYPTO_PANIC;
        }
-       if (crypto_internal_final(op, (unsigned char*)srcData, srcLen, (unsigned char*)destData, destLen)) {
+       int ret = crypto_internal_final(op, (unsigned char*)srcData, srcLen, (unsigned char*)destData, destLen);
+       switch (ret) {
+       case 0:
+               return TEE_SUCCESS;
+       case -2:
+               return TEE_ERROR_SHORT_BUFFER;
+       case -1:
+       default:
                CRYPTO_PANIC;
        }
-       return TEE_SUCCESS;
 }
 
 // MAC Functions
@@ -2209,10 +2213,16 @@ TEE_Result TEE_MACComputeFinal(TEE_OperationHandle operation, const void* messag
        if (!(op->info.handleState & TEE_HANDLE_FLAG_INITIALIZED)) {
                CRYPTO_PANIC;
        }
-       if(crypto_internal_final(op, (unsigned char*)message, messageLen, (unsigned char*)mac, macLen)) {
+       int ret = crypto_internal_final(op, (unsigned char*)message, messageLen, (unsigned char*)mac, macLen);
+       return TEE_SUCCESS;
+       switch (ret) {
+       case 0:
+               return TEE_SUCCESS;
+       case -1:
+       case -2:
+       default:
                CRYPTO_PANIC;
        }
-       return TEE_SUCCESS;
 }
 
 TEE_Result TEE_MACCompareFinal(TEE_OperationHandle operation, void* message, size_t messageLen, void* mac, size_t *macLen)
@@ -2461,10 +2471,17 @@ TEE_Result TEE_AsymmetricEncrypt(TEE_OperationHandle operation, const TEE_Attrib
        if (crypto_internal_init(op, &key, NULL, 0)) {
                CRYPTO_PANIC;
        }
-       if (crypto_internal_final(op, (unsigned char*)srcData, srcLen, (unsigned char*)destData, destLen)) {
-               return TEE_ERROR_SIGNATURE_INVALID;
+       int ret = crypto_internal_final(op, (unsigned char*)srcData, srcLen, (unsigned char*)destData, destLen);
+       switch (ret) {
+       case 0:
+               return TEE_SUCCESS;
+       case -1:
+               return TEE_ERROR_BAD_PARAMETERS;
+       case -2:
+               return TEE_ERROR_SHORT_BUFFER;
+       default:
+               CRYPTO_PANIC;
        }
-       return TEE_SUCCESS;
 }
 
 TEE_Result TEE_AsymmetricDecrypt(TEE_OperationHandle operation, const TEE_Attribute* params, uint32_t paramCount, const void* srcData, size_t srcLen, void* destData, size_t *destLen)
@@ -2539,10 +2556,17 @@ TEE_Result TEE_AsymmetricDecrypt(TEE_OperationHandle operation, const TEE_Attrib
        if (crypto_internal_init(op, &key, NULL, 0)) {
                CRYPTO_PANIC;
        }
-       if (crypto_internal_final(op, (unsigned char*)srcData, srcLen, (unsigned char*)destData, destLen)) {
+       int ret = crypto_internal_final(op, (unsigned char*)srcData, srcLen, (unsigned char*)destData, destLen);
+       switch (ret) {
+       case 0:
+               return TEE_SUCCESS;
+       case -1:
+               return TEE_ERROR_BAD_PARAMETERS;
+       case -2:
+               return TEE_ERROR_SHORT_BUFFER;
+       default:
                CRYPTO_PANIC;
        }
-       return TEE_SUCCESS;
 }
 
 TEE_Result TEE_AsymmetricSignDigest( TEE_OperationHandle operation, const TEE_Attribute* params, uint32_t paramCount, const void* digest, size_t digestLen, void* signature, size_t *signatureLen)
@@ -2616,10 +2640,16 @@ TEE_Result TEE_AsymmetricSignDigest( TEE_OperationHandle operation, const TEE_At
        if (crypto_internal_init(op, &key, NULL, 0)) {
                CRYPTO_PANIC;
        }
-       if (crypto_internal_final(op, (unsigned char*)digest, digestLen, (unsigned char*)signature, signatureLen)) {
+       int ret = crypto_internal_final(op, (unsigned char*)digest, digestLen, (unsigned char*)signature, signatureLen);
+       switch (ret) {
+       case 0:
+               return TEE_SUCCESS;
+       case -2:
                return TEE_ERROR_SHORT_BUFFER;
+       case -1:
+       default:
+               CRYPTO_PANIC;
        }
-       return TEE_SUCCESS;
 }
 
 TEE_Result TEE_AsymmetricVerifyDigest( TEE_OperationHandle operation, const TEE_Attribute* params, uint32_t paramCount, const void* digest, size_t digestLen, void* signature, size_t signatureLen)
@@ -2663,10 +2693,16 @@ TEE_Result TEE_AsymmetricVerifyDigest( TEE_OperationHandle operation, const TEE_
        if (crypto_internal_init(op, &key, NULL, 0)) {
                CRYPTO_PANIC;
        }
-       if (crypto_internal_final(op, (unsigned char*)digest, digestLen, (unsigned char*)signature, &sign_len)) {
+       int ret = crypto_internal_final(op, (unsigned char*)digest, digestLen, (unsigned char*)signature, &sign_len);
+       switch (ret) {
+       case 0:
+               return TEE_SUCCESS;
+       case -1:
                return TEE_ERROR_SIGNATURE_INVALID;
+       case -2:
+       default:
+               CRYPTO_PANIC;
        }
-       return TEE_SUCCESS;
 }
 
 // Key Derivation Functions