| Commit message (Collapse) | Author | Files | Lines |
|---|
|
|
|
|
|
|
|
kirill reported that his nginx reverse proxy setup stopped working
with x509_verify.c r1.74 and r1.75. It turns out that nginx relies
on a verify callback that always returns 1.
In revision 1.74 we removed the possibility of the verify_cb() to
override X509_V_ERR_CERT_CHAIN_TOO_LONG, which is what breaks the
config in kirill's setup since it used to use the nginx default of
setting the depth to 1. Re-enable this to make the new scenario
"2a with depth 1 and depth callback" pass.
As shown by the other new test scenario "14b with yolo calback"
with a "just say yes" cb, the guard added in r1.74 still prevents
the overwrite.
This makes kirill's reproducer work as verified by kirill and myself.
It was also tested by kirill in the real life setup.
discussed with beck
ok jsing kenjiro
|
|
Add three more test variants for scenario 2a:
1) verify that a chain of length 3 validates with depth 2.
2) verify that a chain of length 3 fails to validate with depth 1.
3) verify that a chain of length 3 validates with depth 1 if we
allow the callback to override the depth.
Variant 3) fails in -current and reproduces a scenario reported
by kirill.
Also add two test variants for the scenarios in 14:
4): run the chain of length 32 with a yolo callback returning 1
5): run the chain of length 33 with a yolo callback returning 1
Test 5) fails because we currently bail out at the wrong depth.
The verify callback should allow overriding the failure and will
then hit the bounds check added in x509_verify.c r1.74 to avoid
an overwrite.
Reuse the existing test cases 2a and 14a/14b for this and add an
optional vct->desc that uniquely identifies the test case.
incorporates various feedback from jsing
|
|
This is pretty ugly and probably the the vct should be handed down to
the verify_cert*() functions, but this works and doesn't make these
tests any uglier than they already are.
The callback regress was modified with a least effort approach.
|
|
|
|
If the call to dtls1_write_bytes() fails, handle the potential MTU update
and return/continue, which allows for the remainder to be moved out of an
else statement.
ok kenjiro@ tb@
|
|
dtls1_do_write() is currently a single function that handles both handshake
messages and CCS. This is a strange mix that only serves to complicate the
code - handshake messages have their own headers and may need to be
fragmented, while CCS must be sent verbatim (and only contain a single
byte). Pull the CCS part out into a separate function, simplifying the
code. By definition, when sending a CCS message the MTU will already be
set appropriately.
ok kenjiro@ tb@
|
|
dtls1_retransmit_buffered_messages() is iterating over the sent_messages
pqueue, only to pass dtls1_retransmit_message() a sequence number that it
turns back into a priority, to then do a lookup on the sent_messages pqueue.
This is pointless given that we already have the message that we need to
retransmit - just pass that to dtls1_retransmit_message() directly.
ok kenjiro@ tb@
|
|
ok kenjiro@ tb@
|
|
This function is only called from dtls1_retransmit_buffered_messages().
Make it static and move it above the caller.
ok kenjiro@ tb@
|
|
This is only used in one place and it makes no sense to have it as a
separate function. Furthermore, pull up an assertion so that we check
before assigning frag_len.
ok kenjiro@ tb@
|
|
ok kenjiro@ tb@
|
|
X509v3_addr_add_range() requires that min and max of an address range
have network encoding. In the RFC 3779 encoding of an actual address
range (as opposed to a prefix) as a SEQUENCE OF two ASN.1 BIT STRINGs,
the trailing one bits of the maximum become unused bits and therefore
must be DER encoded as zeroes. The DER encoder will clear them via i2d
but these trailing ones are annoying. Make a copy in which the unused
bits are cleared.
ok kenjiro
|
|
In both these functions, if the X509_ATTRIBUTE_dup() fails, the
remainder of the sk stack is shared with p7si->{un,}auth_attr and
the caller will likely end up freeing it twice.
Fix this by writing another sk_deep_copy() patterned after the existing
ones in x509_lu.c and x509_vpm.c. PKCS7_set_{un,}signed_attributes()
become trivial wrappers of that.
ok jsing kenjiro
|
|
Add tests that ensure the wire bytes for DTLS are what we expect for both
CCS and fragmented handshake messages.
|
|
ok jsing kenjiro
|
|
Use nid for NIDs and use i only for for loops.
ok jsing kenjiro
|
|
There's no need to assign to i before the switch and j is a terrible
name for a NID. Inline the latter and switch directly over the return
value of OBJ_obj2nid().
ok jsing kenjiro
|
|
We can switch over the return value of OBJ_obj2nid() rather than using i
for an indirection.
ok jsing kenjiro
|
|
Remove unnecessary isOther and nid variables and use direct returns.
The function should probably be removed...
ok jsing kenjiro
|
|
|
|
The i variable is unused. Likewise for the first assignment to ri.
Instead of an incomplete check that idx is in range, which still
results in a NULL deref if idx < 0, check if ri is not NULL before
accessing, as sk_value() checks the index correctly.
ok jsing kenjiro
|
|
|
|
|
|
patch from portable
|
|
from bcook kenjiro
|
|
|
|
Use tls_set_errorx() or tls_error_setx() rather than the versions without
x for TLS_ERROR_OUT_OF_MEMORY. ENOMEM adds no further info.
From Michael Forney
ok bcook
|
|
tls_config_load_file() hat a spot that used TLS_ERROR_UNKNOWN, so switch
that to the usual error code. Use tls_error_setx() since strerror(ENOMEM)
adds nothing.
From Michael Forney
ok bcook
|
|
This is the only place where tls_error_set() was used. While the new length
check now guarantees that the failure is due to ENOMEM, this info does not
add value.
From Michael Forney
ok bcook
|
|
If a check fails and errno is not necessarily set by the previous API call
use tls_set_errorx() or tls_error_setx() since turning an unrelated errno
into an error string is unhelpful.
From Michael Forney
ok bcook
|
|
Like all proper libcrypto APIs, BIO_new_mem_buf() takes an int as a length
argument. Check the size_t passed in to be at most INT_MAX to avoid issues
with truncation and overflow like it's done everywhere else. After release
this should probably be clamped down further since legitimate files (certs
and keys) are nowhere near this large.
Prompted by a diff by Michael Forney
ok jsing
|
|
Otherwise tls_error_clear() (called e.g. via tls_error_vset()) will
free the bad error->msg pointer.
From Michael Forney
|
|
on a SAN DNSname constraint. This is not correct, as with
a DNSname constraint, it may exacly match or match zero or
more additional components on the front of the candidte to
match.
Spotted by Haruto Kimura <hkimura2026@gmail.com>
ok tb@ kenjiro@
|
|
ok deraadt
|
|
Requested by jsing, ok beck
|
|
This converts rsa_cms_decrypt() to use X509_ALGOR_get0() and fixes a
NULL deref when a parameter is (invalidly) omitted similar to the fix
in ec/ec_ameth.c r1.66 from a couple years back. There is currently
an XXX annotating a hairy leak due to trying to be smart and stealing
the parameters from the oaep object. Instead, just make a copy of the
label string and free it in the exit path.
The diff adds an error for labellen == 0 since that is an invalid
encoding of pSpecifiedEmpty (see RFC 8017) -- per the DER the default
must be omitted. This way we avoid a malloc(0) implementation-defined
behavior.
This minor issue was assigned CVE-2026-28390 by OpenSSL and was reported
by too many to list. The fix is my own. It is similar to OpenSSL's fix
only because I rewiewed theirs and suggested an improvement or two.
This is the last of the "security fixes" in today's OpenSSL release that
"affect" LibreSSL. All the other bugs were already fixed a few years back
or we didn't have the code/bugs in the first place.
ok beck jsing
|
|
Instead of reaching deep inside the OCSP_BASICRESP and ignoring its
semantics and then try to untangle things in ocsp_find_signer_sk(),
pass the OCSP_BASICRESP and use OCSP_resp_get0_id() which has the
logic built in. Avoids a crash if you call OCSP_basic_verify() after
OCSP_BASICRESP_new() without OCSP_basic_sign(). This cannot happen on
a deserialized OCSP object.
Prompted by a report by Kamil Frankowicz, Jan Kaminski, Bartosz Michalowski.
ok jsing
|
|
Prompted by the "fix" fighting symptoms of misdesign in Delta CRL processing
rather than addressing the root cause. Probably the best fix is to remove
support for Indirect CRLs and Delta CRLs outright.
ok jsing
|
|
A malformed Delta CRL could cause a crash. Funnily enough the deserializer
recognizes this and marks such a CRL as invalid, but nothing ever checks
the EXFLAG_INVALID for CRLs. For certificates this would usually result in
verification failure due to x509v3_cache_extensions() failing.
This is only reachable if the X509_V_FLAG_USE_DELTAS is used, which only
a handful of ports do, plus openssl(1) does if you use the undocumented
-use_deltas flag.
Reported by Igor Morgenstern to OpenSSL who then sat on this since Jan 8
and assigned CVE-2026-28388.
ok jsing
|
|
x509v3.h has included conf.h since June 20, 1999, OpenSSL commit ba404b5e,
so HEADER_CONF_H has been defined since then. Also since then, CONF_VALUE
(only available via conf.h) has been used outside of HEADER_CONF_H, making
that #ifdef doubly pointless.
ok bcook jsing kenjiro
|
|
All thirteen files including cms_local.h do that after including cms.h,
which already includes x509v3.h, so this is always defined. While here
make the cms_local.h a bit more selfstanding by including asn1.h and
x509v3.h
ok bcook jsing (who had the same diff) kenjiro
|
|
This is all unhelpful historical cruft.
Discussed with tb@
|
|
We no longer support TLSv1.0 and definitely do not support SSLv3 - remove
the empty fragments workaround for the CBC vulnerability in these
protocols.
ok kenjiro@ tb@
|
|
TLS versions prior to TLSv1.2 were disabled a while ago, however this
was done in the version handling code. Remove TLSv1.1 and earlier from
ssl_get_method() and add an explicit min version check in the legacy
client and server, to provide a stronger guarantee.
ok kenjiro@ tb@
|
|
|
|
|
|
This has not been reachable since we made the TLSv1.3 stack the default
entry point - tls13_record_layer_read_record() will send a protocol
version alert and raise an error, which means we never transition into
the legacy stack.
ok kenjiro@
|
