* Reworked the cipher context once again to be more robustly defined
* Removed redundant memset
* Unified behaviour on failure between driver and software in cipher_finish
* Cipher test driver setup function now also returns early when its status
is overridden, like the other test driver functions
* Removed redundant test cases
* Added bad-order checking to verify the driver doesn't get called where
the spec says it won't.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
As pointed out by Ronald. The key slot is populated using
get_key_from_slot, and after calling the driver the slot is
validated to not contain an external key, so calling
get_transparent_key is superfluous.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Added zeroization of the wrapper context on failure/abort, and reliance on
the crypto core to not call an uninitialised wrapper.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Once an operation has been 'accepted' by a driver, the remainder is bound
to the same driver, since driver-specific context structs cannot be shared.
This provides a pretty good gate mechanism for the fallback logic, too.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
ssl_tls1_3_keys.c exports a structure containing all labels used
in the TLS 1.3 key schedule, but the TLS 1.3 key scheduling unit
tests so far replicated those labels in the test file. In particular,
wrong label values in ssl_tls1_3_keys.c wouldn't have been caught
by the unit tests.
This commit modifies the TLS 1.3 key schedule unit tests to use
the TLS 1.3 labels as exported by ssl_tls1_3_keys.c. This not only
makes sure that those labels are correct, but also avoids hardcoding
their hex-encoding in the test file.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
Needed to make additional fixes so that when MBEDTLS_USE_PSA_CRYPTO
is defined, the depends-hashes test will succeed. There are two
versions of the ecdsa_verify_wrap() function, one with
MBEDTLS_USE_PSA_CRYPTO and when when it is not enabled. The non PSA
version is not using the md_alg parameter since it is not required.
The PSA version was using that parameter to derive a different value
it needed for PSA_ALG_ECDSA. The arguement of PSA_ALG_ECDSA is
ignored for psa_sign_hash and psa_verify_hash. It is present because
it is used and must be a valid hash, not zero, for psa_sign_hash
(but not psa_verify_hash) with PSA_ALG_DETERMINISTIC_ECDSA, and it is
needed for psa_sign_message and psa_verify_message which are not
implemented yet. The local parameter now uses PSA_ALG_ECDSA_ANY for
the verify function to avoid using the md_alg parameter and avoids
returning incorrect error codes.
Fixes#3587
Signed-off-by: John Durkop <john.durkop@fermatsoftware.com>
This is in line with how the entries of the TLS 1.3 label
structure `mbedtls_ssl_tls1_3_labels` are initialized.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
`common.h` takes care of the logic of chosing the correct
configuration file, so we don't need to replicate it in
each source file.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
This commit introduces a new file library/ssl_tls13_key.c
which will subsequently be populated with functionality relating
to the TLS 1.3 key schedule.
Those functions are expected to be internal and are documented
in the internal header library/ssl_tls13_keys.h.
The first function to be implemented is the key expansion
function `HKDF-Expand-Label`. See the documentation in
library/ssl_tls13_keys.h for more information.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
Trying to compile in the PSA accelerator test driver under MBEDTLS_TEST_HOOKS
turned out to be awkward regarding existing builds. We'll put it under a
custom (not in config.h) define instead, since it's something that only
should happen in test.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
This is a more natural place for handling the drivers belonging to the
'previous' SE driver spec. It makes for a cleaner psa_crypto.c, and
potentially an easier overview of how to migrate from the old SE driver
interface to the 'opaque accelerator' interface.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Use macros instead of accessing directly the key identifier
fields for coding consistency and ease maintenance.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Fix PSA code and unit tests for the unit tests
to pass with key identifiers encoding owner
identifiers.
The changes in PSA code just make the enablement
of key identifiers encoding owner identifiers
platform independent. Previous to this commit,
such key identifiers were used only in the case
of PSA SPM platforms.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
With PSA crypto v1.0.0, a volatile key identifier may
contain a owner identifier but no file is associated
to it. Thus rename the type psa_key_file_id_t to
mbedtls_svc_key_id_t to avoid a direct link with a
file when a key identifier involves an owner
identifier.
The new type name is prefixed by mbedtls to highlight
that the type is specific to Mbed TLS implementation
and not defined in the PSA Cryptography API
specification.
The svc in the type name stands for service as this
is the key identifier type from the point of view of
the service providing the Cryptography services.
The service can be completely provided by the present
library or partially in case of a multi-client service.
As a consequence rename as well:
. MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER to
MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER
. PSA_KEY_ID_INIT to MBEDTLS_SVC_KEY_ID_INIT
. PSA_KEY_FILE_GET_KEY_ID to MBEDTLS_SVC_KEY_ID_GET_KEY_ID
. psa_key_file_id_make to mbedtls_svc_key_id_make
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Define always psa_key_id_t as defined in the PSA
Cryptography API specification independently of
whether the MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER
configuration file is set or not.
As a consequence, get rid of `psa_app_key_id_t` that is
not needed anymore.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
The purpose of this commit and the following is for
psa_key_id_t to always be as defined by the PSA
Cryptography API specification.
Currently psa_key_id_t departs from its specification
definition when MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER
configuration flag is set. In that configuration, it is set
to be equal to psa_key_file_id_t which in that configuration
encodes an owner identifier along the key identifier.
Type psa_key_file_id_t was meant to be the key identifier type
used throughout the library code. If
MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER is set it
includes both a key and owner identifier, otherwise it is
equal to psa_key_id_t.
It has not been the key identifier type throughout the
library so far because when the PSA Cryptography
specification was developped the library Doxygen
documentation was used to generate the PSA Cryptography API
specification thus the need to use psa_key_id_t and not
psa_key_file_id_t.
As this constraint does not hold anymore, move
to psa_key_file_id_t as the key identifier type throughout
the library code.
By the way, this commit updates the key identifier
initialization in the tests to be compatible with a
composit key identifier. A psa_key_id_make()
inline function is introduced to initialize key
identifiers (composit ot not) at runtime.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
This way we can have a single call to mbedtls_platform_zeroize, which
saves a few bytes of code size.
Additionally, on my PC, I notice a significant speed improvement
(x86_64 build with MBEDTLS_AESNI_C disabled, gcc 5.4.0 -O3). I don't
have an explanation for that (I expected no measurable difference).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Remove the zeroization of a pointer variable in the AES block
functions. The code was valid but spurious and misleading since it
looked like a mistaken attempt to zeroize the pointed-to buffer.
Reported by Antonio de la Piedra, CEA Leti, France.
Note that we do not zeroize the buffer here because these are the
round keys, and they need to stay until all the blocks are processed.
They will be zeroized in mbedtls_aes_free().
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Minor updates to changelog for more concise wording and fixed styling
in other files as needed.
Signed-off-by: John Durkop <john.durkop@fermatsoftware.com>
Add guards in pk_wrap.c to ensure if ECDSA is not defined, errors
are returned.
Remove warnings in pk.c for unused variables.
Add new test (test_depends_pkalgs_psa) to all.sh to confirm
when USE_PSA_CRYPTO is defined that features are working properly.
Fix#3294
Signed-off-by: John Durkop <john.durkop@fermatsoftware.com>
Probably the `W[2 << MBEDTLS_MPI_WINDOW_SIZE]` notation is based on a transcription of 2**MBEDTLS_MPI_WINDOW_SIZE.
Signed-off-by: Daniel Otte <d.otte@wut.de>
Wrap multi-line string literals in parentheses
to prevent a Clang 12 -Wstring-concatenation warning
(activated by -Wall), which caused the build to fail.
Fixes https://github.com/ARMmbed/mbedtls/issues/3586
Signed-off-by: Guido Vranken <guidovranken@gmail.com>
Update copyright notices to newly added files since merge of original
PR #3546 "Update copyright notices to use Linux Foundation guidance".
Generated using the same script.
Signed-off-by: Dan Handley <dan.handley@arm.com>
* development:
Update copyright notices to use Linux Foundation guidance
Undef ASSERT before defining it to ensure that no previous definition has sneaked in through included files.
Add ChangeLog entry for X.509 CN-type vulnerability
Improve documentation of cn in x509_crt_verify()
Fix comparison between different name types
Add test: DNS names should not match IP addresses
Remove obsolete buildbot reference in compat.sh
Fix misuse of printf in shell script
Fix added proxy command when IPv6 is used
Simplify test syntax
Fix logic error in setting client port
ssl-opt.sh: include test name in log files
ssl-opt.sh: remove old buildbot-specific condition
ssl-opt.sh: add proxy to all DTLS tests
Signed-off-by: Dan Handley <dan.handley@arm.com>
Currently the new component in all.sh fails because
mbedtls_ssl_cf_memcpy_offset() is not actually constant flow - this is on
purpose to be able to verify that the new test works.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
mbedtls_md_setup() allocates a hash-specific context and then, if
requested, an extra HMAC context. If the second allocation failed, the
hash context was not freed.
Fix this by ensuring that the mbedtls_md_context_t object is always in
a consistent state, in particular, that the md_info field is always
set. For robustness, ensure that the object is in a consistent state
even on errors (other than BAD_INPUT_DATA if the object was not in a
consistent state on entry).
Fix#3486
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The tests are supposed to be failing now (in all.sh component
test_memsan_constant_flow), but they don't as apparently MemSan doesn't
complain when the src argument of memcpy() is uninitialized, see
https://github.com/google/sanitizers/issues/1296
The next commit will add an option to test constant flow with valgrind, which
will hopefully correctly flag the current non-constant-flow implementation.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This paves the way for a constant-flow implementation of HMAC checking, by
making sure that the comparison happens at a constant address. The missing
step is obviously to copy the HMAC from the secret offset to this temporary
buffer with constant flow, which will be done in the next few commits.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
As a result, the copyright of contributors other than Arm is now
acknowledged, and the years of publishing are no longer tracked in the
source files.
Also remove the now-redundant lines declaring that the files are part of
MbedTLS.
This commit was generated using the following script:
# ========================
#!/bin/sh
# Find files
find '(' -path './.git' -o -path './3rdparty' ')' -prune -o -type f -print | xargs sed -bi '
# Replace copyright attribution line
s/Copyright.*Arm.*/Copyright The Mbed TLS Contributors/I
# Remove redundant declaration and the preceding line
$!N
/This file is part of Mbed TLS/Id
P
D
'
# ========================
Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
RFC5280 does not state that the `revocationDate` should be checked.
In addition, when no time source is available (i.e., when MBEDTLS_HAVE_TIME_DATE is not defined), `mbedtls_x509_time_is_past` always returns 0. This results in the CRL not being checked at all.
https://tools.ietf.org/html/rfc5280
Signed-off-by: Raoul Strackx <raoul.strackx@fortanix.com>
The client previously reproted the offered ciphersuites through
their numerical identifier only, while the server reported them
through their name.
This commit modifies the debug output on client and server to
both use the format `ID (NAME)` for the ciphersuites.
* development: (55 commits)
Log change as bugfix
Add changelog entry
Clarify updates to the persistent state in storage
With multiple applicable transparent drivers, the order is unspecified
Minor clarifications
Give some examples of purpsoses of pure-software transparent driver
Fix typos
Add a link to the PSA API specification
Explain locations vs lifetimes
Initialize key pointer in ecdh to NULL
Add buffer zeroization when ecp_write_key fails
Simplified key slot deletion
Style fixes
Use arc4random_buf instead of rand on NetBSD
Apply review feedback
Update open question section about public key storage
Remove the paragraph about declaring application needs
Change driver persistent data to a callback interface
Rework and expand key management in opaque drivers
Fix typos and copypasta
...
Since it is being dereferenced by free on exit it should be inited to NULL.
Also added a small test that would trigger the issue.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
* return is treated as a function call
* space between opening and closing parentheses
* remove whiteline between assignment and checking of same variable
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
* No need to check for NULL before free'ing
* No need to reset variables that weren't touched
* Set output buffer to zero if key output fails
* Document internal functions and rearrange order of input arguments to
better match other functions.
* Clean up Montgomery fix to be less verbose code
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
PSA Crypto was checking the byte length of a to-be-imported public ECP key
against the expected length for Weierstrass keys, forgetting that
Curve25519/Curve448 exists.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Avoids stack-allocating a key slot during ECDH, and mock-attaching a
key to a key slot during key import.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
* No null-check before calling free
* Close memory leak
* No need for double check of privkey validity
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
* Allocate internal representation contexts on the heap (i.e. don't change
where they're being allocated)
* Unify load_xxx_representation in terms of allocation and init behaviour
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
* Updated wording
* Split out buffer allocation to a convenience function
* Moved variable declarations to beginning of their code block
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Now that both ECP and RSA keys are represented in export representation,
they can be treated more uniformly.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Change to on-demand loading of the internal representation when required
in order to call an mbed TLS cryptography API.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Change to on-demand loading of the internal representation when required
in order to call an mbed TLS cryptography API.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
In preparation for the implementation of the accelerator APIs. This is
ramping up to the goal of only storing the export representation in the
key slot, and not keeping the crypto implementation-specific representations
around.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Run some self-test both for a short Weierstrass curve and for a
Montgomery curve, if the build-time configuration includes a curve of
both types. Run both because there are significant differences in the
implementation.
The test data is suitable for Curve25519.
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
The constants used in the test worked with every supported curve
except secp192k1. For secp192k1, the "N-1" exponent was too large.
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
For some curves (semi-coincidentally, short Weierstrass curves), the
ECP module calculates some group parameters dynamically. Build the
code to calculate the parameters only if a relevant curve is enabled.
This fixes an unused function warning when building with only
Montgomery curves.
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
Replace the now-redundant internal curve type macros ECP_xxx by the
macros MBEDTLS_ECP__xxx_ENABLED which are declared in ecp.h.
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
Document that mbedtls_ecp_muladd and mbedtls_ecp_muladd_restartable
are only implemented on short Weierstrass curves.
Exclude these functions at build time if no short Weierstrass curve
is included in the build. Before, these functions failed to compile in
such a configuration.
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
In the entries (mbedtls_x509_crl_entry values) on the list constructed
by mbedtls_x509_crl_parse_der(), set entry->raw.tag to
(SEQUENCE | CONSTRUCTED) rather than to the tag of the first ASN.1
element of the entry (which happens to be the tag of the serial
number, so INTEGER or INTEGER | CONTEXT_SPECIFIC). This is doesn't
really matter in practice (and in particular the value is never used
in Mbed TLS itself), and isn't documented, but at least it's
consistent with how mbedtls_x509_buf is normally used.
The primary importance of this change is that the old code tried to
access the tag of the first element of the entry even when the entry
happened to be empty. If the entry was empty and not followed by
anything else in the CRL, this could cause a read 1 byte after the end
of the buffer containing the CRL.
The test case "X509 CRL ASN1 (TBSCertList, single empty entry at end)"
hit the problematic buffer overflow, which is detected with ASan.
Credit to OSS-Fuzz for detecting the problem.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
