Commit graph

1521 commits

Author SHA1 Message Date
Matt Caswell
a329ae2268 Add more error state transitions (client)
Ensure all fatal errors transition into the new error state on the client
side.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit cc273a9361)

Conflicts:
	ssl/s3_clnt.c

Conflicts:
	ssl/s3_clnt.c
2015-05-05 20:08:35 +01:00
Matt Caswell
f3c4abb377 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>
(cherry picked from commit cf9b0b6fb2)

Conflicts:
	ssl/s3_srvr.c
2015-05-05 20:07:48 +01:00
Matt Caswell
189e20c68c 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>
(cherry picked from commit a89db885e0)

Conflicts:
	ssl/s3_srvr.c
	ssl/ssl_stat.c
2015-05-05 20:07:48 +01:00
Matt Caswell
39b36cb438 Add sanity check to ssl_get_prev_session
Sanity check the |len| parameter to ensure it is positive. Thanks to Kevin
Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
reporting this issue.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit cb0f400b0c)
2015-04-30 23:27:07 +01:00
Matt Caswell
26800340db Sanity check the return from final_finish_mac
The return value is checked for 0. This is currently safe but we should
really check for <= 0 since -1 is frequently used for error conditions.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit c427570e50)

Conflicts:
	ssl/ssl_locl.h

Conflicts:
	ssl/ssl_locl.h
2015-04-30 23:27:05 +01:00
Matt Caswell
592ac25342 Add sanity check in ssl3_cbc_digest_record
For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst
this is true for all SSLv3 ciphersuites, this fact is far from obvious by
looking at the code. If this were not the case then an integer overflow
would occur, leading to a subsequent buffer overflow. Therefore I have
added an explicit sanity check to ensure header_length is always valid.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 29b0a15a48)
2015-04-30 23:26:07 +01:00
Matt Caswell
974d4d675c Sanity check EVP_CTRL_AEAD_TLS_AAD
The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at
least 13 bytes long. Add sanity checks to ensure that the length is at
least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to
represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and
Paramjot Oberoi (Int3 Solutions) for reporting this issue.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit c826988109)

Conflicts:
	ssl/record/ssl3_record.c

Conflicts:
	apps/speed.c
	crypto/evp/e_aes_cbc_hmac_sha256.c
	crypto/evp/evp.h
2015-04-30 23:26:06 +01:00
Matt Caswell
80a06268ae Add length sanity check in SSLv2 n_do_ssl_write()
Fortify flagged up a problem in n_do_ssl_write() in SSLv2. Analysing the
code I do not believe there is a real problem here. However the logic flows
are complicated enough that a sanity check of |len| is probably worthwhile.

Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit c5f8cd7bc6)
2015-04-29 17:44:02 +01:00
Emilia Kasper
d695a02254 Repair EAP-FAST session resumption
EAP-FAST session resumption relies on handshake message lookahead
to determine server intentions. Commits
980bc1ec61
and
7b3ba508af
removed the lookahead so broke session resumption.

This change partially reverts the commits and brings the lookahead back
in reduced capacity for TLS + EAP-FAST only. Since EAP-FAST does not
support regular session tickets, the lookahead now only checks for a
Finished message.

Regular handshakes are unaffected by this change.

Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 6e3d015363)
2015-04-21 19:37:17 +02:00
Emilia Kasper
92caee08d3 make update
Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2015-04-21 17:53:36 +02:00
Emilia Kasper
31d085ca74 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>
(cherry picked from commit 3ae91cfb32)
2015-04-17 18:47:25 +02:00
Viktor Dukhovni
c70908d247 Code style: space after 'if'
Reviewed-by: Matt Caswell <gitlab@openssl.org>
2015-04-16 13:51:51 -04:00
Matt Caswell
40f26ac782 Fix ssl_get_prev_session overrun
If OpenSSL is configured with no-tlsext then ssl_get_prev_session can read
past the end of the ClientHello message if the session_id length in the
ClientHello is invalid. This should not cause any security issues since the
underlying buffer is 16k in size. It should never be possible to overrun by
that many bytes.

This is probably made redundant by the previous commit - but you can never be
too careful.

With thanks to Qinghao Tang for reporting this issue.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 5e0a80c1c9)
2015-04-14 14:59:54 +01:00
Matt Caswell
89c2720298 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>
(cherry picked from commit c9642eb1ff79a30e2c7632ef8267cc34cc2b0d79)
2015-04-14 14:50:20 +01:00
Emilia Kasper
a20db08e77 Harden SSLv2-supporting servers against Bleichenbacher's attack.
There is no indication that the timing differences are exploitable in
OpenSSL, and indeed there is some indication (Usenix '14) that they
are too small to be exploitable. Nevertheless, be careful and apply
the same countermeasures as in s3_srvr.c

Thanks to Nimrod Aviram, Sebastian Schinzel and Yuval Shavitt for
reporting this issue.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit ae50d82700)
2015-04-08 16:42:28 +02:00
Matt Caswell
750190567a Fix RAND_(pseudo_)?_bytes returns
Ensure all calls to RAND_bytes and RAND_pseudo_bytes have their return
value checked correctly

Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 8f8e4e4f52)

Conflicts:
	crypto/evp/e_des3.c
2015-03-25 12:45:17 +00:00
Kurt Roeckx
23a9b24aa1 Don't send a for ServerKeyExchange for kDHr and kDHd
The certificate already contains the DH parameters in that case.
ssl3_send_server_key_exchange() would fail in that case anyway.

Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 93f1c13619)
2015-03-24 22:58:30 +01:00
Matt Caswell
8ca79fcbf4 Fix unsigned/signed warnings
Fix some unsigned/signed warnings introduced as part of the fix
for CVE-2015-0293

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-03-19 12:59:31 +00:00
Emilia Kasper
cd56a08d4e Fix reachable assert in SSLv2 servers.
This assert is reachable for servers that support SSLv2 and export ciphers.
Therefore, such servers can be DoSed by sending a specially crafted
SSLv2 CLIENT-MASTER-KEY.

Also fix s2_srvr.c to error out early if the key lengths are malformed.
These lengths are sent unencrypted, so this does not introduce an oracle.

CVE-2015-0293

This issue was discovered by Sean Burford (Google) and Emilia Käsper of
the OpenSSL development team.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-03-19 12:59:31 +00:00
Matt Caswell
f08731cd82 Add sanity check to PRF
The function tls1_PRF counts the number of digests in use and partitions
security evenly between them. There always needs to be at least one digest
in use, otherwise this is an internal error. Add a sanity check for this.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 668f6f08c6)
2015-03-17 13:49:32 +00:00
Matt Caswell
58d8a271ab Cleanse buffers
Cleanse various intermediate buffers used by the PRF (backported version
from master).

Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 35fafc4dbc)

Conflicts:
	ssl/s3_enc.c
2015-03-11 10:49:22 +00:00
Emilia Kasper
8b7e469d06 Harmonize return values in dtls1_buffer_record
Ensure all malloc failures return -1.

Reported by Adam Langley (Google).

Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 06c6a2b4a3)
2015-03-10 13:52:37 -07:00
Dr. Stephen Henson
a67303954c fix warning
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit d6ca1cee8b)
2015-03-08 22:42:23 +00:00
Kurt Roeckx
bc2e18a3c8 Remove export ciphers from the DEFAULT cipher list
They are moved to the COMPLEMENTOFDEFAULT instead.
This also fixes SSLv2 to be part of COMPLEMENTOFDEFAULT.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit f417997a32)

Conflicts:
	ssl/ssl_ciph.c
2015-03-07 23:08:12 +01:00
Kurt Cancemi
183db9af80 Use constants not numbers
This patch uses warning/fatal constants instead of numbers with comments for
warning/alerts in d1_pkt.c and s3_pkt.c

RT#3725

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit fd865cadcb)
2015-03-05 09:30:35 +00:00
Matt Caswell
5c921f14cb Fix d2i_SSL_SESSION for DTLS1_BAD_VER
Some Cisco appliances use a pre-standard version number for DTLS. We support
this as DTLS1_BAD_VER within the code.

This change fixes d2i_SSL_SESSION for that DTLS version.

Based on an original patch by David Woodhouse <dwmw2@infradead.org>

RT#3704

Reviewed-by: Tim Hudson <tjh@openssl.org>

Conflicts:
	ssl/ssl_asn1.c

Conflicts:
	ssl/dtls1.h
2015-02-27 20:32:49 +00:00
Matt Caswell
d58a852fbd Fixed missing return value checks.
Added various missing return value checks in tls1_change_cipher_state.

Reviewed-by: Richard Levitte <levitte@openssl.org>

Conflicts:
	ssl/t1_enc.c
2015-02-27 15:26:06 +00:00
Matt Caswell
323a7e76e6 Fix missing return value checks.
Fixed various missing return value checks in ssl3_send_newsession_ticket.
Also a mem leak on error.

Reviewed-by: Richard Levitte <levitte@openssl.org>

Conflicts:
	ssl/s3_srvr.c

Conflicts:
	ssl/s3_srvr.c
2015-02-27 15:25:05 +00:00
Matt Caswell
ea65e92b22 Fix no-ec warning
This is a partial back port of commit 5b430cfc to remove a warning when
compiling with no-ec.

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2015-02-27 08:57:44 +00:00
Eric Dequin
a1331af032 Missing OPENSSL_free on error path.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 1d2932de4c)
2015-02-12 11:15:39 -05:00
Matt Caswell
f8e662e71c Fix error handling in ssltest
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit ae632974f9)
2015-02-06 10:10:49 +00:00
Matt Caswell
1895583835 Make DTLS always act as if read_ahead is set. The actual value of read_ahead
is ignored for DTLS.

RT#3657

Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 8dd4ad0ff5)
2015-01-27 14:33:32 +00:00
Matt Caswell
cda8845ded Re-align some comments after running the reformat script.
This should be a one off operation (subsequent invokation of the
script should not move them)

This commit is for the 1.0.1 changes

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:39:01 +00:00
Matt Caswell
47050853f1 Rerun util/openssl-format-source -v -c .
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:38:49 +00:00
Matt Caswell
10621efd32 Run util/openssl-format-source -v -c .
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:38:39 +00:00
Matt Caswell
e498b83fed More tweaks for comments due indent issues
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:38:30 +00:00
Matt Caswell
f7b36402d6 Tweaks for comments due to indent's inability to handle them
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:38:11 +00:00
Matt Caswell
0f6c965823 Move more comments that confuse indent
Conflicts:
	crypto/dsa/dsa.h
	demos/engines/ibmca/hw_ibmca.c
	ssl/ssl_locl.h

Conflicts:
	crypto/bn/rsaz_exp.c
	crypto/evp/e_aes_cbc_hmac_sha1.c
	crypto/evp/e_aes_cbc_hmac_sha256.c
	ssl/ssl_locl.h

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:38:04 +00:00
Matt Caswell
1ac02e4b89 Fix indent comment corruption issue
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:37:15 +00:00
Matt Caswell
4017726f72 Fix strange formatting by indent
Conflicts:
	crypto/hmac/hmac.h

Conflicts:
	crypto/evp/e_aes_cbc_hmac_sha256.c

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:36:29 +00:00
Matt Caswell
3cf9f81b09 indent has problems with comments that are on the right hand side of a line.
Sometimes it fails to format them very well, and sometimes it corrupts them!
This commit moves some particularly problematic ones.

Conflicts:
	crypto/bn/bn.h
	crypto/ec/ec_lcl.h
	crypto/rsa/rsa.h
	demos/engines/ibmca/hw_ibmca.c
	ssl/ssl.h
	ssl/ssl3.h

Conflicts:
	crypto/ec/ec_lcl.h
	ssl/tls1.h

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:36:16 +00:00
Matt Caswell
1566497495 Fix source where indent will not be able to cope
Conflicts:
	apps/ciphers.c
	ssl/s3_pkt.c

Conflicts:
	crypto/ec/ec_curve.c

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:33:54 +00:00
Matt Caswell
712548231e Additional comment changes for reformat of 1.0.1
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:33:47 +00:00
Matt Caswell
ac84cb4cfe Further comment changes for reformat
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:33:38 +00:00
Tim Hudson
3e9a08ecb1 mark all block comments that need format preserving so that
indent will not alter them when reformatting comments

(cherry picked from commit 1d97c84351)

Conflicts:
	crypto/bn/bn_lcl.h
	crypto/bn/bn_prime.c
	crypto/engine/eng_all.c
	crypto/rc4/rc4_utl.c
	crypto/sha/sha.h
	ssl/kssl.c
	ssl/t1_lib.c

Conflicts:
	crypto/rc4/rc4_enc.c
	crypto/x509v3/v3_scts.c
	crypto/x509v3/v3nametest.c
	ssl/d1_both.c
	ssl/s3_srvr.c
	ssl/ssl.h
	ssl/ssl_locl.h
	ssl/ssltest.c
	ssl/t1_lib.c

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-01-22 09:33:23 +00:00
Matt Caswell
04685bc949 A memory leak can occur in dtls1_buffer_record if either of the calls to
ssl3_setup_buffers or pqueue_insert fail. The former will fail if there is a
malloc failure, whilst the latter will fail if attempting to add a duplicate
record to the queue. This should never happen because duplicate records should
be detected and dropped before any attempt to add them to the queue.
Unfortunately records that arrive that are for the next epoch are not being
recorded correctly, and therefore replays are not being detected.
Additionally, these "should not happen" failures that can occur in
dtls1_buffer_record are not being treated as fatal and therefore an attacker
could exploit this by sending repeated replay records for the next epoch,
eventually causing a DoS through memory exhaustion.

Thanks to Chris Mueller for reporting this issue and providing initial
analysis and a patch. Further analysis and the final patch was performed by
Matt Caswell from the OpenSSL development team.

CVE-2015-0206

Reviewed-by: Dr Stephen Henson <steve@openssl.org>
2015-01-08 13:43:20 +00:00
Dr. Stephen Henson
98a0f9660d Unauthenticated DH client certificate fix.
Fix to prevent use of DH client certificates without sending
certificate verify message.

If we've used a client certificate to generate the premaster secret
ssl3_get_client_key_exchange returns 2 and ssl3_get_cert_verify is
never called.

We can only skip the certificate verify message in
ssl3_get_cert_verify if the client didn't send a certificate.

Thanks to Karthikeyan Bhargavan for reporting this issue.
CVE-2015-0205
Reviewed-by: Matt Caswell <matt@openssl.org>
2015-01-08 13:43:20 +00:00
Matt Caswell
45fe66b8ba Follow on from CVE-2014-3571. This fixes the code that was the original source
of the crash due to p being NULL. Steve's fix prevents this situation from
occuring - however this is by no means obvious by looking at the code for
dtls1_get_record. This fix just makes things look a bit more sane.

Reviewed-by: Dr Steve Henson <steve@openssl.org>
2015-01-08 13:43:20 +00:00
Dr. Stephen Henson
8d7aab986b Fix crash in dtls1_get_record whilst in the listen state where you get two
separate reads performed - one for the header and one for the body of the
handshake record.

CVE-2014-3571

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-01-08 13:43:20 +00:00
Dr. Stephen Henson
ffd14272c4 fix error discrepancy
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 4a4d415857)
2015-01-07 18:10:51 +00:00