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 1 commit
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
80 changes: 70 additions & 10 deletions src/XrdTls/XrdTlsTempCA.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@

#include "XrdTlsTempCA.hh"

#include <sstream>
#include <vector>

namespace {

typedef std::unique_ptr<FILE, decltype(&fclose)> file_smart_ptr;
Expand Down Expand Up @@ -205,20 +208,36 @@ CRLSet::processFile(file_smart_ptr &fp, const std::string &fname)


std::unique_ptr<XrdTlsTempCA::TempCAGuard>
XrdTlsTempCA::TempCAGuard::create(XrdSysError &err) {
char ca_fname[] = "/tmp/xrootd_ca_file.XXXXXX.pem";
int ca_fd = mkstemps(ca_fname, 4);
XrdTlsTempCA::TempCAGuard::create(XrdSysError &err, const std::string &ca_tmp_dir) {

if (-1 == mkdir(ca_tmp_dir.c_str(), S_IRWXU) && errno != EEXIST) {
err.Emsg("TempCA", "Unable to create CA temp directory", ca_tmp_dir.c_str(), strerror(errno));
}

std::stringstream ss;
ss << ca_tmp_dir << "/ca_file.XXXXXX.pem";
std::vector<char> ca_fname;
ca_fname.resize(ss.str().size() + 1);
memcpy(ca_fname.data(), ss.str().c_str(), ss.str().size());

int ca_fd = mkstemps(ca_fname.data(), 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);

std::stringstream ss2;
ss2 << ca_tmp_dir << "/crl_file.XXXXXX.pem";
std::vector<char> crl_fname;
crl_fname.resize(ss2.str().size() + 1);
memcpy(crl_fname.data(), ss2.str().c_str(), ss2.str().size());

int crl_fd = mkstemps(crl_fname.data(), 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(ca_fd, crl_fd, ca_fname, crl_fname));
return std::unique_ptr<TempCAGuard>(new TempCAGuard(ca_fd, crl_fd, ca_tmp_dir, ca_fname.data(), crl_fname.data()));
}


Expand All @@ -234,8 +253,32 @@ XrdTlsTempCA::TempCAGuard::~TempCAGuard() {
}


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)
bool
XrdTlsTempCA::TempCAGuard::commit() {
if (m_ca_fd < 0 || m_ca_tmp_dir.empty()) {return false;}
close(m_ca_fd);
m_ca_fd = -1;
std::string ca_fname = m_ca_tmp_dir + "/ca_file.pem";
if (-1 == rename(m_ca_fname.c_str(), ca_fname.c_str())) {
return false;
}
m_ca_fname = ca_fname;

if (m_crl_fd < 0 || m_ca_tmp_dir.empty()) {return false;}
close(m_crl_fd);
m_crl_fd = -1;
std::string crl_fname = m_ca_tmp_dir + "/crl_file.pem";
if (-1 == rename(m_crl_fname.c_str(), crl_fname.c_str())) {
return false;
}
m_crl_fname = crl_fname;

return true;
}


XrdTlsTempCA::TempCAGuard::TempCAGuard(int ca_fd, int crl_fd, const std::string &ca_tmp_dir, const std::string &ca_fname, const std::string &crl_fname)
: m_ca_fd(ca_fd), m_crl_fd(crl_fd), m_ca_tmp_dir(ca_tmp_dir), m_ca_fname(ca_fname), m_crl_fname(crl_fname)
{}


Expand Down Expand Up @@ -266,6 +309,7 @@ XrdTlsTempCA::XrdTlsTempCA(XrdSysError *err, std::string ca_dir)
if (rc) {
m_log.Emsg("XrdTlsTempCA", "Failed to launch CA monitoring thread");
m_ca_file.reset();
m_crl_file.reset();
}
}

Expand Down Expand Up @@ -293,7 +337,14 @@ XrdTlsTempCA::Maintenance()
{
m_log.Emsg("TempCA", "Reloading the list of CAs and CRLs in directory");

std::unique_ptr<TempCAGuard> new_file(TempCAGuard::create(m_log));
auto adminpath = getenv("XRDADMINPATH");
if (!adminpath) {
m_log.Emsg("TempCA", "Admin path is not set!");
return false;
}
std::string ca_tmp_dir = std::string(adminpath) + "/.xrdtls";

std::unique_ptr<TempCAGuard> new_file(TempCAGuard::create(m_log, ca_tmp_dir));
if (!new_file) {
m_log.Emsg("TempCA", "Failed to create a new temp CA / CRL file");
return false;
Expand All @@ -314,6 +365,7 @@ XrdTlsTempCA::Maintenance()
}

struct dirent *result;
errno = 0;
while ((result = readdir(dirp))) {
//m_log.Emsg("Will parse file for CA certificates", result->d_name);
if (result->d_type != DT_REG && result->d_type != DT_LNK) {continue;}
Expand All @@ -333,6 +385,7 @@ XrdTlsTempCA::Maintenance()
if (!crl_builder.processFile(fp, result->d_name)) {
m_log.Emsg("Maintenance", "Failed to process file for CRLs", result->d_name);
}
errno = 0;
}
if (errno) {
m_log.Emsg("Maintenance", "Failure during readdir", strerror(errno));
Expand All @@ -341,7 +394,14 @@ XrdTlsTempCA::Maintenance()
}
closedir(dirp);

m_ca_file.reset(new_file.release());
if (!new_file->commit()) {
m_log.Emsg("Mainteance", "Failed to finalize new CA / CRL files");
return false;
}
//m_log.Emsg("Maintenance", "Successfully created CA and CRL files", new_file->getCAFilename().c_str(),
// new_file->getCRLFilename().c_str());
m_ca_file.reset(new std::string(new_file->getCAFilename()));
m_crl_file.reset(new std::string(new_file->getCRLFilename()));
return true;
}

Expand Down
29 changes: 19 additions & 10 deletions src/XrdTls/XrdTlsTempCA.hh
Original file line number Diff line number Diff line change
Expand Up @@ -48,40 +48,48 @@ public:
~XrdTlsTempCA();

/**
* Return a handle to the current CA file. The shared_ptr
* *must* be kept alive while the CA file is in use; if it
* goes out of scope, the corresponding temporary file may
* be garbage collected.
* Returns true if object is valid.
*/
std::shared_ptr<TempCAGuard> getHandle() {return m_ca_file;}
bool IsValid() const {return m_ca_file.get() && m_crl_file.get();}

/**
* Returns true if object is valid.
* Returns the current location of the CA temp file.
*/
std::string CAFilename() const {auto file_ref = m_ca_file; return file_ref ? *file_ref : "";}

/**
* Returns the current location of the CA temp file.
*/
bool IsValid() const {return m_ca_file.get();}
std::string CRLFilename() const {auto file_ref = m_crl_file; return file_ref ? *file_ref : "";}

/**
* Manages the temporary file associated with the curl handle
*/
class TempCAGuard {
public:
static std::unique_ptr<TempCAGuard> create(XrdSysError &);
static std::unique_ptr<TempCAGuard> create(XrdSysError &, const std::string &ca_tmp_dir);

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;}

/**
* Move temporary file to the permanent location.
*/
bool commit();

TempCAGuard(const TempCAGuard &) = delete;

~TempCAGuard();

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

int m_ca_fd{-1};
int m_crl_fd{-1};
std::string m_ca_tmp_dir;
std::string m_ca_fname;
std::string m_crl_fname;
};
Expand Down Expand Up @@ -111,7 +119,8 @@ private:
int m_maintenance_thread_pipe_w{-1};
XrdSysError &m_log;
const std::string m_ca_dir;
std::shared_ptr<TempCAGuard> m_ca_file;
std::shared_ptr<std::string> m_ca_file;
std::shared_ptr<std::string> m_crl_file;

// After success, how long to wait until the next CA reload.
static constexpr unsigned m_update_interval = 900;
Expand Down
2 changes: 1 addition & 1 deletion src/XrdTpc/XrdTpcConfigure.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ bool TPCHandler::Configure(const char *configfn, XrdOucEnv *myEnv)
if (!env_cadir) {
m_ca_file.reset(new XrdTlsTempCA(&m_log, m_cadir));
if (!m_ca_file->IsValid()) {
m_log.Emsg("Config", "Workaround for libnss is required but failed to initialize.");
m_log.Emsg("Config", "CAs / CRL generation for libcurl failed.");
return false;
}
}
Expand Down
19 changes: 9 additions & 10 deletions src/XrdTpc/XrdTpcTPC.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,25 +101,24 @@ std::string encode_xrootd_opaque_to_uri(CURL *curl, const std::string &opaque)
return output.str();
}

std::shared_ptr<XrdTlsTempCA::TempCAGuard>
void
TPCHandler::ConfigureCurlCA(CURL *curl)
{
std::shared_ptr<XrdTlsTempCA::TempCAGuard> ca_guard(
m_ca_file.get() ? m_ca_file->getHandle() : nullptr
);
if (ca_guard) {
curl_easy_setopt(curl, CURLOPT_CAINFO, ca_guard->getCAFilename().c_str());
curl_easy_setopt(curl, CURLOPT_CRLFILE, ca_guard->getCRLFilename().c_str());
auto ca_filename = m_ca_file ? m_ca_file->CAFilename() : "";
auto crl_filename = m_ca_file ? m_ca_file->CRLFilename() : "";
if (!ca_filename.empty() && !crl_filename.empty()) {
curl_easy_setopt(curl, CURLOPT_CAINFO, ca_filename.c_str());
curl_easy_setopt(curl, CURLOPT_CRLFILE, crl_filename.c_str());
}
else if (!m_cadir.empty()) {
curl_easy_setopt(curl, CURLOPT_CAPATH, m_cadir.c_str());
}
if (!m_cafile.empty()) {
curl_easy_setopt(curl, CURLOPT_CAINFO, m_cafile.c_str());
}
return ca_guard;
}


bool TPCHandler::MatchesPath(const char *verb, const char *path) {
return !strcmp(verb, "COPY") || !strcmp(verb, "OPTIONS");
}
Expand Down Expand Up @@ -680,7 +679,7 @@ int TPCHandler::ProcessPushReq(const std::string & resource, XrdHttpExtReq &req)
fh->close();
return resp_result;
}
auto ca_guard = ConfigureCurlCA(curl);
ConfigureCurlCA(curl);
curl_easy_setopt(curl, CURLOPT_URL, resource.c_str());

Stream stream(std::move(fh), 0, 0, m_log);
Expand Down Expand Up @@ -772,7 +771,7 @@ int TPCHandler::ProcessPullReq(const std::string &resource, XrdHttpExtReq &req)
fh->close();
return resp_result;
}
auto ca_guard = ConfigureCurlCA(curl);
ConfigureCurlCA(curl);
curl_easy_setopt(curl, CURLOPT_URL, resource.c_str());
Stream stream(std::move(fh), streams * m_pipelining_multiplier, streams > 1 ? m_block_size : m_small_block_size, m_log);
State state(0, stream, curl, false);
Expand Down
6 changes: 3 additions & 3 deletions src/XrdTpc/XrdTpcTPC.hh
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ private:

static std::string GetAuthz(XrdHttpExtReq &req);

// Configure curl handle's CA settings. The returned object MUST BE KEPT IN SCOPE
// for as long as the curl handle is used.
std::shared_ptr<XrdTlsTempCA::TempCAGuard> ConfigureCurlCA(CURL *curl);
// Configure curl handle's CA settings. The CA files present here should
// be valid for the lifetime of the process.
void ConfigureCurlCA(CURL *curl);

// Redirect the transfer according to the contents of an XrdOucErrInfo object.
int RedirectTransfer(CURL *curl, const std::string &redirect_resource, XrdHttpExtReq &req,
Expand Down