| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
| |
find leaf cert issuers. This breaks perl and ruby regress, as noticed
by tb that "we tried this before".
Jan's regress that cares about 21 vs 20 needs to change
ok tb@
|
|
|
|
|
|
|
|
| |
then fix the only thing it still has complaints about which
is that we don't return the leaf version of the error code
when we can't verify the leaf (as opposed to the rest of the chain)
ok jan@ tb@
|
|
|
|
|
|
|
| |
This fixes a problem in the perl regress where it notices the
callback is called twice and complains.
ok tb@ bluhm@
|
|
|
|
|
|
|
|
| |
Due to the need to support by_dir, we use the get_issuer stuff when running
in x509_vfy compatibility mode amyway - so just use it any time we are
doing that. Removes a bunch of yukky stuff and a "Don't Look Ethel"
ok tb@ jsing@
|
|
|
|
|
|
|
| |
roots were not checked correctly before intermediates that has since been fixed
and is no longer necessary. It is regress checked by case 2c in
regress/lib/libcrypto/x509/verify.c
ok jsing@ tb@
|
|
|
|
|
|
|
|
| |
not necessarily NUL terminated). Same as schwarze's fix in t_x509a.c r1.9.
From David Benjamin and Matt Caswell (part of the fixes in OpenSSL 1.1.1l)
ok inoguchi
|
|
|
|
|
|
|
| |
to handly by_dir and fun things correctly. - fixes dlg@'s case and
by_dir regress in openssl-ruby
ok jsing@
|
|
|
|
|
|
|
|
|
| |
the result in order to return the same errors as OpenSSL users expect to override
the generic "Untrusted cert" error.
This fixes the openssl-ruby timestamp test.
ok tb@
|
|
|
|
|
|
| |
own function, in preparation for subesquent change. No functional change.
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Delete some code from X509_TRUST_cleanup(3) that had no effect:
it called a function on static objects that returns right away
unless the argument is dynamically allocated.
Pointed out by tb@.
This commit is identical to:
OpenSSL commit 5e6e650d62af09f47d63bfdd6c92e3b16e9da644
Author: Kurt Cancemi <kurt at x64architecture dot com>
Date: Thu Jun 9 21:57:36 2016 -0400
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
it called a function on static objects that returns right away
unless the argument is dynamically allocated.
OK jsing@ tb@
The useless code was independently discovered while writing documentation.
This commit is identical to:
OpenSSL commit fa3a0286d178eb3b87bf2eb5fd7af40f81453314
Author: Kurt Cancemi <kurt at x64architecture dot com>
Date: Wed Jun 8 19:15:38 2016 -0400
|
|
|
|
|
|
|
| |
calling the OpenSSL legacy cache extensions goo.
Requested by tb@
ok tb@
|
|
|
|
|
|
|
|
|
| |
the saving of the first error case so that the "autochain" craziness from
openssl will work with the new verifier. This should allow the new verification
code to work with a bunch of the autochain using cases in some software.
(and should allow us to stop using the legacy verifier with autochain)
ok tb@
|
|
|
|
|
|
|
|
| |
verifier." (r1.27). While this may have "fixed" one corner case, it
broke expectations of Perl Net::SSLeay and Ruby OpenSSL regression
tests.
ok bcook
|
|
|
|
|
|
|
|
|
|
|
| |
wildcards. While we may choose not to support them the standards
appear to permit them optionally so we can't declare a certificate
containing them invalid. Noticed by jeremy@, and Steffan Ulrich
and others. Modify the regression tests to test these cases and
not check the SAN DNSnames as "hostnames" anymore (which don't support
wildcards).
ok jsing@, tb@
|
|
|
|
|
|
| |
out in this release cycles.
discussed with deraadt and jsing
|
|
|
|
|
|
|
|
|
|
|
| |
This is disappointing as a lot of work was put into the new verifier
during this cycle. However, there are still too many known bugs and
incompatibilities. It is better to be faced with known broken behavior
than with new broken behavior and to switch now rather than via errata.
This way we have another cycle to iron out the kinks and to fix some of
the remaining bugs.
ok jsing
|
|
|
|
|
|
|
|
|
| |
For dynamically allocated verify parameters, param->name is only ever set
in X509_VERIFY_set1_name() where the old one is freed and the new one is
assigned via strdup(). Setting it to NULL without freeing it beforehand is
a leak.
looks correct to millert, ok inoguchi
|
|
|
|
| |
ok bcook inoguchi jsing
|
|
|
|
|
|
|
| |
Found the hard way by lists y42 org via an OCSP validation failure that
in turn caused pkg_add over TLS to fail. Detailed report by sthen.
ok sthen
|
|
|
|
|
|
| |
x509v3_cache_extensions().
ok tb@
|
|
|
|
| |
suggested by jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
x509_internal.h defines caps on the number of name constraints and
other names (such as subjectAltNames) that we want to allocate per
cert chain. These limits are checked too late. In a particularly
silly cert that jan found on ugos.ugm.ac.id 443, we ended up
allocating six times 2048 x509_constraint_name structures before
deciding that these are more than 512.
Fix this by adding a names_max member to x509_constraints_names which
is set on allocation against which each addition of a name is checked.
cluebat/ok jsing
ok inoguchi on earlier version
|
|
|
|
|
|
|
|
| |
If we're about to add a chain we have a trust path, so we have at least
one trusted certificate. This fixes a thinko from r1.31 and fixes the
openssl(1) cms verify test.
ok jsing (who had the same diff)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
To integrate the new X.509 verifier, X509_verify_cert() was refactored.
The code building chains in the legacy verifier was split into a
separate function. The first bug is that its return value was treated
as a Boolean although it wasn't. Second, the return alone is not enough
to decide whether to carry on the validation or not.
Slightly rearrange things to restore the behavior of the legacy verifier
prior to this refactoring.
Issue found and test case provided by Anton Borowka and jan.
ok jan jsing
|
|
|
|
| |
ok tb@
|
|
|
|
|
|
|
|
|
| |
In x509_verify_ctx_set_xsc_chain(), an ENOMEM case is currently passing
the last certificate and depth (which is no longer actually depth) to
x509_verify_cert_error(). Given we've hit an ENOMEM situation, neither
of these are useful so remove both.
ok tb@
|
|
|
|
|
|
| |
num_untrusted, but unfortunately it's public...
ok jsing tobhe
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As should be obvious from the name and the comment in x509_vfy.h
int last_untrusted; /* index of last untrusted cert */
last_untrusted actually counts the number of untrusted certs at the
bottom of the chain.
Unfortunately, an earlier fix introducing x509_verify_set_xsc_chain()
assumed that last_untrusted actually meant the index of the last
untrusted cert in the chain, resulting in an off-by-one, which in turn
led to x509_vfy_check_chain_extension() skipping the check for the
EXFLAG_CRITICAL flag.
A second bug in x509_verify_set_xsc_chain() assumed that it is always
called with a trusted root, which is not necessarily the case anymore.
Address this with a temporary fix which will have to be revisited once
we will allow chains with more than one trusted cert.
Reported with a test case by tobhe.
ok jsing tobhe
|
| |
|
|
|
|
|
|
|
|
| |
Prior to calling the callback, ensure that the current (invalid and likely
incomplete) chain is set on the xsc. Some things (like auto chain) depend
on this functionality.
ok beck@
|
|
|
|
|
|
|
|
| |
x509_vfy and have an xsc. There's no point in finding more chains since that
API can not return them, and all we do is trigger buggy callbacks in
calling software.
ok jsing@
|
|
|
|
|
|
|
| |
this in the comments. helps avoid annoying situations with the legacy
callback
ok jsing@
|
|
|
|
|
|
| |
Yet another mostly meaningless error value...
Noted by and ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When a certificate (namely a root) is specified as both a trusted and
untrusted certificate, the new verifier will find multiple chains - the
first being back to the trusted root certificate and a second via the root
that is untrusted, followed by the trusted root certificate. This situation
can be triggered by a server that (unnecessarily) includes the root
certificate in its certificate list.
While this validates correctly (using the first chain), it means that we
encounter a failure while building the second chain due to the root
certificate already being in the chain. When this occurs we call the verify
callback indicating a bad certificate. Some sensitive software (including
bacula and icinga), treat this single bad chain callback as terminal, even
though we successfully verify the certificate.
Avoid this problem by simply dumping the chain if we encounter a situation
where the certificate is already in the chain and also a trusted root -
we'll have already picked up the trusted root as a shorter path.
Issue with icinga2 initially reported by Theodore Wynnychenko.
Fix tested by sthen@ for both bacula and icinga2.
ok tb@
|
|
|
|
| |
pointed out by jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Comparing two GENERAL_NAME structures containing an EDIPARTYNAME can lead
to a crash. This enables a denial of service attack for an attacker who can
control both sides of the comparison.
Issue reported to OpenSSL on Nov 9 by David Benjamin.
OpenSSL shared the information with us on Dec 1st.
Fix from Matt Caswell (OpenSSL) with a few small tweaks.
ok jsing
|
|
|
|
|
|
|
|
|
| |
This happens if name->der_len == 0. Since we already have a length
check, we can malloc and memcpy inside the conditional. This also
makes the code easier to read.
agreement from millert
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
x509_verify_chain_new() allocates a few members of a certificate chain:
an empty stack of certificates, a list of errors encountered while
validating the chain, and a list of name constraints. The function to
copy a chain would allocate a new chain using x509_verify_chain_new()
and then clobber its members by copies of the old chain. Fix this by
replacing x509_verify_chain_new() with calloc().
Found by review while investigating the report by Hanno Zysik who
found the same leak using valgrind. This is a cleaner version of
my initial fix from jsing.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The legacy validator would only call x509_vfy_check_policy() once at
the very end after cobbling together a chain. Therefore it didn't
matter that X509_policy_check() always allocates a new tree on top of
the one that might have been passed in. This is in stark contrast to
other, similar APIs in this code base. The new validator calls this
function several times over while building its chains. This adds up
to a sizable leak in the new validator.
Reported with a reproducer by Hanno Zysik on github, who also bisected
this to the commit enabling the new validator.
Narrowed down to x509_vfy_check_policy() by jsing.
We simultaenously came up with a functionally identical fix.
ok jsing
|
| |
|
|
|
|
|
|
| |
a few lines after.
stylistic nit from jsing
|
| |
|
|
|
|
| |
ok beck@ tb@
|
|
|
|
|
|
|
|
| |
This was inadvertently removed in r1.19.
Spotted by tb@
ok beck@ tb@
|
|
|
|
|
|
|
|
|
| |
in order to be compatible with the openssl error craziness in the legacy
verifier case.
This will fix a regress problem noticed by znc
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
With the old verifier, the verify callback can always return 1 instructing
the verifier to simply continue regardless of a certificate verification
failure (e.g. the certificate is expired or revoked). This would result
in a chain being built, however the first error encountered would be
persisted, which allows the caller to build the chain, have the
verification process succeed, yet upon inspecting the error code note
that the chain is not valid for some reason.
Mimic this behaviour by keeping track of certificate errors while building
chains - when we finish verification, find the certificate error closest
to the leaf certificate and expose that via the X509_STORE_CTX. There are
various corner cases that we also have to handle, like the fact that we
keep an certificate error until we find the issuer, at which point we have
to clear it.
Issue reported by Ilya Shipitcin due to failing haproxy regression tests.
With much discussion and input from beck@ and tb@!
ok beck@ tb@
|
| |
|