The default supported groups code was disabled in the event of a build
with no-ec and no-dh. However now that providers can add there own
groups (which might not fit into either of these categories), this is
no longer appropriate.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13916)
Fixes#13183
From the original issue report, before this commit, on master and on
1.1.1, the issue can be detected with the following steps:
- Start with a default SSL_CTX, initiate a TLS 1.3 connection with SNI,
"Accept" count of default context gets incremented
- After servername lookup, "Accept" count of default context gets
decremented and that of SNI context is incremented
- Server sends a "Hello Retry Request"
- Client sends the second "Client Hello", now again "Accept" count of
default context is decremented. Hence giving a negative value.
This commit fixes it by adding a check on `s->hello_retry_request` in
addition to `SSL_IS_FIRST_HANDSHAKE(s)`, to ensure the counter is moved
only on the first ClientHello.
CLA: trivial
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/13297)
Since SSLfatal() doesn't take a function code any more, we drop that
argument everywhere. Also, we convert all combinations of SSLfatal()
and ERR_add_data() to an SSLfatal_data() call.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/13316)
Automatically rename all instances of _with_libctx() to _ex() as per
our coding style.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/12970)
Convert various mac key creation function calls to use the _with_libctx
variants.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/12637)
-Added EVP_SignFinal_with_libctx() and EVP_VerifyFinal_with_libctx()
-Renamed EVP_DigestSignInit_ex() and EVP_DigestVerifyInit_with_libctx() to
EVP_DigestSignInit_with_libctx() and EVP_DigestVerifyInit_with_libctx()
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11884)
Abort renegotiation if server receives client hello with Extended Master
Secret extension dropped in comparison to the initial session.
Fixes#9754
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12045)
... and only *define* them in the source files that need them.
Use DEFINE_OR_DECLARE which is set appropriately for internal builds
and not non-deprecated builds.
Deprecate stack-of-block
Better documentation
Move some ASN1 struct typedefs to types.h
Update ParseC to handle this. Most of all, ParseC needed to be more
consistent. The handlers are "recursive", in so far that they are called
again and again until they terminate, which depends entirely on what the
"massager" returns. There's a comment at the beginning of ParseC that
explains how that works. {Richard Levtte}
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/10669)
If the servername cb decides to send back a warning alert then the
handshake continues, but we should not signal to the client that the
servername has been accepted.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/10018)
The SNI behaviour for TLSv1.3 and the behaviour of SSL_get_servername()
was not quite right, and not entirely consistent with the RFC.
The TLSv1.3 RFC explicitly says that SNI is negotiated on each handshake
and the server is not required to associate it with the session. This was
not quite reflected in the code so we fix that.
Additionally there were some additional checks around early_data checking
that the SNI between the original session and this session were
consistent. In fact the RFC does not require any such checks, so they are
removed.
Finally the behaviour of SSL_get_servername() was not quite right. The
behaviour was not consistent between resumption and normal handshakes,
and also not quite consistent with historical behaviour. We clarify the
behaviour in various scenarios and also attempt to make it match historical
behaviour as closely as possible.
Fixes#8822
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/10018)
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)
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)
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/9333)
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)
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/9162)
Make sure that the combination of no-ec with no-dh builds successfully.
If neither ec or dh are available then TLSv1.3 is not possible.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9156)
Now that we have TLSv1.3 FFDHE support there is no reason why we should
not allow TLSv1.3 to be used in a no-ec build. This commit enables that
to happen.
It also fixes no-ec which was previously broken.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9156)
With the removal of SSLv2, the s3 structure is always allocated, so
there is little point in having it be an allocated pointer. Collapse
the ssl3_state_st structure into ssl_st and fixup any references.
This should be faster than going through an indirection and due to
fewer allocations, but I'm not seeing any significant performance
improvement; it seems to be within the margin of error in timing.
Reviewed-by: Paul Yang <yang.yang@baishancloud.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7888)
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)
Add SSL_OP64_NO_EXTENDED_MASTER_SECRET, that can be set on either
an SSL or an SSL_CTX. When processing a ClientHello, if this flag
is set, do not indicate that the EMS TLS extension was received in
either the ssl3 object or the SSL_SESSION. Retain most of the
sanity checks between the previous and current session during
session resumption, but weaken the check when the current SSL
object is configured to not use EMS.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3910)
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)
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)
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)
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)
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)
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)
In particular, adhere to the rule that we must not modify any
property of an SSL_SESSION object once it is (or might be) in
a session cache. Such modifications are thread-unsafe and have
been observed to cause crashes at runtime.
To effect this change, standardize on the property that
SSL_SESSION->ext.hostname is set only when that SNI value
has been negotiated by both parties for use with that session.
For session resumption this is trivially the case, so only new
handshakes are affected.
On the client, the new semantics are that the SSL->ext.hostname is
for storing the value configured by the caller, and this value is
used when constructing the ClientHello. On the server, SSL->ext.hostname
is used to hold the value received from the client. Only if the
SNI negotiation is successful will the hostname be stored into the
session object; the server can do this after it sends the ServerHello,
and the client after it has received and processed the ServerHello.
This obviates the need to remove the hostname from the session object
in case of failed negotiation (a change that was introduced in commit
9fb6cb810b in order to allow TLS 1.3
early data when SNI was present in the ClientHello but not the session
being resumed), which was modifying cached sessions in certain cases.
(In TLS 1.3 we always produce a new SSL_SESSION object for new
connections, even in the case of resumption, so no TLS 1.3 handshakes
were affected.)
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6378)
TLSv1.3 ignores the alert level, so we should suppress sending of
warning only alerts.
Fixes#6211
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6370)
All tickets on a connection need to have a unique nonce. When this was
originally implemented we only ever sent one ticket on the conneciton so
this didn't matter. We were just using the value 0. Now we can get multiple
tickets to we need to start doing the ticket nonce properly.
Fixes#6387
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6415)
Renamed to EVP_PKEY_new_raw_private_key()/EVP_new_raw_public_key() as per
feedback.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5520)
At the core of things is the concept that each extension is only
defined in certain context(s) -- the ClientHello, EncryptedExtensions,
etc., and sometimes only for a specific protocol or protocol range;
we want to enforce that we only parse or generate extensions in the
context(s) for which they are defined. There is some subtlety here,
in that the protocol version in use is not known when generating the
ClientHello (but it is known when the ClientHello extensions are
being parsed!), so the SSL_IS_TLS13() macro must be used with caution.
Nonetheless, by making assertions about whether we are acting in a
server role and whether the current context is (not) a ClientHello,
we can consolidate almost all of the logic for determining whether
an extension is permitted in a given protocol message, whether we
are generating or parsing that message.
The only logic that remains separate relates to generating the ClientHello,
as it depends on an external factor (the maximum permitted TLS version) that
is not defined in the parsing context.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2945)
We also default to SHA256 as per the spec if we do not have an explicit
digest defined.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5554)
They are valid for use in a CertificateRequest message, but we did not
allow it. If a server sent such a message using either of those two
extensions then the handshake would abort.
This corrects that error, but does not add support for actually processing
the extensions. They are simply ignored, and a TODO is inserted to add
support at a later time.
This was found during interoperability testing with btls:
https://gitlab.com/ilari_l/btls
Prompted by these errors I reviewed the complete list of extensions and
compared them with the latest table in draft-24 to confirm there were no
other errors of a similar type. I did not find any.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5490)
