Fix a stack buffer overflow with mbedtls_net_poll() and
mbedtls_net_recv_timeout() when given a file descriptor that is beyond
FD_SETSIZE. The bug was due to not checking that the file descriptor
is within the range of an fd_set object.
Fix#4169
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Although the library documentation does not guarantee that calling
mbedtls_entropy_free() twice works, it's a plausible assumption and it's
natural to write code that frees an object twice. While this is uncommon for
an entropy context, which is usually a global variable, it came up in our
own unit tests (random_twice tests in test_suite_random).
Announce this in the same changelog entry as for RSA because it's the same
bug in the two modules.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
mbedtls_rsa_gen_key() was not freeing the RSA object, and specifically
not freeing the mutex, in some error cases.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
When MBEDTLS_THREADING_C is enabled, RSA code protects the use of the
key with a mutex. mbedtls_rsa_free() frees this mutex by calling
mbedtls_mutex_free(). This does not match the usage of
mbedtls_mutex_free(), which in general can only be done once.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
mbedtls_hmac_drbg_free() left a mutex in the initialized state. This
caused a resource leak on platforms where mbedtls_mutex_init()
allocates resources.
To fix this, mbedtls_hmac_drbg_free() no longer reinitializes the
mutex. To preserve the property that mbedtls_hmac_drbg_free() leaves
the object in an initialized state, which is generally true throughout
the library except regarding mutex objects on some platforms, no
longer initialize the mutex in mbedtls_hmac_drbg_init(). Since the
mutex is only used after seeding, and seeding is only permitted once,
call mbedtls_mutex_init() as part of the seeding process.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
mbedtls_ctr_drbg_free() left a mutex in the initialized state. This
caused a resource leak on platforms where mbedtls_mutex_init()
allocates resources.
To fix this, mbedtls_ctr_drbg_free() no longer reinitializes the
mutex. To preserve the property that mbedtls_ctr_drbg_free() leaves
the object in an initialized state, which is generally true throughout
the library except regarding mutex objects on some platforms, no
longer initialize the mutex in mbedtls_ctr_drbg_init(). Since the
mutex is only used after seeding, and seeding is only permitted once,
call mbedtls_mutex_init() as part of the seeding process.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Document the usage inside the library, and relate it with how it's
additionally used in the test code.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Change psa_ecdsa_sign/verify signature to that of
a sign/verify_hash driver entry point before to
move them to the psa_crypto_ecp.c ECP specific file.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change psa_rsa_sign/verify signature to that of
a sign/verify_hash driver entry point before to
move them to the psa_crypto_rsa.c RSA specific file.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change psa_driver_wrapper_sign/verify_hash signature
to that of a sign/verify_hash driver entry point.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Wrap sign/verify_hash software implementation into
psa_sign/verify_hash_internal() functions whose
signature is that of a sign/verify_hash driver
entry point.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change the psa_crypto use of the CHACHA20 cipher to also use the new
MBEDTLS_PSA_BUILTIN_KE_TYPE_CHACHA20.
Signed-off-by: David Brown <david.brown@linaro.org>
Change a few conditionals in the psa library to be based on the
MBEDTLS_PSA_BUILTIN_KEY_TYPE_DES instead of the WANT macros. Future
additions of HW acceleration will need to be mindful of these
definitions if any of this code is needed in those instances.
Signed-off-by: David Brown <david.brown@linaro.org>
When converting definitions to use the new PSA defines, one erroneously
was conditionalized on the WANT macro instead of on the BUILTIN macro.
Signed-off-by: David Brown <david.brown@linaro.org>
There are a few instances of MBEDTLS_*_C (specifically for DES) in
psa_crypto.c. Change to the PSA_WANT_KEY_TYPE_DES macros to reflect the
new PSA crypto config.
Signed-off-by: David Brown <david.brown@linaro.org>
This file will always be used with the PSA configurations, so use the
MBEDTLS_PSA_BUILTIN... definitions for the symmetric cyphers.
Signed-off-by: David Brown <david.brown@linaro.org>
There was some intentional duplication between
library/psa_crypto_random_impl.h and include/mbedtls/psa_util.h, with
the intent that the compiler would complain if one file was edited in
a way that's incompatible with the other file. However, the two files
were never included together, and in fact could not be included
together because some definitions can't be duplicated (type, static
variable).
Now library/psa_crypto_random_impl.h includes
include/mbedtls/psa_util.h, so the compiler will check what it can.
There is less redundancy since it isn't always possible to declare
something twice (specifically, types can't be declared).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Use PSA_EXPORT_KEY_OUTPUT_SIZE macro to compute the
size of the buffer to contain the generated key
instead of computing it alongside the key type and
size validation.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
When generating transparent keys, we need to be able
to compute the size of the key buffer whether the
key is generated by the Mbed TLS library or by an
accelerator. Thus, change the RSA/ECP
MBEDTLS_PSA_BUILTIN_... compilation guards with
their PSA_WANT_... counterparts.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change psa_generate_key_internal() signature to
that of a PSA driver generate_key entry point.
That way, this function can be called by the
driver wrapper when a software fallback is
necessary.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Rename and export get_key_buffer_size to be able to call
it from psa_crypto.c to compute the size of buffers to
contain keys generated by an opaque driver without
storage.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Restrict the scope of get_expected_key_size to
generation of key in a secure element or
cryptoprocessor without storage.
For transparent driver, the key buffer size calculation is
for the time being moved to psa_driver_wrapper_generate_key
and will eventually be done by psa_get_key_buffer_size.
Rename the function to get_key_buffer_size to
align its naming with that of psa_get_key_buffer_size.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
In case of a secure element or cryptoprocessor with
storage, when generating a key, the key material is
not exported from the secure element or cryptoprocessor
thus there is no need to allocate a buffer in that case.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Preparatory commit to eventually change
psa_generate_key_internal() signature to that of
a PSA driver generate_key entry point.
To be able to change the signature, the buffer to
store the generated key has to be allocated before
the call to psa_generate_key_internal().
This commit moves the allocation and clean-up in
case of error of the buffer to store the generated
key from psa_generate_key_internal() to
psa_generate_key().
This has the nice benefit of factorizing the key
buffer allocation and clean-up.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Preparatory commit to eventually change
psa_generate_key_internal() signature to that of
a PSA driver generate_key entry point.
To be able to change the signature, the buffer to
store the key has to be allocated before the call
to psa_generate_key_internal() thus its size has
to be calculed beforehand as well.
This is the purpose of this commit: to move the
computation of the key size in bytes out of
psa_generate_key_internal().
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Small improvements to psa_generate_key_internal()
implementation:
. declare only once the status local variable and
initialize it to PSA_ERROR_CORRUPTION_DETECTED
to improve robustness against FI attacks.
. remove an unnecessary assignment.
. use type local variable instead of its global
variable equivalent.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
This brings them in line with PSA Crypto API 1.0.0
PSA_ALG_AEAD_WITH_DEFAULT_TAG_LENGTH -> PSA_ALG_AEAD_WITH_DEFAULT_LENGTH_TAG
PSA_ALG_AEAD_WITH_TAG_LENGTH -> PSA_ALG_AEAD_WITH_SHORTENED_TAG
Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
In psa_generate_derived_key_internal() an error case was returning
directly rather than jumping to the exit label, which meant that an
allocated buffer would not be free'd.
Found via coverity.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Make sure MBEDTLS_PSA_CRYPTO_CLIENT is defined
when MBEDTLS_PSA_CRYPTO_C is defined and guard
PSA client code only with MBEDTLS_PSA_CRYPTO_CLIENT.
The definition of MBEDTLS_PSA_CRYPTO_CLIENT is done
in crypto_types.h before the definition of
psa_key_attributes_t. That way as PSA crypto client
code is related to key attributes we can be quite
confident that MBEDTLS_PSA_CRYPTO_CLIENT will be
defined when needed.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Attempting to create an ECC key with a curve specification that is not
valid can plausibly fail with PSA_ERROR_INVALID_ARGUMENT ("this is not
a curve specification at all") or PSA_ERROR_NOT_SUPPORTED ("this may
be a curve specification, but not one I support"). The choice of error
is somewhat subjective.
Before this commit, due to happenstance in the implementation, an
attempt to use a curve that is declared in the PSA API but not
implemented in Mbed TLS returned PSA_ERROR_INVALID_ARGUMENT, whereas
an attempt to use a curve that Mbed TLS supports but for which support
was disabled at compile-time returned PSA_ERROR_NOT_SUPPORTED. This
inconsistency made it difficult to write negative tests that could
work whether the curve is implemented via Mbed TLS code or via a
driver.
After this commit, any attempt to use parameters that are not
recognized fails with NOT_SUPPORTED, whether a curve with the
specified size might plausibly exist or not, because "might plausibly
exist" is not something Mbed TLS can determine.
To keep returning INVALID_ARGUMENT when importing an ECC key with an
explicit "bits" attribute that is inconsistent with the size of the
key material, this commit changes the way mbedtls_ecc_group_of_psa()
works: it now works on a size in bits rather than bytes, with an extra
flag indicating whether the bit-size must be exact or not.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Add ECP/RSA transparent test driver import_key
entry point and use it in the transparent test
driver entry supporting both ECP and RSA.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Move key buffer allocation from psa_import_key_into_slot()
function up to the two functions calling it.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Rename ECP key import function before to move
it to psa_crypto_ecp.c to adapt to the naming
of exported functions in psa_crypto_ecp.c.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Rename psa_import_rsa_key to mbedtls_psa_rsa_import_key to
align its name with the naming conventions of exported
functions in psa_crypto_rsa.c.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Add ECP/RSA transparent test driver export_public_key
entry point and use it in the transparent test driver
supporting both ECP and RSA.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change psa_export_(public_)key_internal signature to
the signature of an export_(public_)key driver entry
point.
This is a preparatory commit to be able to call the
software implementations as a driver.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Disentangle public export and export code to be
able to move the call to export and public
export operations to the driver wrapper.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Align naming of export internal functions with the way
other psa_crypto.c internal functions are named, ending
with _internal.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Rename psa_export_rsa/ecp_key to
mbedtls_psa_rsa/ecp_export_key before to move them to
RSA/ECP specific PSA crypto C files.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Rename psa_load_rsa/ecp_representation to
mbedtls_psa_rsa/ecp_load_representation
before to move them in their RSA/ECP
specific PSA crypto C modules.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change psa_driver_wrapper_export_public_key() signature
to the signature of an export_public_key driver entry point.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
In the course of the development of the PSA unified
driver interface, the validate_key entry point for
opaque drivers has been removed and replaced by an
import_key entry point. This commit takes into account
this change of specification.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
In key slots containing the description of a key of a
dynamically registered Secure Element (SE), store the
key slot number in a key context as defined in the
PSA driver interface for opaque drivers.
That way transparent key data and slot numbers are
, in a key slot, both stored in a dynamically allocated
buffer. The `data` union in structures of type
psa_key_slot_t to distinguish between the storage of
transparent key data and slot numbers is consequently
not necessary anymore and thus removed.
This alignement of some part of the code dedicated to
dynamically registered SE with the PSA driver interface
specification is done to ease the support of both
dynamically registered and statically defined secure
elements.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
ext_len is unsigned and the loop over the extensions checks
while( ext_len != 0 )
{
if ( ext_len < 4 ) {
so additional checks are redundant.
Signed-off-by: Glenn Strauss <gstrauss@gluelogic.com>
Fix a buffer overflow in mbedtls_mpi_sub_abs() when calculating
|A| - |B| where |B| is larger than |A| and has more limbs (so the
function should return MBEDTLS_ERR_MPI_NEGATIVE_VALUE).
Fix#4042
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
if MBEDTLS_MPI_MAX_SIZE is odd then RSA_PRV_DER_MAX_BYTES will be two less than expected, since the macros are lacking parentheses.
Signed-off-by: Daniel Otte <d.otte@wut.de>
These implementations don't necessarily consume entropy the same way the
mbed TLS internal software implementation does, and the 'reference
handshake' test vectors can thus not be applied to an ALT implementation.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Rename the enum constants TLS12_PRF_xxx, which are declared in a
public header but not intended for use in application code, to start
with MBEDTLS_PSA_.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Code style changes.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Co-authored-by: Chris Jones <70633990+chris-jones-arm@users.noreply.github.com>
When an Mbed TLS error code combines a low-level error and a
high-level error, the low-level error is usually closer to the root
cause (for example HW_ACCEL_FAILED or ENTROPY_SOURCE_FAILED is more
informative than RSA_PRIVATE_FAILED). So prioritize the low-level code
when converting to a PSA error code, rather than the high-level code
as was (rather arbitrarily) done before.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
mbedtls_rsa_private() could return the sum of two RSA error codes
instead of a valid error code in some rare circumstances:
* If rsa_prepare_blinding() returned MBEDTLS_ERR_RSA_RNG_FAILED
(indicating a misbehaving or misconfigured RNG).
* If the comparison with the public value failed (typically indicating
a glitch attack).
Make sure not to add two high-level error codes.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Closing a wrapped key with the new SE driver interface while
MBEDTLS_PSA_CRYPTO_SE_C is also enabled leads to the key material not
being freed, even though an old SE driver is not in use, leading to a
memory leak. This is because a wrapped key is also considered external.
This commit extends the check for skipping by checking whether an
old-style SE driver is registered with the provided slot, in addition to
checking whether the key is external.
Signed-off-by: Fredrik Strupe <fredrik.strupe@silabs.com>
Make it clear that this is an abstraction of the random generator
abstraction, and not an abstraction of the PSA random generator.
mbedtls_psa_get_random and MBEDTLS_PSA_RANDOM_STATE are public-facing
definitions and will be moved in a subsequent commit.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
In the external RNG case, don't make mbedtls_psa_get_random() a
static inline function: this would likely result in identical
instances of this function in every module that uses it. Instead, make
it a single function with external linkage.
In the non-external case, instead of a trivial wrapper function, make
mbedtls_psa_get_random a constant pointer to whichever DRBG function
is being used.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Hide the obtention of the pointer to the RNG state behind a macro.
To make it possible to use this macro in a module other than
psa_crypto.c, which will happen in the future, make sure that the
definition of the macro does not reference internal variables of
psa_crypto.c. For this purpose, in the internal-DRBG case, export a
symbol containing the address of the DRBG state.
When the RNG state is a pointer a DRBG state, just keep this pointer
in a variable: there's no need to store a pointer to a larger structure.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Executed ./scripts/bump_version.sh --version 2.25.0 --so-crypto 6
Increasing the SO version of the crypto library, because the openless
API improvement came with API/ABI incompatibilities. For example
- the size of psa_key_handle_t changed
- the type of a parameter in 18 public functions has changed from
psa_key_handle_t to mbedtls_svc_key_id_t
Signed-off-by: Janos Follath <janos.follath@arm.com>
T1 is set to a 2-limb value. The first operation that takes it as
input is mbedtls_mpi_mul_int, which makes it grow to 3 limbs. Later it
is shifted left, which causes it to grow again. Set its size to the
final size from the start. This saves two calls to calloc(), at the
expense of a slowdown in some operations involving T1 as input since
it now has more leading zeros.
Setting T1 to 3 limbs initially instead of 2 saves about 6% of the
calloc() calls in test_suite_ecp and does not incur a performance
penalty. Setting T1 to A->n + 2 limbs instead of 2 saves about 20% of
the calloc calls and does not cause a measurable performance
difference on my Linux PC.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Note a possible microoptimization in mbedtls_mpi_mul_hlp that I tried
in the hope of reducing the number of allocations, but turned out to
be counterproductive.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Rewrite mbedtls_mpi_mul_int to call mpi_mul_hlp directly rather than
create a temporary mpi object. This has the benefit of not performing
an allocation when the multiplication is in place (mpi operand aliased
with the result) and the result mpi is large enough.
This saves about 40% of the calloc() calls in test_suite_ecp. There is
no measurable performance difference on my Linux PC.
The cost is a few bytes in bignum.o.
When there is no aliasing, or when there is aliasing but the mpi
object needs to be enlarged, the performance difference is negligible.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
mbedtls_mpi_sub_abs systematically allocated a new mpi when the result
was aliased with the right operand (i.e. X = A - X). This aliasing
very commonly happens during ECP operations. Rewrite the function to
allocate only if the result might not fit otherwise.
This costs a few bytes of code size in bignum.o, and might make
mbedtls_mpi_sub_abs very very slightly slower when no reallocation is
done. However, there is a substantial performance gain in ECP
operations with Montgomery curves (10-20% on my PC).
test_suite_ecp drops from 1422794 to 1271506 calls to calloc().
This commit also fixes a bug whereby mbedtls_mpi_sub_abs would leak
memory when X == B (so TB was in use) and the result was negative.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Reduce the code size, stack consumption and heap consumption in
fix_negative by encoding the special-case subtraction manually.
* Code size: ecp_curves.o goes down from 7837B down to 7769 in a
sample Cortex-M0 build with all curves enabled. The savings come
from not having to set up C in INIT (which is used many times) and
from not having to catch errors in fix_negative.
* Stack consumption: get rid of C on the stack.
* Heap: mbedtls_mpi_sub_abs with destination == second operand would
make a heap allocation. The new code doesn't do any heap allocation.
* Performance: no measurable difference.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This algorithm replaces the pre-existing stream cipher algorithms.
The underlying stream cipher is determined by the key type.
Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
In order to remove large buffers from the stack, the der data is written
into the same buffer that the pem is eventually written into, however
although the pem data is zero terminated, there is now data left in the
buffer after the zero termination, which can cause
mbedtls_x509_crt_parse to fail to parse the same buffer if passed back
in. Patches also applied to mbedtls_pk_write_pubkey_pem, and
mbedtls_pk_write_key_pem, which use similar methods of writing der data
to the same buffer, and tests modified to hopefully catch any future
regression on this.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
