| Commit message (Collapse) | Author | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
With the introduction of a secondary key share, we fail to ensure that the
HelloRetryRequest does not specify the group that was used for the
secondary key share. We also fail to free the secondary key share early in
this case, meaning that it lingers in memory until the SSL is reset or
freed. Fix both of these issues.
ok tb@
|
|
While we currently correctly handle the no-shared-group case, it currently
fails late when we try to create the key share. Improve detection and
handling so that we fail sooner and send an alert to the client when
processing client key shares.
While here rename preferred_group_found to shared_group_found - we look for
the client preferred group, but any group that we select will always be in
the client list (even if it's the last one).
Reported by the tlspuffin team.
ok tb@
|
|
Be more RFC compliant and send illegal parameter alerts when the client
receives a HelloRetryRequest that requests a group that we did not offer
or a group that we sent a key share for in the ClientHello. These were
annotated as missing, but not previously implemented.
Prompted by a report from the tlspuffin team.
ok tb@
|
|
Include coverage of Renegotiation Indication and legacy connection
handling.
|
|
SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS is now a no-op, tidy up
SSL_OP_LEGACY_SERVER_CONNECT and reflect the current state of SSL_OP_ALL
Delete the entire "SECURE RENEGOTIATION" section that contained ancient
ramblings.
ok beck@ tb@
|
|
Remove SSL_OP_LEGACY_SERVER_CONNECT from the default SSL options and the
SSL_OP_ALL define. This means that we will now refuse to connect to a
TLSv1.2 server if it does not support the Renegotiation Indication (RI)
extension. This prevents a class of attacks against TLS clients that are
talking to TLSv1.2-only servers that permit client initiated renegotiation.
Raised by Lucca Hirschi et al from Inria.
ok beck@ tb@
|
|
|
|
If a purpose is configured on the verify context (which it currently
never is), this check would only accept certificates for which the
purpose check fails. Also, this code is not currently reachable from
public API since x509_verify() is only ever called with a legacy xsc
set on the verify ctx, so x509_verify_ccert_extensions() takes the
path returning 1 earlier.
X509_check_purpose() is one of these strange legacy APIs. It returns -1
on error, 0 if the cert fails the purpose check, 1 if it passes it and
values between 2 and 5 indicate various legacy garbage meaning that the
cert might possibly have been fit for this purpose until around a quarter
century ago. While for CA certs the checks in "No we don't care about ..."
exclude return values > 1, it is still possible for the S/MIME purpose
to return 2 due to a workaround for some buggy NS certs, for example.
In short: anything but 1 means unfit for the purpose or at best dubious,
so reject such certs.
Reported by Frank Denis
ok kenjiro
|
|
If a PKCS#7 S/MIME message comes with an empty set of digestAlgorithms
in the SignedData, PKCS7_verify() would incorrectly free a caller-owned
buffer. Fix the freeing logic to avoid this situation.
From Igor Ustinov via OpenSSL
|
|
If the in string is unreasonably long, assigning strlen(in) to an int
may overflow, so exclude this situation.
Moreover, the code would unconditionally multiply nchar by 2 or 4,
which could again overflow an int. Check for this situation and error
out to avoid an out of bounds write.
More may be needed in here, which will be revisited later.
Based on a diff by Viktor Dukhovni via OpenSSL.
|
|
The RFC 3211 PWRI integrity check when unwrapping the password-derived key
accesses seven bytes from a heap-allocated buffer. If an (invalid) block
cipher with short blocks is in use 2 * blocksize may not be sufficient room
for 7 bytes. In that silly case, the function performs an OOB read. Add
length check to avoid this situation
From Igor Ustinov via OpenSSL.
|
|
The keyDerivationAlgorithm field is OPTIONAL in the ASN.1 but required
for password-based CMS. Check that the field is present before using it
to avoid a crash.
From Igor Ustinov via OpenSSL
|
|
The point at infinity would previously raise EC_R_POINT_AT_INFINITY via
EC_POINT_get_affine_coordinates(). For consistency, also raise an error
for off-curve points.
pointed out by/ok kenjiro
|
|
EC_POINT_is_at_infinity() has a Boolean return nowadays, so drop > 0.
ok jsing kenjiro
|
|
|
|
|
|
While we already check that the peer's public point is on the curve and
will reject the point at infinity when getting the affine coordinates,
doing this earlier avoids doing work with the private key in a clearly
invalid case.
Suggested by Lucca Hirschi et al.
[An EC_KEY_check_key() call was also suggested but this is a bit expensive
and punishes callers that do that or equivalent already (e.g. ssh)]
ok jsing kenjiro
|
|
ok jsing kenjiro
|
|
|
|
EC_POINT_oct2point() does most of the validation we need it to do, but it
has to accept the point at infinity, compressed and hybrid encodings for
historic reasons. So exclude these cases: the point at infinity makes no
sense here and will be caught later in ECDH_compute_key(), the compressed
and hybrid encodings MUST NOT be supported per RFC 8422 section 5.1.2.
This is implemented using the strategy already used in ec_convert.c since
the point_conversion_form_t is completely unfit for anything.
Set decode_error to ensure we send that alert. We may make some effort to
use illegal_parameter later.
Issue about the missing alert and the point at infinity raised by Lucca
Hirschi et al.
ok jsing kenjiro
|
|
|
|
ok jsing kenjiro
|
|
|
|
|
|
|
|
|
|
libtls clients to connect to servers which don't offer any other curves
(this has been seen in the wild with got, reported by stsp). ok tb@
depends on recent libssl changes (ssl_tlsext.c r1.161, t1_lib.c r1.209).
|
|
If we fail to buffer an outgoing DTLS message, we're not going to be able
to retransmit it. Correctly propagate the failure.
ok kenjiro@ tb@
|
|
When a TLSv1.2 change cipher spec message has been built, call a separate
function that can handle the DTLS specific processing rather than including
this in the TLS code.
ok kenjiro@ tb@
|
|
When a TLSv1.2 handshake message has been built, call a separate function
that can handle the DTLS specific processing rather than including this in
the TLS code.
ok kenjiro@ tb@
|
|
This function is only called in one location, which is always a DTLS
context - use DTLS1_HM_HEADER_LENGTH directly instead.
ok kenjiro@ tb@
|
|
If the list of 'groups' starts with X25519MLKEM768 for a TLSv1.2 server,
ssl3_send_server_kex_ecdhe() attempts to use NID_X25519MLKEM768, which
it receives from tls1_get_supported_groups(). This does not work because
it never received the peer's public keys, which causes an error return
from tls_key_share_server_generate_mlkem768x25519().
For a TLSv1.2-only client with custom supported group list we will
currently send ML-KEM if configured. We should not do this.
There is more to fix here: if a TLSv1.2 client is misconfigured with
only X25519MLKEM768, we should not send a supported groups extension
(with this commit we'll send an empty one, which is an RFC violation).
This commit simply filters X25519MLKEM768 out of the supported groups
list if we're configured to be TLSv1.2-only.
feedback/ok jsing kenjiro (on an earlier version)
|
|
|
|
discussed with miod
|
|
None of these functions modifies the SSL pointer. An upcoming diff will
need to call one of them with a const pointer, so fix all of them.
ok jsing kenjiro
|
|
|
|
|
|
Now that we no longer support TLSv1.0 and TLSv1.1, SSL_USE_SIGALGS()
is always true - remove all of the code that handles the non-sigalgs
path, along with SSL_USE_SIGALGS() and the related flags.
Also remove SIGALG_RSA_PKCS1_MD5_SHA1 and references to it, since this
is also now unused.
ok kenjiro@ tb@
|
|
Found by Frank Denis.
ok tb@
|
|
This currently matches when we should reject it.
|
|
The test may have matched when it should not have.
|
|
enabled will treat strlcpy(NULL, ...) as undefined behavior and optimize
the invocation away.
Regression introduced after the llvm 22 upgrade.
|
|
|
|
|
|
ok jsing@
|