Emilia Kasper [Tue, 19 Aug 2014 11:18:07 +0000 (13:18 +0200)]
define inline for Visual Studio
In Visual Studio, inline is available in C++ only, however __inline is available for C, see
http://msdn.microsoft.com/en-us/library/z8y1yy88.aspx
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Dr Stephen Henson <steve@openssl.org>
(cherry picked from commit f511b25a7370c775dc9fd6198dbacd1706cf242b)
Adam Langley [Mon, 3 Jun 2013 19:45:11 +0000 (15:45 -0400)]
Add volatile qualifications to two blocks of inline asm to stop GCC from
eliminating them as dead code.
Both volatile and "memory" are used because of some concern that the compiler
may still cache values across the asm block without it, and because this was
such a painful debugging session that I wanted to ensure that it's never
repeated.
Adam Langley [Fri, 6 Jun 2014 21:47:07 +0000 (14:47 -0700)]
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>
Matt Caswell [Thu, 24 Jul 2014 22:54:28 +0000 (23:54 +0100)]
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.
Adam Langley [Fri, 6 Jun 2014 21:44:20 +0000 (14:44 -0700)]
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>
Adam Langley [Fri, 6 Jun 2014 21:30:33 +0000 (14:30 -0700)]
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>
Matt Caswell [Fri, 6 Jun 2014 21:25:52 +0000 (14:25 -0700)]
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.
Adam Langley [Fri, 6 Jun 2014 21:19:21 +0000 (14:19 -0700)]
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>
Tim Hudson [Mon, 21 Jul 2014 10:03:50 +0000 (20:03 +1000)]
Minor documentation update removing "really" and a
statement of opinion rather than a fact.
Reviewed-by: Dr. Stephen Henson <steve@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit c8d133e4b6f1ed1b7ad3c1a6d2c62f460e26c050)
PR#3456 Reviewed-by: Stephen Henson <steve@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit d48e78f0cf22aaddb563f4bcfccf25b1a45ac8a4)
Matt Caswell [Thu, 10 Jul 2014 22:47:31 +0000 (23:47 +0100)]
Fixed valgrind complaint due to BN_consttime_swap reading uninitialised data.
This is actually ok for this function, but initialised to zero anyway if
PURIFY defined.
This does have the impact of masking any *real* unitialised data reads in bn though.
defined(@array) is deprecated at ../util/mkerr.pl line 792.
(Maybe you should just omit the defined()?)
defined(@array) is deprecated at ../util/mkerr.pl line 800.
(Maybe you should just omit the defined()?)
Jeffrey Walton [Sun, 29 Jun 2014 22:34:21 +0000 (23:34 +0100)]
Clarified that the signature's buffer size, `s`, is not used as an
IN parameter.
Under the old docs, the only thing stated was "at most
EVP_PKEY_size(pkey) bytes will be written". It was kind of misleading
since it appears EVP_PKEY_size(pkey) WILL be written regardless of the
signature's buffer size.
If CSR verify fails in ca utility print out error messages.
Otherwise some errors give misleading output: for example
if the key size exceeds the library limit.
Allow CCS after finished has been sent by client: at this point
keys have been correctly set up so it is OK to accept CCS from
server. Without this renegotiation can sometimes fail.
Matt Caswell [Tue, 10 Jun 2014 22:24:28 +0000 (23:24 +0100)]
Fixed incorrect return code handling in ssl3_final_finish_mac.
Based on an original patch by Joel Sing (OpenBSD) who also originally identified the issue.
Jakub Wilk [Sat, 7 Jun 2014 12:37:17 +0000 (14:37 +0200)]
Create ~/.rnd with mode 0600 instead of 0666
Because of a missing include <fcntl.h> we don't have O_CREATE and don't create
the file with open() using mode 0600 but fall back to using fopen() with the
default umask followed by a chmod().