From 283ac37fc2ee24c5cea6c4f54cb13962ccad7dc4 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Sun, 9 Jun 2024 11:43:30 -0500 Subject: [PATCH 1/4] Separately initialize libcurl Previously, libcurl was initialized for each request; this could have triggered a race condition as the global init is not thread-safe. Now, pull this out to a standalone `init` function for each library. --- src/HTTPCommands.cc | 15 ++++++++------- src/HTTPCommands.hh | 6 ++++++ src/HTTPFile.cc | 1 + src/S3Commands.hh | 2 ++ src/S3File.cc | 1 + 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/HTTPCommands.cc b/src/HTTPCommands.cc index 999ca8f..7772570 100644 --- a/src/HTTPCommands.cc +++ b/src/HTTPCommands.cc @@ -168,12 +168,6 @@ bool HTTPRequest::sendPreparedRequest(const std::string &protocol, m_log.Log(XrdHTTPServer::Debug, "SendRequest", "Sending HTTP request", uri.c_str()); - CURLcode rv = curl_global_init(CURL_GLOBAL_ALL); - if (rv != 0) { - this->errorCode = "E_CURL_LIB"; - this->errorMessage = "curl_global_init() failed."; - return false; - } std::unique_ptr curl( curl_easy_init(), &curl_easy_cleanup); @@ -185,7 +179,7 @@ bool HTTPRequest::sendPreparedRequest(const std::string &protocol, } char errorBuffer[CURL_ERROR_SIZE]; - rv = curl_easy_setopt(curl.get(), CURLOPT_ERRORBUFFER, errorBuffer); + auto rv = curl_easy_setopt(curl.get(), CURLOPT_ERRORBUFFER, errorBuffer); if (rv != CURLE_OK) { this->errorCode = "E_CURL_LIB"; this->errorMessage = "curl_easy_setopt( CURLOPT_ERRORBUFFER ) failed."; @@ -448,6 +442,13 @@ bool HTTPUpload::SendRequest(const std::string &payload, off_t offset, return SendHTTPRequest(payload); } +void HTTPRequest::init() { + CURLcode rv = curl_global_init(CURL_GLOBAL_ALL); + if (rv != 0) { + throw std::runtime_error("libcurl failed to initialize"); + } +} + // --------------------------------------------------------------------------- HTTPDownload::~HTTPDownload() {} diff --git a/src/HTTPCommands.hh b/src/HTTPCommands.hh index cc78c33..b366b1f 100644 --- a/src/HTTPCommands.hh +++ b/src/HTTPCommands.hh @@ -57,6 +57,12 @@ class HTTPRequest { size_t sentSoFar; }; + // Initialize libraries for HTTP. + // + // Should be called at least once per application from a non-threaded + // context. + static void init(); + protected: bool sendPreparedRequest(const std::string &protocol, const std::string &uri, diff --git a/src/HTTPFile.cc b/src/HTTPFile.cc index 364f2fe..8632339 100644 --- a/src/HTTPFile.cc +++ b/src/HTTPFile.cc @@ -256,6 +256,7 @@ XrdOss *XrdOssGetStorageSystem2(XrdOss *native_oss, XrdSysLogger *Logger, envP->Export("XRDXROOTD_NOPOSC", "1"); try { + HTTPRequest::init(); g_http_oss = new HTTPFileSystem(Logger, config_fn, envP); return g_http_oss; } catch (std::runtime_error &re) { diff --git a/src/S3Commands.hh b/src/S3Commands.hh index 6d4eddc..a1ad5e9 100644 --- a/src/S3Commands.hh +++ b/src/S3Commands.hh @@ -82,6 +82,8 @@ class AmazonRequest : public HTTPRequest { virtual bool SendRequest(); virtual bool SendS3Request(const std::string &payload); + static void init() { HTTPRequest::init(); } + protected: bool sendV4Request(const std::string &payload, bool sendContentSHA = false); diff --git a/src/S3File.cc b/src/S3File.cc index e90e5ca..b15bb00 100644 --- a/src/S3File.cc +++ b/src/S3File.cc @@ -208,6 +208,7 @@ XrdOss *XrdOssGetStorageSystem2(XrdOss *native_oss, XrdSysLogger *Logger, envP->Export("XRDXROOTD_NOPOSC", "1"); try { + AmazonRequest::init(); g_s3_oss = new S3FileSystem(Logger, config_fn, envP); return g_s3_oss; } catch (std::runtime_error &re) { From 43a93c055f7a89ed874918b9dc06f3524efd2e12 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Sun, 9 Jun 2024 11:50:07 -0500 Subject: [PATCH 2/4] Improve handling of errors for fstat failures The HTTP server will do a HttpFileSystem::Stat (ignoring the result) followed by a HttpFile::Open. Since the HttpFile::Open was a no-op, the fact a file didn't exist (or was otherwise broken) wasn't recorded, a HTTP 200 was always returned (and _then_ there was a subsequent failure when transferring). This change causes a stat to always be done on file-open, causing the correct HTTP status code to be sent. Additionally, it improves the error code handling for both HTTP and S3 stat's. --- src/HTTPFile.cc | 65 +++++++++++++++++++++++++++++++++---------- src/HTTPFileSystem.cc | 9 +----- src/S3File.cc | 33 +++++++++++++++------- 3 files changed, 74 insertions(+), 33 deletions(-) diff --git a/src/HTTPFile.cc b/src/HTTPFile.cc index 8632339..a62907b 100644 --- a/src/HTTPFile.cc +++ b/src/HTTPFile.cc @@ -142,6 +142,20 @@ ssize_t HTTPFile::Read(void *buffer, off_t offset, size_t size) { } int HTTPFile::Fstat(struct stat *buff) { + if (m_stat) { + buff->st_mode = 0600 | S_IFREG; + buff->st_nlink = 1; + buff->st_uid = 1; + buff->st_gid = 1; + buff->st_size = content_length; + buff->st_mtime = last_modified; + buff->st_atime = 0; + buff->st_ctime = 0; + buff->st_dev = 0; + buff->st_ino = 0; + return 0; + } + m_log.Log(LogMask::Debug, "HTTPFile::Fstat", "About to perform HTTPFile::Fstat():", hostUrl.c_str(), object.c_str()); @@ -153,11 +167,29 @@ int HTTPFile::Fstat(struct stat *buff) { // than code 200. If xrootd wants us to distinguish between // these cases, head.getResponseCode() is initialized to 0, so // we can check. - std::stringstream ss; - ss << "Failed to send HeadObject command: " << head.getResponseCode() - << "'" << head.getResultString() << "'"; - m_log.Log(LogMask::Warning, "HTTPFile::Fstat", ss.str().c_str()); - return -ENOENT; + auto httpCode = head.getResponseCode(); + if (httpCode) { + std::stringstream ss; + ss << "HEAD command failed: " << head.getResponseCode() << ": " + << head.getResultString(); + m_log.Log(LogMask::Warning, "HTTPFile::Fstat", ss.str().c_str()); + switch (httpCode) { + case 404: + return -ENOENT; + case 500: + return -EIO; + case 403: + return -EPERM; + default: + return -EIO; + } + } else { + std::stringstream ss; + ss << "Failed to send HEAD command: " << head.getErrorCode() << ": " + << head.getErrorMessage(); + m_log.Log(LogMask::Warning, "HTTPFile::Fstat", ss.str().c_str()); + return -EIO; + } } std::string headers = head.getResultString(); @@ -197,16 +229,19 @@ int HTTPFile::Fstat(struct stat *buff) { current_newline = next_newline; } - buff->st_mode = 0600 | S_IFREG; - buff->st_nlink = 1; - buff->st_uid = 1; - buff->st_gid = 1; - buff->st_size = this->content_length; - buff->st_mtime = this->last_modified; - buff->st_atime = 0; - buff->st_ctime = 0; - buff->st_dev = 0; - buff->st_ino = 0; + if (buff) { + buff->st_mode = 0600 | S_IFREG; + buff->st_nlink = 1; + buff->st_uid = 1; + buff->st_gid = 1; + buff->st_size = this->content_length; + buff->st_mtime = this->last_modified; + buff->st_atime = 0; + buff->st_ctime = 0; + buff->st_dev = 0; + buff->st_ino = 0; + } + m_stat = true; return 0; } diff --git a/src/HTTPFileSystem.cc b/src/HTTPFileSystem.cc index 5e17b9d..ab0d4ee 100644 --- a/src/HTTPFileSystem.cc +++ b/src/HTTPFileSystem.cc @@ -161,14 +161,7 @@ int HTTPFileSystem::Stat(const char *path, struct stat *buff, int opts, m_log.Emsg("Stat", "Failed to open path:", path); } // Assume that HTTPFile::FStat() doesn't write to buff unless it succeeds. - rv = httpFile.Fstat(buff); - if (rv != 0) { - formatstr(error, "File %s not found.", path); - m_log.Emsg("Stat", error.c_str()); - return -ENOENT; - } - - return 0; + return httpFile.Fstat(buff); } int HTTPFileSystem::Create(const char *tid, const char *path, mode_t mode, diff --git a/src/S3File.cc b/src/S3File.cc index b15bb00..daf7ffb 100644 --- a/src/S3File.cc +++ b/src/S3File.cc @@ -102,16 +102,29 @@ int S3File::Fstat(struct stat *buff) { AmazonS3Head head(m_ai, m_object, m_log); if (!head.SendRequest()) { - // SendRequest() returns false for all errors, including ones - // where the server properly responded with something other - // than code 200. If xrootd wants us to distinguish between - // these cases, head.getResponseCode() is initialized to 0, so - // we can check. - std::stringstream ss; - ss << "Failed to send HeadObject command: " << head.getResponseCode() - << "'" << head.getResultString() << "'"; - m_log.Log(LogMask::Warning, "S3File::Fstat", ss.str().c_str()); - return -ENOENT; + auto httpCode = head.getResponseCode(); + if (httpCode) { + std::stringstream ss; + ss << "HEAD command failed: " << head.getResponseCode() << ": " + << head.getResultString(); + m_log.Log(LogMask::Warning, "S3ile::Fstat", ss.str().c_str()); + switch (httpCode) { + case 404: + return -ENOENT; + case 500: + return -EIO; + case 403: + return -EPERM; + default: + return -EIO; + } + } else { + std::stringstream ss; + ss << "Failed to send HEAD command: " << head.getErrorCode() << ": " + << head.getErrorMessage(); + m_log.Log(LogMask::Warning, "S3File::Fstat", ss.str().c_str()); + return -EIO; + } } std::string headers = head.getResultString(); From 37cacbac6547c74ea42301b9921cc62034b9663b Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Sun, 9 Jun 2024 11:55:12 -0500 Subject: [PATCH 3/4] Add ability to specify a bearer token in a file This adds a new configuration option, `httpserver.token_file`, where the administrator can specify the location of a token. When set, all requests will use the token in the file as a bearer token in requests. --- CMakeLists.txt | 4 +-- src/HTTPCommands.cc | 18 ++++++++++ src/HTTPCommands.hh | 32 ++++++++++------- src/HTTPFile.cc | 24 +++++++------ src/HTTPFile.hh | 8 +++-- src/HTTPFileSystem.cc | 11 ++++-- src/HTTPFileSystem.hh | 5 +++ src/S3Commands.hh | 2 +- src/TokenFile.cc | 84 +++++++++++++++++++++++++++++++++++++++++++ src/TokenFile.hh | 52 +++++++++++++++++++++++++++ test/http_tests.cc | 2 +- 11 files changed, 210 insertions(+), 32 deletions(-) create mode 100644 src/TokenFile.cc create mode 100644 src/TokenFile.hh diff --git a/CMakeLists.txt b/CMakeLists.txt index 2f5ea1e..7378ae5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -56,8 +56,8 @@ endif() include_directories(${XROOTD_INCLUDES} ${CURL_INCLUDE_DIRS} ${LIBCRYPTO_INCLUDE_DIRS}) -add_library(XrdS3 SHARED src/S3File.cc src/S3Directory.cc src/S3AccessInfo.cc src/S3FileSystem.cc src/AWSv4-impl.cc src/S3Commands.cc src/HTTPCommands.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc) -add_library(XrdHTTPServer SHARED src/HTTPFile.cc src/HTTPFileSystem.cc src/HTTPCommands.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc) +add_library(XrdS3 SHARED src/S3File.cc src/S3Directory.cc src/S3AccessInfo.cc src/S3FileSystem.cc src/AWSv4-impl.cc src/S3Commands.cc src/HTTPCommands.cc src/TokenFile.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc) +add_library(XrdHTTPServer SHARED src/HTTPFile.cc src/HTTPFileSystem.cc src/HTTPCommands.cc src/TokenFile.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc) target_link_libraries(XrdS3 -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES} tinyxml2::tinyxml2) target_link_libraries(XrdHTTPServer -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES}) diff --git a/src/HTTPCommands.cc b/src/HTTPCommands.cc index 7772570..b0933be 100644 --- a/src/HTTPCommands.cc +++ b/src/HTTPCommands.cc @@ -341,6 +341,24 @@ bool HTTPRequest::sendPreparedRequest(const std::string &protocol, } } + if (m_token) { + const auto iter = headers.find("Authorization"); + if (iter == headers.end()) { + std::string token; + if (m_token->Get(token) && !token.empty()) { + headers["Authorization"] = "Bearer " + token; + } else { + errorCode = "E_TOKEN"; + errorMessage = "failed to load authorization token from file"; + } + } + } + { + const auto iter = headers.find("User-Agent"); + if (iter == headers.end()) { + headers["User-Agent"] = "xrootd-http/devel"; + } + } std::string headerPair; struct curl_slist *header_slist = NULL; for (auto i = headers.begin(); i != headers.end(); ++i) { diff --git a/src/HTTPCommands.hh b/src/HTTPCommands.hh index b366b1f..cd8bb42 100644 --- a/src/HTTPCommands.hh +++ b/src/HTTPCommands.hh @@ -18,6 +18,8 @@ #pragma once +#include "TokenFile.hh" + #include #include #include @@ -26,9 +28,9 @@ class XrdSysError; class HTTPRequest { public: - HTTPRequest(const std::string &hostUrl, XrdSysError &log) - : hostUrl(hostUrl), requiresSignature(false), responseCode(0), - includeResponseHeader(false), httpVerb("POST"), m_log(log) { + HTTPRequest(const std::string &hostUrl, XrdSysError &log, + const TokenFile *token) + : hostUrl(hostUrl), m_log(log), m_token(token) { // Parse the URL and populate // What to do if the function returns false? // TODO: Figure out best way to deal with this @@ -75,7 +77,7 @@ class HTTPRequest { std::string hostUrl; std::string protocol; - bool requiresSignature; + bool requiresSignature{false}; struct timespec signatureTime; std::string errorMessage; @@ -84,18 +86,22 @@ class HTTPRequest { std::string resultString; unsigned long responseCode{0}; unsigned long expectedResponseCode = 200; - bool includeResponseHeader; + bool includeResponseHeader{false}; - std::string httpVerb; + std::string httpVerb{"POST"}; std::unique_ptr callback_payload; XrdSysError &m_log; + + private: + const TokenFile *m_token; }; class HTTPUpload : public HTTPRequest { public: - HTTPUpload(const std::string &h, const std::string &o, XrdSysError &log) - : HTTPRequest(h, log), object(o) { + HTTPUpload(const std::string &h, const std::string &o, XrdSysError &log, + const TokenFile *token) + : HTTPRequest(h, log, token), object(o) { hostUrl = hostUrl + "/" + object; } @@ -111,8 +117,9 @@ class HTTPUpload : public HTTPRequest { class HTTPDownload : public HTTPRequest { public: - HTTPDownload(const std::string &h, const std::string &o, XrdSysError &log) - : HTTPRequest(h, log), object(o) { + HTTPDownload(const std::string &h, const std::string &o, XrdSysError &log, + const TokenFile *token) + : HTTPRequest(h, log, token), object(o) { hostUrl = hostUrl + "/" + object; } @@ -126,8 +133,9 @@ class HTTPDownload : public HTTPRequest { class HTTPHead : public HTTPRequest { public: - HTTPHead(const std::string &h, const std::string &o, XrdSysError &log) - : HTTPRequest(h, log), object(o) { + HTTPHead(const std::string &h, const std::string &o, XrdSysError &log, + const TokenFile *token) + : HTTPRequest(h, log, token), object(o) { hostUrl = hostUrl + "/" + object; } diff --git a/src/HTTPFile.cc b/src/HTTPFile.cc index a62907b..e15e9c7 100644 --- a/src/HTTPFile.cc +++ b/src/HTTPFile.cc @@ -111,22 +111,24 @@ int HTTPFile::Open(const char *path, int Oflag, mode_t Mode, XrdOucEnv &env) { return rv; } - // We used to query S3 here to see if the object existed, but of course - // if you're creating a file on upload, you don't care. + m_object = object; + m_hostname = configured_hostname; + m_hostUrl = configured_hostUrl; - this->object = object; - this->hostname = configured_hostname; - this->hostUrl = configured_hostUrl; + if (!Oflag) { + struct stat buf; + return Fstat(&buf); + } return 0; } ssize_t HTTPFile::Read(void *buffer, off_t offset, size_t size) { - HTTPDownload download(this->hostUrl, this->object, m_log); + HTTPDownload download(m_hostUrl, m_object, m_log, m_oss->getToken()); m_log.Log( LogMask::Debug, "HTTPFile::Read", "About to perform download from HTTPFile::Read(): hostname / object:", - hostname.c_str(), object.c_str()); + m_hostname.c_str(), m_object.c_str()); if (!download.SendRequest(offset, size)) { std::stringstream ss; @@ -157,9 +159,9 @@ int HTTPFile::Fstat(struct stat *buff) { } m_log.Log(LogMask::Debug, "HTTPFile::Fstat", - "About to perform HTTPFile::Fstat():", hostUrl.c_str(), - object.c_str()); - HTTPHead head(hostUrl, object, m_log); + "About to perform HTTPFile::Fstat():", m_hostUrl.c_str(), + m_object.c_str()); + HTTPHead head(m_hostUrl, m_object, m_log, m_oss->getToken()); if (!head.SendRequest()) { // SendRequest() returns false for all errors, including ones @@ -247,7 +249,7 @@ int HTTPFile::Fstat(struct stat *buff) { } ssize_t HTTPFile::Write(const void *buffer, off_t offset, size_t size) { - HTTPUpload upload(this->hostUrl, this->object, m_log); + HTTPUpload upload(m_hostUrl, m_object, m_log, m_oss->getToken()); std::string payload((char *)buffer, size); if (!upload.SendRequest(payload, offset, size)) { diff --git a/src/HTTPFile.hh b/src/HTTPFile.hh index df10aca..68642e4 100644 --- a/src/HTTPFile.hh +++ b/src/HTTPFile.hh @@ -95,12 +95,14 @@ class HTTPFile : public XrdOssDF { time_t getLastModified() { return last_modified; } private: + bool m_stat{false}; + XrdSysError &m_log; HTTPFileSystem *m_oss; - std::string hostname; - std::string hostUrl; - std::string object; + std::string m_hostname; + std::string m_hostUrl; + std::string m_object; size_t content_length; time_t last_modified; diff --git a/src/HTTPFileSystem.cc b/src/HTTPFileSystem.cc index ab0d4ee..186147b 100644 --- a/src/HTTPFileSystem.cc +++ b/src/HTTPFileSystem.cc @@ -42,7 +42,7 @@ using namespace XrdHTTPServer; HTTPFileSystem::HTTPFileSystem(XrdSysLogger *lp, const char *configfn, XrdOucEnv *envP) - : m_env(envP), m_log(lp, "httpserver_") { + : m_env(envP), m_log(lp, "httpserver_"), m_token("", &m_log) { m_log.Say("------ Initializing the HTTP filesystem plugin."); if (!Config(lp, configfn)) { throw std::runtime_error("Failed to configure HTTP filesystem plugin."); @@ -86,6 +86,7 @@ bool HTTPFileSystem::Config(XrdSysLogger *lp, const char *configfn) { char *temporary; std::string value; std::string attribute; + std::string token_file; Config.Attach(cfgFD); while ((temporary = Config.GetMyFirstWord())) { attribute = temporary; @@ -109,7 +110,9 @@ bool HTTPFileSystem::Config(XrdSysLogger *lp, const char *configfn) { !handle_required_config(attribute, "httpserver.url_base", value, m_url_base) || !handle_required_config(attribute, "httpserver.storage_prefix", - value, m_storage_prefix)) { + value, m_storage_prefix) || + !handle_required_config(attribute, "httpserver.token_file", value, + token_file)) { Config.Close(); return false; } @@ -128,6 +131,10 @@ bool HTTPFileSystem::Config(XrdSysLogger *lp, const char *configfn) { } } + if (!token_file.empty()) { + m_token = std::move(TokenFile(token_file, &m_log)); + } + int retc = Config.LastError(); if (retc) { m_log.Emsg("Config", -retc, "read config file", configfn); diff --git a/src/HTTPFileSystem.hh b/src/HTTPFileSystem.hh index eebc43e..c888463 100644 --- a/src/HTTPFileSystem.hh +++ b/src/HTTPFileSystem.hh @@ -18,9 +18,12 @@ #pragma once +#include "TokenFile.hh" + #include #include #include +#include #include #include @@ -103,6 +106,7 @@ class HTTPFileSystem : public XrdOss { const std::string &getHTTPHostUrl() const { return http_host_url; } const std::string &getHTTPUrlBase() const { return m_url_base; } const std::string &getStoragePrefix() const { return m_storage_prefix; } + const TokenFile *getToken() const { return &m_token; } protected: XrdOucEnv *m_env; @@ -117,4 +121,5 @@ class HTTPFileSystem : public XrdOss { std::string http_host_url; std::string m_url_base; std::string m_storage_prefix; + TokenFile m_token; }; diff --git a/src/S3Commands.hh b/src/S3Commands.hh index a1ad5e9..8b2e016 100644 --- a/src/S3Commands.hh +++ b/src/S3Commands.hh @@ -37,7 +37,7 @@ class AmazonRequest : public HTTPRequest { const std::string &skf, const std::string &b, const std::string &o, const std::string &style, int sv, XrdSysError &log) - : HTTPRequest(s, log), accessKeyFile(akf), secretKeyFile(skf), + : HTTPRequest(s, log, nullptr), accessKeyFile(akf), secretKeyFile(skf), signatureVersion(sv), bucket(b), object(o), m_style(style) { requiresSignature = true; // Start off by parsing the hostUrl, which we use in conjunction with diff --git a/src/TokenFile.cc b/src/TokenFile.cc new file mode 100644 index 0000000..e0640a1 --- /dev/null +++ b/src/TokenFile.cc @@ -0,0 +1,84 @@ +/*************************************************************** + * + * Copyright (C) 2024, Pelican Project, Morgridge Institute for Research + * + * Licensed under the Apache License, Version 2.0 (the "License"); you + * may not use this file except in compliance with the License. You may + * obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ***************************************************************/ + +#include "TokenFile.hh" +#include "logging.hh" +#include "shortfile.hh" +#include "stl_string_utils.hh" + +#include + +using namespace std::chrono_literals; + +const std::chrono::steady_clock::duration TokenFile::m_token_expiry = 5s; + +// Retrieve the bearer token to use with HTTP requests +// +// Returns true on success and sets `token` to the value of +// the bearer token to use. If there were no errors - but no +// token is to be used - token is set to the empty string. +// Otherwise, returns false. +bool TokenFile::Get(std::string &token) const { + if (m_token_file.empty()) { + token.clear(); + return true; + } + + XrdSysRWLockHelper lock(m_token_mutex.get(), true); + if (m_token_load_success) { + auto now = std::chrono::steady_clock::now(); + if (now - m_last_token_load <= m_token_expiry) { + token = m_token_contents; + return true; + } + } + lock.UnLock(); + + // Upgrade to write lock - we will mutate the data structures. + lock.Lock(m_token_mutex.get(), false); + std::string contents; + if (!readShortFile(m_token_file, contents)) { + if (m_log) { + m_log->Log( + XrdHTTPServer::LogMask::Warning, "getAuthToken", + "Failed to read token authorization file:", strerror(errno)); + } + m_token_load_success = false; + return false; + } + std::istringstream istream; + istream.str(contents); + m_last_token_load = std::chrono::steady_clock::now(); + m_token_load_success = true; + for (std::string line; std::getline(istream, line);) { + trim(line); + if (line.empty()) { + continue; + } + if (line[0] == '#') { + continue; + } + m_token_contents = line; + token = m_token_contents; + return true; + } + // If there are no error reading the file but the file has no tokens, we + // assume this indicates no token should be used. + token = ""; + return true; +} diff --git a/src/TokenFile.hh b/src/TokenFile.hh new file mode 100644 index 0000000..afee102 --- /dev/null +++ b/src/TokenFile.hh @@ -0,0 +1,52 @@ +/*************************************************************** + * + * Copyright (C) 2024, Pelican Project, Morgridge Institute for Research + * + * Licensed under the Apache License, Version 2.0 (the "License"); you + * may not use this file except in compliance with the License. You may + * obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ***************************************************************/ + +#pragma once + +#include +#include + +#include +#include + +// A class representing a bearer token found from a file on disk +class TokenFile { + public: + TokenFile(std::string filename, XrdSysError *log) + : m_log(log), m_token_file(filename), + m_token_mutex(new XrdSysRWLock()) {} + + TokenFile(const TokenFile &) = delete; + TokenFile(TokenFile &&other) noexcept = default; + TokenFile &operator=(TokenFile &&other) noexcept = default; + + bool Get(std::string &) const; + + private: + mutable bool m_token_load_success{false}; + XrdSysError *m_log; + std::string m_token_file; // Location of a file containing a bearer token + // for auth'z. + mutable std::string m_token_contents; // Cached copy of the token itself. + mutable std::chrono::steady_clock::time_point + m_last_token_load; // Last time the token was loaded from disk. + static const std::chrono::steady_clock::duration m_token_expiry; + mutable std::unique_ptr + m_token_mutex; // Note: when we move to C++17, convert to + // std::shared_mutex +}; diff --git a/test/http_tests.cc b/test/http_tests.cc index 49a21c0..e665592 100644 --- a/test/http_tests.cc +++ b/test/http_tests.cc @@ -27,7 +27,7 @@ class TestHTTPRequest : public HTTPRequest { XrdSysLogger log{}; XrdSysError err{&log, "TestS3CommandsLog"}; - TestHTTPRequest(const std::string &url) : HTTPRequest(url, err) {} + TestHTTPRequest(const std::string &url) : HTTPRequest(url, err, nullptr) {} }; TEST(TestHTTPParseProtocol, Test1) { From b3487954a3b19b6910025445dd6ec6f34a049093 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Sun, 9 Jun 2024 12:53:56 -0500 Subject: [PATCH 4/4] Fix compilation issues on Linux --- CMakeLists.txt | 5 +++-- src/TokenFile.hh | 1 + test/CMakeLists.txt | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7378ae5..1238fa4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,6 +10,7 @@ set( CMAKE_BUILD_TYPE Debug ) find_package( Xrootd REQUIRED ) find_package( CURL REQUIRED ) +find_package( Threads REQUIRED ) include (FindPkgConfig) pkg_check_modules(LIBCRYPTO REQUIRED libcrypto) @@ -59,8 +60,8 @@ include_directories(${XROOTD_INCLUDES} ${CURL_INCLUDE_DIRS} ${LIBCRYPTO_INCLUDE_ add_library(XrdS3 SHARED src/S3File.cc src/S3Directory.cc src/S3AccessInfo.cc src/S3FileSystem.cc src/AWSv4-impl.cc src/S3Commands.cc src/HTTPCommands.cc src/TokenFile.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc) add_library(XrdHTTPServer SHARED src/HTTPFile.cc src/HTTPFileSystem.cc src/HTTPCommands.cc src/TokenFile.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc) -target_link_libraries(XrdS3 -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES} tinyxml2::tinyxml2) -target_link_libraries(XrdHTTPServer -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES}) +target_link_libraries(XrdS3 -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES} tinyxml2::tinyxml2 Threads::Threads) +target_link_libraries(XrdHTTPServer -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES} Threads::Threads) # The CMake documentation strongly advises against using these macros; instead, the pkg_check_modules # is supposed to fill out the full path to ${LIBCRYPTO_LIBRARIES}. As of cmake 3.26.1, this does not diff --git a/src/TokenFile.hh b/src/TokenFile.hh index afee102..5453edf 100644 --- a/src/TokenFile.hh +++ b/src/TokenFile.hh @@ -22,6 +22,7 @@ #include #include +#include #include // A class representing a bearer token found from a file on disk diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index f06ebe7..0c68eb2 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -7,6 +7,7 @@ add_executable( s3-gtest s3_tests.cc ../src/S3FileSystem.cc ../src/shortfile.cc ../src/stl_string_utils.cc + ../src/TokenFile.cc ../src/HTTPCommands.cc ../src/S3Commands.cc ) @@ -16,6 +17,7 @@ add_executable( http-gtest http_tests.cc ../src/HTTPFileSystem.cc ../src/HTTPCommands.cc ../src/stl_string_utils.cc + ../src/TokenFile.cc ../src/shortfile.cc ../src/logging.cc )