If `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is not set, `mbedtls_ssl_session`
contains the digest of the peer's certificate for the sole purpose of
detecting a CRT change on renegotiation. Hence, it is not needed if
renegotiation is disabled.
This commit removes the `peer_cert_digest` fields (and friends) from
`mbedtls_ssl_session` if
`!MBEDTLS_SSL_KEEP_PEER_CERTIFICATE + !MBEDTLS_SSL_RENEGOTIATION`,
which is a sensible configuration for constrained devices.
Apart from straightforward replacements of
`if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)`
by
`if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) && \
defined(MBEDTLS_SSL_RENEGOTIATION)`,
there's one notable change: On the server-side, the CertificateVerify
parsing function is a no-op if the client hasn't sent a certificate.
So far, this was determined by either looking at the peer CRT or the
peer CRT digest in the SSL session structure (depending on the setting
of `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE`), which now no longer works if
`MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is unset. Instead, this function
now checks whether the temporary copy of the peer's public key within
the handshake structure is initialized or not (which is also a
beneficial simplification in its own right, because the pubkey is
all the function needs anyway).
We must dispatch between the peer's public key stored as part of
the peer's CRT in the current session structure (situation until
now, and future behaviour if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is
enabled), and the sole public key stored in the handshake structure
(new, if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is disabled).
We must dispatch between the peer's public key stored as part of
the peer's CRT in the current session structure (situation until
now, and future behaviour if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is
enabled), and the sole public key stored in the handshake structure
(new, if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is disabled).
We must dispatch between the peer's public key stored as part of
the peer's CRT in the current session structure (situation until
now, and future behaviour if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is
enabled), and the sole public key stored in the handshake structure
(new, if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is disabled).
mbedtls_ssl_parse_certificate() will fail if a ciphersuite requires
a certificate, but none is provided. While it is sensible to double-
check this, failure should be reported as an internal error and not
as an unexpected message.
This commit simplifies the client-side code for outgoing CertificateVerify
messages, and server-side code for outgoing CertificateRequest messages and
incoming CertificateVerify messages, through the use of the macro
`MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED`
indicating whether a ciphersuite allowing CertificateRequest messages
is enabled in the configuration, as well as the helper function
`mbedtls_ssl_ciphersuite_cert_req_allowed()`
indicating whether a particular ciphersuite allows CertificateRequest
messages.
These were already used in the client-side code to simplify the
parsing functions for CertificateRequest messages.
This commit handles occurrences of case 2 and 3 in the following list:
1. Some DTLS-specific code with no TLS-specific code (most frequent)
2. Some specific code for each protocol
3. Some TLS-specific code with no DTLS-specific code (least frequent)
Case 3 previously had a weird structure in that the TLS-specific code was
always present, but the if structure was conditional on DTLS being enabled.
This is changed by this commit to a more logical structure where both the code
and the test are conditional on TLS being enabled.
Case 2 doesn't require any change in the code structure in general. However,
there is one occurrence where the if/else structure is simplified to assigning
the result of a boolean operation, and one occurrence where I also noticed a
useless use of `ssl_ep_len()` in a TLS-specific branch, that I turned to the
constant 0 as it makes more sense.
Case 1 will be handled in the next commit, as it can easily be handled in an
automated way - only cases 2 and 3 (sometimes) required manual intervention.
The list of occurrences for cases 2 and 3 was established manually by looking
for occurrences of '= MBEDTLS_SSL_TRANSPORT_' in the code and manually
checking if there was a TLS-specific branch.
New sizes (see previous commit for the measuring script):
```
both
text data bss dec hex filename
1820 0 4 1824 720 debug.o (ex library/libmbedtls.a)
0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a)
548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a)
11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a)
17156 0 0 17156 4304 ssl_cli.o (ex library/libmbedtls.a)
460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a)
17649 0 0 17649 44f1 ssl_srv.o (ex library/libmbedtls.a)
800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a)
39286 60 0 39346 99b2 ssl_tls.o (ex library/libmbedtls.a)
88874 60 600 89534 15dbe (TOTALS)
DTLS-only
text data bss dec hex filename
1820 0 4 1824 720 debug.o (ex library/libmbedtls.a)
0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a)
548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a)
11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a)
17068 0 0 17068 42ac ssl_cli.o (ex library/libmbedtls.a)
460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a)
17553 0 0 17553 4491 ssl_srv.o (ex library/libmbedtls.a)
800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a)
38499 60 0 38559 969f ssl_tls.o (ex library/libmbedtls.a)
87903 60 600 88563 159f3 (TOTALS)
TLS-only
text data bss dec hex filename
1820 0 4 1824 720 debug.o (ex library/libmbedtls.a)
0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a)
548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a)
11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a)
14912 0 0 14912 3a40 ssl_cli.o (ex library/libmbedtls.a)
460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a)
15868 0 0 15868 3dfc ssl_srv.o (ex library/libmbedtls.a)
800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a)
27619 60 0 27679 6c1f ssl_tls.o (ex library/libmbedtls.a)
73182 60 600 73842 12072 (TOTALS)
```
Context: During a handshake, the SSL/TLS handshake logic constructs
an instance of ::mbedtls_ssl_session representing the SSL session
being established. This structure contains information such as the
session's master secret, the peer certificate, or the session ticket
issues by the server (if applicable).
During a renegotiation, the new session is constructed aside the existing
one and destroys and replaces the latter only when the renegotiation is
complete. While conceptually clear, this means that during the renegotiation,
large pieces of information such as the peer's CRT or the session ticket
exist twice in memory, even though the original versions are removed
eventually.
This commit starts removing this memory inefficiency by freeing the old
session's SessionTicket before the one for the new session is allocated.
The SSL context structure mbedtls_ssl_context contains several pointers
ssl->in_hdr, ssl->in_len, ssl->in_iv, ssl->in_msg pointing to various
parts of the record header in an incoming record, and they are setup
in the static function ssl_update_in_pointers() based on the _expected_
transform for the next incoming record.
In particular, the pointer ssl->in_msg is set to where the record plaintext
should reside after record decryption, and an assertion double-checks this
after each call to ssl_decrypt_buf().
This commit removes the dependency of ssl_update_in_pointers() on the
expected incoming transform by setting ssl->in_msg to ssl->in_iv --
the beginning of the record content (potentially including the IV) --
and adjusting ssl->in_msg after calling ssl_decrypt_buf() on a protected
record.
Care has to be taken to not load ssl->in_msg before calling
mbedtls_ssl_read_record(), then, which was previously the
case in ssl_parse_server_hello(); the commit fixes that.
Prior to this commit, the security parameter struct `ssl_transform`
contained a `ciphersuite_info` field pointing to the information
structure for the negotiated ciphersuite. However, the only
information extracted from that structure that was used in the core
encryption and decryption functions `ssl_encrypt_buf`/`ssl_decrypt_buf`
was the authentication tag length in case of an AEAD cipher.
The present commit removes the `ciphersuite_info` field from the
`ssl_transform` structure and adds an explicit `taglen` field
for AEAD authentication tag length.
This is in accordance with the principle that the `ssl_transform`
structure should contain the raw parameters needed for the record
encryption and decryption functions to work, but not the higher-level
information that gave rise to them. For example, the `ssl_transform`
structure implicitly contains the encryption/decryption keys within
their cipher contexts, but it doesn't contain the SSL master or
premaster secrets. Likewise, it contains an explicit `maclen`, while
the status of the 'Truncated HMAC' extension -- which determines the
value of `maclen` when the `ssl_transform` structure is created in
`ssl_derive_keys` -- is not contained in `ssl_transform`.
The `ciphersuite_info` pointer was used in other places outside
the encryption/decryption functions during the handshake, and for
these functions to work, this commit adds a `ciphersuite_info` pointer
field to the handshake-local `ssl_handshake_params` structure.
The SSL module accesses ECDH context members directly. This can't work
with the new context, where we can't make any assumption about the
implementation of the context.
This commit makes use of the new functions to avoid accessing ECDH
members directly. The only members that are still accessed directly are
the group ID and the point format and they are independent from the
implementation.
The previous code triggered a compiler warning because of a comparison
of a signed and an unsigned integer.
The conversion is safe because `len` is representable by 16-bits,
hence smaller than the maximum integer.
In the previous bounds check `(*p) > end - len`, the computation
of `end - len` might underflow if `end` is within the first 64KB
of the address space (note that the length `len` is controlled by
the peer). In this case, the bounds check will be bypassed, leading
to `*p` exceed the message bounds by up to 64KB when leaving
`ssl_parse_server_psk_hint()`. In a pure PSK-based handshake,
this doesn't seem to have any consequences, as `*p*` is not accessed
afterwards. In a PSK-(EC)DHE handshake, however, `*p` is read from
in `ssl_parse_server_ecdh_params()` and `ssl_parse_server_dh_params()`
which might lead to an application crash of information leakage.
* development-restricted: (578 commits)
Update library version number to 2.13.1
Don't define _POSIX_C_SOURCE in header file
Don't declare and define gmtime()-mutex on Windows platforms
Correct preprocessor guards determining use of gmtime()
Correct documentation of mbedtls_platform_gmtime_r()
Correct typo in documentation of mbedtls_platform_gmtime_r()
Correct POSIX version check to determine presence of gmtime_r()
Improve documentation of mbedtls_platform_gmtime_r()
platform_utils.{c/h} -> platform_util.{c/h}
Don't include platform_time.h if !MBEDTLS_HAVE_TIME
Improve wording of documentation of MBEDTLS_PLATFORM_GMTIME_R_ALT
Fix typo in documentation of MBEDTLS_PLATFORM_GMTIME_R_ALT
Replace 'thread safe' by 'thread-safe' in the documentation
Improve documentation of MBEDTLS_HAVE_TIME_DATE
ChangeLog: Add missing renamings gmtime -> gmtime_r
Improve documentation of MBEDTLS_HAVE_TIME_DATE
Minor documentation improvements
Style: Add missing period in documentation in threading.h
Rename mbedtls_platform_gmtime() to mbedtls_platform_gmtime_r()
Guard decl and use of gmtime mutex by HAVE_TIME_DATE and !GMTIME_ALT
...
Previously, mbedtls_ssl_read_record() always updated the handshake
checksum in case a handshake record was received. While desirable
most of the time, for the CertificateVerify message the checksum
update must only happen after the message has been fully processed,
because the validation requires the handshake digest up to but
excluding the CertificateVerify itself. As a remedy, the bulk
of mbedtls_ssl_read_record() was previously duplicated within
ssl_parse_certificate_verify(), hardening maintenance in case
mbedtls_ssl_read_record() is subject to changes.
This commit adds a boolean parameter to mbedtls_ssl_read_record()
indicating whether the checksum should be updated in case of a
handshake message or not. This allows using it also for
ssl_parse_certificate_verify(), manually updating the checksum
after the message has been processed.
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
