Commit graph

243 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
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
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
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
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
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
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
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
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
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
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
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
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
Rich Salz
cbbe9186f3 Additional check to handle BAD SSL_write retry
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3124)
2017-04-11 11:59:59 -04:00
Matt Caswell
59cebcf9f6 Don't handle handshake messages when writing early data on server
If we have received the EoED message but not yet had the CF then we are
"in init". Despite that we still want to write application data, so suppress
the "in init" check in ssl3_write_bytes() in that scenario.

Fixes #3041

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3091)
2017-04-03 19:07:29 +01:00
Matt Caswell
39ef78210c Fix bug with SSL_read_early_data()
If read_ahead is set, or SSL_MODE_AUTO_RETRY is used then if
SSL_read_early_data() hits an EndOfEarlyData message then it will
immediately retry automatically, but this time read normal data instead
of early data!

Fixes #3041

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3077)
2017-03-30 09:09:21 +01:00
FdaSilvaYY
69687aa829 More typo fixes
Fix some comments too
[skip ci]

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3069)
2017-03-29 07:14:29 +02:00
Matt Caswell
ef6c191bce Update end of early data processing for draft-19
The end of early data is now indicated by a new handshake message rather
than an alert.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2895)
2017-03-16 14:20:38 +00:00
Benjamin Kaduk
aebe9e3991 Fix some -Wshadow warnings
Found using various (old-ish) versions of gcc.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2940)
2017-03-14 11:44:31 -05:00
Matt Caswell
4f7b76bf0f Fix no-comp
The value of SSL3_RT_MAX_ENCRYPTED_LENGTH normally includes the compression
overhead (even if no compression is negotiated for a connection). Except in
a build where no-comp is used the value of SSL3_RT_MAX_ENCRYPTED_LENGTH does
not include the compression overhead.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2872)
2017-03-08 11:03:37 +00:00
Matt Caswell
b8c49611bc Provide a function to test whether we have unread records pending
Also updates SSL_has_pending() to use it. This actually fixes a bug in
SSL_has_pending() which is supposed to return 1 if we have any processed
or unprocessed data sitting in OpenSSL buffers. However it failed to return
1 if we had processed non-application data pending.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2875)
2017-03-07 16:41:25 +00:00
Matt Caswell
4321969513 Tweak the TLSv1.3 record overflow limits
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2861)
2017-03-06 20:07:40 +00:00
Matt Caswell
7daf7156d8 Don't attempt to write more early_data than we know the server will accept
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)
2017-03-02 17:44:15 +00:00
Matt Caswell
a832b5ef7a Skip early_data if appropriate after a HelloRetryRequest
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)
2017-03-02 17:44:15 +00:00
Matt Caswell
70ef40a05e Check max_early_data against the amount of early data we actually receive
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)
2017-03-02 17:44:15 +00:00
Matt Caswell
67f78eadd0 Make sure we reset the read sequence when skipping records
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)
2017-03-02 17:44:15 +00:00
Matt Caswell
d781d247d1 Provide an SSL_read_early() function for reading early data
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)
2017-03-02 17:44:15 +00:00
Matt Caswell
0a87d0ac62 Parse the early_data extension
We also skip any early_data that subsequently gets sent. Later commits will
process it if we can.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)
2017-03-02 17:44:15 +00:00
Matt Caswell
49e7fe12ea Provide functions to write early data
We provide SSL_write_early() which *must* be called first on a connection
(prior to any other IO function including SSL_connect()/SSL_do_handshake()).
Also SSL_write_early_finish() which signals the end of early data.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)
2017-03-02 17:44:14 +00:00
Matt Caswell
f33f9ddefb Fix a compression bug
do_ssl3_write() was crashing when compression was enabled. We calculate
the maximum length that a record will be after compression and reserve
those bytes in the WPACKET. Unfortunately we were adding the maximum
compression overhead onto the wrong variable resulting in a corrupted
record.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2814)
2017-03-02 16:49:28 +00:00
Jon Spillett
a3004c8203 Check for zero records and return immediately
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2822)
2017-03-02 09:28:58 -05:00
Emilia Kasper
2f0ca54c32 Remove some obsolete/obscure internal define switches:
- FLAT_INC
- PKCS1_CHECK (the SSL_OP_PKCS1_CHECK options have been
  no-oped)
- PKCS_TESTVECT (debugging leftovers)
- SSL_AD_MISSING_SRP_USERNAME (unfinished feature)
- DTLS_AD_MISSING_HANDSHAKE_MESSAGE (unfinished feature)
- USE_OBJ_MAC (note this removes a define from the public header but
   very unlikely someone would be depending on it)
- SSL_FORBID_ENULL

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Stephen Henson <steve@openssl.org>
Reviewed-by: Andy Polyakov <appro@openssl.org>
2017-03-01 10:44:49 +01:00
Matt Caswell
38f2837b1b Remove some commented out code in libssl
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2774)
2017-02-28 16:02:11 +00:00
Emilia Kasper
b53338cbf8 Clean up references to FIPS
This removes the fips configure option. This option is broken as the
required FIPS code is not available.

FIPS_mode() and FIPS_mode_set() are retained for compatibility, but
FIPS_mode() always returns 0, and FIPS_mode_set() can only be used to
turn FIPS mode off.

Reviewed-by: Stephen Henson <steve@openssl.org>
2017-02-28 15:26:25 +01:00
Matt Caswell
df15c84901 Remove some dead code from libssl
There are a small number of functions in libssl that are internal only
and never used by anything.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2770)
2017-02-28 12:54:52 +00:00
Matt Caswell
2c55b28a34 Remove an OPENSSL_assert() and replace with a soft assert and check
Following on from CVE-2017-3733, this removes the OPENSSL_assert() check
that failed and replaces it with a soft assert, and an explicit check of
value with an error return if it fails.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2017-02-16 09:35:56 +00:00
Matt Caswell
28a31a0a10 Don't change the state of the ETM flags until CCS processing
In 1.1.0 changing the ciphersuite during a renegotiation can result in
a crash leading to a DoS attack. In master this does not occur with TLS
(instead you get an internal error, which is still wrong but not a security
issue) - but the problem still exists in the DTLS code.

The problem is caused by changing the flag indicating whether to use ETM
or not immediately on negotiation of ETM, rather than at CCS. Therefore,
during a renegotiation, if the ETM state is changing (usually due to a
change of ciphersuite), then an error/crash will occur.

Due to the fact that there are separate CCS messages for read and write
we actually now need two flags to determine whether to use ETM or not.

CVE-2017-3733

Reviewed-by: Richard Levitte <levitte@openssl.org>
2017-02-16 09:35:56 +00:00
Matt Caswell
18b3a80a5f Fix crash in tls13_enc
If s->s3->tmp.new_cipher is NULL then a crash can occur. This can happen
if an alert gets sent after version negotiation (i.e. we have selected
TLSv1.3 and ended up in tls13_enc), but before a ciphersuite has been
selected.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2575)
2017-02-08 11:41:45 +00:00
Dr. Stephen Henson
20fc2051d2 Use contants for Chacha/Poly, redo algorithm expressions.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2550)
2017-02-08 02:16:27 +00:00
Dr. Stephen Henson
ec07b1d872 Add CCM mode support for TLS 1.3
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2550)
2017-02-08 02:16:27 +00:00
Matt Caswell
cc2455bfa8 Move TLSv1.3 Session Ticket processing into the state machine
We still ignore it for now, but at least its in the right place.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)
2017-01-30 10:17:01 +00:00
Matt Caswell
c7f47786a5 Move state machine knowledge out of the record layer
The record layer was making decisions that should really be left to the
state machine around unexpected handshake messages that are received after
the initial handshake (i.e. renegotiation related messages). This commit
removes that code from the record layer and updates the state machine
accordingly. This simplifies the state machine and paves the way for
handling other messages post-handshake such as the NewSessionTicket in
TLSv1.3.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)
2017-01-30 10:17:00 +00:00
Matt Caswell
0386aad1ab Remove use of the SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS flag
This flag is never set by anything so remove it.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)
2017-01-30 09:36:55 +00:00
Matt Caswell
9799748974 ChangeCipherSpec is not allowed in TLSv1.3
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)
2017-01-30 09:36:55 +00:00
Todd Short
9d6fcd4295 Cleanup EVP_CIPH/EP_CTRL duplicate defines
Remove duplicate defines from EVP source files.
Most of them were in evp.h, which is always included.
Add new ones evp_int.h
EVP_CIPH_FLAG_TLS1_1_MULTIBLOCK is now always defined in evp.h, so
remove conditionals on it

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2201)
2017-01-24 18:47:10 +01:00