Fix TLS key share check to not fire when using < TLS 1.3

The check was being too aggressive and was catching us when the
extension was being sent by a client which supports tls 1.3 but
the server wa

Fix TLS key share check to not fire when using < TLS 1.3

The check was being too aggressive and was catching us when the
extension was being sent by a client which supports tls 1.3 but
the server was capped at TLS 1.2. This moves the check after the
max version check, so we won't error out if we do not support
TLS 1.3

Reported by obsd@bartula.de

ok tb@

show more ...

remove psk_idx from tlsext_randomize_build_order()

ok jsing

tls_extension_find(): make output index optional

suggested by jsing

Fix TLS extension shuffling

The diff decoupling the shuffle from the table order still relied on PSK
being last because it failed to adjust the upper bound in the for loop.

ok jsing

ssl_tlsext: fix uninitialized variable warning with gcc

This is a false positive but as is well-known, gcc is terrible at
understanding conditionally initialized variables and it is tedious
to expla

ssl_tlsext: fix uninitialized variable warning with gcc

This is a false positive but as is well-known, gcc is terrible at
understanding conditionally initialized variables and it is tedious
to explain this to downstream maintainers who look at warnings.

ok miod

show more ...

Fix key share negotiation in HRR case

In the ClientHello retrying the handshake after a HelloRetryRequest, the
client must send a single key share matching the group selected by the
server in the HR

Fix key share negotiation in HRR case

In the ClientHello retrying the handshake after a HelloRetryRequest, the
client must send a single key share matching the group selected by the
server in the HRR. This is not necessarily the mutually preferred group.
Incorrect logic added in ssl_tlsect.c r1.134 would potentially reject
such a key share because of that.

Instead, add logic to ensure on the server side that there is a single
share matching the group we selected in the HRR.

Fixes a regress test in p5-IO-Socket-SSL where server is configured
with P-521:P-384 and the client with P-256:P-384:P-521. Since the
client sends an initial P-256 key share, a HRR is triggered which
the faulty logic rejected because it was not the mutually preferred
P-384 but rather matching the server-selected P-521.

This will need some deduplication in subsequent commits. We may also
want to consider honoring the mutual preference and request a key
accordingly in the HRR.

reported by bluhm, fix suggested by jsing
ok beck jsing

show more ...

Recommit a better version of the removal of the F5 workaround

Unlike for previous TLS versions, TLSv1.3 servers can send the supported
groups extension to inform a client of the server's preferences

Recommit a better version of the removal of the F5 workaround

Unlike for previous TLS versions, TLSv1.3 servers can send the supported
groups extension to inform a client of the server's preferences. The
intention is that a client can adapt for subsequent commits. We ignore
this info for now, but sthen ran into java-based servers that do this.

Thus, rejecting the extension outright was incorrect. Instead, only allow
the extension in TLSv1.3 encrypted extensions. This way the F5 workaround
is also disabled, but we continue to interoperate with TLSv1.3 servers that
do follow the last paragraph of RFC 8446, section 4.2.7.

This mostly adjusts outdated/misleading comments.

ok jsing sthen

show more ...

Backout previous commit (intending that libressl client rejects a supported
groups extension from the server). It triggers 'CONNECT_CR_SRVR_HELLO:tlsv1
alert decode error' when connecting to a (moder

Backout previous commit (intending that libressl client rejects a supported
groups extension from the server). It triggers 'CONNECT_CR_SRVR_HELLO:tlsv1
alert decode error' when connecting to a (modern) java server (tomcat 10.1.18
on openjdk 17.0.10).

"please revert" tb@

show more ...

Stop pandering to the loadbalancer industrial complex.

So we initially kept this hack around for f5 boxes that
should have been patched in 2014, and were not as of 2017.

The f5 article for the bug

Stop pandering to the loadbalancer industrial complex.

So we initially kept this hack around for f5 boxes that
should have been patched in 2014, and were not as of 2017.

The f5 article for the bug archived on their web site,
and any of these devices on the public internet will have
since been upgraded to deal with a host of record layer, TLS,
and other bugs, or they likely won't be talking to modern
stacks, since as of this point the software with the bug
would not have been updated in 10 years.

So just make this spec compliant and reject a supported groups
extension that should not have been sent by a server.

ok tb@ jsing@

show more ...

Fix up server processing of key shares.

Ensure that the client can not provide a duplicate key share
for any group, or send more key shares than groups they support.

Ensure that the key shares must

Fix up server processing of key shares.

Ensure that the client can not provide a duplicate key share
for any group, or send more key shares than groups they support.

Ensure that the key shares must be provided in the same order
as the client preference order specified in supported_groups.

Ensure we only will choose to use a key share that is for the
most preferred group by the client that we also support,
to avoid the client being downgraded by sending a less preferred
key share. If we do not end up with a key share for the most preferred
mutually supported group, will then do a hello retry request
selecting that group.

Add regress for this to regress/tlsext/tlsexttest.c

ok jsing@

show more ...

Do not allow duplicate groups in supported groups.

While we are here refactor this to single return.

ok jsing@ tb@

Add an indicator that an extension has been processed.

ok jsing@

Process supported groups before key share.

This will allow us to know the client preferences for an upcoming
change to key share processing.

ok jsing@

Simplify TLS extension parsing and processing.

Rather than having a separate parse and process step for each TLS
extension, do a first pass that parses all of the TLS outer extensions and
retains th

Simplify TLS extension parsing and processing.

Rather than having a separate parse and process step for each TLS
extension, do a first pass that parses all of the TLS outer extensions and
retains the extension data, before running a second pass that calls the TLS
extension processing code.

ok beck@ tb@

show more ...

Fix typo msg_types -> msg_type

from jsing

Split TLS extension parsing from processing.

The TLS extension parsing and processing order is currently dependent on
the order of the extensions in the handshake message. This means that the
proces

Split TLS extension parsing from processing.

The TLS extension parsing and processing order is currently dependent on
the order of the extensions in the handshake message. This means that the
processing order (and callback order) is not under our control. Split the
parsing from the processing such that the processing (and callbacks) are
run in a defined order.

Convert ALPN to the new model - other extensions will be split into
separate parse/process in following diffs.

ok beck@ tb@

show more ...

Decouple TLS extension table order from tlsext_randomize_build_order()

The PSK extension must be the last extension in the client hello. This is
currently implemented by relying on the fact that it

Decouple TLS extension table order from tlsext_randomize_build_order()

The PSK extension must be the last extension in the client hello. This is
currently implemented by relying on the fact that it is the last extension
in the TLS extension table. Remove this dependency so that we can reorder
the table as needed.

ok tb@

show more ...

Too many stupid things whine about these being used uninitialized
(which they aren't), so appease them.

ssl_tlsext.c: Add an accessor for the tls extension type.

Needed for the tlsexttest.c

ok jsing

Fix allocation size

Reported by anton

Free and calloc() the tlsext_build_order and remember its length

Aligns tlsext_randomize_build_order() with tlsext_linearize_build_order()
and will help regression testing.

ok jsing

Use TLSEXT_TYPE_alpn instead of the stupid long one

Randomize the order of TLS extensions

On creation of an SSL using SSL_new(), randomize the order in which the
extensions will be sent. There are several constraints: the PSK extension
must always co

Randomize the order of TLS extensions

On creation of an SSL using SSL_new(), randomize the order in which the
extensions will be sent. There are several constraints: the PSK extension
must always come last. The order cannot be randomized on a per-message
basis as the strict interpretation of the standard chosen in the CH hashing
doesn't allow changing the order between first and second ClientHello.

Another constraint is that the current code calls callbacks directly on
parsing an extension, which means that the order callbacks are called
depends on the order in which the peer sent the extensions. This results
in breaking apache-httpd setups using virtual hosts with full ranomization
because virtual hosts don't work if the SNI is unknown at the time the
ALPN callback is called. So for the time being, we ensure that SNI always
precedes ALPN to avoid issues until this issue is fixed.

This is based on an idea by David Benjamin
https://boringssl-review.googlesource.com/c/boringssl/+/48045

Input & ok jsing

show more ...

Make internal header file names consistent

Libcrypto currently has a mess of *_lcl.h, *_locl.h, and *_local.h names
used for internal headers. Move all these headers we inherited from
OpenSSL to *_l

Make internal header file names consistent

Libcrypto currently has a mess of *_lcl.h, *_locl.h, and *_local.h names
used for internal headers. Move all these headers we inherited from
OpenSSL to *_local.h, reserving the name *_internal.h for our own code.
Similarly, move dtls_locl.h and ssl_locl.h to dtls_local and ssl_local.h.
constant_time_locl.h is moved to constant_time.h since it's special.

Adjust all .c files in libcrypto, libssl and regress.

The diff is mechanical with the exception of tls13_quic.c, where
#include <ssl_locl.h> was fixed manually.

discussed with jsing,
no objection bcook

show more ...

Get rid of SSL_CTX_INTERNAL and SSL_INTERNAL.

These are no longer necessary due to SSL_CTX and SSL now being fully
opaque. Merge SSL_CTX_INTERNAL back into SSL_CTX and SSL_INTERNAL back
into SSL.

P

Get rid of SSL_CTX_INTERNAL and SSL_INTERNAL.

These are no longer necessary due to SSL_CTX and SSL now being fully
opaque. Merge SSL_CTX_INTERNAL back into SSL_CTX and SSL_INTERNAL back
into SSL.

Prompted by tb@

show more ...