Session-ID based session resumption requires that the resumed session
is consistent with the client's ClientHello in terms of choice of
ciphersuite and choice of compression.
This check was previously assumed to be performed in the session cache
implementation, which seems wrong: The session cache should be an id-based
lookup only, and protocol specific checks should be left to Mbed TLS.
This commit
- adds an explicit ciphersuite and compression consistency check after
the SSL session cache has been queried
- removes the ciphersuite and compression consistency check from
Mbed TLS' session cache reference implementation.
Signed-off-by: Hanno Becker <hanno.becker@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>
Fixes for printf format specifiers, where they have been flagged as
invalid sizes by coverity, and new build flags to enable catching these
errors when building using CMake. Note that this patch uses %zu, which
requires C99 or later.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Printf could potentially produce 2 64 bit numbers here when there is
only space for one, thus causing a buffer overflow. This was caught by
the new warning flags.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
We were not getting any warnings on printf format errors, as we do not
explicitly use printf anywhere in the code. Thankfully there is a way
to mark a function as having printf behaviour so that its inputs can be
checked in the same way as printf would be.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
`entropy_poll.h` is not supposed to be used by application code and
is therefore being made internal.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
`aesni.h` is an internal header and is moved accordingly.
Also removes some references to internal headers in scripts with
only public headers.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Revert changes introduced in 50518f4195
as it is now clear that these headers are internal without the
`*_internal.h` suffix.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
This gives it a more descriptive name and indicates to alt developers
that the definitions inside are not fully internal and are available
to alt developers for use.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Rename both `rsa_internal.h` and `rsa_internal.c` to more descriptive
names: `rsa_alt_helpers.h` and `rsa_alt_helpers.c`.
Also re-orders `rsa_internal.c` to match the order in `rsa_internal.h`
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Move `include/mbedtls/bn_mul.h` to `library/bn_mul.h`.
Update includes and references to `bn_mul.h` to new location.
Also remove internal headers from `cpp_dummy_build.cpp` as it should only
test public headers in the library.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Simple find and replace using `#include (<|")mbedtls/(.*)_internal.h(>|")`
and `#include $1$2_internal.h$3`.
Also re-generated visualc files by running
`scripts/generate_visualc_files.pl`.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
`cipher_internal.h`, `ecp_internal.h`, `md_internal.h`, `pk_internal.h`
and `ssl_internal.h` have all been moved.
Includes and dependnecies have not been renamed yet, only direct mv.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Only move `rsa_internal.h` for now to test dependancies. Other internal
headers will be moved in following commits.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Apply the right define guards for the right purpose. The 'core' hash
driver is included if any hash algorithm is either to be tested through
the test driver, or if it is requested by a user and not accelerated
(i.e. 'fallback'/'software' driver requested for the algorithm).
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Apparently there's a goal to make the PSA Crypto core free from
dynamic memory allocations. Therefore, all driver context structures
need to be known at compile time in order for the core to know their
final size.
This change defines & implements for hashing operations how the context
structures get defined.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
mac size is previously checked to not be less than 4, so it can't be zero
anymore at this point.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Memsan build was reporting a false positive use of uninitialised memory
in x509_crt.c on a struct filled by an _stat function call. According to
the man pages, the element reported has to be filled in by the call, so
to be safe, and keep memsan happy, zero the struct first.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Fix function mbedtls_ecp_mul_shortcuts() to skip multiplication when m
is 0 and simply assignt 0 to R. Additionally fix ecjpake_zkp_read() to
return MBEDTLS_ERR_ECP_INVALID_KEY when the above condintion is met.
Fix#1792
Signed-off-by: TRodziewicz <rodziewicz@gmail.com>
When ECDSA is not supported by the library, prefer
to return NOT_SUPPORTED than INVALID_ARGUMENT when
asked for an ECDSA signature.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Move the check that ECDSA is supported from the
caller of the function responsible for Mbed TLS
ECDSA signatures to this function, namely
mbedtls_psa_ecdsa_sign_hash().
This makes the caller code more readable and is
more aligned with what is expected from a
sign_hash() PSA driver entry point.
Add a negative test case where a deterministic
ECDSA signature is requested while the library
does not support deterministic ECDSA.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Reworked the validation of MAC algorithm with the used key type by
introducing psa_mac_key_can_do, which guarantees that PSA_MAC_LENGTH can
be called successfully after validation of the algorithm and key type.
This means psa_get_mac_output_length is no longer required.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Multiplication is not constant flow on any CPU we are generally
targetting, so replace this with bit twiddling.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Comparing algorithm with its FULL_LENGTH_MAC version doesn't work in
cases where algorithm is a wildcard. Wildcard input is not specified in
the documentation of the function, but in order to test the function
using the same test as PSA_MAC_LENGTH we're mimicking that behaviour here.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Avoid code duplication. Also update the guarantees made by the function
doc to match the guarantees given by PSA_MAC_LENGTH.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
This makes it more in-line with how psa_key_policy_permits works. It
also adds consistency: the intersection of MAC with default length and
MAC with exact-length is now computed correctly in case the exact length
equals the default length of the algorithm when used with the given
key type.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
The field `cur_out_ctr` in the SSL context keeps track of the
record sequence number for the next outgoing record.
For TLS, this sequence number is implicit and not transmitted
on the wire, while for DTLS, it's part of of the record header.
For DTLS, the position of the record sequence number of the next
outgoing record in that record's header is tracked in the pointer
`out_ctr`. This pointer moves forward along with other pointers
such as `out_hdr` or `out_msg` within the outgoing data buffer
`out_buf` as multiple records are written in the same datagram.
For TLS, the `out_ctr` pointer is logically superfluous, but
for some reason, we're still maintaining it by having it point
to the 8 Bytes prior to the header of the next outgoing record,
and always copying `cur_out_ctr` to this position prior to
encrypting an outgoing record.
After a record has been prepared for writing in `ssl_write_record()`,
the `out_xxx` pointers (except for `out_buf`, which is static),
are shifted forward so that they point to the header and content
of the next outgoing record. This is used only in DTLS in order
to stack multiple records into a single datagram, but the shifting
is happening for TLS as well. However, it has little effect in TLS
because we're always flushing immediately after writing, and afterwards
reset the `out_xxx` pointers.
While the present code works as-is, it is wrong to shift `out_ctr`
in the case of TLS, because it makes `out_ctr` point to the last 8
Bytes of the ciphertext of the last outgoing record. Should we ever
aim to prepare more than one protected record in `out_buf` before
dispatching it to the underlying transport, the superfluous copying
of `cur_out_ctr` to `out_buf` will corrupt the last 8 bytes of the
last record.
This commit aims to fix this problem in the minimal possible way,
by simply not shifting `out_ctr` after a record has been written.
It does deliberately not attempt to remove `out_ctr` for TLS altogether,
because any change in the messaging layer is hard to review, and
we're going to replace it soon anyhow.
The shifting happens in the helper routine mbedtls_ssl_update_out_pointers,
which assumed correctness of `out_hdr` for the beginning of the record header
of the next outgoing record, and derives the other `out_xxx` variables.
We remove the update of `out_ctr` from this function in the case of TLS,
and instead move the proper initialization of `out_ctr` to
`out_buf == initial_out_hdr - 8` to the function
mbedtls_ssl_reset_in_out_pointers().
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
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>
