In the case of *ret we might need to preserve a 0 value throughout the
loop and therefore we need an extra condition to protect it from being
overwritten.
The value of done is always 1 after *ret has been set and does not need
to be protected from overwriting. Therefore in this case the extra
condition can be removed.
The code relied on the assumptions that CHAR_BIT is 8 and that unsigned
does not have padding bits.
In the Bignum module we already assume that the sign of an MPI is either
-1 or 1. Using this, we eliminate the above mentioned dependency.
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.
Multiplication is known to have measurable timing variations based on
the operands. For example it typically is much faster if one of the
operands is zero. Remove them from constant time code.
The blinding applied to the scalar before modular inversion is
inadequate. Bignum is not constant time/constant trace, side channel
attacks can retrieve the blinded value, factor it (it is smaller than
RSA keys and not guaranteed to have only large prime factors). Then the
key can be recovered by brute force.
Reducing the blinded value makes factoring useless because the adversary
can only recover pk*t+z*N instead of pk*t.
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.
Move the definitions of mbedtls_ctr_drbg_seed_entropy_len() and
mbedtls_ctr_drbg_seed() to after they are used. This makes the code
easier to read and to maintain.
mbedtls_hmac_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_hmac_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().
ssl_decompress_buf() was operating on data from the ssl context, but called at
a point where this data is actually in the rec structure. Call it later so
that the data is back to the ssl structure.
Signed-off-by: Simon Butcher <simon.butcher@arm.com>
There is a 50% performance drop in the SCA_CM enabled encrypt and
decrypt functions. Therefore use the older version of encrypt/decypt
functions when SCA_CM is disabled.
-Do not reuse any part of randomized number, use separate byte for
each purpose.
-Combine some separate loops together to get rid of gap between them
-Extend usage of flow_control
* upstream/pr/2945:
Rename macro MBEDTLS_MAX_RAND_DELAY
Update signature of mbedtls_platform_random_delay
Replace mbedtls_platform_enforce_volatile_reads 2
Replace mbedtls_platform_enforce_volatile_reads
Add more variation to random delay countermeasure
Add random delay to enforce_volatile_reads
Update comments of mbedtls_platform_random_delay
Follow Mbed TLS coding style
Add random delay function to platform_utils
When reading the input, buffer will be initialised with random data
and the reading will start from a random offset. When writing the data,
the output will be initialised with random data and the writing will start
from a random offset.
When reading the input, the buffer will be initialised with random data
and the reading will start from a random offset. When writing the data,
the output will be initialised with random data and the writing will
start from a random offset.
Add more variation to the random delay function by xor:ing two
variables. It is not enough to increment just a counter to create a
delay as it will be visible as uniform delay that can be easily
removed from the trace by analysis.
The flag is used for tracking if the premaster has
been succesfully generated. Note that when resuming
a session, the flag should not be used when trying to
notice if all the key generation/derivation has been done.
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.
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.
-Replace usage of rand() with mbedtls_platform_random_in_range()
-Prevent for-ever loop by hardcoding SCA countermeasure position in
case of used random function is always returning constant number.
-Use separate control bytes for start and final round to get them
randomized separately.
-Remove struct name.
-Fix comments and follow Mbed TLS coding style.
SCA CM implementation caused AES performance drop. For example
AES-CCM-128 calculation speed was dropped from 240 KB/s to 111 KB/s.
(-54%), Similarily AES-CBC-128 calculation speed was dropped from
536 KB/s to 237 KB/s (-56%).
Use functions instead of macros to reduce code indirections and
therefore increase performance. Now the performance is 163 KB/s for
AES-CCM-128 (-32%) and 348 KB/s for AES-CBC-128 (-35%).
When SCA countermeasures are activated the performance is as follows:
122 KB/s for AES-CCM-128 (-49%) and 258 KB/s for AES-CBC-128 (-52%)
compared to the original AES implementation.
Use control bytes to instruct AES calculation rounds. Each
calculation round has a control byte that indicates what data
(real/fake) is used and if any offset is required for AES data
positions.
First and last AES calculation round are calculated with SCA CM data
included. The calculation order is randomized by the control bytes.
Calculations between the first and last rounds contains 3 SCA CMs
in randomized positions.
- 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
- 3 additional dummy AES rounds calculated with random data for each
AES encryption/decryption
- additional rounds can be occur in any point in sequence of rounds
- 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.
-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
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.
In the previous version, it was enough for the attacker to glitch the
top-level 'if' to skip the entire block. We want two independent blocks here,
so that an attacker can only succeed with two successive glitches.
Before this commit, if a certificate only had one issue (for example, if the
"untrusted" bit was the only set in flags), an attacker that could flip this
single bit between the moment it's set and the moment flags are checked before
returning from mbedtls_x509_crt_verify() could make the entire verification
routine appear to succeed (return 0 with no bit set in flags).
Avoid that by making sure that flags always has either 0 or at least 9 bits
set during the execution of the function. However, to preserve the API, clear
the 8 extra bits before returning. This doesn't open the door to other
attacks, as fortunately the API already had redundancy: either both flags and
the return value are 0, or flags has bits set and the return value is non-zero
with at least 16 bits set (assuming 32-bit 2-complement ints).
If signature_is_good is 0 (invalid) of 1 (valid), then it's all too easy for
an active physical attacker to turn invalid into valid by flipping a single
bit in RAM, on the bus or in a CPU register.
Use a special value to represent "valid" that can't easily be reached by
flipping a few bits.
x509_crt_check_signature() directly returns the return value of
pk_verify_xxx() without looking at it, so nothing to do here. But its caller
compares the value to 0, which ought to be double-checked.
Inspection of the generated assembly showed that before this commit, armcc 5
was optimizing away the successive reads to the volatile local variable that's
used for double-checks. Inspection also reveals that inserting a call to an
external function is enough to prevent it from doing that.
The tested versions of ARM-GCC, Clang and Armcc 6 (aka armclang) all keep the
double read, with our without a call to an external function in the middle.
The inserted function can also be changed to insert a random delay if
desired in the future, as it is appropriately places between the reads.
This can be used by Mbed TLS functions in any module to signal that a fault
attack is likely happening, so this can be appropriately handled by the
application (report, fall back to safer mode or even halt, etc.)
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.
This is a temporary work-around for an integration issue.
A future task will re-integrate randomness into these functions are their
entire point is to be randomized; this is really just temporary.
Record checking fails if mbedtls_ssl_check_record() is called with
external buffer. Received record sequence number is available in the
incoming record but it is not available in the ssl contexts `in_ctr`-
variable that is used when decoding the sequence number.
To fix the problem, temporarily update ssl context `in_ctr` to
point to the received record header and restore value later.
-Add config option for AES encyption only to config.h. Feature is
disabled by default.
-Enable AES encrypt only feature in baremetal.h configuration
-Remove AES encypt only feature from full config
- out_ctr is public because it's transmited over the wire in DTLS (and in TLS
it can be inferred by a passive network attacker just by counting records).
- handshake mask is not a secret because it can be inferred by a passive
network attacker just logging record sequence number seen so far.
This commits reverts to plain memset() for cases like:
some_type foo;
memset( &foo, 0, sizeof( foo ) );
(Sometimes there is code between declaration in memset(), but it doesn't
matter as long as it doesn't touch foo.)
The reasoning is the same as in the previous commit: the stack shouldn't
contain sensitive data as we carefully wipe it after use.
We call xxx_init() on a structure when it has been freshly allocated (on the
stack or heap).
At this point it contains random-looking data none of which should be
sensitive, as all sensitive data is wiped using mbedtls_platform_zeroize()
when we're done using it and the memory area is going to be reclaimed (by
exiting the function or free()ing the buffer).
Add it in all files that use mbedtls_plaform_memset() but didn't already
include platfom_util.h.
In some configurations it just happened to work, either because it was
included indirectly or because the part of the code that used that function
was disabled, but it some configurations it broke, so let's fix it properly.
Steps:
1. sed -i 's/\bmemset(\([^)]\)/mbedtls_platform_memset(\1/g' library/*.c tinycrypt/*.c include/mbedtls/*.h scripts/data_files/*.fmt
2. Manually edit library/platform_util.c to revert to memset() in the
implementations of mbedtls_platform_memset() and mbedtls_platform_memcpy()
3. egrep -n '\<memset\>' library/*.c include/mbedtls/*.h tinycrypt/*.c
The remaining occurrences are in three categories:
a. From point 2 above.
b. In comments.
c. In the initialisation of memset_func, to be changed in a future commit.
* mbedtls-2.16: (25 commits)
Fix compilation error
Add const to variable
Fix endianity issue when reading uint32
Increase test suite timeout
Reduce stack usage of test_suite_pkcs1_v15
Reduce stack usage of test_suite_pkcs1_v21
Reduce stack usage of test_suite_rsa
Reduce stack usage of test_suite_pk
Enable MBEDTLS_MEMORY_DEBUG in memory buffer alloc test in all.sh
Remove unnecessary memory buffer alloc and memory backtrace unsets
Disable DTLS proxy tests for MEMORY_BUFFER_ALLOC test
all.sh: restructure memory allocator tests
Add missing dependency in memory buffer alloc set in all.sh
Don't set MBEDTLS_MEMORY_DEBUG through `scripts/config.pl full`
Add cfg dep MBEDTLS_MEMORY_DEBUG->MBEDTLS_MEMORY_BUFFER_ALLOC_C
Add all.sh run with full config and ASan enabled
Add all.sh run with MBEDTLS_MEMORY_BUFFER_ALLOC_C enabled
Update documentation of exceptions for `config.pl full`
Adapt all.sh to removal of buffer allocator from full config
Disable memory buffer allocator in full config
...
Use MBEDTLS_ENTROPY_HARDWARE_ALT instead of a new global RNG
flag. When this flag is enabled, the platform provides the RNG.
When running unit tests, rnd_std_rand should be used by overriding
the mbedtls_hardware_poll.
As replacements of standard library functions, they should have the same
prototype, including return type.
While it doesn't usually matter when used directly, it does when the address
of the function is taken, as done with memset_func, used for implementing
mbedtls_platform_zeroize().
Warnings are treated as errors in Mbed TLS test. An error
"ssl_parse_client_hello_v2’ defined but not used" can occur in some
specific configurations and therefore tests will break.
Use similar flags for static function "ssl_parse_client_hello_v2" as
what is used when calling the function to prevent the compilation
warning/error.
-Add comments to Makefiles about test env auto-detection
-Fix indentation
-Remove parent folder from include dirs
-Do not use environment variable for defining config file because
env variable usage is not fully implemented
-Revert changes to config.pl
This makes a mbedtls_pk_context memory-wise equivalent to a
mbedtls_uecc_keypair and removes a dynamic allocation, making the PK layer
zero-cost in terms of memory when PK_SINGLE_TYPE is enabled.
In very reduced configurations, we don't want the overhead of maintaining a
bool just to remember if the context is valid and checking that bit at every
point of entry.
Note: so far this validity bit also served as a proxy to ensure that pk_ctx
was valid (currently this is a pointer to a dynamically-allocated buffer). In
the next series of commits, this will be changed to a statically-allocated
buffer, so there will be no question about its validity.
In the end (after this commit and the next series), a pk_context_t will be
(memory-wise) just the same as a mbedtls_uecc_keypair when SINGLE_TYPE is
enabled - meaning the PK layer will have zero memory overhead in that case.
So far, with MBEDTLS_SSL_KEEP_PEER_CERTIFICATE disabled, the SSL module relied
on a undocumented feature of the PK module: that you can distinguish between
contexts that have been setup and context that haven't. This feature is going
to go away in the case of PK_SINGLE_TYPE, as we'll soon (as in: the next
commit does that) no longer be storing the (now two-valued) pk_info member.
Note even with this change, we could still distinguish if the context has been
set up by look if pk_ctx is NULL or not, but this is also going away in the
near future (a few more commits down the road), so not a good option either.
This is the first in a series of commit aimed at removing the pk_info
structures when we're building with MBEDTLS_PK_SINGLE_TYPE enabled.
Introducing this abstraction allows us to later make it a two-valued type
(valid, invalid) instead, which is much lighter.
For optional functions, we introduce an extra macro to tell if the function is
omitted. As the C preprocessor doesn't directly support comparing strings,
testing if the _FUNC macro is defined to NULL isn't obvious. One could
probably play tricks to avoid the need for _OMIT macros, but the small amount
of (entirely local) duplication here is probably a lesser evil than extra
preprocessor complexity.
We want public functions to resolve to the internal wrappers at compile-time.
For this we need the wrappers to be visible from where the public functions
are defined. A simple declaration is not enough if we want the compiler to be
able to inline the wrapper and eliminate function overhead.
This commit just copies verbatim the contents of pk_wrap.c into pk.c. The next
commit will clean up the result (redundant includes etc.).
This achieves two related goals:
1. Those members are now only accessed via the accessor function (except in
code paths that we don't care about: those guarded by
MBEDTLS_PK_RSA_ALT_SUPPORT or MBEDTLS_ECP_RESTARTABLE)
2. When we turn on compile-time dispatch, we don't obviously don't want to
keep a runtime NULL check.
For debug this requires changing the signature or the accessor function to
return int; this is done without changing the signature of the accessed
function.
1. Mark an RSA-alt-specific code path as such.
2. Move NULL check for wrapper function closer to the use of that function.
Those are in preparation of the next commit.
This is the first commit in a series aiming at implementing optional
compile-time dispatch when a single PK type is hardcoded. At the end of this
series, the functions introduced here will directly resolve to the correct
function at compile-time when this (to be created) option is enabled.
* baremetal: (78 commits)
Review corrections 6
Review corrections 5
Minor changes to tinycrypt README
Typos in the tinycrypt README
Addition of copyright statements to tinycrypt files
Add LICENSE and README for tinycrypt
Add SPDX lines to each imported TinyCrypt file
Review corrections 4
Review corrections 3
Review corrections 2
Review corrections
Update signature of BE conversion functions
Use function for 16/24/32-bit BE conversion
x509.c: Minor readability improvement
x509_crt.c: Indicate guarding condition in #else branch
X.509: Don't remove verify callback by default
Fix Doxygen warnings regarding removed verify cb+ctx parameters
ECC restart: Use optional verification mode in bad signature test
Re-implement verify chain if vrfy cbs are disabled
Add zero-cost abstraction layer for CRT verification chain
...
On the mbedtls-2.16 side, there was a change in commit
a7cfdad82e (PR r#503) in order to write
fixed-length private keys. It added a new helper function
pk_write_ec_private() for that.
On the baremetal side, there were changes in order to add a tinycrypt-based
implementation. It added a new helper function pk_write_ec_privkey() with two
implementations (with or without tinycrypt).
This commit keeps the function pk_write_ec_privkey() but changes its
implementation in the non-tinycrypt configuration in order to match the
implementation of pk_write_ec_private(), which is in turn removed it was only
used in that place.
The tinycrypt version of pk_write_ec_private() was already writing
constant-length private keys, so there is nothing to change here.
* mbedtls-2.16: (28 commits)
Bump version to Mbed TLS 2.16.3
Changelog entry
Check for zero length and NULL buffer pointer
ssl-opt.sh: wait for proxy to start before running the script further
Fix uninitialized variable in x509_crt
HMAC DRBG: Split entropy-gathering requests to reduce request sizes
Fix the license header of hkdf
Add a change log entry
Add a test for mlaformed ECJPAKE context
Fix handling of md failure
Add a test for signing content with a long ECDSA key
Add documentation notes about the required size of the signature buffers
Add missing MBEDTLS_ECP_C dependencies in check_config.h
Change size of preallocated buffer for pk_sign() calls
Adapt ChangeLog
Fix mpi_bigendian_to_host() on bigendian systems
Add ChangeLog entry for new function
Add ChangeLog entry
Correct deterministic ECDSA behavior
Add warning for alternative ECDSA implementations
...
- Try to follow english grammar in function documentation
- Fix too long line
- Remove additional brackets
- Follow mbedtls coding style in for-statement
-Fix MSVC compiler warnings about size_t to uint32_t conversions by
updating GET/PUT functions signature to use size_t.
-Add type casts to functions calling GET/PUT conversions
-Remove additional space after return statement
This commit re-implements the previously introduced internal
verification chain API in the case where verification callbacks
are disabled. In this situation, it is not necessary to maintain
the list of individual certificates and flags comprising the
verification chain - instead, it suffices to just keep track
of the length and the total (=merged) flags.
