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>
We can't rename ssleay_rand_bytes to md_rand_bytes_lock as this will cause
an error code discrepancy. Instead keep ssleay_rand_bytes and add an
extra parameter: since ssleay_rand_bytes is not part of the public API
this wont cause any binary compatibility issues. Reviewed-by: Kurt Roeckx <kurt@openssl.org >
(cherry picked from commit 8068a675a7d1a657c54546f24e673e59e6707f03)
Don't call internal functions directly call them through
SSL_test_functions(). This also makes unit testing work on
Windows and platforms that don't export internal functions
from shared libraries.
By default unit testing is not enabled: it requires the compile
time option "enable-unit-test". Reviewed-by: Geoff Thorpe <geoff@openssl.org>
(cherry picked from commit e0fc7961c4fbd27577fb519d9aea2dc788742715)
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()?)
Internal pointers in CCM, GCM and XTS contexts should either be
NULL or set to point to the appropriate key schedule. This needs
to be adjusted when copying contexts.
(cherry picked from commit c2fd5d79ffc4fc9d120a0faad579ce96473e6a2f)
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.
Hubert Kario [Tue, 10 Jun 2014 12:13:33 +0000 (14:13 +0200)]
backport changes to ciphers(1) man page
Backport of the patch:
add ECC strings to ciphers(1), point out difference between DH and ECDH
and few other changes applicable to the 1.0.1 code base.
* Make a clear distinction between DH and ECDH key exchange.
* Group all key exchange cipher suite identifiers, first DH then ECDH
* add descriptions for all supported *DH* identifiers
* add ECDSA authentication descriptions
* add example showing how to disable all suites that offer no
authentication or encryption
* backport listing of elliptic curve cipher suites.
* backport listing of TLS 1.2 cipher suites, add note that DH_RSA
and DH_DSS is not implemented in this version
* backport of description of PSK and listing of PSK cipher suites
* backport description of AES128, AES256 and AESGCM options
* backport description of CAMELLIA128, CAMELLIA256 options
Mike Bland [Sat, 7 Jun 2014 17:05:50 +0000 (13:05 -0400)]
Create test/testutil.h for unit test helper macros
Defines SETUP_TEST_FIXTURE and EXECUTE_TEST, and updates ssl/heartbeat_test.c
using these macros. SETUP_TEST_FIXTURE makes use of the new TEST_CASE_NAME
macro, defined to use __func__ or __FUNCTION__ on platforms that support those
symbols, or to use the file name and line number otherwise. This should fix
several reported build problems related to lack of C99 support.
SRP ciphersuites do not have no authentication. They have authentication
based on SRP. Add new SRP authentication flag and cipher string.
(cherry picked from commit a86b88acc373ac1fb0ca709a5fb8a8fa74683f67)
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().
projects / openssl / log