Commit graph

18210 commits

Author SHA1 Message Date
Matt Caswell
83ae466131 Fix missing NULL checks in NewSessionTicket construction
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 16:15:16 +01:00
Matt Caswell
e4e1aa903e Fix an mis-matched function code so that "make update" doesn't fail
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 15:32:35 +01:00
Matt Caswell
0023baffb8 Add an example of usage to the WPACKET_reserve_bytes() documentation
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 15:09:02 +01:00
Matt Caswell
ff8194774c Address style feedback comments
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 15:09:02 +01:00
Matt Caswell
4a424545c4 Fix a bug in CKE construction for PSK
In plain PSK we don't need to do anymore construction after the preamble.
We weren't detecting this case and treating it as an unknown cipher.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 15:09:02 +01:00
Matt Caswell
c13d2a5be7 Convert ServerKeyExchange construction to WPACKET
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 15:09:02 +01:00
Matt Caswell
1ff8434040 Add the WPACKET_reserve_bytes() function
WPACKET_allocate_bytes() requires you to know the size of the data you
are allocating for, before you create it. Sometimes this isn't the case,
for example we know the maximum size that a signature will be before we
create it, but not the actual size. WPACKET_reserve_bytes() enables us to
reserve bytes in the WPACKET, but not count them as written yet. We then
subsequently need to acall WPACKET_allocate_bytes to actually count them as
written.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 15:09:02 +01:00
Matt Caswell
ac8cc3efb2 Remove tls12_copy_sigalgs_old()
This was a temporary function needed during the conversion to WPACKET. All
callers have now been converted to the new way of doing this so this
function is no longer required.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 14:52:55 +01:00
Matt Caswell
28ff8ef3f7 Convert CertificateRequest construction to WPACKET
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 14:52:55 +01:00
Matt Caswell
25849a8f8b Address style feedback comments
Merge declarations of same type together.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 10:06:46 +01:00
Matt Caswell
7facdbd66f Fix a bug in the construction of the ClienHello SRTP extension
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 10:06:46 +01:00
Matt Caswell
7507e73d40 Fix heartbeat compilation error
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 10:06:46 +01:00
Matt Caswell
150e298551 Delete some unneeded code
Some functions were being called from both code that used WPACKETs and code
that did not. Now that more code has been converted to use WPACKETs some of
that duplication can be removed.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 10:06:46 +01:00
Matt Caswell
8157d44b62 Convert ServerHello construction to WPACKET
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 10:06:46 +01:00
Matt Caswell
2f2d6e3e3c Fix an Uninit read in DTLS
If we have a handshake fragment waiting then dtls1_read_bytes() was not
correctly setting the value of recvd_type, leading to an uninit read.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-29 09:58:14 +01:00
Matt Caswell
55386bef80 Fix no-dtls
The new large message test in sslapitest needs OPENSSL_NO_DTLS guards

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-09-29 09:52:31 +01:00
Richard Levitte
49e476a538 apps/apps.c: initialize and de-initialize engine around key loading
Before loading a key from an engine, it may need to be initialized.
When done loading the key, we must de-initialize the engine.
(if the engine is already initialized somehow, only the reference
counter will be incremented then decremented)

Reviewed-by: Stephen Henson <steve@openssl.org>
2016-09-28 21:45:17 +02:00
Rich Salz
56e36bdaef Revert "Call ENGINE_init() before trying to use keys from engine"
This reverts commit 0a72002993.
This fails to call ENGINE_finish; an alternate fix is forthcoming.

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2016-09-28 14:40:25 -04:00
David Woodhouse
0a72002993 Call ENGINE_init() before trying to use keys from engine
When I said before that s_client "used to work in 1.0.2" that was only
partly true. It worked for engines which provided a default generic
method for some key type, because it called ENGINE_set_default() and
that ended up being an implicit initialisation and functional refcount.

But an engine which doesn't provide generic methods doesn't get initialised,
and then when you try to use it you get an error:

cannot load client certificate private key file from engine
140688147056384:error:26096075:engine routines:ENGINE_load_private_key:not initialised:crypto/engine/eng_pkey.c:66:
unable to load client certificate private key file

cf. https://github.com/OpenSC/libp11/issues/107 (in which we discover
that engine_pkcs11 *used* to provide generic methods that OpenSSL would
try to use for ephemeral DH keys when negotiating ECDHE cipher suites in
TLS, and that didn't work out very well.)

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1639)
2016-09-28 12:15:17 -04:00
David Woodhouse
a6972f3462 Restore '-keyform engine' support for s_client
This used to work in 1.0.2 but disappeared when the argument parsing was
revamped.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1639)
2016-09-28 12:15:17 -04:00
Matt Caswell
f9b1b6644a Add DTLS renegotiation tests
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-28 09:15:07 +01:00
Matt Caswell
fe7dd55341 Extend the renegotiation tests
Add the ability to test both server initiated and client initiated reneg.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-28 09:15:07 +01:00
Matt Caswell
1329b952a6 Update README.ssltest.md
Add update for testing renegotiation. Also change info on CTLOG_FILE
environment variable - which always seems to be required.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-28 09:15:07 +01:00
Matt Caswell
e42c4544c8 Add support for testing renegotiation
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-28 09:15:07 +01:00
Matt Caswell
2f97192c78 Fix a bug in Renegotiation extension construction
The conversion to WPACKET broke the construction of the renegotiation
extension.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-28 09:15:07 +01:00
Matt Caswell
0086ca4e9b Convert HelloRequest construction to WPACKET
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-28 09:15:07 +01:00
David Benjamin
243ecf19dd Add missing parameter.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-09-26 23:10:29 +01:00
David Benjamin
f3ea8d7708 Switch back to assuming TLS 1.2.
The TLSProxy::Record->new call hard-codes a version, like
70-test_sslrecords.t.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-09-26 23:10:29 +01:00
David Benjamin
3058b74266 Address review comments.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-09-26 23:10:29 +01:00
David Benjamin
5cf6d7c51f Don't test quite so many of them.
Avoid making the CI blow up.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-09-26 23:10:29 +01:00
David Benjamin
8523288e6d Test CBC mode padding.
This is a regression test for
https://github.com/openssl/openssl/pull/1431. It tests a
maximally-padded record with each possible invalid offset.

This required fixing a bug in Message.pm where the client sending a
fatal alert followed by close_notify was still treated as success.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-09-26 23:10:29 +01:00
Matt Caswell
fa454945cf Fix some mem leaks in sslapitest
A mem leak could occur on an error path. Also the mempacket BIO_METHOD
needs to be cleaned up, because of the newly added DTLS test.

Also fixed a double semi-colon in ssltestlib.c

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-26 17:26:50 +01:00
David Benjamin
8ff70f3326 Add a basic test for BN_bn2dec.
This would have caught 099e2968ed. This is
a port of the test added in
https://boringssl.googlesource.com/boringssl/+/7c040756178e14a4d181b6d93abb3827c93189c4

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1496)
2016-09-26 11:03:37 -04:00
Matt Caswell
98c1f5b429 Fix HelloVerifyRequest construction
commit c536b6be1a introduced a bug that causes a reachable assert. This fixes
it.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-26 14:40:33 +01:00
Matt Caswell
3133c2d306 Updates CHANGES and NEWS for new release
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-09-26 10:24:37 +01:00
Robert Swiecki
44f206aa9d Add to fuzz corpora for CVE-2016-6309
Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-09-26 08:52:48 +01:00
Matt Caswell
0d698f6696 Fix Use After Free for large message sizes
The buffer to receive messages is initialised to 16k. If a message is
received that is larger than that then the buffer is "realloc'd". This can
cause the location of the underlying buffer to change. Anything that is
referring to the old location will be referring to free'd data. In the
recent commit c1ef7c97 (master) and 4b390b6c (1.1.0) the point in the code
where the message buffer is grown was changed. However s->init_msg was not
updated to point at the new location.

CVE-2016-6309

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-09-26 08:52:48 +01:00
Matt Caswell
f789b04f40 Fix a WPACKET bug
If we request more bytes to be allocated than double what we have already
written, then we grow the buffer by the wrong amount.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-09-26 08:52:48 +01:00
Matt Caswell
84d5549e69 Add a test for large messages
Ensure that we send a large message during the test suite.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-09-26 08:52:48 +01:00
Matt Caswell
c536b6be1a Convert HelloVerifyRequest construction to WPACKET
We actually construct a HelloVerifyRequest in two places with common code
pulled into a single function. This one commit handles both places.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 23:12:38 +01:00
Matt Caswell
4b0fc9fc7a Add warning about a potential pitfall with WPACKET_allocate_bytes()
If the underlying BUF_MEM gets realloc'd then the pointer returned could
become invalid. Therefore we should always ensure that the allocated
memory is filled in prior to any more WPACKET_* calls.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 23:12:38 +01:00
Rich Salz
f3b3d7f003 Add -Wswitch-enum
Change code so when switching on an enumeration, have case's for all
enumeration values.

Reviewed-by: Andy Polyakov <appro@openssl.org>
2016-09-22 08:36:26 -04:00
Matt Caswell
39c136cc53 Updates CHANGES and NEWS for new release
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-09-22 09:27:45 +01:00
Dmitry Belyavsky
41b4280772 Avoid KCI attack for GOST
Russian GOST ciphersuites are vulnerable to the KCI attack because they use
long-term keys to establish the connection when ssl client authorization is
on. This change brings the GOST implementation into line with the latest
specs in order to avoid the attack. It should not break backwards
compatibility.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-09-22 09:27:45 +01:00
Matt Caswell
b8d2439562 Fix a hang with SSL_peek()
If while calling SSL_peek() we read an empty record then we go into an
infinite loop, continually trying to read data from the empty record and
never making any progress. This could be exploited by a malicious peer in
a Denial Of Service attack.

CVE-2016-6305

GitHub Issue #1563

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 09:27:45 +01:00
Matt Caswell
c31dbed70c Fix a mem leak in NPN handling
If a server sent multiple NPN extensions in a single ClientHello then a
mem leak can occur. This will only happen where the client has requested
NPN in the first place. It does not occur during renegotiation. Therefore
the maximum that could be leaked in a single connection with a malicious
server is 64k (the maximum size of the ServerHello extensions section). As
this is client side, only occurs if NPN has been requested and does not
occur during renegotiation this is unlikely to be exploitable.

Issue reported by Shi Lei.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 09:27:45 +01:00
Matt Caswell
ba881d3b39 Add some more OCSP testing
Test that the OCSP callbacks work as expected.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 09:27:45 +01:00
Matt Caswell
a671b3e64a Add OCSP_RESPID_match()
Add a function for testing whether a given OCSP_RESPID matches with a
certificate.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 09:27:45 +01:00
Matt Caswell
e12c0beb5a Add the ability to set OCSP_RESPID fields
OCSP_RESPID was made opaque in 1.1.0, but no accessors were provided for
setting the name/key value for the OCSP_RESPID.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 09:27:45 +01:00
Matt Caswell
e408c09bbf Fix OCSP Status Request extension unbounded memory growth
A malicious client can send an excessively large OCSP Status Request
extension. If that client continually requests renegotiation,
sending a large OCSP Status Request extension each time, then there will
be unbounded memory growth on the server. This will eventually lead to a
Denial Of Service attack through memory exhaustion. Servers with a
default configuration are vulnerable even if they do not support OCSP.
Builds using the "no-ocsp" build time option are not affected.

I have also checked other extensions to see if they suffer from a similar
problem but I could not find any other issues.

CVE-2016-6304

Issue reported by Shi Lei.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 09:27:45 +01:00