On Wed, 2020-09-09 at 11:41 +0200, Richard Levitte wrote: > > Regarding the library context, when viewed as a global state, it > makes > sense to have it as a first argument where it's being passed, if at > all. The question is, where should we actually pass it? We have a > few different suggestions, in wording or in code, on the table: > > 1. Pass it as first argument everywhere. This is problematic, as > "everywhere" is a *lot*, and if we do this, we might as well > re-design the whole libcrypto API [*], and that's definitely not > what OpenSSL 3.0 is about, quite the contrary. > > This has been partially done, with all the _with_libctx > functions. As far as I've understood, these have been added on > need basis, i.e. somewhere down the code path, a library context > or a property query string is needed. I can't say if this gives > any sense of completeness or consistency, viewed as an cohesive > API, or if we stand any chance of getting there without a > complete > API re-design. > > Something to be noted is that it doesn't make sense to pass the > library context everywhere, because it's already part of other > structures that are passed. For example, any method structure, > such as EVP_CIPHER, EVP_MD, etc are tied to a provider, which is > in turn tied to a library context. Passing another library > context to anything that includes one of those method structures > would only be confusing. > > 2. Pass it when constructing different structures, mostly other > context structures. As an example, EVP_PKEY_CTX_new_from_pkey() > that I displayed above. There may be cases where we need to pass > it directly to functions that aren't constructors, but I expect > those cases to be relatively few. > > This has been done for some other structures as well, on an as > needed basis: > > X509 *X509_new_with_libctx(OPENSSL_CTX *libctx, const char > *propq); > > X509_STORE_CTX *X509_STORE_CTX_new_with_libctx(OPENSSL_CTX > *libctx, > const char > *propq); > > (the following are internal only) > > int x509_set0_libctx(X509 *x, OPENSSL_CTX *libctx, const char > *propq); > int x509_crl_set0_libctx(X509_CRL *x, OPENSSL_CTX *libctx, > const char *propq); > > 3. Set a current library context, a pointer in a thread local > variable. We already have support for that, which this function: > > OPENSSL_CTX *OPENSSL_CTX_set0_default(OPENSSL_CTX *libctx); > > The usage model is this: > > OPENSSL_CTX *prevctx = OPENSSL_CTX_set0_default(libctx); > > /* do stuff with |libctx| as implicit current global state */ > > OPENSSL_CTX_set0_default(prevctx) > > Looking at that list now, I realise that it goes from most intrusive > to least intrusive, viewed as a public API. > > The third choice in particular would let any application or library > just set their library context for the duration of the code that > should be execute with that as a "global state", and restore it when > done, leaving the rest of the OpenSSL calls untouched.
We could even provide a convenience thread local stack of lib contexts so the caller would not have to keep the old value but would just push the new libctx when entering and pop the old one when leaving. With that, I think the changes needed in the application code would be fairly simple and minimal. > Something to be noted is that model 2 and 3 is possible to combine, > which could give us a smoother transition between the current API and > whatever we design going forward. Although I have big sympathy with people that worked hard to add the various _with_libctx() calls I would say that keeping the new with_libctx variants would be too confusing. Especially for the reason we would not have a complete set of them anyway. The exception might be the low level EVP calls where we deal with the libctx directly and where the algorithm fetched is actually associated with the context. > -------- > > Regarding the property query string, looking at it separate from the > library context, the question remains where to put it, and a few > proposals are on the table: > > 1. Put it last. > > 2. Put it next to the algorithm name or the object that an algorithm > name is inferred from. > > 3. Set it "globally" with a thread local variable, a bit like what > OPENSSL_CTX_set0_default() does for library contexts. > > For this model, it's been argued if it should simply be stored in > the current library context, thereby avoiding to add another > thread local variable (which, for all intents and purposes, is > another actually global thing to deal with, even though on > per-thread level). > > In my mind, model 2 would be more sensible than model 1, because of > the implied tie between algorithm name and property query string. > Also, even here, model 3 is possible to combine with model 2, and > even > with model 1. I agree that the model 2 makes more sense than model 1. However it is also indicative of one thing - that the property query really does not make sense to be a parameter of things like X509_new(), because from the top level point of view of a caller you do not actually know for what purpose it will be later used. You might want a different property query to be used for the internal SHA1 digest of the certificate and a different property query when you want to verify a certificate signature. So again instead of trying to put property query as an additional member of higher level objects I'd say that using model 3 would make more sense to me here as well. Of course for the low level algorithm fetches the property query as parameter should stay. > Regarding model 3, it must be said that there is potential for > confusion > on what it's supposed to do, replace the default property query > string > (settable with EVP_set_default_properties()), or merge with it. > Remember that a property query string given through any XXX_fetch() > function is temporarly merged with the default properties when > looking > for fitting algorithms. Here I would actually argue, that there should be another property query in the libctx in addition to the default, that would have the exactly same semantics as the propq parameter has now and for the functions with the propq parameter it even would not be applied. How to name it so it won't be confused with the default property query, that's hard to say though. -- Tomáš Mráz No matter how far down the wrong road you've gone, turn back. Turkish proverb [You'll know whether the road is wrong if you carefully listen to your conscience.]