 |
 |
@abh3 requested changes on this pull request.
OK, how does all of this integrate? That is what gets the auto-generation of the cafile started? My original thought was to provide a directive something like:
Original: xrd.tlsca noverify | {certdir | certfile} path ...
Modified: xrd.tlsca noverify | {certdir | certfile [auto]} path
So, you would specify certdir and then also specify "certfile auto path" which means that the certfile would be automatically generated from certdir and preferentially used. This is important since I don't see how we can safely come up with a path to something we create without assistance from the config. It would also allow uniform distribution of that information (which is currently done if you specify both). Plus, if we run into problems autogenerating this, we can turn it off. My assumption is that you want this done all the time which would mean this better work perfectly through all of time which is unlikely. Remember that some sites may already be using their own cafile and we can't displace that. Any thoughts on this?
Finally, only one change requested to align namespace usage with the rest of the repo.
In src/XrdTls/XrdTlsTempCA.hh:
> +/* along with XRootD in a file called COPYING.LESSER (LGPL license) and file */
+/* COPYING (GPL license). If not, see <http://www.gnu.org/licenses/>. */
+/* */
+/* The copyright holder's institutional names and contributor's names may not */
+/* be used to endorse or promote products derived from this software without */
+/* specific prior written permission of the institution or contributor. */
+/******************************************************************************/
+
+#include <atomic>
+#include <string>
+#include <memory>
+
+// Forward dec'ls.
+class XrdSysError;
+
+namespace XrdTls {
We don't use namespaces in this way. If the class is named "XrdTls" then it already has placed itself in an implicit flat namespace. If a class that would have been in a namespace, say XrdTls, it won't call itself XrdTls but simply . This avoids redundant references to XrdTls::XrdTls which is rather annoying. No, you cannot use "using" to avoid that (though obviously it does avoid it). I suggest simply not putting the class in the new XrdTls namespace as it's simply not needed and actually makes it inconvenient to debug.
> @@ -67,6 +67,25 @@ add_library( XrdTls/XrdTlsNotaryUtils.icc XrdTls/XrdTlsNotaryUtils.hh XrdTls/XrdTlsPeerCerts.cc XrdTls/XrdTlsPeerCerts.hh XrdTls/XrdTlsSocket.cc XrdTls/XrdTlsSocket.hh + XrdTls/XrdTlsTempCA.cc XrdTls/XrdTlsTempCA.hh + + #----------------------------------------------------------------------------- + # XrdCrypto: linking against a few functions that are useful for XrdTls; avoids + # linking against all of libXrdCryptossl in XrdUtils
OK, I see the problem. I really don't know why this was made a plug-in to begin with. It would probably take too much time to figure out the history. My presumption is that at the time gsi was written there could be a choice of which crypto functions could be used. Practically, I don't think there is a choice any more so it needn't be a plug-in. That said, not making it a plugin would then require changes in gsi, sigh. So, let's live with it this way. Note that we will have to mollify EPEL in that these files will be compiled twice, once for the plugin and once for XrdUtils.so, ughhh.
> @@ -151,6 +151,10 @@ inline int XrdSysFD_Socketpair(int domain, int type, int protocol, int sfd[2])
}
#endif
+// openat is part of POSIX.1-2008; in Linux, BSD, and Solaris
You mentioned in the pull comments that you used SysFD where possible which implies there are places which would have been good to use SysFD but were omitted allowing for a possible FD leak. Is that really the case or did this addition fix it?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
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