Cover the serialized out_cid length with the same construction as the
in_cid one, and rename the existing case to IN_CID accordingly. Give
the client and server sentinel CIDs different lengths so the two
fields can be told apart in the serialized data. Skip the CID cases
under configurations of MBEDTLS_SSL_CID_IN_LEN_MAX and
MBEDTLS_SSL_CID_OUT_LEN_MAX that the test does not support.
Signed-off-by: Naveed <naveed@bugqore.com>
The previous approach documented the in_cid_len > sizeof(in_cid) cap
as untestable, since the overflowing memcpy() lands inside the
transform structure where sanitizers cannot see it. It can be guarded
without a sanitizer: grow the in_cid field in the loaded data (keep
its bytes, pad to MBEDTLS_SSL_CID_IN_LEN_MAX + 1) and append the rest
of the serialized data untouched, so the in-buffer bounds check passes
and the cap is what must reject the length. Without the cap the
remaining data parses correctly aligned and the load succeeds, so the
test catches a regression by the return value alone.
Signed-off-by: Naveed <naveed@bugqore.com>
With the loaded data cut right after the in_cid bytes, even the
pristine in_cid length tripped the (end - p) < in_cid_len + 1u bounds
check, since the out_cid length byte it accounts for was missing: the
truncation, not the corruption, was what got rejected. Keep the
out_cid length byte in the loaded data, so the pristine length passes
the check and only the corrupted length trips it. Also document why
this pre-existing check is the only one in the in_cid path that a
regression test can guard: the in_cid_len > sizeof(in_cid) cap
protects the memcpy() destination, and without it the overflow lands
in the out_cid field inside the transform structure, where sanitizers
cannot see it.
Signed-off-by: Naveed <naveed@bugqore.com>
The corrupted in_cid length exercised the pre-existing size-cap check,
not the bounds check: the in_cid field is not the last one serialized,
and the MBEDTLS_SSL_CID_IN_LEN_MAX cap is smaller than the size of the
fields that follow it, so no corrupted length could reach past the end
of the full buffer, and reverting the bounds check did not change the
test's outcome. Truncate the loaded data right after the in_cid bytes,
load from a tightly allocated copy, and corrupt the length to one byte
past that end, so that reverting the bounds check now trips ASan with
an out-of-bounds read in the memcpy(), like the ALPN case.
Signed-off-by: Naveed <naveed@bugqore.com>
Load the serialized context once before corrupting it, to check the
data is well-formed to begin with. Shorten the second ALPN protocol so
the corrupted length overruns the serialized data by exactly one byte,
and assert that property at runtime. Document that the corrupted load
usually fails even without the bounds check, so the regression is only
caught when the suite runs under ASan or Valgrind.
Signed-off-by: Naveed <naveed@bugqore.com>
Fold ssl_context_load_rejects_oob_cid_length and
ssl_context_load_rejects_oob_alpn_length into a single parameterized
test function, since they shared most of their code.
Signed-off-by: Naveed <naveed@bugqore.com>
The serialized ALPN length was passed to memcmp() against each configured protocol without checking it against the remaining serialized data, so a malformed context could read past the end of the buffer. Bound it before the comparison, like the adjacent CID fields.
Signed-off-by: Naveed <naveed@bugqore.com>
Align to recent changes in TLS group ID related functions:
- 'mbedtls_ssl_get_ecp_group_id_from_tls_id' renamed to
'mbedtls_ssl_is_tls_id_supported'.
- 'mbedtls_ssl_get_tls_id_from_ecp_group_id' renamed to
'mbedtls_ssl_get_tls_id_from_ecp_group_id'.
Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>
RFC 8446 (TLS 1.3 specification), Section 5.1, states:
Handshake messages MAY be coalesced into a single TLSPlaintext record
or fragmented across several records, provided that:
...
-Handshake messages MUST NOT span key changes. Implementations
MUST verify that all messages immediately preceding a key change
align with a record boundary; if not, then they MUST terminate the
connection with an "unexpected_message" alert. Because the
ClientHello, EndOfEarlyData, ServerHello, Finished, and KeyUpdate
messages can immediately precede a key change, implementations
MUST send these messages in alignment with a record boundary.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Move functions in "ssl_helpers.h" that depend on internal headers to
"ssl_helpers_internal.h". This allows to include "ssl_helpers.h" also
in programs (which do not have access to internal headers).
Signed-off-by: Valerio Setti <vsetti@baylibre.com>
Add a test entry point for a handshake with a specially crafted context.
This can be used for defense in depths for scenarios that we think can't
happen, but might actually happen due to a bug, or due to a new feature that
is insufficiently tested. This can also be a proxy for testing aspects of
renegotiation or session resumption in circumstances that are hard to
arrange.
In this commit, use it to test a DTLS handshake with `ssl->badmac_seen`
:(actually `ssl->badmac_seen_or_in_hsfraglen` in the 3.6 branch) set to a
nonzero value. This can't happen, as far as I know (`badmac_seen` can only
be nonzero during renegotiation, not during a nominal handshake or in
session resumption). But it could happen, for example, if
`mbedtls_ssl_session_reset()` failed to reset `badmac_seen`, as was the case
in Mbed TLS 4.0.0 to 4.1.0.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Non-regression test for a bug introduced with TLS handshake defragmentation
support in Mbed TLS 3.6.3, whereby the confusion between the `badmac_seen`
meaning and the `in_hsfraglen` meaning of `ssl->badmac_seen_or_in_hsfraglen`
causes renegotiation to be processed incorrectly after receiving a message
with a bad MAC.
The bug doesn't affect Mbed TLS 4.0.0 and above, but it's still good to have
more testing.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
