Reinstate bounds check accidentally disabled when defining OPENSSL_NO_DTLS1

From Kenjiro Nakayama
Closes https://github.com/libressl/portable/issues/1097

Prepare to provide SSL_CTX_set1_cert_store()

SSL_CTX_set_cert_store() should have been called SSL_CTX_set0_cert_store()
since it takes ownership of the store argument. Apparently a few people ran
in

Prepare to provide SSL_CTX_set1_cert_store()

SSL_CTX_set_cert_store() should have been called SSL_CTX_set0_cert_store()
since it takes ownership of the store argument. Apparently a few people ran
into the issue of not bumping the refcount themselves, leading to use after
frees about 10 years ago. This is a quite rarely used API and there are no
misuses in the ports tree, but since someone did the work of writing a diff,
we can still add it.

Needless to say that SSL_CTX_get_cert_store() obviously has the exact same
issue and nobody seems to have thought of adding a get0 or get1 version to
match...

Fixes https://github.com/libressl/openbsd/issues/71
From Kenjiro Nakayama

show more ...

Remove cipher from SSL_SESSION.

For a long time SSL_SESSION has had both a cipher ID and a pointer to
an SSL_CIPHER (and not both are guaranteed to be populated). There is also
a pointer to an SSL_C

Remove cipher from SSL_SESSION.

For a long time SSL_SESSION has had both a cipher ID and a pointer to
an SSL_CIPHER (and not both are guaranteed to be populated). There is also
a pointer to an SSL_CIPHER in the SSL_HANDSHAKE that denotes the cipher
being used for this connection. Some code has been using the cipher from
SSL_SESSION and some code has been using the cipher from SSL_HANDSHAKE.

Remove cipher from SSL_SESSION and use the version in SSL_HANDSHAKE
everywhere. If resuming from a session then we need to use the SSL_SESSION
cipher ID to set the SSL_HANDSHAKE cipher. And we still need to ensure that
we update the cipher ID in the SSL_SESSION whenever the SSL_HANDSHAKE
cipher changes (this only occurs in a few places).

ok tb@

show more ...

Move client ciphers from SSL_SESSION to SSL_HANDSHAKE.

SSL_SESSION has a 'ciphers' member which contains a list of ciphers
that were advertised by the client. Move this from SSL_SESSION to
SSL_HANDS

Move client ciphers from SSL_SESSION to SSL_HANDSHAKE.

SSL_SESSION has a 'ciphers' member which contains a list of ciphers
that were advertised by the client. Move this from SSL_SESSION to
SSL_HANDSHAKE and rename it to match reality.

ok tb@

show more ...

Follow BoringSSL's nomenclature in SSL_select_next_proto()

SSL_select_next_poto() was written with NPN in mind. NPN has a weird
fallback mechanism which is baked into the API. This is makes no sense

Follow BoringSSL's nomenclature in SSL_select_next_proto()

SSL_select_next_poto() was written with NPN in mind. NPN has a weird
fallback mechanism which is baked into the API. This is makes no sense
for ALPN, where the API behavior is undesirable since it a server
should not end up choosing a protocol it doesn't (want to) support.
Arguably, ALPN should simply have had its own API for protocol selection
supporting the proper semantics, instead of shoehorning an NPN API into
working for ALPN.

Commit https://boringssl-review.googlesource.com/c/boringssl/+/17206/
renamed the arguments to work for both NPN and ALPN, with the slight
downside of honoring client preference instead of the SHOULD in
RFC 7301, section 3.2. This grates for most consumers in the wild,
but so be it. The behavior is saner and safer.

discussed with davidben
ok beck

show more ...

fix typo

Fix SSL_select_next_proto()

SSL_select_next_proto() is already quite broken by its design: const in,
non-const out, with the intention of pointing somewhere inside of the two
input pointers. A lengt

Fix SSL_select_next_proto()

SSL_select_next_proto() is already quite broken by its design: const in,
non-const out, with the intention of pointing somewhere inside of the two
input pointers. A length returned in an unsigned char (because, you know,
the individual protocols are encoded in Pascal strings). Can't signal
uailure either. It also has an unreachable public return code.

Also, due to originally catering to NPN, this function opportunistically
selects a protocol from the second input (client) parameters, which makes
little sense for ALPN since that means the server falls back to a protocol
it doesn't (want to) support. If there's no overlap, it's the callback's
job to signal error to its caller for ALPN.

As if that wasn't enough misdesign and bugs, the one we're concerned with
here wasn't reported to us twice in ten years is that if you pass this API
a zero-length (or a sufficiently malformed client protocol list), it would
return a pointer pointing somewhere into the heap instead into one of the
two input pointers. This pointer could then be interpreted as a Pascal
string, resulting in an information disclosure of up to 255 bytes from the
heap to the peer, or a crash.

This can only happen for NPN (where it does happen in old python and node).

A long time ago jsing removed NPN support from LibreSSL, because it had
an utter garbage implementation and because it was practically unused.
First it was already replaced by the somewhat less bad ALPN, and the only
users were the always same language bindings that tend to use every feature
they shouldn't use. There were a lot of complaints due to failing test
cases in there, but in the end the decision turned out to be the right
one: the consequence is that LibreSSL isn't vulnerable to CVE-2024-5535.

Still, there is a bug here to fix. It is completely straightforward to
do so. Rewrite this mess using CBS, preserving the current behavior.
Also, we do not follow BoringSSL's renaming of the variables. It would
result in confusing code in almost all alpn callbacks I've seen in the
wild. The only exception is the accidental example of Qt.

ok jsing

show more ...

Unexport SSL_version_str

ok jsing

Unify up_ref implementations in libssl

ok jsing

Remove SSL_debug

The garbage truck is quite full by now. Collect the last symbol
straggler for this bump.

ok jsing

Of course libssl also has a few missing void

From Christian Andersen

Remove GOST and STREEBOG support from libssl.

This version of GOST is old and not anywhere close to compliant with
modern GOST standards. It is also very intrusive in libssl and
makes a mess everywh

Remove GOST and STREEBOG support from libssl.

This version of GOST is old and not anywhere close to compliant with
modern GOST standards. It is also very intrusive in libssl and
makes a mess everywhere. Efforts to entice a suitably minded anyone
to care about it have been unsuccessful.

At this point it is probably best to remove this, and if someone
ever showed up who truly needed a working version, it should be
a clean implementation from scratch, and have it use something
closer to the typical API in libcrypto so it would integrate less
painfully here.

This removes it from libssl in preparation for it's removal from
libcrypto with a future major bump

ok tb@

show more ...

Neuter the SSL_set_debug(3) API

The TLSv1.3 stack didn't support this in the first place, and in the legacy
stack it only added some dubious BIO_flush(3) calls. The sleep call between
SSL_read(3) an

Neuter the SSL_set_debug(3) API

The TLSv1.3 stack didn't support this in the first place, and in the legacy
stack it only added some dubious BIO_flush(3) calls. The sleep call between
SSL_read(3) and SSL_write(3) advertised in the comment next to the flag has
been a sleep call in the s_server since time immemorial, nota bene between
calls to BIO_gets(3). Anyway. This can all go and what remains will go with
the next major bump.

ok jsing

show more ...

Convert ssl3_cipher_by_id() to bsearch()

This was previously the only user of OBJ_bsearch_ssl_cipher_id(), which
in turn is the one remaining user of OBJ_bsearch_() outside of libcrypto.
OBJ_bsearch

Convert ssl3_cipher_by_id() to bsearch()

This was previously the only user of OBJ_bsearch_ssl_cipher_id(), which
in turn is the one remaining user of OBJ_bsearch_() outside of libcrypto.
OBJ_bsearch_() is OpenSSL's idiosyncratic reimplementation of ANSI C89's
bsearch(). Since this used to be hidden behind macro insanity, the result
was three inscrutable layers of comparison functions.

It is much simpler and cleaner to use the standard API. Move all the code
to s3_lib.c, since it's ony used there.

In a few further diffs, OBJ_bsearch_() will be removed from libcrypto.
Unfortunately, we'll need to keep OBJ_bsearch_ex(), because it is
exposed via sk_find_ex(), which is exposed by M2Crypto...

ok jsing

show more ...

Move ssl_cipher_id_cmp() next to its only caller

It was left alone and forlorn in the middle of other nonsense. Since there
is only one caller (the OBJ_bsearch_ stupidity), it can be static and ther

Move ssl_cipher_id_cmp() next to its only caller

It was left alone and forlorn in the middle of other nonsense. Since there
is only one caller (the OBJ_bsearch_ stupidity), it can be static and there
is no need to prototype it in ssl_local.h.

show more ...

Unifdef OPENSSL_NO_ENGINE in libssl

As usual, a few manual fixes to avoid duplicate lines.

ok jsing

More 0/NULL confusions in SSL_CTX_new()

Fix some NULL/0 misspellings in SSL_CTX_new()

Fix two aliases in libcrypto spotted by the new symbols test

ok jsing

Hide all public symbols in libssl

With the guentherizer 9000

ok tb@

Move verified_chain from SSL to SSL_HANDSHAKE

This is a better version of the fix for the missing pointer invalidation
but a bit larger, so errata got the minimal fix.

tested by jcs
ok jsing

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 ...

Rewrite TLSv1.2 key exporter.

Replace the grotty TLSv1.2 key exporter with a cleaner version that uses
CBB and CBS.

ok tb@

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 ...