Since we no longer do version negotiation during the processing of an HRR
we do not need the TLSv1.3 specific write transition for ClientHello
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
The new ServerHello format is essentially now the same as the old TLSv1.2
one, but it must additionally include supported_versions. The version
field is fixed at TLSv1.2, and the version negotiation happens solely via
supported_versions.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
The check_fatal macro is supposed to only be called if we are already
expecting to be in the fatal state. The macro asserts that we are and
puts us into the fatal state if not.
This issue combined with the problem fixed in the previous commit meant
that the fuzzer detected a crash at a point in the processing when we
should have already been in the fatal state.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4847)
The recent SSL error overhaul left a case where an error occurs but
SSLfatal() is not called.
Credit to OSSfuzz for finding this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4847)
The most likely explanation for us ending up at this point in the code
is that we were called by the user application incorrectly - so use an
appropriate error code.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
Follow up from the conversion to use SSLfatal() in the state machine to
clean things up a bit more.
[extended tests]
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
We shouldn't call SSLfatal() multiple times for the same error condition.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
Sometimes at the top level of the state machine code we know we are
supposed to be in a fatal error condition. This commit adds some sanity
checks to ensure that SSLfatal() has been called.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
This is an initial step towards using SSLfatal() everywhere. Initially in
this commit and in subsequent commits we focus on the state machine code.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
Typically if a fatal error occurs three things need to happen:
- Put an error on the error queue
- Send an alert
- Put the state machine into the error state
Although all 3 of these things need to be done every time we hit a fatal
error the responsibilities for doing this are distributed throughout the
code. The place where the error goes on the queue, where the alert gets
sent and where the state machine goes into the error state are almost
invariably different. It has been a common pattern to pass alert codes up
and down the stack to get the alert information from the point in the code
where the error is detected to the point in the code where the alert gets
sent.
This commit provides an SSLfatal() macro (backed by an ossl_statem_fatal
function) that does all 3 of the above error tasks. This is largely a drop
in replacement for SSLerr, but takes a couple of extra parameters (the SSL
object, and an alert code).
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
SNI needs to be consistent before we accept early_data. However a
server may choose to not acknowledge SNI. In that case we have to
expect that a client may send it anyway. We change the consistency
checks so that not acknowledging is treated more a like a "wild card",
accepting any SNI as being consistent.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4738)
We can only send early_data if the SNI is consistent. However it is valid
for the client to set SNI and the server to not use it. This would still be
counted as consistent. OpenSSL client was being overzealous in this check
and disallowing this scenario.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4738)
It's argued that /WX allows to keep better focus on new code, which
motivates its comeback...
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4721)
We currently increment the SSL_CTX stats.sess_accept field in
tls_setup_handshake(), which is invoked from the state machine well
before ClientHello processing would have had a chance to switch
the SSL_CTX attached to the SSL object due to a provided SNI value.
However, stats.sess_accept_good is incremented in tls_finish_handshake(),
and uses the s->ctx.stats field (i.e., the new SSL_CTX that was switched
to as a result of SNI processing). This leads to the confusing
(nonsensical) situation where stats.sess_accept_good is larger than
stats.sess_accept, as the "sess_accept" value was counted on the
s->session_ctx.
In order to provide some more useful numbers, increment
s->ctx.stats.sess_accept after SNI processing if the SNI processing
changed s->ctx to differ from s->session_ctx. To preserve the
property that any given accept is counted only once, make the
corresponding decrement to s->session_ctx.stats.sess_accept when
doing so.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
For client SSL objects and before any callbacks have had a chance
to be called, we can write the stats accesses using the session_ctx,
which makes sense given that these values are all prefixed with
"sess_".
For servers after a client_hello or servername callback has been
called, retain the existing behavior of modifying the statistics
for the current (non-session) context. This has some value,
in that it allows the statistics to be viewed on a per-vhost level.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
It is expected that SSL_CTX objects are shared across threads,
and as such we are responsible for ensuring coherent data accesses.
Aligned integer accesses ought to be atomic already on all supported
architectures, but we can be formally correct.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
Since return is inconsistent, I removed unnecessary parentheses and
unified them.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4541)
The previous commit removed version negotiation on an HRR. However we should
still sanity check the contents of the version field.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)
Previously if a client received an HRR then we would do version negotiation
immediately - because we know we are going to get TLSv1.3. However this
causes a problem when we emit the 2nd ClientHello because we start changing
a whole load of stuff to ommit things that aren't relevant for < TLSv1.3.
The spec requires that the 2nd ClientHello is the same except for changes
required from the HRR. Therefore the simplest thing to do is to defer the
version negotiation until we receive the ServerHello.
Fixes#4292
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)
The function tls_check_curve is only called on clients and contains
almost identical functionaity to tls1_check_group_id when called from
a client. Merge the two.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4475)
When an SSL's context is swtiched from a ticket-enabled context to
a ticket-disabled context in the servername callback, no session-id
is generated, so the session can't be resumed.
If a servername callback changes the SSL_OP_NO_TICKET option, check
to see if it's changed to disable, and whether a session ticket is
expected (i.e. the client indicated ticket support and the SSL had
tickets enabled at the time), and whether we already have a previous
session (i.e. s->hit is set).
In this case, clear the ticket-expected flag, remove any ticket data
and generate a session-id in the session.
If the SSL hit (resumed) and switched to a ticket-disabled context,
assume that the resumption was via session-id, and don't bother to
update the session.
Before this fix, the updated unit-tests in 06-sni-ticket.conf would
fail test #4 (server1 = SNI, server2 = no SNI).
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/1529)
Rename tls1_get_curvelist to tls1_get_grouplist, change to void as
it can never fail and remove unnecessary return value checks. Clean
up the code.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/=4412)
Setup EVP_PKEY structure from a group ID in ssl_generate_param_group,
replace duplicate code with this function.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/=4412)
Replace tls1_ec_curve_id2nid() with tls_group_id_lookup() which returns
the TLS_GROUP_INFO for the group.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/=4412)
Instead of storing supported groups in on-the-wire format store
them as parsed uint16_t values. This simplifies handling of groups
as the values can be directly used instead of being converted.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4406)
It is otherwise unclear what all the magic numbers mean.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4349)
"Early callback" is a little ambiguous now that early data exists.
Perhaps "ClientHello callback"?
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4349)
In OpenSSL 1.1.0, when there were no extensions added to the ServerHello,
we did not write the extension data length bytes to the end of the
ServerHello; this is needed for compatibility with old client implementations
that do not support TLS extensions (such as the default configuration of
OpenSSL 0.9.8). When ServerHello extension construction was converted
to the new extensions framework in commit
7da160b0f4, this behavior was inadvertently
limited to cases when SSLv3 was negotiated (and similarly for ClientHellos),
presumably since extensions are not defined at all for SSLv3. However,
extensions for TLS prior to TLS 1.3 have been defined in separate
RFCs (6066, 4366, and 3546) from the TLS protocol specifications, and as such
should be considered an optional protocol feature in those cases.
Accordingly, be conservative in what we send, and skip the extensions block
when there are no extensions to be sent, regardless of the TLS/SSL version.
(TLS 1.3 requires extensions and can safely be treated differently.)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4296)
This is actually not all warnings, only return values.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4297)
SNI and ALPN must be set to be consistent with the PSK. Otherwise this is
an error.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
cryptilib.h is the second.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4188)
Removed e_os.h from all bar three headers (apps/apps.h crypto/bio/bio_lcl.h and
ssl/ssl_locl.h).
Added e_os.h into the files that need it now.
Directly reference internal/nelem.h when required.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4188)
Force non-empty padding extension.
When enabled, force the padding extension to be at least 1 byte long.
WebSphere application server cannot handle having an empty
extension (e.g. EMS/EtM) as the last extension in a client hello.
This moves the SigAlgs extension last for TLSv1.2 to avoid this
issue.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3921)
Commit 02f0274e8c moved ALPN processing
into an extension finalization function, as the only documented ordering
requirement from previous commits was that ALPN processing occur after
SNI processing, and SNI processing is performed before the extension
finalization step. However, it is useful for applications'
alpn_select callbacks to run after ciphersuite selection as well -- at
least one application protocol specification (HTTP/2) imposes restrictions
on which ciphersuites are usable with that protocol. Since it is generally
more preferrable to have a successful TLS connection with a default application
protocol than to fail the TLS connection and not be able to have the preferred
application protocol, it is good to give the alpn_select callback information
about the ciphersuite to be used, so that appropriate restrctions can be
enforced in application code.
Accordingly, split the ALPN handling out into a separate tls_handl_alpn()
function akin to tls_handle_status_request(), called from
tls_post_process_client_hello(). This is an alternative to resuscitating
ssl_check_clienthello_tlsext_late(), something of an awkwward name itself.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4070)
If s->hit is set, s->session corresponds to a session created on
a previous connection, and is a data structure that is potentially
shared across other SSL objects. As such, there are thread-safety
issues with modifying the structure without taking its lock (and
of course all corresponding read accesses would also need to take
the lock as well), which have been observed to cause double-frees.
Regardless of thread-safety, the resumed session object is intended
to reflect parameters of the connection that created the session,
and modifying it to reflect the parameters from the current connection
is confusing. So, modifications to the session object during
ClientHello processing should only be performed on new connections,
i.e., those where s->hit is not set.
The code mostly got this right, providing such checks when processing
SNI and EC point formats, but the supported groups (formerly
supported curves) extension was missing it, which is fixed by this commit.
However, TLS 1.3 makes the suppported_groups extension mandatory
(when using (EC)DHE, which is the normal case), checking for the group
list in the key_share extension processing. But, TLS 1.3 only [0] supports
session tickets for session resumption, so the session object in question
is the output of d2i_SSL_SESSION(), and will not be shared across SSL
objects. Thus, it is safe to modify s->session for TLS 1.3 connections.
[0] A psk_find_session callback can also be used, but the restriction that
each callback execution must produce a distinct SSL_SESSION structure
can be documented when the psk_find_session callback documentation is
completed.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4123)
Give each SSL object it's own DRBG, chained to the parent global
DRBG which is used only as a source of randomness into the per-SSL
DRBG. This is used for all session, ticket, and pre-master secret keys.
It is NOT used for ECDH key generation which use only the global
DRBG. (Doing that without changing the API is tricky, if not impossible.)
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4050)
If RAND_add wraps around, XOR with existing. Add test to drbgtest that
does the wrap-around.
Re-order seeding and stop after first success.
Add RAND_poll_ex()
Use the DF and therefore lower RANDOMNESS_NEEDED. Also, for child DRBG's,
mix in the address as the personalization bits.
Centralize the entropy callbacks, from drbg_lib to rand_lib.
(Conceptually, entropy is part of the enclosing application.)
Thanks to Dr. Matthias St Pierre for the suggestion.
Various code cleanups:
-Make state an enum; inline RANDerr calls.
-Add RAND_POLL_RETRIES (thanks Pauli for the idea)
-Remove most RAND_seed calls from rest of library
-Rename DRBG_CTX to RAND_DRBG, etc.
-Move some code from drbg_lib to drbg_rand; drbg_lib is now only the
implementation of NIST DRBG.
-Remove blocklength
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4019)
Move the definition of ossl_assert() out of e_os.h which is intended for OS
specific things. Instead it is moved into internal/cryptlib.h.
This also changes the definition to remove the (int) cast.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4073)
If a new_session_cb is set then it was only ever getting invoked if !s->hit
is true. This is sensible for <=TLSv1.2 but does not work for TLSv1.3.
Fixes#4045
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4068)
Some extensions were being displayed twice, before they were parsed, and
again after they were parsed.
The supported_versions extension was not being fully displayed, as it
was processed differently than other extensions.
Move the debug callback to where the extensions are first collected, to
catch all the extensions as they come in, so they are ordered correctly.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3911)
Fixes: issue #3747
make SSL_CIPHER_standard_name globally available and introduce a new
function OPENSSL_cipher_name.
A new option '-convert' is also added to 'openssl ciphers' app.
Documentation and test cases are added.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/3859)
TLSv1.3 draft-21 requires the ticket nonce to be at least 1 byte in length.
However NSS sends a zero length nonce. This is actually ok because the next
draft will allow zero length nonces anyway, so we should tolerate this.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3957)
In most scenarios the length of the input data is the hashsize, or 0 if
the data is NULL. However with the new ticket_nonce changes the length can
be different.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3852)
This just adds the processing for sending and receiving the newly added
ticket_nonce field. It doesn't actually use it yet.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3852)
Following on from the previous commit this fixes another instance where
we need to treat a -ve return from EVP_DigestVerify() as a bad signature.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3756)
Prior to 72ceb6a we treated all failures from the call to
EVP_DigestVerifyFinal() as if it were a bad signature, and failures in
EVP_DigestUpdate() as an internal error. After that commit we replaced
this with the one-shot function EVP_DigestVerify() and treated a 0 return
as a bad signature and a negative return as an internal error. However,
some signature errors can be negative (e.g. according to the docs if the
form of the signature is wrong). Therefore we should treat all <=0
returns as a bad signature.
This fixes a boringssl test failure.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3756)
initialize some local variables
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3741)
The value of BIO_CTRL_DGRAM_SET_PEEK_MODE was clashing with the value for
BIO_CTRL_DGRAM_SCTP_SET_IN_HANDSHAKE. In an SCTP enabled build
BIO_CTRL_DGRAM_SCTP_SET_IN_HANDSHAKE was used unconditionally with
the reasoning that it would be ignored if SCTP wasn't in use. Unfortunately
due to this clash, this wasn't the case. The BIO ended up going into peek
mode and was continually reading the same data over and over - throwing it
away as a replay.
Fixes#3723
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3724)
This does things as per the recommendation in the TLSv1.3 spec. It also
means that the server will always choose its preferred ciphersuite.
Previously the server would only select ciphersuites compatible with the
session.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3623)
It is an API to be used from the early callback that indicates what
extensions were present in the ClientHello, and in what order.
This can be used to eliminate unneeded calls to SSL_early_get0_ext()
(which itself scales linearly in the number of extensions supported
by the library).
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2976)
Signed-off-by: Paul Yang <paulyang.inf@gmail.com>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3622)
SSLv3 (specifically with client auth) cannot use one shot APIs: the digested
data and the master secret are handled in separate update operations. So
in the special case of SSLv3 use the streaming API.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3527)
We are quite inconsistent about which alerts get sent. Specifically, these
alerts should be used (normally) in the following circumstances:
SSL_AD_DECODE_ERROR = The peer sent a syntactically incorrect message
SSL_AD_ILLEGAL_PARAMETER = The peer sent a message which was syntactically
correct, but a parameter given is invalid for the context
SSL_AD_HANDSHAKE_FAILURE = The peer's messages were syntactically and
semantically correct, but the parameters provided were unacceptable to us
(e.g. because we do not support the requested parameters)
SSL_AD_INTERNAL_ERROR = We messed up (e.g. malloc failure)
The standards themselves aren't always consistent but I think the above
represents the best interpretation.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3480)
add_key_share() is a helper function used during key_share extension
construction. It is expected to be a simple boolean success/fail return.
It shouldn't be using the new EXT_RETURN type but it was partially converted
anyway. This changes it back.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3488)
The current TLSv1.3 spec says:
'If a server is authenticating via a certificate and the client has not
sent a "signature_algorithms" extension, then the server MUST abort the
handshake with a "missing_extension" alert (see Section 8.2).'
If we are resuming then we are not "authenticating via a certificate" but
we were still aborting with the missing_extension alert if sig algs was
missing.
This commit ensures that we only send the alert if we are not resuming.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3478)
We already did this on an ad-hoc per extension basis (for some extensions).
This centralises it and makes sure we do it for all extensions.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3418)
- Mostly missing fall thru comments
- And uninitialized value used in sslapitest.c
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3440)
Add "single part" digest sign and verify functions. These sign and verify
a message in one function. This simplifies some operations and it will later
be used as the API for algorithms which do not support the update/final
mechanism (e.g. PureEdDSA).
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3409)
The function SSL_set_SSL_CTX() can be used to swap the SSL_CTX used for
a connection as part of an SNI callback. One result of this is that the
s->cert structure is replaced. However this structure contains information
about any custom extensions that have been loaded. In particular flags are
set indicating whether a particular extension has been received in the
ClientHello. By replacing the s->cert structure we lose the custom
extension flag values, and it appears as if a client has not sent those
extensions.
SSL_set_SSL_CTX() should copy any flags for custom extensions that appear
in both the old and the new cert structure.
Fixes#2180
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3425)
It is invalid if we receive an HRR but no change will result in
ClientHello2.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3414)
If an HRR gets sent without a key_share (e.g. cookie only) then the code
fails when it should not.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3414)
It is illegal in a TLSv1.3 ClientHello to send anything other than the
NULL compression method. We should send an alert if we find anything else
there. Previously we were ignoring this error.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3410)
The TLSv1.3 spec says that a server SHOULD send supported_groups in the
EE message if there is a group that it prefers to the one used in the
key_share. Clients MAY act on that. At the moment we don't do anything
with it on the client side, but that may change in the future.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3395)
We already did this for ServerHello and EncryptedExtensions. We should be
doing it for Certificate and HelloRetryRequest as well.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3298)
This fixes a segfault if a NULL parse_cb is passed to
SSL_CTX_add_{client,server}_custom_ext, which was supported in the
pre-1.1.1 implementation.
This behaviour is consistent with the other custom_ext_*_old_cb_wrap
functions, and with the new SSL_CTX_add_custom_ext function.
CLA: trivial
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3310)
Previously, init and finalization function for extensions are called
per extension block, rather than per message. This commit changes
that behaviour, and now they are called per message. The parse
function is still called per extension block.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3244)
Because NST messages arrive post-handshake, the session may have already
gone into the cache. Once in the cache a session must be immutable -
otherwise you could get multi-thread issues.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3008)
Provide a way to test whether the SSL_SESSION object can be used to resume a
sesion or not.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3008)
TLSv1.3 will do the same thing as TLSv1.2 with tickets with regards to session
ids, i.e. it will create a synthetic session id when the session is established,
so it is reasonable to check the session id length, even in TLSv1.3.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3008)
Ensure that there are ciphersuites enabled for the maximum supported
version we will accept in a ClientHello.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3316)
Ensure that there are ciphersuites enabled for the maximum supported
version we are claiming in the ClientHello.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3316)
The function tls_early_post_process_client_hello() was overwriting the
passed "al" parameter even if it was successful. The caller of that
function, tls_post_process_client_hello(), sets "al" to a sensible default
(HANDSHAKE_FAILURE), but this was being overwritten to be INTERNAL_ERROR.
The result is a "no shared cipher" error (and probably other similar errors)
were being reported back to the client with an incorrect INTERNAL_ERROR
alert.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3314)
We were allocating the write buffer based on the size of max_send_fragment,
but ignoring it when writing data. We should fragment handshake messages
if they exceed max_send_fragment and reject application data writes that
are too large.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3286)
There was code existing which attempted to handle the case where application
data is received after a reneg handshake has started in SCTP. In normal DTLS
we just fail the connection if this occurs, so there doesn't seem any reason
to try and work around it for SCTP. In practice it didn't work properly
anyway and is probably a bad idea to start with.
Fixes#3251
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3286)
ECDHE is not properly defined for SSLv3. Commit fe55c4a2 prevented ECDHE
from being selected in that protocol. However, historically, servers do
still select ECDHE anyway so that commit causes interoperability problems.
Clients that previously worked when talking to an SSLv3 server could now
fail.
This commit introduces an exception which enables a client to continue in
SSLv3 if the server selected ECDHE.
[extended tests]
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3204)
doing the pms assignment after log is successful
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3278)
RFC 7301 mandates that the server SHALL respond with a fatal
"no_application_protocol" alert when there is no overlap between
the client's supplied list and the server's list of supported protocols.
In commit 062178678f we changed from
ignoring non-success returns from the supplied alpn_select_cb() to
treating such non-success returns as indicative of non-overlap and
sending the fatal alert.
In effect, this is using the presence of an alpn_select_cb() as a proxy
to attempt to determine whether the application has configured a list
of supported protocols. However, there may be cases in which an
application's architecture leads it to supply an alpn_select_cb() but
have that callback be configured to take no action on connections that
do not have ALPN configured; returning SSL_TLSEXT_ERR_NOACK from
the callback would be the natural way to do so. Unfortunately, the
aforementioned behavior change also treated SSL_TLSEXT_ERR_NOACK as
indicative of no overlap and terminated the connection; this change
supplies special handling for SSL_TLSEXT_ERR_NOACK returns from the
callback. In effect, it provides a way for a callback to obtain the
behavior that would have occurred if no callback was registered at
all, which was not possible prior to this change.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2570)
The old custom extensions API was not TLSv1.3 aware. Extensions are used
extensively in TLSv1.3 and they can appear in many different types of
messages. Therefore we need a new API to be able to cope with that.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
This move prepares for the later addition of the new custom extensions
API. The context codes have an additional "SSL_" added to their name to
ensure we don't have name clashes with other applications.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3139)
Add functions to add/retrieve the certificate_authorities. The older
client_CA functions mainly just call the new versions now.
Rename fields sice new extension can be generated by client and server.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3015)
The macro SSL_get_server_tmp_key() returns information about the temp key
used by the server during a handshake. This was returning NULL for TLSv1.3
and causing s_client to omit this information in its connection summary.
Fixes#3081
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3114)
Variable 'pktype' was set but not used under OPENSSL_NO_GOST. This change
will fix the build warning under [-Werror=unused-but-set-variable].
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2961)
A similar change that probably should have been wrapped into
commit e0926ef49d.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3010)
Fix some comments too
[skip ci]
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3069)
Commit 6b1bb98fa moved the processing of ClientHello extensions into the
state machine post-processing stage. After processing s->init_num is reset
to 0, so by post-processing we cannot rely on its value. Unfortunately we
were using it to handle the PSK extension. This causes the handshake to
fail.
We were using init_num to figure out the length of ClientHello2 so we can
remove it from the handshake_buffer. The handshake_buffer holds the
transcript of all the messages sent so far. For PSK processing though we
only want to add in a partial ClientHello2. This commit changes things so
we just work out where ClientHello2 starts, working forward from the
beginning of handshake_buffer.
Fixes#2983
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2996)
In OpenSSL 1.1.0 the padding extension MUST be last because it calculates
the length of everything that has been written into the ClientHello to
determine whether it needs to be padded or not. With TLSv1.3 that isn't
possible because the specification requires that the PSK extension is last.
Therefore we need to fix the padding extension to take account of any PSK
extension that will be later added.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2968)
Choose a new ciphersuite for the HRR. Don't just use the one from the
session.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2895)
Don't include a PSK that does not have the right hash for the selected
ciphersuite following an HRR.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2895)
Draft-19 changes the HRR transcript hash so that the initial ClientHello
is replaced in the transcript with a special synthetic message_hash message
that just contains a hash of ClientHello1 as its message body.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2895)
The end of early data is now indicated by a new handshake message rather
than an alert.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2895)
tls1_get_curvelist() does not read from its third parameter, so
the assignments prior to function call were dead code and can be removed.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2952)
Instead of making a positive comparison against the invalid value
that our server would send, make a negative check against the only
value that is not an error.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2953)
Zero out the length alongside the NULLing of the pointer, to
bring parity between the selected and proposed fields..
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2954)
No need to break out of the loop and repeat the loop termination
condition when we can just return.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2949)
In TLSv1.3 the above messages signal a key change. The spec requires that
the end of these messages must align with a record boundary. We can detect
this by checking for decrypted but as yet unread record data sitting in
OpenSSL buffers at the point where we process the messages.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2875)
If early data is sent to a server, but ALPN is not used then memcmp is
called with a NULL pointer which is undefined behaviour.
Fixes#2841
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2845)
We do not allow the generation of TLSv1.3 cookies. But if we receive one
in an HRR we will echo it back in the ClientHello.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2839)
A leak of an SSL_SESSION object can occur when decoding a psk extension on
an error path when using TLSv1.3
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2843)
