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>
Make parse_identifiers less complex. Pylint was complaining that it had too
many local variables, and it had a point.
* Lift the constants identifier_regex and exclusion_lines to class
constants (renamed to uppercase because they're constants).
* Lift the per-file loop into a new function parse_identifiers_in_file.
No intended behavior change.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix cases like
```
/*short comment*/ /*long
comment */
int mbedtls_foo;
```
where the previous code thought that the second line started outside of a
comment and ended inside of a comment.
I believe that the new code strips comments correctly. It also strips string
literals, just in case.
Fixes#5191.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This is technically an API break according to the unwritten rules of API
compatibility for Mbed TLS 2.x. However, it is very unlikely to affect any
realistic application, with the possible exception of applications that
define a global constant of type mbedtls_ssl_config.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This slightly increases the RAM consumption per context, but saves code
size on architectures with an instruction for direct byte access (which is
most of them).
Although this is technically an API break, in practice, a realistic
application won't break: it would have had to bypass API functions and rely
on the field size (e.g. relying on -1 == 1 in a 1-bit field).
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/ssl_cli.o: 19952 -> 19900 (diff: 52)
library/ssl_msg.o: 25810 -> 25798 (diff: 12)
library/ssl_srv.o: 22371 -> 22299 (diff: 72)
library/ssl_tls.o: 23274 -> 23038 (diff: 236)
Results (same architecture, config-suite-b.h + MBEDTLS_ECDH_LEGACY_CONTEXT +
MBEDTLS_ECP_RESTARTABLE):
library/ssl_cli.o: 2868 -> 2848 (diff: 20)
library/ssl_msg.o: 2916 -> 2924 (diff: -8)
library/ssl_srv.o: 3204 -> 3184 (diff: 20)
library/ssl_tls.o: 5860 -> 5756 (diff: 104)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Move small fields first so that more fields can be within the Arm Thumb
128-element direct access window.
The ordering in this commit is not based on field access frequency.
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/ssl_cli.o: 20104 -> 19952 (diff: 152)
library/ssl_msg.o: 25942 -> 25810 (diff: 132)
library/ssl_srv.o: 22467 -> 22371 (diff: 96)
library/ssl_tls.o: 23390 -> 23274 (diff: 116)
Results (same architecture, config-suite-b.h + MBEDTLS_ECDH_LEGACY_CONTEXT +
MBEDTLS_ECP_RESTARTABLE):
library/ssl_cli.o: 2928 -> 2868 (diff: 60)
library/ssl_msg.o: 2924 -> 2916 (diff: 8)
library/ssl_srv.o: 3232 -> 3204 (diff: 28)
library/ssl_tls.o: 5904 -> 5860 (diff: 44)
Signed-off-by: Lukasz Gniadzik <lukasz.gniadzik@mobica.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Reorder fields mbedtls_ssl_handshake_params in order to save code on Arm
Thumb builds. The general idea is to put often-used fields in the direct
access window of 128 elements from the beginning of the structure.
The reordering is a human selection based on a report of field offset and
use counts, and informed by measuring the code size with various
arrangements. Some notes:
* I moved most byte-sized fields at the beginning where they're sure to be
in the direct access window.
* I moved buffering earlier because it can be around the threshold depending
on the configuration, and it's accessed in a lot of places.
* I moved several fields, including update_checksum and friends, early so
that they're guaranteed to be in the early access window.
* I tried moving randbytes or premaster to the early access window, but
I couldn't find a placement which would save code size, presumably because
they're bumping too many other fields, and they're mostly accessed through
memcpy and friends which translates to instructions that don't have an
offset for free anyway.
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/ssl_cli.o: 20200 -> 20104 (diff: 96)
library/ssl_msg.o: 25978 -> 25942 (diff: 36)
library/ssl_srv.o: 22691 -> 22467 (diff: 224)
library/ssl_tls.o: 23570 -> 23390 (diff: 180)
Results (same architecture, config-suite-b.h + MBEDTLS_ECDH_LEGACY_CONTEXT +
MBEDTLS_ECP_RESTARTABLE):
library/ssl_cli.o: 3012 -> 2928 (diff: 84)
library/ssl_msg.o: 2932 -> 2924 (diff: 8)
library/ssl_srv.o: 3288 -> 3232 (diff: 56)
library/ssl_tls.o: 6032 -> 5904 (diff: 128)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Replace bitfields mbedtls_ssl_handshake_params by bytes. This saves some
code size, and since the bitfields weren't group, this doesn't increase the
RAM usage.
Replace several ints that only store values in the range 0..255 by uint8_t.
This can increase or decrease the code size depending on the architecture
and on how the field is used. I chose changes that save code size on Arm
Thumb builds and may potentially save more after field reordering.
Leave the bitfields in struct mbedtls_ssl_hs_buffer alone: replacing them by
uint8_t slightly increases the code size.
Results (arm-none-eabi-gcc 7.3.1, build_arm_none_eabi_gcc_m0plus build):
library/ssl_srv.o: 22735 -> 22691 (diff: 44)
library/ssl_tls.o: 23566 -> 23570 (diff: -4)
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_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>
