Large GOST ClientKeyExchange messages are sent by VipNet CSP, one of
Russian certified products implementing GOST TLS, when a server
certificate contains 512-bit keys.
This behaviour was present in 1.0.2 branch and needs to be restored.
Backport of #10376
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10376)
As was done for ciphers, supported groups, and EC point formats in
https://github.com/openssl/openssl/pull/9162, only write the negotiated
SNI hostname value to the session object when not resuming, even for
TLS 1.3 resumptions. Otherwise, when using a stateful session cache
(as is done by default when 0-RTT data is enabled), we can have multiple
SSLs active using the same in-memory session object, which leads to
double-frees and similar race conditions in the SNI handler prior
to this commit.
Fortunately, since draft-ietf-tls-tls13-22, there is no requirement
that the SNI hostname be preserved across TLS 1.3 resumption, and thus
not a need to continually update the session object with the "current"
value (to be used when producing session tickets, so that the subsequent
resumption can be checked against the current value). So we can just
relax the logic and only write to the session object for initial handshakes.
This still leaves us in a somewhat inconsistent state, since if the SNI value
does change across handshakes, the session object will continue to record
the initial handshake's value, even if that bears no relation to the
current handshake. The current SSL_get_servername() implementation
prefers the value from the session if s->hit, but a more complete fix
for that and related issues is underway in
https://github.com/openssl/openssl/pull/10018; there is no need to wait
for the complete fix for SNI name handling in order to close the
race condition and avoid runtime crashes.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10441)
(cherry picked from commit 2a5385511051d33be8d2b20d7669d8b1862fe510)
The resumption_label variable when CHARSET_EBCDIC was enabled, was misspelled.
Instead of evaluating to 'res binder' as expected, it evaluated to 'red binder'.
CLA: trivial
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/10396)
(cherry picked from commit 6ed12cec7216c3e81b58f5cafa41775e456feaee)
Fixes#9869
CLA:trivial
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9878)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/9653)
(cherry picked from commit e7c27a6c3716843f8412fd96311b70ac84b785f9)
Apart from public and internal header files, there is a third type called
local header files, which are located next to source files in the source
directory. Currently, they have different suffixes like
'*_lcl.h', '*_local.h', or '*_int.h'
This commit changes the different suffixes to '*_local.h' uniformly.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9681)
If a TLSv1.3 server configured to respond to the status_request extension
also attempted to send a CertificateRequest then it was incorrectly
inserting a non zero length status_request extension into that message.
The TLSv1.3 RFC does allow that extension in that message but it must
always be zero length.
In fact we should not be sending the extension at all in that message
because we don't support it.
Fixes#9767
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9780)
(cherry picked from commit debb64a0ca43969eb3f043aa8895a4faa7f12b6e)
Fix a few places where calling ossl_isdigit does the wrong thing on
EBCDIC based systems.
Replaced with ascii_isdigit.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9556)
(cherry picked from commit 48102247ff)
tls_parse_stoc_key_share was generating a new EVP_PKEY public/private
keypair and then overrides it with the server public key, so the
generation was a waste anyway. Instead, it should create a
parameters-only EVP_PKEY.
(This is a consequence of OpenSSL using the same type for empty key,
empty key with key type, empty key with key type + parameters, public
key, and private key. As a result, it's easy to mistakenly mix such
things up, as happened here.)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/9445)
(cherry picked from commit 166c0b98fd)
This reverts commit 524006dd1b.
While this change did prevent the sigalgs from getting inadvertently
clobbered by SSL_set_SSL_CTX(), it also caused the sigalgs to not be
set when the cert_cb runs. This, in turn, caused significant breakage,
such as SSL_check_chain() failing to find any valid chain. An alternate
approach to fixing the issue from #7244 will follow.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9157)
(cherry picked from commit 6f34d7bc7d)
The previous 2 commits moved supported groups and ciphers out of the
session object to avoid race conditions. We now also move ecpointformats
for consistency. There does not seem to be a race condition with access
to this data since it is only ever set in a non-resumption handshake.
However, there is no reason for it to be in the session.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9176)
Similarly to the previous commit we were storing the peer offered list
of ciphers in the session. In practice there is no need for this
information to be avilable from one resumption to the next since this
list is specific to a particular handshake. Since the session object is
supposed to be immutable we should not be updating it once we have decided
to resume. The solution is to remove the session list out of the session
object.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9176)
In TLSv1.3 the supported groups can be negotiated each time a handshake
occurs, regardless of whether we are resuming or not. We should not store
the supported groups information in the session because session objects
can be shared between multiple threads and we can end up with race
conditions. For most users this won't be seen because, by default, we
use stateless tickets in TLSv1.3 which don't get shared. However if you
use SSL_OP_NO_TICKET (to get stateful tickets in TLSv1.3) then this can
happen.
The answer is to move the supported the supported group information into
the SSL object instead.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9176)
If we receive a KeyUpdate message (update requested) from the peer while
we are in the middle of a write, we should defer sending the responding
KeyUpdate message until after the current write is complete. We do this
by waiting to send the KeyUpdate until the next time we write and there is
no pending write data.
This does imply a subtle change in behaviour. Firstly the responding
KeyUpdate message won't be sent straight away as it is now. Secondly if
the peer sends multiple KeyUpdates without us doing any writing then we
will only send one response, as opposed to previously where we sent a
response for each KeyUpdate received.
Fixes#8677
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/8773)
(cherry picked from commit feb9e31c40)
Sessions must be immutable once they can be shared with multiple threads.
We were breaking that rule by writing the ticket index into it during the
handshake. This can lead to incorrect behaviour, including failed
connections in multi-threaded environments.
Reported by David Benjamin.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8383)
(cherry picked from commit c96ce52ce2)
Prior to this commit we were keeping a count of how many KeyUpdates we
have processed and failing if we had had too many. This simplistic approach
is not sufficient for long running connections. Since many KeyUpdates
would not be a particular good DoS route anyway, the simplest solution is
to simply remove the key update count.
Fixes#8068
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/8299)
(cherry picked from commit 3409a5ff8a)
The original 1.1.1 design was to use SSL_CB_HANDSHAKE_START and
SSL_CB_HANDSHAKE_DONE to signal start/end of a post-handshake message
exchange in TLSv1.3. Unfortunately experience has shown that this confuses
some applications who mistake it for a TLSv1.2 renegotiation. This means
that KeyUpdate messages are not handled properly.
This commit removes the use of SSL_CB_HANDSHAKE_START and
SSL_CB_HANDSHAKE_DONE to signal the start/end of a post-handshake
message exchange. Individual post-handshake messages are still signalled in
the normal way.
This is a potentially breaking change if there are any applications already
written that expect to see these TLSv1.3 events. However, without it,
KeyUpdate is not currently usable for many applications.
Fixes#8069
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8096)
(cherry picked from commit 4af5836b55)
When computing the end-point shared secret, don't take the
terminating NULL character into account.
Please note that this fix breaks interoperability with older
versions of OpenSSL, which are not fixed.
Fixes#7956
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7957)
(cherry picked from commit 09d62b336d)
This commit erroneously kept the DTLS timer running after the end of the
handshake. This is not correct behaviour and shold be reverted.
This reverts commit f7506416b1.
Fixes#7998
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8047)
(cherry picked from commit bcc1f3e2ba)
We were setting a limit of SSL3_RT_MAX_PLAIN_LENGTH on the size of the
ClientHello. AFAIK there is nothing in the standards that requires this
limit.
The limit goes all the way back to when support for extensions was first
added for TLSv1.0. It got converted into a WPACKET max size in 1.1.1. Most
likely it was originally added to avoid the complexity of having to grow
the init_buf in the middle of adding extensions. With WPACKET this is
irrelevant since it will grow automatically.
This issue came up when an attempt was made to send a very large
certificate_authorities extension in the ClientHello.
We should just remove the limit.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7424)
(cherry picked from commit 7835e97b6f)
The cryptopro extension is supposed to be unsolicited and appears in the
ServerHello only. Additionally it is unofficial and unregistered - therefore
we should really treat it like any other unknown extension if we see it in
the ClientHello.
Fixes#7747
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7984)
(cherry picked from commit 23fed8ba0e)
Fix some issues in tls13_hkdf_expand() which impact the above function
for TLSv1.3. In particular test that we can use the maximum label length
in TLSv1.3.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7755)
(cherry picked from commit 0fb2815b87)
If compile OpenSSL with SSL_DEBUG macro, some test cases will cause the
process crashed in the debug code.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7707)
(cherry picked from commit 5a4481f0e0)
SSL(_CTX)?_set_client_CA_list() was a server side only function in 1.1.0.
If it was called on the client side then it was ignored. In 1.1.1 it now
makes sense to have a CA list defined for both client and server (the
client now sends it the the TLSv1.3 certificate_authorities extension).
Unfortunately some applications were using the same SSL_CTX for both
clients and servers and this resulted in some client ClientHellos being
excessively large due to the number of certificate authorities being sent.
This commit seperates out the CA list updated by
SSL(_CTX)?_set_client_CA_list() and the more generic
SSL(_CTX)?_set0_CA_list(). This means that SSL(_CTX)?_set_client_CA_list()
still has no effect on the client side. If both CA lists are set then
SSL(_CTX)?_set_client_CA_list() takes priority.
Fixes#7411
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7503)
(cherry picked from commit 9873297900)
TLSv1.3 is more restrictive about the curve used. There must be a matching
sig alg defined for that curve. Therefore if we are using some other curve
in our certificate then we should not negotiate TLSv1.3.
Fixes#7435
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7442)
(cherry picked from commit de4dc59802)
use_ecc() was always returning 1 because there are default (TLSv1.3)
ciphersuites that use ECC - even if those ciphersuites are disabled by
other options.
Fixes#7471
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/7479)
(cherry picked from commit 589b6227a8)
Commit 9ef9088c15 switched the SSL/SSL_CTX
statistics counters to using Thread-Sanitizer-friendly primitives.
However, it erroneously converted an addition of -1
(for s->session_ctx->stats.sess_accept) to an addition of +1, since that
is the only counter API provided by the internal tsan_assist.h header
until the previous commit. This means that for each accepted (initial)
connection, the session_ctx's counter would get doubly incremented, and the
(switched) ctx's counter would also get incremented.
Restore the counter decrement so that each accepted connection increments
exactly one counter exactly once (in net effect).
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7464)
(cherry picked from commit 2aaa0b146b)
In TLSv1.2 and below a CertificateRequest is sent after the Certificate
from the server. This means that by the time the client_cert_cb is called
on receipt of the CertificateRequest a call to SSL_get_peer_certificate()
will return the server certificate as expected. In TLSv1.3 a
CertificateRequest is sent before a Certificate message so calling
SSL_get_peer_certificate() returns NULL.
To workaround this we delay calling the client_cert_cb until after we
have processed the CertificateVerify message, when we are doing TLSv1.3.
Fixes#7384
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7413)
(cherry picked from commit e45620140f)
PR #3783 introduce coded to reset the server side SNI state in
SSL_do_handshake() to ensure any erroneous config time SNI changes are
cleared. Unfortunately SSL_do_handshake() can be called mid-handshake
multiple times so this is the wrong place to do this and can mean that
any SNI data is cleared later on in the handshake too.
Therefore move the code to a more appropriate place.
Fixes#7014
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7149)
Commit 1c4aa31d79 modified the state machine
to clean up stale ext.hostname values from SSL objects in the case when
SNI was not negotiated for the current handshake. This is natural from
the TLS perspective, since this information is an extension that the client
offered but we ignored, and since we ignored it we do not need to keep it
around for anything else.
However, as documented in https://github.com/openssl/openssl/issues/7014 ,
there appear to be some deployed code that relies on retrieving such an
ignored SNI value from the client, after the handshake has completed.
Because the 1.1.1 release is on a stable branch and should preserve the
published ABI, restore the historical behavior by retaining the ext.hostname
value sent by the client, in the SSL structure, for subsequent retrieval.
[extended tests]
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7115)
The is_tls13_capable() function should not return 0 if no certificates
are configured directly because a certificate callback is present.
Fixes#7140
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7141)
If we've sent a close_notify then we are restricted about what we can do
in response to handshake messages that we receive. However we can sensibly
process NewSessionTicket messages. We can also process a KeyUpdate message
as long as we also ignore any request for us to update our sending keys.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7114)
Treat a connection using an external PSK like we would a resumption and
send a single NewSessionTicket afterwards.
Fixes#6941
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7097)
If a client sends data to a server and then immediately closes without
waiting to read the NewSessionTickets then the server can receive EPIPE
when trying to write the tickets and never gets the opportunity to read
the data that was sent. Therefore we ignore EPIPE when writing out the
tickets in TLSv1.3
Fixes#6904
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6944)
We need to ensure that the min-max version range we use when constructing
the ClientHello is the same range we use when we validate the version
selected by the ServerHello. Otherwise this may appear as a fallback or
downgrade.
Fixes#6964
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7013)
Having post handshake auth automatically switched on breaks some
applications written for TLSv1.2. This changes things so that an explicit
function call is required for a client to indicate support for
post-handshake auth.
Fixes#6933.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6938)
