Commit graph

78 commits

Author SHA1 Message Date
Matt Caswell
c1ef7c971d Excessive allocation of memory in tls_get_message_header()
A TLS message includes 3 bytes for its length in the header for the message.
This would allow for messages up to 16Mb in length. Messages of this length
are excessive and OpenSSL includes a check to ensure that a peer is sending
reasonably sized messages in order to avoid too much memory being consumed
to service a connection. A flaw in the logic of version 1.1.0 means that
memory for the message is allocated too early, prior to the excessive
message length check. Due to way memory is allocated in OpenSSL this could
mean an attacker could force up to 21Mb to be allocated to service a
connection. This could lead to a Denial of Service through memory
exhaustion. However, the excessive message length check still takes place,
and this would cause the connection to immediately fail. Assuming that the
application calls SSL_free() on the failed conneciton in a timely manner
then the 21Mb of allocated memory will then be immediately freed again.
Therefore the excessive memory allocation will be transitory in nature.
This then means that there is only a security impact if:

1) The application does not call SSL_free() in a timely manner in the
event that the connection fails
or
2) The application is working in a constrained environment where there
is very little free memory
or
3) The attacker initiates multiple connection attempts such that there
are multiple connections in a state where memory has been allocated for
the connection; SSL_free() has not yet been called; and there is
insufficient memory to service the multiple requests.

Except in the instance of (1) above any Denial Of Service is likely to
be transitory because as soon as the connection fails the memory is
subsequently freed again in the SSL_free() call. However there is an
increased risk during this period of application crashes due to the lack
of memory - which would then mean a more serious Denial of Service.

This issue does not affect DTLS users.

Issue was reported by Shi Lei (Gear Team, Qihoo 360 Inc.).

CVE-2016-6307

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-09-21 20:37:53 +01:00
Alessandro Ghedini
4f8a5f4da9 Use switch instead of multiple ifs
Makes the logic a little bit clearer.

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1571)
2016-09-16 11:07:08 -04: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
FdaSilvaYY
e8aa8b6c8f Fix a few if(, for(, while( inside code.
Fix some indentation at the same time

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1292)
2016-07-20 07:21:53 -04:00
Matt Caswell
672f3337c3 Fix SSLv3 alert if no Client Ceritifcate sent after a request for one
In TLS if the server sends a CertificateRequest and the client does not
provide one, if the server cannot continue it should send a
HandshakeFailure alert. In SSLv3 the same should happen, but instead we
were sending an UnexpectedMessage alert. This is incorrect - the message
isn't unexpected - it is valid for the client not to send one - its just
that we cannot continue without one.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-07-18 14:30:14 +01:00
Matt Caswell
2c4a056f59 Handle a memory allocation failure in ssl3_init_finished_mac()
The ssl3_init_finished_mac() function can fail, in which case we need to
propagate the error up through the stack.

RT#3198

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-06-03 20:29:04 +01:00
Matt Caswell
1689e7e688 Ensure async IO works with new state machine
In the new state machine if using nbio and we get the header of a
handshake message is one record with the body in the next, with an nbio
event in the middle, then the connection was failing. This is because
s->init_num was getting reset. We should only reset it after we have
read the whole message.

RT#4394

Reviewed-by: Andy Polyakov <appro@openssl.org>
2016-05-20 14:39:07 +01:00
Matt Caswell
464175692f Simplify SSL BIO buffering logic
The write BIO for handshake messages is bufferred so that we only write
out to the network when we have a complete flight. There was some
complexity in the buffering logic so that we switched buffering on and
off at various points through out the handshake. The only real reason to
do this was historically it complicated the state machine when you wanted
to flush because you had to traverse through the "flush" state (in order
to cope with NBIO). Where we knew up front that there was only going to
be one message in the flight we switched off buffering to avoid that.

In the new state machine there is no longer a need for a flush state so
it is simpler just to have buffering on for the whole handshake. This
also gives us the added benefit that we can simply call flush after every
flight even if it only has one message in it. This means that BIO authors
can implement their own buffering strategies and not have to be aware of
the state of the SSL object (previously they would have to switch off
their own buffering during the handshake because they could not rely on
a flush being received when they really needed to write data out). This
last point addresses GitHub Issue #322.

Reviewed-by: Andy Polyakov <appro@openssl.org>
2016-05-20 14:11:11 +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
FdaSilvaYY
0d4fb84390 GH601: Various spelling fixes.
Signed-off-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-02-05 15:25:50 -05: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
Viktor Dukhovni
4fa52141b0 Protocol version selection and negotiation rewrite
The protocol selection code is now consolidated in a few consecutive
short functions in a single file and is table driven.  Protocol-specific
constraints that influence negotiation are moved into the flags
field of the method structure.  The same protocol version constraints
are now applied in all code paths.  It is now much easier to add
new protocol versions without reworking the protocol selection
logic.

In the presence of "holes" in the list of enabled client protocols
we no longer select client protocols below the hole based on a
subset of the constraints and then fail shortly after when it is
found that these don't meet the remaining constraints (suiteb, FIPS,
security level, ...).  Ideally, with the new min/max controls users
will be less likely to create "holes" in the first place.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2016-01-02 10:49:06 -05:00
Matt Caswell
d99b0691d3 Fix compilation problems with SCTP
The SCTP code is not compiled by default. This fixes some compilation
problems in that code.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-11-06 15:40:31 +00:00
Matt Caswell
8481f583d3 Add ossl_statem prefix to various functions
Add the ossl_statem prefix to various funtions to avoid name clashes.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:47 +00:00
Matt Caswell
be3583fa40 Convert enums to typedefs
Various enums were introduced as part of the state machine rewrite. As a
matter of style it is preferred for these to be typedefs.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:47 +00:00
Matt Caswell
006207d1bc Fix a bogus clang warning
Clang with --strict-warnings was complaining about an uninitalised
variable. In reality it will never be used uninitialised but clang can't
figure out the logic, so just init it anyway to silence the warning.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:47 +00:00
Matt Caswell
024f543c15 Move in_handshake into STATEM
The SSL variable |in_handshake| seems misplaced. It would be better to have
it in the STATEM structure.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:47 +00:00
Matt Caswell
31fd10e60d Fix DTLSv1_listen following state machine changes
Adding the new state machine broke the DTLSv1_listen code because
calling SSL_in_before() was erroneously returning true after DTLSv1_listen
had successfully completed. This change ensures that SSL_in_before returns
false.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:47 +00:00
Matt Caswell
91eac8d567 Add a function to get the info_callback
Remove repeated blocks of checking SSL and then SSL_CTX for the
info_callback.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:47 +00:00
Matt Caswell
d78052cf4f Change SUB_STATE_RETURN into a typedef
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:46 +00:00
Matt Caswell
d6f1a6e931 Rename STATEM to OSSL_STATEM
Add an OSSL_ prefix onto STATEM

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:46 +00:00
Matt Caswell
5998e29035 Remove SSL_state and SSL_set_state
SSL_state has been replaced by SSL_get_state and SSL_set_state is no longer
supported.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:46 +00:00
Matt Caswell
a71a4966a3 Move |no_cert_verify| into state machine
The |no_cert_verify| should be in the state machine structure not in SSL

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:46 +00:00
Matt Caswell
fe3a329117 Change statem prefix to ossl_statem
Change various state machine functions to use the prefix ossl_statem
instead.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:46 +00:00
Matt Caswell
35bf6e0537 Change HANDSHAKE_STATE to OSSL_HANDSHAKE_STATE
Rename the enum HANDSHAKE_STATE to OSSL_HANDSHAKE_STATE to ensure there are
no namespace clashes, and convert it into a typedef.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:39:46 +00:00
Matt Caswell
e0a51488c3 Fix a comment
Fix an out of date reference to old state machine code in a comment

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:38:18 +00:00
Matt Caswell
61ae935a98 More state machine reorg
Move some function definitions around within the state machine to make sure
they are in the correct files. Also create a statem_locl.h header for stuff
entirely local to the state machine code and move various definitions into
it.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:38:18 +00:00
Matt Caswell
8ba708e516 Reorganise state machine files
Pull out the state machine into a separate sub directory. Also moved some
functions which were nothing to do with the state machine but were in state
machine files. Pulled all the SSL_METHOD definitions into one place...most
of those files had very little left in them any more.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-10-30 08:38:18 +00:00
Renamed from ssl/statem.c (Browse further)