From: olszomal Date: Tue, 2 Sep 2025 10:02:36 +0000 (+0200) Subject: Refactor cache_objects() loop and object type handling X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3638ffc38015d79e4f411ebc5de1d58b8c56c49f;p=thirdparty%2Fopenssl.git Refactor cache_objects() loop and object type handling Reviewed-by: Tomas Mraz Reviewed-by: David von Oheimb (Merged from https://github.com/openssl/openssl/pull/28382) --- diff --git a/crypto/x509/by_store.c b/crypto/x509/by_store.c index 3fa34717873..a969e3aa05b 100644 --- a/crypto/x509/by_store.c +++ b/crypto/x509/by_store.c @@ -25,7 +25,7 @@ DEFINE_STACK_OF(CACHED_STORE) static int cache_objects(X509_LOOKUP *lctx, CACHED_STORE *store, const OSSL_STORE_SEARCH *criterion, int depth) { - int ok = 0; + int ok = 1; OSSL_STORE_CTX *ctx; X509_STORE *xstore = X509_LOOKUP_get_store(lctx); @@ -63,12 +63,16 @@ static int cache_objects(X509_LOOKUP *lctx, CACHED_STORE *store, /* NULL means error or "end of file". Either way, we break. */ if (info == NULL) + /* + * Cannot rely on OSSL_STORE_error() here: + * file_load() incorrectly reports an error at EOF + */ break; infotype = OSSL_STORE_INFO_get_type(info); - ok = 0; - if (infotype == OSSL_STORE_INFO_NAME) { + switch (infotype) { + case OSSL_STORE_INFO_NAME: /* * This is an entry in the "directory" represented by the current * uri. if |depth| allows, dive into it. @@ -81,27 +85,26 @@ static int cache_objects(X509_LOOKUP *lctx, CACHED_STORE *store, substore.propq = store->propq; ok = cache_objects(lctx, &substore, criterion, depth - 1); } - } else { - /* - * We know that X509_STORE_add_{cert|crl} increments the object's - * refcount, so we can safely use OSSL_STORE_INFO_get0_{cert,crl} - * to get them. - */ - switch (infotype) { - case OSSL_STORE_INFO_CERT: - ok = X509_STORE_add_cert(xstore, - OSSL_STORE_INFO_get0_CERT(info)); - break; - case OSSL_STORE_INFO_CRL: - ok = X509_STORE_add_crl(xstore, - OSSL_STORE_INFO_get0_CRL(info)); - break; - } + break; + /* + * We know that X509_STORE_add_{cert|crl} increments the object's + * refcount, so we can safely use OSSL_STORE_INFO_get0_{cert,crl} + * to get them. + */ + case OSSL_STORE_INFO_CERT: + ok = X509_STORE_add_cert(xstore, OSSL_STORE_INFO_get0_CERT(info)); + break; + case OSSL_STORE_INFO_CRL: + ok = X509_STORE_add_crl(xstore, OSSL_STORE_INFO_get0_CRL(info)); + break; + default: + /* Ignore all other types (PKEY, PUBKEY, PARAMS) */ + break; } OSSL_STORE_INFO_free(info); if (!ok) - break; + break; /* stop on first failure */ } OSSL_STORE_close(ctx); @@ -275,7 +278,7 @@ static int by_store_subject(X509_LOOKUP *ctx, X509_LOOKUP_TYPE type, */ static X509_LOOKUP_METHOD x509_store_lookup = { - "Load certs from STORE URIs", + "Load certificates and CRLs from OSSL_STORE URIs", NULL, /* new_item */ by_store_free, /* free */ NULL, /* init */ diff --git a/doc/man3/X509_LOOKUP_hash_dir.pod b/doc/man3/X509_LOOKUP_hash_dir.pod index 22a59969254..35160e87c53 100644 --- a/doc/man3/X509_LOOKUP_hash_dir.pod +++ b/doc/man3/X509_LOOKUP_hash_dir.pod @@ -26,8 +26,9 @@ X509_load_cert_crl_file_ex, X509_load_cert_crl_file =head1 DESCRIPTION -B and B are two certificate -lookup methods to use with B, provided by OpenSSL library. +B and B, and B are +certificate and CRL lookup methods to use with B, provided by +OpenSSL library. Users of the library typically do not need to create instances of these methods manually, they would be created automatically by @@ -125,6 +126,9 @@ It works with the help of URIs, which can be direct references to certificates or CRLs, but can also be references to catalogues of such objects (that behave like directories). +B ignores in its input anything else besides certificates and +CRLs since OpenSSL 4.0. + This method overlaps the L and L because of the 'file:' scheme loader. It does no caching of its own, but can use a caching L diff --git a/test/recipes/25-test_verify_store.t b/test/recipes/25-test_verify_store.t index 346396a628b..6338b862b55 100644 --- a/test/recipes/25-test_verify_store.t +++ b/test/recipes/25-test_verify_store.t @@ -14,7 +14,7 @@ use OpenSSL::Test::Utils; setup("test_verify_store"); -plan tests => 10; +plan tests => 11; my $dummycnf = srctop_file("apps", "openssl.cnf"); my $cakey = srctop_file("test", "certs", "ca-key.pem"); @@ -23,6 +23,7 @@ my $ukey = srctop_file("test", "certs", "ee-key.pem"); my $cnf = srctop_file("test", "ca-and-certs.cnf"); my $CAkey = "keyCA.ss"; my $CAcert="certCA.ss"; +my $CAobjects="objectsCA.ss"; # DH params, public key, private key, certificate my $CAserial="certCA.srl"; my $CAreq="reqCA.ss"; my $CAreq2="req2CA.ss"; # temp @@ -38,7 +39,7 @@ SKIP: { -key => $cakey, -keyout => $CAkey ); - skip 'failure', 8 unless + skip 'failure', 9 unless x509( 'convert request into self-signed cert', qw(-req -CAcreateserial -days 30), qw(-extensions v3_ca), @@ -47,30 +48,45 @@ SKIP: { -signkey => $CAkey, -extfile => $cnf ); - skip 'failure', 7 unless + skip 'failure', 8 unless x509( 'convert cert into a cert request', qw(-x509toreq), -in => $CAcert, -out => $CAreq2, -signkey => $CAkey ); - skip 'failure', 6 unless + skip 'failure', 7 unless req( 'verify request 1', qw(-verify -noout -section userreq), -config => $dummycnf, -in => $CAreq ); - skip 'failure', 5 unless + skip 'failure', 6 unless req( 'verify request 2', qw(-verify -noout -section userreq), -config => $dummycnf, -in => $CAreq2 ); - skip 'failure', 4 unless - verify( 'verify signature', + skip 'failure', 5 unless + verify( 'verify cert using CAstore including just the cert', -CAstore => $CAcert, $CAcert ); + open(my $out, '>', $CAobjects) or die $!; + my $pubkey = qx(openssl x509 -pubkey -noout -in $CAcert); + print $out $pubkey; + my @files; + push @files, srctop_file("test", "certs", "dhp2048.pem") + unless disabled("dh"); + push @files, ($CAkey, $CAcert); + print $out do { local @ARGV = @files; <> }; + close $out or die $!; + + skip 'failure', 4 unless + verify( 'verify cert using CAstore including cert and extra stuff', + -CAstore => $CAobjects, + $CAcert ); + skip 'failure', 3 unless req( 'make a user cert request', qw(-new -section userreq),