A function that was previously called in multiple places is now called
only once, hence more susceptible to being inlined, hence the test fix.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The overall function is still not constant-time, but it just got a lot
less leaky.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The path using builtin should be OK, as it should be using dedicated CPU
instructions which are constant time.
This fixes the no-builing path.
GCC gained support for __has_builtin in version 10. We're still testing
with older GCC on the CI, so the non-builtin path is tested on the CI.
https://gcc.gnu.org/gcc-10/changes.html
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
In the next refactoring we'll have:
- pretty different paths for N odd or even,
- possibly different paths for A <= 0, in [0, N) or above,
- possibly special cases when A % N is 0 or 1.
Pick two small moduli of different parities (3 and 4)
and go over the range [-(N+1), 2N-1] with A.
This should ensure we naturally run into all special cases.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Addresses https://github.com/Mbed-TLS/mbedtls/issues/9586 .
This is not a fully satisfactory resolution, because we don't run every
constant-flow test with Valgrind in PR jobs, only a small subset. We should
improve the coverage/resource balance.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Some tests from `test_suite_cipher.constant_time.data` follow the same
pattern as `test_suite_cipher.aes.data` and so have the same coverage
discrepancy.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The decrypted length reveals the amount of padding that was eliminated, and
thus reveals partial information about the last ciphertext block.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
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>
New function `mbedtls_cipher_finish_padded()`, similar to
`mbedtls_cipher_finish()`, but reporting padding errors through a separate
output parameter. This makes it easier to avoid leaking the presence of a
padding error, especially through timing. Thus the new function is
recommended to defend against padding oracle attacks.
In this commit, implement this function naively, with timing that depends on
whether an error happened. A subsequent commit will make this function
constant-time.
Copy the test decrypt_test_vec and decrypt_test_vec_cf test cases into
variants that call `mbedtls_cipher_finish_padded()`.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Add some basic constant-flow tests for `mbedtls_cipher_crypt()`. We already
test auxiliary functions and functional behavior pretty thoroughly
elsewhere, so here just focus on the interesting cases for constant-flow
behavior with this specific function: encrypt, valid decrypt and
invalid-padding decrypt.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The main goal is to validate that unpadding is constant-time, including
error reporting.
Use a separate test function, not annotations in the existing function, so
that the functional tests can run on any platform, and we know from test
outcomes where we have run the constant-time tests.
The tests can only be actually constant-time if AES is constant time, since
AES computations are part of what is checked. Thus this requires
hardware-accelerated AES. We can't run our AESNI (or AESCE?) code under
Msan (it doesn't detect when memory is written from assembly code), so these
tests can only be run with Valgrind.
Same test data as the newly introduced functional tests.
#!/usr/bin/env python3
from Crypto.Cipher import AES
KEYS = {
128: bytes.fromhex("ffffffffe00000000000000000000000"),
192: bytes.fromhex("000000000000000000000000000000000000000000000000"),
256: bytes.fromhex("0000000000000000000000000000000000000000000000000000000000000000"),
}
IV = bytes.fromhex("00000000000000000000000000000000")
def decrypt_test_vec(cf, bits, mode, padded_hex, padding_length, note=''):
depends = ['MBEDTLS_AES_C', 'MBEDTLS_CIPHER_MODE_CBC']
plaintext = bytes.fromhex(padded_hex)
plaintext_length = len(plaintext)
if bits != 128:
depends.append('!MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH')
key = KEYS[bits]
iv = IV
result = '0'
if mode == 'NONE':
padding_description = 'no padding'
assert padding_length == 0
else:
depends.append('MBEDTLS_CIPHER_PADDING_' + mode)
padding_description = mode
if padding_length is None:
result = 'MBEDTLS_ERR_CIPHER_INVALID_PADDING'
plaintext_length = 0
else:
plaintext_length -= padding_length
cipher = AES.new(key, AES.MODE_CBC, iv=iv)
ciphertext = cipher.encrypt(plaintext)
function = 'decrypt_test_vec'
cf_maybe = ''
if cf:
function += '_cf'
cf_maybe = 'CF '
depends.append('HAVE_CONSTANT_TIME_AES')
if note:
note = f' ({note})'
print(f'''\
{cf_maybe}AES-{bits}-CBC Decrypt test vector, {padding_description}{note}
depends_on:{':'.join(depends)}
{function}:MBEDTLS_CIPHER_AES_{bits}_CBC:MBEDTLS_PADDING_{mode}:"{key.hex()}":"{iv.hex()}":"{ciphertext.hex()}":"{plaintext[:plaintext_length].hex()}":"":"":{result}:0
''')
def emit_tests(cf):
# Already existing tests
decrypt_test_vec(cf, 128, 'NONE', "00000000000000000000000000000000", 0)
decrypt_test_vec(cf, 192, 'NONE', "fffffffff80000000000000000000000", 0)
decrypt_test_vec(cf, 256, 'NONE', "ff000000000000000000000000000000", 0)
# New tests
decrypt_test_vec(cf, 128, 'PKCS7', "00000000000000000000000000000001", 1, 'good pad 1')
decrypt_test_vec(cf, 192, 'PKCS7', "fffffffff80000000000000000000001", 1, 'good pad 1')
decrypt_test_vec(cf, 256, 'PKCS7', "ff000000000000000000000000000001", 1, 'good pad 1')
decrypt_test_vec(cf, 128, 'PKCS7', "00000000000000000000000000000202", 2, 'good pad 2')
decrypt_test_vec(cf, 192, 'PKCS7', "fffffffff80000000000000000000202", 2, 'good pad 2')
decrypt_test_vec(cf, 256, 'PKCS7', "ff000000000000000000000000000202", 2, 'good pad 2')
decrypt_test_vec(cf, 128, 'PKCS7', "2a0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f", 15, 'good pad 15')
decrypt_test_vec(cf, 192, 'PKCS7', "2a0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f", 15, 'good pad 15')
decrypt_test_vec(cf, 256, 'PKCS7', "2a0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f", 15, 'good pad 15')
decrypt_test_vec(cf, 128, 'PKCS7', "10101010101010101010101010101010", 16, 'good pad 16')
decrypt_test_vec(cf, 192, 'PKCS7', "10101010101010101010101010101010", 16, 'good pad 16')
decrypt_test_vec(cf, 256, 'PKCS7', "10101010101010101010101010101010", 16, 'good pad 16')
decrypt_test_vec(cf, 128, 'PKCS7', "00000000000000000000000000000000", None, 'bad pad 0')
decrypt_test_vec(cf, 192, 'PKCS7', "fffffffff80000000000000000000000", None, 'bad pad 0')
decrypt_test_vec(cf, 256, 'PKCS7', "ff000000000000000000000000000000", None, 'bad pad 0')
decrypt_test_vec(cf, 128, 'PKCS7', "00000000000000000000000000000102", None, 'bad pad 0102')
decrypt_test_vec(cf, 192, 'PKCS7', "fffffffff80000000000000000000102", None, 'bad pad 0102')
decrypt_test_vec(cf, 256, 'PKCS7', "ff000000000000000000000000000102", None, 'bad pad 0102')
decrypt_test_vec(cf, 128, 'PKCS7', "1111111111111111111111111111111111111111111111111111111111111111", None, 'long, bad pad 17')
decrypt_test_vec(cf, 192, 'PKCS7', "1111111111111111111111111111111111111111111111111111111111111111", None, 'long, bad pad 17')
decrypt_test_vec(cf, 256, 'PKCS7', "1111111111111111111111111111111111111111111111111111111111111111", None, 'long, bad pad 17')
decrypt_test_vec(cf, 128, 'PKCS7', "11111111111111111111111111111111", None, 'short, bad pad 17')
decrypt_test_vec(cf, 192, 'PKCS7', "11111111111111111111111111111111", None, 'short, bad pad 17')
decrypt_test_vec(cf, 256, 'PKCS7', "11111111111111111111111111111111", None, 'short, bad pad 17')
emit_tests(True)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Do constant-time testing in a couple of configurations that give some
interesting coverage;
* In a configuration that's close to the default: `test_aes_only_128_bit_keys`.
Having only 128-bit AES keys doesn't reduce the interesting scope much
(except that it doesn't test 192-bit and 256-bit AES, but since that
configuration uses hardware AES, we don't care about that part).
* when PSA buffer copying is not done, i.e. when
`MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS` is enabled. This will be very
relevant for the upcoming PSA constant-time tests.
Use Valgrind, since some of the interesting tests require constant-time AES,
which for us means AESNI or AESCE, which MSan doesn't support.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
