summaryrefslogtreecommitdiffstats
path: root/lib/libcrypto/x509 (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Provide missing prototype for d2i_DSAPrivateKey_fp(3)tb2021-03-311-1/+2
| | | | ok bcook inoguchi jsing
* Fix copy-paste error in previoustb2021-03-191-2/+2
| | | | | | | 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
* Use EXFLAG_INVALID to handle out of memory and parse errors intobhe2021-03-132-11/+45
| | | | | | x509v3_cache_extensions(). ok tb@
* Zap a useless variable.tb2021-03-121-4/+2
| | | | suggested by jsing
* Missing void in function definitiontb2021-03-121-2/+2
| | | | ok jsing
* Fix checks of memory caps of constraints namestb2021-03-123-20/+32
| | | | | | | | | | | | | | | 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
* Set is_trusted in x509_verify_ctx_add_chain()tb2021-02-261-2/+2
| | | | | | | | 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)
* Fix two bugs in the legacy verifiertb2021-02-251-6/+10
| | | | | | | | | | | | | | | 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
* Rename depth to num_untrusted so it identifies what it actually represents.jsing2021-02-251-6/+6
| | | | ok tb@
* Avoid passing last and depth to x509_verify_cert_error() on ENOMEM.jsing2021-02-251-3/+2
| | | | | | | | | 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@
* Fix comment explaining last_untrusted. This should really be calledtb2021-02-241-2/+2
| | | | | | num_untrusted, but unfortunately it's public... ok jsing tobhe
* Make the new validator check for EXFLAG_CRITICALtb2021-02-241-8/+15
| | | | | | | | | | | | | | | | | | | | | | 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
* KNFtb2021-02-111-4/+7
|
* Set chain on xsc on chain build failure.jsing2021-01-091-1/+3
| | | | | | | | 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@
* Bail out early after finding an single chain if we are have been called frombeck2021-01-091-1/+9
| | | | | | | | 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@
* search the intermediates only after searching the root certs, clarifybeck2021-01-081-11/+15
| | | | | | | this in the comments. helps avoid annoying situations with the legacy callback ok jsing@
* Handle X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE in new verifier.jsing2021-01-051-1/+4
| | | | | | Yet another mostly meaningless error value... Noted by and ok tb@
* Gracefully handle root certificates being both trusted and untrusted.jsing2021-01-052-4/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | 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@
* Remove two reduntat memset calls.tb2020-12-161-3/+1
| | | | pointed out by jsing
* Fix some KNF issuestb2020-12-161-7/+8
|
* Fix a NULL dereference in GENERAL_NAME_cmp()tb2020-12-081-6/+46
| | | | | | | | | | | | 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
* Avoid undefined behavior due to memcpy(NULL, NULL, 0)tb2020-11-251-4/+6
| | | | | | | | | 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
* Plug leak in x509_verify_chain_dup()tb2020-11-181-2/+2
| | | | | | | | | | | | | | | 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
* Plug a big memory leak in the new validatortb2020-11-181-1/+6
| | | | | | | | | | | | | | | | | | 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
* zap ugly empty line before closing bracetb2020-11-181-2/+1
|
* Move freeing of the verify context to its natural place instead oftb2020-11-181-2/+2
| | | | | | a few lines after. stylistic nit from jsing
* KNF (whitespace)tb2020-11-184-13/+13
|
* Use X509_V_OK instead of 0.jsing2020-11-161-4/+3
| | | | ok beck@ tb@
* Add back an X509_STORE_CTX error code assignment.jsing2020-11-161-2/+3
| | | | | | | | This was inadvertently removed in r1.19. Spotted by tb@ ok beck@ tb@
* Return the specific failure for a "self signed certificate" in the chainbeck2020-11-151-1/+14
| | | | | | | | | 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@
* Handle additional certificate error cases in new X.509 verifier.jsing2020-11-112-12/+79
| | | | | | | | | | | | | | | | | | | | | | | 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@
* Fix bad indent.jsing2020-11-031-7/+6
|
* Hook X509_STORE_CTX get_issuer() callback from new X509 verifier.jsing2020-11-031-3/+17
| | | | | | | | | | | 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@
* Add a safety net to ensure that we set an error on the store context.tb2020-10-261-1/+3
| | | | Suggested by and discussed with beck
* If x509_verify() fails, ensure that the error is also set on the storetb2020-10-261-10/+15
| | | | | | | context. This is what is returned in SSL_get_verify_result(). Spotted and initial diff from jeremy; discussed with jsing ok beck
* Make sure that x509_vfy_check_id() failure also sets ctx->error, not onlytb2020-10-261-3/+8
| | | | | | ctx->xsc->error. Will be needed in an upcoming diff. from beck
* Ensure leaf is set up on X509_STORE_CTX before verification.jsing2020-09-261-9/+7
| | | | | | | | | | | | | 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@
* jumping into the x509 fray with a bunch of whitespace repairderaadt2020-09-261-6/+6
|
* Ensure chain is set on the X509_STORE_CTX before triggering callback.jsing2020-09-231-12/+39
| | | | | | | | | | | | 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@
* Fix some line wrapping and other whitespace issues.tb2020-09-211-45/+34
| | | | No change in the generated assembly on amd64.
* Move freeing and zeroing up to right after the while loop.tb2020-09-211-5/+5
| | | | Requested by jsing
* Avoid memleak caused by shadowingtb2020-09-201-2/+5
| | | | | | | | | | | 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
* KNF/whitespace nitstb2020-09-202-6/+7
|
* Correct a 1 byte read overflow in x509_contraints_uri and addbeck2020-09-201-4/+9
| | | | | | | | regress to catch it in the future. found by Guido Vranken's cryptofuzzer ok tb@
* Fix a memory leak in x509_constraints_extract_namestb2020-09-201-6/+6
| | | | | | | | | | | | | 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
* remove superfluous NULL checkbeck2020-09-191-2/+2
| | | | ok tb@
* Fix potential overflow in CN subject line parsing, thanks tobeck2020-09-181-4/+5
| | | | | | | | ASN1_STRING - the gift that keeps on giving. Found by Guido Vranken's cryptofuzzer. ok tb@
* revert my putting this on a diet. sadly the NAME_CONSTRAINTS_checkbeck2020-09-161-7/+1
| | | | symbol is exposed api and we probably need to deprecate it thoughtfully.
* noop NAME_CONSTRAINTS_check stubinoguchi2020-09-161-4/+5
| | | | | | | | | | | 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@
* Make check in x509_verify_ctx_set_max_signatures() consistent with others.jsing2020-09-161-4/+2
| | | | ok beck@
Copyright © 1996 – 2023 Jason A. Donenfeld. All Rights Reverse Engineered.