Age | Commit message (Collapse) | Author |
|
|
|
This switches to using the X509_get_signature_info() API instead of hand
rolling a part of it. This is slightly tangly since the security level API
is strange. In particular, some failures are passed to the security level
callback so that applications can override them.
This makes the security level API handle RSA-PSS and EdDSA certificates
correctly and the handshake with such can progress a bit further. Of note,
we check that the certs are actually suitable for use in TLS per RFC 8446
contrary to what OpenSSL does.
ok beck jsing
|
|
|
|
Drop about 1/4 of the lines in here by avoiding the use of a variable.
For some reason the API in this file made me go listen to Pow R. Toc H.
All of a sudden the lyrics made sense. I should probably be worried.
ok beck jsing
|
|
Nothing uses this and it collides with ALPN
|
|
This is so gross...
|
|
ok beck
|
|
For every TLS alert there needs a corresponding error with error code
having an offset of SSL_AD_REASON_OFFSET (aka 1000), otherwise the error
stack fails to set the reason correctly.
ok beck
|
|
From Kenjiro Nakayama
Closes https://github.com/libressl/portable/issues/1097
|
|
We accidentally have two errors 235 since we didn't notice that OpenSSL
removed the unused SSL_R_TRIED_TO_USE_UNSUPPORTED_CIPHER and later that
becamse SSL_R_NO_APPLICATION_PROTOCOL. Getting an "unsupported cipher"
error when fiddling with ALPN is confusing, so fix that.
ok jsing
|
|
Only close_notify and user_cancelled are warning alerts. All others
should be fatal. In order for the lower layers to behave correctly,
the return code for fatal alerts needs to be TLS13_IO_ALERT instead
of TLS13_IO_SUCCESS.
Failure to signal handshake failure in the public API led to a crash
in HAProxy when forcing the tls cipher to TLS_AES_128_CCM_SHA256 as
found by haproxyfred while investigating
https://github.com/haproxy/haproxy/issues/2569
Kenjiro Nakayama found misbehavior of ngtcp2-based servers, wrote a
similar patch and tested this version.
Fixes https://github.com/libressl/portable/issues/1093
ok jsing
|
|
This is a small refactoring that wraps a direct call to the record layer's
alert_sent() callback into a handler for upcoming reuse in the QUIC code.
No functional change.
ok jsing
|
|
ok tb@
|
|
|
|
|
|
|
|
Symbols.list
|
|
|
|
|
|
From Kenjiro Nakayama
|
|
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
|
|
Inline the get_cipher implementation (including the special handling
for DTLS) in ssl_cipher_collect_ciphers() (the only consumer), remove
the get_cipher member of SSL_METHOD and mop up dtls1_get_cipher().
ssl3_get_cipher() has always had a strange property of being a reverse
index, which is relied on by the cipher list ordering code, since it
currently assumes that high cipher suite values are preferable. Rather
than complicating ssl3_get_cipher() (and regress), change the iteration
order in ssl_cipher_collect_ciphers() to match what it requires. Lastly,
rename ssl3_get_cipher() to be more descriptive.
ok tb@
|
|
OpenSSL has had the concept of cipher IDs, which were a way of working
around overlapping cipher suite values between SSLv2 and SSLv3. Given
that we no longer have to deal with this issue, replace the use of IDs
with cipher suite values. In particular, this means that we can stop
mapping back and forth between the two, simplifying things considerably.
While here, remove the 'valid' member of the SSL_CIPHER. The ssl3_ciphers[]
table is no longer mutable, meaning that ciphers cannot be disabled at
runtime (and we have `#if 0' if we want to do it at compile time).
Clean up the comments and add/update RFC references for cipher suites.
ok tb@
|
|
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@
|
|
|
|
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@
|
|
The handshake MAC needs to be upgraded when TLSv1.0 and TLSv1.1
ciphersuites are used with TLSv1.2. Since we no longer support TLSv1.0
and TLSv1.1, we can simply upgrade the handshake MAC in the ciphersuite
table and remove the various defines/macros/code that existed to handle
the upgrade.
ok tb@
|
|
|
|
These have not been used for a long time, however SSL_CIPHER was not opaque
at the time, hence they had to stick around. Now that SSL_CIPHER is opaque
we can simply mop them up.
ok tb@
|
|
|
|
h/t to levitte
|
|
|
|
Needed by newer freeradius. This is a straightforward implementation that
essentially duplicates tls13_cipher_hash().
ok jsing
|
|
Now that the SSL2 client hello support is gone, nothing uses this anymore,
except that a few ports still need SSL2_VERSION.
ok beck
|
|
This could be made cleaner if we expose ERR_load_const_strings(), but for
now this hackier version with casts achieves the same and removes the last
unprotected modifiable globals in this library.
ok jsing
|
|
different asm stanzas to produce strong aliases.
This unbreaks libssl on hppa after the recent switch to LIBRESSL_NAMESPACE.
|
|
Use better argument names, add a link to the relevant standards and add
CAVEATS and BUGS sections pointing out a few pitfalls.
discussed with davidben
ok beck
|
|
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
|
|
Doing so breaks certificate selection if a TLS 1.3 client does not support
EC certs, and needs to fall back to RSA.
ok tb@
|
|
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@
|
|
(instead of commiting only one part)
|
|
|
|
no overlap. Document that explicitly. Also make it more explicit that
that the caller must work with a copy of out.
ok jsing
|
|
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
|
|
This code was only previously enabled if the minimum enabled version was
TLSv1.0 and a non-version locked method is in use. Since TLSv1.0 and
TLSv1.1 were disabled nearly a year ago, this code is no longer ever
being used.
ok tb@
|
|
ok jsing
|
|
suggested by jsing
|
|
RSA key exchange is known to have multiple security weaknesses,
including being potentially susceptible to padding oracle and timing
attacks.
The RSA key exchange code that we inherited from OpenSSL was riddled
with timing leaks, many of which we fixed (or minimised) early on.
However, a number of issues still remained, particularly those
related to libcrypto's RSA decryption and padding checks.
Rework the RSA key exchange code such that we decrypt with
RSA_NO_PADDING and then check the padding ourselves in constant
time. In this case, the pre-master secret is of a known length,
hence the padding is also a known length based on the size of the
RSA key. This makes it easy to implement a check that is much safer
than having RSA_private_decrypt() depad for us.
Regardless, we still strongly recommend disabling RSA key exchange
and using other key exchange methods that provide perfect forward
secrecy and do not depend on client generated keys.
Thanks to Marcel Maehren, Nurullah Erinola, Robert Merget, Juraj
Somorovsky, Joerg Schwenk and Hubert Kario for raising these issues
with us at various points in time.
ok tb@
|
|
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
|
|
|