X509_LOOKUP_store: new X509_LOOKUP_METHOD that works by OSSL_STORE URI

[levitte]

This is a wrapper around OSSL_STORE.

This also adds necessary support functions:

• X509_STORE_load_file
• X509_STORE_load_path
• X509_STORE_load_store
• SSL_add_store_cert_subjects_to_stack
• SSL_CTX_set_default_verify_store
• SSL_CTX_load_verify_file
• SSL_CTX_load_verify_dir
• SSL_CTX_load_verify_store

and deprecates X509_STORE_load_locations and SSL_CTX_load_verify_locations,
as they aren't extensible.

Checklist

• documentation is added or updated
• tests are added or updated

[petrovr]

Before to try to use proposed X.509 lookup store API I would like to discuss some items. Except by_store.c all other modifications look trivial.

1. why is pushed only first found?
   From my point of view "X.509 store" cache functionality is responsible to add, replace and etc. a certificate of crl. With other words lookup is responsible to fetch all(!) items that match name and type and to try to add all of them to "X.509 store".

   Business case:
   Two certificates with one and the same DN(subject) - one expired(1) and one valid(2).
   Verification process is with "check" time in period of

   • "expired" certificate
   • current time, i.e. period of valid one.
     Remark: I don't know in details of current "X.509 store" functionality and may be above business scenario is fully supported.

2. store api defines "expect" function.
   If I understand it is used to restrict returned(loaded) entries.
   If OSSL_STORE_expect is called my "e_ldap" return only specified type - either certificate or crl.
   Could by-store use "expect" function?

[levitte]

Before to try to use proposed X.509 lookup store API I would like to discuss some items. Except by_store.c all other modifications look trivial.

1. why is pushed only first found?
   From my point of view "X.509 store" cache functionality is responsible to add, replace and etc. a certificate of crl. With other words lookup is responsible to fetch all(!) items that match name and type and to try to add all of them to "X.509 store".

That an interesting interpretation. Documentation says that functions such as get_by_subject are supposed to return one object via the passed X509_OBJECT pointer. Of course, it's certainly possible for get_by_subject and similar to fill up the X509 STORE with everything appropriate it finds, but this is in no way mandatory as far as I can see. E.g., by_dir.c's get_cert_by_subject does not push any objects to the store, it just has a cache of its own.

Business case:
Two certificates with one and the same DN(subject) - one expired(1) and one valid(2).
Verification process is with "check" time in period of
- "expired" certificate
- current time, i.e. period of valid one.
Remark: I don't know in details of current "X.509 store" functionality and may be above business scenario is fully supported.

You have this problem with X509 STORE either way. If you have a look in X509_STORE_CTX_get_by_subject, you can see it starting with this:

    CRYPTO_THREAD_write_lock(ctx->lock);
    tmp = X509_OBJECT_retrieve_by_subject(ctx->objs, type, name);
    CRYPTO_THREAD_unlock(ctx->lock);

The X509_OBJECT_retrieve_by_subject call doesn't check anything but the subject name and the object type.

2. store api defines "expect" function.
   If I understand it is used to restrict returned(loaded) entries.
   If OSSL_STORE_expect is called my "e_ldap" return only specified type - either certificate or crl.
   Could by-store use "expect" function?

Good point. I didn't because I was thinking that X509_LU_NONE could be a viable type for "anything"... however, I now notice that by_dir.c regards that as invalid, so...

[petrovr]

Existing functionality push all objects that match specified name and type to X509_STORE.

There is good reason to to that. As pointer above on files system could exist more then one certificate with different validity period. This is because lookup don't know "check(verify)" time.
Proposed methodology "get first" is not equivalent to current functionality.

For crl is expected files system to contain more then one ctl each with different "last update".

Let on 'non-stop' system is started a daemon process that performs validation and verification of certificates, i.e. daemon is expected to run for long time. Let only one directory is set for X.509 look-up and into directory exist only one crl (all ca certs are loaded by_file).

1. start with one crl
   c727cfec.r0
2. first verification, get_cert_by_subject (by_dir.c) for crl.
   Outer loop is not so important (one directory).
   As is pointed in Fix a memory leak if using a custom X509_LOOKUP_METHOD #8708 there is "crl counter cache"
   For first time cache is empty so counter ( variable (k) is set to zero.
   Inner loop
   File c727cfec.r0 exist, X509_load_crl_file push to X509_STORE(+ref up) and increment counter
   File c727cfec.r1 does not exist, so exit.
   Result is fetched from X509_STORE.
   "crl counter cache" is updated with 1
   Copy fetched result to function argument, i.e. X509_OBJECT *ret.
3. second verification...
   crl cache returns 1
   Inner loop
   File c727cfec.r1 does not exist, so exit.
   Result is fetched from X509_STORE.
   "crl counter cache" is updated with 1
   Copy fetched result to function argument, i.e. X509_OBJECT *ret.
   Remark: no new objects added to store!
4. new crl issued
   c727cfec.r1
5. new crl issued
   c727cfec.r2
6. third verification...
   crl cache returns 1
   Inner loop
   File c727cfec.r1 exist, to X509_STORE(ref^), increment counter
   File c727cfec.r2 exist, to X509_STORE(ref^), increment counter
   File c727cfec.r3 does not exist, so exit.
   Fetch result from X509_STORE.
   update "crl counter cache" stored value 3.
   Copy fetched result to function argument, i.e. X509_OBJECT *ret.
   Remark: 2 new objects added to store!

From business point of view for crl by_subject must be called unconditionally. Between two calls to directory could be stored zero or more crl. In all cases by_subject always return success and filled return argument.

//diff by_dir vs by_ldap follow

[petrovr]

Previous comment lack case when crl does not exist at all - no load, no addition to X509_STORE, no result fetched from X509_STORE, no copy to function argument and method return false. Remark: Using suffixes like .r0 , .r1, .r2, ... ,rN where N increase with next CRL is equal to sort by "last update". For certificate is similar - sort by period. //Off topic: based on current functionality and existing public API I guess that is visible another backward compatible approach to turn X.509 lookup related public API into usable level (#6970).

[petrovr]

Currently by_dir stops processing of directories on first found and loaded object.
Some one (nit-picker) will found immediately defect:
Let dir1 is with already loaded crl:
c727cfec.r0
c727cfec.r1
Let in dir2 user add next crl:
c727cfec.r2
Due to "counted cache" by_subject will skip dir1 and will load clr from dir2 as ti match "pattern suffix"
Let user restart server.
On first call by_subject will load only .r0 and .r1 so certificate listed only in r2 will not be considered as revoked.
From my point of view this is user error to use separate directories.

Let see database processing in particular. Working with database is more specific.
Is some case there is no way to order(sort) results. Ldap is such example.

Another point if that by_dir check result of X509_STORE_add* - indirectly returned from methods X509_load_{cert|crl}_file.
Note method return false in two cases: (1) object already exist, (2) on error.

For certs by_subject is called only once (per distinguished name) and of first call all certs will be pushed to store due to different validity period.
For crl is similar for first call but on next call due to "counter cache" to X509_STORE could pushed only new crl.
With other words case (1) should not happen (assuming correct file names in a directory).

So this is DIFFERENCE between by_dir and by_ldap!
In ldap there is no way to know order of returned cert or crl and so for all specified urls and for each urls by_ldap push fetch result to X509_STORE and count number of successful additions.
If count is greater then zero a new(!) object was added to store. Return success with copy of fetched for X509_STORE result.

According my understanding of X.509 lookup process by_store functionality should be similar.

[levitte]

I've changed my mind on the subject of how get_by_subject must behave, based on what the X509_LOOKUP and X509_STORE APIs do, see #8707 (comment)

As a consequence, I've changed by_store to use the same method as by_dir / by_file, as a proof of concept of sorts.

[petrovr]

Hi, I just look into new updates.
Modification get_first_found -> cache_objects is fine for certificates as they are loaded only once (at first request).
For CRL it is fine only for first request. Method "add" to X509_STORE returns false in two cases : object already exist into store or on error.
I try to explain this in previous comments: "... DIFFERENCE between by_dir and by_ldap ...".

To catch this the test case should be more complex: CA, 2 certificates, first revoked. Then in one and the same test (like long running server) : verify second(1), revoke second, update of external "store"(2), verify second(3).
I guess that is preferred to assume that for a period of time into "external" store will coexist more than one CRLs. We cannot assume that step (2) is atomic. Extra system could load new CRL(step 1) and then it could remove obsolete (step 2).

[levitte]

So are you trying to say that when I find a CRL, I should remove the previous one that is cached before add the one I just found?

[petrovr]

Richard Levitte wrote:

So are you trying to say that when I find a CRL, I should remove the previous one that is cached before add the one I just found?

There is more than one solution. (a) clear for x509store crls and then all all found (b) I chose something like this: counter = 0 while (1) { ... ok = "add"(object) if ok then counter ++ } .. if counter > 0 return true else return false; (c) use new more flexible method by example xadd that return positive(1) if added, zero if exist negative on error. If I remember well this is how in the pas "boolean" function are changed to integer. xadd() could be keep internal. (d) ... ? (a) could be analysed in context of multi-threading . I'm not expect here. Another point is that CRL could be big X MB. To release X BM and the then to allocate again .... Does not look good. Roumen

[mattcaswell]

Approved assuming CIs agree.

[levitte]

... assuming CIs agree.

Check!

[levitte]

Merged.

e3c4ad2 OSSL_STORE: constify the criterion parameter a bit more
6dcb100 X509_LOOKUP_store: new X509_LOOKUP_METHOD that works by OSSL_STORE URI
fd3397f Add -CAstore and similar to all openssl commands that have -CApath
2897b00 OSSL_STORE: add tracing
f4aa622 Add a basic test of -CAstore
573e4bf Adapt two test programs that were using now deprecated functions
849d91a Document X509_LOOKUP_store
e90f08f X509_LOOKUP_store: Add CHANGES note
bdb0e04 Document added SSL functions related to X509_LOOKUP_store