Matt Caswell [Fri, 6 Nov 2015 16:31:21 +0000 (16:31 +0000)]
Ensure all EVP calls have their returns checked where appropriate
There are lots of calls to EVP functions from within libssl There were
various places where we should probably check the return value but don't.
This adds these checks.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Thu, 12 Nov 2015 15:11:34 +0000 (15:11 +0000)]
Check error return from sysconf in secure memory code
We use the sysconf function to provide details about the page size in the
secure memory code. This function can return -1 on error so we should check
for this before proceeding.
Matt Caswell [Wed, 11 Nov 2015 10:17:22 +0000 (10:17 +0000)]
Add comment explaining why we don't check a return value
A call to X509_verify_cert() is used to build a chain of certs for the
server to send back to the client. It isn't *actually* used for verifying
the cert at all - just building the chain. Therefore the return value is
ignored.
Matt Caswell [Tue, 10 Nov 2015 23:12:36 +0000 (23:12 +0000)]
Remove an NULL ptr deref in an error path
The |passwd| variable in the code can be NULL if it goes to the err label.
Therefore we cannot call strlen on it without first checking that it is non
NULL.
Andy Polyakov [Fri, 13 Nov 2015 22:44:23 +0000 (23:44 +0100)]
bn/asm/ppc64-mont.pl: adapt for little-endian.
The problem remained unnoticed so far, because it's never called by default.
You have to craft OPENSSL_ppccap environment variable to trigger the problem.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Andy Polyakov [Tue, 10 Nov 2015 20:11:24 +0000 (21:11 +0100)]
bn/asm/s390x.S: improve performance on z196 and z13 by up to 26%. [even z10 is couple percent faster]. Triggered by RT#4128, but solves the problem by real modulo-scheduling.
Print certificate details using accessor functions.
Since X509_CERT_AUX_print is only used in one place and can't
be used by applications (it uses an internal X509_CERT_AUX structure)
this has been removed and replaced by a function X509_aux_print which
takes an X509 pointer instead.
This adds a TLSv1.0 cipher alias for ciphersuites requiring
at least TLSv1.0: currently only PSK ciphersuites using SHA256
or SHA384 MAC (SSLv3 only supports SHA1 and MD5 MAC).
Matt Caswell [Mon, 9 Nov 2015 14:38:59 +0000 (14:38 +0000)]
Fix SSL_use_certificate_chain_file
The new function SSL_use_certificate_chain_file was always crashing in
the internal function use_certificate_chain_file because it would pass a
NULL value for SSL_CTX *, but use_certificate_chain_file would
unconditionally try to dereference it.
Matt Caswell [Mon, 9 Nov 2015 16:37:33 +0000 (16:37 +0000)]
Remove redundant check from tls1_get_curvelist
The function tls1_get_curvelist() has an explicit check to see if s->cert
is NULL or not. However the check appears *after* calling the tls1_suiteb
macro which derefs s->cert. In reality s->cert can never be NULL because
it is created in SSL_new(). If the malloc fails then the SSL_new call fails
and no SSL object is created.
Matt Caswell [Fri, 30 Oct 2015 10:05:53 +0000 (10:05 +0000)]
Standardise our style for checking malloc failures
if we have a malloc |x = OPENSSL_malloc(...)| sometimes we check |x|
for NULL and sometimes we treat it as a boolean |if(!x) ...|. Standardise
the approach in libssl.
Matt Caswell [Mon, 9 Nov 2015 15:31:27 +0000 (15:31 +0000)]
Remove redundant check from SSL_shutdown
The SSL object was being deref'd and then there was a later redundant check
to see if it is NULL. We assume all SSL_foo functions pass a non NULL SSL
object and do not check it.
Richard Levitte [Mon, 9 Nov 2015 08:50:56 +0000 (09:50 +0100)]
Make the match for previous cflags a bit more strict
./Configure [target] --strict-warnings -Wno-pedantic-ms-format
would not add '-pedantic' because it matches '-Wno-pedantic-ms-format',
which was added first.
Matt Caswell [Fri, 6 Nov 2015 09:47:18 +0000 (09:47 +0000)]
Don't finish the handshake twice
We finish the handshake when we move into the TLS_ST_OK state. At various
points we were also unnecessarily finishing it when we were reading/writing
the Finished message. It's much simpler just to do it in TLS_ST_OK, so
remove the other calls.
Richard Levitte [Mon, 2 Nov 2015 15:48:53 +0000 (16:48 +0100)]
Remove the state parameter from BIO_ctrl_set_connected
The actual implementation has the state of the connection being
controlled with the peer parameter, non-NULL meaning connected and
NULL meaning connected.
Matt Caswell [Fri, 30 Oct 2015 17:01:01 +0000 (17:01 +0000)]
Remove a reachable assert from ssl3_write_bytes
A buggy application that call SSL_write with a different length after a
NBIO event could cause an OPENSSL_assert to be reached. The assert is not
actually necessary because there was an explicit check a little further
down that would catch this scenario. Therefore remove the assert an move
the check a little higher up.
Matt Caswell [Fri, 30 Oct 2015 16:39:29 +0000 (16:39 +0000)]
Remove a trivially true OPENSSL_assert
This OPENSSL_assert in (d)tls1_hearbeat is trivially always going to be
true because it is testing the sum of values that have been set as
constants just a few lines above and nothing has changed them. Therefore
remove this.
Matt Caswell [Mon, 26 Oct 2015 23:11:44 +0000 (23:11 +0000)]
Add SRP and PSK to disallowed CertificateRequest ciphersuites
There was a discrepancy between what ciphersuites we allowed to send a
CertificateRequest, and what ciphersuites we allowed to receive one. So
add PSK and SRP to the disallowed ones.
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Mon, 26 Oct 2015 14:05:43 +0000 (14:05 +0000)]
Remove superfluous check
|tls_process_finished| was checking that |peer_finish_md_len| was
non-negative. However neither |tls1_final_finish_mac| or
|ssl3_final_finish_mac| can ever return a negative value, so the check is
superfluous.
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Thu, 22 Oct 2015 14:13:20 +0000 (15:13 +0100)]
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>
Matt Caswell [Thu, 22 Oct 2015 14:02:14 +0000 (15:02 +0100)]
Fix empty NewSessionTicket processing
Rebasing the state machine code introduced a problem with empty
NewSessionTicket processing. The return value from the
tls_process_new_session_ticket() is supposed to be an enum, but a bare
integer was being used. Unfortunately this is valid C so the compiler
doesn't pick it up.
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Thu, 22 Oct 2015 11:18:45 +0000 (12:18 +0100)]
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>
Matt Caswell [Mon, 5 Oct 2015 10:17:08 +0000 (11:17 +0100)]
Remove the old state defines
Remove all the defines for the old state machines states. Mapping old to new
is probably going to cause more problems than it solves so it is probably
better to just remove them.
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Mon, 5 Oct 2015 09:21:11 +0000 (10:21 +0100)]
Don't depend on SSL structure internals
The macros SSL_in_connect_init() and SSL_in_accept_init() inadvertently
depended on SSL structure internals. This fixes it to use public API calls
instead.
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Mon, 14 Sep 2015 14:06:37 +0000 (15:06 +0100)]
Fix some client side transition logic
Fixed some issues in the logic for determining whether an SKE should be
expected or not. In particular only allow an SKE for RSA if its export and
the key size is not allowed. Also fix the ephemeral ciphersuite checks and
add in a missing call to ssl3_check_cert_and_algorithm().
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Fri, 11 Sep 2015 12:11:37 +0000 (13:11 +0100)]
Remove a call to SSL_set_state from s_server
s_server was (ab)using SSL_set_state to force a renegotiation. This is a
bad way to do things and does not work with the new state machine code, so
we need to do it a different way.
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Fri, 11 Sep 2015 10:23:20 +0000 (11:23 +0100)]
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>