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>
When the option is On, CMake will have rules to generate the generated
files using scripts etc. When the option is Off, CMake will assume the
files are available from the source tree; in that mode, it won't require
any extra tools (Perl for example) compared to when we committed the
files to git.
The intention is that users will never need to adjust this option:
- in the development branch (and features branches etc.) the option is
always On (development mode);
- in released tarballs, which include the generated files, we'll switch
the option to Off (release mode) in the same commit that re-adds the
generated files.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@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>
TLS1.3:add read ptr and handshake kex modes
CI merge job: only "Session resume using tickets, DTLS: openssl client" failed in one component thus CI can be considered as passed.
TLS 1.3: ServerHello: add utils functions used by ServerHello
Regarding the merge job, there was only one of the failure we currently encounter on almost all PR (Session resume using tickets, DTLS: openssl client test case see #5012) thus we can consider that this PR passed CI.
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>
Move GCM's update output buffer length verification
from PSA AEAD to the built-in implementation of the GCM.
Signed-off-by: Mateusz Starzyk <mateusz.starzyk@mobica.com>
The requirement of minimum 15 bytes for output buffer in
psa_aead_finish() and psa_aead_verify() does not apply
to the built-in implementation of the GCM.
Alternative implementations are expected to verify the
length of the provided output buffers and to return
the MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL in case the
buffer length is too small.
Signed-off-by: Mateusz Starzyk <mateusz.starzyk@mobica.com>
An initialization vector IV can have any number of bits between 1 and
2^64. So it should be filled to the lower 64-bit in the last step
when computing ghash.
Signed-off-by: openluopworld <luopengxq@gmail.com>
This function is one common function in generic file, get it from
the encrypted extension and submit one patch independently.
Signed-off-by: XiaokangQian <xiaokang.qian@arm.com>
When not using DEBUG_C, but using the DTLS CID feature -
a null pointer was accessed in ssl_tls.c.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
This option only gated an ability to set a callback,
but was deemed unnecessary as it was yet another define to
remember when writing tests, or test configurations. Fixes#4653.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Switch from using MBEDTLS_PSA_BUILTIN_ macros over to using PSA_WANT_
macros, as code was moved from the internal drivers to the PSA Core.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Nonce length checks are now being used in the oneshot AEAD code as well,
which passes variant algorithms, not the base version, so need to
convert to base if necessary.
Signed-off-by: Paul Elliott <paul.elliott@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-arm <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-arm <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-arm <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-arm <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-arm <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-arm <gabor.mezei@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>
Also fiixed the following merge problems:
crypto_struct.h : Added MBEDTLS_PRIVATE to psa_aead_operation_s
members (merge conflict)
psa_crypto_aead.c : Added ciphertext_length to mbedtls_gcm_finish
call (change of API during development)
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Add memory constraints to the aarch64 inline assembly in MULADDC_STOP.
This fixes an issue where Clang 12 and 13 were generating
non-functional code on aarch64 platforms. See #4962, #4943
for further details.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
Now that support for earlier version have been removed, we no longer
need to care about them.
Since TLS 1.3 is being gradually introduced, we might still need a
version check in some places - but here the function is called
ssl_tls12_populate_tranform() and TLS 1.3 has its own function
mbedtls_ssl_tls13_populate_transform(), so when this function is called
we just know we're using TLS 1.2.
Reviewer hint: use the -b option of git diff / git show
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@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>
An initialization vector IV can have any number of bits between 1 and
2^64. So it should be filled to the lower 64-bit in the last step
when computing ghash.
Signed-off-by: openluopworld <luopengxq@gmail.com>
An initialization vector IV can have any number of bits between 1 and
2^64. So it should be filled to the lower 64-bit in the last step
when computing ghash.
Signed-off-by: openluopworld <luopengxq@gmail.com>
An initialization vector IV can have any number of bits between 1 and
2^64. So it should be filled to the lower 64-bit in the last step
when computing ghash.
Signed-off-by: openluopworld <luopengxq@gmail.com>
MULADDC_CORE reads from (%%rsi) and writes to (%%rdi). This fragment is
repeated up to 16 times, and %%rsi and %%rdi are s and d on entry
respectively. Hence the complete asm statement reads 16 64-bit words
from memory starting at s, and writes 16 64-bit words starting at d.
Without any declaration of modified memory, Clang 12 and Clang 13 generated
non-working code for mbedtls_mpi_mod_exp. The constraints make the unit
tests pass with Clang 12.
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 target attributes for key copy could have key bits and
type zeroed. If so, they need to be overwritten/ inherited
from the source key.
This is now forcefully overwritten after validating the
optional attributes. As a result assigning attributes type
and bits after copy are no longer necessary.
Signed-off-by: Archana <archana.madhavan@silabs.com>
A minimal test driver extension is added to support
copy of opaque keys within the same location.
Test vector support is extended to cover opaque keys.
Signed-off-by: Archana <archana.madhavan@silabs.com>
The validation against key width and max key bits is extended to
all key types from the existing validation for only symmetric keys.
Signed-off-by: Archana <archana.madhavan@silabs.com>
-Add test driver support to import/export while wrapping keys
meant to be stored in the PSA core as opaque( emulating an
SE without storage ).
-Export validate_unstructured_key_bit_size as
psa_validate_unstructured_key_bit_size, thereby changing its scope.
-Improve the import/export test cases in test_suite_psa_crypto to also
cover opaque keys, thereby avoiding duplication.
Signed-off-by: Archana <archana.madhavan@silabs.com>
Create a new sizing function for determining the size required for key
storage based on the input key data.
This is required for key imports where the key length might need to be
derived from the data.
Signed-off-by: Archana <archana.madhavan@silabs.com>
The report of RSA public exponent for opaque keys is not
supported. Do not attempt to compute the RSA public
exponent of an RSA opaque key associated to a driver
implementing the new driver interface when
MBEDTLS_PSA_CRYPTO_SE_C is disabled.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Slot number key attribute is specific of dynamically
registered drivers and should thus not be computed/
returned in case of keys associated to drivers
implementing the new unified driver interface.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Clarify the purpose of
psa_get_and_lock_transparent_key_slot_with_policy() and
define it even when MBEDTLS_PSA_CRYPTO_SE_C is disabled
for the purpose of static drivers.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
cleanup `ssl_tls13_write_client_hello_body`, fix comments issues.
And move ciphersuites to separate function
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
This reverts commit cc88b34f7942f57ea0fd27ee4b3e29f49c91f10e.
It causes many test fail. It should be re-considered.
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
mbedtls_cipher_setkey takes an int argument. Cast explicitly, otherwise MSVC
complains.
Where possible, just stick to size_t.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.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>
Conflicts:
library/ccm.c
Conflict resolved by re-applying the MBEDTLS_BYTE_0 macro.
Conflict resolved by ignoring the MBEDTLS_PUT_UINT16_BE macro
used in development branch on the 'b' buffer, because the 'b'
buffer is removed in current branch.
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>
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>
Move common encrypt / decrypt code into common function, and roll in
previously refactored setup checks function, as this is now the only
place it is called.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
minor changes, such as improving the documentation for the byte reading
macros, and using MBEDTLS_PUT_UINT16_xy in place of byte reading
macro combinations
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
Replace the contents of MBEDTLS_PUT_UINTx_yz contained inconsitent
but similar/duplicate code to the MBEDTLS_BYTE_x macros. Therefore
the contents of the macros now utilise the byte reading macros.
MBEDTLS_PUT_UINT64_LE's written order was also not consitent with
the other PUT macros, so that was modified.
Documentation comment said LSB instead of MSB and that has also been
resolved.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
byte shifting opertations throughout library/ were only replaced with
the byte reading macros when an 0xff mask was being used.
The byte reading macros are now more widley used, however they have not
been used in all cases of a byte shift operation, as it detracted from
the immediate readability or otherwise did not seem appropriate.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
Copy over the GET/PUT_UINT64_LE/BE macros from aes.c and sha512.c
Add the MBEDTLS_ prefix to all 4 macros.
Modify the GET_UINT64 macros to no longer take a target variable
as a parameter, so when the macro function is called it must be
assigned to a variable in the same statement.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
The CHAR macros casted to an unsigned char which in this project
is garunteed to be 8 bits - the same as uint8_t (which BYTE casts
to) therefore, instances of CHAR have been swapped with BYTE and
the number of macros have been cut down
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
aria.c has a shift by 3 bytes, but does not use the 0xff masking.
aparently this is not a problem and it is tidier to use the maco.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
Imrpoved the descriptions of the macros and parameters and
changing the name of the MBEDTLS_PUT_UINT... macro parameters
to be more descriptive
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
These cast to an unsigned char rather than a uint8_t
like with MBEDTLS_BYTE_x
These save alot of space and will improve maintence by
replacing the appropriate code with MBEDTLS_CHAR_x
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
The GET macros used to write to a macro parameter, but now
they can be used to assign a value to the desired variable
rather than pass it in as an argument and have it modified
in the macro function.
Due to this MBEDTLS_BYTES_TO_U32_LE is the same as
MBEDTLS_GET_UINT32_LE and was there for replaced in the
appropriate files and removed from common.h
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
Added documenttion comments to common.h and removed the changelog
as it is not really necessary for refactoring.
Also modified a comment in aria.c to be clearer
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
Although these only appear in one file: psa_crypto_storage.c
it is tidy to give it the same prefix as the UINT32 macros
and to store them in the fame file
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
As per tests/scripts/check-names.sh, macros in
library/ header files should be prefixed with
MBEDTLS_
The macro functions in common.h where also indented
to comply with the same test
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
The use of the BYTE_x macro in nist_kw did not seem appropriate
in hind sight as it is working with a character array not an int
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
missed do-while around function-like macros (UINT32_BE and
UINT_LE macros) originally present in the indivdual files,
before being moved to common.h.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
32-bit integer manipulation macros (little edian):
GET_UINT32_LE and PUT_UINT32_LE appear in several
files in library/.
Removes duplicate code and save vertical
space the macro has been moved to common.h.
Improves maintainability.
Also provided brief comment in common.h for
BYTES_TO_U32_LE. comment/documentation will
probably need to be edited further for all
recent additions to library/common.h
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
32-bit integer manipulation macros (big edian):
GET_UINT32_BE and PUT_UINT32_BE appear in several
files in library/.
Removes duplicate code and save vertical
space the macro has been moved to common.h.
Improves maintainability.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
The macro BYTES_TO_U32_LE appears in poly1305.c and
chacha20.c.
Removes duplicate code and save vertical
space the macro has been moved to common.h.
Improves maintainability.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
To improve readability by saving horizontal and vertical space.
Removed unecessary & 0xFF.
Byte reading macros implemented in library/common.h, All files
containing "& 0xff" were modified.
Comments/Documentation not yet added to the macro definitions.
Fixes#4274
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
To keep consistent with ssl_{clien2t,server2}.
Change-Id: I08dbe47a3d9b778ba3acad283f608fef4e63c626
CustomizedGitHooks: yes
Signed-off-by: Jerry Yu <jerry.h.yu@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>
Add the missing nonce length checks (this function is being used by
oneshot functions as well as multipart, and thus all cipher suites are
being used) and cover the case where a NULL buffer gets passed in.
Extended the set nonce test to cover this.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Remove error clearing from ccm_starts() and ccm_set_lengths().
Add error check in ccm_update_ad(), ccm_update() and ccm_finish().
Signed-off-by: Mateusz Starzyk <mateusz.starzyk@mobica.com>
Move logic to ccm_starts, ccm_set_lengths, ccm_update_ad,
ccm_update and ccm_finish
Use separate variable to track context state.
Encode first block only if both mbedtls_ccm_starts() and
mbedtls_ccm_set_lengths() were called.
Signed-off-by: Mateusz Starzyk <mateusz.starzyk@mobica.com>
Base on version config, `handshack_{clinet,server}_step`
will call different step function. TLS1.3 features will
be gradully added base on it.
And a new test cases is added to make sure it reports
`feature is not available`.
Change-Id: I4f0e36cb610f5aa59f97910fb8204bfbf2825949
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
Check configuration parameter in structure setup
function to make sure the config data is available
and valid.
Current implementation checks the version config.
Available version configs are
- tls1_3 only
- tls1_2 only
issues: #4844
Change-Id: Ia762bd3d817440ae130b45f19b80a2868afae924
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
The helper `mbedtls_ssl_write_handshake_msg` writes a handshake message
and updates the handshake transcript.
With TLS 1.3, we need finer control over the checksum: updating
at message granularity is not sufficient. To allow for manual maintenance
of the checksum in those cases, refine `mbedtls_ssl_write_handshake_msg()`
into `mbedtls_ssl_write_handshake_msg_ext()` which takes a parameter
determining whether the checksum should be updated.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
- Improves readability
- Will be useful when we introduce MPS as an alternative msg layer.
- Will be useful when we need to reset the messaging layer upon
receipt of a HelloRetryRequest in TLS 1.3.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
