mirror of
https://github.com/Mbed-TLS/mbedtls.git
synced 2025-12-24 17:41:01 +03:00
psa_cipher_decrypt: treat status and output length as sensitive
In `psa_cipher_decrypt()` and in the corresponding function in our built-in implementation `mbedtls_psa_cipher_decrypt()`, treat `status` and `*output_length` as sensitive variables whose value must not leak through a timing side channel. This is important when doing decryption with unpadding, where leaking the validity or amount of padding can enable a padding oracle attack. With this change, `psa_cipher_decrypt()` should be constant-time if the underlying legacy function (including the cipher implementation) is. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
@@ -4857,13 +4857,17 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key,
|
||||
|
||||
exit:
|
||||
unlock_status = psa_unregister_read_under_mutex(slot);
|
||||
if (status == PSA_SUCCESS) {
|
||||
if (unlock_status != PSA_SUCCESS) {
|
||||
status = unlock_status;
|
||||
}
|
||||
|
||||
if (status != PSA_SUCCESS) {
|
||||
*output_length = 0;
|
||||
}
|
||||
/* Set *output_length to 0 if status != PSA_SUCCESS, without
|
||||
* leaking the value of status through a timing side channel
|
||||
* (status == PSA_ERROR_INVALID_PADDING is sensitive when doing
|
||||
* unpadded decryption, due to the risk of padding oracle attack). */
|
||||
mbedtls_ct_condition_t success =
|
||||
mbedtls_ct_bool_not(mbedtls_ct_bool(status));
|
||||
*output_length = mbedtls_ct_size_if_else_0(success, *output_length);
|
||||
|
||||
LOCAL_INPUT_FREE(input_external, input);
|
||||
LOCAL_OUTPUT_FREE(output_external, output);
|
||||
|
||||
@@ -724,17 +724,21 @@ psa_status_t mbedtls_psa_cipher_decrypt(
|
||||
&operation,
|
||||
mbedtls_buffer_offset(output, accumulated_length),
|
||||
output_size - accumulated_length, &olength);
|
||||
if (status != PSA_SUCCESS) {
|
||||
goto exit;
|
||||
}
|
||||
|
||||
*output_length = accumulated_length + olength;
|
||||
|
||||
exit:
|
||||
if (status == PSA_SUCCESS) {
|
||||
status = mbedtls_psa_cipher_abort(&operation);
|
||||
} else {
|
||||
mbedtls_psa_cipher_abort(&operation);
|
||||
/* C99 doesn't allow a declaration to follow a label */;
|
||||
psa_status_t abort_status = mbedtls_psa_cipher_abort(&operation);
|
||||
/* Normally abort shouldn't fail unless the operation is in a bad
|
||||
* state, in which case we'd expect finish to fail with the same error.
|
||||
* So it doesn't matter much which call's error code we pick when both
|
||||
* fail. However, in unauthenticated decryption specifically, the
|
||||
* distinction between PSA_SUCCESS and PSA_ERROR_INVALID_PADDING is
|
||||
* security-sensitive (risk of a padding oracle attack), so here we
|
||||
* must not have a code path that depends on the value of status. */
|
||||
if (abort_status != PSA_SUCCESS) {
|
||||
status = abort_status;
|
||||
}
|
||||
|
||||
return status;
|
||||
|
||||
Reference in New Issue
Block a user