Commit graph

77 commits

Author SHA1 Message Date
Matt Caswell
d46de4ce48 DTLS handshake message fragments musn't span packets
It should not be possible for DTLS message fragments to span multiple
packets. However previously if the message header fitted exactly into one
packet, and the fragment body was in the next packet then this would work.
Obviously this would fail if packets get re-ordered mid-flight.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-06-10 12:12:57 +01:00
Matt Caswell
f3e85f4359 Check the message type requested is the type received in DTLS
dtls1_get_message has an |mt| variable which is the type of the message that
is being requested. If it is negative then any message type is allowed.
However the value of |mt| is not checked in one of the main code paths, so a
peer can send a message of a completely different type and it will be
processed as if it was the message type that we were expecting. This has
very little practical consequences because the current behaviour will still
fail when the format of the message isn't as expected.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 8c2b1d872b)
2015-06-01 00:33:22 +01:00
Viktor Dukhovni
c70908d247 Code style: space after 'if'
Reviewed-by: Matt Caswell <gitlab@openssl.org>
2015-04-16 13:51:51 -04:00
Matt Caswell
750190567a Fix RAND_(pseudo_)?_bytes returns
Ensure all calls to RAND_bytes and RAND_pseudo_bytes have their return
value checked correctly

Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 8f8e4e4f52)

Conflicts:
	crypto/evp/e_des3.c
2015-03-25 12:45:17 +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
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
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
9673056c25 Remove extraneous white space, and add some braces
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 55e530265a)
2014-12-16 00:08:04 +00:00
Matt Caswell
f50730d361 DTLS fixes for signed/unsigned issues
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 1904d21123)
2014-12-16 00:08:04 +00:00
Matt Caswell
9844573039 Remove "#if 0" code
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 4bb8eb9ce4)
2014-12-03 09:35:25 +00:00
Matt Caswell
3242e5938b If we really get a situation where the underlying mtu is less than the minimum
we will support then dtls1_do_write can go into an infinite loop. This commit
fixes that.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit d3d9eef316)
2014-12-03 09:35:24 +00:00
Matt Caswell
35384e8e5b Fix dtls_query_mtu so that it will always either complete with an mtu that is
at least the minimum or it will fail.
There were some instances in dtls1_query_mtu where the final mtu can end up
being less than the minimum, i.e. where the user has set an mtu manually. This
shouldn't be allowed. Also remove dtls1_guess_mtu that, despite having
logic for guessing an mtu, was actually only ever used to work out the minimum
mtu to use.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 1620a2e49c)
2014-12-03 09:35:24 +00:00
Matt Caswell
4c21e004a3 Remove instances in libssl of the constant 28 (for size of IPv4 header + UDP)
and instead use the value provided by the underlying BIO. Also provide some
new DTLS_CTRLs so that the library user can set the mtu without needing to
know this constant. These new DTLS_CTRLs provide the capability to set the
link level mtu to be used (i.e. including this IP/UDP overhead). The previous
DTLS_CTRLs required the library user to subtract this overhead first.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 59669b6abf)

Conflicts:
	ssl/d1_both.c
	ssl/ssl_lib.c
2014-12-03 09:35:23 +00:00
Matt Caswell
43e569921e The first call to query the mtu in dtls1_do_write correctly checks that the
mtu that we have received is not less than the minimum. If its less it uses the
minimum instead. The second call to query the mtu does not do that, but
instead uses whatever comes back. We have seen an instance in RT#3592 where we
have got an unreasonably small mtu come back. This commit makes both query
checks consistent.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 6abb0d1f8e)
2014-12-03 09:32:48 +00:00
Matt Caswell
8aaeec9f9a The SSL_OP_NO_QUERY_MTU option is supposed to stop the mtu from being
automatically updated, and we should use the one provided instead.
Unfortunately there are a couple of locations where this is not respected.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 001235778a)
2014-12-03 09:32:48 +00:00
Matt Caswell
8454d3f924 Verify that we have a sensible message len and fail if not
RT#3592 provides an instance where the OPENSSL_assert that this commit
replaces can be hit. I was able to recreate this issue by forcing the
underlying BIO to misbehave and come back with very small mtu values. This
happens the second time around the while loop after we have detected that the
MTU has been exceeded following the call to dtls1_write_bytes.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit cf75017bfd)
2014-12-03 09:32:48 +00:00
Matt Caswell
2e84084fbc Fixed memory leak due to incorrect freeing of DTLS reassembly bit mask
PR#3608

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 8a35dbb6d8)
2014-11-26 10:14:13 +00:00
Erik Auerswald
b537ea9ce4 RT3301: Discard too-long heartbeat requests
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit af4c6e348e)
2014-09-08 11:23:10 -04:00
Adam Langley
4e0fbdc4ec Remove some duplicate DTLS code.
In a couple of functions, a sequence number would be calculated twice.

Additionally, in |dtls1_process_out_of_seq_message|, we know that
|frag_len| <= |msg_hdr->msg_len| so the later tests for |frag_len <
msg_hdr->msg_len| can be more clearly written as |frag_len !=
msg_hdr->msg_len|, since that's the only remaining case.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2014-08-06 20:27:51 +01:00
Matt Caswell
0c37aed3f3 Applying same fix as in dtls1_process_out_of_seq_message. A truncated DTLS fragment would cause *ok to be clear, but the return value would still be the number of bytes read.
Problem identified by Emilia Käsper, based on previous issue/patch by Adam
Langley.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2014-08-06 20:27:51 +01:00
Adam Langley
099ccdb808 Fix return code for truncated DTLS fragment.
Previously, a truncated DTLS fragment in
|dtls1_process_out_of_seq_message| would cause *ok to be cleared, but
the return value would still be the number of bytes read. This would
cause |dtls1_get_message| not to consider it an error and it would
continue processing as normal until the calling function noticed that
*ok was zero.

I can't see an exploit here because |dtls1_get_message| uses
|s->init_num| as the length, which will always be zero from what I can
see.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2014-08-06 20:27:51 +01:00
Adam Langley
9871417fb7 Fix memory leak from zero-length DTLS fragments.
The |pqueue_insert| function can fail if one attempts to insert a
duplicate sequence number. When handling a fragment of an out of
sequence message, |dtls1_process_out_of_seq_message| would not call
|dtls1_reassemble_fragment| if the fragment's length was zero. It would
then allocate a fresh fragment and attempt to insert it, but ignore the
return value, leaking the fragment.

This allows an attacker to exhaust the memory of a DTLS peer.

Fixes CVE-2014-3507

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2014-08-06 20:27:51 +01:00
Matt Caswell
fc7804ec39 Fix DTLS handshake message size checks.
In |dtls1_reassemble_fragment|, the value of
|msg_hdr->frag_off+frag_len| was being checked against the maximum
handshake message size, but then |msg_len| bytes were allocated for the
fragment buffer. This means that so long as the fragment was within the
allowed size, the pending handshake message could consume 16MB + 2MB
(for the reassembly bitmap). Approx 10 outstanding handshake messages
are allowed, meaning that an attacker could consume ~180MB per DTLS
connection.

In the non-fragmented path (in |dtls1_process_out_of_seq_message|), no
check was applied.

Fixes CVE-2014-3506

Wholly based on patch by Adam Langley with one minor amendment.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2014-08-06 20:27:51 +01:00
Matt Caswell
e7b9d9be48 Added comment for the frag->reassembly == NULL case as per feedback from Emilia
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2014-08-06 20:27:51 +01:00
Adam Langley
2172d4f63c Avoid double free when processing DTLS packets.
The |item| variable, in both of these cases, may contain a pointer to a
|pitem| structure within |s->d1->buffered_messages|. It was being freed
in the error case while still being in |buffered_messages|. When the
error later caused the |SSL*| to be destroyed, the item would be double
freed.

Thanks to Wah-Teh Chang for spotting that the fix in 1632ef74 was
inconsistent with the other error paths (but correct).

Fixes CVE-2014-3505

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2014-08-06 20:27:51 +01:00
Dr. Stephen Henson
aa59369b4c Fix null pointer errors.
PR#3394
(cherry picked from commit 7a9d59c148)
2014-06-10 14:48:07 +01:00
Dr. Stephen Henson
d3152655d5 Fix CVE-2014-0221
Unnecessary recursion when receiving a DTLS hello request can be used to
crash a DTLS client. Fixed by handling DTLS hello request without recursion.

Thanks to Imre Rad (Search-Lab Ltd.) for discovering this issue.
2014-06-05 09:04:27 +01:00
Dr. Stephen Henson
1632ef7448 Fix for CVE-2014-0195
A buffer overrun attack can be triggered by sending invalid DTLS fragments
to an OpenSSL DTLS client or server. This is potentially exploitable to
run arbitrary code on a vulnerable client or server.

Fixed by adding consistency check for DTLS fragments.

Thanks to Jüri Aedla for reporting this issue.
2014-06-05 09:04:27 +01:00
Sami Farin
1dd26414df Typo: set i to -1 before goto.
PR#3302
(cherry picked from commit 9717f01951f976f76dd40a38d9fc7307057fa4c4)
2014-06-02 14:22:06 +01:00
Dr. Stephen Henson
96db9023b8 Add heartbeat extension bounds check.
A missing bounds check in the handling of the TLS heartbeat extension
can be used to reveal up to 64k of memory to a connected client or
server.

Thanks for Neel Mehta of Google Security for discovering this bug and to
Adam Langley <agl@chromium.org> and Bodo Moeller <bmoeller@acm.org> for
preparing the fix (CVE-2014-0160)
2014-04-07 17:53:31 +01:00
Dr. Stephen Henson
34628967f1 Fix DTLS retransmission from previous session.
For DTLS we might need to retransmit messages from the previous session
so keep a copy of write context in DTLS retransmission buffers instead
of replacing it after sending CCS. CVE-2013-6450.
2013-12-20 23:12:18 +00:00
Michael Tuexen
3972dbe462 Avoid unnecessary fragmentation.
(cherry picked from commit 80ccc66d7e)
2013-03-18 14:33:09 +00:00
Dr. Stephen Henson
25bfdca16a PR: 2755
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>

Reduce MTU after failed transmissions.
2012-03-06 13:47:27 +00:00
Dr. Stephen Henson
a54ce007e6 PR: 2739
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>

Fix padding bugs in Heartbeat support.
2012-02-27 16:38:10 +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
5372f5f989 PR: 2628
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Reviewed by: steve

Send alert instead of assertion failure for incorrectly formatted DTLS
fragments.
2011-10-27 13:06:43 +00:00
Dr. Stephen Henson
56f5ab43c2 PR: 2602
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Reviewed by: steve

Fix DTLS bug which prevents manual MTU setting
2011-09-23 13:35:05 +00:00
Dr. Stephen Henson
2ab42de1ec PR: 2462
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Reviewed by: steve

Fix DTLS Retransmission Buffer Bug
2011-04-03 17:14:48 +00:00
Dr. Stephen Henson
ac2024ccbf PR: 2458
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Reviewed by: steve

Don't change state when answering DTLS ClientHello.
2011-04-03 16:25:54 +00:00
Dr. Stephen Henson
93164a7d64 PR: 2457
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Reviewed by: steve

Fix DTLS fragment reassembly bug.
2011-04-03 15:48:32 +00:00
Dr. Stephen Henson
8c1e7de6cb PR: 2230
Submitted By: Robin Seggelmann <seggelmann@fh-muenster.de>

Fix bug in bitmask macros and stop warnings.
2010-05-03 13:01:50 +00:00
Dr. Stephen Henson
9f827ded1c fix signed/unsigned comparison warnings 2010-04-14 00:41:01 +00:00
Dr. Stephen Henson
1507f3abba PR: 2230
Submitted By: Robin Seggelmann <seggelmann@fh-muenster.de>

Fix various DTLS fragment reassembly bugs.
2010-04-14 00:17:29 +00:00
Dr. Stephen Henson
7b52778eff PR: 1731 and maybe 2197
Clear error queue in a few places in SSL code where errors are expected
so they don't stay in the queue.
2010-03-24 23:16:49 +00:00
Dr. Stephen Henson
d5b8c46499 PR: 2115
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Approved by: steve@openssl.org

Add Renegotiation extension to DTLS, fix DTLS ClientHello processing bug.
2009-12-01 17:41:42 +00:00
Dr. Stephen Henson
23b97c6bb5 PR: 2089
Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Approved by: steve@openssl.org

DTLS Fragment size bug fix.
2009-11-02 13:37:17 +00:00
Dr. Stephen Henson
29c2fd46d2 PR: 2054
Submitted by: Julia Lawall <julia@diku.dk>
Approved by: steve@openssl.org

Correct BIO_ctrl error handling
2009-10-01 00:03:50 +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