Commit graph

37 commits

Author SHA1 Message Date
Matt Caswell
d1bfd8076e Buffer a ClientHello with a cookie received via DTLSv1_listen
Previously when a ClientHello arrives with a valid cookie using
DTLSv1_listen() we only "peeked" at the message and left it on the
underlying fd. This works fine for single threaded applications but for
multi-threaded apps this does not work since the fd is typically reused for
the server thread, while a new fd is created and connected for the client.
By "peeking" we leave the message on the server fd, and consequently we
think we've received another valid ClientHello and so we create yet another
fd for the client, and so on until we run out of fds.

In this new approach we remove the ClientHello and buffer it in the SSL
object.

Fixes #6934

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

(cherry picked from commit 079ef6bd53)
2018-10-19 14:29:52 +01:00
Matt Caswell
585e691948 Use the read and write buffers in DTLSv1_listen()
Rather than using init_buf we use the record layer read and write buffers
in DTLSv1_listen(). These seem more appropriate anyway and will help with
the next commit.

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

(cherry picked from commit 2fc4c77c3f)
2018-10-19 14:29:52 +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
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
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
bebc0c7d85 Use the TLSv1.3 nonce construction
This updates the record layer to use the TLSv1.3 style nonce construciton.
It also updates TLSProxy and ossltest to be able to recognise the new
layout.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-11-29 23:31:10 +00:00
Matt Caswell
54105ddd23 Rename all "read" variables with "readbytes"
Travis is reporting one file at a time shadowed variable warnings where
"read" has been used. This attempts to go through all of libssl and replace
"read" with "readbytes" to fix all the problems in one go.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-11-04 12:09:46 +00:00
Matt Caswell
8b0e934afb Fix some missed size_t updates
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-11-04 12:09:45 +00:00
Matt Caswell
72716e79bf Convert some misc record layer functions for size_t
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-11-04 12:09:45 +00:00
Matt Caswell
7ee8627f6e Convert libssl writing for size_t
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-11-04 12:09:45 +00:00
Matt Caswell
eda757514e Further libssl size_t-ify of reading
Writing still to be done

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-11-04 12:09:45 +00:00
Matt Caswell
8e6d03cac4 Convert record layer to use size_t
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-11-04 12:09:45 +00:00
Matt Caswell
af58be768e Don't allow too many consecutive warning alerts
Certain warning alerts are ignored if they are received. This can mean that
no progress will be made if one peer continually sends those warning alerts.
Implement a count so that we abort the connection if we receive too many.

Issue reported by Shi Lei.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-21 20:17:04 +01:00
Emilia Kasper
a230b26e09 Indent ssl/
Run util/openssl-format-source on ssl/

Some comments and hand-formatted tables were fixed up
manually by disabling auto-formatting.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-08-18 14:02:29 +02:00
Matt Caswell
44efb88a21 Address feedback on SSLv2 ClientHello processing
Feedback on the previous SSLv2 ClientHello processing fix was that it
breaks layering by reading init_num in the record layer. It also does not
detect if there was a previous non-fatal warning.

This is an alternative approach that directly tracks in the record layer
whether this is the first record.

GitHub Issue #1298

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-08-15 23:14:30 +01:00
FdaSilvaYY
0485d5406a Whitespace cleanup in ssl folder
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1264)
2016-06-29 09:56:39 -04:00
Matt Caswell
255cfeacd8 Reject out of context empty records
Previously if we received an empty record we just threw it away and
ignored it. Really though if we get an empty record of a different content
type to what we are expecting then that should be an error, i.e. we should
reject out of context empty records. This commit makes the necessary changes
to achieve that.

RT#4395

Reviewed-by: Andy Polyakov <appro@openssl.org>
2016-06-07 22:07:36 +01:00
Rich Salz
846e33c729 Copyright consolidation 01/10
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2016-05-17 14:19:19 -04:00
Matt Caswell
3720597107 Rename the numpipes argument to ssl3_enc/tls1_enc
The numpipes argument to ssl3_enc/tls1_enc is actually the number of
records passed in the array. To make this clearer rename the argument to
|n_recs|.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-03-07 21:42:09 +00:00
Matt Caswell
f482740f23 Remove the wrec record layer field
We used to use the wrec field in the record layer for keeping track of the
current record that we are writing out. As part of the pipelining changes
this has been moved to stack allocated variables to do the same thing,
therefore the field is no longer needed.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-03-07 21:39:28 +00:00
Matt Caswell
49580f25b3 Add an SSL_has_pending() function
This is similar to SSL_pending() but just returns a 1 if there is data
pending in the internal OpenSSL buffers or 0 otherwise (as opposed to
SSL_pending() which returns the number of bytes available). Unlike
SSL_pending() this will work even if "read_ahead" is set (which is the
case if you are using read pipelining, or if you are doing DTLS). A 1
return value means that we have unprocessed data. It does *not* necessarily
indicate that there will be application data returned from a call to
SSL_read(). The unprocessed data may not be application data or there
could be errors when we attempt to parse the records.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-03-07 21:39:27 +00:00
Matt Caswell
dad78fb13d Add an ability to set the SSL read buffer size
This capability is required for read pipelining. We will only read in as
many records as will fit in the read buffer (and the network can provide
in one go). The bigger the buffer the more records we can process in
parallel.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-03-07 21:39:27 +00:00
Matt Caswell
0220fee47f Lazily initialise the compression buffer
With read pipelining we use multiple SSL3_RECORD structures for reading.
There are SSL_MAX_PIPELINES (32) of them defined (typically not all of these
would be used). Each one has a 16k compression buffer allocated! This
results in a significant amount of memory being consumed which, most of the
time, is not needed.  This change swaps the allocation of the compression
buffer to be lazy so that it is only done immediately before it is actually
used.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-03-07 21:39:27 +00:00
Matt Caswell
94777c9c86 Implement read pipeline support in libssl
Read pipelining is controlled in a slightly different way than with write
pipelining. While reading we are constrained by the number of records that
the peer (and the network) can provide to us in one go. The more records
we can get in one go the more opportunity we have to parallelise the
processing.

There are two parameters that affect this:
* The number of pipelines that we are willing to process in one go. This is
controlled by max_pipelines (as for write pipelining)
* The size of our read buffer. A subsequent commit will provide an API for
adjusting the size of the buffer.

Another requirement for this to work is that "read_ahead" must be set. The
read_ahead parameter will attempt to read as much data into our read buffer
as the network can provide. Without this set, data is read into the read
buffer on demand. Setting the max_pipelines parameter to a value greater
than 1 will automatically also turn read_ahead on.

Finally, the read pipelining as currently implemented will only parallelise
the processing of application data records. This would only make a
difference for renegotiation so is unlikely to have a significant impact.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-03-07 21:39:27 +00:00
Matt Caswell
d102d9df86 Implement write pipeline support in libssl
Use the new pipeline cipher capability to encrypt multiple records being
written out all in one go. Two new SSL/SSL_CTX parameters can be used to
control how this works: max_pipelines and split_send_fragment.

max_pipelines defines the maximum number of pipelines that can ever be used
in one go for a single connection. It must always be less than or equal to
SSL_MAX_PIPELINES (currently defined to be 32). By default only one
pipeline will be used (i.e. normal non-parallel operation).

split_send_fragment defines how data is split up into pipelines. The number
of pipelines used will be determined by the amount of data provided to the
SSL_write call divided by split_send_fragment. For example if
split_send_fragment is set to 2000 and max_pipelines is 4 then:
SSL_write called with 0-2000 bytes == 1 pipeline used
SSL_write called with 2001-4000 bytes == 2 pipelines used
SSL_write called with 4001-6000 bytes == 3 pipelines used
SSL_write_called with 6001+ bytes == 4 pipelines used

split_send_fragment must always be less than or equal to max_send_fragment.
By default it is set to be equal to max_send_fragment. This will mean that
the same number of records will always be created as would have been
created in the non-parallel case, although the data will be apportioned
differently. In the parallel case data will be spread equally between the
pipelines.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-03-07 21:39:27 +00:00
Rich Salz
349807608f Remove /* foo.c */ comments
This was done by the following
        find . -name '*.[ch]' | /tmp/pl
where /tmp/pl is the following three-line script:
        print unless $. == 1 && m@/\* .*\.[ch] \*/@;
        close ARGV if eof; # Close file to reset $.

And then some hand-editing of other files.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
2016-01-26 16:40:43 -05:00
Rich Salz
cf2cede4a7 Move pqueue into ssl
This is an internal facility, never documented, not for
public consumption.  Move it into ssl (where it's only used
for DTLS).

I also made the typedef's for pqueue and pitem follow our style: they
name structures, not pointers.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-01-24 18:25:04 -05:00
Viktor Dukhovni
919ba00942 DANE support structures, constructructors and accessors
Also tweak some of the code in demos/bio, to enable interactive
testing of BIO_s_accept's use of SSL_dup.  Changed the sconnect
client to authenticate the server, which now exercises the new
SSL_set1_host() function.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-01-05 19:31:49 -05:00
Matt Caswell
e3d0dae7cf DTLSv1_listen rewrite
The existing implementation of DTLSv1_listen() is fundamentally flawed. This
function is used in DTLS solutions to listen for new incoming connections
from DTLS clients. A client will send an initial ClientHello. The server
will respond with a HelloVerifyRequest containing a unique cookie. The
client the responds with a second ClientHello - which this time contains the
cookie.

Once the cookie has been verified then DTLSv1_listen() returns to user code,
which is typically expected to continue the handshake with a call to (for
example) SSL_accept().

Whilst listening for incoming ClientHellos, the underlying BIO is usually in
an unconnected state. Therefore ClientHellos can come in from *any* peer.
The arrival of the first ClientHello without the cookie, and the second one
with it, could be interspersed with other intervening messages from
different clients.

The whole purpose of this mechanism is as a defence against DoS attacks. The
idea is to avoid allocating state on the server until the client has
verified that it is capable of receiving messages at the address it claims
to come from. However the existing DTLSv1_listen() implementation completely
fails to do this. It attempts to super-impose itself on the standard state
machine and reuses all of this code. However the standard state machine
expects to operate in a stateful manner with a single client, and this can
cause various problems.

A second more minor issue is that the return codes from this function are
quite confused, with no distinction made between fatal and non-fatal errors.
Most user code treats all errors as non-fatal, and simply retries the call
to DTLSv1_listen().

This commit completely rewrites the implementation of DTLSv1_listen() and
provides a stand alone implementation that does not rely on the existing
state machine. It also provides more consistent return codes.

Reviewed-by: Andy Polyakov <appro@openssl.org>
2015-09-23 13:53:26 +01:00
Matt Caswell
657da85eea Move TLS CCS processing into the state machine
The handling of incoming CCS records is a little strange. Since CCS is not
a handshake message it is handled differently to normal handshake messages.
Unfortunately whilst technically it is not a handhshake message the reality
is that it must be processed in accordance with the state of the handshake.
Currently CCS records are processed entirely within the record layer. In
order to ensure that it is handled in accordance with the handshake state
a flag is used to indicate that it is an acceptable time to receive a CCS.

Previously this flag did not exist (see CVE-2014-0224), but the flag should
only really be considered a workaround for the problem that CCS is not
visible to the state machine.

Outgoing CCS messages are already handled within the state machine.

This patch makes CCS visible to the TLS state machine. A separate commit
will handle DTLS.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-08-03 11:18:05 +01:00
Matt Caswell
b821df5f5b Correct type of RECORD_LAYER_get_rrec_length()
The underlying field returned by RECORD_LAYER_get_rrec_length() is an
unsigned int. The return type of the function should match that.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-06-10 12:06:29 +01:00
Matt Caswell
a3680c8f9c Version negotiation rewrite cleanup
Following the version negotiation rewrite all of the previous code that was
dedicated to version negotiation can now be deleted - all six source files
of it!!

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-05-16 09:20:38 +01:00
Matt Caswell
32ec41539b Server side version negotiation rewrite
This commit changes the way that we do server side protocol version
negotiation. Previously we had a whole set of code that had an "up front"
state machine dedicated to the negotiating the protocol version. This adds
significant complexity to the state machine. Historically the justification
for doing this was the support of SSLv2 which works quite differently to
SSLv3+. However, we have now removed support for SSLv2 so there is little
reason to maintain this complexity.

The one slight difficulty is that, although we no longer support SSLv2, we
do still support an SSLv3+ ClientHello in an SSLv2 backward compatible
ClientHello format. This is generally only used by legacy clients. This
commit adds support within the SSLv3 code for these legacy format
ClientHellos.

Server side version negotiation now works in much the same was as DTLS,
i.e. we introduce the concept of TLS_ANY_VERSION. If s->version is set to
that then when a ClientHello is received it will work out the most
appropriate version to respond with. Also, SSLv23_method and
SSLv23_server_method have been replaced with TLS_method and
TLS_server_method respectively. The old SSLv23* names still exist as
macros pointing at the new name, although they are deprecated.

Subsequent commits will look at client side version negotiation, as well of
removal of the old s23* code.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-05-16 09:19:56 +01:00
Matt Caswell
fb45690275 Remove redundant includes from dtls1.h
There were a set of includes in dtls1.h which are now redundant due to the
libssl opaque work. This commit removes those includes, which also has the
effect of resolving one issue preventing building on windows (i.e. the
include of winsock.h)

Reviewed-by: Andy Polyakov <appro@openssl.org>
2015-04-30 11:34:51 +01:00
Matt Caswell
d2200cafd4 Fix record.h formatting
Fix some strange formatting in record.h. This was probably originally
introduced as part of the reformat work.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-03-26 17:26:26 +00:00
Matt Caswell
e5bf62f716 Define SEQ_NUM_SIZE
Replace the hard coded value 8 (the size of the sequence number) with a
constant defined in a macro.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-03-26 17:25:48 +00:00
Matt Caswell
c99c4c11a2 Renamed record layer header files
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-03-26 15:02:01 +00:00
Renamed from ssl/record/rec_layer.h (Browse further)