Well, this is going to be a long post (10 points and all are relevant). So, here it goes:

  1. For standard framework TLS, if you specify cadir and cafile we feed that to OpenSSL and it does the documented thing. The nice thing about autogenerated cafile is that you can dispense giving openssl the cadir; thus avoiding directory lookups that will fail as you know everything in cadir is in cafile and there is no point in reverting to cadir ever.

  2. Assuming you agree with (1) then the directive could be augmented as:
    xrd.tlsca noverify | {certdir | certfile [auto [only]]} path
    meaning: 'auto only' generate the cafile and only use that. The reason for not forcing this is to have a bypass should there be a bug in collecting certs from the cadir. The meaning of "path" for "auto" cafile was my hope that this could be done transparently and that the most current version of the cafile would exist at the specified path. The way to possibly do this is explained in point (7).

  3. The code in XrdTlsTempCA generates a new one on demand when needed. Since it uses a unique tmp name there indeed needs to be an external mechanism to keep that file alive and deep-six it should a new one be generated after all uses of the current one (hence the shared pointer). However, that approach assumes the file is not always kept open by curl/nss and might be used for refer-backs (it would be surprising if either case was true but then again....). I suppose it's also the case there could be multiple invocations of curl and you want to use the same cafile; though I don't see any harm in moving to a new one at any time. In the end while I understand the use of the shared pointer, it might be overkill in this context.

  4. With respect to (3) since cafile is regenerated upon demand it means the caller has to wait until that has been done and it certainly could take some time to do that (though it's likely under a minute). Depending on the client's timeout setting it could trigger a retry which is not exactly great. Certainly, this is an argument for generating these in the background and having a fresh one always ready.


  1. Question! I did not see anywhere in the code how other accesses subsequent to the one that triggered the regeneration are forced to wait. In fact, if I read the code correctly, multiple such files may be created until the first one completes and the timer is reset. If that's the case, could that be fixed some how? It's implicitly fixed if we go with background regeneration.

  1. Question! I see that the code also generates a composite CRL file and, indeed, curl let's you specify it and presumably it's used to verify the cert. The openssl command also allows you to do that. However, nowhere in the openssl API is it documented on how to provide the CRL list to the library. My presumption is that the library does not know how to use CRL's and it was implemented as an outboard feature in the command itself. Do you know anything more about that? Since the CRL file is available is would be nice to be able to actually use it for openssl.

  1. Indeed doing an autogenerated cafile is a bear especially when you don't know how the consumer of the file actually uses it. In OpenSSL the file is read in its entirety once and you need to reset the verify locations in a new context to get a refresh. A real pain. I would assume curl/nss does roughly the same but it's easier since it has to be done for every connection. So usage of an auto-cafile in HTTP/XrootD differs from how it is used in XrdTpc. Trying to align both might be possible since we could potentially marry it with the x509_store refresh code. However, it would need to be done in a transparent way. So, here is another proposal (yeah, like you need one at this point :-) So, consider what appears to be a very simple augmentation...

a) The cafile is regenerated every x minutes in the background much the same way it is done now (simple but not trivial). However, if XrdTlsTempCA is given a specific path then it needs to atomically create a symlink to the regenerated file using that path so that the name of the latest cafile is accessible by a static name (e.g. ln -s to path.tmp and then mv path.tmp to path). You can use the framework's timed scheduler to drive this; though I will likely have to do some mods when I interface it with x509_store refresh to keep the timing in sync. That's a future thing.

b) The code in XrdTpc doesn't need to change at all though it would be nice if it used the static path for consistency.

c) Now all we need is a mechanism to feed XrdTlsTempCA.cc the path to use (see the next point).

  1. I would propose the following new method in XrdTlsTempCA
    Init(const char *cadirpath, const char *cafilepath, bool autogen, int interval);

If autogen is true generate cafile from cadirpath and if cafilepath is not nil we want to always have a symlink
cafilepath->tempcafile. If autogen is false we want you to use cafilepath as the cafile if it's not nil. You can ignore interval -- that's for the future. This would be called by the framework during the course of TLS configuration.

  1. Should this be done no matter what? Well, it's not all that straightforward to answer. So, here are the constraints that I think should be used to guide the answer:

a) If a site specified cafile without "auto" then auto-generation should be disabled and the specified cafile should be used. Is that correct? Maybe yes maybe not. It certainly is correct if the site atomically regenerates the file (which some sites do) and we do a corresponding change to XrdTlsTempCA which would make the choice possible. That is, it's told to use that cafile and not regenerate (see proposed new method in point 8).
b) While some sites might switch to making the XRootD framework auto-regenerate; I doubt those that regenerate themselves will go with that feature. Why? Because their auto cafile is used by many other applications outside xroot/http and is typically placed in a root-only writable directory. So doing it wouldn't work for them. That said, they are the ones that would likely specify non-auto cafile anyway (with all the attendant problems).
c) What if there is no cafile specification? This is the tricky part because we all know that libnss needs it and openssl would benefit as well. Common security practice says don't do security related things behind an admin's back. This feature is relatively benign until something goes wrong and then, quite reasonably, the security hats will jump all over it leading to a very bad situation.
d) Compromises: 1) print a rather wordy long warning in the log saying you really should allow this feature, 2) allow a plugin to tell the framework to turn on auto-regeneration if not specifically disabled. This would be specific to XrdTpc and could be done by it setting an envar (say "XRD_WANT_AUTO_CAFILE"). The issue is that it's a one way request and it might not be possible to turn it on. However, I think the XrdTpc code has a fallback when it can't get a cafile (like using cadir), right? Plus, if it's a fixed cafile it should get that one (after some code changes in XrdTlsTempCA).
e) I'm totally open to any other suggestion as the answer is not at all obvious.

  1. Finally! Now all that said, I noticed an unfortunate nit-picking copy-past issue whose details are posted with the pull request. Could you correct that?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/xrootd/xrootd/pull/1431#issuecomment-811707419", "url": "https://github.com/xrootd/xrootd/pull/1431#issuecomment-811707419", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1