The signature of mbedtls_mpi_cmp_mpi_ct() meant to support using it in
place of mbedtls_mpi_cmp_mpi(). This meant full comparison functionality
and a signed result.
To make the function more universal and friendly to constant time
coding, we change the result type to unsigned. Theoretically, we could
encode the comparison result in an unsigned value, but it would be less
intuitive.
Therefore we won't be able to represent the result as unsigned anymore
and the functionality will be constrained to checking if the first
operand is less than the second. This is sufficient to support the
current use case and to check any relationship between MPIs.
The only drawback is that we need to call the function twice when
checking for equality, but this can be optimised later if an when it is
needed.
mbedtls_ctr_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_ctr_drbg_set_entropy_len() before seed() had no
effect. Change this to the more intuitive behavior that
set_entropy_len() sets the entropy length and seed() respects that and
only uses the default entropy length if there was no call to
set_entropy_len().
The former test-only function mbedtls_ctr_drbg_seed_entropy_len() is
no longer used, but keep it for strict ABI compatibility.
When running 'make test' with GNU make, if a test suite program
displays "PASSED", this was automatically counted as a pass. This
would in particular count as passing:
* A test suite with the substring "PASSED" in a test description.
* A test suite where all the test cases succeeded, but the final
cleanup failed, in particular if a sanitizer reported a memory leak.
Use the test executable's return status instead to determine whether
the test suite passed. It's always 0 on PASSED unless the executable's
cleanup code fails, and it's never 0 on any failure.
FixARMmbed/mbed-crypto#303
Some sanitizers default to displaying an error message and recovering.
This could result in a test being recorded as passing despite a
complaint from the sanitizer. Turn off sanitizer recovery to avoid
this risk.
Exercise the library functions with calloc returning NULL for a size
of 0. Make this a separate job with UBSan (and ASan) to detect
places where we try to dereference the result of calloc(0) or to do
things like
buf = calloc(size, 1);
if (buf == NULL && size != 0) return INSUFFICIENT_MEMORY;
memcpy(buf, source, size);
which has undefined behavior when buf is NULL at the memcpy call even
if size is 0.
This is needed because other test components jobs either use the system
malloc which returns non-NULL on Linux and FreeBSD, or the
memory_buffer_alloc malloc which returns NULL but does not give as
useful feedback with ASan (because the whole heap is a single C
object).
- Add configuration for AES_SCA_COUNTERMEASURES to config.h. By
default the feature is disabled.
- Add AES_SCA_COUNTERMEASURES configuration check to check_config.h
- Add AES_SCA_COUNTERMEASURES test to all.sh
This commit first changes the return convention of EccPoint_mult_safer() so
that it properly reports when faults are detected. Then all functions that
call it need to be changed to (1) follow the same return convention and (2)
properly propagate UECC_FAULT_DETECTED when it occurs.
Here's the reverse call graph from EccPoint_mult_safer() to the rest of the
library (where return values are translated to the MBEDTLS_ERR_ space) and test
functions (where expected return values are asserted explicitly).
EccPoint_mult_safer()
EccPoint_compute_public_key()
uECC_compute_public_key()
pkparse.c
tests/suites/test_suite_pkparse.function
uECC_make_key_with_d()
uECC_make_key()
ssl_cli.c
ssl_srv.c
tests/suites/test_suite_pk.function
tests/suites/test_suite_tinycrypt.function
uECC_shared_secret()
ssl_tls.c
tests/suites/test_suite_tinycrypt.function
uECC_sign_with_k()
uECC_sign()
pk.c
tests/suites/test_suite_tinycrypt.function
Note: in uECC_sign_with_k() a test for uECC_vli_isZero(p) is suppressed
because it is redundant with a more thorough test (point validity) done at the
end of EccPoint_mult_safer(). This redundancy was introduced in a previous
commit but not noticed earlier.
Added an additional Makefile option of 'TINYCRYPT_BUILD' to exclude the
TinyCrypt source files from the build. This allows some tests to exclude those
files as and when necessary.
Specifically this includes in all.sh the test
'component_build_arm_none_eabi_gcc_no_64bit_multiplication' which was failing as
64bit cannot be disabled in TinyCrypt, and check-names.sh as TinyCrypt obviously
does not conform to Mbed TLS naming conventions.
This is a first step in protecting against fault injection attacks: the
attacker can no longer change failure into success by flipping a single bit.
Additional steps are needed to prevent other attacks (instruction skip etc)
and will be the object of future commits.
The return value of uECC_vli_equal() should be protected as well, which will
be done in a future commit as well.
The certificate has a corrupted public key and signature.
Generating it through Makefile isn't trivial and since it is
a corrupted certificate, that shouldn't be accepted, there
shouldn't be a need to generate it again anyway.
Only add test when both are enforcing. This is configured in baremetal.h
and is checked in the check_cmdline_compat, would render other cases
with baremetal.h to skipped.
- Unset MBEDTLS_PADLOCK_C in aes_only_128_bit_keys test to get RAM
optimised version tested
- Use compiler flag "-O1" instead of "-Wall -Wextra" to see warnings
Ultimately, mbedtls_hardware_poll() is going to be provided by the
OS/environment when running on target. But for on-host programs and tests, we
need to define (a fake version) in each program that we want to be able to
link.
A previous commit took care of ssl_client2 and ssl_server2. But if we want to
be able to compile all programs, we need to modify each of them. This doesn't
seem useful, so instead let's just build the programs we need for testing -
this means only udp_proxy needs fixing in addition to what's already done.
This issue went unnoticed in the PR that introduced the new all.sh component,
because at that time the platform_memxxx() functions were not actually used in
the library (nor in programs), so the linker could live with
mbedtls_hardware_poll() not being defined, as it wasn't called anywhere. This
changed when we started using the new platform_memxxx() functions in the
library.
