| Age | Commit message (Collapse) | Author |
|---|
|
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@
|
|
ok jsing
|
|
suggested by jsing
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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@
|
|
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@
|
|
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@
|
|
While we are here refactor this to single return.
ok jsing@ tb@
|
|
ok jsing@
|
|
This will allow us to know the client preferences for an upcoming
change to key share processing.
ok jsing@
|
|
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@
|
|
from jsing
|
|
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@
|
|
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@
|
|
(which they aren't), so appease them.
|
|
Needed for the tlsexttest.c
ok jsing
|
|
Reported by anton
|
|
Aligns tlsext_randomize_build_order() with tlsext_linearize_build_order()
and will help regression testing.
ok jsing
|
|
|
|
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
|
|
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
|
|
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@
|
|
ok jsing
|
|
None of these functions are used outside of ssl_tlsext.c. The only reason
they are prototyped in the header is for the use of tlsexttest.c. Rather
than having a big pile of useless copy-paste in the header, we can adapt
the test to avoid using these functions directly.
ok jsing
|
|
Instead of setting the alert manually in various parse handlers, we can
make use of the fact that tlsext_parse() sets the alert to decode_error
by default. This simplifies the code quite a bit.
ok jsing
|
|
The main parsing function already checks that the entire extension data
was consumed, so the length checks inside some of the parse handlers are
redundant. They were also not done everywhere, so this makes the parse
handlers more consistent.
Similar diff was sent by jsing a long while back
ok jsing
|
|
Add an early return in the s->internal->hit case so that we can unindent
a lot of this code. In the HRR case, we do not need to check that the list
of supported groups is unmodified from the first CH. The CH extension
hashing already does that for us.
ok jsing
|
|
ok jsing
|
|
ok jsing
|
|
The ALPN extension must contain a non-empty list of protocol names.
Split a check of this out of tlsext_alpn_server_parse() so that it
can be reused elsewhere in the library.
ok jsing
|
|
Remove duplicate U16 length prefix, since tlsext_build() already adds this
for us. Condition on SSL_is_quic() rather than TLS version - RFC 9001 is
clear that this extension is only permitted on QUIC transport and an
fatal unsupported extension alert is required if used elsewhere.
Additionally, at the point where extensions are parsed, we do not
necessarily know what TLS version has been negotiated.
ok beck@ tb@
|
|
Per RFC 9001, TLSEXT_TYPE_quic_transport_parameters may only appear in
ClientHello and EncryptedExtensions (not ServerHello).
ok beck@ tb@
|
|
The API is ugly and we can easily abstract it away. The SSL_SECOP_* stuff
is now confined into ssl_seclevel.c and the rest of the library can make
use of the more straightforward wrappers, which makes it a lot easier on
the eyes.
ok beck jsing
|
|
This reworks various tls1_ curve APIs to indicate success via a boolean
return value and move the output to an out parameter. This makes the
caller code easier and more consistent.
Based on a suggestion by jsing
ok jsing
|
|
ok jsing
|
|
ok beck jsing
|
|
This is the start of adding the boringssl API for QUIC support,
and the TLS extensions necessary to send and receive QUIC transport
data.
Inspired by boringssl's https://boringssl-review.googlesource.com/24464
ok jsing@ tb@
|
|
ok beck jsing
|
|
Found by anton with tlsfuzzer
ok anton
|
|
ok jsing
|
|
ok jsing
|
|
S3I has served us well, however now that libssl is fully opaque it is time
to say goodbye. Aside from removing the calloc/free/memset, the rest is
mechanical sed.
ok inoguchi@ tb@
|
|
If the hostname is too long, tlsext_sni_is_valid_hostname() will fail
without having initialized *is_ip. As a result, the garbage value could
lead to accepting (but otherwise ignoring) overlong and possibly invalid
hostnames without erroring in tlsext_sni_server_parse().
ok inoguchi jsing
|
|
Distinguish between decode errors and other errors, so that we can send
a SSL_AD_DECODE_ERROR alert when appropriate.
Fixes a tlsfuzzer failure, due to it expecting a decode error alert and
not receiving one.
Prompted by anton@
ok tb@
|
|
ok tb@
|
|
SSL_AD_DECODE_ERROR is the default alert for a TLS extension parsing
failure - remove the various gotos and simply return 0 instead.
ok tb@
|
|
This requires adding DHE support to tls_key_share. In doing so,
tls_key_share_peer_public() has to lose the group argument and gains
an invalid_key argument. The one place that actually needs the group
check is tlsext_keyshare_client_parse(), so add code to do this.
ok inoguchi@ tb@
|
