Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul curl's usage of CAs. #1431

Merged
merged 18 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5e31d97
Overhaul curl's usage of CAs.
bbockelm Mar 18, 2021
75f631e
Add support for certfile directive for TPC handler.
bbockelm Mar 18, 2021
c84668f
Provide a pure-environment override for the XrdTpc cadir.
bbockelm Mar 18, 2021
399d5a7
XrdTpc: Switch update variables to std::atomics.
bbockelm Mar 19, 2021
1fe8f5d
XrdTpc: Remove deprecated readdir_r.
bbockelm Mar 19, 2021
5793ac7
Remove use of smart pointers.
bbockelm Mar 25, 2021
97deb89
XrdTpc: Pass filename to parsing / exporting functions.
bbockelm Mar 28, 2021
2719b4b
XrdTpc: Use XrdSysFD functions where possible for CLOEXEC protection.
bbockelm Mar 28, 2021
380f476
XrdTpc: If NSS hack is needed and fails, do not startup server.
bbockelm Mar 28, 2021
5f49669
XrdTls: Move temp CA generator code into core XrdTls.
bbockelm Mar 28, 2021
ab5fad4
XrdTpc: Remove XrdTpcNSSSupport implementation.
bbockelm Mar 28, 2021
1eb60a5
XrdTls: HACK - temporarily link crypto files into XrdUtils.
bbockelm Mar 28, 2021
c3dc4c1
XrdTls: Extend XrdCryptosslX509Crl to load / write CRLs to a FILE*
bbockelm Mar 28, 2021
342ba6c
XrdTls: HACK - add CRLs to XrdUtils. Revert when we understand linki…
bbockelm Mar 28, 2021
399fc03
XrdTls: Add CRL concatenation support to TempCA manager.
bbockelm Mar 28, 2021
7d3a823
XrdTls: Remove XrdTlsTempCA from its dedicated namespace.
bbockelm Mar 30, 2021
639e6e3
XrdTls: Convert TempCA loader to a separate thread.
bbockelm Apr 4, 2021
87b341c
XrdTls: Refactor temp CA code to use ADMINPATH directory.
bbockelm Apr 13, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 55 additions & 23 deletions src/XrdCrypto/XrdCryptosslX509Crl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,6 @@ XrdCryptosslX509Crl::XrdCryptosslX509Crl(const char *cf, int opt)
// Constructor certificate from file 'cf'.
EPNAME("X509Crl::XrdCryptosslX509Crl_file");

// Init private members
crl = 0; // The crl object
lastupdate = -1; // begin-validity time in secs since Epoch
nextupdate = -1; // end-validity time in secs since Epoch
issuer = ""; // issuer;
issuerhash = ""; // hash of issuer;
srcfile = ""; // source file;
nrevoked = 0; // number of revoked certificates

// Make sure file name is defined;
if (opt == 0) {
if (Init(cf) != 0) {
Expand All @@ -82,6 +73,18 @@ XrdCryptosslX509Crl::XrdCryptosslX509Crl(const char *cf, int opt)
}
}

//_____________________________________________________________________________
XrdCryptosslX509Crl::XrdCryptosslX509Crl(FILE *fc, const char *cf)
{
// Constructe CRL from a FILE handle `fc` with (assumed) filename `cf`.
EPNAME("X509Crl::XrdCryptosslX509Crl_file");

if (Init(fc, cf)) {
DEBUG("could not initialize the CRL from " << cf);
return;
}
}

//_____________________________________________________________________________
XrdCryptosslX509Crl::XrdCryptosslX509Crl(XrdCryptoX509 *cacert)
: XrdCryptoX509Crl()
Expand All @@ -92,15 +95,6 @@ XrdCryptosslX509Crl::XrdCryptosslX509Crl(XrdCryptoX509 *cacert)
// loads it in the cache
EPNAME("X509Crl::XrdCryptosslX509Crl_CA");

// Init private members
crl = 0; // The crl object
lastupdate = -1; // begin-validity time in secs since Epoch
nextupdate = -1; // end-validity time in secs since Epoch
issuer = ""; // issuer;
issuerhash = ""; // hash of issuer;
srcfile = ""; // source file;
nrevoked = 0; // number of revoked certificates

// The CA certificate must be defined
if (!cacert || cacert->type != XrdCryptoX509::kCA) {
DEBUG("the CA certificate is undefined or not CA! ("<<cacert<<")");
Expand Down Expand Up @@ -161,8 +155,8 @@ XrdCryptosslX509Crl::~XrdCryptosslX509Crl()
//_____________________________________________________________________________
int XrdCryptosslX509Crl::Init(const char *cf)
{
// Constructor certificate from file 'cf'.
// Return 0 on success, -1 on failure
// Load a CRL from an open file handle; for debugging purposes,
// we assume it's loaded from file named `cf`.
EPNAME("X509Crl::Init");

// Make sure file name is defined;
Expand All @@ -187,15 +181,30 @@ int XrdCryptosslX509Crl::Init(const char *cf)
DEBUG("cannot open file "<<cf<<" (errno: "<<errno<<")");
return -1;
}

auto rval = Init(fc, cf);

//
// Close the file
fclose(fc);

return rval;
}


//_____________________________________________________________________________
int XrdCryptosslX509Crl::Init(FILE *fc, const char *cf)
{
// Constructor certificate from file 'cf'.
// Return 0 on success, -1 on failure
EPNAME("X509Crl::Init");

//
// Read the content:
if (!PEM_read_X509_CRL(fc, &crl, 0, 0)) {
DEBUG("Unable to load CRL from file");
return -1;
}
//
// Close the file
fclose(fc);

//
// Notify
Expand Down Expand Up @@ -310,6 +319,29 @@ int XrdCryptosslX509Crl::InitFromURI(const char *uri, const char *hash)
return 0;
}

//_____________________________________________________________________________
bool XrdCryptosslX509Crl::ToFile(FILE *fh)
{
// Write the CRL's contents to a file in the PEM format.
EPNAME("ToFile");

if (!crl) {
DEBUG("CRL object invalid; cannot write to a file");
return false;
}

if (PEM_write_X509_CRL(fh, crl) == 0) {
DEBUG("Unable to write CRL to file");
return false;
}

//
// Notify
DEBUG("CRL successfully written to file");

return true;
}

//_____________________________________________________________________________
int XrdCryptosslX509Crl::GetFileType(const char *crlfn)
{
Expand Down
27 changes: 16 additions & 11 deletions src/XrdCrypto/XrdCryptosslX509Crl.hh
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class XrdCryptosslX509Crl : public XrdCryptoX509Crl {
public:

XrdCryptosslX509Crl(const char *crlf, int opt = 0);
XrdCryptosslX509Crl(FILE *, const char *crlf);
XrdCryptosslX509Crl(XrdCryptoX509 *cacert);
virtual ~XrdCryptosslX509Crl();

Expand Down Expand Up @@ -79,22 +80,26 @@ public:
// Verify signature
bool Verify(XrdCryptoX509 *ref);

// Dump CRL object to a file.
bool ToFile(FILE *fh);

private:
X509_CRL *crl; // The CRL object
time_t lastupdate; // time of last update
time_t nextupdate; // time of next update
XrdOucString issuer; // issuer name;
XrdOucString issuerhash; // hash of issuer name (default algorithm);
XrdOucString issueroldhash; // hash of issuer name (md5 algorithm);
XrdOucString srcfile; // source file name, if any;
XrdOucString crluri; // URI from where to get the CRL file, if any;

int nrevoked; // Number of certificates revoked
XrdSutCache cache; // cached infor about revoked certificates
X509_CRL *crl{nullptr}; // The CRL object
time_t lastupdate{-1}; // time of last update
time_t nextupdate{-1}; // time of next update
XrdOucString issuer; // issuer name;
XrdOucString issuerhash; // hash of issuer name (default algorithm);
XrdOucString issueroldhash; // hash of issuer name (md5 algorithm);
XrdOucString srcfile; // source file name, if any;
XrdOucString crluri; // URI from where to get the CRL file, if any;

int nrevoked{0}; // Number of certificates revoked
XrdSutCache cache; // cached infor about revoked certificates

int GetFileType(const char *crlfn); //Determine file type
int LoadCache(); // Load the cache
int Init(const char *crlf); // Init from file
int Init(FILE *fc, const char *crlf); // Init from file handle
int InitFromURI(const char *uri, const char *hash); // Init from URI
};

Expand Down
115 changes: 101 additions & 14 deletions src/XrdTls/XrdTlsTempCA.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "XrdSys/XrdSysPlugin.hh"
#include "XrdCrypto/XrdCryptoX509Chain.hh"
#include "XrdCrypto/XrdCryptosslAux.hh"
#include "XrdCrypto/XrdCryptosslX509Crl.hh"
#include "XrdVersion.hh"

#include "XrdTlsTempCA.hh"
Expand Down Expand Up @@ -120,6 +121,77 @@ CASet::processFile(file_smart_ptr &fp, const std::string &fname)
return true;
}


class CRLSet {
public:
CRLSet(int output_fd, XrdSysError &err)
: m_log(err),
m_output_fd(output_fd)
{}

/**
* Given an open file descriptor pointing to
* a file potentially containing a CRL, process it
* for PEM-formatted entries. If a new, unique CRL
* is found, then it is written into the current
* tempfile.
*
* The fname argument is used solely for debugging.
*
* Returns true on success.
*/
bool processFile(file_smart_ptr &fd, const std::string &fname);

private:
XrdSysError &m_log;

// Grid CA directories tend to keep everything in triplicate;
// we keep a unique hash of all known CRLs so we write out each
// one only once.
std::unordered_set<std::string> m_known_crls;
const int m_output_fd;
};


bool
CRLSet::processFile(file_smart_ptr &fp, const std::string &fname)
{
// Note we purposely leak the outputfp here; we are just borrowing the handle.
FILE *outputfp = fdopen(m_output_fd, "w");
if (!outputfp) {
m_log.Emsg("CAset", "Failed to reopen file for output", fname.c_str());
return false;
}

// Assume we can safely ignore a failure to parse; we load every file in
// the directory and that will naturally include a number of non-CRL files.
for (std::unique_ptr<XrdCryptosslX509Crl> xrd_crl(new XrdCryptosslX509Crl(fp.get(), fname.c_str()));
xrd_crl->IsValid();
xrd_crl = std::unique_ptr<XrdCryptosslX509Crl>(new XrdCryptosslX509Crl(fp.get(), fname.c_str())))
{
auto hash_ptr = xrd_crl->IssuerHash(1);
if (!hash_ptr) {
continue;
}
auto iter = m_known_crls.find(hash_ptr);
if (iter != m_known_crls.end()) {
//m_log.Emsg("CRLset", "Skipping known CRL with hash", fname.c_str(), hash_ptr);
continue;
}
//m_log.Emsg("CRLset", "New CRL with hash", fname.c_str(), hash_ptr);
m_known_crls.insert(hash_ptr);

if (!xrd_crl->ToFile(outputfp)) {
m_log.Emsg("CRLset", "Failed to write out CRL", fname.c_str());
fflush(outputfp);
return false;
}
}
fflush(outputfp);

return true;
}

}


Expand All @@ -128,26 +200,36 @@ using namespace XrdTls;

std::unique_ptr<XrdTlsTempCA::TempCAGuard>
XrdTlsTempCA::TempCAGuard::create(XrdSysError &err) {
char fname[] = "/tmp/xrootd_ca_file.XXXXXX.pem";
int fd = mkstemps(fname, 4);
if (fd < 0) {
char ca_fname[] = "/tmp/xrootd_ca_file.XXXXXX.pem";
int ca_fd = mkstemps(ca_fname, 4);
if (ca_fd < 0) {
err.Emsg("TempCA", "Failed to create temp file:", strerror(errno));
Copy link
Member

@abh3 abh3 Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referencing line 209; both messages are identical even though they actually refer to different files. To avoid confusion I suggest the message be:
"Failed to create CA temp file:"

return std::unique_ptr<TempCAGuard>();
}
char crl_fname[] = "/tmp/xrootd_crl_file.XXXXXX.pem";
int crl_fd = mkstemps(crl_fname, 4);
if (crl_fd < 0) {
err.Emsg("TempCA", "Failed to create temp file:", strerror(errno));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referencing line 203; both messages are identical even though they actually refer to different files. To avoid confusion I suggest the message be:
"Failed to create CRL temp file:"

return std::unique_ptr<TempCAGuard>();
}
return std::unique_ptr<TempCAGuard>(new TempCAGuard(fd, fname));
return std::unique_ptr<TempCAGuard>(new TempCAGuard(ca_fd, crl_fd, ca_fname, crl_fname));
}


XrdTlsTempCA::TempCAGuard::~TempCAGuard() {
if (m_fd >= 0) {
unlink(m_fname.c_str());
close(m_fd);
if (m_ca_fd >= 0) {
unlink(m_ca_fname.c_str());
close(m_ca_fd);
}
if (m_crl_fd >= 0) {
unlink(m_crl_fname.c_str());
close(m_crl_fd);
}
}


XrdTlsTempCA::TempCAGuard::TempCAGuard(int fd, const std::string &fname)
: m_fd(fd), m_fname(fname)
XrdTlsTempCA::TempCAGuard::TempCAGuard(int ca_fd, int crl_fd, const std::string &ca_fname, const std::string &crl_fname)
: m_ca_fd(ca_fd), m_crl_fd(crl_fd), m_ca_fname(ca_fname), m_crl_fname(crl_fname)
{}


Expand All @@ -162,14 +244,15 @@ XrdTlsTempCA::XrdTlsTempCA(XrdSysError *err, std::string ca_dir)
bool
XrdTlsTempCA::Maintenance()
{
m_log.Emsg("TempCA", "Reloading the list of CAs in directory");
m_log.Emsg("TempCA", "Reloading the list of CAs and CRLs in directory");

std::unique_ptr<TempCAGuard> new_file(TempCAGuard::create(m_log));
if (!new_file) {
m_log.Emsg("TempCA", "Failed to create a new temp CA file");
m_log.Emsg("TempCA", "Failed to create a new temp CA / CRL file");
return false;
}
CASet builder(new_file->getFD(), m_log);
CASet ca_builder(new_file->getCAFD(), m_log);
CRLSet crl_builder(new_file->getCRLFD(), m_log);

int fddir = XrdSysFD_Open(m_ca_dir.c_str(), O_DIRECTORY);
if (fddir < 0) {
Expand All @@ -196,8 +279,12 @@ XrdTlsTempCA::Maintenance()
}
file_smart_ptr fp(fdopen(fd, "r"), &fclose);

if (!builder.processFile(fp, result->d_name)) {
m_log.Emsg("Maintenance", "Failed to process file", result->d_name);
if (!ca_builder.processFile(fp, result->d_name)) {
m_log.Emsg("Maintenance", "Failed to process file for CAs", result->d_name);
}
rewind(fp.get());
if (!crl_builder.processFile(fp, result->d_name)) {
m_log.Emsg("Maintenance", "Failed to process file for CRLs", result->d_name);
}
}
if (errno) {
Expand Down
15 changes: 10 additions & 5 deletions src/XrdTls/XrdTlsTempCA.hh
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,23 @@ public:
public:
static std::unique_ptr<TempCAGuard> create(XrdSysError &);

int getFD() const {return m_fd;}
std::string getFilename() const {return m_fname;}
int getCAFD() const {return m_ca_fd;}
std::string getCAFilename() const {return m_ca_fname;}

int getCRLFD() const {return m_crl_fd;}
std::string getCRLFilename() const {return m_crl_fname;}

TempCAGuard(const TempCAGuard &) = delete;

~TempCAGuard();

private:
TempCAGuard(int fd, const std::string &fname);
TempCAGuard(int ca_fd, int crl_fd, const std::string &ca_fname, const std::string &crl_fname);

int m_fd;
std::string m_fname;
int m_ca_fd{-1};
int m_crl_fd{-1};
std::string m_ca_fname;
std::string m_crl_fname;
};


Expand Down
3 changes: 2 additions & 1 deletion src/XrdTpc/XrdTpcTPC.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ TPCHandler::ConfigureCurlCA(CURL *curl)
m_ca_file.get() ? m_ca_file->getHandle() : nullptr
);
if (ca_guard) {
curl_easy_setopt(curl, CURLOPT_CAINFO, ca_guard->getFilename().c_str());
curl_easy_setopt(curl, CURLOPT_CAINFO, ca_guard->getCAFilename().c_str());
curl_easy_setopt(curl, CURLOPT_CRLFILE, ca_guard->getCRLFilename().c_str());
}
else if (!m_cadir.empty()) {
curl_easy_setopt(curl, CURLOPT_CAPATH, m_cadir.c_str());
Expand Down
2 changes: 2 additions & 0 deletions src/XrdUtils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ add_library(
XrdCrypto/XrdCryptosslX509Req.cc XrdCrypto/XrdCryptosslX509Req.hh
XrdCrypto/XrdCryptoX509Req.cc XrdCrypto/XrdCryptoX509Req.hh
XrdCrypto/XrdCryptoAux.cc XrdCrypto/XrdCryptoAux.hh
XrdCrypto/XrdCryptosslX509Crl.cc XrdCrypto/XrdCryptosslX509Crl.hh
XrdCrypto/XrdCryptoX509Crl.cc XrdCrypto/XrdCryptoX509Crl.hh
XrdCrypto/XrdCryptoTrace.hh

#-----------------------------------------------------------------------------
Expand Down