Switch legacy cipher to constant-time invalid padding reporting
In internal `get_padding` functions, report whether the padding was invalid
through a separate output parameter, rather than the return code. Take
advantage of this to have `mbedtls_cipher_finish_padded()` be the easy path
that just passes the `invalid_padding` through. Make
`mbedtls_cipher_finish()` a wrapper around `mbedtls_cipher_finish_padded()`
that converts the invalid-padding output into an error code.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h
index 6ef703d..3778f44 100644
--- a/include/mbedtls/cipher.h
+++ b/include/mbedtls/cipher.h
@@ -329,8 +329,15 @@
/** Padding functions to use, if relevant for
* the specific cipher mode.
*/
- void(*MBEDTLS_PRIVATE(add_padding))(unsigned char *output, size_t olen, size_t data_len);
- int(*MBEDTLS_PRIVATE(get_padding))(unsigned char *input, size_t ilen, size_t *data_len);
+ void(*MBEDTLS_PRIVATE(add_padding))(unsigned char *output, size_t olen,
+ size_t data_len);
+ /* Report invalid-padding condition through the output parameter
+ * invalid_padding. To minimize changes in Mbed TLS 3.6, where this
+ * declaration is in a public header, use the public type size_t
+ * rather than the internal type mbedtls_ct_condition_t. */
+ int(*MBEDTLS_PRIVATE(get_padding))(unsigned char *input, size_t ilen,
+ size_t *data_len,
+ size_t *invalid_padding);
#endif
/** Buffer for input that has not been processed yet. */
diff --git a/library/cipher.c b/library/cipher.c
index f3e2f91..f9d4621 100644
--- a/library/cipher.c
+++ b/library/cipher.c
@@ -846,7 +846,8 @@
*/
MBEDTLS_STATIC_TESTABLE int mbedtls_get_pkcs_padding(unsigned char *input,
size_t input_len,
- size_t *data_len)
+ size_t *data_len,
+ size_t *invalid_padding)
{
size_t i, pad_idx;
unsigned char padding_len;
@@ -872,7 +873,8 @@
/* If the padding is invalid, set the output length to 0 */
*data_len = mbedtls_ct_if(bad, 0, input_len - padding_len);
- return mbedtls_ct_error_if_else_0(bad, MBEDTLS_ERR_CIPHER_INVALID_PADDING);
+ *invalid_padding = mbedtls_ct_size_if_else_0(bad, SIZE_MAX);
+ return 0;
}
#endif /* MBEDTLS_CIPHER_PADDING_PKCS7 */
@@ -893,7 +895,7 @@
}
static int get_one_and_zeros_padding(unsigned char *input, size_t input_len,
- size_t *data_len)
+ size_t *data_len, size_t *invalid_padding)
{
if (NULL == input || NULL == data_len) {
return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA;
@@ -916,7 +918,8 @@
in_padding = mbedtls_ct_bool_and(in_padding, mbedtls_ct_bool_not(is_nonzero));
}
- return mbedtls_ct_error_if_else_0(bad, MBEDTLS_ERR_CIPHER_INVALID_PADDING);
+ *invalid_padding = mbedtls_ct_size_if_else_0(bad, SIZE_MAX);
+ return 0;
}
#endif /* MBEDTLS_CIPHER_PADDING_ONE_AND_ZEROS */
@@ -937,7 +940,7 @@
}
static int get_zeros_and_len_padding(unsigned char *input, size_t input_len,
- size_t *data_len)
+ size_t *data_len, size_t *invalid_padding)
{
size_t i, pad_idx;
unsigned char padding_len;
@@ -963,7 +966,8 @@
bad = mbedtls_ct_bool_or(bad, nonzero_pad_byte);
}
- return mbedtls_ct_error_if_else_0(bad, MBEDTLS_ERR_CIPHER_INVALID_PADDING);
+ *invalid_padding = mbedtls_ct_size_if_else_0(bad, SIZE_MAX);
+ return 0;
}
#endif /* MBEDTLS_CIPHER_PADDING_ZEROS_AND_LEN */
@@ -978,7 +982,7 @@
}
static int get_zeros_padding(unsigned char *input, size_t input_len,
- size_t *data_len)
+ size_t *data_len, size_t *invalid_padding)
{
size_t i;
mbedtls_ct_condition_t done = MBEDTLS_CT_FALSE, prev_done;
@@ -994,6 +998,7 @@
*data_len = mbedtls_ct_size_if(mbedtls_ct_bool_ne(done, prev_done), i, *data_len);
}
+ *invalid_padding = 0;
return 0;
}
#endif /* MBEDTLS_CIPHER_PADDING_ZEROS */
@@ -1005,20 +1010,21 @@
* but a trivial get_padding function
*/
static int get_no_padding(unsigned char *input, size_t input_len,
- size_t *data_len)
+ size_t *data_len, size_t *invalid_padding)
{
if (NULL == input || NULL == data_len) {
return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA;
}
*data_len = input_len;
-
+ *invalid_padding = 0;
return 0;
}
#endif /* MBEDTLS_CIPHER_MODE_WITH_PADDING */
-int mbedtls_cipher_finish(mbedtls_cipher_context_t *ctx,
- unsigned char *output, size_t *olen)
+int mbedtls_cipher_finish_padded(mbedtls_cipher_context_t *ctx,
+ unsigned char *output, size_t *olen,
+ size_t *invalid_padding)
{
if (ctx->cipher_info == NULL) {
return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA;
@@ -1034,6 +1040,7 @@
#endif /* MBEDTLS_USE_PSA_CRYPTO && !MBEDTLS_DEPRECATED_REMOVED */
*olen = 0;
+ *invalid_padding = 0;
#if defined(MBEDTLS_CIPHER_MODE_WITH_PADDING)
/* CBC mode requires padding so we make sure a call to
@@ -1110,7 +1117,7 @@
/* Set output size for decryption */
if (MBEDTLS_DECRYPT == ctx->operation) {
return ctx->get_padding(output, mbedtls_cipher_get_block_size(ctx),
- olen);
+ olen, invalid_padding);
}
/* Set output size for encryption */
@@ -1124,20 +1131,16 @@
return MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
}
-int mbedtls_cipher_finish_padded(mbedtls_cipher_context_t *ctx,
- unsigned char *output, size_t *olen,
- size_t *invalid_padding)
+int mbedtls_cipher_finish(mbedtls_cipher_context_t *ctx,
+ unsigned char *output, size_t *olen)
{
- *invalid_padding = 0;
- int ret = mbedtls_cipher_finish(ctx, output, olen);
-#if defined(MBEDTLS_CIPHER_PADDING_PKCS7) || \
- defined(MBEDTLS_CIPHER_PADDING_ONE_AND_ZEROS) || \
- defined(MBEDTLS_CIPHER_PADDING_ZEROS_AND_LEN)
- if (ret == MBEDTLS_ERR_CIPHER_INVALID_PADDING) {
- ret = 0;
- *invalid_padding = SIZE_MAX;
+ size_t invalid_padding = 0;
+ int ret = mbedtls_cipher_finish_padded(ctx, output, olen,
+ &invalid_padding);
+ if (ret == 0) {
+ ret = mbedtls_ct_error_if_else_0(invalid_padding,
+ MBEDTLS_ERR_CIPHER_INVALID_PADDING);
}
-#endif
return ret;
}
@@ -1410,14 +1413,17 @@
return ret;
}
- if ((ret = mbedtls_cipher_finish(ctx, output + *olen,
- &finish_olen)) != 0) {
+ size_t invalid_padding = 0;
+ if ((ret = mbedtls_cipher_finish_padded(ctx, output + *olen,
+ &finish_olen,
+ &invalid_padding)) != 0) {
return ret;
}
-
*olen += finish_olen;
- return 0;
+ ret = mbedtls_ct_error_if_else_0(invalid_padding,
+ MBEDTLS_ERR_CIPHER_INVALID_PADDING);
+ return ret;
}
#if defined(MBEDTLS_CIPHER_MODE_AEAD)
diff --git a/library/cipher_invasive.h b/library/cipher_invasive.h
index 702f8f7..e82a0a7 100644
--- a/library/cipher_invasive.h
+++ b/library/cipher_invasive.h
@@ -20,7 +20,8 @@
MBEDTLS_STATIC_TESTABLE int mbedtls_get_pkcs_padding(unsigned char *input,
size_t input_len,
- size_t *data_len);
+ size_t *data_len,
+ size_t *invalid_padding);
#endif
diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function
index fbc48b7..9e3e026 100644
--- a/tests/suites/test_suite_cipher.function
+++ b/tests/suites/test_suite_cipher.function
@@ -1475,8 +1475,8 @@
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_CIPHER_MODE_CBC */
-void check_padding(int pad_mode, data_t *input, int ret, int dlen_check
- )
+void check_padding(int pad_mode, data_t *input,
+ int expected_ret, int dlen_check)
{
mbedtls_cipher_info_t cipher_info;
mbedtls_cipher_context_t ctx;
@@ -1489,10 +1489,20 @@
TEST_ASSERT(0 == mbedtls_cipher_set_padding_mode(&ctx, pad_mode));
-
- TEST_ASSERT(ret == ctx.get_padding(input->x, input->len, &dlen));
- if (0 == ret) {
- TEST_ASSERT(dlen == (size_t) dlen_check);
+ size_t invalid_padding = 42;
+ int ret = ctx.get_padding(input->x, input->len, &dlen, &invalid_padding);
+ switch (expected_ret) {
+ case 0:
+ TEST_EQUAL(ret, 0);
+ TEST_EQUAL(invalid_padding, 0);
+ TEST_EQUAL(dlen, dlen_check);
+ break;
+ case MBEDTLS_ERR_CIPHER_INVALID_PADDING:
+ TEST_EQUAL(ret, 0);
+ TEST_EQUAL(invalid_padding, SIZE_MAX);
+ break;
+ default:
+ TEST_EQUAL(ret, expected_ret);
}
}
/* END_CASE */
@@ -1585,14 +1595,27 @@
{
int ret;
size_t calculated_len;
+ size_t invalid_padding;
TEST_CF_SECRET(decrypted_block->x, decrypted_block->len);
ret = mbedtls_get_pkcs_padding(decrypted_block->x, decrypted_block->len,
- &calculated_len);
+ &calculated_len, &invalid_padding);
- TEST_EQUAL(ret, exp_ret);
- if (exp_ret == 0) {
- TEST_EQUAL(calculated_len, exp_len);
+ switch (exp_ret) {
+ case 0:
+ TEST_EQUAL(ret, 0);
+ TEST_EQUAL(invalid_padding, 0);
+ TEST_EQUAL(calculated_len, exp_len);
+ break;
+
+ case MBEDTLS_ERR_CIPHER_INVALID_PADDING:
+ TEST_EQUAL(ret, 0);
+ TEST_EQUAL(invalid_padding, ~(size_t) 0);
+ break;
+
+ default:
+ TEST_EQUAL(ret, exp_ret);
+ break;
}
}
/* END_CASE */