Commit graph

1599 commits

Author SHA1 Message Date
Matt Caswell
5802758eb4 Update function error code
A function error code needed updating due to merge issues.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-08-19 14:05:09 +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
Matt Caswell
6f35f6deb5 Avoid some undefined pointer arithmetic
A common idiom in the codebase is:

if (p + len > limit)
{
    return; /* Too long */
}

Where "p" points to some malloc'd data of SIZE bytes and
limit == p + SIZE

"len" here could be from some externally supplied data (e.g. from a TLS
message).

The rules of C pointer arithmetic are such that "p + len" is only well
defined where len <= SIZE. Therefore the above idiom is actually
undefined behaviour.

For example this could cause problems if some malloc implementation
provides an address for "p" such that "p + len" actually overflows for
values of len that are too big and therefore p + len < limit!

Issue reported by Guido Vranken.

CVE-2016-2177

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-06-01 14:25:03 +01:00
Viktor Dukhovni
3d4f83a5c4 Ensure verify error is set when X509_verify_cert() fails
Set ctx->error = X509_V_ERR_OUT_OF_MEM when verificaiton cannot
continue due to malloc failure.  Similarly for issuer lookup failures
and caller errors (bad parameters or invalid state).

Also, when X509_verify_cert() returns <= 0 make sure that the
verification status does not remain X509_V_OK, as a last resort set
it it to X509_V_ERR_UNSPECIFIED, just in case some code path returns
an error without setting an appropriate value of ctx->error.

Add new and some missing error codes to X509 error -> SSL alert switch.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-05-26 16:36:49 -04:00
David Benjamin
7a433893ad Fix memory leak on invalid CertificateRequest.
Free up parsed X509_NAME structure if the CertificateRequest message
contains excess data.

The security impact is considered insignificant. This is a client side
only leak and a large number of connections to malicious servers would
be needed to have a significant impact.

This was found by libFuzzer.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Stephen Henson <steve@openssl.org>
(cherry picked from commit ec66c8c988)
2016-04-07 19:27:45 +01:00
Matt Caswell
4275ee389b Add a check for a failed malloc
Ensure we check for a NULL return from OPENSSL_malloc

Issue reported by Guido Vranken.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-03-18 11:59:11 +00:00
Matt Caswell
d31b25138f Ensure that memory allocated for the ticket is freed
If a call to EVP_DecryptUpdate fails then a memory leak could occur.
Ensure that the memory is freed appropriately.

Issue reported by Guido Vranken.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-03-18 11:59:11 +00:00
Kurt Roeckx
6629966097 Add no-ssl2-method
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>

MR: #2341
(cherry picked from commit 4256957570)
2016-03-14 21:17:18 +01:00
Viktor Dukhovni
03c71b84d3 expose SSLv2 method prototypes
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2016-03-09 03:13:06 -05:00
Viktor Dukhovni
5bac9d44e7 Retain SSLv2 methods as functions that return NULL
This improves ABI compatibility when symbol resolution is not lazy.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-03-08 09:08:28 -05:00
Kurt Roeckx
6e7a1f35b7 Remove LOW from the default
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit 29cce50897)
2016-03-07 18:57:40 +01:00
Matt Caswell
5d2b93ad7b make update
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-03-01 13:40:45 +00:00
Viktor Dukhovni
abd5d8fbef Disable EXPORT and LOW SSLv3+ ciphers by default
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-03-01 11:24:02 +00:00
Viktor Dukhovni
56f1acf5ef Disable SSLv2 default build, default negotiation and weak ciphers.
SSLv2 is by default disabled at build-time.  Builds that are not
configured with "enable-ssl2" will not support SSLv2.  Even if
"enable-ssl2" is used, users who want to negotiate SSLv2 via the
version-flexible SSLv23_method() will need to explicitly call either
of:

    SSL_CTX_clear_options(ctx, SSL_OP_NO_SSLv2);
or
    SSL_clear_options(ssl, SSL_OP_NO_SSLv2);

as appropriate.  Even if either of those is used, or the application
explicitly uses the version-specific SSLv2_method() or its client
or server variants, SSLv2 ciphers vulnerable to exhaustive search
key recovery have been removed.  Specifically, the SSLv2 40-bit
EXPORT ciphers, and SSLv2 56-bit DES are no longer available.

Mitigation for CVE-2016-0800

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-03-01 11:23:45 +00:00
Viktor Dukhovni
4040a7fd10 Better SSLv2 cipher-suite enforcement
Based on patch by: Nimrod Aviram <nimrod.aviram@gmail.com>

CVE-2015-3197

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-01-28 17:06:38 +00:00
Matt Caswell
8bc643efc8 Always generate DH keys for ephemeral DH cipher suites
Modified version of the commit ffaef3f15 in the master branch by Stephen
Henson. This makes the SSL_OP_SINGLE_DH_USE option a no-op and always
generates a new DH key for every handshake regardless.

This is a follow on from CVE-2016-0701. This branch is not impacted by
that CVE because it does not support X9.42 style parameters. It is still
possible to generate parameters based on primes that are not "safe",
although by default OpenSSL does not do this. The documentation does
sign post that using such parameters is unsafe if the private DH key is
reused. However to avoid accidental problems or future attacks this commit
has been backported to this branch.

Issue reported by Antonio Sanso

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
2016-01-28 10:27:55 +00:00
Alessandro Ghedini
51223748e5 Validate ClientHello session_id field length and send alert on failure
RT#4080

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-01-19 15:42:23 +00:00
Viktor Dukhovni
e9a6c72e3c Empty SNI names are not valid
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-01-16 21:14:02 -05:00
Kurt Roeckx
f5fc9404c2 Change minimum DH size from 768 to 1024
Reviewed-by: Viktor Dukhovni <openssl-users@dukhovni.org>
2016-01-11 00:13:54 +01:00
Matt Caswell
604f67f521 Ensure we don't call the OCSP callback if resuming a session
It makes no sense to call the OCSP status callback if we are resuming a
session because no certificates will be sent.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit 0ac6239955)
2015-12-27 22:05:36 +00:00
Matt Caswell
a7316aace3 Fix error when server does not send CertificateStatus message
If a server sends the status_request extension then it may choose
to send the CertificateStatus message. However this is optional.
We were treating it as mandatory and the connection was failing.

Thanks to BoringSSL for reporting this issue.

RT#4120

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit 905943af3b)
2015-12-27 22:05:36 +00:00
Matt Caswell
583f4bf7e8 Fix more URLs mangled by reformat
Fix some more URLs mangled by indent in the reformat. These ones don't exist
in master so we have a separate commit. Based on a patch supplied by Arnaud
Lacombe <al@aerilon.ca>

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-12-19 20:40:39 +00:00
Richard Levitte
e961c7a201 Remove the "eay" c-file-style indicators
Since we don't use the eay style any more, there's no point tryint to
tell emacs to use it.

Reviewed-by: Ben Laurie <ben@openssl.org>
2015-12-18 13:13:31 +01:00
Matt Caswell
f612bdb342 Ensure |rwstate| is set correctly on BIO_flush
A BIO_flush call in the DTLS code was not correctly setting the |rwstate|
variable to SSL_WRITING. This means that SSL_get_error() will not return
SSL_ERROR_WANT_WRITE in the event of an IO retry.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 67f60be8c9)
2015-12-10 12:50:56 +00:00
Matt Caswell
4a53424318 Fix DTLS handshake fragment retries
If using DTLS and NBIO then if a second or subsequent handshake message
fragment hits a retry, then the retry attempt uses the wrong fragment
offset value. This commit restores the fragment offset from the last
attempt.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 2ad226e88b)
2015-12-10 12:50:55 +00:00
Dr. Stephen Henson
d585cc32a5 typo
Reviewed-by: Andy Polyakov <appro@openssl.org>
2015-12-02 21:49:37 +00:00
Richard Levitte
fb35ea28f4 _BSD_SOURCE is deprecated, use _DEFAULT_SOURCE instead
The feature_test_macros(7) manual tells us that _BSD_SOURCE is
deprecated since glibc 2.20 and that the compiler will warn about it
being used, unless _DEFAULT_SOURCE is defined as well.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit f9fd35248c)
2015-12-02 18:49:57 +01:00
Matt Caswell
41d049e1cd Return errors even if the cookie validation has succeeded
In the DTLS ClientHello processing the return value is stored in |ret| which
by default is -1. We wish to return 1 on success or 2 on success *and* we
have validated the DTLS cookie. Previously on successful validation of the
cookie we were setting |ret| to 2. Unfortunately if we later encounter an
error then we can end up returning a successful (positive) return code from
the function because we already set |ret| to a positive value.

This does not appear to have a security consequence because the handshake
just fails at a later point.

Reviewed-by: Andy Polyakov <appro@openssl.org>
2015-11-30 10:51:43 +00:00
Pascal Cuoq
784934498f ssl3_free(): Return if it wasn't created
If somewhere in SSL_new() there is a memory allocation failure, ssl3_free() can
get called with s->s3 still being NULL.

Patch also provided by Willy Tarreau <wtarreau@haproxy.com>

Signed-off-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Viktor Dukhovni <openssl-users@dukhovni.org>
(cherry picked from commit 3e7bd2ce0b16f8611298175d6dc7cb35ee06ea6d)
2015-11-24 21:56:39 +01:00
Kurt Roeckx
0b5f9ce37b Set reference count earlier
Backport of 0e04674e96

Reviewed-by: Steve Henson <steve@openssl.org>

RT #4047, #4110, MR #1356
2015-11-24 21:53:40 +01:00
Matt Caswell
a5184a6c89 Ensure all EVP calls have their returns checked where appropriate
There are lots of calls to EVP functions from within libssl There were
various places where we should probably check the return value but don't.
This adds these checks.

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

Conflicts:
	ssl/s3_enc.c
	ssl/s3_srvr.c
2015-11-20 15:56:42 +00:00
Matt Caswell
78b9d13474 Stop DTLS servers asking for unsafe legacy renegotiation
If a DTLS client that does not support secure renegotiation connects to an
OpenSSL DTLS server then, by default, renegotiation is disabled. If a
server application attempts to initiate a renegotiation then OpenSSL is
supposed to prevent this. However due to a discrepancy between the TLS and
DTLS code, the server sends a HelloRequest anyway in DTLS.

This is not a security concern because the handshake will still fail later
in the process when the client responds with a ClientHello.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit d40ec4ab8e)
2015-11-10 19:27:25 +00:00
Matt Caswell
e83009840a Only call ssl3_init_finished_mac once for DTLS
In DTLS if an IO retry occurs during writing of a fragmented ClientHello
then we can end up reseting the finish mac variables on the retry, which
causes a handshake failure. We should only reset on the first attempt not
on retries.

Thanks to BoringSSL for reporting this issue.

RT#4119

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 15a7164eb7)
2015-11-10 18:47:57 +00:00
Matt Caswell
84d0c40f3f Fix missing malloc return value checks
During work on a larger change in master a number of locations were
identified where return value checks were missing. This backports the
relevant fixes.

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

Conflicts:
	crypto/cms/cms_sd.c
2015-11-09 23:00:37 +00:00
Alessandro Ghedini
200c8ed4f5 Remove useless code
RT#4081

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 070c23325a)
2015-10-23 20:47:53 +02:00
Alessandro Ghedini
edd0f5c201 Fix references to various RFCs
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 3240e7cf5f)
2015-10-23 20:43:09 +02:00
Alessandro Ghedini
71d5679cd3 Fix memory leaks and other mistakes on errors
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 3f6c769187)
2015-10-23 20:38:52 +02:00
Matt Caswell
f141376ae2 Change functions to pass in a limit rather than calculate it
Some extension handling functions were passing in a pointer to the start
of the data, plus the length in order to calculate the end, rather than
just passing in the end to start with. This change makes things a little
more readable.

Reviewed-by: Emilia Käsper <emilia@openssl.org>

Conflicts:
	ssl/s3_srvr.c
	ssl/ssl_locl.h
	ssl/t1_lib.c
2015-10-05 19:52:38 +01:00
Alessandro Ghedini
e4840c88c5 Validate ClientHello extension field length
RT#4069

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2015-10-05 19:48:28 +01:00
Emilia Kasper
64ec479559 RT2772: accept empty SessionTicket
RFC 5077 section 3.3 says: If the server determines that it does not
want to include a ticket after it has included the SessionTicket
extension in the ServerHello, then it sends a zero-length ticket in the
NewSessionTicket handshake message.

Previously the client would fail upon attempting to allocate a
zero-length buffer. Now, we have the client ignore the empty ticket and
keep the existing session.

Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 21b538d616)
2015-09-28 16:13:45 +02:00
Dr. Stephen Henson
2bc914eb29 Handle SSL_ERROR_WANT_X509_LOOKUP
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit f1c412c9e6)
2015-09-20 14:22:52 +01:00
Ivo Raisr
f95d1af064 Make no-psk compile without warnings.
PR#4035

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Stephen Henson <steve@openssl.org>
(cherry picked from commit 929f6d6f55)
2015-09-16 18:12:04 +01:00
Rich Salz
12650153ec RT4044: Remove .cvsignore files.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 3be39dc1e3)
2015-09-15 12:00:18 -04:00
Matt Caswell
dd642deea8 Fix session resumption
Commit f0348c842e introduced a problem with session resumption. The
version for the session is fixed when the session is created. By moving
the creation of the session earlier in the process the version is fixed
*before* version negotiation has completed when processing the ServerHello
on the client side. This fix updates the session version after version neg
has completed.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit dc0c888811)
2015-09-02 00:31:33 +01:00
Matt Caswell
927f7a8703 Fix building with OPENSSL_NO_TLSEXT.
Builds using no-tlsext in 1.0.0 and 0.9.8 are broken. This commit fixes the
issue. The same commit is applied to 1.0.1 and 1.0.2 branches for code
consistency. However this commit will not fix no-tlsext in those branches
which have always been broken for other reasons. The commit is not applied
to master at all, because no-tlsext has been completely removed from that
branch.

Based on a patch by Marc Branchaud <marcnarc@xiplink.com>

Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 9a931208d7)
2015-09-02 00:07:24 +01:00
Matt Caswell
be8b8603d6 Fix DTLS session ticket renewal
A DTLS client will abort a handshake if the server attempts to renew the
session ticket. This is caused by a state machine discrepancy between DTLS
and TLS discovered during the state machine rewrite work.

The bug can be demonstrated as follows:

Start a DTLS s_server instance:
openssl s_server -dtls

Start a client and obtain a session but no ticket:
openssl s_client -dtls -sess_out session.pem -no_ticket

Now start a client reusing the session, but allow a ticket:
openssl s_client -dtls -sess_in session.pem

The client will abort the handshake.

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

Conflicts:
	ssl/d1_clnt.c
2015-08-26 10:27:35 +01:00
Matt Caswell
396e300449 Fix "make test" seg fault with SCTP enabled
When config'd with "sctp" running "make test" causes a seg fault. This is
actually due to the way ssltest works - it dives under the covers and frees
up BIOs manually and so some BIOs are NULL when the SCTP code does not
expect it. The simplest fix is just to add some sanity checks to make sure
the BIOs aren't NULL before we use them.

This problem occurs in master and 1.0.2. The fix has also been applied to
1.0.1 to keep the code in sync.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit f75d5171be)
2015-08-11 22:27:05 +01:00
Matt Caswell
402634f8aa Fix missing return value checks in SCTP
There are some missing return value checks in the SCTP code. In master this
was causing a compilation failure when config'd with
"--strict-warnings sctp".

Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit d8e8590ed9)
2015-08-11 22:27:05 +01:00