Commit graph

355 commits

Author SHA1 Message Date
Emilia Kasper
ec30e8566e PACKET: add methods for reading length-prefixed TLS vectors.
Rewrite ssl3_get_client_hello to use the new methods.

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-08-26 13:25:48 +02:00
Emilia Kasper
9cc3e8f1f2 Fix SSLv2-compatible ClientHello processing.
If the client challenge is less than 32 bytes, it is padded with leading - not trailing - zero bytes.

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-08-26 13:23:24 +02:00
Matt Caswell
efcdbcbeda PACKETise ClientKeyExchange processing
Use the new PACKET code to process the CKE message

Reviewed-by: Stephen Henson <steve@openssl.org>
2015-08-14 17:19:57 +01:00
Matt Caswell
bc6616a434 Enhance PACKET readability
Enhance the PACKET code readability, and fix a stale comment. Thanks
to Ben Kaduk (bkaduk@akamai.com) for pointing this out.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2015-08-13 20:34:51 +01:00
Matt Caswell
f9f6053442 Add missing return check for PACKET_buf_init
The new ClientHello PACKET code is missing a return value check.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2015-08-13 20:34:51 +01:00
Rich Salz
ade44dcb16 Remove Gost94 signature algorithm.
This was obsolete in 2001.  This is not the same as Gost94 digest.
Thanks to Dmitry Belyavsky <beldmit@gmail.com> for review and advice.

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-08-11 18:23:29 -04:00
Adam Eijdenberg
6f136aa6fc Change error reason to match previous behaviour.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2015-08-04 15:15:38 -07:00
Adam Eijdenberg
6c3cca5793 Fix unhandled error condition in sslv2 client hello parsing.
--strict-warnings started showing warnings for this today...

Surely an error should be raised if these reads fail?

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2015-08-04 15:05:01 -07:00
Matt Caswell
c3fc7eeab8 PACKETise NextProto
Change NextProto message processing to use the PACKET API.

Reviewed-by: Stephen Henson <steve@openssl.org>
2015-08-04 14:05:10 +01:00
Matt Caswell
f532a35d2a PACKETise CertificateVerify processing
Modify CertificateVerify processing to use the new PACKET API.

Reviewed-by: Stephen Henson <steve@openssl.org>
2015-08-04 13:56:50 +01:00
Matt Caswell
0bc09ecd26 PACKETise ClientCertificate processing
Use the PACKET API for processing ClientCertificate messages

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-08-04 13:15:29 +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
9ceb2426b0 PACKETise ClientHello processing
Uses the new PACKET code to process the incoming ClientHello including all
extensions etc.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-08-03 11:01:42 +01:00
Matt Caswell
e1e088ec7f Remove erroneous server_random filling
Commit e481f9b90b removed OPENSSL_NO_TLSEXT from the code.

Previously if OPENSSL_NO_TLSEXT *was not* defined then the server random was
filled during getting of the ClientHello. If it *was* defined then the
server random would be filled in ssl3_send_server_hello(). Unfortunately in
commit e481f9b90b the OPENSSL_NO_TLSEXT guards were removed but *both*
server random fillings were left in. This could cause problems for session
ticket callbacks.

Reviewed-by: Stephen Henson <steve@openssl.org>
2015-07-31 20:30:35 +01:00
Dr. Stephen Henson
a3f7ff2b2d Don't request certificates for any PSK ciphersuite
Reviewed-by: Matt Caswell <matt@openssl.org>
2015-07-30 14:55:33 +01:00
Dr. Stephen Henson
85269210ff Extended PSK server support.
Add support for RSAPSK, DHEPSK and ECDHEPSK server side.

Update various checks to ensure certificate and server key exchange messages
are only sent when required.

Update message handling. PSK server key exchange parsing now include an
identity hint prefix for all PSK server key exchange messages. PSK
client key exchange message expects PSK identity and requests key for
all PSK key exchange ciphersuites.

Update flags for RSA, DH and ECDH so they are also used in PSK.

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-07-30 14:43:35 +01:00
Dr. Stephen Henson
57b272b01a Use single master secret generation function.
Reviewed-by: Matt Caswell <matt@openssl.org>
2015-06-29 11:47:59 +01:00
Dr. Stephen Henson
124037fdc0 Tidy up ssl3_digest_cached_records logic.
Rewrite ssl3_digest_cached_records handling. Only digest cached records
if digest array is NULL: this means it is safe to call
ssl3_digest_cached_records multiple times (subsequent calls are no op).

Remove flag TLS1_FLAGS_KEEP_HANDSHAKE instead only update handshake buffer
if digest array is NULL.

Add additional "keep" parameter to ssl3_digest_cached_records to indicate
if the handshake buffer should be retained after digesting cached records
(needed for TLS 1.2 client authentication).

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-06-23 22:24:09 +01:00
Dr. Stephen Henson
389ebcecae Remove SESS_CERT entirely.
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-06-22 13:52:24 +01:00
Dr. Stephen Henson
c34b0f9930 Move peer chain to SSL_SESSION structure.
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-06-22 13:52:24 +01:00
Matt Caswell
b7ee4815f2 Clean premaster_secret for GOST
Ensure OPENSSL_cleanse() is called on the premaster secret value calculated for GOST.

With thanks to the Open Crypto Audit Project for reporting this issue.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-06-04 11:20:51 +01:00
Rich Salz
e0f9bf1de7 clear/cleanse cleanup
Where we called openssl_cleanse, make sure we do it on all error
paths.  Be consistent in use of sizeof(foo) when possible.

Reviewed-by: Andy Polyakov <appro@openssl.org>
2015-05-30 12:28:05 -04:00
Matt Caswell
e481f9b90b Remove support for OPENSSL_NO_TLSEXT
Given the pervasive nature of TLS extensions it is inadvisable to run
OpenSSL without support for them. It also means that maintaining
the OPENSSL_NO_TLSEXT option within the code is very invasive (and probably
not well tested). Therefore it is being removed.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-05-22 23:10:51 +01:00
Dr. Stephen Henson
76106e60a8 CERT tidy
Move per-connection state out of the CERT structure: which should just be
for shared configuration data (e.g. certificates to use).

In particular move temporary premaster secret, raw ciphers, peer signature
algorithms and shared signature algorithms.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-05-18 18:49:13 +01:00
Matt Caswell
b2ce0337e8 Further version negotiation updates
More miscellaneous updates to version negotiation following feedback.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-05-16 09:21:07 +01:00
Matt Caswell
a304d38590 Move SSLv3_*method() functions
Move these functions into t1_clnt.c, t1_srvr.c and t1_meth.c and take
advantage of the existing tls1_get*_method() functions that all the other
methods are using. Since these now have to support SSLv3 anyway we might
as well use the same set of get functions for both TLS and SSLv3.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-05-16 09:20:58 +01:00
Matt Caswell
d45ba43dab Updates following review comments
Miscellaneous updates following review comments on the version negotiation
rewrite patches.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-05-16 09:20:52 +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
Richard Levitte
6857079791 Identify and move OpenSSL internal header files
There are header files in crypto/ that are used by the rest of
OpenSSL.  Move those to include/internal and adapt the affected source
code, Makefiles and scripts.

The header files that got moved are:

crypto/constant_time_locl.h
crypto/o_dir.h
crypto/o_str.h

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-05-14 15:13:49 +02:00
Matt Caswell
55a9a16f1c Remove Kerberos support from libssl
Remove RFC2712 Kerberos support from libssl. This code and the associated
standard is no longer considered fit-for-purpose.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-05-13 15:07:57 +01:00
Matt Caswell
a0bd649336 Don't allow a CCS when expecting a CertificateVerify
Currently we set change_cipher_spec_ok to 1 before calling
ssl3_get_cert_verify(). This is because this message is optional and if it
is not sent then the next thing we would expect to get is the CCS. However,
although it is optional, we do actually know whether we should be receiving
one in advance. If we have received a client cert then we should expect
a CertificateVerify message. By the time we get to this point we will
already have bombed out if we didn't get a Certificate when we should have
done, so it is safe just to check whether |peer| is NULL or not. If it is
we won't get a CertificateVerify, otherwise we will. Therefore we should
change the logic so that we only attempt to get the CertificateVerify if
we are expecting one, and not allow a CCS in this scenario.

Whilst this is good practice for TLS it is even more important for DTLS.
In DTLS messages can be lost. Therefore we may be in a situation where a
CertificateVerify message does not arrive even though one was sent. In that
case the next message the server will receive will be the CCS. This could
also happen if messages get re-ordered in-flight. In DTLS if
|change_cipher_spec_ok| is not set and a CCS is received it is ignored.
However if |change_cipher_spec_ok| *is* set then a CCS arrival will
immediately move the server into the next epoch. Any messages arriving for
the previous epoch will be ignored. This means that, in this scenario, the
handshake can never complete. The client will attempt to retransmit
missing messages, but the server will ignore them because they are the wrong
epoch. The server meanwhile will still be waiting for the CertificateVerify
which is never going to arrive.

RT#2958

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2015-05-13 11:17:17 +01:00
Rich Salz
75ebbd9aa4 Use p==NULL not !p (in if statements, mainly)
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-05-11 10:06:38 -04:00
Rich Salz
16f8d4ebf0 memset, memcpy, sizeof consistency fixes
Just as with the OPENSSL_malloc calls, consistently use sizeof(*ptr)
for memset and memcpy.  Remove needless casts for those functions.
For memset, replace alternative forms of zero with 0.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-05-05 22:18:59 -04:00
Matt Caswell
cf9b0b6fb2 Add more error state transitions
Ensure all fatal errors transition into the new error state on the server
side.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-05-05 19:45:37 +01:00
Matt Caswell
a89db885e0 Add Error state
Reusing an SSL object when it has encountered a fatal error can
have bad consequences. This is a bug in application code not libssl
but libssl should be more forgiving and not crash.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-05-05 19:45:17 +01:00
mancha security
59ef580a14 ssl/s3_srvr.c: Fix typo introduced via 69f6823748.
Incorrect name used for SSL_AD_INTERNAL_ERROR.

Signed-off-by: mancha security <mancha1@zoho.com>
Signed-off-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-05-03 23:36:25 +01:00
Rich Salz
25aaa98aa2 free NULL cleanup -- coda
After the finale, the "real" final part. :)  Do a recursive grep with
"-B1 -w [a-zA-Z0-9_]*_free" to see if any of the preceeding lines are
an "if NULL" check that can be removed.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-05-01 14:37:16 -04:00
Rich Salz
666964780a Remove goto inside an if(0) block
There were a dozen-plus instances of this construct:
   if (0) { label: ..... }

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-05-01 14:29:48 -04:00
Rich Salz
b548a1f11c free null cleanup finale
Don't check for NULL before calling OPENSSL_free

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-05-01 10:02:07 -04:00
Rich Salz
222561fe8e free NULL cleanup 5a
Don't check for NULL before calling a free routine.  This gets X509_.*free:
    x509_name_ex_free X509_policy_tree_free X509_VERIFY_PARAM_free
    X509_STORE_free X509_STORE_CTX_free X509_PKEY_free
    X509_OBJECT_free_contents X509_LOOKUP_free X509_INFO_free

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-04-30 17:33:59 -04:00
Emilia Kasper
3ae91cfb32 Error out immediately on empty ciphers list.
A 0-length ciphers list is never permitted. The old code only used to
reject an empty ciphers list for connections with a session ID. It
would later error out on a NULL structure, so this change just moves
the alert closer to the problem source.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-04-17 18:43:30 +02:00
Viktor Dukhovni
61986d32f3 Code style: space after 'if'
Reviewed-by: Matt Caswell <matt@openssl.org>
2015-04-16 13:44:59 -04:00
Matt Caswell
5e9f0eebcf Check for ClientHello message overruns
The ClientHello processing is insufficiently rigorous in its checks to make
sure that we don't read past the end of the message. This does not have
security implications due to the size of the underlying buffer - but still
needs to be fixed.

With thanks to Qinghao Tang for reporting this issue.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-04-14 14:58:25 +01:00
Rich Salz
c5ba2d9904 free NULL cleanup
EVP_.*free; this gets:
        EVP_CIPHER_CTX_free EVP_PKEY_CTX_free EVP_PKEY_asn1_free
        EVP_PKEY_asn1_set_free EVP_PKEY_free EVP_PKEY_free_it
        EVP_PKEY_meth_free; and also EVP_CIPHER_CTX_cleanup

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-03-28 10:54:15 -04:00
Rich Salz
8fdc3734c0 free NULL cleanup.
This gets EC_GROUP_clear_free EC_GROUP_free, EC_KEY_free,
EC_POINT_clear_free, EC_POINT_free

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-03-25 18:35:24 -04:00
Matt Caswell
266483d2f5 RAND_bytes updates
Ensure RAND_bytes return value is checked correctly, and that we no longer
use RAND_pseudo_bytes.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-03-25 12:38:07 +00:00
Matt Caswell
4f9fab6bd0 Add ticket length before buffering DTLS message
In ssl3_send_new_session_ticket the message to be sent is constructed. We
skip adding the length of the session ticket initially, then call
ssl_set_handshake_header, and finally go back and add in the length of the
ticket. Unfortunately, in DTLS, ssl_set_handshake_header also has the side
effect of buffering the message for subsequent retransmission if required.
By adding the ticket length after the call to ssl_set_handshake_header the
message that is buffered is incomplete, causing an invalid message to be
sent on retransmission.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-03-25 12:24:36 +00:00
Kurt Roeckx
24a0d3933d Make sure that cert is never NULL
Also removes for it being NULL

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-03-23 21:08:20 +01:00
Matt Caswell
77d514c5a0 ssl3_set_handshake_header returns
Change ssl_set_handshake_header from return void to returning int, and
handle error return code appropriately.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-03-23 15:23:24 +00:00
Matt Caswell
69f6823748 Fix missing return value checks
Ensure that all functions have their return values checked where
appropriate. This covers all functions defined and called from within
libssl.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-03-23 15:23:11 +00:00