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>
That rule is common to the whole module and not a likely mistake to
make. Also, the test was not really precise as G, I, T were oversized.
Better remove it than give a false sense of security.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
With this data, the loop only settles to its final state u == 0 and v ==
GCD(A, N) at the last iteration. However it already has v == GCD(A, N)
at the previous iteration. Concretely, this means that if in
mbedtls_mpi_core_gcd_modinv_odd() we change the main loop as follows
- for (size_t i = 0; i < (A_limbs + N_limbs) * biL; i++) {
+ for (size_t i = 0; i < (A_limbs + N_limbs) * biL - 2; i++) {
then this test case would fail. Ideally we'd like a test case that would
fail with -1 above but I've not been able to find one and I have no idea
whether that's possible.
Experimentally I've systematically tried small values (8 bit) and
noticed the case A = 2^n and N significantly larger then A is promising,
so I explored that further. Clearly we want A and N's bitlength to be a
multiple of biL because the bound in the paper is with bitlenths while
we use limbs * biL.
Anyway, I ended up with the following Python script.
import secrets
import math
bil = 64
def bitlimbs(x):
return (x.bit_length() + bil - 1) // bil * bil
def sict_gcd(p, a):
assert p >= a >= 0
assert p & 1 != 0 or a & 1 != 0
u, v = a, p
for i in range(2 * p.bit_length()):
s, z = u & 1, v & 1
t1 = (s ^ z) * v + (2 * s * z - 1) * u
t2 = (s * v + (2 - 2 * s - z) * u) >> 1
if t2 >= t1:
u, v = t1, t2
else:
u, v = t2, t1
if u == 0: # v == 1 ideally, but can't get it
return bitlimbs(a) + bitlimbs(p) - (i + 1)
return 0
a = 2 ** (bil - 1)
m = 1000
while m != 0:
n = secrets.randbits(2 * bil) | 1
d = sict_gcd(n, a)
if d < m:
m = d
print(d)
g = math.gcd(a, n)
i = pow(a, -1, n)
print(f'mpi_core_gcd_modinv_odd:"{a:x}":"{n:x}":"{g:x}":"{i:x}"')
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
A == N (as pointers) will not happen in pratice: in our context, it
would mean we know at compile time that A == N (as values), and we
wouldn't be calling this function if we knew that already.
N == 1 when I != NULL is also not going to happen: we don't care about
operations mod 1.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This function has specific code to handle carries and it's not clear how
to exercises that code through the modinv function, so well, that's what
unit tests are for.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This is consistent with the general rules documented at the top of the
file:
- when computing GCD(A, N), there is no modular arithmetic, so the
output can alias any of the inputs;
- when computing a modular inverse, N is the modulus, so it can't be
aliased by any of the outputs (we'll use it for modular operations
over the entire course of the function's execution).
But since this function has two modes of operations with different
aliasing rules (G can alias N only if I == NULL), I think it should
really be stated explicitly.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This is a direct translation of sict_mi2() from
https://github.com/mpg/cryptohack/blob/main/ct-pres.py
which was presented in the book club's special session.
This commit only includes two test cases which is very little. Most of
the test cases will be generated by Python modules that belong to the
framework. However we can't have the framework generate those before we
have the corresponding test function in the consuming branches. So,
extended tests are coming as a 2nd step, after the test function has
been merged.
(The test cases in .misc should stay, as they can be convenient when
working on the test function.)
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The dependencies declared in ci.requirements.txt are only used in
scripts that we run on the Linux CI.
Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
Recent versions of cryptography require a Rust toolchain to install on
FreeBSD, which we do not have set up yet.
Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
The version was unspecified because of our use of Python 3.5 on the CI,
whichi has since been eliminated.
Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
