This for example lead to the following corner case bug:
The code attempted to piggy-back a Finished message at
the end of a datagram where precisely 12 bytes of payload
were still available. This lead to an empty Finished fragment
being sent, and when mbedtls_ssl_flight_transmit() was called
again, it believed that it was just starting to send the
Finished message, thereby calling ssl_swap_epochs() which
had already happened in the call sending the empty fragment.
Therefore, the second call would send the 'rest' of the
Finished message with wrong epoch.
This commit adds a public function
`mbedtls_ssl_conf_datagram_packing()`
that allows to allow / forbid the packing of multiple
records within a single datagram.
The `partial` argument is only used when DTLS and same port
client reconnect are enabled. This commit marks the variable
as unused if that's not the case.
If neither the maximum fragment length extension nor DTLS
are used, the SSL context argument is unnecessary as the
maximum payload length is hardcoded as MBEDTLS_SSL_MAX_CONTENT_LEN.
This commit finally enables datagram packing by modifying the
record preparation function ssl_write_record() to not always
calling mbedtls_ssl_flush_output().
The packing of multiple records within a single datagram works
by increasing the pointer `out_hdr` (pointing to the beginning
of the next outgoing record) within the datagram buffer, as
long as space is available and no flush was mandatory.
This commit does not yet change the code's behavior of always
flushing after preparing a record, but it introduces the logic
of increasing `out_hdr` after preparing the record, and resetting
it after the flush has been completed.
Previously, the record sequence number was incremented at the
end of each successful call to mbedtls_ssl_flush_output(),
which works as long as there is precisely one such call for
each outgoing record.
When packing multiple records into a single datagram, this
property is no longer true, and instead the increment of the
record sequence number must happen after the record has been
prepared, and not after it has been dispatched.
This commit moves the code for incrementing the record sequence
number from mbedtls_ssl_flush_output() to ssl_write_record().
This commit is another step towards supporting the packing of
multiple records within a single datagram.
Previously, the incremental outgoing record sequence number was
statically stored within the record buffer, at its final place
within the record header. This slightly increased efficiency
as it was not necessary to copy the sequence number when writing
outgoing records.
When allowing multiple records within a single datagram, it is
necessary to allow the position of the current record within the
datagram buffer to be flexible; in particular, there is no static
address for the record sequence number field within the record header.
This commit introduces an additional field `cur_out_ctr` within
the main SSL context structure `mbedtls_ssl_context` to keep track
of the outgoing record sequence number independent of the buffer used
for the current record / datagram. Whenever a new record is written,
this sequence number is copied to the the address `out_ctr` of the
sequence number header field within the current outgoing record.
The SSL/TLS module maintains a number of internally used pointers
`out_hdr`, `out_len`, `out_iv`, ..., indicating where to write the
various parts of the record header.
These pointers have to be kept in sync and sometimes need update:
Most notably, the `out_msg` pointer should always point to the
beginning of the record payload, and its offset from the pointer
`out_iv` pointing to the end of the record header is determined
by the length of the explicit IV used in the current record
protection mechanism.
This commit introduces functions deducing these pointers from
the pointers `out_hdr` / `in_hdr` to the beginning of the header
of the current outgoing / incoming record.
The flexibility gained by these functions will subsequently
be used to allow shifting of `out_hdr` for the purpose of
packing multiple records into a single datagram.
For now, just check that it causes us to fragment. More tests are coming in
follow-up commits to ensure we respect the exact value set, including when
renegotiating.
Note: no interop tests in ssl-opt.sh for now, as some of them make us run into
bugs in (the CI's default versions of) OpenSSL and GnuTLS, so interop tests
will be added later once the situation is clarified. <- TODO
This will allow fragmentation to always happen in the same place, always from
a buffer distinct from ssl->out_msg, and with the same way of resuming after
returning WANT_WRITE
- take advantage of the fact that we're only called for first send
- put all sanity checks at the top
- rename and constify shortcut variables
- improve comments
`mbedtls_ssl_get_record_expansion()` is supposed to return the maximum
difference between the size of a protected record and the size of the
encapsulated plaintext.
It had the following two bugs:
(1) It did not consider the new ChaChaPoly ciphersuites, returning
the error code #MBEDTLS_ERR_SSL_INTERNAL_ERROR in this case.
(2) It did not correctly estimate the maximum record expansion in case
of CBC ciphersuites in (D)TLS versions 1.1 and higher, in which
case the ciphertext is prefixed by an explicit IV.
This commit fixes both bugs.
The length to the debug message could conceivably leak through the time it
takes to print it, and that length would in turn reveal whether padding was
correct or not.
The basis for the Lucky 13 family of attacks is for an attacker to be able to
distinguish between (long) valid TLS-CBC padding and invalid TLS-CBC padding.
Since our code sets padlen = 0 for invalid padding, the length of the input to
the HMAC function, and the location where we read the MAC, give information
about that.
A local attacker could gain information about that by observing via a
cache attack whether the bytes at the end of the record (at the location of
would-be padding) have been read during MAC verification (computation +
comparison).
Let's make sure they're always read.
For the situation where the mbedTLS device has limited RAM, but the
other end of the connection doesn't support the max_fragment_length
extension. To be spec-compliant, mbedTLS has to keep a 16384 byte
incoming buffer. However the outgoing buffer can be made smaller without
breaking spec compliance, and we save some RAM.
See comments in include/mbedtls/config.h for some more details.
(The lower limit of outgoing buffer size is the buffer size used during
handshake/cert negotiation. As the handshake is half-duplex it might
even be possible to store this data in the "incoming" buffer during the
handshake, which would save even more RAM - but it would also be a lot
hackier and error-prone. I didn't really explore this possibility, but
thought I'd mention it here in case someone sees this later on a mission
to jam mbedTLS into an even tinier RAM footprint.)
As a protection against the Lucky Thirteen attack, the TLS code for
CBC decryption in encrypt-then-MAC mode performs extra MAC
calculations to compensate for variations in message size due to
padding. The amount of extra MAC calculation to perform was based on
the assumption that the bulk of the time is spent in processing
64-byte blocks, which is correct for most supported hashes but not for
SHA-384. Correct the amount of extra work for SHA-384 (and SHA-512
which is currently not used in TLS, and MD2 although no one should
care about that).
