Commit Graph

720 Commits

Author SHA1 Message Date
Paul Yang
56d362881e Remove spaces at end of line in ssl/statem
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matt Caswell <matt@openssl.org>
GH: #4934
2017-12-17 23:04:41 +01:00
Matt Caswell
318d3c0e62 Issue a CCS from the client if we received an HRR
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
2017-12-14 15:06:38 +00:00
Matt Caswell
75259b4346 Fix server side HRR flushing
Flush following the CCS after an HRR. Only flush the HRR if middlebox
compat is turned off.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
2017-12-14 15:06:38 +00:00
Matt Caswell
5cc807da25 Delay flush until after CCS with early_data
Normally we flush immediately after writing the ClientHello. However if
we are going to write a CCS immediately because we've got early_data to
come, then we should move the flush until after the CCS.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
2017-12-14 15:06:38 +00:00
Matt Caswell
426dfc9ff7 Send supported_versions in an HRR
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
e7dd763e51 Make sure supported_versions appears in an HRR too
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
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
db37d32cb8 Send a CCS after ServerHello in TLSv1.3 if using middlebox compat mode
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
066904ccee Send a CCS from a client in an early_data handshake
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
4d02f87063 Send a CCS from the client in a non-early_data handshake
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
fa9f9350f3 Remove TLSv1.3 specific write transition for ClientHello
Since we no longer do version negotiation during the processing of an HRR
we do not need the TLSv1.3 specific write transition for ClientHello

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
a5816a5ab9 Implement session id TLSv1.3 middlebox compatibility mode
Clients will send a "fake" session id and servers must echo it back.

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
c285338293 More record layer conversions to use SSLfatal()
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4841)
2017-12-08 16:42:02 +00:00
Rich Salz
cbe2964821 Consistent formatting for sizeof(foo)
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4872)
2017-12-07 19:11:49 -05:00
Matt Caswell
e1dd8fa00a Fix the check_fatal macro
The check_fatal macro is supposed to only be called if we are already
expecting to be in the fatal state. The macro asserts that we are and
puts us into the fatal state if not.

This issue combined with the problem fixed in the previous commit meant
that the fuzzer detected a crash at a point in the processing when we
should have already been in the fatal state.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4847)
2017-12-06 14:56:28 +00:00
Matt Caswell
2894e9cb6d Fix bug in TLSv1.3 PSK processing
The recent SSL error overhaul left a case where an error occurs but
SSLfatal() is not called.

Credit to OSSfuzz for finding this issue.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4847)
2017-12-06 14:56:28 +00:00
Matt Caswell
dd5a4279f9 Fix some formatting nits
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
2017-12-04 13:37:01 +00:00
Matt Caswell
8e7677aeac Update an error reason code to be ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED
The most likely explanation for us ending up at this point in the code
is that we were called by the user application incorrectly - so use an
appropriate error 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
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
f9f674eb76 Assert that SSLfatal() only gets called once
We shouldn't call SSLfatal() multiple times for the same error condition.

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
47e2ee0722 Add some sanity checks for the fatal error condition
Sometimes at the top level of the state machine code we know we are
supposed to be in a fatal error condition. This commit adds some sanity
checks to ensure that SSLfatal() has been called.

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
635c8f7715 Fix up a few places in the state machine that got missed with 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
d273b60b41 Convert more functions in ssl/statem/statem_dtls.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
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
3ec8d113a5 Convert remaining functions in statem_srvr.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
Matt Caswell
1f359471f9 Provide an SSLfatal() macro
Typically if a fatal error occurs three things need to happen:

- Put an error on the error queue
- Send an alert
- Put the state machine into the error state

Although all 3 of these things need to be done every time we hit a fatal
error the responsibilities for doing this are distributed throughout the
code. The place where the error goes on the queue, where the alert gets
sent and where the state machine goes into the error state are almost
invariably different. It has been a common pattern to pass alert codes up
and down the stack to get the alert information from the point in the code
where the error is detected to the point in the code where the alert gets
sent.

This commit provides an SSLfatal() macro (backed by an ossl_statem_fatal
function) that does all 3 of the above error tasks. This is largely a drop
in replacement for SSLerr, but takes a couple of extra parameters (the SSL
object, and an alert 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
Matt Caswell
281bf2332c If a server is not acknowledging SNI then don't reject early_data
SNI needs to be consistent before we accept early_data. However a
server may choose to not acknowledge SNI. In that case we have to
expect that a client may send it anyway. We change the consistency
checks so that not acknowledging is treated more a like a "wild card",
accepting any SNI as being consistent.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4738)
2017-11-21 17:46:22 +00:00
Matt Caswell
bfab12bb7d Allow a client to send early_data with SNI if the session has no SNI
We can only send early_data if the SNI is consistent. However it is valid
for the client to set SNI and the server to not use it. This would still be
counted as consistent. OpenSSL client was being overzealous in this check
and disallowing this scenario.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4738)
2017-11-21 17:46:22 +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
FdaSilvaYY
cf72c75792 Implement Maximum Fragment Length TLS extension.
Based on patch from Tomasz Moń:
https://groups.google.com/forum/#!topic/mailing.openssl.dev/fQxXvCg1uQY

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/1008)
2017-11-05 17:46:48 +01:00
Benjamin Kaduk
3be08e3011 Provide SSL_CTX.stats.sess_accept for switched ctxs
We currently increment the SSL_CTX stats.sess_accept field in
tls_setup_handshake(), which is invoked from the state machine well
before ClientHello processing would have had a chance to switch
the SSL_CTX attached to the SSL object due to a provided SNI value.
However, stats.sess_accept_good is incremented in tls_finish_handshake(),
and uses the s->ctx.stats field (i.e., the new SSL_CTX that was switched
to as a result of SNI processing).  This leads to the confusing
(nonsensical) situation where stats.sess_accept_good is larger than
stats.sess_accept, as the "sess_accept" value was counted on the
s->session_ctx.

In order to provide some more useful numbers, increment
s->ctx.stats.sess_accept after SNI processing if the SNI processing
changed s->ctx to differ from s->session_ctx.  To preserve the
property that any given accept is counted only once, make the
corresponding decrement to s->session_ctx.stats.sess_accept when
doing so.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
2017-10-30 10:21:10 -05:00
Benjamin Kaduk
0e6161bcae Normalize on session_ctx for stats where possible
For client SSL objects and before any callbacks have had a chance
to be called, we can write the stats accesses using the session_ctx,
which makes sense given that these values are all prefixed with
"sess_".

For servers after a client_hello or servername callback has been
called, retain the existing behavior of modifying the statistics
for the current (non-session) context.  This has some value,
in that it allows the statistics to be viewed on a per-vhost level.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
2017-10-30 10:21:10 -05:00
Benjamin Kaduk
1fcb4e4d52 Use atomics for SSL_CTX statistics
It is expected that SSL_CTX objects are shared across threads,
and as such we are responsible for ensuring coherent data accesses.
Aligned integer accesses ought to be atomic already on all supported
architectures, but we can be formally correct.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
2017-10-30 10:18:09 -05:00
KaoruToda
26a7d938c9 Remove parentheses of return.
Since return is inconsistent, I removed unnecessary parentheses and
unified them.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4541)
2017-10-18 16:05:06 +01:00
Matt Caswell
61278ff3f9 Sanity check the HRR version field
The previous commit removed version negotiation on an HRR. However we should
still sanity check the contents of the version field.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)
2017-10-16 15:52:19 +01:00
Matt Caswell
a2b97bdf3d Don't do version neg on an HRR
Previously if a client received an HRR then we would do version negotiation
immediately - because we know we are going to get TLSv1.3. However this
causes a problem when we emit the 2nd ClientHello because we start changing
a whole load of stuff to ommit things that aren't relevant for < TLSv1.3.
The spec requires that the 2nd ClientHello is the same except for changes
required from the HRR. Therefore the simplest thing to do is to defer the
version negotiation until we receive the ServerHello.

Fixes #4292

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)
2017-10-16 15:52:18 +01:00
Matt Caswell
9fb6cb810b Fix bug where early_data does not work if no SNI callback is present
Fixes #4496

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4519)
2017-10-12 15:19:49 +01:00
Tatsuhiro Tsujikawa
751b26b132 Don't change client random in Client Hello in its second flight
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4490)
2017-10-10 09:51:53 +01:00
KaoruToda
208fb891e3 Since return is inconsistent, I removed unnecessary parentheses and
unified them.
- return (0); -> return 0;
- return (1); -> return 1;
- return (-1); -> return -1;

Reviewed-by: Stephen Henson <steve@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4500)
2017-10-09 13:17:09 +01:00
Dr. Stephen Henson
6447e8184c Merge tls1_check_curve into tls1_check_group_id
The function tls_check_curve is only called on clients and contains
almost identical functionaity to tls1_check_group_id when called from
a client. Merge the two.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4475)
2017-10-06 19:09:51 +01:00
Dr. Stephen Henson
f48d826e33 Change curves to groups where relevant
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4475)
2017-10-06 13:23:45 +01:00
Dr. Stephen Henson
ff6d20a67b Use separate functions for supported and peer groups lists
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4475)
2017-10-06 13:23:45 +01:00
Matt Caswell
786b4df402 Remove an incorrect comment
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4456)
2017-10-04 15:56:56 +01:00
Todd Short
a84e5c9aa8 Session resume broken switching contexts
When an SSL's context is swtiched from a ticket-enabled context to
a ticket-disabled context in the servername callback, no session-id
is generated, so the session can't be resumed.

If a servername callback changes the SSL_OP_NO_TICKET option, check
to see if it's changed to disable, and whether a session ticket is
expected (i.e. the client indicated ticket support and the SSL had
tickets enabled at the time), and whether we already have a previous
session (i.e. s->hit is set).

In this case, clear the ticket-expected flag, remove any ticket data
and generate a session-id in the session.

If the SSL hit (resumed) and switched to a ticket-disabled context,
assume that the resumption was via session-id, and don't bother to
update the session.

Before this fix, the updated unit-tests in 06-sni-ticket.conf would
fail test #4 (server1 = SNI, server2 = no SNI).

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/1529)
2017-10-04 10:21:08 +10:00
Dr. Stephen Henson
34e5292c57 Rename tls1_get_curvelist.
Rename tls1_get_curvelist to tls1_get_grouplist, change to void as
it can never fail and remove unnecessary return value checks. Clean
up the code.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/=4412)
2017-09-26 13:00:26 +01:00
Dr. Stephen Henson
612f9d2227 New function ssl_generate_param_group
Setup EVP_PKEY structure from a group ID in ssl_generate_param_group,
replace duplicate code with this function.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/=4412)
2017-09-26 13:00:26 +01:00
Dr. Stephen Henson
43b95d7365 Replace tls1_ec_curve_id2nid.
Replace tls1_ec_curve_id2nid() with tls_group_id_lookup() which returns
the TLS_GROUP_INFO for the group.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/=4412)
2017-09-26 13:00:26 +01:00
Dr. Stephen Henson
8841154a90 Return group id in tls1_shared_group
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/=4412)
2017-09-26 13:00:26 +01:00
Dr. Stephen Henson
9e84a42db4 Store groups as uint16_t
Instead of storing supported groups in on-the-wire format store
them as parsed uint16_t values. This simplifies handling of groups
as the values can be directly used instead of being converted.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4406)
2017-09-22 22:59:42 +01:00
Benjamin Kaduk
f5d270cab5 Fix no-ec no-dh build
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4369)
2017-09-14 10:23:20 -05:00
David Benjamin
f1b97da1fd Introduce named constants for the ClientHello callback.
It is otherwise unclear what all the magic numbers mean.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4349)
2017-09-08 13:58:59 -05:00
David Benjamin
a9c0d8beea Rename SSL_CTX_set_early_cb to SSL_CTX_set_client_hello_cb.
"Early callback" is a little ambiguous now that early data exists.
Perhaps "ClientHello callback"?

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4349)
2017-09-08 13:58:59 -05:00
Benjamin Kaduk
1c259bb518 Restore historical behavior for absent ServerHello extensions
In OpenSSL 1.1.0, when there were no extensions added to the ServerHello,
we did not write the extension data length bytes to the end of the
ServerHello; this is needed for compatibility with old client implementations
that do not support TLS extensions (such as the default configuration of
OpenSSL 0.9.8).  When ServerHello extension construction was converted
to the new extensions framework in commit
7da160b0f4, this behavior was inadvertently
limited to cases when SSLv3 was negotiated (and similarly for ClientHellos),
presumably since extensions are not defined at all for SSLv3.  However,
extensions for TLS prior to TLS 1.3 have been defined in separate
RFCs (6066, 4366, and 3546) from the TLS protocol specifications, and as such
should be considered an optional protocol feature in those cases.

Accordingly, be conservative in what we send, and skip the extensions block
when there are no extensions to be sent, regardless of the TLS/SSL version.
(TLS 1.3 requires extensions and can safely be treated differently.)

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4296)
2017-09-07 09:53:21 -05:00
Matt Caswell
3d85c7f408 Don't attempt to add a zero length padding extension
The padding extension should always be at least 1 byte long

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4331)
2017-09-04 15:15:30 +01:00
Andy Polyakov
89bc9cf682 ssl/statem/extensions_clnt.c: fix return code buglet.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4297)
2017-09-01 08:44:41 +02:00
Andy Polyakov
eb5fd03bb2 ssl/statem/*.c: address "enum mixed with another type" warnings.
This is actually not all warnings, only return values.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4297)
2017-09-01 08:43:45 +02:00
Rich Salz
ccb76685e0 Avoid possible uninitialized variable.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4307)
2017-08-31 18:27:06 -04:00
Matt Caswell
0ef2802165 Various review fixes for PSK early_data support
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
2017-08-31 15:03:35 +01:00
Matt Caswell
4be3a7c7aa Client side sanity check of ALPN after server has accepted early_data
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
2017-08-31 15:03:35 +01:00
Matt Caswell
fff202e5f7 Add some fixes for Travis failures
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
2017-08-31 15:03:35 +01:00
Matt Caswell
630369d9ce Add server side sanity checks of SNI/ALPN for use with early_data
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
2017-08-31 15:03:35 +01:00
Matt Caswell
ae8d7d994a Make sure we save ALPN data in the session
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
2017-08-31 15:03:35 +01:00
Matt Caswell
ffc5bbaaee Complain if we are writing early data but SNI or ALPN is incorrect
SNI and ALPN must be set to be consistent with the PSK. Otherwise this is
an error.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
2017-08-31 15:03:34 +01:00
Matt Caswell
add8d0e9e0 Enable the ability to use an external PSK for sending early_data
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
2017-08-31 15:02:22 +01:00
Pauli
198c42f5af Move OPENSSL_CONF from e_os.h to cryptlib.h
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4188)
2017-08-30 07:20:44 +10:00
Pauli
07016a8a31 Move e_os.h to be the very first include.
cryptilib.h is the second.

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4188)
2017-08-30 07:20:44 +10:00
Pauli
677963e5a4 e_os.h removal from other headers and source files.
Removed e_os.h from all bar three headers (apps/apps.h crypto/bio/bio_lcl.h and
ssl/ssl_locl.h).

Added e_os.h into the files that need it now.

Directly reference internal/nelem.h when required.

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4188)
2017-08-30 07:20:43 +10:00
Todd Short
10ed1b7239 Reorder extensions to put SigAlgs last
Force non-empty padding extension.
When enabled, force the padding extension to be at least 1 byte long.
WebSphere application server cannot handle having an empty
extension (e.g. EMS/EtM) as the last extension in a client hello.
This moves the SigAlgs extension last for TLSv1.2 to avoid this
issue.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3921)
2017-08-18 09:52:17 -04:00
Benjamin Kaduk
5626f634c3 Move ALPN handling from finalizer to delayed call
Commit 02f0274e8c moved ALPN processing
into an extension finalization function, as the only documented ordering
requirement from previous commits was that ALPN processing occur after
SNI processing, and SNI processing is performed before the extension
finalization step.  However, it is useful for applications'
alpn_select callbacks to run after ciphersuite selection as well -- at
least one application protocol specification (HTTP/2) imposes restrictions
on which ciphersuites are usable with that protocol.  Since it is generally
more preferrable to have a successful TLS connection with a default application
protocol than to fail the TLS connection and not be able to have the preferred
application protocol, it is good to give the alpn_select callback information
about the ciphersuite to be used, so that appropriate restrctions can be
enforced in application code.

Accordingly, split the ALPN handling out into a separate tls_handl_alpn()
function akin to tls_handle_status_request(), called from
tls_post_process_client_hello().  This is an alternative to resuscitating
ssl_check_clienthello_tlsext_late(), something of an awkwward name itself.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4070)
2017-08-15 10:52:21 -05:00
FdaSilvaYY
44e6995155 Fix some Typos and indents
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4108)
2017-08-11 10:16:33 -04:00
Benjamin Kaduk
e3743355e8 Don't modify resumed session objects
If s->hit is set, s->session corresponds to a session created on
a previous connection, and is a data structure that is potentially
shared across other SSL objects.  As such, there are thread-safety
issues with modifying the structure without taking its lock (and
of course all corresponding read accesses would also need to take
the lock as well), which have been observed to cause double-frees.

Regardless of thread-safety, the resumed session object is intended
to reflect parameters of the connection that created the session,
and modifying it to reflect the parameters from the current connection
is confusing.  So, modifications to the session object during
ClientHello processing should only be performed on new connections,
i.e., those where s->hit is not set.

The code mostly got this right, providing such checks when processing
SNI and EC point formats, but the supported groups (formerly
supported curves) extension was missing it, which is fixed by this commit.

However, TLS 1.3 makes the suppported_groups extension mandatory
(when using (EC)DHE, which is the normal case), checking for the group
list in the key_share extension processing.  But, TLS 1.3 only [0] supports
session tickets for session resumption, so the session object in question
is the output of d2i_SSL_SESSION(), and will not be shared across SSL
objects.  Thus, it is safe to modify s->session for TLS 1.3 connections.

[0] A psk_find_session callback can also be used, but the restriction that
each callback execution must produce a distinct SSL_SESSION structure
can be documented when the psk_find_session callback documentation is
completed.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4123)
2017-08-09 14:54:47 -05:00
Rich Salz
ae3947de09 Add a DRBG to each SSL object
Give each SSL object it's own DRBG, chained to the parent global
DRBG which is used only as a source of randomness into the per-SSL
DRBG.  This is used for all session, ticket, and pre-master secret keys.
It is NOT used for ECDH key generation which use only the global
DRBG. (Doing that without changing the API is tricky, if not impossible.)

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4050)
2017-08-03 10:24:03 -04:00
Rich Salz
75e2c87765 Switch from ossl_rand to DRBG rand
If RAND_add wraps around, XOR with existing. Add test to drbgtest that
does the wrap-around.

Re-order seeding and stop after first success.

Add RAND_poll_ex()

Use the DF and therefore lower RANDOMNESS_NEEDED.  Also, for child DRBG's,
mix in the address as the personalization bits.

Centralize the entropy callbacks, from drbg_lib to rand_lib.
(Conceptually, entropy is part of the enclosing application.)
Thanks to Dr. Matthias St Pierre for the suggestion.

Various code cleanups:
    -Make state an enum; inline RANDerr calls.
    -Add RAND_POLL_RETRIES (thanks Pauli for the idea)
    -Remove most RAND_seed calls from rest of library
    -Rename DRBG_CTX to RAND_DRBG, etc.
    -Move some code from drbg_lib to drbg_rand; drbg_lib is now only the
     implementation of NIST DRBG.
    -Remove blocklength

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4019)
2017-08-03 09:23:28 -04:00
Matt Caswell
67dc995eaf Move ossl_assert
Move the definition of ossl_assert() out of e_os.h which is intended for OS
specific things. Instead it is moved into internal/cryptlib.h.

This also changes the definition to remove the (int) cast.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4073)
2017-08-03 10:48:00 +01:00
Matt Caswell
5d61491c88 Fix new_session_cb calls in TLSv1.3
If a new_session_cb is set then it was only ever getting invoked if !s->hit
is true. This is sensible for <=TLSv1.2 but does not work for TLSv1.3.

Fixes #4045

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4068)
2017-08-01 13:09:31 +01:00
Todd Short
b93a295a36 Fix SSL_set_tlsext_debug_callback/-tlsextdebug
Some extensions were being displayed twice, before they were parsed, and
again after they were parsed.
The supported_versions extension was not being fully displayed, as it
was processed differently than other extensions.

Move the debug callback to where the extensions are first collected, to
catch all the extensions as they come in, so they are ordered correctly.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3911)
2017-07-31 13:38:05 -04:00
Paul Yang
bbb4ceb86e Support converting cipher name to RFC name and vice versa
Fixes: issue #3747

make SSL_CIPHER_standard_name globally available and introduce a new
function OPENSSL_cipher_name.

A new option '-convert' is also added to 'openssl ciphers' app.

Documentation and test cases are added.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/3859)
2017-07-21 07:20:14 +10:00
Matt Caswell
00848ea842 Tolerate a zero length ticket nonce
TLSv1.3 draft-21 requires the ticket nonce to be at least 1 byte in length.
However NSS sends a zero length nonce. This is actually ok because the next
draft will allow zero length nonces anyway, so we should tolerate this.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3957)
2017-07-18 16:59:37 +01:00
Matt Caswell
d4504fe579 Fix early_data with an HRR
early_data is not allowed after an HRR. We failed to handle that
correctly.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3933)
2017-07-18 16:51:58 +01:00
Dr. Stephen Henson
dd24857b78 Use cert tables instead of X509_certificate_type
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3858)
2017-07-13 12:38:42 +01:00
Dr. Stephen Henson
52fd27f978 Use certificate tables instead of ssl_cert_type
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3858)
2017-07-13 12:38:42 +01:00
Dr. Stephen Henson
7f6b466b2c Use certificate tables instead of ssl_cipher_get_cert_index.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3858)
2017-07-13 12:38:42 +01:00
Matt Caswell
e3c0d76bc7 Do not allow non-dhe kex_modes by default
Allow that mode to be configured if desired.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3833)
2017-07-07 16:08:05 +01:00
Matt Caswell
5159821540 Updates following review feedback of TLSv1.3 draft-21 code
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3852)
2017-07-07 15:02:09 +01:00
Matt Caswell
b81bd33680 Update the early_secret generation to use the new ticket_nonce field
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3852)
2017-07-07 15:02:09 +01:00
Matt Caswell
a19ae67d8d Update tls13_hkdf_expand() to take the length of the data
In most scenarios the length of the input data is the hashsize, or 0 if
the data is NULL. However with the new ticket_nonce changes the length can
be different.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3852)
2017-07-07 15:02:09 +01:00
Matt Caswell
9b6a825461 Send and receive the ticket_nonce field in a NewSessionTicket
This just adds the processing for sending and receiving the newly added
ticket_nonce field. It doesn't actually use it yet.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3852)
2017-07-07 15:02:09 +01:00
Bernd Edlinger
b43c376586 Fix potential crash in tls_construct_finished.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3667)
2017-07-03 16:34:33 +02:00
Bernd Edlinger
c31ad0bbf8 Fix a crash in tls_construct_client_certificate.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3667)
2017-07-03 16:34:33 +02:00
Matt Caswell
25ffeb11ea Fix another EVP_DigestVerify() instance
Following on from the previous commit this fixes another instance where
we need to treat a -ve return from EVP_DigestVerify() as a bad signature.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3756)
2017-06-23 17:23:52 +01:00
Matt Caswell
cfba06758e Treat all failures from EVP_DigestVerify() as a bad signature
Prior to 72ceb6a we treated all failures from the call to
EVP_DigestVerifyFinal() as if it were a bad signature, and failures in
EVP_DigestUpdate() as an internal error. After that commit we replaced
this with the one-shot function EVP_DigestVerify() and treated a 0 return
as a bad signature and a negative return as an internal error. However,
some signature errors can be negative (e.g. according to the docs if the
form of the signature is wrong). Therefore we should treat all <=0
returns as a bad signature.

This fixes a boringssl test failure.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3756)
2017-06-23 17:23:25 +01:00
Paul Yang
15b1688ac9 Avoid compiler complaining
initialize some local variables

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3741)
2017-06-21 14:36:02 -04:00
Matt Caswell
9924087573 Fix DTLS failure when used in a build which has SCTP enabled
The value of BIO_CTRL_DGRAM_SET_PEEK_MODE was clashing with the value for
BIO_CTRL_DGRAM_SCTP_SET_IN_HANDSHAKE. In an SCTP enabled build
BIO_CTRL_DGRAM_SCTP_SET_IN_HANDSHAKE was used unconditionally with
the reasoning that it would be ignored if SCTP wasn't in use. Unfortunately
due to this clash, this wasn't the case. The BIO ended up going into peek
mode and was continually reading the same data over and over - throwing it
away as a replay.

Fixes #3723

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3724)
2017-06-21 14:56:24 +01:00
Matt Caswell
72257204bd PSK related tweaks based on review feedback
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3670)
2017-06-21 14:45:36 +01:00