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.
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>
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>
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>
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>
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>
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>
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>
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>
The AEAD additional data (AAD) is computed differently in TLS 1.3
compared to TLS 1.2, but this change hasn't yet been reflected in
the codee, rendering the current implementation of
```
mbedtls_ssl_{encrypt,decrypt}_buf()
```
not standard compliant.
This commit fixes this by adjusting the AAD extraction function
ssl_extract_add_data_from_record() and its call-sites.
Please see the documentation of the code for an explanation
of how the AAD has changed from TLS 1.2 to TLS 1.3.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
The sequence of calls starts-update-starts-update-finish is not a
guaranteed valid way to abort an operation and start a new one. Our
software implementation just happens to support it, but alt
implementations may very well not support it.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Handling the receipt of a handshake record after the initial handshake
requires non-trivial logic depending on the protocol version and the
endpoint. This logic is currently embedded in mbedtls_ssl_read().
With the introduction of support for [D]TLS 1.3, the logic will become
even more complex, since [D]TLS 1.3 drops support for renegotiation --
which in [D]TLS 1.2 is the main purpose of post-handshake handshake
messages -- but instead introduces numerous other post-handshake
handshake messages.
In order to pave the way for those changes, this commit improves
readability and maintainability of mbedtls_ssl_read() by moving
the TLS <=1.2 logic for handling post-handshake handshake messages
into a separate helper function ssl_handle_hs_message_post_handshake().
The logic of the code is entirely unchanged.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
MinGW and older windows compilers cannot cope with %zu or %lld (there is
a workaround for MinGW, but it involves linking more code, there is no
workaround for Windows compilers prior to 2013). Attempt to work around
this by defining printf specifiers for size_t per platform for the
compilers that cannot use the C99 specifiers.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Fixes for printf format specifiers, where they have been flagged as
invalid sizes by coverity, and new build flags to enable catching these
errors when building using CMake. Note that this patch uses %zu, which
requires C99 or later.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Simple find and replace using `#include (<|")mbedtls/(.*)_internal.h(>|")`
and `#include $1$2_internal.h$3`.
Also re-generated visualc files by running
`scripts/generate_visualc_files.pl`.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
The field `cur_out_ctr` in the SSL context keeps track of the
record sequence number for the next outgoing record.
For TLS, this sequence number is implicit and not transmitted
on the wire, while for DTLS, it's part of of the record header.
For DTLS, the position of the record sequence number of the next
outgoing record in that record's header is tracked in the pointer
`out_ctr`. This pointer moves forward along with other pointers
such as `out_hdr` or `out_msg` within the outgoing data buffer
`out_buf` as multiple records are written in the same datagram.
For TLS, the `out_ctr` pointer is logically superfluous, but
for some reason, we're still maintaining it by having it point
to the 8 Bytes prior to the header of the next outgoing record,
and always copying `cur_out_ctr` to this position prior to
encrypting an outgoing record.
After a record has been prepared for writing in `ssl_write_record()`,
the `out_xxx` pointers (except for `out_buf`, which is static),
are shifted forward so that they point to the header and content
of the next outgoing record. This is used only in DTLS in order
to stack multiple records into a single datagram, but the shifting
is happening for TLS as well. However, it has little effect in TLS
because we're always flushing immediately after writing, and afterwards
reset the `out_xxx` pointers.
While the present code works as-is, it is wrong to shift `out_ctr`
in the case of TLS, because it makes `out_ctr` point to the last 8
Bytes of the ciphertext of the last outgoing record. Should we ever
aim to prepare more than one protected record in `out_buf` before
dispatching it to the underlying transport, the superfluous copying
of `cur_out_ctr` to `out_buf` will corrupt the last 8 bytes of the
last record.
This commit aims to fix this problem in the minimal possible way,
by simply not shifting `out_ctr` after a record has been written.
It does deliberately not attempt to remove `out_ctr` for TLS altogether,
because any change in the messaging layer is hard to review, and
we're going to replace it soon anyhow.
The shifting happens in the helper routine mbedtls_ssl_update_out_pointers,
which assumed correctness of `out_hdr` for the beginning of the record header
of the next outgoing record, and derives the other `out_xxx` variables.
We remove the update of `out_ctr` from this function in the case of TLS,
and instead move the proper initialization of `out_ctr` to
`out_buf == initial_out_hdr - 8` to the function
mbedtls_ssl_reset_in_out_pointers().
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
Mbed TLS requires users of DTLS to configure timer callbacks
needed to implement the wait-and-retransmit logic of DTLS.
Previously, the presence of these timer callbacks was checked
at every invocation of `mbedtls_ssl_fetch_input()`, so lowest
layer of the messaging stack interfacing with the underlying
transport.
This commit removes this recurring check and instead checks the
presence of timers once at the beginning of the handshake.
The main rationale for this change is that it is a step towards
separating the various layers of the messaging stack more cleanly:
datagram layer, record layer, message layer, retransmission layer.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
According to https://www.bearssl.org/ctmul.html even single-precision
multiplication is not constant-time on some older platforms.
An added benefit of the new code is that it removes the somewhat mysterious
constant 0x1ff - which was selected because at that point the maximum value of
padlen was 256. The new code is perhaps a bit more readable for that reason.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The previous code used comparison operators >= and == that are quite likely to
be compiled to branches by some compilers on some architectures (with some
optimisation levels).
For example, take the following function:
void old_update( size_t data_len, size_t *padlen )
{
*padlen *= ( data_len >= *padlen + 1 );
}
With Clang 3.8, let's compile it for the Arm v6-M architecture:
% clang --target=arm-none-eabi -march=armv6-m -Os foo.c -S -o - |
sed -n '/^old_update:$/,/\.size/p'
old_update:
.fnstart
@ BB#0:
.save {r4, lr}
push {r4, lr}
ldr r2, [r1]
adds r4, r2, #1
movs r3, #0
cmp r4, r0
bls .LBB0_2
@ BB#1:
mov r2, r3
.LBB0_2:
str r2, [r1]
pop {r4, pc}
.Lfunc_end0:
.size old_update, .Lfunc_end0-old_update
We can see an unbalanced secret-dependant branch, resulting in a total
execution time depends on the value of the secret (here padlen) in a
straightforward way.
The new version, based on bit operations, doesn't have this issue:
new_update:
.fnstart
@ BB#0:
ldr r2, [r1]
subs r0, r0, #1
subs r0, r0, r2
asrs r0, r0, #31
bics r2, r0
str r2, [r1]
bx lr
.Lfunc_end1:
.size new_update, .Lfunc_end1-new_update
(As a bonus, it's smaller and uses less stack.)
While there's no formal guarantee that the version based on bit operations in
C won't be translated using branches by the compiler, experiments tend to show
that's the case [1], and it is commonly accepted knowledge in the practical
crypto community that if we want to sick to C, bit operations are the safest
bet [2].
[1] https://github.com/mpg/ct/blob/master/results
[2] https://github.com/veorq/cryptocoding
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@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>
