This commit removes the test_psa_crypto_config_basic
all.sh component that can no longer work without
adapting it to the separately compiled test driver
library. This component is replaced by several
components in the following commits to test various
type of acceleration independently.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
The PSA max hash size has to be 64 if SHA512 or
SHA384 is supported by the library or an
accelerator, not just in case of the library.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Several files among include/psa/crypto_*.h are not meant to be included
directly, and are not guaranteed to be valid if included directly. This
makes it harder to perform some static analyses. So make these files more
self-contained so that at least, if included on their own, there is no
missing macro or type definition (excluding the deliberate use of forward
declarations of structs and unions).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
- Add return value description to the docs of mbedtls_sha256
- Remove description of non-existing "mode" parameter from the docs of mbedtls_aes_crypt_ctr
Backport of #5105
Signed-off-by: Tom Cosgrove <tom.cosgrove@arm.com>
This slightly increases the RAM consumption per context, but saves code
size on architectures with an instruction for direct byte access (which is
most of them).
Although this is technically an API break, in practice, a realistic
application won't break: it would have had to bypass API functions and rely
on the field size (e.g. relying on -1 == 1 in a 1-bit field).
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/ssl_cli.o: 19952 -> 19900 (diff: 52)
library/ssl_msg.o: 25810 -> 25798 (diff: 12)
library/ssl_srv.o: 22371 -> 22299 (diff: 72)
library/ssl_tls.o: 23274 -> 23038 (diff: 236)
Results (same architecture, config-suite-b.h + MBEDTLS_ECDH_LEGACY_CONTEXT +
MBEDTLS_ECP_RESTARTABLE):
library/ssl_cli.o: 2868 -> 2848 (diff: 20)
library/ssl_msg.o: 2916 -> 2924 (diff: -8)
library/ssl_srv.o: 3204 -> 3184 (diff: 20)
library/ssl_tls.o: 5860 -> 5756 (diff: 104)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Move small fields first so that more fields can be within the Arm Thumb
128-element direct access window.
The ordering in this commit is not based on field access frequency.
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/ssl_cli.o: 20104 -> 19952 (diff: 152)
library/ssl_msg.o: 25942 -> 25810 (diff: 132)
library/ssl_srv.o: 22467 -> 22371 (diff: 96)
library/ssl_tls.o: 23390 -> 23274 (diff: 116)
Results (same architecture, config-suite-b.h + MBEDTLS_ECDH_LEGACY_CONTEXT +
MBEDTLS_ECP_RESTARTABLE):
library/ssl_cli.o: 2928 -> 2868 (diff: 60)
library/ssl_msg.o: 2924 -> 2916 (diff: 8)
library/ssl_srv.o: 3232 -> 3204 (diff: 28)
library/ssl_tls.o: 5904 -> 5860 (diff: 44)
Signed-off-by: Lukasz Gniadzik <lukasz.gniadzik@mobica.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Reorder fields mbedtls_ssl_handshake_params in order to save code on Arm
Thumb builds. The general idea is to put often-used fields in the direct
access window of 128 elements from the beginning of the structure.
The reordering is a human selection based on a report of field offset and
use counts, and informed by measuring the code size with various
arrangements. Some notes:
* I moved most byte-sized fields at the beginning where they're sure to be
in the direct access window.
* I moved buffering earlier because it can be around the threshold depending
on the configuration, and it's accessed in a lot of places.
* I moved several fields, including update_checksum and friends, early so
that they're guaranteed to be in the early access window.
* I tried moving randbytes or premaster to the early access window, but
I couldn't find a placement which would save code size, presumably because
they're bumping too many other fields, and they're mostly accessed through
memcpy and friends which translates to instructions that don't have an
offset for free anyway.
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/ssl_cli.o: 20200 -> 20104 (diff: 96)
library/ssl_msg.o: 25978 -> 25942 (diff: 36)
library/ssl_srv.o: 22691 -> 22467 (diff: 224)
library/ssl_tls.o: 23570 -> 23390 (diff: 180)
Results (same architecture, config-suite-b.h + MBEDTLS_ECDH_LEGACY_CONTEXT +
MBEDTLS_ECP_RESTARTABLE):
library/ssl_cli.o: 3012 -> 2928 (diff: 84)
library/ssl_msg.o: 2932 -> 2924 (diff: 8)
library/ssl_srv.o: 3288 -> 3232 (diff: 56)
library/ssl_tls.o: 6032 -> 5904 (diff: 128)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Replace bitfields mbedtls_ssl_handshake_params by bytes. This saves some
code size, and since the bitfields weren't group, this doesn't increase the
RAM usage.
Replace several ints that only store values in the range 0..255 by uint8_t.
This can increase or decrease the code size depending on the architecture
and on how the field is used. I chose changes that save code size on Arm
Thumb builds and may potentially save more after field reordering.
Leave the bitfields in struct mbedtls_ssl_hs_buffer alone: replacing them by
uint8_t slightly increases the code size.
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/ssl_srv.o: 22735 -> 22691 (diff: 44)
library/ssl_tls.o: 23566 -> 23570 (diff: -4)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Move fields around to have fewer accesses outside the 128-element Thumb
direct access window.
In psa_hkdf_key_derivation_t, move the large fields (output_block, prk,
hmac) after the state bit-fields. Experimentally, it's slightly better
to put hmac last.
In aead_operation_t, tag_length was outside the window. The details depend
on the sizes of contexts included in ctx. Make the large ctx be the last
field.
In mbedtls_psa_hmac_operation_t, the opad field is outside the window when
SHA-512 is enabled. Moving opad before hash_ctx only saves 4 bytes and made
the structure clumsy, so I left it alone.
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/psa_crypto.o: 16246 -> 16166 (diff: 80)
library/psa_crypto_aead.o: 952 -> 928 (diff: 24)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The current definition of PSA_ALG_IS_HASH_AND_SIGN includes
PSA_ALG_RSA_PKCS1V15_SIGN_RAW and PSA_ALG_ECDSA_ANY, which don't strictly
follow the hash-and-sign paradigm: the algorithm does not encode a hash
algorithm that is applied prior to the signature step. The definition in
fact encompasses what can be used with psa_sign_hash/psa_verify_hash, so
it's the correct definition for PSA_ALG_IS_SIGN_HASH. Therefore this commit
moves definition of PSA_ALG_IS_HASH_AND_SIGN to PSA_ALG_IS_SIGN_HASH, and
replace the definition of PSA_ALG_IS_HASH_AND_SIGN by a correct one (based
on PSA_ALG_IS_SIGN_HASH, excluding the algorithms where the pre-signature
step isn't to apply the hash encoded in the algorithm).
In the definition of PSA_ALG_SIGN_GET_HASH, keep the condition for a nonzero
output to be PSA_ALG_IS_HASH_AND_SIGN.
Everywhere else in the code base (definition of PSA_ALG_IS_SIGN_MESSAGE, and
every use of PSA_ALG_IS_HASH_AND_SIGN outside of crypto_values.h), we meant
PSA_ALG_IS_SIGN_HASH where we wrote PSA_ALG_IS_HASH_AND_SIGN, so do a
global replacement.
```
git grep -l IS_HASH_AND_SIGN ':!include/psa/crypto_values.h' | xargs perl -i -pe 's/ALG_IS_HASH_AND_SIGN/ALG_IS_SIGN_HASH/g'
```
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This is a variant of PSA_ALG_RSA_PSS which currently has exactly the same
behavior, but is intended to have a different behavior when verifying
signatures.
In a subsequent commit, PSA_ALG_RSA_PSS will change to requiring the salt
length to be what it would produce when signing, as is currently documented,
whereas PSA_ALG_RSA_PSS_ANY_SALT will retain the current behavior of
allowing any salt length (including 0).
Changes in this commit:
* New algorithm constructor PSA_ALG_RSA_PSS_ANY_SALT.
* New predicates PSA_ALG_IS_RSA_PSS_STANDARD_SALT (corresponding to
PSA_ALG_RSA_PSS) and PSA_ALG_IS_RSA_PSS_ANY_SALT (corresponding to
PSA_ALG_RSA_PSS_ANY_SALT).
* Support for the new predicates in macro_collector.py (needed for
generate_psa_constant_names).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Use the encoding from an upcoming version of the specification.
Add as much (or as little) testing as is currently present for Camellia.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The previous implementation was misparsed in constructs like
`if (condition) MBEDTLS_IGNORE_RETURN(...); else ...;`.
Implement it as an expression, tested with GCC, Clang and MSVC.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This macro is introduced here for use in deprecated functions. It may also
be useful in user code, so it is in a public header.
Signed-off-by: Mateusz Starzyk <mateusz.starzyk@mobica.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
MBEDTLS_CHECK_RETURN_TYPICAL defaults off, but is enabled if
MBEDTLS_CHECK_RETURN_WARNING is enabled at compile time.
(MBEDTLS_CHECK_RETURN_CRITICAL is always enabled.)
The default is off so that a plausible program that builds with one version
of Mbed TLS in the default configuration will still build under the next
version.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This is normally equivalent, but works even if some other header defines a
macro called warn_unused_result.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
An empty expansion is possible, but as documented its effect is to disable
the feature, so that isn't a good example. Instead, use the GCC
implementation as the default: it's plausible that it could work even on
compilers that don't advertise themselves as sufficiently GCC-like to define
__GNUC__, and if not it gives users a concrete idea of what the macro is
supposed to do.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
For all of these functions, the only possible failures are a hardware
accelerator (not possible unless using an ALT implementation), an internal
error or runtime corruption.
Exception: the self-tests, which serve little purpose if their status isn't
tested.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Define macros MBEDTLS_CHECK_RETURN_CRITICAL, MBEDTLS_CHECK_RETURN_TYPICAL
and MBEDTLS_CHECK_RETURN_OPTIONAL so that we can indicate on a
function-by-function basis whether checking the function's return value is
almost always necessary (CRITICAL), typically necessary in portable
applications but unnecessary in some reasonable cases (TYPICAL), or
typically unnecessary (OPTIONAL).
Update the documentation of MBEDTLS_CHECK_RETURN accordingly. This is split
between the user documentation (Doxygen, in config.h) and the internal
documentation (non-Doxygen, in platform_util.h, of minor importance since
the macro isn't meant to be used directly).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Declare all AES and DES functions that return int as needing to have
their result checked, and do check the result in our code.
A DES or AES block operation can fail in alternative implementations of
mbedtls_internal_aes_encrypt() (under MBEDTLS_AES_ENCRYPT_ALT),
mbedtls_internal_aes_decrypt() (under MBEDTLS_AES_DECRYPT_ALT),
mbedtls_des_crypt_ecb() (under MBEDTLS_DES_CRYPT_ECB_ALT),
mbedtls_des3_crypt_ecb() (under MBEDTLS_DES3_CRYPT_ECB_ALT).
A failure can happen if the accelerator peripheral is in a bad state.
Several block modes were not catching the error.
This commit does the following code changes, grouped together to avoid
having an intermediate commit where the build fails:
* Add MBEDTLS_CHECK_RETURN to all functions returning int in aes.h and des.h.
* Fix all places where this causes a GCC warning, indicating that our code
was not properly checking the result of an AES operation:
* In library code: on failure, goto exit and return ret.
* In pkey programs: goto exit.
* In the benchmark program: exit (not ideal since there's no error
message, but it's what the code currently does for failures).
* In test code: TEST_ASSERT.
* Changelog entry.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The API reached 1.0.0 some time ago, and we've caught up with the
incompatible changes already.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Add memory constraints to the aarch64 inline assembly in MULADDC_STOP.
This fixes an issue where Clang 12 and 13 were generating
non-functional code on aarch64 platforms. See #4962, #4943
for further details.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
MULADDC_CORE reads from (%%rsi) and writes to (%%rdi). This fragment is
repeated up to 16 times, and %%rsi and %%rdi are s and d on entry
respectively. Hence the complete asm statement reads 16 64-bit words
from memory starting at s, and writes 16 64-bit words starting at d.
Without any declaration of modified memory, Clang 12 and Clang 13 generated
non-working code for mbedtls_mpi_mod_exp. The constraints make the unit
tests pass with Clang 12.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Put this macro before a function declaration to indicate that its result
must be checked. This commit supports GCC-like compilers and MSVC >=2012.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Now that descriptions of error codes no longer have to be on the same line
for the sake of generate_errors.pl, move them to their own line before the
definition. This aligns them with what we do for other definitions, and
means that we no longer need to have very long lines containing both the C
definition and the comment.
```
perl -i -pe 's~^(#define +MBEDTLS_ERR_\w+ +-\w+) */\*[*!]<(.*)\*/~/**$2*/\n$1~' include/mbedtls/*.h
```
This commit does not change the output of generate_errors.pl.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Although checking if the key was symmetric was correct, its easier to
read if we just check the block length is not zero before we use it in a
division.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
If PSA_CIPHER_ENCRYPT_OUTPUT_SIZE was called on a non symmetric key,
then a divide by zero could happen, as PSA_CIPHER_BLOCK_LENGTH will
return 0 for such a key, and PSA_ROUND_UP_TO_MULTIPLE will divide by
the block length.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
These macros were moved into a header and now check-names.sh is failing.
Add an MBEDTLS_ prefix to the macro names to make it pass.
Signed-off-by: Janos Follath <janos.follath@arm.com>
Backport 2.x: Fix and test the MBEDTLS_PSA_CRYPTO_SPM build
Straightforward backport from development to developement_2.x plus one trivial commit, only one approval is enough.
Fix initialization of mbedtls_psa_cipher_operation_t by not initializing the mbedtls_cipher_context_t typed field completely.
Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
According to the PSA specification the PSA_USAGE_SIGN_HASH has the
permission to sign a message as PSA_USAGE_SIGN_MESSAGE. Similarly the
PSA_USAGE_VERIFY_HASH has the permission to verify a message as
PSA_USAGE_VERIFY_MESSAGE. These permission will also be present when
the application queries the usage flags of the key.
Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
This makes it easier to ensure that crypto_spe.h is included everywhere it
needs to be, and that it's included early enough to do its job (it must be
included before any mention of psa_xxx() functions with external linkage,
because it defines macros to rename these functions).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Hashes used in RSA-PSS encoding (EMSA-PSS-ENCODE, see §9.1.1 in RFC
8017):
- H1: Hashing the message (step 2)
- H2: Hashing in the salt (step 6)
- H3: Mask generation function (step 9)
According to the standard:
- H1 and H2 MUST be done by the same hash function
- H3 is RECOMMENDED to be the same as the hash used for H1 and H2.
According to the implementation:
- H1 happens outside of the function call. It might or might not happen
and the implementation might or might not be aware of the hash used.
- H2 happens inside the function call, consistency with H1 is not
enforced and might not even be possible to detect.
- H3 is done with the same hash as H2 (with the exception of
mbedtls_rsassa_pss_verify_ext(), which takes a dedicated parameter for
the hash used in the MGF).
Issues with the documentation:
- The comments weren't always clear about the three hashes involved and
often only mentioned two of them (which two varied from function to
function).
- The documentation was giving the impression that the standard
recommends aligning H2 and H1 (which is not a recommendation but a
must).
Signed-off-by: Janos Follath <janos.follath@arm.com>
Montgomery curves are not in the expected place in the curve list.
This is a bug (https://github.com/ARMmbed/mbedtls/issues/4698), but
until this bug is fixed, document the current behavior and indicate
that it's likely to change.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
MBEDTLS_ECP_MAX_BITS is now determined automatically from the configured
curves and no longer needs to be configured explicitly to save RAM. Setting
it explicit in config.h is still supported for backward compatibility.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.
Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7ebaaa1 ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.
Fixes#4633
Signed-off-by: Jaeden Amero <jaeden.amero@arm.com>
We stated that curves were listed "in order of preference", but we never
explained what the preference was, so this was not meaningful.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
MBEDTLS_ECP_WINDOW_SIZE is a compromise between memory usage (growing based
on the value) and performance (faster with larger values). There are
disminishing returns as the value grows larger. Based on Manuel's benchmarks
recorded in https://github.com/ARMmbed/mbedtls/issues/4127, 4 is a good
compromise point, with larger values bringing little advantage. So reduce
the default from 6 to 4.
Document the default value as in optimized for performance mostly, but don't
document the specific value, so we may change it later or make it
platform-dependent.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Note that this error has a negligible probability with a "crypto-sized"
bound, but macroscopic probability with a small bound.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Since mbedtls_mpi_random() is not specific to ECC code, move it from
the ECP module to the bignum module.
This increases the code size in builds without short Weierstrass
curves (including builds without ECC at all) that do not optimize out
unused functions.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
