| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
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@
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
If we fail to find a parent certificate from either the supplied roots or
intermediates and we have a X509_STORE_CTX, call its get_issuer() callback
to see if it can supply a suitable certificate. This makes things like
certificates by directory (aka by_dir) work correctly.
Issue noted by Uwe Werler <uwe@werler.is>
ok beck@ tb@
|
|
|
|
| |
Suggested by and discussed with beck
|
|
|
|
|
|
|
| |
context. This is what is returned in SSL_get_verify_result().
Spotted and initial diff from jeremy; discussed with jsing
ok beck
|
|
|
|
|
|
| |
ctx->xsc->error. Will be needed in an upcoming diff.
from beck
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously the leaf certificate was only being set up on the X509_STORE_CTX
after two verification steps were performed, however at least one of those
steps could result in the verification callback being triggered and
existing code breaking.
Issue noticed by Raf Czlonka when attempting to connect to talk.google.com
using profanity (which does not set SNI and ends up receiving an invalid
certificate).
ok beck@ deraadt@ tb@
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Various software expects the previous behaviour where the certificate chain
is available on the X509_STORE_CTX when the verify callback is triggered.
Issue hit by bket@ with lastpass-cli which has built in certificate
pinning that is checked via the verify callback.
Fix confirmed by bket@.
ok beck@
|
|
|
|
| |
No change in the generated assembly on amd64.
|
|
|
|
| |
Requested by jsing
|
|
|
|
|
|
|
|
|
|
|
| |
The outer scope in x509_constraints_extract_names() contains a vname
variable which will be freed on error, but an inner scope contains
another vname that won't be freed, e.g., if x509_constraints_names_add
fails.
Found by llvm scan-build.
ok beck
|
| |
|
|
|
|
|
|
|
|
| |
regress to catch it in the future.
found by Guido Vranken's cryptofuzzer
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the default path of the switch is taken, vname will not be added
to the names list and will leak when it is set to NULL. Simplify the
logic by eliminating the add Boolean. Instead, free and zero vname in
the default case and continue the while loop directly. At the bottom
of the switch, add vname to the names list unconditionally zero it out
since it's now owned by names.
Found by Guido Vranken's cryptofuzzer
ok beck
|
|
|
|
| |
ok tb@
|
|
|
|
|
|
|
|
| |
ASN1_STRING - the gift that keeps on giving.
Found by Guido Vranken's cryptofuzzer.
ok tb@
|
|
|
|
| |
symbol is exposed api and we probably need to deprecate it thoughtfully.
|
|
|
|
|
|
|
|
|
|
|
| |
On OSX and Windows platform portable build fails since it can't find
NAME_CONSTRAINTS_check.
It is still remain in x509v3.h and Symbols.list but '#if 0'ed from
x509_ncons.c.
In this situation, some platforms appears to get error.
Add noop NAME_CONSTRAINTS_check stub to solve this issue.
ok beck@
|
|
|
|
| |
ok beck@
|
|
|
|
|
|
|
| |
Rather than duplicating code, have x509_verify_ctx_new_from_xsc() call
x509_verify_ctx_new(), then handle the xsc specific parts.
ok beck@
|
|
|
|
|
|
| |
happy
ok tb@
|
|
|
|
|
|
| |
verification code.
ok jsing@
|
|
|
|
|
|
| |
of staying with the new name constraint code this will be deleted
ok jsing@
|
|
|
|
|
|
|
|
| |
to X509_V_ERR_OUT_OF_MEM instead of UNSPECIFIED in chain_append when
allocation fails.
noticed by ingo@
ok tb@
|
| |
|
|
|
|
|
|
|
| |
the extension list.
found by llvm static analyzer
ok tb@
|
|
|
|
|
| |
Yak for my shaving pleasure found by llvm static analyzer
ok tb@
|
|
|
|
|
| |
found by llvm static analyzer.
ok tb@
|
|
|
|
|
| |
noticed by llvm static analyzer
ok tb@
|
|
|
|
|
| |
noticed by llvm static analyzer
ok tb@
|
|
|
|
|
|
|
| |
eat a NULL.
found by clang static analyzer
ok tb@
|
|
|
|
| |
ok tb@
|
|
|
|
|
|
|
| |
the roots for a ctx are only freed in the free function, not in the
clear function, so that a ctx can be re-used with the same roots.
ok tb@
|
|
|
|
| |
so we don't need to pop free the roots separately
|
| |
|
|
|
|
| |
ok jsing@ tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The new validator finds multiple validated chains to handle the modern
PKI cases which may frequently have multiple paths via different
intermediates to different roots. It is loosely based on golang's x509
validator
This includes integration so that the new validator can be used via
X509_verify_cert() as well as a new api x509_verify() which will
return multiple chains (similar to go).
The new validator is not enabled by default with this commit, this
will be changed in a follow on commit.
The new public API is not yet exposed, and will be finalized and
exposed with a man page and a library minor bump later.
ok tb@ inoguchi@ jsing@
|
|
|
|
| |
ok jsing@
|