The function reset_checksum() can be called more than once with the same
handshake context (this happens with DTLS clients, and perhaps in other
cases as well). When that happens, we need to free the old MD contexts
before setting them up again.
Note: the PSA path was already doing the right thing by calling abort,
we just needed to do the same on the MD path.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
With the introduction of #7047, ssl_tls.c uses
mbedtls_md_error_from_psa. This complicates
the dependencies for compiling in psa_to_md_errors,
since now these should be ifdeffed also by
MBEDTLS_USE_PSA_CRYPTO followed by a series of or'ed
MBEDTLS_HAS_ALG_SHA_XXX_VIA_MD_OR_PSA_BASED_ON_USE_PSA.
Since this mechanism will be removed soon, we can simplify it to
just MBEDTLS_USE_PSA_CRYPTO.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
GCC 4.6+ complains if a deprecated function calls another.
Working around this universally would require a lot of
preprocessing, this seems to be an easier solution.
Copy mbedtls_pk_error_from_psa code without duplicates
instead of calling the function.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Move all error translation utilities to psa_util.c.
Introduce macros and functions to avoid having
a local copy of the error translating function in
each place.
Identify overlapping errors and introduce a
generic function.
Provide a single macro for all error translations
(unless one file needs a couple of different ones).
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Changes for interruptible {sign|verify} hash were not merged at the time of the
previous clang 15 /retval fixes, thus this fixes code added at that time.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
The first round of carry reduction can not generate a carry thus the
secound round is not needed. The comments illustrating when the
carry is 1. The reduction is simmetric so the case when the carry is
-1 is similar.
The illustration is trying to calculate the input value starting with
setting the carry to 1 before the second round of the carry reduction.
It calculates backwords and tries to determine the value range of
each word. It ends up with a contradiction that A10 must have the
value of 0 and UINT32_MAX.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
Move the global variable to the PSA layer, and just set that when calling PSA
level functions.
Move the internal ecp set to before each ecp call.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
The name sha512 might have made sense when it was an
mbedtls_sha512_context, but now it's weird to see things like
mbedtls_md_setup(&sha512, ...MBEDTLS_MD_SHA384...);
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
X.509 already depends on MD_C || USE_PSA_CRYPTO, and this is for the
!USE_PSA_CRYPTO branch, so we're free to use MD.
This change supports our ability to use MBEDTLS_MD_CAN_xxx macros
everywhere in the future, once they have been introduced.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
There's little reason for accessing the hash implementation's internal
state, its output contains most of the same information.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Restore same PSK length enforcement in
conf_psk and set_hs_psk, whether the
negotiated protocol is TLS 1.2 or TLS 1.3.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Change name and document to ensure suitability only for "tags" is explicit. Add
support for output size of zero in PSA_SUCCESS case.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Also in the core part change stage to computation after return from psa_driver_wrapper_pake_setup() regardless of the result. At this point driver context is active even if init has failed.
Additionally handle deallocation of password on failure in mbedtls_psa_pake_setup(). The plan was to handle deallocation on core level by calling abort on failure.
Unfortunately in this case when mbedtls_psa_pake_setup() fails with an unsupported result the built-in implementation is executed (if available) and it will reallocate the password leading to the memory leak.
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
Adapt pake test (passing NULL buffers is not allowed).
Passing the null buffer to psa_pake_output results in a hard fault.
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
In the future key attributes will be available for opaque driver via psa_crypto_driver_pake_get_password_key().
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
Driver is now responsible for creating its own copy of the password in the setup function.
After calling pake setup driver entry point core frees memory for password.
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
- Add `alg` and `computation_stage` to `psa_pake_operation_s`.
Now when logic is moved to core information about `alg` is required.
`computation_stage` is a structure that provides a union of computation stages for pake algorithms.
- Move the jpake operation logic from driver to core. This requires changing driver entry points for `psa_pake_output`/`psa_pake_input` functions and adding a `computation_stage` parameter. I'm not sure if this solution is correct. Now the driver can check the current computation stage and perform some action. For jpake drivers `step` parameter is now not used, but I think it needs to stay as it might be needed for other pake algorithms.
- Removed test that seems to be redundant as we can't be sure that operation is aborted after failure.
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
psa_get_and_lock_key_slot_with_policy() becomes public temporarily as part of:
https://github.com/Mbed-TLS/mbedtls/pull/6608
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
Also while at it, fix debug level for existing DEBUG_RET: errors should
always be level 1.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Found by depends.py MBEDTLS_SHA512_C
In principle, the case where neither SHA-256 nor SHA-384 are available
should never occur, as both TLS 1.2 and TLS 1.3 depend on one of those
being defined. However for now dependencies for TLS 1.2 are not as tight
as they should be; this will be fixed later and is tracked as #6441.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
That's the last family of functions. All calls to mbedtls_sha* and
psa_hash_* in library/ssl_tls.c are now checked for errors.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
On top on some calls not being checked, the PSA path was missing a call
to abort() on errors.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
A few functions were changed from returning void to returning int three
commits ago. Make sure their callers check the return values.
This commits was basically a matter of declaring newly-int-returning
functions MBEDTLS_CHECK_RETURN_CRITICAL and then fixing the resulting
warnings. A few functions had to be made int in the process; they were
applied the same process as well.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This function was manually resetting just the hash that would be used;
it's simpler to just call the function that resets all hashes. This also
avoids calling low-level code from TLS 1.3.
While at it, remove the guards about SHA-256 || SHA-384 that were around
update_checksum, as they are redundant: update_checksum already has
appropriate guards (and TLS 1.3 already depends on one of those tow
hashes being present anyway).
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This part can fail, so it shouldn't be intermixed with the part that
can't fail and is there to ensure all structures are in a clean state,
should any error happen.
Fortunately, the part that should be split out already had a function
doing it: reset_checksum. Also, handshake_params_init had only one
calling site to update.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
There are three family of functions: update_checksum, calc_verify,
calc_finished, that perform hashing operations and were returning void
so far. This is not correct, as hashing functions can return errors (for
example, on hardware failure when accelerated). Change them to return
int.
This commit just changes the types: for now the functions always return
0, and their return value is not checked; this will be fixed in the
next few commits.
There is a related function in TLS 1.3,
mbedtls_ssl_reset_transcript_for_hrr, which also handles hashes, and
already returns int but does not correctly check for errors from hashing
functions so far, it will also be handled in the next few commits.
There's a special case with handshake_params_init: _init functions
should return void, so we'll need to split out the part that can return
errors, see the next commit.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This patch introduces the following changes:
* Documentation for `mbedtls_ecp_modulus_setup()`
moved to `ecp_invasive.h`.
* Added invalid modulus selector `MBEDTLS_ECP_MOD_NONE`.
* Adjusted negative tests to use invalid selectors.
* Reworded documentation.
Signed-off-by: Minos Galanakis <minos.galanakis@arm.com>
This patch introduces a new static method, responsible
for automatically initialising an modulus structure,
based on the curve id and a modulus type selector.
Signed-off-by: Minos Galanakis <minos.galanakis@arm.com>
MBEDTLS_PSK_MAX_LEN main purpose is to determine
a miximum size for the TLS 1.2 pre-master secret.
This is not relevant to TLS 1.3 thus disable in
TLS 1.3 case the check against MBEDTLS_PSK_MAX_LEN
when setting during the handshake the PSK through
mbedtls_ssl_set_hs_psk(). This fixes the session
resumption with 384 bits PSKs when MBEDTLS_PSK_MAX_LEN
is smaller than that.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Check ciphersuite list length parity once,
mainly to enable the possibility of getting
out of the loop of the ciphersuites whenever
we want.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Previously calling get_num_ops more than once would have ended up with ops
getting double counted, and not calling inbetween completes would have ended up
with ops getting missed. Fix this by moving this to where the work is actually
done, and add tests for double calls to get_num_ops().
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
See docs/architecture/psa-migration/md-cipher-dispatch.md
Regarding testing, the no_md component was never very useful, as that's
not something people are likely to want to do: it was mostly useful as
executable documentation of what depends on MD. It's going to be even
less useful when more and more modules auto-enable MD_LIGHT or even
MD_C. So, recycle it to test the build with only MD_LIGHT, which is
something that might happen in practice, and is necessary to ensure that
the division is consistent.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
It was already marked as internal use only, and no longer used
internally. Also, it won't work when we dispatch to PSA.
Remove it before the MD_LIGHT split to avoid a corner case: it's
technically a hashing function, no HMAC or extra metadata, but we still
don't want it in MD_LIGHT really.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Truncate input hashes to curve private key size as that is all that is required
for the internal implementation.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
After moving the MPIs used to output from the operation into the complete
function, I failed to move the accompanying free as well.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Move the obfuscation of the internal library only returning a delta of ops done
into the driver wrapper, thus meaning driver wrapper and API call both return
absolute values of work done. Document the differences at the internal
implementation level.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
If an error occurs, calling any function on the same operation should return
PSA_ERROR_BAD_STATE, and we were not honouring that for all errors. Add extra
failure tests to try and ratify this.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
For sign and verify, the pointer passed in to the hash is not guaranteed to
remain valid inbetween calls, thus we need to store the hash in the
operation. Added a test to ensure this is the case.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Set the psa level global anyway, regardless of having a built in
implementation, to match the set function. Also, ensure that value returned
is the same as value passed in, irregardless of internal implementation
requirements.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Ensure that num_ops is cleared when manual abort is called, but obviously not
when an operation just completes, and test this.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Move to accumulate ops in context rather than attempting to read straight out
of structures due to structure ops getting reset per operation, and also
issues with _abort clearing internal data. Fix usage of size_t in structures
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Make public the versions of ECSDA sign and verify which return raw signatures
rather than returning ASN.1 encoded signatures, in order to use them for the
internal implemention of psa_sign/verify_hash_interruptible.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Apply the usual parameter name and align the local variables and
comments. This naming diverges from the standard notation, but this is
beneficial as our variable meanings diverge as well and the difference
can help avoiding confusion.
Signed-off-by: Janos Follath <janos.follath@arm.com>
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
It is not necessary to save the middle limb upfront as overwriting it is
the desired result: in the first step we are reducing modulo
2^{512+biL}.
Arguably, the original flow is more intuitive and easier to see the idea
behind it.
Signed-off-by: Janos Follath <janos.follath@arm.com>
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
The prototype calculated with wrong limb size and not taken into account
the overflow in the shared limb.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
OID subidentifiers are encoded as follow. For every byte:
* The top bit is 1 if there is another byte to come, 0 if this is the
last byte.
* The other 7 bits form 7 bits of the number. These groups of 7 are
concatenated together in big-endian order.
Overlong encodings are explicitly disallowed by the BER/DER/X690
specification. For example, the number 1 cannot be encoded as:
0x80 0x80 0x01
It must be encoded as:
0x01
Enforce this in Mbed TLS' OID DER-to-string parser.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
Pacify Clang >=15 which complained:
```
include/psa/crypto.h:91:23: error: empty paragraph passed to '\retval' command [-Werror,-Wdocumentation]
* \retval #PSA_SUCCESS
~~~~~~~~~~~~~~~~~~~^
```
This commit performs the following systematic replacement:
```
perl -i -0777 -p -e 's/([\\@])(retval +\S+)\n(?! *\*? *([^\n \\*\/]|\\[cp]\b))/$1$2 ${1}emptydescription\n/g' $(git ls-files '*.[hc]' '*.function' '*.jinja')
```
i.e. add an `\emptydescription` argument to `\retval` commands (or
`@retval`, which we don't normally used) that are followed by a single word,
unless the next line looks like it contains text which would be the
description.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The first 2 components of an OID are combined together into the same
subidentifier via the formula:
subidentifier = (component1 * 40) + component2
The current code extracts component1 and component2 using division and
modulo as one would expect. However, there is a subtlety in the
specification[1]:
>This packing of the first two object identifier components recognizes
>that only three values are allocated from the root node, and at most
>39 subsequent values from nodes reached by X = 0 and X = 1.
If the root node (component1) is 2, the subsequent node (component2)
may be greater than 38. For example, the following are real OIDs:
* 2.40.0.25, UPU standard S25
* 2.49.0.0.826.0, Met Office
* 2.999, Allocated example OID
This has 2 implications that the current parsing code does not take
account of:
1. The second component may be > 39, so (subidentifier % 40) is not
correct in all circumstances.
2. The first subidentifier (containing the first 2 components) may be
more than one byte long. Currently we assume it is just 1 byte.
Improve parsing code to deal with these cases correctly.
[1] Rec. ITU-T X.690 (02/2021), 8.19.4
Signed-off-by: David Horstmann <david.horstmann@arm.com>
IAR was warning that conditional execution could bypass initialisation of
variables, although those same variables were not used uninitialised.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Since only a single hash algorithm is currenlty supported, this avoids
having to perform hashing more than once.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
A CMS signature can have internal data, but mbedTLS does not support
verifying such signatures. Reject them during parsing.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Since only one content type (signed data) is supported, storing the
content type just wastes memory.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
The lstrlenW() function isn't available to UWP apps, and isn't necessary, since
when given -1, WideCharToMultiByte() will process the terminating null character
itself (and the length returned by the function includes this character).
Resolves#2994
Signed-off-by: Tom Cosgrove <tom.cosgrove@arm.com>
To better reflect what the code relies on, limit the headers that are
included when MBEDTLS_USE_PSA_CRYPTO is disabled. Also stop including
"pkwrite.h" when it is no longer needed.
Include "mbedlts/platform_util.h" unconditionally. It was only included for
RSA ALT but was also used for MBEDTLS_USE_PSA_CRYPTO (the code worked
because other headers include "mbedtls/platform_util.h").
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Under MBEDTLS_USE_PSA_CRYPTO, ecdsa_sign_wrap() was calling
mbedtls_pk_write_key_der() to write a private key in SEC1 format, only to
then extract the part that represents the private value which is what
psa_import_key() actually wants. Instead, call an mpi function to directly
get the private key in the desired format.
This slightly reduces the code size and stack usage, and removes a
dependency on pk_write.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Under MBEDTLS_USE_PSA_CRYPTO, ecdsa_verify_wrap() was calling
mbedtls_pk_write_pubkey() to write a public key in the form of a
subjectPublicKey, only to then extract the part that represents the EC
point which psa_import_key() actually wants. Instead, call an ecp
function to directly get the public key in the desired format (just the
point).
This slightly reduces the code size and stack usage, and removes a
dependency on pk_write.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Handshake should abort will illeagal parameter allert when
receiving early data extentions but the selected_identity
parsed from pre-share key isn't equal to 0.
Signed-off-by: Xiaokang Qian <xiaokang.qian@arm.com>
The contentInfo field of PKCS7 Signed Data structures can
optionally contain the content of the signature. Per RFC 2315
it can also contain any of the PKCS7 data types. Add test and
comments making it clear that the current implementation
only supports the DATA content type and the data must be empty.
Return codes should be clear whether content was invalid or
unsupported.
Identification and fix provided by:
- Demi Marie Obenour <demiobenour@gmail.com>
- Dave Rodgman <dave.rodgman@arm.com>
Signed-off-by: Nick Child <nick.child@ibm.com>
- ASN.1 parsing functions check that length don't exceed buffer bounds,
so checks `p + len > end` are redundant.
- If `p + len == end`, this is erroneous because we expect further fields,
which is automatically caught by the next ASN.1 parsing call.
Hence, the two branches handling `p + len >= end` in x509_get_other_name()
can be removed.
Further, zeroization of the `other_name` structure isn't necessary
because it's not confidential (and it's also not performed on other
error conditions in this function).
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
