Matt Caswell [Thu, 30 Apr 2015 14:20:25 +0000 (15:20 +0100)]
Tighten extension handling
This adds additional checks to the processing of extensions in a ClientHello
to ensure that either no extensions are present, or if they are then they
take up the exact amount of space expected.
With thanks to the Open Crypto Audit Project for reporting this issue.
Matt Caswell [Thu, 30 Apr 2015 13:51:10 +0000 (14:51 +0100)]
Fix memory leaks in BIO_dup_chain()
This fixes a memory leak that can occur whilst duplicating a BIO chain if
the call to CRYPTO_dup_ex_data() fails. It also fixes a second memory leak
where if a failure occurs after successfully creating the first BIO in the
chain, then the beginning of the new chain was not freed.
With thanks to the Open Crypto Audit Project for reporting this issue.
Matt Caswell [Thu, 30 Apr 2015 13:04:30 +0000 (14:04 +0100)]
Replace memset with OPENSSL_clear_free()
BUF_MEM_free() attempts to cleanse memory using memset immediately prior
to a free. This is at risk of being optimised away by the compiler, so
replace with a call to OPENSSL_clear_free() instead.
With thanks to the Open Crypto Audit Project for reporting this issue.
Richard Levitte [Tue, 9 Jun 2015 21:06:23 +0000 (23:06 +0200)]
When making libcrypto from apps or test, make sure to include engines
For librypto to be complete, the stuff in both crypto/ and engines/
have to be built. Doing 'make test' or 'make apps' from a clean
source tree failed to do so.
Corrected by using the new 'build_libcrypto' in the top Makefile.
Richard Levitte [Tue, 9 Jun 2015 23:34:26 +0000 (01:34 +0200)]
Add and rearrange building of libraries
There's a need for a target that will build all of libcrypto, so let's
add 'build_libcrypto' that does this. For ortogonality, let's also
add 'build_libssl'. Have both also depend on 'libcrypto.pc' and
'libssl.pc' so those get built together with the libraries.
This makes 'all' depend on fewer things directly.
Check return value when calling ASN1_INTEGER_get to retrieve a certificate
serial number. If an error occurs (which will be caused by the value being
out of range) revert to hex dump of serial number.
Matt Caswell [Tue, 19 May 2015 12:59:47 +0000 (13:59 +0100)]
Fix off-by-one error in BN_bn2hex
A BIGNUM can have the value of -0. The function BN_bn2hex fails to account
for this and can allocate a buffer one byte too short in the event of -0
being used, leading to a one byte buffer overrun. All usage within the
OpenSSL library is considered safe. Any security risk is considered
negligible.
With thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and
Filip Palian for discovering and reporting this issue.
Matt Caswell [Wed, 6 May 2015 10:40:06 +0000 (11:40 +0100)]
Fix DTLS session resumption
The session object on the client side is initially created during
construction of the ClientHello. If the client is DTLS1.2 capable then it
will store 1.2 as the version for the session. However if the server is only
DTLS1.0 capable then when the ServerHello comes back the client switches to
using DTLS1.0 from then on. However the session version does not get
updated. Therefore when the client attempts to resume that session the
server throws an alert because of an incorrect protocol version.
Sergey Agievich [Mon, 1 Jun 2015 11:48:27 +0000 (12:48 +0100)]
Add funtions to set item_sign and item_verify
PR#3872
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Stephen Henson <steve@openssl.org>
(cherry picked from commit ad0fb7f4988c8a717fe6bcb035304385fbdaef41)
Rich Salz [Fri, 15 May 2015 17:50:38 +0000 (13:50 -0400)]
Standardize handling of #ifdef'd options.
Here are the "rules" for handling flags that depend on #ifdef:
- Do not ifdef the enum. Only ifdef the OPTIONS table. All ifdef'd
entries appear at the end; by convention "engine" is last. This
ensures that at run-time, the flag will never be recognized/allowed.
The next two bullets entries are for silencing compiler warnings:
- In the while/switch parsing statement, use #ifdef for the body to
disable it; leave the "case OPT_xxx:" and "break" statements outside
the ifdef/ifndef. See ciphers.c for example.
- If there are multiple options controlled by a single guard, OPT_FOO,
OPT_BAR, etc., put a an #ifdef around the set, and then do "#else"
and a series of case labels and a break. See OPENSSL_NO_AES in cms.c
for example.
Matt Caswell [Mon, 18 May 2015 15:27:48 +0000 (16:27 +0100)]
Fix race condition in NewSessionTicket
If a NewSessionTicket is received by a multi-threaded client when
attempting to reuse a previous ticket then a race condition can occur
potentially leading to a double free of the ticket data.
CVE-2015-1791
This also fixes RT#3808 where a session ID is changed for a session already
in the client session cache. Since the session ID is the key to the cache
this breaks the cache access.
Matt Caswell [Fri, 29 May 2015 16:05:01 +0000 (17:05 +0100)]
Check the message type requested is the type received in DTLS
dtls1_get_message has an |mt| variable which is the type of the message that
is being requested. If it is negative then any message type is allowed.
However the value of |mt| is not checked in one of the main code paths, so a
peer can send a message of a completely different type and it will be
processed as if it was the message type that we were expecting. This has
very little practical consequences because the current behaviour will still
fail when the format of the message isn't as expected.
Richard Levitte [Thu, 28 May 2015 19:48:17 +0000 (21:48 +0200)]
Remove OPENSSL_CONF=/dev/null from tests
Almost two months ago, the warning about non-existing config file was
supressed by setting the environment variable OPENSSL_CONF to /dev/null
everywhere. Now that this warning is gone, that practice is no longer
needed.
Reviewed-by: Stephen Henson <steve@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
Rich Salz [Thu, 28 May 2015 17:52:55 +0000 (13:52 -0400)]
RT3876: Only load config when needed
Create app_load_config(), a routine to load config file. Remove the
"always load config" from the main app. Change the places that used to
load config to call the new common routine.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Thu, 28 May 2015 15:36:51 +0000 (16:36 +0100)]
Change return type of the new accessors
The new accessors SSL_get_client_random, SSL_get_server_random and
SSL_SESSION_get_master_key should return a size_t to match the type of the
|outlen| parameter.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Nick Mathewson [Mon, 25 May 2015 21:12:49 +0000 (17:12 -0400)]
Add new functions to extract {client,server}_random, master_key
Tor uses these values to implement a low-rent clone of RFC 5705 (which,
in our defense, we came up with before RFC 5705 existed). But now that
ssl_st is opaque, we need another way to get at them.
Includes documentation, with suitable warnings about not actually
using these functions.
Signed-off-by: Nick Mathewson <nickm@torproject.org> Signed-off-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org>
Nick Mathewson [Tue, 26 May 2015 01:16:53 +0000 (21:16 -0400)]
Add SSL_get_client_ciphers() to return ciphers from ClientHello
On the server side, if you want to know which ciphers the client
offered, you had to use session->ciphers. But that field is no
longer visible, so we need a method to get at it.
Signed-off-by: Nick Mathewson <nickm@torproject.org> Signed-off-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org>
Matt Caswell [Mon, 25 May 2015 23:02:57 +0000 (00:02 +0100)]
Fix error check in GOST engine
The return value of i2d functions can be negative if an error occurs.
Therefore don't assign the return value to an unsigned type and *then*
check if it is negative.
RT#3862
Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Sat, 23 May 2015 20:32:12 +0000 (21:32 +0100)]
Don't send an alert if we've just received one
If the record received is for a version that we don't support, previously we
were sending an alert back. However if the incoming record already looks
like an alert then probably we shouldn't do that. So suppress an outgoing
alert if it looks like we've got one incoming.
Matt Caswell [Sat, 23 May 2015 20:05:19 +0000 (21:05 +0100)]
Set first_packet for TLS clients
Version negotiation was broken (one of the late changes in the review
process broke it). The problem is that TLS clients do not set first_packet,
whereas TLS/DTLS servers and DTLS clients do. The simple fix is to set
first_packet for TLS clients too.
Andy Polyakov [Wed, 20 May 2015 07:36:48 +0000 (09:36 +0200)]
bn/asm/x86_64-mont5.pl: fix valgrind error.
bn_get_bits5 was overstepping array boundary by 1 byte. It was exclusively
read overstep and data could not have been used. The only potential problem
would be if array happens to end on the very edge of last accesible page.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Fri, 22 May 2015 12:48:49 +0000 (13:48 +0100)]
Fix typo setting up certificate masks
The certificate masks are used to select which ciphersuite we are going to
use. The variables |emask_k| and |emask_a| relate to export grade key
exchange and authentication respecitively. The variables |mask_k| and
|mask_a| are the equivalent versions for non-export grade. This fixes an
instance where the two usages of export/non-export were mixed up. In
practice it makes little difference since it still works!
Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Fri, 22 May 2015 12:33:19 +0000 (13:33 +0100)]
Remove export static DH ciphersuites
Remove support for the two export grade static DH ciphersuites. These two
ciphersuites were newly added (along with a number of other static DH
ciphersuites) to 1.0.2. However the two export ones have *never* worked
since they were introduced. It seems strange in any case to be adding new
export ciphersuites, and given "logjam" it also does not seem correct to
fix them.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Tue, 19 May 2015 15:03:02 +0000 (16:03 +0100)]
Fix off-by-one in BN_rand
If BN_rand is called with |bits| set to 1 and |top| set to 1 then a 1 byte
buffer overflow can occur. There are no such instances within the OpenSSL at
the moment.
Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke, Filip Palian for
discovering and reporting this issue.
Matt Caswell [Tue, 19 May 2015 14:19:30 +0000 (15:19 +0100)]
Reject negative shifts for BN_rshift and BN_lshift
The functions BN_rshift and BN_lshift shift their arguments to the right or
left by a specified number of bits. Unpredicatable results (including
crashes) can occur if a negative number is supplied for the shift value.
Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and Filip Palian
for discovering and reporting this issue.
Matt Caswell [Fri, 15 May 2015 09:49:56 +0000 (10:49 +0100)]
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>
Richard Levitte [Fri, 22 May 2015 15:54:06 +0000 (17:54 +0200)]
Fix the update target and remove duplicate file updates
We had updates of certain header files in both Makefile.org and the
Makefile in the directory the header file lived in. This is error
prone and also sometimes generates slightly different results (usually
just a comment that differs) depending on which way the update was
done.
This removes the file update targets from the top level Makefile, adds
an update: target in all Makefiles and has it depend on the depend: or
local_depend: targets, whichever is appropriate, so we don't get a
double run through the whole file tree.
Matt Caswell [Fri, 22 May 2015 08:28:43 +0000 (09:28 +0100)]
Ignore files from other branches
There are a number of files that are created on other branches that are
not held in git and are not needed in master. When checking out master
after working on another branch these files show up in "git status", so
just add them to .gitignore
Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Thu, 21 May 2015 13:06:52 +0000 (14:06 +0100)]
Fix a memory leak in compression
The function RECORD_LAYER_clear() is supposed to clear the contents of the
RECORD_LAYER structure, but retain certain data such as buffers that are
allocated. Unfortunately one buffer (for compression) got missed and was
inadvertently being wiped, thus causing a memory leak.
In part this is due to the fact that RECORD_LAYER_clear() was reaching
inside SSL3_BUFFERs and SSL3_RECORDs, which it really shouldn't. So, I've
rewritten it to only clear the data it knows about, and to defer clearing
of SSL3_RECORD and SSL3_BUFFER structures to SSL_RECORD_clear() and the
new function SSL3_BUFFER_clear().
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
This adds support for the ASN.1 structures in draft-josefsson-scrypt-kdf-03
Private keys encrypted by scrypt can now be decrypted transparently as long
as they don't exceed the memory limits.
Rich Salz [Fri, 15 May 2015 20:32:49 +0000 (16:32 -0400)]
Replace switch/assignments with table lookup
Typedef STRINT_PAIR to be the same as OPT_PAIR, and use that structure and
a bunch of tables instead of switch statements to lookup various values
out of the SSL/TLS message buffers. Shrinks a bunch of code.
Reviewed-by: Richard Levitte <levitte@openssl.org>