psa_key_derivation_output_key: prioritize BAD_STATE over NOT_PERMITTED
If psa_key_derivation_output_key() is called on an operation which hasn't been
set up or which has been aborted, return PSA_ERROR_BAD_STATE. Only return
PSA_ERROR_NOT_PERMITTED if the operation state is ok for
psa_key_derivation_input_bytes() or psa_key_derivation_output_bytes() but not
ok to output a key.
Ideally psa_key_derivation_output_key() would return PSA_ERROR_NOT_PERMITTED
only when psa_key_derivation_output_bytes() is possible, but this is clumsier
to implement.
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Move fields around to have fewer accesses outside the 128-element Thumb
direct access window.
In psa_crypto.c's global_data, put the state fields first (-20).
In psa_crypto_slot_management.c's global_data, keep the key slots first
(otherwise it's +24).
In mbedtls_psa_random_context_t, swapping entropy and drbg makes no
difference (at least when the DRBG is mbedtls_ctr_drbg_context).
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/psa_crypto.o: 16166 -> 16146 (diff: 20)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Move fields around to have fewer accesses outside the 128-element Thumb
direct access window.
In psa_hkdf_key_derivation_t, move the large fields (output_block, prk,
hmac) after the state bit-fields. Experimentally, it's slightly better
to put hmac last.
In aead_operation_t, tag_length was outside the window. The details depend
on the sizes of contexts included in ctx. Make the large ctx be the last
field.
In mbedtls_psa_hmac_operation_t, the opad field is outside the window when
SHA-512 is enabled. Moving opad before hash_ctx only saves 4 bytes and made
the structure clumsy, so I left it alone.
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/psa_crypto.o: 16246 -> 16166 (diff: 80)
library/psa_crypto_aead.o: 952 -> 928 (diff: 24)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This comment is about how the functions are implemented, not about their
public interface, so it doesn't belong in the header file.
It applies to everything in constant_time.c so moved there.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
The equality checker functions always return 0 or 1 value,
thus the type of return value can be the same dispite of the
size of the parameters.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
Tne unpadding part of `mbedtls_rsa_rsaes_pkcs1_v15_decrypt` function is
contant-time therefore it moved to a separate function to be prepared
for moving to the contant-time module.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
Add a constant-time function with size_t parameter for choosing
between two integer values, like the ?: ternary operator.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
All function declaration provided by ssl_invasive.h is needed only for
testing purposes and all of them are provided by constant_time.h as well.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
Elinimate macros defined by modules locally in the functions that are
moving to the new constant-time module.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
There were multiple functions called mbedtls_cf_size_bool_eq. They had exactly
the same behavior, so move the one in bignum.c and remove the other.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
This is a workaround for an issue with mkstemp() in older MinGW releases that
causes simultaneous creation of .a files in the same directory to fail.
Fixes#5146
Signed-off-by: Tom Cosgrove <tom.cosgrove@arm.com>
The makefile build specifies -L. -lmbedx509 -lmbedcrypto flags first,
and only then object files referencing symbols from those libraries.
In this order the linker will not add the linked libraries to the
DT_NEEDED section because they are not referenced yet (at least that
happens for me on ubuntu 20.04 with the default gnu compiler tools).
By first specifying the object files and then the linked libraries, we
do end up with libmbedx509 and libmbedcrypto in the DT_NEEDED sections.
This way running dlopen(...) on libmedtls.so just works.
Note that the CMake build does this by default.
Signed-off-by: Harmen Stoppels <harmenstoppels@gmail.com>
The current definition of PSA_ALG_IS_HASH_AND_SIGN includes
PSA_ALG_RSA_PKCS1V15_SIGN_RAW and PSA_ALG_ECDSA_ANY, which don't strictly
follow the hash-and-sign paradigm: the algorithm does not encode a hash
algorithm that is applied prior to the signature step. The definition in
fact encompasses what can be used with psa_sign_hash/psa_verify_hash, so
it's the correct definition for PSA_ALG_IS_SIGN_HASH. Therefore this commit
moves definition of PSA_ALG_IS_HASH_AND_SIGN to PSA_ALG_IS_SIGN_HASH, and
replace the definition of PSA_ALG_IS_HASH_AND_SIGN by a correct one (based
on PSA_ALG_IS_SIGN_HASH, excluding the algorithms where the pre-signature
step isn't to apply the hash encoded in the algorithm).
In the definition of PSA_ALG_SIGN_GET_HASH, keep the condition for a nonzero
output to be PSA_ALG_IS_HASH_AND_SIGN.
Everywhere else in the code base (definition of PSA_ALG_IS_SIGN_MESSAGE, and
every use of PSA_ALG_IS_HASH_AND_SIGN outside of crypto_values.h), we meant
PSA_ALG_IS_SIGN_HASH where we wrote PSA_ALG_IS_HASH_AND_SIGN, so do a
global replacement.
```
git grep -l IS_HASH_AND_SIGN ':!include/psa/crypto_values.h' | xargs perl -i -pe 's/ALG_IS_HASH_AND_SIGN/ALG_IS_SIGN_HASH/g'
```
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
PSA_ALG_RSA_PSS algorithm now accepts only the same salt length for
verification that it produces when signing, as documented.
Fixes#4946.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
To test c <= high, instead of testing the sign of (high + 1) - c, negate the
sign of high - c (as we're doing for c - low). This is a little easier to
read and shaves 2 instructions off the arm thumb build with
arm-none-eabi-gcc 7.3.1.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
n was used for two different purposes. Give it a different name the second
time. This does not seem to change the generated code when compiling with
optimization for size or performance.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Instead of doing constant-flow table lookup, which requires 64 memory loads
for each lookup into a 64-entry table, do a range-based calculation, which
requires more CPU instructions per range but there are only 5 ranges.
I expect a significant performance gain (although smaller than for decoding
since the encoding table is half the size), but I haven't measured. Code
size is slightly smaller.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Document what each local variable does when it isn't obvious from the name.
Don't reuse a variable for different purposes.
This commit has very little impact on the generated code (same code size on
a sample Thumb build), although it does fix a theoretical bug that 2^32
spaces inside a line would be ignored instead of treated as an error.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Instead of doing constant-flow table lookup, which requires 128 memory loads
for each lookup into a 128-entry table, do a range-based calculation, which
requires more CPU instructions per range but there are only 5 ranges.
Experimentally, this is ~12x faster on my PC (based on
programs/x509/load_roots). The code is slightly smaller, too.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Base64 decoding uses equality comparison tests for characters that don't
leak information about the content of the data other than its length, such
as whitespace. Do this with '=' as well, since it only reveals information
about the length. This way the table lookup can focus on character validity
and decoding value.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Use the encoding from an upcoming version of the specification.
Add as much (or as little) testing as is currently present for Camellia.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This macro is introduced here for use in deprecated functions. It may also
be useful in user code, so it is in a public header.
Signed-off-by: Mateusz Starzyk <mateusz.starzyk@mobica.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Declare all AES and DES functions that return int as needing to have
their result checked, and do check the result in our code.
A DES or AES block operation can fail in alternative implementations of
mbedtls_internal_aes_encrypt() (under MBEDTLS_AES_ENCRYPT_ALT),
mbedtls_internal_aes_decrypt() (under MBEDTLS_AES_DECRYPT_ALT),
mbedtls_des_crypt_ecb() (under MBEDTLS_DES_CRYPT_ECB_ALT),
mbedtls_des3_crypt_ecb() (under MBEDTLS_DES3_CRYPT_ECB_ALT).
A failure can happen if the accelerator peripheral is in a bad state.
Several block modes were not catching the error.
This commit does the following code changes, grouped together to avoid
having an intermediate commit where the build fails:
* Add MBEDTLS_CHECK_RETURN to all functions returning int in aes.h and des.h.
* Fix all places where this causes a GCC warning, indicating that our code
was not properly checking the result of an AES operation:
* In library code: on failure, goto exit and return ret.
* In pkey programs: goto exit.
* In the benchmark program: exit (not ideal since there's no error
message, but it's what the code currently does for failures).
* In test code: TEST_ASSERT.
* Changelog entry.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This parameter was set but not used, which was pointless. Clang 14 detects
this and legitimately complains.
Remove the parameter. This is an internal function, only called once. The
caller already has a sufficient check on the output buffer size which
applies in more cases, so there is no real gain in robustness in adding the
same check inside the internal function.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The psa_open_key API depends on MBEDTLS_PSA_CRYPTO_STORAGE_C.
This is unnecessary for builtin keys and so is fixed.
Updated an open_fail test vector keeping with the same.
Signed-off-by: Archana <archana.madhavan@silabs.com>
Warning reported with IAR compiler:
"mbedtls\library\pkparse.c",1167 Warning[Pe550]: variable "ret" was set but never used
Signed-off-by: Kenneth Soerensen <knnthsrnsn@gmail.com>
exchange groups of the byte reading macros with MBEDTLS_PUT_UINTxyz
and then shift the pointer afterwards. Easier to read as you can
see how big the data is that you are putting in, and in the case of
UINT32 AND UINT64 it saves some vertical space.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
