Commit graph

102 commits

Author SHA1 Message Date
Matt Caswell
fb790f1673 Add WPACKET_sub_memcpy() function
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-13 09:41:21 +01:00
Matt Caswell
0217dd19c0 Move from explicit sub-packets to implicit ones
No need to declare an explicit sub-packet. Just start one.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-13 09:41:21 +01:00
Matt Caswell
ae2f7b37da Rename PACKETW to WPACKET
To avoid confusion with the read PACKET structure.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-13 09:41:21 +01:00
Matt Caswell
2c7b4dbc1a Convert tls_construct_client_hello() to use PACKETW
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-13 09:41:21 +01:00
Matt Caswell
2f3930bc0e Fix leak on error in tls_construct_cke_gost
Don't leak pke_ctx on error.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2016-08-23 00:19:15 +01:00
Emilia Kasper
a230b26e09 Indent ssl/
Run util/openssl-format-source on ssl/

Some comments and hand-formatted tables were fixed up
manually by disabling auto-formatting.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-08-18 14:02:29 +02:00
Dr. Stephen Henson
0a699a0723 Fix no-ec
Fix no-ec builds by having separate functions to create keys based on
an existing EVP_PKEY and a curve id.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-08-15 14:07:33 +01:00
Dr. Stephen Henson
ec24630ae2 Modify TLS support for new X25519 API.
When handling ECDH check to see if the curve is "custom" (X25519 is
currently the only curve of this type) and instead of setting a curve
NID just allocate a key of appropriate type.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-08-13 14:11:05 +01:00
David Woodhouse
e6027420b7 Fix ossl_statem_client_max_message_size() for DTLS1_BAD_VER
The Change Cipher Spec message in this ancient pre-standard version of DTLS
that Cisco are unfortunately still using in their products, is 3 bytes.

Allow it.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-08-04 20:56:23 +01:00
Ben Laurie
3260adf190 peer_tmp doesn't exist if no-ec no-dh.
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-08-01 11:30:33 +01:00
Richard Levitte
8b9546c708 Correct misspelt OPENSSL_NO_SRP
RT#4619

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-07-23 10:47:52 +02:00
Matt Caswell
4fa88861ee Update error codes following tls_process_key_exchange() refactor
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:18:46 +01:00
Matt Caswell
e1e588acae Tidy up tls_process_key_exchange()
After the refactor of tls_process_key_exchange(), this commit tidies up
some loose ends.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:18:46 +01:00
Matt Caswell
ff74aeb1fa Split out ECDHE from tls_process_key_exchange()
Continuing from the previous commit. Refactor tls_process_key_exchange() to
split out into a separate function the ECDHE aspects.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:18:46 +01:00
Matt Caswell
e01a610db8 Split out DHE from tls_process_key_exchange()
Continuing from the previous commit. Refactor tls_process_key_exchange() to
split out into a separate function the DHE aspects.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:18:46 +01:00
Matt Caswell
25c6c10cd7 Split out SRP from tls_process_key_exchange()
Continuing from the previous commit. Refactor tls_process_key_exchange() to
split out into a separate function the SRP aspects.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:18:46 +01:00
Matt Caswell
7dc1c64774 Split out the PSK preamble from tls_process_key_exchange()
The tls_process_key_exchange() function is too long. This commit starts
the process of splitting it up by moving the PSK preamble code to a
separate function.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:18:46 +01:00
Matt Caswell
02a74590bb Move the PSK preamble for tls_process_key_exchange()
The function tls_process_key_exchange() is too long. This commit moves
the PSK preamble processing out to a separate function.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:18:46 +01:00
Matt Caswell
be8dba2c92 Narrow scope of locals vars in tls_process_key_exchange()
Narrow the scope of the local vars in preparation for split up this
function.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:18:46 +01:00
Matt Caswell
e4612d02c5 Remove sessions from external cache, even if internal cache not used.
If the SSL_SESS_CACHE_NO_INTERNAL_STORE cache mode is used then we weren't
removing sessions from the external cache, e.g. if an alert occurs the
session is supposed to be automatically removed.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-19 12:08:49 +01:00
Richard Levitte
340a282853 Fixup a few SSLerr calls in ssl/statem/
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-07-19 11:50:31 +02:00
Matt Caswell
05ec6a25f8 Fix up error codes after splitting up tls_construct_key_exchange()
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-18 23:05:15 +01:00
Matt Caswell
a7a752285a Some tidy ups after the CKE construction refactor
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-18 23:05:15 +01:00
Matt Caswell
840a2bf8ec Split out SRP CKE construction into a separate function
Continuing previous commit to break up the
tls_construct_client_key_exchange() function. This splits out the SRP
code.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-18 23:05:14 +01:00
Matt Caswell
e00e0b3d84 Split out GOST CKE construction into a separate function
Continuing previous commit to break up the
tls_construct_client_key_exchange() function. This splits out the GOST
code.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-18 23:05:14 +01:00
Matt Caswell
67ad5aabe6 Split out DHE CKE construction into a separate function
Continuing previous commit to break up the
tls_construct_client_key_exchange() function. This splits out the ECDHE
code.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-18 23:05:14 +01:00
Matt Caswell
a8c1c7040a Split out DHE CKE construction into a separate function
Continuing previous commit to break up the
tls_construct_client_key_exchange() function. This splits out the DHE
code.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-18 23:05:14 +01:00
Matt Caswell
13c0ec4ad4 Split out CKE construction PSK pre-amble and RSA into a separate function
The tls_construct_client_key_exchange() function is too long. This splits
out the construction of the PSK pre-amble into a separate function as well
as the RSA construction.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-18 23:05:14 +01:00
Matt Caswell
0bce0b02d8 Narrow the scope of local variables in tls_construct_client_key_exchange()
This is in preparation for splitting up this over long function.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-18 23:05:14 +01:00
Matt Caswell
7d2c13a705 Simplify key_exchange_expected() logic
The static function key_exchange_expected() used to return -1 on error.
Commit 361a119127 changed that so that it can never fail. This means that
some tidy up can be done to simplify error handling in callers of that
function.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-07-18 14:30:14 +01:00
Matt Caswell
672f3337c3 Fix SSLv3 alert if no Client Ceritifcate sent after a request for one
In TLS if the server sends a CertificateRequest and the client does not
provide one, if the server cannot continue it should send a
HandshakeFailure alert. In SSLv3 the same should happen, but instead we
were sending an UnexpectedMessage alert. This is incorrect - the message
isn't unexpected - it is valid for the client not to send one - its just
that we cannot continue without one.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-07-18 14:30:14 +01:00
Matt Caswell
05c4f1d563 Prepare the client certificate earlier
Move the preparation of the client certificate to be post processing work
after reading the CertificateRequest message rather than pre processing
work prior to writing the Certificate message. As part of preparing the
client certificate we may discover that we do not have one available. If
we are also talking SSLv3 then we won't send the Certificate message at
all. However, if we don't discover this until we are about to send the
Certificate message it is too late and we send an empty one anyway. This
is wrong for SSLv3.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-07-18 14:30:14 +01:00
Dr. Stephen Henson
d166ed8c11 check return values for EVP_Digest*() APIs
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-07-15 14:09:05 +01:00
David Benjamin
e99ab8ffd7 Fix DH error-handling in tls_process_key_exchange.
The set0 setters take ownership of their arguments, so the values should
be set to NULL to avoid a double-free in the cleanup block should
ssl_security(SSL_SECOP_TMP_DH) fail. Found by BoringSSL's WeakDH test.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1299)
2016-07-12 15:39:42 -04:00
Richard Levitte
2ac6115d9e Deal with the consequences of constifying getters
Reviewed-by: Stephen Henson <steve@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-06-15 20:09:27 +02:00
Laszlo Kovacs
4f6eaa592a RT3720 Increment session miss counter properly
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-06-09 23:34:18 +01:00
Matt Caswell
2c4a056f59 Handle a memory allocation failure in ssl3_init_finished_mac()
The ssl3_init_finished_mac() function can fail, in which case we need to
propagate the error up through the stack.

RT#3198

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-06-03 20:29:04 +01:00
Matt Caswell
464175692f Simplify SSL BIO buffering logic
The write BIO for handshake messages is bufferred so that we only write
out to the network when we have a complete flight. There was some
complexity in the buffering logic so that we switched buffering on and
off at various points through out the handshake. The only real reason to
do this was historically it complicated the state machine when you wanted
to flush because you had to traverse through the "flush" state (in order
to cope with NBIO). Where we knew up front that there was only going to
be one message in the flight we switched off buffering to avoid that.

In the new state machine there is no longer a need for a flush state so
it is simpler just to have buffering on for the whole handshake. This
also gives us the added benefit that we can simply call flush after every
flight even if it only has one message in it. This means that BIO authors
can implement their own buffering strategies and not have to be aware of
the state of the SSL object (previously they would have to switch off
their own buffering during the handshake because they could not rely on
a flush being received when they really needed to write data out). This
last point addresses GitHub Issue #322.

Reviewed-by: Andy Polyakov <appro@openssl.org>
2016-05-20 14:11:11 +01:00
Rich Salz
846e33c729 Copyright consolidation 01/10
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2016-05-17 14:19:19 -04:00
Matt Caswell
d4d7894379 Fix some out of date comments
Fix various references to s3_clnt.c and s3_srvr.c which don't exist
any more.

GitHub Issue #765

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-05-17 14:34:30 +01:00
David Benjamin
c45d6b2b0d The NewSessionTicket message is not optional.
Per RFC 4507, section 3.3:

   This message [NewSessionTicket] MUST be sent if the
   server included a SessionTicket extension in the ServerHello.  This
   message MUST NOT be sent if the server did not include a
   SessionTicket extension in the ServerHello.

The presence of the NewSessionTicket message should be determined
entirely from the ServerHello without probing.

RT#4389

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-05-13 13:04:46 +01:00
Matt Caswell
6f137370dd Client side CKE processing can double free on error
The tls_client_key_exchange_post_work() frees the pms on error. It also
calls ssl_generate_master_secret() which also free the pms. If an error
occurs after ssl_generate_master_secret() has been called then a double
free can occur.

Reviewed-by: Andy Polyakov <appro@openssl.org>
2016-04-29 09:17:51 +01:00
FdaSilvaYY
8483a003bf various spelling fixes
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/952)
2016-04-28 14:22:26 -04:00
Matt Caswell
ee85fc1dd6 Don't set peer_tmp until we have finished constructing it
If we fail halfway through constructing the peer_tmp EVP_PKEY but we have
already stored it in s->s3->peer_tmp then if anything tries to use it then
it will likely fail. This was causing s_client to core dump in the
sslskewith0p test. s_client was trying to print out the connection
parameters that it had negotiated so far. Arguably s_client should not do
that if the connection has failed...but given it is existing functionality
it's easier to fix libssl.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
2016-04-22 15:37:17 +01:00
Matt Caswell
0aeddcfa61 Make DH opaque
Move the dh_st structure into an internal header file and provide
relevant accessors for the internal fields.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-04-09 10:10:55 +01:00
Viktor Dukhovni
43341433a8 Suppress CT callback as appropriate
Suppress CT callbacks with aNULL or PSK ciphersuites that involve
no certificates.  Ditto when the certificate chain is validated via
DANE-TA(2) or DANE-EE(3) TLSA records.  Also skip SCT processing
when the chain is fails verification.

Move and consolidate CT callbacks from libcrypto to libssl.  We
also simplify the interface to SSL_{,CTX_}_enable_ct() which can
specify either a permissive mode that just collects information or
a strict mode that requires at least one valid SCT or else asks to
abort the connection.

Simplified SCT processing and options in s_client(1) which now has
just a simple pair of "-noct" vs. "-ct" options, the latter enables
the permissive callback so that we can complete the handshake and
report all relevant information.  When printing SCTs, print the
validation status if set and not valid.

Signed-off-by: Rob Percival <robpercival@google.com>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-04-07 14:41:34 -04:00
Viktor Dukhovni
c636c1c470 Fix client verify mode to check SSL_VERIFY_PEER
The original check for != SSL_VERIFY_NONE can give surprising results
when flags SSL_VERIFY_PEER is not set, but other flags are.  Note
that SSL_VERIFY_NONE (0) is not a flag bit, it is rather the absense
of all other flag bits.

Signed-off-by: Rob Percival <robpercival@google.com>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
2016-04-07 14:41:34 -04:00
David Benjamin
6afef8b1fb Fix memory leak on invalid CertificateRequest.
Free up parsed X509_NAME structure if the CertificateRequest message
contains excess data.

The security impact is considered insignificant. This is a client side
only leak and a large number of connections to malicious servers would
be needed to have a significant impact.

This was found by libFuzzer.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Stephen Henson <steve@openssl.org>
2016-04-07 19:22:20 +01:00
Rich Salz
e771eea6d8 Revert "various spelling fixes"
This reverts commit 620d540bd4.
It wasn't reviewed.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-04-04 16:11:43 -04:00
FdaSilvaYY
620d540bd4 various spelling fixes
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-04-04 15:06:32 -04:00