Commit Graph

548 Commits

Author SHA1 Message Date
Matt Caswell
fb34a0f4e0 Try to be more consistent about the alerts we send
We are quite inconsistent about which alerts get sent. Specifically, these
alerts should be used (normally) in the following circumstances:

SSL_AD_DECODE_ERROR = The peer sent a syntactically incorrect message
SSL_AD_ILLEGAL_PARAMETER = The peer sent a message which was syntactically
correct, but a parameter given is invalid for the context
SSL_AD_HANDSHAKE_FAILURE = The peer's messages were syntactically and
semantically correct, but the parameters provided were unacceptable to us
(e.g. because we do not support the requested parameters)
SSL_AD_INTERNAL_ERROR = We messed up (e.g. malloc failure)

The standards themselves aren't always consistent but I think the above
represents the best interpretation.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3480)
2017-05-19 08:47:08 +01:00
Matt Caswell
d8028b202b Fix EXT_RETURN usage for add_key_share()
add_key_share() is a helper function used during key_share extension
construction. It is expected to be a simple boolean success/fail return.
It shouldn't be using the new EXT_RETURN type but it was partially converted
anyway. This changes it back.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3488)
2017-05-18 10:05:02 +01:00
Matt Caswell
108d45df73 Allow a missing sig algs extension if resuming
The current TLSv1.3 spec says:

'If a server is authenticating via a certificate and the client has not
sent a "signature_algorithms" extension, then the server MUST abort the
handshake with a "missing_extension" alert (see Section 8.2).'

If we are resuming then we are not "authenticating via a certificate" but
we were still aborting with the missing_extension alert if sig algs was
missing.

This commit ensures that we only send the alert if we are not resuming.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3478)
2017-05-17 15:33:42 +01:00
Matt Caswell
355a0d10a9 Add a new unsolicited extension error code and add enum tag
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3418)
2017-05-17 15:32:25 +01:00
Matt Caswell
b186a59283 Fail if we receive a response to an extension that we didn't request
We already did this on an ad-hoc per extension basis (for some extensions).
This centralises it and makes sure we do it for all extensions.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3418)
2017-05-17 15:23:49 +01:00
Bernd Edlinger
018fcbec38 Fix gcc-7 warnings.
- Mostly missing fall thru comments
- And uninitialized value used in sslapitest.c

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3440)
2017-05-11 19:39:38 +02:00
Matt Caswell
9010b7bc6e Add some extra comments following alert changes
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3436)
2017-05-11 13:13:04 +01:00
Matt Caswell
26b9172a50 Add some checks for trailing data after extension blocks
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3436)
2017-05-11 13:13:04 +01:00
Matt Caswell
b6fdc12d94 Send a missing_extension alert if key_share/supported groups not present
Only applies if we're not doing psk.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3436)
2017-05-11 13:13:04 +01:00
Matt Caswell
2d871227fa Send an illegal parameter alert if the update type in a KeyUpdate is wrong
Previously we sent a decode_error alert.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3436)
2017-05-11 13:13:04 +01:00
Matt Caswell
1a281aab73 Ensure we fail with a decode error alert if the server sends and empty Cert
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3436)
2017-05-11 13:13:04 +01:00
Matt Caswell
f69fe73a2a Fix more alert codes
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3436)
2017-05-11 13:13:04 +01:00
Matt Caswell
ef57a475a0 Verify that there is no trailing data after the extensions block
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3436)
2017-05-11 13:13:04 +01:00
Matt Caswell
721586eadf Fix some alert codes
Make sure we are using the correct alert codes as per the spec.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3436)
2017-05-11 13:13:04 +01:00
Dr. Stephen Henson
7539418981 Add EVP_DigestSign and EVP_DigesVerify
Add "single part" digest sign and verify functions. These sign and verify
a message in one function. This simplifies some operations and it will later
be used as the API for algorithms which do not support the update/final
mechanism (e.g. PureEdDSA).

Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3409)
2017-05-11 12:59:25 +01:00
Matt Caswell
21181889d7 Copy custom extension flags in a call to SSL_set_SSL_CTX()
The function SSL_set_SSL_CTX() can be used to swap the SSL_CTX used for
a connection as part of an SNI callback. One result of this is that the
s->cert structure is replaced. However this structure contains information
about any custom extensions that have been loaded. In particular flags are
set indicating whether a particular extension has been received in the
ClientHello. By replacing the s->cert structure we lose the custom
extension flag values, and it appears as if a client has not sent those
extensions.

SSL_set_SSL_CTX() should copy any flags for custom extensions that appear
in both the old and the new cert structure.

Fixes #2180

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3425)
2017-05-10 16:49:00 +01:00
Richard Levitte
cf53cbea5b Fix 'no-ec'
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3430)
2017-05-10 17:09:35 +02:00
Matt Caswell
66d4bf6b20 Verify that if we have an HRR then something will change
It is invalid if we receive an HRR but no change will result in
ClientHello2.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3414)
2017-05-09 17:23:58 +01:00
Matt Caswell
7b1ec1cfb7 Fix HRR bug
If an HRR gets sent without a key_share (e.g. cookie only) then the code
fails when it should not.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3414)
2017-05-09 17:23:58 +01:00
Matt Caswell
07d447a6fc Don't do the final key_share checks if we are in an HRR
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3414)
2017-05-09 17:23:58 +01:00
Matt Caswell
1fe3549428 Verify that only NULL compression is sent in TLSv1.3 ClientHello
It is illegal in a TLSv1.3 ClientHello to send anything other than the
NULL compression method. We should send an alert if we find anything else
there. Previously we were ignoring this error.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3410)
2017-05-09 17:02:48 +01:00
Matt Caswell
12635aa09d Updates to supported_groups following review feedback
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3395)
2017-05-08 11:09:02 +01:00
Matt Caswell
6af8754637 Send the supported_groups extension in EE where applicable
The TLSv1.3 spec says that a server SHOULD send supported_groups in the
EE message if there is a group that it prefers to the one used in the
key_share. Clients MAY act on that. At the moment we don't do anything
with it on the client side, but that may change in the future.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3395)
2017-05-08 11:09:02 +01:00
Matt Caswell
17aa119e7b Update the HKDF labels for draft-20
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3371)
2017-05-03 17:23:02 +01:00
Matt Caswell
7f533d6fa1 Only send custom extensions where we have received one in the ClientHello
We already did this for ServerHello and EncryptedExtensions. We should be
doing it for Certificate and HelloRetryRequest as well.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3298)
2017-05-03 14:37:42 +01:00
Matt Caswell
33564cb749 Fix comment around safari fingerprint check
Fixes #2442

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3367)
2017-05-02 17:08:41 +01:00
Graham Edgecombe
e596c68c31 Return success in custom_ext_parse_old_cb_wrap if parse_cb is NULL
This fixes a segfault if a NULL parse_cb is passed to
SSL_CTX_add_{client,server}_custom_ext, which was supported in the
pre-1.1.1 implementation.

This behaviour is consistent with the other custom_ext_*_old_cb_wrap
functions, and with the new SSL_CTX_add_custom_ext function.

CLA: trivial

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3310)
2017-04-26 17:37:30 +01:00
Tatsuhiro Tsujikawa
8e1634ec36 Don't treat PACKET_remaining() as boolean
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3244)
2017-04-26 16:56:37 +01:00
Tatsuhiro Tsujikawa
bf5c84f5d1 Break before && operator
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3244)
2017-04-26 16:56:36 +01:00
Tatsuhiro Tsujikawa
735d5b59df Call init and finalization functions per extension message
Previously, init and finalization function for extensions are called
per extension block, rather than per message.  This commit changes
that behaviour, and now they are called per message.  The parse
function is still called per extension block.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3244)
2017-04-26 16:56:35 +01:00
Matt Caswell
150840b944 Always duplicate the session on NewSessionTicket in TLSv1.3
Because NST messages arrive post-handshake, the session may have already
gone into the cache. Once in the cache a session must be immutable -
otherwise you could get multi-thread issues.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3008)
2017-04-26 16:42:29 +01:00
Matt Caswell
e586eac885 Add support for SSL_SESSION_is_resumable()
Provide a way to test whether the SSL_SESSION object can be used to resume a
sesion or not.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3008)
2017-04-26 16:42:29 +01:00
Matt Caswell
3348fc7e89 Remove TLS1.3 TODO around testing for session id length
TLSv1.3 will do the same thing as TLSv1.2 with tickets with regards to session
ids, i.e. it will create a synthetic session id when the session is established,
so it is reasonable to check the session id length, even in TLSv1.3.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3008)
2017-04-26 16:42:29 +01:00
Matt Caswell
38a7315060 Add a ciphersuite config sanity check for servers
Ensure that there are ciphersuites enabled for the maximum supported
version we will accept in a ClientHello.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3316)
2017-04-26 14:31:00 +01:00
Matt Caswell
aafec89c63 Add a ciphersuite config sanity check for clients
Ensure that there are ciphersuites enabled for the maximum supported
version we are claiming in the ClientHello.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3316)
2017-04-26 14:31:00 +01:00
Matt Caswell
bf846a6d47 Don't overwrite the alert value if there is no alert to send
The function tls_early_post_process_client_hello() was overwriting the
passed "al" parameter even if it was successful. The caller of that
function, tls_post_process_client_hello(), sets "al" to a sensible default
(HANDSHAKE_FAILURE), but this was being overwritten to be INTERNAL_ERROR.
The result is a "no shared cipher" error (and probably other similar errors)
were being reported back to the client with an incorrect INTERNAL_ERROR
alert.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3314)
2017-04-26 14:13:26 +01:00
Matt Caswell
aefb925647 Don't attempt to send fragments > max_send_fragment in DTLS
We were allocating the write buffer based on the size of max_send_fragment,
but ignoring it when writing data. We should fragment handshake messages
if they exceed max_send_fragment and reject application data writes that
are too large.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3286)
2017-04-25 11:13:39 +01:00
Matt Caswell
bd79bcb42b Remove special case code for SCTP reneg handling
There was code existing which attempted to handle the case where application
data is received after a reneg handshake has started in SCTP. In normal DTLS
we just fail the connection if this occurs, so there doesn't seem any reason
to try and work around it for SCTP. In practice it didn't work properly
anyway and is probably a bad idea to start with.

Fixes #3251

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3286)
2017-04-25 11:13:39 +01:00
Matt Caswell
8af91fd9d0 Don't fail the connection in SSLv3 if server selects ECDHE
ECDHE is not properly defined for SSLv3. Commit fe55c4a2 prevented ECDHE
from being selected in that protocol. However, historically, servers do
still select ECDHE anyway so that commit causes interoperability problems.
Clients that previously worked when talking to an SSLv3 server could now
fail.

This commit introduces an exception which enables a client to continue in
SSLv3 if the server selected ECDHE.

[extended tests]

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3204)
2017-04-24 16:15:40 +01:00
Rich Salz
26fb4b033c if log fails, avoid pms getting double free
doing the pms assignment after log is successful

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3278)
2017-04-24 09:41:51 -04:00
Todd Short
d1186c30a2 Fix minor compiler issues.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3220)
2017-04-19 12:51:08 -04:00
Benjamin Kaduk
8313a787d7 Allow an ALPN callback to pretend to not exist
RFC 7301 mandates that the server SHALL respond with a fatal
"no_application_protocol" alert when there is no overlap between
the client's supplied list and the server's list of supported protocols.
In commit 062178678f we changed from
ignoring non-success returns from the supplied alpn_select_cb() to
treating such non-success returns as indicative of non-overlap and
sending the fatal alert.

In effect, this is using the presence of an alpn_select_cb() as a proxy
to attempt to determine whether the application has configured a list
of supported protocols.  However, there may be cases in which an
application's architecture leads it to supply an alpn_select_cb() but
have that callback be configured to take no action on connections that
do not have ALPN configured; returning SSL_TLSEXT_ERR_NOACK from
the callback would be the natural way to do so.  Unfortunately, the
aforementioned behavior change also treated SSL_TLSEXT_ERR_NOACK as
indicative of no overlap and terminated the connection; this change
supplies special handling for SSL_TLSEXT_ERR_NOACK returns from the
callback.  In effect, it provides a way for a callback to obtain the
behavior that would have occurred if no callback was registered at
all, which was not possible prior to this change.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2570)
2017-04-10 11:57:37 -04:00
Matt Caswell
0f5af6b1b8 Remove an out of date TODO
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
2017-04-07 13:41:04 +01:00
Matt Caswell
787d9ec74c Create an ENDPOINT enum type for use internally
We need it for the custom extensions API

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
2017-04-07 13:41:04 +01:00
Matt Caswell
cd17bb190c Prefix custom extension API callback types with SSL_
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
2017-04-07 13:41:04 +01:00
Matt Caswell
64350ab587 Various style tweaks based on feedback
Style updates for the new custom extensions API

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
2017-04-07 13:41:04 +01:00
Matt Caswell
43ae5eed6f Implement a new custom extensions API
The old custom extensions API was not TLSv1.3 aware. Extensions are used
extensively in TLSv1.3 and they can appear in many different types of
messages. Therefore we need a new API to be able to cope with that.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
2017-04-07 13:41:04 +01:00
Matt Caswell
fe874d27d3 Move the extensions context codes into the public API
This move prepares for the later addition of the new custom extensions
API. The context codes have an additional "SSL_" added to their name to
ensure we don't have name clashes with other applications.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
2017-04-07 13:41:04 +01:00
Matt Caswell
b443c84594 Move ssl/t1_ext.c to ssl/statem/extensions_cust.c
Brings all the extensions code together.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
2017-04-07 13:41:04 +01:00
Dr. Stephen Henson
9784ec0474 Don't use client specific functions to retrieve CA list
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3015)
2017-04-03 23:47:21 +01:00