This commit removes from the TinyCrypt header and source code files, the
configuration condition on MBEDTLS_USE_TINYCRYPT to include the file
contents.
This is to allow use of the library by the Factory Tool without enabling
MBEDTLS_USE_TINYCRYPT, and also removes a modification we've made to make the
code closer to the upstream TinyCrypt making it easier to maintain.
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).
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.)
Previously it was returning 0 or 1, so flipping a single bit in the return
value reversed its meaning. Now it's returning the diff itself.
This is safe because in the two places it's used (signature verification and
point validation), invalid values will have a large number of bits differing
from the expected value, so diff will have a large Hamming weight.
An alternative would be to return for example -!(diff == 0), but the
comparison itself is prone to attacks (glitching the appropriate flag in the
CPU flags register, or the conditional branch if the comparison uses one). So
we'd need to protect the comparison, and it's simpler to just skip it and
return diff itself.
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.
Currently functions that may return success or failure tend to do so by
returning 0 or 1. If an active physical attacker can flip a bit in memory or
registers at the right time, they may easily change a failure value into a
success value, with potentially catastrophic security consequences.
As typical attackers can only flip a few bits, an element of protection
against such attacks is to ensure a sufficient Hamming distance between
failure values and the success value. This commit introduces such values,
which will put to use in critical functions in future commits.
In addition to SUCCESS and FAILURE, a third value ATTACK_DETECTED is
introduced, which can be used later when suspicious-looking events are noticed
(static data changed when it shouldn't, double condition checking returning
inconsistent results, etc.).
Values are chosen so that Hamming distances are large, and that no value is
the complement of another, in order to avoid unwanted compiler optimisations.
Note: the error values used by Mbed TLS are already safe (assuming 32-bit
integers) as they are of the form -x with x in the range [1, 2^15) so their
Hamming distance with the success value (0) is at least 17, so it's hard for
an attacker to turn an error value into the success value (or vice-versa).
This avoids the need for each calling site to manually regularize the scalar
and randomize coordinates, which makes for simpler safe use and saves 50 bytes
of code size in the library.
-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
Do not reserve additionl space for mbedtls_aes_context if config
option AES_ONLY_128_BIT_KEY_LENGTH is used and PADLOCK_C is not used.
This reduces RAM usage by 96 bytes.
In baremetal configuration reserve space for 10 128-bit keys in order
to save 112 bytes of heap.
- Do not include MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH to full config
as it requires also MBEDTLS_CTR_DRBG_USE_128_BIT_KEY
- Update check_config to check availability of flags:
MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH
MBEDTLS_CTR_DRBG_USE_128_BIT_KEY
* 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().
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.
No effect for now, just declaring it here, implemented in subsequent commits.
The option requires MBEDTLS_USE_TINYCRYPT and is incompatible with
MBEDTLS_PK_RSA_ALT_SUPPORT and MBEDTLS_RSA_C.
Currently users (including the X.509 and SSL libraries) assume that if both PK
and RSA are enabled, then RSA is available through PK. If we allowed RSA to be
enabled together with PK_SINGLE_TYPE, we'd break that assumption. Going
through the code to find all place that rely on that assumption and fix them
would be cumbersome, and people who want PK_SINGLE_TYPE are unlikely to care
about RSA anyway, so let's just make them incompatible.
This is also consistent with what's done in the MD module: MD_SINGLE_HASH
requires that exactly one hash be enabled.
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.
* 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
...
* 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, the underlying digest's
context is embedded into mbedtls_md_context_t, which is
zeroized before the underlying digest's init() function
is called. For those digests where initialization is
zeroization, the init() call can therefore be omitted.
Similarly, when free()-ing an mbedtls_md_context_t, the
entire context is zeroized in the end, hence if the
underlying digest's free() function is zeroization,
it can be omitted.
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.
This commit modifies check_config.h to check that precisely one
hash is enabled if MBEDTLS_MD_SINGLE_HASH is set.
This is not only a reasonable expectation, it is also necessary,
because test suites assume that if a digest is enabled, it is also
accessible through the MD abstraction layer.
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.
In builds enabling only a single MD digest, we want to be able to
implement the MD info getter functions by returning compile-time
constants matching the fields of the MD info structures used so far.
To avoid information duplication hardening maintainability, this
commit introduces the possibility of providing the various aspects
of a particular digest implementation by defining macros
MBEDTLS_MD_INFO_DIGEST_FIELD (e.g. MBEDTLS_MD_INFO_SHA256_SIZE)
and to generate the corresponding mbedtls_md_info instance from
this set of macros, via the new macro MBEDTLS_MD_INFO().
This way, we'll be able to switch between MD info based builds
and single-digest builds without information duplication.
This commit continues the introduction of the MD digest implementation
abstraction layer given by `mbedtls_md_handle_t` by adding getter
functions returning the various properties of an implementation
(e.g. name, digest type, digest size). For the existing implementation,
these are just structure field accesses; however, in configurations
hardcoding the choice of a fixed digest algorithm, we'll be able to
implement them as inline functions returning compile-time constants.
As has been previously done for ciphersuites, this commit introduces
a zero-cost abstraction layer around the type
mbedtls_md_info const *
whose valid values represent implementations of message digest algorithms.
Access to a particular digest implementation can be requested by name or
digest ID through the API mbedtls_md_info_from_xxx(), which either returns
a valid implementation or NULL, representing failure.
This commit replaces such uses of `mbedtls_md_info const *` by an abstract
type `mbedtls_md_handle_t` whose valid values represent digest implementations,
and which has a designated invalid value MBEDTLS_MD_INVALID_HANDLE.
The purpose of this abstraction layer is to pave the way for builds which
support precisely one digest algorithm. In this case, mbedtls_md_handle_t
can be implemented as a two-valued type, with one value representing the
invalid handle, and the unique valid value representing the unique enabled
digest.
This commit changes the internal identifiers
MBEDTLS_SSL_MINOR_VERSION_XXX
in DTLS-only builds to match the version encoding used by the
DTLS standard, encoding DTLS 1.0 as 255 and DTLS 1.2 as DTLS 1.0.
Accordingly, the version comparison functions introduced in the
previous commit must be re-implemented, as older version have
_larger_ identifiers now.
Further, since we identify DTLS 1.0 as MBEDTLS_SSL_MINOR_VERSION_2
and DTLS 1.2 as MBEDTLS_SSL_MINOR_VERSION_3, what remains is to
define MBEDTLS_SSL_MINOR_VERSION_{0|1}. While these don't have any
meaning meaning in DTLS, they still need to be set and obey the
ordering in the sense that the version comparison functions '<='
should attest that
MBEDTLS_SSL_MINOR_VERSION_i '<=' MBEDTLS_SSL_MINOR_VERSION_j
for i <= j. Since '<=' is actually >= and the wire format value
for DTLS 1.0 == MBEDTLS_SSL_MINOR_VERSION_2 is the 255, this
forces us to use values beyond 255, and hence to extend the
storage type for minor versions from uint8_t to uint16_t.
Moved some functions under defined to get rid of compiler warnings.
Functions moved under defines:
- mbedtls_x509_get_alg
- mbedtls_x509_get_alg_null
- mbedtls_x509_get_time
- mbedtls_x509_get_ext
- mbedtls_x509_sig_alg_gets
- mbedtls_x509_key_size_helper
Left one function (mbedtls_x509_write_names) as non static as it increased code size.
Previously, this wasn't necessary because ecdh.h was included
through ssl.h, but now that this is no longer the case (because
ssl.h doesn't use ECDH), we have to include it explicitly.
Previously, ecp.h was included only if MBEDTLS_ECDH_C was set,
which broke the build in configurations using ECDSA, but not ECDH.
An example of such a config is configs/config-thread.h, which
uses ECJPAKE exclusively.
Moreover, the inclusion of ecdh.h isn't needed, because the header
only uses constants defined in the ECP module.
There is no apparent direct dependency, and the indirect dependency
through the RSA and legacy ECP modules is already encoded in the
chain
MBEDTLS_X509_USE_C
-> MBEDTLS_PK_PARSE_C
-> MBEDTLS_PK_C
-> MBEDTLS_RSA_C || MBEDTLS_ECP_C
-> MBEDTLS_BIGNUM_C
which will be modified to
MBEDTLS_X509_USE_C
-> MBEDTLS_PK_PARSE_C
-> MBEDTLS_PK_C
-> MBEDTLS_RSA_C || MBEDTLS_ECP_C || MBEDTLS_USE_TINYCRYPT
in which case MBEDTLS_BIGNUM_C is not needed for MBEDTLS_X509_USE_C
if only MBEDTLS_USE_TINYCRYPT is set, but not MBEDTLS_RSA_C or
MBEDTLS_ECP_C.
asn1.h uses uint8_t which is defined in stdint.h.
This wasn't caught earlier by the luck that whenever asn1.h
was included, another header was included earlier that did in
turn include stdint.h.
Remark: Including ecp.h is actually redundant because it's
also included from ecdh.h. However, it's good practice to
explicitly include header files that are being used directly,
and ssl.h does use MBEDTLS_ECP_MAX_BYTES which is defined in ecp.h.
ssl.h contains a dummy union of fields each large enough to
hold the PMS for a particular ciphersuite. In particular, for
pure-ECDH ciphersuites, it contains a field large enough to
hold the ECDH shared secret in any of the enabled curves.
So far, this upper bound was unconditionally chosen to be
MBEDTLS_ECP_MAX_BYTES from the ECP module.
With the introduction of TinyCrypt as an alternative implementation
for ECDH, we need to
- guard the use of MBEDTLS_ECP_MAX_BYTES because MBEDTLS_ECP_C
is no longer implied by the surrounding MBEDTLS_KEY_EXCHANGE_XXX
guards
- add another field which contains the maximum length of shared
ECDH secrets for curves supported by TinyCrypt.
mbedtls/ecp.h defines constants
MBEDTLS_ECP_PF_UNCOMPRESSED
MBEDTLS_ECP_PF_COMPRESSED
MBEDTLS_ECP_TLS_NAMED_CURVE
which regard the encoding of elliptic curves and curve point formats in TLS.
As such, they should be defined in the SSL namespace. Asides, this will help
replacing the legacy ECC crypto by alternative ECC implementations.
The PK-type MBEDTLS_PK_ECDSA isn't really used by the library.
Especially, when parsing a generic EC key, a PK context of type
MBEDTLS_PK_ECKEY will be requested. Hence, to drop in TinyCrypt
for the legacy-ECC implementation, the PK type that TinyCrypt
implements must be MBEDTLS_PK_ECKEY.
TinyCrypt should be used as a replacement of legacy ECC. In particular,
there shouldn't be any use of identifiers from the legacy ECC module.
So far, there's the configuration option
MBEDTLS_SSL_CONF_SINGLE_EC_GRP_ID
that's relevant if MBEDTLS_SSL_CONF_SINGLE_CURVE is set, and which in
this case must resolve to an identifier of type mbedtls_ecp_group_id
indicating which single curve to enable.
With the introduction of TinyCrypt, we must either change the type
of this option to mbedtls_uecc_group_id, or introduce a separate
compilation option.
In order to avoid type confusion, this commit follows tha latter
approach, introducing the configuration option
MBEDTLS_SSL_CONF_SINGLE_UECC_GRP_ID
that indicatesthe TinyCrypt group identifier of the single curve
to use (must be Secp256r1) if MBEDTLS_SSL_CONF_SINGLE_CURVE
and MBEDTLS_USE_TINYCRYPT are set.
