The tls_process_client_key_exchange() function is far too long. This
splits out the PSK preamble processing, and the RSA processing into
separate functions.
Reviewed-by: NRichard Levitte <levitte@openssl.org>

In preparation for splitting this function up into smaller functions this
commit reduces the scope of some of the variables to only be in scope for
the algorithm specific parts. In some cases that makes the error handling
more verbose than it needs to be - but we'll clean that up in a later
commit.
Reviewed-by: NRichard Levitte <levitte@openssl.org>

Also elaborated a comment based on feedback.
Reviewed-by: NEmilia Käsper <emilia@openssl.org>

The logic testing whether a CKE message is allowed or not was a little
difficult to follow. This tries to clean it up.
Reviewed-by: NEmilia Käsper <emilia@openssl.org>

The static function key_exchange_expected() used to return -1 on error.
Commit 361a1191 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: NEmilia Käsper <emilia@openssl.org>

Having received a ClientKeyExchange message instead of a Certificate we
know that we are not going to receive a CertificateVerify message. This
means we can free up the handshake_buffer. However we better call
ssl3_digest_cached_records() instead of just freeing it up, otherwise we
later try and use it anyway and a core dump results. This could happen,
for example, in SSLv3 where we send a CertificateRequest but the client
sends no Certificate message at all. This is valid in SSLv3 (in TLS
clients are required to send an empty Certificate message).

Found using the BoringSSL test suite.
Reviewed-by: NEmilia Käsper <emilia@openssl.org>

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: NEmilia Käsper <emilia@openssl.org>

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: NEmilia Käsper <emilia@openssl.org>

Reviewed-by: NKurt Roeckx <kurt@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1313)

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

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: NKurt Roeckx <kurt@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1299)

In light of potential UKS (unknown key share) attacks on some
applications, primarily browsers, despite RFC761, name checks are
by default applied with DANE-EE(3) TLSA records.  Applications for
which UKS is not a problem can optionally disable DANE-EE(3) name
checks via the new SSL_CTX_dane_set_flags() and friends.
Reviewed-by: NRich Salz <rsalz@openssl.org>

Commit aea145e3 removed some error codes that are generated
algorithmically: mapping alerts to error texts.  Found by
Andreas Karlsson.  This restores them, and adds two missing ones.
Reviewed-by: NMatt Caswell <matt@openssl.org>

The SSL_load_client_CA_file() failed to load any CAs due to an
inccorrect assumption about the return value of lh_*_insert(). The
return value when inserting into a hash is the old value of the key.

The bug was introduced in 3c82e437.
Reviewed-by: NKurt Roeckx <kurt@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1279)

We calculate the size required for the ServerKeyExchange message and then
call BUF_MEM_grow_clean() on the buffer. However we fail to take account of
2 bytes required for the signature algorithm and 2 bytes for the signature
length, i.e. we could overflow by 4 bytes. In reality this won't happen
because the buffer is pre-allocated to a large size that means it should be
big enough anyway.

Addresses an OCAP Audit issue.
Reviewed-by: NRich Salz <rsalz@openssl.org>

Reviewed-by: NAndy Polyakov <appro@openssl.org>
Reviewed-by: NKurt Roeckx <kurt@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1264)

Reviewed-by: NAndy Polyakov <appro@openssl.org>
Reviewed-by: NKurt Roeckx <kurt@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1264)

In some situations (such as when we receive a fragment of an alert)
we try to get the next packet but did not mark the current one as read,
meaning that we got the same record back again - leading to an infinite
loop.

Found using the BoringSSL test suite.
Reviewed-by: NAndy Polyakov <appro@openssl.org>

Reviewed-by: NRich Salz <rsalz@openssl.org>
Reviewed-by: NMatt Caswell <matt@openssl.org>

Reviewed-by: NKurt Roeckx <kurt@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1245)

Using RSA_PKCS1_PADDING with RSA_private_decrypt is inherently unsafe.
The API requires writing output on success and touching the error queue
on error. Thus, although the padding check itself is constant-time as of
294d1e36, and the logic after the
decryption in the SSL code is constant-time as of
adb46dbc, the API boundary in the middle
still leaks whether the padding check succeeded, giving us our
much-loved Bleichenbacher padding oracle.

Instead, PKCS#1 padding must be handled by the caller which uses
RSA_NO_PADDING, in timing-sensitive code integrated with the
Bleichenbacher mitigation. Removing PKCS#1 padding in constant time is
actually much simpler when the expected length is a constant (and if
it's not a constant, avoiding a padding oracle seems unlikely), so just
do it inline.
Signed-off-by: NKurt Roeckx <kurt@roeckx.be>
Reviewed-by: NRich Salz <rsalz@openssl.org>

GH: #1222

Reviewed-by: NKurt Roeckx <kurt@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1168)

Reviewed-by: NStephen Henson <steve@openssl.org>
Reviewed-by: NEmilia Käsper <emilia@openssl.org>

ssl_session_hash() always looks at the first 4 bytes, regardless of the length.
A client can send a session id that's shorter, and the callback could also
generate one that's shorter.  So we make sure that the rest of the buffer is
initliazed to 0 so that we always calculate the same hash.

Found by tis-interpreter, also previously reported as RT #2871
Reviewed-by: NRich Salz <rsalz@openssl.org>

MR: #2911

Fix a comment following commit c2c49969.

RT2388
Reviewed-by: NRich Salz <rsalz@openssl.org>

Some misc return value checks
Reviewed-by: NRich Salz <rsalz@openssl.org>

SSL_clear() and SSL_free() will remove a session from the cache if it is
considered "bad". However SSL_set_session() does not do this for the session
it is replacing.

SSL_clear() clears an SSL object ready for reuse. It does not clear the
session though. This means that:

  SSL_clear(s)
  SSL_set_session(s, sess);

and
  SSL_set_session(s, sess);
  SSL_clear(s);

do not do the same thing, although logically you would expect that they
would.

The failure of SSL_set_session() to remove bad sessions from the cache
should be considered a bug, so this commit fixes it.

RT#597
Reviewed-by: NRich Salz <rsalz@openssl.org>

Found by tis-interpreter
Reviewed-by: NRich Salz <rsalz@openssl.org>

GH: #1173

Reviewed-by: NKurt Roeckx <kurt@openssl.org>
Reviewed-by: NMatt Caswell <matt@openssl.org>

When session tickets are used, it's possible that SNI might swtich the
SSL_CTX on an SSL. Normally, this is not a problem, because the
initial_ctx/session_ctx are used for all session ticket/id processes.

However, when the SNI callback occurs, it's possible that the callback
may update the options in the SSL from the SSL_CTX, and this could
cause SSL_OP_NO_TICKET to be set. If this occurs, then two bad things
can happen:

1. The session ticket TLSEXT may not be written when the ticket expected
flag is set. The state machine transistions to writing the ticket, and
the client responds with an error as its not expecting a ticket.
2. When creating the session ticket, if the ticket key cb returns 0
the crypto/hmac contexts are not initialized, and the code crashes when
trying to encrypt the session ticket.

To fix 1, if the ticket TLSEXT is not written out, clear the expected
ticket flag.
To fix 2, consider a return of 0 from the ticket key cb a recoverable
error, and write a 0 length ticket and continue. The client-side code
can explicitly handle this case.

Fix these two cases, and add unit test code to validate ticket behavior.
Reviewed-by: NEmilia Käsper <emilia@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1098)

GH1098: Add X509_get_pathlen() (and a test)
GH1097:  Add SSL_is_dtls() function.

Documented.
Reviewed-by: NMatt Caswell <matt@openssl.org>

Sessions are stored on the session_ctx, which doesn't change after
SSL_set_SSL_CTX().
Reviewed-by: NRich Salz <rsalz@openssl.org>
Reviewed-by: NMatt Caswell <matt@openssl.org>

Previously if we received an empty record we just threw it away and
ignored it. Really though if we get an empty record of a different content
type to what we are expecting then that should be an error, i.e. we should
reject out of context empty records. This commit makes the necessary changes
to achieve that.

RT#4395
Reviewed-by: NAndy Polyakov <appro@openssl.org>

The number of read pipelines should be reset in the event of reuse of an
SSL object.
Reviewed-by: NAndy Polyakov <appro@openssl.org>

Reviewed-by: NRich Salz <rsalz@openssl.org>

Reviewed-by: NRich Salz <rsalz@openssl.org>

The tlsext_status_type field in SSL is used by e.g. OpenResty to determine
if the client requested the certificate status, but SSL is now opaque.
Reviewed-by: NRich Salz <rsalz@openssl.org>
Reviewed-by: NMatt Caswell <matt@openssl.org>

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

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: NRich Salz <rsalz@openssl.org>

Reviewed-by: NMatt Caswell <matt@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1135)