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.
When verifying an X.509 certificate, the current verification logic
maintains an instance of the internal mbedtls_x509_crt_verify_chain
structure representing the state of the verification process. This
instance references the list of certificates that comprise the chain
built so far together with their verification flags. This information
must be stored during verification because it's being passed to the
verification callback at the end of verification - if the user has
specified those.
If the user hasn't specified a verification callback, it is not
necessary to maintain the list of CRTs, and it is also not necessary
to maintain verification flags for each CRT individually, as they're
merged at the end of the verification process.
To allow a readable simplification of the code in case no verification
callbacks are used, this commit introduces a zero-cost abstraction layer
for the functionality that's required from the verification chain structure:
- init/reset
- add a new CRT to the chain
- get pointer to current CRT flags
- add flags to EE certificate
- get current chain length
- trigger callbacks and get final (merged) flags
This gives flexibility for re-implementing the verification chain
structure, e.g. in the case where no verification callbacks are
provided, and there's hence no need to store CRTs and flags
individually. This will be done in a later commit.
When MBEDTLS_MD_SINGLE_HASH is set, both the underlying digest context
and the HMAC data are embedded into the mbedtls_md_context; otherwise,
they're dynamically allocated and referenced from mbedtls_md_context.
When the HMAC data is embedded in mbedtls_md_context, it's unnecessary
to check whether mbedtls_md_context::hmac_ctx is NULL, because that's
never the case in defined behaviour, but the check has kept for
uniformity so far. However, contrary to the expectation that compilers
would silently remove this check as always false, ARMC6 complains about
it, breaking some tests in all.sh.
This commit fixes this by guarding checks for
mbedtls_md_context::hmac_ctx == NULL
by !MBEDTLS_MD_SINGLE_HASH.
Recall that in the default configuration, Mbed TLS provides access
digest implementations through two layers of indirection:
1) Call of MD API (e.g. mbedtls_md_update())
2) Call of function pointer from MD info structure
3) Actual digest implementation (e.g. mbedtls_sha256_update()).
Ideally, if only a single digest is enabled - say SHA-256 - then calling
mbedtls_md_update() should _directly_ jump to mbedtls_sha256_update(),
with both layers of indirection removed. So far, however, setting
MBEDTLS_MD_SINGLE_HASH will only remove the second - function pointer -
layer of indirection, while keeping the non-inlined stub implementations
of e.g. mbedtls_md_update() around.
This commit is a step towards allowing to define implementations of
the MD API as `static inline` in case we know that they are so small
that they should be defined in md.h and not in md.c.
In a nutshell, the approach is as follows: For an MD API function
mbedtls_md_xxx() that should be inlin-able, introduce its implementation
as a `static inline` wrapper `mbedtls_md_xxx_internal()` in md.h,
and then define mbedtls_md_xxx() either in md.h or in md.c, by just
calling mbedtls_md_xxx_internal().
Moving the implementations of those MD API functions that should be
inlinable to md.h requires the presence of both the MD info struct
and all specific digest wrapper functions in md.h, and this is what
this commit ensures, by moving them from md.c into a new internal
header file md_internal.h. Implementing the aforementioned wrappers for
those MD API that should be inlinable is left for subsequent commits.
ARMC5 appears to use the heuristic that as soon as a function's address
is taken, the function can no longer be removed from the resulting object
file (which is not necessarily true if all uses of the functions address
can be inlined).
Circumvent this lack of optimization by not returning function pointers.
This commit introduces the configuration option
MBEDTLS_MD_SINGLE_HASH
which can be used to hardcode support for a single digest algorithm
at compile-time, at the benefit of reduced code-size.
To use, it needs to be defined to evaluate to a macro of the form
MBEDTLS_MD_INFO_{DIGEST}, and macros MBEDTLS_MD_INFO_{DIGEST}_FIELD
must be defined, giving rise to the various aspects (name, type,
size, ...) of the chosen digest algorithm. MBEDTLS_MD_INFO_SHA256
provides an example, but other algorithms can be added if needed.
At the moment, the effect of using MBEDTLS_MD_SINGLE_HASH is that
the implementation of the MD API (e.g. mbedtls_md_update()) need no
longer to through the abstraction of the mbedtls_md_info structures
by calling their corresponding function pointers fields (akin to
virtual functions in C++), but the directly call the corresponding
core digest function (such as mbedtls_sha256_update()).
Therefore, MBEDTLS_MD_SINGLE_HASH so far removes the second layer
of indirection in the chain
User calls MD API -> MD API calls underlying digest impl'n
-> Core digest impl'n does the actual work,
but the first indirection remains, as the MD API remains untouched
and cannot yet be inlined. Studying to what extend inlining the
shortened MD API implementations would lead to further code-savings
is left for a later commit.
