This commit ensures that there is a small packet test for at least any
combination of
- SSL/TLS version: SSLv3, TLS 1.0, TLS 1.1 or TLS 1.2
- Stream cipher (RC4) or Block cipher (AES)
- Usage of Encrypt then MAC extension [TLS only]
- Usage of truncated HMAC extension [TLS only]
Noticed that the test cases in ssl-opt.sh exercising the truncated HMAC
extension do not depend on MBEDTLS_SSL_TRUNCATED_HMAC being enabled in
config.h. This commit fixes this.
The truncated HMAC extension as described in
https://tools.ietf.org/html/rfc6066.html#section-7 specifies that when truncated
HMAC is used, only the HMAC output should be truncated, while the HMAC key
generation stays unmodified. This commit fixes Mbed TLS's behavior of also
truncating the key, potentially leading to compatibility issues with peers
running other stacks than Mbed TLS.
Details:
The keys for the MAC are pieces of the keyblock that's generated from the
master secret in `mbedtls_ssl_derive_keys` through the PRF, their size being
specified as the size of the digest used for the MAC, regardless of whether
truncated HMAC is enabled or not.
/----- MD size ------\ /------- MD size ----\
Keyblock +----------------------+----------------------+------------------+---
now | MAC enc key | MAC dec key | Enc key | ...
(correct) +----------------------+----------------------+------------------+---
In the previous code, when truncated HMAC was enabled, the HMAC keys
were truncated to 10 bytes:
/-10 bytes-\ /-10 bytes-\
Keyblock +-------------+-------------+------------------+---
previously | MAC enc key | MAC dec key | Enc key | ...
(wrong) +-------------+-------------+------------------+---
The reason for this was that a single variable `transform->maclen` was used for
both the keysize and the size of the final MAC, and its value was reduced from
the MD size to 10 bytes in case truncated HMAC was negotiated.
This commit fixes this by introducing a temporary variable `mac_key_len` which
permanently holds the MD size irrespective of the presence of truncated HMAC,
and using this temporary to obtain the MAC key chunks from the keyblock.
Previously, MAC validation for an incoming record proceeded as follows:
1) Make a copy of the MAC contained in the record;
2) Compute the expected MAC in place, overwriting the presented one;
3) Compare both.
This resulted in a record buffer overflow if truncated MAC was used, as in this
case the record buffer only reserved 10 bytes for the MAC, but the MAC
computation routine in 2) always wrote a full digest.
For specially crafted records, this could be used to perform a controlled write of
up to 6 bytes past the boundary of the heap buffer holding the record, thereby
corrupting the heap structures and potentially leading to a crash or remote code
execution.
This commit fixes this by making the following change:
1) Compute the expected MAC in a temporary buffer that has the size of the
underlying message digest.
2) Compare to this to the MAC contained in the record, potentially
restricting to the first 10 bytes if truncated HMAC is used.
A similar fix is applied to the encryption routine `ssl_encrypt_buf`.
* development: (30 commits)
update README file (#1144)
Fix typo in asn1.h
Improve leap year test names in x509parse.data
Correctly handle leap year in x509_date_is_valid()
Renegotiation: Add tests for SigAlg ext parsing
Parse Signature Algorithm ext when renegotiating
Minor style fix
config.pl get: be better behaved
config.pl get: don't rewrite config.h; detect write errors
Fixed "config.pl get" for options with no value
Fix typo and bracketing in macro args
Ensure failed test_suite output is sent to stdout
Remove use of GNU sed features from ssl-opt.sh
Fix typos in ssl-opt.sh comments
Add ssl-opt.sh test to check gmt_unix_time is good
Extend ssl-opt.h so that run_test takes function
Always print gmt_unix_time in TLS client
Restored note about using minimum functionality in makefiles
Note in README that GNU make is required
Fix changelog for ssl_server2.c usage fix
...
This commit adds regression tests for the bug when we didn't parse the
Signature Algorithm extension when renegotiating. (By nature, this bug
affected only the server)
The tests check for the fallback hash (SHA1) in the server log to detect
that the Signature Algorithm extension hasn't been parsed at least in
one of the handshakes.
A more direct way of testing is not possible with the current test
framework, since the Signature Algorithm extension is parsed in the
first handshake and any corresponding debug message is present in the
logs.
Signature algorithm extension was skipped when renegotiation was in
progress, causing the signature algorithm not to be known when
renegotiating, and failing the handshake. Fix removes the renegotiation
step check before parsing the extension.
When printing an option's value, print a newline at the end.
When the requested option is missing, fail with status 1 (the usual
convention for "not found") rather than -1 (which has a
system-dependent effect).
scripts/config.pl would always rewrite config.h if it was reading it.
This commit changes it to not modify the file when only reading is
required, i.e. for the get command.
Also, die if writing config.h fails (e.g. disk full).
Between 2.5.0 and 2.6.0, "scripts/config.pl get MBEDTLS_XXX" was fixed
for config.h lines with a comment at the end, but that broke the case
of macros with an empty expansion. Support all cases.
This commit adds regression tests for the bug when we didn't parse the
Signature Algorithm extension when renegotiating. (By nature, this bug
affected only the server)
The tests check for the fallback hash (SHA1) in the server log to detect
that the Signature Algorithm extension hasn't been parsed at least in
one of the handshakes.
A more direct way of testing is not possible with the current test
framework, since the Signature Algorithm extension is parsed in the
first handshake and any corresponding debug message is present in the
logs.
Signature algorithm extension was skipped when renegotiation was in
progress, causing the signature algorithm not to be known when
renegotiating, and failing the handshake. Fix removes the renegotiation
step check before parsing the extension.
When printing an option's value, print a newline at the end.
When the requested option is missing, fail with status 1 (the usual
convention for "not found") rather than -1 (which has a
system-dependent effect).
scripts/config.pl would always rewrite config.h if it was reading it.
This commit changes it to not modify the file when only reading is
required, i.e. for the get command.
Also, die if writing config.h fails (e.g. disk full).
Between 2.5.0 and 2.6.0, "scripts/config.pl get MBEDTLS_XXX" was fixed
for config.h lines with a comment at the end, but that broke the case
of macros with an empty expansion. Support all cases.
The change modifies the template code in tests/suites/helpers.function
and tests/suites/main.function so that error messages are printed to
stdout instead of being discarded. This makes errors visible regardless
of the --verbose flag being passed or not to the test suite programs.
The change modifies the template code in tests/suites/helpers.function
and tests/suites/main.function so that error messages are printed to
stdout instead of being discarded. This makes errors visible regardless
of the --verbose flag being passed or not to the test suite programs.
Add a test to ssl-opt.sh that parses the client and server debug
output and then checks that the Unix timestamp in the ServerHello
message is within acceptable bounds.
Extend the run_test function in ssl-opt.sh so that it accepts the -f
and -F options. These parameters take an argument which is the name of
a shell function that will be called by run_test and will be given the
client input and output debug log. The idea is that these functions are
defined by each test and they can be used to do some custom check
beyon those allowed by the pattern matching capabilities of the
run_test function.
Change ssl_parse_server_hello() so that the parsed first four random
bytes from the ServerHello message are printed by the TLS client as
a Unix timestamp regardless of whether MBEDTLS_DEBUG_C is defined. The
debug message will only be printed if debug_level is 3 or higher.
Unconditionally enabling the debug print enabled testing of this value.
Our README claims that we only use basic Make functionality, but in
fact GNU make is required for conditional compilation. Document this.
Addresses issue #967
Add a test case calling ssl_set_hostname twice to test_suite_ssl.
When run in CMake build mode ASan, this catches the current leak,
but will hopefully be fine with the new version.
