Default flow assumes failure causes multiple issues with
compatibility tests when the return value is initialised
with error value in ssl_in_server_key_exchange_parse.
The function would need a significant change in structure for this.
The verification could be skipped in server, changed the default flow
so that the handshake status is ever updated if the verify
succeeds, and that is checked twice.
The MBEDTLS_ERR_SSL_WANT_READ and MBEDTLS_ERR_SSL_WANT_WRITE are
errors that can be ignored, so increase the hamming distance between
them and the non-ignorable errors and keep still some distance from
a success case. This mitigates an attack where single bit-flipping could
change a non-ignorable error to being an ignorable one.
Check that the encryption has been done for the outbut buffer.
This is to ensure that glitching out the encryption doesn't
result as a unecrypted buffer to be sent.
This is to enable hardening the security when changing
states in state machine so that the state cannot be changed by bit flipping.
The later commit changes the enumerations so that the states have large
hamming distance in between them to prevent this kind of attack.
Found by the IAR compiler.
While at it, make 'diff' non-volatile in uECC_check_curve_integrity(), as
there is no good reason to make it volatile, and making it volatile only
increases the code size and the burden of defining access ordering.
- MSVC doesn't like -1u
- We need to include platform.h for MBEDTLS_ERR_PLATFORM_FAULT_DETECTED - in
some configurations it was already included indirectly, but not in all
configurations, so better include it directly.
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.
We don't really need a secure hash for that, something like CRC32 would
probably be enough - but we have SHA-256 handy, not CRC32, so use that for the
sake of simplicity.
By semi-internal I mean functions that are only public because they're used in
more than once compilation unit in the library (for example in ecc.c and
ecc_dsa.c) but should not really be part of the public-facing API.
Same motivation as for the other parameters. This is the last one, making the
curve structure empty, so it's left with a dummy parameter for legal reasons.
-Add flow monitor, loop integrity check and variable doubling to
harden mbedtls_hmac_drbg_update_ret.
-Use longer hamming distance for nonce usage in hmac_drbg_reseed_core
-Return actual value instead of success in mbedtls_hmac_drbg_seed and
mbedtls_hmac_drbg_seed_buf
-Check illegal condition in hmac_drbg_reseed_core.
-Double buf/buf_len variables in mbedtls_hmac_drbg_random_with_add
-Add more hamming distance to MBEDTLS_HMAC_DRBG_PR_ON/OFF