An incorrect error code addition was spotted by the new invasive testing
infrastructure whereby pk_get_pk_alg will always return a high level
error or zero and pk_parse_key_pkcs8_unencrypted_der will try to add
another high level error, resulting in a garbage error code.
Apply the same fix from ae3741e8a to fix the bug.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Replace all occurences of error code addition in the library with the new
MBEDTLS_ERROR_ADD macro.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Add a missing guard for the definition and declaration of
mbedtls_test_hook_error_add.
Also make the declaration always visible when MBEDTLS_TEST_HOOKS is
enabled. This fixes an issue when MBEDTLS_ERROR_C is not defined but
MBEDTLS_TEST_HOOKS is.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Fix unused parameter warnings when MBEDTLS_TEST_HOOKS is not enabled.
A few issues were caught by check-names.sh namely:
- mbedtls_error_add was not capitalised.
- mbedtls_test_hook_error_add was being defined multiple times as the
definition was in a header.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
The previous implementation of the error addition interface did not comply
with the invasive testing architecture guidelines. This commit fixes that
by:
- Renaming functions/macros/variables to follow the mbedtls_error_xxx or
mbedtls_test_hook_xxx convention.
- Making mbedtls_test_hook_error_add a global variable that can be set
by the testing code.
- Using a static inline function call, as opposed to macro, to keep
discrepancies between debug and production version to a minimum.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Change the signature of the `hook` parameter of `mbedtls_set_err_add_hook`
to use the actual signature of the function as opposed to `void *`. This
fixes a warning when compiling with clang `-pedantic`.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
`error.c` and error.h are the more logical place to keep this code and it
prevents issues with building `common.c` and conflicts with other projects
that use mbedtls (such as mbedOS).
`error.c` has been automatically generated by first adding the code to
`error.fmt` and then running `./scripts/generate_errors.pl`.
Also add parenthesis to the addition in `MBEDTLS_ERR_ADD`.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Fix building by adding `common.c` to the build scripts (both make and Cmake).
Also reworks the hook function pointer (also renamed to `err_add_hook`) to be
a static local to `common.c` with a setter function to set the pointer to a
checking function.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
`error.c` is a file generated from `error.h` and thus cannot contain the code
that was previously added. This commit fixes that issue by moving the
`MBEDTLS_ERR_ADD` macro and associated function and function pointer into
`common.h` and `common.c`.
Also fix a typo in `tests/include/test/helpers.h` where tabs were accidentally
used instead of spaces.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
All occurences of manual error code addition/combination, in `rsa.c`, have
been replaced with the `MBEDTLS_ERR_ADD` macro.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Adds a macro (`MBEDTLS_ERR_ADD`) to add error codes together and check that the
result will not be corrupted. This additional check is only enabled during
testing when `MBEDTLS_TEST_HOOKS` is defined.
Also includes a reference usage example in `rsa.c` where two high-level error
codes could be incorrectly added together under the right conditions. This now
ensures that when this error occurs during testing it will be correctly
reported.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Corresponds better to the validation done in other modules of PSA Crypto.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Capitalise the MPS trace macros, as per the coding style (and make a slight
change to naming convention to avoid a name collision).
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
As we want to do Mbed TLS aead operations as a
driver does, aead operations should not access
the key slot as key slots are not available to
drivers.
Second step in this PR: do not unlock the key slot
as part of operation abort.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
As we want to do Mbed TLS aead operations as a
driver does, aead operations should not access
the key slot as key slots are not available to
drivers.
First step in this PR: move key resolution from
aead operation setup to psa_aead_encrypt/decrypt
APIs.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Fix the subtraction in fix_negative, which was incorrectly not looking
for a carry. This caused the result to be wrong when the least
significant limb of N was 0. Fix#4296.
The bug was introduced by d10e8fae9e
"Optimize fix_negative". Thanks to Philippe Antoine (catenacyber) for
reporting the bug which was found by his EC differential fuzzer.
Credit to OSS-Fuzz.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This header file will contain declarations of functions that are not
part of the public ABI/API, and must not be called from other modules,
but can be called from unit tests.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Move the handling of the sign out of the base-specific loops. This
both simplifies the code, and corrects an edge case: the code in the
non-hexadecimal case depended on mbedtls_mpi_mul_int() preserving the
sign bit when multiplying a "negative zero" MPI by an integer, which
used to be the case but stopped with PR #2512.
Fix#4295. Thanks to Guido Vranken for analyzing the cause of the bug.
Credit to OSS-Fuzz.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
In a TLS client, enforce the Diffie-Hellman minimum parameter size
set with mbedtls_ssl_conf_dhm_min_bitlen() precisely. Before, the
minimum size was rounded down to the nearest multiple of 8.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
When creating a persistent key or registering a key
with an invalid key identifier return
PSA_ERROR_INVALID_ARGUMENT instead of
PSA_ERROR_INVALID_HANDLE.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
This check was added earlier to avoid useless calls to `memcpy()`
with length `0` in the _frequent_ case where we're not accumulating.
By now, the whole code path has been moved to a branch which is only
executed if the reader is accumulating, and the only time this check
would be relevant is if we happen to feed an empty fragment to the
reader. In this case, the call to memcpy() could be removed, but
since this case is exceptional and the call to memcpy() is still
correct even for a length 0 copy, we remove the check for simplicity
of the code.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
Previously, the semantics of mbedtls_mps_reader_commit() was to invalidate
all buffers previously fetched via mbedtls_mps_reader_get(), forbidding
any further use by the 'consumer'. This was in fact a necessary constraint
for the current implementation, which did some memory moving in
mbedtls_mps_reader_commit().
This commit simplifies the reader's semantics and implementation in
the following way:
- API: A call to mbedtls_mps_reader_commit() does no longer invalidate
the buffers previously obtained via mbedtls_mps_reader_get().
Instead, they can continue to be used until
mbedtls_mps_reader_reclaim() is called.
Calling mbedtls_mps_reader_commit() now only sets a marker
indicating which parts of the data received through
mbedtls_mps_reader_get() need not be backed up once
mbedtls_mps_reader_reclaim() is called. Allowing the user
to call mbedtls_mbedtls_reader_commit() multiple times
before mbedtls_mps_reader_reclaim() is mere convenience:
We'd get exactly the same functionality if instead of
mbedtls_mps_reader_commit(), there was an additional argument
to mbedtls_mps_reader_reclaim() indicating how much data
to retain. However, the present design is more convenient
for the user and doesn't appear to introduce any unnecessary
complexity (anymore), so we stick with it for now.
- Implementation: mbedtls_mps_reader_commit() is now a 1-liner,
setting the 'commit-marker', but doing nothing else.
Instead, the complexity of mbedtls_mp_reader_reclaim()
slightly increases because it has to deal with creating
backups from both the accumulator and the current
fragment. In the previous implementation, which shifted
the accumulator content with every call to
mbedtls_mps_reader_commit(), only the backup from the
fragment was necessary; with the new implementation
which doesn't shift anything in
mbedtls_mps_reader_commit(), we need to do the
accumulator shift in mbedtls_mps_reader_reclaim().
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
This commit adds an implementation of the MPS trace module
based on `printf()`.
The enabling macro MBEDTLS_MPS_TRACE remains unset by default
because MPS tracing is very verbose and consumes unnecessary
space in the CI.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
This commit adds an internal header `library/mps/error.h` related
to error codes in MPS.
For now, those error codes can be considered internal and thus we
don't have to avoid clashes with other Mbed TLS error codes. This
is OK as long as it's true that MPS isn't public API, and its error
codes are never forwarded to the return values of public API calls.
The error code allocation of MPS will likely need revisiting over time.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
Most buffers that MPS deals with are small and representable
with integer types of width 16-bit or more.
For highly memory constrained systems, it is therefore a potential
for significant memory savings to use 16-bit types for buffer sizes
throughout MPS.
In prepraration for this, this commit introduces typdefs
```
mbedtls_mps_size_t
mbedtls_mps_stored_size_t
```
for buffer sizes in the MPS implementation and the MPS structures,
respectively.
So far, those MUST be defined as `size_t`: While an effort has been made
to write most of MPS code in terms of `mbedtls_mps_[stored_]size_t` in a
way that would allow narrower types, those aren't yet supported. Still,
we retain the typedefs in order to avoid unnecessary rewriting of a large
body of the MPS codebase.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
This commit adds the interface fo the MPS reader component as
`library/mps/reader.h`.
Please see the file itself for extensive documentation.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
MPS' tracing module uses four macros:
1) TRACE( type, fmt, ... )
This acts like `printf( fmt, ... )` but also allows
the specification of a type of trace output (comment,
warning, error, ...)
2) TRACE_INIT
This acts like TRACE() but increases the level of
indentation. It will be used at the beginning of
function calls.
3) RETURN( val )
Equivalent to `return( val )` plus a decrement in the
level of indentation. This should be used at the end of
functions that have been started with TRACE_INIT.
4) TRACE_END
This combines a trace output with a decrement of the
level of indentation. It's necessary prior to leaving
functions which have been started with TRACE_INIT
but which don't have a return value.
This commit defines those macros as no-op dummies in
`library/mps/trace.h` for now.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
This commit adds an MPS-specific compile-time option
`MBEDTLS_MPS_TRACE` to the internal MPS header `mps/common.h`.
So far -- this may need revisiting -- MPS comes with its own
internal tracing module which allows to track the operation of
MPS' various layers for the purpose of understanding of it workings
as well as for debugging.
The reasons for the introduction of a module separate from SSL debug
are the following:
1) The SSL debug module requires an SSL context to function because
debug callbacks are part of the runtime configuration of the SSL
module.
The MPS tracing module, in contrast, is not supposed to be used
in production environments, and there is no need for a runtime
configuration. Instead, a compile-time defined tracing callback
is used.
2) In the interest of modularity, MPS' tracing module shouldn't
require having an SSL context around.
3) Purely visually, MPS' tracing module adds support for indentation
according to call-depth and coloring according to which module is
being used, which makes it very useful for what's going on; however,
those features aren't available in the SSL debug module (and they
shouldn't be).
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
This commit adds the compile-time option MBEDTLS_MPS_ENABLE_ASSERTIONS
which controls the presence of runtime assertions in MPS code.
See the documentation in the header for more information.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
This commit introduces the internal MPS header `mps/common.h`
which will subsequently be populated with MPS-specific compile-time
options and helper macros. For now, it's a stub.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
Remove cipher_generate_iv driver entry point as there
is no known use case to delegate this to a driver.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
The IV length computed in the cipher PSA implementation is
the default IV length thus use the PSA macro PSA_CIPHER_IV_LENGTH
defined to do that.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Use psa_generate_random() to generate IVs instead of
mbedtls_psa_get_random(). mbedtls_psa_get_random() is
meant to be used as the f_rng argument of Mbed TLS
library functions.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Symmetric key management is not intended to be
delegated to drivers. Thus, key management code
for a given symmetric key type should be included
in the library whether or not the support for
cryptographic operations based on that type of
symmetric key may be delegated to drivers.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Isolate the Mbed TLS cipher driver interfaces.
Do the actual cipher operations in utility
functions that are just called by the interface
functions.
The utility functions are intended to be also called
by the cipher test driver interface functions (to be
introduced subsequently) and allow to test the case
where cipher operations are fully accelerated with no
fallback (component test_psa_crypto_config_basic of
all.sh).
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
As per drivers, pass to the Mbed TLS implementation of
the cipher multi-part operation its operation context
and not the PSA operation context.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Move members that are of no use to the PSA crypto core
to the Mbed TLS implementation specific operation context.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
For cipher multi-part operations, dispatch based on
the driver identifier even in the case of the
Mbed TLS software implementation (viewed as a driver).
Also use the driver identifier to check that an
cipher operation context is active or not.
This aligns the way hash and cipher multi-part
operations are dispatched.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change the operation context to the PSA one to be
able to call the software implementation from
the driver wrapper later on.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change the signature of
psa_driver_wrapper_cipher_encrypt/decrypt_setup to
that of a PSA driver cipher_encrypt/decrypt_setup
entry point.
Change the operation context to the PSA one to be
able to call the software implementation from
the driver wrapper later on.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Split out the cipher setup based on cipher.c
in psa_cipher_setup_internal() whose signature
is that of a PSA driver cipher_setup entry
point.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Rework psa_cipher_setup in preparation of
calling the cipher setup based on cipher.c
through the interface of a PSA driver
cipher_setup entry point.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Add psa_crypto_cipher.[ch] files to contain the
Mbed TLS implementation of PSA driver cipher driver
entry points.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
To run succesfully the test
"PSA sign: invalid algorithm for ECC key" of
test_suite_psa_crypto when ECDSA support is not included
in the library, always return INVALID_ARGUMENT
in case of an ECC key not used for ECDSA, whether
ECDSA support is present or not.
Then apply the same logic to RSA sign RSA and RSA/ECC
verify for the sake of consistency.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Add missing PSA_WANT_CCM/GCM/CMAC. This completes
the set of PSA_WANT config options given the
current support of PSA crypto in Mbed TLS.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
If an elliptic curve was enabled in the Mbed TLS classic API (#define
MBEDTLS_ECP_DP_xxx), but not enabled in the PSA configuration (#define
PSA_WANT_ECC_xxx), it would still work if you tried to use it through
PSA.
This is generally benign, but could be a security issue if you want to
disable a curve in PSA for some security reason (such as a known bug
in its implementation, which may not matter in the classic API if Mbed
TLS is running in a secure enclave and is only reachable from
untrusted callers through the PSA API). More urgently, this broke
test_suite_psa_crypto_not_supported.generated.
So if a curve is not enabled in the PSA configuration, ensure that
it's treated as unsupported through the PSA software implementation.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Return PSA_ERROR_INVALID_HANDLE instead of
PSA_ERROR_DOES_NOT_EXIST if invalid key is passed for some key
operations.
Signed-off-by: Maulik Patel <Maulik.Patel@arm.com>
The hash driver entry points (and consequentially the hash driver core)
are now always compiled on when PSA_CRYPTO_DRIVER_TEST is turned on.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
The PSA Core is already calling psa_hash_abort, so the driver doesn't
have to do that explicitly.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Drivers (both built-in and external) need to declare their context
structures in a way such that they are accessible by the
to-be-autogenerated crypto_driver_contexts.h file. That file lives in
include/psa, which means all builtin driver context structure
declarations also need to live in include/psa.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
MinGW and older windows compilers cannot cope with %zu or %lld (there is
a workaround for MinGW, but it involves linking more code, there is no
workaround for Windows compilers prior to 2013). Attempt to work around
this by defining printf specifiers for size_t per platform for the
compilers that cannot use the C99 specifiers.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
This was a false positive caused by the compiler seeing the %08lx
specifiers and judging the output on that, rather than the numbers being
fed in. Given these are going to be maximum 32 bit numbers, then better
to use %08x, which keeps -Wformat-truncation=2 happy as well.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
