Commit Graph

19 Commits

Author SHA1 Message Date
Hanno Becker
17263803aa Simplify AEAD nonce derivation
This commit simplifies nonce derivation for AEAD based record protection
routines in the following way.

So far, code distinguished between the cases of GCM+CCM and ChachaPoly:

- In the case of GCM+CCM, the AEAD nonce is the concatentation
  of a 4-byte Fixed IV and a dynamically chosen 8-byte IV which is prepended
  to the record. In Mbed TLS, this is always chosen to be the record sequence
  number, but it need not to.

- In the case of ChaChaPoly, the AEAD nonce is derived as

    `( 12-byte Fixed IV ) XOR ( 0 || 8-byte dynamic IV == record seq nr )`

  and the dynamically chosen IV is no longer prepended to the record.

This commit removes this distinction by always computing the record nonce
via the formula

  `IV == ( Fixed IV || 0 ) XOR ( 0 || Dynamic IV )`

The ChaChaPoly case is recovered in case `Len(Fixed IV) == Len(IV)`, and
GCM+CCM is recovered when `Len(IV) == Len(Fixed IV) + Len(Dynamic IV)`.

Moreover, a getter stub `ssl_transform_aead_dynamic_iv_is_explicit()`
is introduced which infers from a transform whether the dynamically
chosen part of the IV is explicit, which in the current implementation
of `mbedtls_ssl_transform` can be derived from the helper field
`mbedtls_ssl_transform::fixed_ivlen`.

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
2020-05-28 10:32:23 +01:00
Hanno Becker
df8be226ba TLS record protection: Add helper function for nonce derivation
The computation of the per-record nonce for AEAD record protection
varies with the AEAD algorithm and the TLS version in use.
This commit introduces a helper function for the nonce computation
to ease readability of the quite monolithic record encrytion routine.

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
2020-05-28 10:32:23 +01:00
Hanno Becker
bd5ed1d11b TLS record protection: Add explicit IV after record protection.
The previous record protection code added the explicit part of the
record nonce prior to encrypting the record. This temporarily leaves
the record structure in the undesireable state that the data outsie
of the interval `rec->data_offset, .., rec->data_offset + rec->data_len`
has already been written.

This commit moves the addition of the explicit IV past record encryption.

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
2020-05-28 10:32:23 +01:00
Hanno Becker
1cb6c2a69d TLS record protection: Rewrite AAD setup and add case of TLS 1.3
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
2020-05-28 10:32:23 +01:00
Hanno Becker
9231340d71 Improve documentation of (D)TLSInnerPlaintext handling
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
2020-05-28 10:32:23 +01:00
Hanno Becker
ccc13d03c3 TLS 1.3: Implement TLSInnerPlaintext parsing/building
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
2020-05-28 10:32:23 +01:00
Hanno Becker
581bc1b908 Remove ref to CID from inner plaintext parsing/building functions
The internal functions

  `ssl_cid_{build/parse}_inner_plaintext()`

implement the TLSInnerPlaintext mechanism used by DTLS 1.2 + CID
in order to allow for flexible length padding and to protect the
true content type of a record.

This feature is also present in TLS 1.3 support for which is under
development. As a preparatory step towards sharing the code between
the case of DTLS 1.2 + CID and TLS 1.3, this commit renames

   `ssl_cid_{build/parse}_inner_plaintext()`

to

   `ssl_{build/parse}_inner_plaintext()`.

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
2020-05-28 10:32:23 +01:00
Jaeden Amero
66e21efe47
Merge pull request #3163 from AndrzejKurek/variable-buffers-renegotiation
Variable buffers & renegotiation - fixes
2020-04-09 12:11:02 +01:00
Andrzej Kurek
90c6e84a9c
Split the maximum fragment length into two - an input and output MFL
Since the server might want to have a different maximum fragment length
for the outgoing messages than the negotiated one - introduce a new way of
computing it. This commit also adds additional ssl-opt.sh tests ensuring
that the maximum fragment lengths are set as expected. 
mbedtls_ssl_get_max_frag_len() is now a deprecated function,
being an alias to mbedtls_ssl_get_output_max_frag_len(). The behaviour
of this function is the same as before.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-04-09 04:30:34 -04:00
Manuel Pégourié-Gonnard
243d70f2a5 Improve debug logging of client hard reconnect
The current logging was sub-standard, in particular there was no trace
whatsoever of the HelloVerifyRequest being sent. Now it's being logged with
the usual levels: 4 for full content, 2 return of f_send, 1 decision about
sending it (or taking other branches in the same function) because that's the
same level as state changes in the handshake, and also same as the "possible
client reconnect" message" to which it's the logical continuation (what are we
doing about it?).

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-31 12:33:57 +02:00
Manuel Pégourié-Gonnard
824655c837 Fix lack of cookie check on hard reconnect
Section 4.2.8 of RFC 6347 describes how to handle the case of a DTLS client
establishing a new connection using the same UDP quartet as an already active
connection, which we implement under the compile option
MBEDTLS_SSL_DLTS_CLIENT_PORT_REUSE. Relevant excerpts:

    [the server] MUST NOT destroy the existing
    association until the client has demonstrated reachability either by
    completing a cookie exchange or by completing a complete handshake
    including delivering a verifiable Finished message.
    [...]
    The reachability requirement prevents
    off-path/blind attackers from destroying associations merely by
    sending forged ClientHellos.

Our code chooses to use a cookie exchange for establishing reachability, but
unfortunately that check was effectively removed in a recent refactoring,
which changed what value ssl_handle_possible_reconnect() needs to return in
order for ssl_get_next_record() (introduced in that refactoring) to take the
proper action. Unfortunately, in addition to changing the value, the
refactoring also changed a return statement to an assignment to the ret
variable, causing the function to reach the code for a valid cookie, which
immediately destroys the existing association, effectively bypassing the
cookie verification.

This commit fixes that by immediately returning after sending a
HelloVerifyRequest when a ClientHello without a valid cookie is found. It also
updates the description of the function to reflect the new return value
convention (the refactoring updated the code but not the documentation).

The commit that changed the return value convention (and introduced the bug)
is 2fddd3765e, whose commit message explains the
change.

Note: this bug also indirectly caused the ssl-opt.sh test case "DTLS client
reconnect from same port: reconnect" to occasionally fail due to a race
condition between the reception of the ClientHello carrying a valid cookie and
the closure of the connection by the server after noticing the ClientHello
didn't carry a valid cookie after it incorrectly destroyed the previous
connection, that could cause that ClientHello to be invisible to the server
(if that message reaches the server just before it does `net_close()`). A
welcome side effect of this commit is to remove that race condition, as the
new connection will immediately start with a ClientHello carrying a valid
cookie in the SSL input buffer, so the server will not call `net_close()` and
not risk discarding a better ClientHello that arrived in the meantime.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-27 10:50:05 +01:00
Manuel Pégourié-Gonnard
21d1cbccda
Merge pull request #2262 from andresag01/iotssl-2544-deprecate-record-accel
Fix compilation failure when MBEDTLS_SSL_HW_RECORD_ACCEL is enabled
2020-03-16 10:37:16 +01:00
Darryl Green
b33cc7688e
Add I/O buffer length fields to mbedtls_ssl_context
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Signed-off-by: Darryl Green <darryl.green@arm.com>
2020-03-03 10:44:49 -05:00
Manuel Pégourié-Gonnard
e07bc20155 Fix compile errors with MBEDTLS_SSL_HW_RECORD_ACCEL
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-02-26 09:53:42 +01:00
Hanno Becker
9d062f9cd7 Move ssl_mac() from ssl_tls.c to ssl_msg.c 2020-02-07 11:38:03 +00:00
Hanno Becker
f1a3828ad8 Adapt preamble for newly created ssl_msg.c 2020-02-05 16:14:29 +00:00
Hanno Becker
af752d4bd6 Remove code from ssl_tls.c & ssl_msg.c to get disjoint functionality
This commit is the final step in separating the functionality of
what was originally ssl_tls.c into both ssl_tls.c and ssl_msg.c.

So far, ssl_msg.c has been created as an identical copy of ssl_tls.c.
For each block of code in these files, this commit removes it from
precisely one of the two files, depending on where the respective
functionality belongs.

The splitting separates the following functionalities:

1) An implementation of the TLS and DTLS messaging layer, that is,
   the record layer as well as the DTLS retransmission state machine.

   This is now contained in ssl_msg.c

2) Handshake parsing and writing functions shared between client and
   server (functions specific to either client or server are implemented
   in ssl_cli.c and ssl_srv.c, respectively).

   This is remains in ssl_tls.c.
2020-02-05 16:12:24 +00:00
Hanno Becker
6e7051af13 Add guarded ssl_msg.c to Makefiles
This commit adds the newly created copy ssl_msg.c of ssl_tls.c
to the build system but guards its content by an `#if 0 ... #endif`
preprocessor guard in order to avoid compilation failures resulting
from code duplication. This guard will be removed once the contents
of ssl_tls.c and ssl_msg.c have been made disjoint.
2020-02-05 16:10:40 +00:00
Hanno Becker
abd9cef520 Duplicate ssl_tls.c as ssl_tls_old.c and ssl_msg.c
This commit is the first in a series of commits aiming to split
the content of ssl_tls.c in two files ssl_tls.c and ssl_msg.c.

As a first step, this commit replaces ssl_tls.c by two identical
copies ssl_tls_old.c and ssl_msg.c. Even though the file
ssl_tls_old.c will subsequently be renamed back into ssl_tls.c,
this approach retains the git history in both files.
2020-02-05 16:08:41 +00:00