Commit graph

209 commits

Author SHA1 Message Date
Matt Caswell
f6d64b5142 Don't restrict the number of KeyUpdate messages we can process
Prior to this commit we were keeping a count of how many KeyUpdates we
have processed and failing if we had had too many. This simplistic approach
is not sufficient for long running connections. Since many KeyUpdates
would not be a particular good DoS route anyway, the simplest solution is
to simply remove the key update count.

Fixes #8068

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/8299)

(cherry picked from commit 3409a5ff8a)
2019-02-22 18:30:05 +00:00
Matt Caswell
37857e9b52 Don't signal SSL_CB_HANDSHAKE_START for TLSv1.3 post-handshake messages
The original 1.1.1 design was to use SSL_CB_HANDSHAKE_START and
SSL_CB_HANDSHAKE_DONE to signal start/end of a post-handshake message
exchange in TLSv1.3. Unfortunately experience has shown that this confuses
some applications who mistake it for a TLSv1.2 renegotiation. This means
that KeyUpdate messages are not handled properly.

This commit removes the use of SSL_CB_HANDSHAKE_START and
SSL_CB_HANDSHAKE_DONE to signal the start/end of a post-handshake
message exchange. Individual post-handshake messages are still signalled in
the normal way.

This is a potentially breaking change if there are any applications already
written that expect to see these TLSv1.3 events. However, without it,
KeyUpdate is not currently usable for many applications.

Fixes #8069

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8096)

(cherry picked from commit 4af5836b55)
2019-02-14 16:25:44 +00:00
Matt Caswell
8e3df4012a Revert "Keep the DTLS timer running after the end of the handshake if appropriate"
This commit erroneously kept the DTLS timer running after the end of the
handshake. This is not correct behaviour and shold be reverted.

This reverts commit f7506416b1.

Fixes #7998

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8047)

(cherry picked from commit bcc1f3e2ba)
2019-01-24 13:44:29 +00:00
Paul Yang
9465e71639 Fix access zero memory if SSL_DEBUG is enabled
If compile OpenSSL with SSL_DEBUG macro, some test cases will cause the
process crashed in the debug code.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7707)

(cherry picked from commit 5a4481f0e0)
2018-11-27 11:27:18 +08:00
Matt Caswell
eaa32f3679 Fix no-ec and no-tls1_2
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7620)

(cherry picked from commit 65d2c16cbe)
2018-11-14 11:33:24 +00:00
Matt Caswell
b4970e8bf5 Separate ca_names handling for client and server
SSL(_CTX)?_set_client_CA_list() was a server side only function in 1.1.0.
If it was called on the client side then it was ignored. In 1.1.1 it now
makes sense to have a CA list defined for both client and server (the
client now sends it the the TLSv1.3 certificate_authorities extension).
Unfortunately some applications were using the same SSL_CTX for both
clients and servers and this resulted in some client ClientHellos being
excessively large due to the number of certificate authorities being sent.

This commit seperates out the CA list updated by
SSL(_CTX)?_set_client_CA_list() and the more generic
SSL(_CTX)?_set0_CA_list(). This means that SSL(_CTX)?_set_client_CA_list()
still has no effect on the client side. If both CA lists are set then
SSL(_CTX)?_set_client_CA_list() takes priority.

Fixes #7411

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7503)

(cherry picked from commit 9873297900)
2018-11-12 14:38:47 +00:00
Matt Caswell
6f54ae7a90 Don't negotiate TLSv1.3 if our EC cert isn't TLSv1.3 capable
TLSv1.3 is more restrictive about the curve used. There must be a matching
sig alg defined for that curve. Therefore if we are using some other curve
in our certificate then we should not negotiate TLSv1.3.

Fixes #7435

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7442)

(cherry picked from commit de4dc59802)
2018-11-12 11:19:58 +00:00
Matt Caswell
a2388b50af Don't call the client_cert_cb immediately in TLSv1.3
In TLSv1.2 and below a CertificateRequest is sent after the Certificate
from the server. This means that by the time the client_cert_cb is called
on receipt of the CertificateRequest a call to SSL_get_peer_certificate()
will return the server certificate as expected. In TLSv1.3 a
CertificateRequest is sent before a Certificate message so calling
SSL_get_peer_certificate() returns NULL.

To workaround this we delay calling the client_cert_cb until after we
have processed the CertificateVerify message, when we are doing TLSv1.3.

Fixes #7384

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7413)

(cherry picked from commit e45620140f)
2018-10-30 12:18:55 +00:00
Richard Levitte
7ccfce81db ssl/statem: Don't compare size_t with less than zero
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7194)

(cherry picked from commit 60690b5b83)
2018-10-29 14:25:45 +01:00
Matt Caswell
cd3b53b8f8 Ensure certificate callbacks work correctly in TLSv1.3
The is_tls13_capable() function should not return 0 if no certificates
are configured directly because a certificate callback is present.

Fixes #7140

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7141)
2018-09-07 11:20:37 +01:00
Matt Caswell
1bf4cb0fe3 Process KeyUpdate and NewSessionTicket messages after a close_notify
If we've sent a close_notify then we are restricted about what we can do
in response to handshake messages that we receive. However we can sensibly
process NewSessionTicket messages. We can also process a KeyUpdate message
as long as we also ignore any request for us to update our sending keys.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7114)
2018-09-07 11:15:20 +01:00
Matt Caswell
5627f9f217 Don't detect a downgrade where the server has a protocol version hole
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7013)
2018-08-22 15:15:19 +01:00
Matt Caswell
b5b993b229 Use the same min-max version range on the client consistently
We need to ensure that the min-max version range we use when constructing
the ClientHello is the same range we use when we validate the version
selected by the ServerHello. Otherwise this may appear as a fallback or
downgrade.

Fixes #6964

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7013)
2018-08-22 15:15:19 +01:00
Matt Caswell
9f22c52723 Turn on TLSv1.3 downgrade protection by default
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6741)
2018-08-15 12:33:30 +01:00
Matt Caswell
35e742ecac Update code for the final RFC version of TLSv1.3 (RFC8446)
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6741)
2018-08-15 12:33:30 +01:00
Dmitry Yakovlev
572fa0249d Move SSL_DEBUG md fprintf after assignment
To avoid crash (same as #5138 fixed in 44f23cd)

CLA: trivial

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6937)
2018-08-14 08:01:14 -04:00
Matt Caswell
5df2206048 Improve fallback protection
A client that has fallen back could detect an inappropriate fallback if
the TLSv1.3 downgrade protection sentinels are present.

Fixes #6756

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6894)
2018-08-09 10:53:09 +01:00
Matt Caswell
de9e884b2f Tolerate encrypted or plaintext alerts
At certain points in the handshake we could receive either a plaintext or
an encrypted alert from the client. We should tolerate both where
appropriate.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6887)
2018-08-08 10:16:58 +01:00
Andy Polyakov
9ef9088c15 ssl/*: switch to switch to Thread-Sanitizer-friendly primitives.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6786)
2018-08-07 09:08:23 +02:00
Matt Caswell
d8434cf856 Validate legacy_version
The spec says that a client MUST set legacy_version to TLSv1.2, and
requires servers to verify that it isn't SSLv3.

Fixes #6600

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6747)
2018-07-20 10:52:02 +01:00
Matt Caswell
d162340d36 Fix no-psk
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6724)
2018-07-17 11:07:22 +01:00
Matt Caswell
baa45c3e74 As a server don't select TLSv1.3 if we're not capable of it
Check that we are either configured for PSK, or that we have a TLSv1.3
capable certificate type. DSA certs can't be used in TLSv1.3 and we
don't (currently) allow GOST ones either (owing to the lack of standard
sig algs).

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6650)
2018-07-13 18:14:29 +01:00
Matt Caswell
4fd12788eb Use ssl_version_supported() when choosing server version
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6650)
2018-07-13 18:14:29 +01:00
Matt Caswell
4cb004573a Remove TLSv1.3 tickets from the client cache as we use them
Tickets are supposed to be single use so we remove them from the cache on
use.

Fixes #6377

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/6601)
2018-07-03 09:44:46 +01:00
Matt Caswell
73cc84a132 Suport TLSv1.3 draft 28
Also retains support for drafts 27 and 26

Fixes #6257

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6258)
2018-05-15 10:02:59 +01:00
Matt Caswell
c0638adeec Fix ticket callbacks in TLSv1.3
The return value from the ticket_key callback was not properly handled in
TLSv1.3, so that a ticket was *always* renewed even if the callback
requested that it should not be.

Also the ticket decrypt callback was not being called at all in TLSv1.3.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6198)
2018-05-11 14:51:08 +01:00
Matt Caswell
f7506416b1 Keep the DTLS timer running after the end of the handshake if appropriate
During a full handshake the server is the last one to "speak". The timer
should continue to run until we know that the client has received our last
flight (e.g. because we receive some application data).

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6170)
2018-05-08 09:40:17 +01:00
Matt Caswell
447cc0ad73 In a reneg use the same client_version we used last time
In 1.0.2 and below we always send the same client_version in a reneg
ClientHello that we sent the first time around, regardless of what
version eventually gets negotiated. According to a comment in
statem_clnt.c this is a workaround for some buggy servers that choked if
we changed the version used in the RSA encrypted premaster secret.

In 1.1.0+ this behaviour no longer occurs. This restores the original
behaviour.

Fixes #1651

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6059)
2018-04-24 09:54:31 +01:00
Matt Caswell
22eb2d1c80 Remove some logically dead code
This dead code should have been removed as part of #5874 but got missed.

Found by Coverity.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/6049)
2018-04-24 09:16:20 +01:00
Matt Caswell
4ce787b97a Make sure SSL_in_init() returns 0 at SSL_CB_HANDSHAKE_DONE
In 1.1.0 and before calling SSL_in_init() from the info_callback
at SSL_CB_HANDSHAKE_DONE would return 0. This commit fixes it so
that it does again for 1.1.1. This broke Node.

Fixes #4574

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6019)
2018-04-20 11:46:12 +01:00
Matt Caswell
c2c1d8a495 Call the info callback on all handshake done events
Fixes #5721

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5874)
2018-04-17 16:51:03 +01:00
Rich Salz
c6d38183d6 Rewrite the X509->alert mapping code
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5780)
2018-03-28 15:34:16 -04:00
Matt Caswell
424afe931e Don't wait for dry at the end of a handshake
For DTLS/SCTP we were waiting for a dry event during the call to
tls_finish_handshake(). This function just tidies up various internal
things, and after it completes the handshake is over. I can find no good
reason for waiting for a dry event here, and nothing in RFC6083 suggests
to me that we should need to. More importantly though it seems to be
wrong. It is perfectly possible for a peer to send app data/alerts/new
handshake while we are still cleaning up our handshake. If this happens
then we will never get the dry event and so we cannot continue.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5084)
2018-03-21 10:27:29 +00:00
Matt Caswell
16ff13427f Only update the server session cache when the session is ready
In TLSv1.3 the session is not ready until after the end of the handshake
when we are constructing the NewSessionTicket.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5621)
2018-03-15 08:59:27 +00:00
Matt Caswell
5d67110173 Don't calculate the Finished MAC twice
In <= TLSv1.2 a Finished message always comes immediately after a CCS
except in the case of NPN where there is an additional message between
the CCS and Finished. Historically we always calculated the Finished MAC
when we processed the CCS. However to deal with NPN we also calculated it
when we receive the Finished message. Really this should only have been
done if we hand negotiated NPN.

This simplifies the code to only calculate the MAC when we receive the
Finished. In 1.1.1 we need to do it this way anyway because there is no
CCS (except in middlebox compat mode) in TLSv1.3.

Coincidentally, this commit also fixes the fact that no-nextprotoneg does
not currently work in master.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5285)
2018-02-09 15:27:32 +00:00
Todd Short
9d75dce3e1 Add TLSv1.3 post-handshake authentication (PHA)
Add SSL_verify_client_post_handshake() for servers to initiate PHA

Add SSL_force_post_handshake_auth() for clients that don't have certificates
initially configured, but use a certificate callback.

Update SSL_CTX_set_verify()/SSL_set_verify() mode:

* Add SSL_VERIFY_POST_HANDSHAKE to postpone client authentication until after
the initial handshake.

* Update SSL_VERIFY_CLIENT_ONCE now only sends out one CertRequest regardless
of when the certificate authentication takes place; either initial handshake,
re-negotiation, or post-handshake authentication.

Add 'RequestPostHandshake' and 'RequirePostHandshake' SSL_CONF options that
add the SSL_VERIFY_POST_HANDSHAKE to the 'Request' and 'Require' options

Add support to s_client:
* Enabled automatically when cert is configured
* Can be forced enabled via -force_pha

Add support to s_server:
* Use 'c' to invoke PHA in s_server
* Remove some dead code

Update documentation

Update unit tests:
* Illegal use of PHA extension
* TLSv1.3 certificate tests

DTLS and TLS behave ever-so-slightly differently. So, when DTLS1.3 is
implemented, it's PHA support state machine may need to be different.
Add a TODO and a #error

Update handshake context to deal with PHA.

The handshake context for TLSv1.3 post-handshake auth is up through the
ClientFinish message, plus the CertificateRequest message. Subsequent
Certificate, CertificateVerify, and Finish messages are based on this
handshake context (not the Certificate message per se, but it's included
after the hash). KeyUpdate, NewSessionTicket, and prior Certificate
Request messages are not included in post-handshake authentication.

After the ClientFinished message is processed, save off the digest state
for future post-handshake authentication. When post-handshake auth occurs,
copy over the saved handshake context into the "main" handshake digest.
This effectively discards the any KeyUpdate or NewSessionTicket messages
and any prior post-handshake authentication.

This, of course, assumes that the ID-22 did not mean to include any
previous post-handshake authentication into the new handshake transcript.
This is implied by section 4.4.1 that lists messages only up to the
first ClientFinished.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4964)
2018-02-01 17:07:56 +00:00
Matt Caswell
3faa07b582 Move decisions about whether to accept reneg into the state machine
If a server receives an unexpected ClientHello then we may or may not
accept it. Make sure all such decisions are made in the state machine
and not in the record layer. This also removes a disparity between the
TLS and the DTLS code. The TLS code was making this decision in the
record layer, while the DTLS code was making it later.

Finally it also solves a problem where a warning alert was being sent
during tls_setup_handshake() and the function was returning a failure
return code. This is problematic because it can be called from a
transition function - which we only allow fatal errors to occur in.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5190)
2018-01-30 11:28:12 +00:00
Matt Caswell
e93597193d Don't send unexpected_message if we receive CCS while stateless
Probably this is the CCS between the first and second ClientHellos. It
should be ignored.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4435)
2018-01-24 18:02:37 +00:00
Matt Caswell
43054d3d73 Add support for sending TLSv1.3 cookies
This just adds the various extension functions. More changes will be
required to actually use them.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4435)
2018-01-24 18:02:35 +00:00
Matt Caswell
2a8db71713 Don't flush the ClientHello if we're going to send early data
We'd like the first bit of early_data and the ClientHello to go in the
same TCP packet if at all possible to enable things like TCP Fast Open.
Also, if you're only going to send one block of early data then you also
don't need to worry about TCP_NODELAY.

Fixes #4783

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4802)
2017-12-28 17:32:41 +00:00
Matt Caswell
fc7129dc37 Update state machine to send CCS based on whether we did an HRR
The CCS may be sent at different times based on whether or not we
sent an HRR earlier. In order to make that decision this commit
also updates things to make sure we remember whether an HRR was
used or not.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
2017-12-14 15:06:37 +00:00
Matt Caswell
6f40214f68 Fix an HRR bug
Ensure that after an HRR we can only negotiate TLSv1.3

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
2017-12-14 15:06:37 +00:00
Matt Caswell
597c51bc98 Merge HRR into ServerHello
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
2017-12-14 15:06:37 +00:00
Matt Caswell
88050dd196 Update ServerHello to new draft-22 format
The new ServerHello format is essentially now the same as the old TLSv1.2
one, but it must additionally include supported_versions. The version
field is fixed at TLSv1.2, and the version negotiation happens solely via
supported_versions.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
2017-12-14 15:06:37 +00:00
Matt Caswell
29bfd5b79a Add some more cleanups
Follow up from the conversion to use SSLfatal() in the state machine to
clean things up a bit more.

[extended tests]

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
2017-12-04 13:31:48 +00:00
Matt Caswell
d4d2f3a4c1 Convert more functions in ssl/statem/statem.c to use SSLfatal()
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
2017-12-04 13:31:48 +00:00
Matt Caswell
a2c2e00050 Convert remaining functions in statem_clnt.c to use SSLfatal()
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
2017-12-04 13:31:48 +00:00
Matt Caswell
f63a17d66d Convert the state machine code to use SSLfatal()
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
2017-12-04 13:31:48 +00:00
Matt Caswell
4752c5deb2 Replace some usage of SSLerr with SSLfatal()
This is an initial step towards using SSLfatal() everywhere. Initially in
this commit and in subsequent commits we focus on the state machine code.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
2017-12-04 13:31:48 +00:00
Andy Polyakov
3a63c0edab Resolve warnings in VC-WIN32 build, which allows to add /WX.
It's argued that /WX allows to keep better focus on new code, which
motivates its comeback...

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4721)
2017-11-13 10:58:57 +01:00