Commit graph

90 commits

Author SHA1 Message Date
Matt Caswell
cfd40fd39e Prevent DTLS Finished message injection
Follow on from CVE-2016-2179

The investigation and analysis of CVE-2016-2179 highlighted a related flaw.

This commit fixes a security "near miss" in the buffered message handling
code. Ultimately this is not currently believed to be exploitable due to
the reasons outlined below, and therefore there is no CVE for this on its
own.

The issue this commit fixes is a MITM attack where the attacker can inject
a Finished message into the handshake. In the description below it is
assumed that the attacker injects the Finished message for the server to
receive it. The attack could work equally well the other way around (i.e
where the client receives the injected Finished message).

The MITM requires the following capabilities:
- The ability to manipulate the MTU that the client selects such that it
is small enough for the client to fragment Finished messages.
- The ability to selectively drop and modify records sent from the client
- The ability to inject its own records and send them to the server

The MITM forces the client to select a small MTU such that the client
will fragment the Finished message. Ideally for the attacker the first
fragment will contain all but the last byte of the Finished message,
with the second fragment containing the final byte.

During the handshake and prior to the client sending the CCS the MITM
injects a plaintext Finished message fragment to the server containing
all but the final byte of the Finished message. The message sequence
number should be the one expected to be used for the real Finished message.

OpenSSL will recognise that the received fragment is for the future and
will buffer it for later use.

After the client sends the CCS it then sends its own Finished message in
two fragments. The MITM causes the first of these fragments to be
dropped. The OpenSSL server will then receive the second of the fragments
and reassemble the complete Finished message consisting of the MITM
fragment and the final byte from the real client.

The advantage to the attacker in injecting a Finished message is that
this provides the capability to modify other handshake messages (e.g.
the ClientHello) undetected. A difficulty for the attacker is knowing in
advance what impact any of those changes might have on the final byte of
the handshake hash that is going to be sent in the "real" Finished
message. In the worst case for the attacker this means that only 1 in
256 of such injection attempts will succeed.

It may be possible in some situations for the attacker to improve this such
that all attempts succeed. For example if the handshake includes client
authentication then the final message flight sent by the client will
include a Certificate. Certificates are ASN.1 objects where the signed
portion is DER encoded. The non-signed portion could be BER encoded and so
the attacker could re-encode the certificate such that the hash for the
whole handshake comes to a different value. The certificate re-encoding
would not be detectable because only the non-signed portion is changed. As
this is the final flight of messages sent from the client the attacker
knows what the complete hanshake hash value will be that the client will
send - and therefore knows what the final byte will be. Through a process
of trial and error the attacker can re-encode the certificate until the
modified handhshake also has a hash with the same final byte. This means
that when the Finished message is verified by the server it will be
correct in all cases.

In practice the MITM would need to be able to perform the same attack
against both the client and the server. If the attack is only performed
against the server (say) then the server will not detect the modified
handshake, but the client will and will abort the connection.
Fortunately, although OpenSSL is vulnerable to Finished message
injection, it is not vulnerable if *both* client and server are OpenSSL.
The reason is that OpenSSL has a hard "floor" for a minimum MTU size
that it will never go below. This minimum means that a Finished message
will never be sent in a fragmented form and therefore the MITM does not
have one of its pre-requisites. Therefore this could only be exploited
if using OpenSSL and some other DTLS peer that had its own and separate
Finished message injection flaw.

The fix is to ensure buffered messages are cleared on epoch change.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-08-22 11:03:14 +01:00
Matt Caswell
b77ab018b7 Fix DTLS replay protection
The DTLS implementation provides some protection against replay attacks
in accordance with RFC6347 section 4.1.2.6.

A sliding "window" of valid record sequence numbers is maintained with
the "right" hand edge of the window set to the highest sequence number we
have received so far. Records that arrive that are off the "left" hand
edge of the window are rejected. Records within the window are checked
against a list of records received so far. If we already received it then
we also reject the new record.

If we have not already received the record, or the sequence number is off
the right hand edge of the window then we verify the MAC of the record.
If MAC verification fails then we discard the record. Otherwise we mark
the record as received. If the sequence number was off the right hand edge
of the window, then we slide the window along so that the right hand edge
is in line with the newly received sequence number.

Records may arrive for future epochs, i.e. a record from after a CCS being
sent, can arrive before the CCS does if the packets get re-ordered. As we
have not yet received the CCS we are not yet in a position to decrypt or
validate the MAC of those records. OpenSSL places those records on an
unprocessed records queue. It additionally updates the window immediately,
even though we have not yet verified the MAC. This will only occur if
currently in a handshake/renegotiation.

This could be exploited by an attacker by sending a record for the next
epoch (which does not have to decrypt or have a valid MAC), with a very
large sequence number. This means the right hand edge of the window is
moved very far to the right, and all subsequent legitimate packets are
dropped causing a denial of service.

A similar effect can be achieved during the initial handshake. In this
case there is no MAC key negotiated yet. Therefore an attacker can send a
message for the current epoch with a very large sequence number. The code
will process the record as normal. If the hanshake message sequence number
(as opposed to the record sequence number that we have been talking about
so far) is in the future then the injected message is bufferred to be
handled later, but the window is still updated. Therefore all subsequent
legitimate handshake records are dropped. This aspect is not considered a
security issue because there are many ways for an attacker to disrupt the
initial handshake and prevent it from completing successfully (e.g.
injection of a handshake message will cause the Finished MAC to fail and
the handshake to be aborted). This issue comes about as a result of trying
to do replay protection, but having no integrity mechanism in place yet.
Does it even make sense to have replay protection in epoch 0? That
issue isn't addressed here though.

This addressed an OCAP Audit issue.

CVE-2016-2181

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-08-19 14:04:56 +01:00
Matt Caswell
fa75569758 Fix DTLS unprocessed records bug
During a DTLS handshake we may get records destined for the next epoch
arrive before we have processed the CCS. In that case we can't decrypt or
verify the record yet, so we buffer it for later use. When we do receive
the CCS we work through the queue of unprocessed records and process them.

Unfortunately the act of processing wipes out any existing packet data
that we were still working through. This includes any records from the new
epoch that were in the same packet as the CCS. We should only process the
buffered records if we've not got any data left.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-08-19 14:04:44 +01:00
Matt Caswell
05200ee5c6 Change usage of RAND_pseudo_bytes to RAND_bytes
RAND_pseudo_bytes() allows random data to be returned even in low entropy
conditions. Sometimes this is ok. Many times it is not. For the avoidance
of any doubt, replace existing usage of RAND_pseudo_bytes() with
RAND_bytes().

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-06-27 15:02:34 +01:00
Lubom
0a9f8e0621 Lost alert in DTLS
If a client receives a bad hello request in DTLS then the alert is not
sent correctly.

RT#2801

Signed-off-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
(cherry picked from commit 4dc1aa0436)
2015-05-22 10:24:49 +01:00
Emilia Kasper
8b7e469d06 Harmonize return values in dtls1_buffer_record
Ensure all malloc failures return -1.

Reported by Adam Langley (Google).

Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 06c6a2b4a3)
2015-03-10 13:52:37 -07:00
Kurt Cancemi
183db9af80 Use constants not numbers
This patch uses warning/fatal constants instead of numbers with comments for
warning/alerts in d1_pkt.c and s3_pkt.c

RT#3725

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit fd865cadcb)
2015-03-05 09:30:35 +00:00
Matt Caswell
cda8845ded Re-align some comments after running the reformat script.
This should be a one off operation (subsequent invokation of the
script should not move them)

This commit is for the 1.0.1 changes

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:39:01 +00:00
Matt Caswell
10621efd32 Run util/openssl-format-source -v -c .
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:38:39 +00:00
Matt Caswell
0f6c965823 Move more comments that confuse indent
Conflicts:
	crypto/dsa/dsa.h
	demos/engines/ibmca/hw_ibmca.c
	ssl/ssl_locl.h

Conflicts:
	crypto/bn/rsaz_exp.c
	crypto/evp/e_aes_cbc_hmac_sha1.c
	crypto/evp/e_aes_cbc_hmac_sha256.c
	ssl/ssl_locl.h

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:38:04 +00:00
Matt Caswell
3cf9f81b09 indent has problems with comments that are on the right hand side of a line.
Sometimes it fails to format them very well, and sometimes it corrupts them!
This commit moves some particularly problematic ones.

Conflicts:
	crypto/bn/bn.h
	crypto/ec/ec_lcl.h
	crypto/rsa/rsa.h
	demos/engines/ibmca/hw_ibmca.c
	ssl/ssl.h
	ssl/ssl3.h

Conflicts:
	crypto/ec/ec_lcl.h
	ssl/tls1.h

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:36:16 +00:00
Matt Caswell
712548231e Additional comment changes for reformat of 1.0.1
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:33:47 +00:00
Matt Caswell
ac84cb4cfe Further comment changes for reformat
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:33:38 +00:00
Tim Hudson
3e9a08ecb1 mark all block comments that need format preserving so that
indent will not alter them when reformatting comments

(cherry picked from commit 1d97c84351)

Conflicts:
	crypto/bn/bn_lcl.h
	crypto/bn/bn_prime.c
	crypto/engine/eng_all.c
	crypto/rc4/rc4_utl.c
	crypto/sha/sha.h
	ssl/kssl.c
	ssl/t1_lib.c

Conflicts:
	crypto/rc4/rc4_enc.c
	crypto/x509v3/v3_scts.c
	crypto/x509v3/v3nametest.c
	ssl/d1_both.c
	ssl/s3_srvr.c
	ssl/ssl.h
	ssl/ssl_locl.h
	ssl/ssltest.c
	ssl/t1_lib.c

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:33:23 +00:00
Matt Caswell
04685bc949 A memory leak can occur in dtls1_buffer_record if either of the calls to
ssl3_setup_buffers or pqueue_insert fail. The former will fail if there is a
malloc failure, whilst the latter will fail if attempting to add a duplicate
record to the queue. This should never happen because duplicate records should
be detected and dropped before any attempt to add them to the queue.
Unfortunately records that arrive that are for the next epoch are not being
recorded correctly, and therefore replays are not being detected.
Additionally, these "should not happen" failures that can occur in
dtls1_buffer_record are not being treated as fatal and therefore an attacker
could exploit this by sending repeated replay records for the next epoch,
eventually causing a DoS through memory exhaustion.

Thanks to Chris Mueller for reporting this issue and providing initial
analysis and a patch. Further analysis and the final patch was performed by
Matt Caswell from the OpenSSL development team.

CVE-2015-0206

Reviewed-by: Dr Stephen Henson <steve@openssl.org>
2015-01-08 13:43:20 +00:00
Matt Caswell
45fe66b8ba Follow on from CVE-2014-3571. This fixes the code that was the original source
of the crash due to p being NULL. Steve's fix prevents this situation from
occuring - however this is by no means obvious by looking at the code for
dtls1_get_record. This fix just makes things look a bit more sane.

Reviewed-by: Dr Steve Henson <steve@openssl.org>
2015-01-08 13:43:20 +00:00
Dr. Stephen Henson
8d7aab986b Fix crash in dtls1_get_record whilst in the listen state where you get two
separate reads performed - one for the header and one for the body of the
handshake record.

CVE-2014-3571

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-01-08 13:43:20 +00:00
Matt Caswell
244d0955ad Add checks to the return value of EVP_Cipher to prevent silent encryption failure.
PR#1767

Reviewed-by: Richard Levitte <levitte@openssl.org>
2014-11-27 21:53:02 +00:00
Matt Caswell
056389eb1c Added SSLErr call for internal error in dtls1_buffer_record 2014-06-01 21:38:01 +01:00
David Ramos
a07856a08d Delays the queue insertion until after the ssl3_setup_buffers() call due to use-after-free bug. PR#3362 2014-06-01 21:37:47 +01:00
Michael Tuexen
83a3af9f4e DTLS message_sequence number wrong in rehandshake ServerHello
This fix ensures that
* A HelloRequest is retransmitted if not responded by a ClientHello
* The HelloRequest "consumes" the sequence number 0. The subsequent
ServerHello uses the sequence number 1.
* The client also expects the sequence number of the ServerHello to
be 1 if a HelloRequest was received earlier.
This patch fixes the RFC violation.
(cherry picked from commit b62f4daac0)
2013-08-13 18:55:41 +01:00
Michael Tuexen
76bf0cf27c DTLS handshake fix.
Reported by: Prashant Jaikumar <rmstar@gmail.com>

Fix handling of application data received before a handshake.
(cherry picked from commit 0c75eeacd3)
2013-08-08 13:32:11 +01:00
Andy Polyakov
8545f73b89 ssl/[d1|s3]_pkt.c: harmomize orig_len handling. 2013-02-07 22:47:05 +01:00
Dr. Stephen Henson
35d732fc2e Fix error codes. 2013-02-04 21:13:18 +00:00
Andy Polyakov
ec07246a08 ssl/*: remove SSL3_RECORD->orig_len to restore binary compatibility. 2013-02-01 15:34:09 +01:00
Ben Laurie
9f27de170d Update DTLS code to match CBC decoding in TLS.
This change updates the DTLS code to match the constant-time CBC
behaviour in the TLS.
2013-01-28 17:34:33 +00:00
Ben Laurie
6cb19b7681 Don't crash when processing a zero-length, TLS >= 1.1 record.
The previous CBC patch was bugged in that there was a path through enc()
in s3_pkt.c/d1_pkt.c which didn't set orig_len. orig_len would be left
at the previous value which could suggest that the packet was a
sufficient length when it wasn't.
2013-01-28 17:33:18 +00:00
Ben Laurie
2ee798880a Add and use a constant-time memcmp.
This change adds CRYPTO_memcmp, which compares two vectors of bytes in
an amount of time that's independent of their contents. It also changes
several MAC compares in the code to use this over the standard memcmp,
which may leak information about the size of a matching prefix.
2013-01-28 17:30:38 +00:00
Dr. Stephen Henson
ce1605b508 PR: 2756
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>

Fix DTLS timeout handling.
2012-03-09 15:52:20 +00:00
Dr. Stephen Henson
2dc4b0dbe8 Fix for DTLS DoS issue introduced by fix for CVE-2011-4109.
Thanks to Antonio Martin, Enterprise Secure Access Research and
Development, Cisco Systems, Inc. for discovering this bug and
preparing a fix. (CVE-2012-0050)
2012-01-18 18:14:56 +00:00
Dr. Stephen Henson
0044739ae5 Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>, Michael Tuexen <tuexen@fh-muenster.de>
Reviewed by: steve

Fix for DTLS plaintext recovery attack discovered by Nadhem Alfardan and
Kenny Paterson.
2012-01-04 23:52:05 +00:00
Dr. Stephen Henson
bd6941cfaa PR: 2658
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Reviewed by: steve

Support for TLS/DTLS heartbeats.
2011-12-31 23:00:36 +00:00
Dr. Stephen Henson
e065e6cda2 PR: 2535
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Reviewed by: steve

Add SCTP support for DTLS (RFC 6083).
2011-12-25 14:45:40 +00:00
Dr. Stephen Henson
ec5d74f868 PR: 2573
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Reviewed by: steve

Fix DTLS buffering and decryption bug.
2011-09-01 14:02:14 +00:00
Dr. Stephen Henson
a8515e2d28 Since DTLS 1.0 is based on TLS 1.1 we should never return a decryption_failed
alert.
2011-01-04 19:33:30 +00:00
Dr. Stephen Henson
c6dd154b3e oops, revert previous patch 2010-08-27 12:10:12 +00:00
Dr. Stephen Henson
35cae95032 PR: 1833
Submitted By: Robin Seggelmann <seggelmann@fh-muenster.de>

Fix other cases not covered by original patch.
2010-08-27 11:57:42 +00:00
Dr. Stephen Henson
48ae85b6ff PR: 1833
Submitted By: Robin Seggelmann <seggelmann@fh-muenster.de>

Support for abbreviated handshakes when renegotiating.
2010-08-26 14:22:40 +00:00
Dr. Stephen Henson
e97359435e Fix warnings (From HEAD, original patch by Ben). 2010-06-15 17:25:15 +00:00
Dr. Stephen Henson
30e8defe52 PR: 2229
Submitted By: Robin Seggelmann <seggelmann@fh-muenster.de>

Don't drop DTLS connection if mac or decryption failed.
2010-04-14 00:09:55 +00:00
Dr. Stephen Henson
9f4dd3e3e3 PR: 2228
Submitted By: Robin Seggelmann <seggelmann@fh-muenster.de>

Fix DTLS buffer record MAC failure bug.
2010-04-14 00:03:13 +00:00
Richard Levitte
0a4fe6c8db Undo the previous change, it was incorrect in this branch. 2010-04-13 11:10:07 +00:00
Richard Levitte
7bba401d5d Third argument to dtls1_buffer_record is by reference 2010-04-13 08:41:58 +00:00
Dr. Stephen Henson
6dfd3cf68e PR: 2218
Submitted By: Robin Seggelmann <seggelmann@fh-muenster.de>

Fixes for DTLS replay bug.
2010-04-06 12:44:55 +00:00
Dr. Stephen Henson
073775cbbb PR: 2219
Submitted By: Robin Seggelmann <seggelmann@fh-muenster.de>

Fixes for DTLS buffering bug.
2010-04-06 12:40:10 +00:00
Dr. Stephen Henson
ad187f8905 Fix unitialized warnings 2009-10-04 16:52:35 +00:00
Dr. Stephen Henson
af3d4e1b02 PR: 2039
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Approved by: steve@openssl.org

DTLS listen bug fix,
2009-09-15 22:48:30 +00:00
Dr. Stephen Henson
17f8d8db61 PR: 2006
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Approved by: steve@openssl.org

Do not use multiple DTLS records for a single user message
2009-08-26 11:51:23 +00:00
Dr. Stephen Henson
a4bade7aac PR: 1997
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Approved by: steve@openssl.org

DTLS timeout handling fix.
2009-08-12 13:21:26 +00:00
Dr. Stephen Henson
d7406b1528 PR: 1993
Fix from 0.9.8-stable.
2009-07-24 11:52:32 +00:00