Commit graph

3323 commits

Author SHA1 Message Date
Todd Short
dffdcc773a Fix inconsistent check of UNSAFE_LEGACY_RENEGOTIATION
The check for SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION is
inconsistent. Most places check SSL->options, one place is checking
SSL_CTX->options; fix that.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
GH: #3523
2017-05-26 11:29:18 +02:00
Dr. Stephen Henson
caf2b6b54f Don't use one shot API for SSLv3.
SSLv3 (specifically with client auth) cannot use one shot APIs: the digested
data and the master secret are handled in separate update operations. So
in the special case of SSLv3 use the streaming API.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3527)
2017-05-23 15:51:24 +01:00
Matt Caswell
42bd7a16d0 Add an error to the stack on failure in dtls1_write_bytes()
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3496)
2017-05-22 14:00:52 +01:00
Matt Caswell
b77f3ed171 Convert existing usage of assert() to ossl_assert() in libssl
Provides consistent output and approach.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3496)
2017-05-22 14:00:43 +01:00
Matt Caswell
a89325e41f Fix some style issues in returns
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3496)
2017-05-22 14:00:34 +01:00
Matt Caswell
380a522f68 Replace instances of OPENSSL_assert() with soft asserts in libssl
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3496)
2017-05-22 14:00:19 +01:00
Matt Caswell
aabe3a3591 Fix return code in tls1_mac
The return code from tls1_mac is supposed to be a boolean 0 for fail, 1 for
success. In one place we returned -1 on error. This would cause code calling
the mac function to erroneously see this as a success (because a non-zero
value is being treated as success in all call sites).

Fortunately, AFAICT, the place that returns -1 can only happen on an
internal error so is not under attacker control. Additionally this code only
appears in master. In 1.1.0 the return codes are treated differently.
Therefore there are no security implications.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3495)
2017-05-19 17:02:11 +01:00
Matt Caswell
6944311688 Make SSL_is_server() accept a const SSL
Fixes #1526

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3360)
2017-05-19 08:54:39 +01:00
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
Matt Caswell
bd990e2535 Don't allow fragmented alerts
An alert message is 2 bytes long. In theory it is permissible in SSLv3 -
TLSv1.2 to fragment such alerts across multiple records (some of which
could be empty). In practice it make no sense to send an empty alert
record, or to fragment one. TLSv1.3 prohibts this altogether and other
libraries (BoringSSL, NSS) do not support this at all. Supporting it adds
significant complexity to the record layer, and its removal is unlikely
to cause inter-operability issues.

The DTLS code for this never worked anyway and it is not supported at a
protocol level for DTLS. Similarly fragmented DTLS handshake records only
work at a protocol level where at least the handshake message header
exists within the record. DTLS code existed for trying to handle fragmented
handshake records smaller than this size. This code didn't work either so
has also been removed.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3476)
2017-05-17 10:40:04 +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
fc4c15faa4 TLSv1.3 alert and handshake messages can never be 0 length
We abort if we read a message like this.

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
0b367d7955 TLSv1.3 alerts cannot be fragmented and only one per record
We should be validating that.

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
3c544acc38 Check that a TLSv1.3 encrypted message has an app data content type
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
Matt Caswell
f66f8a4491 Reject unknown warning alerts in TLSv1.3
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
ad5100bc41 Add support to SSL_trace() for inner content types
When using the -trace option with TLSv1.3 all records appear as "application
data". This adds the ability to see the inner content type too.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3408)
2017-05-08 11:42:37 +01:00
Matt Caswell
eee2750bd3 Remove support for OPENSSL_SSL_TRACE_CRYPTO
This trace option does not appear in Configure as a separate option and is
undocumented. It can be switched on using "-DOPENSSL_SSL_TRACE_CRYPTO",
however this does not compile in master or in any 1.1.0 released version.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3408)
2017-05-08 11:42:37 +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
7a4e6a1e50 Updates to serverinfo fix based on review feedback
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3382)
2017-05-04 16:02:08 +01:00
Matt Caswell
bade29da33 Fix SSL_CTX_use_serverinfo_ex() et al to properly handle V1 data
SSL_CTX_use_serverinfo_ex() et al were always processing data as if it was
V2 format, even if it was V1. This bug was masked because, although we had
a test which loaded V1 serverinfo data from a file, the function
SSL_CTX_use_serverinfo_file() transparently converts V1 data to V2 before
calling SSL_CTX_use_serverinfo_ex().

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3382)
2017-05-04 16:02:08 +01:00
Matt Caswell
bb78552ee1 Revert "Fix clang compile time error"
This reverts commit 1608d658af.

This is the wrong fix for this issue. The next commit provides a better
fix.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3382)
2017-05-04 16:02:08 +01:00
Todd Short
1608d658af Fix clang compile time error
|version| "could" be used uninitialized here, not really, but the
compiler doesn't understand the flow

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3373)
2017-05-04 05:37:30 +02: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
Todd Short
96c9aee2a8 Limit padded record to max plaintext
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3374)
2017-05-03 17:12:00 +01:00
Matt Caswell
f233a9d181 Clarify serverinfo usage with Certificate messages
Ensure that serverinfo only gets added for the first Certificate in a list.

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
2698bbfec6 Add an SSL_ prefix to SERVERINFOV2 and SERVERINFOV1
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
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
84c34ba876 Extend the SERVERINFO file format to include an extensions context
This enables us to know what messages the extensions are relevant for in
TLSv1.3. The new file format is not compatible with the previous one so
we call it SERVERINFOV2.

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
Todd Short
c649d10d3f TLS1.3 Padding
Add padding callback for application control
Standard block_size callback
Documentation and tests included
Configuration file/s_client/s_srver option

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3130)
2017-05-02 09:44:43 +01:00