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>
The function tls_construct_cert_status() is called by both TLS and DTLS
code. However it only ever constructed a TLS message header for the message
which obviously failed in DTLS.
Reviewed-by: Rich Salz <rsalz@openssl.org>
DTLS can handle out of order record delivery. Additionally since
handshake messages can be bigger than will fit into a single packet, the
messages can be fragmented across multiple records (as with normal TLS).
That means that the messages can arrive mixed up, and we have to
reassemble them. We keep a queue of buffered messages that are "from the
future", i.e. messages we're not ready to deal with yet but have arrived
early. The messages held there may not be full yet - they could be one
or more fragments that are still in the process of being reassembled.
The code assumes that we will eventually complete the reassembly and
when that occurs the complete message is removed from the queue at the
point that we need to use it.
However, DTLS is also tolerant of packet loss. To get around that DTLS
messages can be retransmitted. If we receive a full (non-fragmented)
message from the peer after previously having received a fragment of
that message, then we ignore the message in the queue and just use the
non-fragmented version. At that point the queued message will never get
removed.
Additionally the peer could send "future" messages that we never get to
in order to complete the handshake. Each message has a sequence number
(starting from 0). We will accept a message fragment for the current
message sequence number, or for any sequence up to 10 into the future.
However if the Finished message has a sequence number of 2, anything
greater than that in the queue is just left there.
So, in those two ways we can end up with "orphaned" data in the queue
that will never get removed - except when the connection is closed. At
that point all the queues are flushed.
An attacker could seek to exploit this by filling up the queues with
lots of large messages that are never going to be used in order to
attempt a DoS by memory exhaustion.
I will assume that we are only concerned with servers here. It does not
seem reasonable to be concerned about a memory exhaustion attack on a
client. They are unlikely to process enough connections for this to be
an issue.
A "long" handshake with many messages might be 5 messages long (in the
incoming direction), e.g. ClientHello, Certificate, ClientKeyExchange,
CertificateVerify, Finished. So this would be message sequence numbers 0
to 4. Additionally we can buffer up to 10 messages in the future.
Therefore the maximum number of messages that an attacker could send
that could get orphaned would typically be 15.
The maximum size that a DTLS message is allowed to be is defined by
max_cert_list, which by default is 100k. Therefore the maximum amount of
"orphaned" memory per connection is 1500k.
Message sequence numbers get reset after the Finished message, so
renegotiation will not extend the maximum number of messages that can be
orphaned per connection.
As noted above, the queues do get cleared when the connection is closed.
Therefore in order to mount an effective attack, an attacker would have
to open many simultaneous connections.
Issue reported by Quan Luo.
CVE-2016-2179
Reviewed-by: Richard Levitte <levitte@openssl.org>
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>
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>
Fix some indentation at the same time
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1292)
- Always process ALPN (previously there was an early return in the
certificate status handling)
- Don't send a duplicate alert. Previously, both
ssl_check_clienthello_tlsext_late and its caller would send an
alert. Consolidate alert sending code in the caller.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Don't call strncpy with strlen of the source as the length. Don't call
strlen multiple times. Eventually we will want to replace this with a proper
PACKET style handling (but for construction of PACKETs instead of just
reading them as it is now). For now though this is safe because
PSK_MAX_IDENTITY_LEN will always fit into the destination buffer.
This addresses an OCAP Audit issue.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Continuing from the previous commits, this splits out the GOST code into
a separate function from the process CKE code.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Continuing from the previous commits, this splits out the ECDHE code into
a separate function from the process CKE code.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Continuing from the previous commit, this splits out the DHE code into
a separate function from the process CKE code.
Reviewed-by: Richard Levitte <levitte@openssl.org>
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: Richard 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: Richard Levitte <levitte@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: Emilia 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: Emilia 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: Emilia Käsper <emilia@openssl.org>
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: Rich Salz <rsalz@openssl.org>
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
294d1e36c2, and the logic after the
decryption in the SSL code is constant-time as of
adb46dbc6d, 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: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Rich Salz <rsalz@openssl.org>
GH: #1222
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: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1098)
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>
This involves providing more session ticket key data, for both the cipher and
the digest
Signed-off-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matt Caswell <matt@openssl.org>
GH: #515, MR: #2153
The current limit of 2^14 bytes is too low (e.g. RFC 5246 specifies the
maximum size of just the extensions field to be 2^16-1), and may cause
bogus failures.
RT#4063
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/413)
