Commit graph

625 commits

Author SHA1 Message Date
Gilles Peskine e8dd77ba58 Fix Lucky13 attack protection when using HMAC-SHA-384
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 was correct for most supported hashes but not for
SHA-384. Adapt the formula to 128-byte blocks for SHA-384.
2018-06-06 17:24:50 +02:00
Andrzej Kurek 078014aebe Change variable bytes_written to header_bytes in record decompression
The name is changed to better reflect the input, decompression case
2018-04-24 06:33:49 -04:00
Andrzej Kurek bb6661479f ssl_tls: Fix invalid buffer sizes during compression / decompression
Adjust information passed to zlib to include already written data.
2018-04-23 08:29:36 -04:00
Jaeden Amero ac9939c096 Merge remote-tracking branch 'upstream-public/pr/1461' into mbedtls-2.1-proposed 2018-04-03 18:27:18 +01:00
Jaeden Amero ee6c822076 Merge remote-tracking branch 'upstream-public/pr/1396' into mbedtls-2.1-proposed 2018-04-03 12:07:19 +01:00
mohammad1603 ad2908c9d6 Fix compatibility problem in the printed message
Replace %zu with %lu and add cast for the printed value.
2018-04-02 07:30:32 -07:00
mohammad1603 f72e51f2b8 Check whether INT_MAX larger than SIZE_MAX scenario
Check whether INT_MAX larger than SIZE_MAX scenario
2018-03-28 23:44:39 -07:00
mohammad1603 cee0890b19 Verify that f_send and f_recv send and receive the expected length
Verify that f_send and f_recv send and receive the expected length

Conflicts:
	ChangeLog
2018-03-22 15:01:02 -07:00
Gilles Peskine 823734b96c Robustness fix in mbedtls_ssl_derive_keys
In mbedtls_ssl_derive_keys, don't call mbedtls_md_hmac_starts in
ciphersuites that don't use HMAC. This doesn't change the behavior of
the code, but avoids relying on an uncaught error when attempting to
start an HMAC operation that hadn't been initialized.
2018-03-19 19:06:08 +01:00
mohammad1603 89c12ecfb5 Avoid wraparound on in_left
Avoid wraparound on in_left
2018-03-19 07:15:50 -07:00
Gilles Peskine 9a00ef3cf1 Merge branch 'pr_953' into HEAD 2018-03-11 00:52:24 +01:00
Gilles Peskine 25ec9cc9b3 Merge branch 'prr_428' into mbedtls-2.1-proposed 2018-02-22 16:24:13 +01:00
mohammad1603 f65add4f60 Backport 2.1:Add guard to out_left to avoid negative values
return error when f_send return a value greater than out_left
2018-02-22 05:07:15 -08:00
Jaeden Amero bfafd12789 Merge remote-tracking branch 'upstream-restricted/pr/414' into mbedtls-2.1-restricted 2018-01-26 18:09:14 +00:00
Ron Eldor 1ac9aa7085 Set correct minimal versions in default conf
Set `MBEDTLS_SSL_MIN_MAJOR_VERSION` and `MBEDTLS_SSL_MIN_MINOR_VERSION`
instead of `MBEDTLS_SSL_MAJOR_VERSION_3` and `MBEDTLS_SSL_MINOR_VERSION_1`
2018-01-22 22:03:12 +01:00
Gilles Peskine c83f57b4c6 Merge remote-tracking branch 'upstream-restricted/pr/434' into mbedtls-2.1-restricted 2017-12-19 19:49:44 +01:00
Manuel Pégourié-Gonnard 451ea75286 Merge remote-tracking branch 'restricted/pr/412' into mbedtls-2.1-restricted
* restricted/pr/412:
  Correct record header size in case of TLS
  Don't allocate space for DTLS header if DTLS is disabled
  Improve debugging output
  Adapt ChangeLog
  Add run-time check for handshake message size in ssl_write_record
  Add run-time check for record content size in ssl_encrypt_buf
  Add compile-time checks for size of record content and payload
2017-12-19 11:33:07 +01:00
Manuel Pégourié-Gonnard 4b133e6dcf Fix magic constant in previous commit 2017-12-19 10:05:03 +01:00
Manuel Pégourié-Gonnard b67a5c1f29 Fix SSLv3 MAC computation
In a previous PR (Fix heap corruption in implementation of truncated HMAC
extension #425) the place where MAC is computed was changed from the end of
the SSL I/O buffer to a local buffer (then (part of) the content of the local
buffer is either copied to the output buffer of compare to the input buffer).

Unfortunately, this change was made only for TLS 1.0 and later, leaving SSL
3.0 in an inconsistent state due to ssl_mac() still writing to the old,
hard-coded location, which, for MAC verification, resulted in later comparing
the end of the input buffer (containing the computed MAC) to the local buffer
(uninitialised), most likely resulting in MAC verification failure, hence no
interop (even with ourselves).

This commit completes the move to using a local buffer by using this strategy
for SSL 3.0 too. Fortunately ssl_mac() was static so it's not a problem to
change its signature.
2017-12-19 10:05:00 +01:00
Gilles Peskine aed7188b2e Merge remote-tracking branch 'upstream-restricted/pr/427' into mbedtls-2.1-restricted 2017-12-01 18:05:40 +01:00
Hanno Becker adb30b9453 Improve documentation of MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT option
Explain more clearly when this option should be used and which versions of Mbed
TLS build on the non-compliant implementation.
2017-12-01 10:20:44 +00:00
Hanno Becker 053b3459d4 Add fallback to non-compliant truncated HMAC for compatibiltiy
In case truncated HMAC must be used but the Mbed TLS peer hasn't been updated
yet, one can use the compile-time option MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT to
temporarily fall back to the old, non-compliant implementation of the truncated
HMAC extension.
2017-12-01 10:18:41 +00:00
Hanno Becker 64f0aed966 Don't truncate MAC key when truncated HMAC is negotiated
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.
2017-12-01 10:18:22 +00:00
Gilles Peskine 6cf85ff1a4 Merge branch 'mbedtls-2.1' into mbedtls-2.1-restricted 2017-11-29 21:07:28 +01:00
Gilles Peskine 49349bacb9 Merge remote-tracking branch 'upstream-public/pr/1153' into mbedtls-2.1 2017-11-29 20:53:58 +01:00
Gilles Peskine 336b7de48a Merge remote-tracking branch 'upstream-restricted/pr/386' into mbedtls-2.1-restricted 2017-11-28 14:24:05 +01:00
Gilles Peskine 7aa24190b4 Merge remote-tracking branch 'upstream-public/pr/1107' into mbedtls-2.1 2017-11-23 20:09:48 +01:00
Hanno Becker ce516ff449 Fix heap corruption in ssl_decrypt_buf
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`.
2017-11-20 10:16:17 +00:00
Manuel Pégourié-Gonnard ea0aa655f6 Merge branch 'mbedtls-2.1' into mbedtls-2.1-restricted
* mbedtls-2.1:
  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
  Fix changelog for ssl_server2.c usage fix
  Fix ssl_server2 sample application prompt
  Update ChangeLog for fix to #836
  Enhance documentation of ssl_write_hostname_ext, adapt ChangeLog.
  Enhance documentation of mbedtls_ssl_set_hostname
  Add test case calling ssl_set_hostname twice
  Make mbedtls_ssl_set_hostname safe to be called multiple times
  Fix typo in configs/README.txt file
2017-11-14 08:38:52 +01:00
Hanno Becker 3cd07be889 Fix handling of HS msgs in mbedtls_ssl_read if renegotiation unused
Previously, if `MBEDTLS_SSL_RENEGOTIATION` was disabled, incoming handshake
messages in `mbedtls_ssl_read` (expecting application data) lead to the
connection being closed. This commit fixes this, restricting the
`MBEDTLS_SSL_RENEGOTIATION`-guard to the code-paths responsible for accepting
renegotiation requests and aborting renegotiation attempts after too many
unexpected records have been received.
2017-10-24 11:49:19 +01:00
Hanno Becker e454d73cc0 Swap branches accepting/refusing renegotiation in in ssl_read 2017-10-24 11:47:37 +01:00
Florin a360411e4f Fixed SIGSEGV problem when writing with ssl_write_real a buffer that is over MBEDTLS_SSL_MAX_CONTENT_LEN bytes
Signed-off-by: Florin <petriuc.florin@gmail.com>
2017-10-19 15:44:37 +01:00
Hanno Becker e298c8b46c Correct typo 2017-10-19 15:44:37 +01:00
Hanno Becker 80e0d46062 Use 2048-bit DHE parameters from RFC 3526 instead of 5114 by default
The parameters from RFC 5114 are not considered trustworthy, while those from
RFC 3526 have been generated in a nothing-up-my-sleeve manner.
2017-10-13 16:51:54 +01:00
Hanno Becker 6e052b0fbe Improve debugging output 2017-10-04 14:15:53 +01:00
Hanno Becker 0983dc49d6 Add run-time check for handshake message size in ssl_write_record 2017-10-04 14:15:30 +01:00
Hanno Becker aede183630 Add run-time check for record content size in ssl_encrypt_buf 2017-10-04 14:15:30 +01:00
Hanno Becker c7845e51f3 Enhance documentation of ssl_write_hostname_ext, adapt ChangeLog.
Add a reference to the relevant RFC, adapt ChangeLog.
2017-09-30 23:49:01 +01:00
Hanno Becker 593b0d33f6 Make mbedtls_ssl_set_hostname safe to be called multiple times
Zeroize and free previously set hostnames before overwriting
them. Also, allow clearance of hostname by providing NULL parameter.
2017-09-30 23:47:34 +01:00
Andres Amaya Garcia 1042d8637c Dont send alert on invalid DTLS record type
Do not send fatal alerts when receiving a record with an invalid header
while running DTLS as this is not compliant behaviour.
2017-09-15 10:57:29 +01:00
Andres Amaya Garcia 3d23146f62 Set len var to 0 when buf is freed in ssl_tls.c 2017-07-11 17:31:02 +01:00
Andres Amaya Garcia 1b7d6f8c03 Zeroize old psk buf when changing value in ssl_tls 2017-07-11 17:31:02 +01:00
Hanno Becker cc019084b8 Ensure application data records are not kept when fully processed
This commit fixes the following case: If a client is both expecting a
SERVER_HELLO and has an application data record that's partially
processed in flight (that's the situation the client gets into after
receiving a ServerHelloRequest followed by ApplicationData), a
subsequent call to mbedtls_ssl_read will set keep_current_message = 1
when seeing the unexpected application data, but not reset it to 0
after the application data has been processed. This commit fixes this.
2017-06-09 10:51:37 +01:00
Hanno Becker bfbc494114 Add hard assertion to mbedtls_ssl_read_record_layer
This commit adds a hard assertion to mbedtls_ssl_read_record_layer
triggering if both ssl->in_hslen and ssl->in_offt are not 0. This
should never happen, and if it does, there's no sensible way of
telling whether the previous message was a handshake or an application
data message.
2017-06-08 14:04:31 +01:00
Hanno Becker 6a582e80f2 Fix mbedtls_ssl_read
Don't fetch a new record in mbedtls_ssl_read_record_layer as long as an application data record is being processed.
2017-06-08 14:04:22 +01:00
Hanno Becker 704f493730 Simplify retaining of messages for future processing
There are situations in which it is not clear what message to expect
next. For example, the message following the ServerHello might be
either a Certificate, a ServerKeyExchange or a CertificateRequest. We
deal with this situation in the following way: Initially, the message
processing function for one of the allowed message types is called,
which fetches and decodes a new message. If that message is not the
expected one, the function returns successfully (instead of throwing
an error as usual for unexpected messages), and the handshake
continues to the processing function for the next possible message. To
not have this function fetch a new message, a flag in the SSL context
structure is used to indicate that the last message was retained for
further processing, and if that's set, the following processing
function will not fetch a new record.

This commit simplifies the usage of this message-retaining parameter
by doing the check within the record-fetching routine instead of the
specific message-processing routines. The code gets cleaner this way
and allows retaining messages to be used in other situations as well
without much effort. This will be used in the next commits.
2017-06-08 13:08:45 +01:00
Hanno Becker 61c0c70418 Add tests for missing CA chains and bad curves.
This commit adds four tests to tests/ssl-opt.sh:
(1) & (2): Check behaviour of optional/required verification when the
trusted CA chain is empty.
(3) & (4): Check behaviour of optional/required verification when the
client receives a server certificate with an unsupported curve.
2017-06-07 11:36:12 +01:00
Hanno Becker a3929bac1e Fix implementation of VERIFY_OPTIONAL verification mode
This commit changes the behaviour of mbedtls_ssl_parse_certificate
to make the two authentication modes MBEDTLS_SSL_VERIFY_REQUIRED and
MBEDTLS_SSL_VERIFY_OPTIONAL be in the following relationship:

    Mode == MBEDTLS_SSL_VERIFY_REQUIRED
<=> Mode == MBEDTLS_SSL_VERIFY_OPTIONAL + check verify result

Also, it changes the behaviour to perform the certificate chain
verification even if the trusted CA chain is empty. Previously, the
function failed in this case, even when using optional verification,
which was brought up in #864.
2017-06-07 11:36:09 +01:00
Gilles Peskine 7344e1bd05 SHA-1 deprecation: allow it in key exchange
By default, keep allowing SHA-1 in key exchange signatures. Disabling
it causes compatibility issues, especially with clients that use
TLS1.2 but don't send the signature_algorithms extension.

SHA-1 is forbidden in certificates by default, since it's vulnerable
to offline collision-based attacks.
2017-06-06 19:09:02 +02:00
Gilles Peskine 955738a4f2 Remove SHA-1 in TLS by default
Default to forbidding the use of SHA-1 in TLS where it is unsafe: for
certificate signing, and as the signature hash algorithm for the TLS
1.2 handshake signature. SHA-1 remains allowed in HMAC-SHA-1 in the
XXX_SHA ciphersuites and in the PRF for TLS <= 1.1.

For easy backward compatibility for use in controlled environments,
turn on the MBEDTLS_TLS_DEFAULT_ALLOW_SHA1 compiled-time option.
2017-06-06 19:08:19 +02:00