Commit Graph

610 Commits

Author SHA1 Message Date
mohammad1603
89c12ecfb5 Avoid wraparound on in_left
Avoid wraparound on in_left
2018-03-19 07:15:50 -07: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
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
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
Hanno Becker
aa8a2bd05a Remember suitable hash function for any signature algorithm.
This commit changes `ssl_parse_signature_algorithms_ext` to remember
one suitable ( := supported by client and by our config ) hash
algorithm per signature algorithm.

It also modifies the ciphersuite checking function
`ssl_ciphersuite_match` to refuse a suite if there
is no suitable hash algorithm.

Finally, it adds the corresponding entry to the ChangeLog.
2017-05-24 11:04:50 +01:00
Andres AG
7fa66d4622 Fix renegotiation at incorrect times in DTLS
Fix an incorrect condition in ssl_check_ctr_renegotiate() that compared
64 bits of record counter instead of 48 bits as described in RFC 6347
Section 4.3.1. This would cause the function's return value to be
occasionally incorrect and the renegotiation routines to be triggered
at unexpected times.
2017-02-04 23:35:14 +00:00
Brian J Murray
e7f8dc304e Clarify Comments and Fix Typos (#651)
Fixes many typos, and errors in comments.

* Clarifies many comments
* Grammar correction in config.pl help text
* Removed comment about MBEDTLS_X509_EXT_NS_CERT_TYPE.
* Comment typo fix (Dont => Don't)
* Comment typo fix (assure => ensure)
* Comment typo fix (byes => bytes)
* Added citation for quoted standard
* Comment typo fix (one complement => 1's complement)

The is some debate about whether to prefer "one's complement",  "ones'
complement", or "1's complement".  The more recent RFCs related to TLS
(RFC 6347,  RFC 4347, etc) use " 1's complement", so I followed that
convention.

* Added missing ")" in comment
* Comment alignment
* Incorrect comment after #endif
2016-11-10 17:38:34 +00:00
Janos Follath
0be55a0549 Remove MBEDTLS_SSL_AEAD_RANDOM_IV feature
In a USENIX WOOT '16 paper the authors warn about a security risk
of random Initialisation Vectors (IV) repeating values.

The MBEDTLS_SSL_AEAD_RANDOM_IV feature is affected by this risk and
it isn't compliant with RFC5116. Furthermore, strictly speaking it
is a different cipher suite from the TLS (RFC5246) point of view.

Removing the MBEDTLS_SSL_AEAD_RANDOM_IV feature to resolve the above
problems.

Hanno Böck, Aaron Zauner, Sean Devlin, Juraj Somorovsky and Philipp
Jovanovic, "Nonce-Disrespecting Adversaries: Practical Forgery Attacks
on GCM in TLS", USENIX WOOT '16
2016-10-13 10:35:34 +01:00
Simon Butcher
c941b6cb31 Fix for unused variable warning 2015-12-28 01:29:10 +00:00
Simon Butcher
aa4114910a Merge 'iotssl-558-2.1-md5-tls-sigs-restricted' 2015-12-23 18:52:18 +00:00
Simon Butcher
e103aa8a53 Added description of change to the Changelog
Also clarified some comments following review.
2015-12-16 01:51:01 +00:00
Manuel Pégourié-Gonnard
b39528e2e8 Disable MD5 in handshake signatures by default 2015-12-04 15:13:36 +01:00
Manuel Pégourié-Gonnard
013198f30f DTLS: avoid dropping too many records
When the peer retransmits a flight with many record in the same datagram, and
we already saw one of the records in that datagram, we used to drop the whole
datagram, resulting in interoperability failure (spurious handshake timeouts,
due to ignoring record retransmitted by the peer) with some implementations
(issues with Chrome were reported).

So in those cases, we want to only drop the current record, and look at the
following records (if any) in the same datagram. OTOH, this is not something
we always want to do, as sometime the header of the current record is not
reliable enough.

This commit introduces a new return code for ssl_parse_header() that allows to
distinguish if we should drop only the current record or the whole datagram,
and uses it in mbedtls_ssl_read_record()

fixes #345
2015-12-03 19:22:55 +01:00
Manuel Pégourié-Gonnard
c28240596a Fix other int casts in bounds checking
Not a security issue as here we know the buffer is large enough (unless
something else if badly wrong in the code), and the value cast to int is less
than 2^16 (again, unless issues elsewhere).

Still changing to a more correct check as a matter of principle
2015-11-02 10:43:03 +09:00
Manuel Pégourié-Gonnard
ffb8180733 Fix potential double-free in ssl_conf_psk() 2015-11-02 10:40:14 +09:00
Simon Butcher
94c5e3c654 Fixed typo in comment 2015-10-28 13:21:12 +01:00
Manuel Pégourié-Gonnard
a314076486 Fix handling of non-fatal alerts
fixes #308
2015-10-28 13:19:55 +01:00
Manuel Pégourié-Gonnard
f9945bc283 Fix #ifdef inconsistency
fixes #310

Actually all key exchanges that use a certificate use signatures too, and
there is no key exchange that uses signatures but no cert, so merge those two
flags.

Conflicts:
	ChangeLog
2015-10-28 13:16:33 +01:00
Manuel Pégourié-Gonnard
770f453547 Remove useless code
closes #321
2015-10-27 10:29:26 +01:00