diff --git a/.github/workflows/merge_conan_build.yml b/.github/workflows/merge_conan_build.yml index 8ff164c6..6a838f68 100644 --- a/.github/workflows/merge_conan_build.yml +++ b/.github/workflows/merge_conan_build.yml @@ -5,6 +5,7 @@ on: push: branches: - stable/v8.x + - stable/v9.x - master jobs: diff --git a/.github/workflows/pr_conan_build.yml b/.github/workflows/pr_conan_build.yml index 07b61b22..ed21e40e 100644 --- a/.github/workflows/pr_conan_build.yml +++ b/.github/workflows/pr_conan_build.yml @@ -4,6 +4,7 @@ on: pull_request: branches: - stable/v8.x + - stable/v9.x - master jobs: diff --git a/.jenkins/Jenkinsfile b/.jenkins/Jenkinsfile index 9ac4c6fd..45866e0a 100644 --- a/.jenkins/Jenkinsfile +++ b/.jenkins/Jenkinsfile @@ -16,18 +16,13 @@ pipeline { steps { script { sh(script: "sed -Ei 's, version = .*\"([[:digit:]]+\\.[[:digit:]]+\\.[[:digit:]]+).*, version = \"\\1-${env.BUILD_NUMBER}\",' conanfile.py") - sh(script: "sed -Ei 's,#LIBCURLFIXTOKEN.*,self.requires(\"libcurl/7.86.0\"\\, override=True),' conanfile.py") - BUILD_MISSING = "--build missing" } } } - stage('Adjust for Testing/Stable') { - when { - branch "${STABLE_BRANCH}" - } + stage('include build missing') { steps { script { - BUILD_MISSING = "" + BUILD_MISSING = "--build missing" } } } @@ -109,7 +104,7 @@ pipeline { } steps { sh "conan user -r ebay-local -p ${ARTIFACTORY_PASS} _service_sds" - sh "conan upload ${PROJECT}/${TAG} -c --all -r ebay-local" + sh "conan upload ${PROJECT}/${TAG} --parallel -c --all -r ebay-local" } } } diff --git a/conanfile.py b/conanfile.py index b4cfe63d..118c9d8b 100644 --- a/conanfile.py +++ b/conanfile.py @@ -8,7 +8,7 @@ class SISLConan(ConanFile): name = "sisl" - version = "9.4.5" + version = "9.4.6" homepage = "https://github.com/eBay/sisl" description = "Library for fast data structures, utilities" @@ -87,6 +87,7 @@ def requirements(self): #LIBCURLFIXTOKEN self.requires("libevent/2.1.12", override=True) self.requires("openssl/1.1.1q", override=True) + self.requires("libcurl/7.86.0") self.requires("xz_utils/5.2.5", override=True) self.requires("zlib/1.2.12", override=True) diff --git a/include/sisl/auth_manager/LRUCache.h b/include/sisl/auth_manager/LRUCache.h new file mode 100644 index 00000000..504141d4 --- /dev/null +++ b/include/sisl/auth_manager/LRUCache.h @@ -0,0 +1,83 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +namespace sisl { + +/** + * + * written by @jiankun + * + * A high performance LRU cache implementation. + * + * The cache provides two atomic operations: + * put(key, value): put an object into the cache. + * get(key): returns a optional reference to the value found by key in cache + * + * Important notes: + * 1. The get() method returns a const reference, any change to the reference + * needs to be done by a Put call. + * 2. The put/get methods are thread safe. + */ +template < typename key_t, typename value_t > +class LRUCache { +public: + using kv_pair_t = std::pair< key_t, value_t >; + using list_iterator_t = typename std::list< kv_pair_t >::iterator; + + explicit LRUCache(size_t capacity) : capacity_(capacity) {} + + template < typename K, typename V > + void put(K&& key, V&& value) { + std::unique_lock< std::shared_mutex > l{mtx_}; + + auto it = items_map_.find(key); + if (it != items_map_.end()) { + items_list_.erase(it->second); + items_map_.erase(it); + } + + items_list_.emplace_front(std::make_pair(std::forward< K >(key), std::forward< V >(value))); + items_map_[key] = items_list_.begin(); + + if (items_map_.size() > capacity_) { + auto last = items_list_.rbegin(); + items_map_.erase(last->first); + items_list_.pop_back(); + } + } + + [[nodiscard]] std::optional< value_t > get(const key_t& key) { + // we need unique lock for the splice operation + std::unique_lock< std::shared_mutex > l{mtx_}; + + auto it = items_map_.find(key); + if (it == items_map_.end()) { return std::nullopt; } + + items_list_.splice(items_list_.begin(), items_list_, it->second); + return std::optional(it->second->second); + } + + bool exists(const key_t& key) const { + std::shared_lock< std::shared_mutex > l{mtx_}; + return items_map_.find(key) != items_map_.end(); + } + + [[nodiscard]] size_t size() const { + std::shared_lock< std::shared_mutex > l{mtx_}; + return items_map_.size(); + } + +private: + std::list< kv_pair_t > items_list_; + std::unordered_map< key_t, list_iterator_t > items_map_; + size_t capacity_; + mutable std::shared_mutex mtx_; +}; + +} // namespace sisl diff --git a/include/sisl/auth_manager/auth_manager.hpp b/include/sisl/auth_manager/auth_manager.hpp index bf5ea957..01885809 100644 --- a/include/sisl/auth_manager/auth_manager.hpp +++ b/include/sisl/auth_manager/auth_manager.hpp @@ -18,14 +18,40 @@ #include #include "security_config.hpp" +#include "LRUCache.h" namespace sisl { ENUM(AuthVerifyStatus, uint8_t, OK, UNAUTH, FORBIDDEN) +template < typename key_t, typename value_t > +class LRUCache; + +/** + * This struct holds information of a token, that can be used as if + * they were extracted from decoded token. + */ +struct CachedToken { + AuthVerifyStatus response_status; + std::string msg; + bool valid; + std::chrono::system_clock::time_point expires_at; + + inline void set_invalid(AuthVerifyStatus code, const std::string& reason) { + valid = false; + response_status = code; + msg = reason; + } + + inline void set_valid() { + valid = true; + response_status = AuthVerifyStatus::OK; + } +}; + class AuthManager { public: - AuthManager() {} + AuthManager(); virtual ~AuthManager() = default; AuthVerifyStatus verify(const std::string& token, std::string& msg) const; @@ -33,5 +59,13 @@ class AuthManager { void verify_decoded(const jwt::decoded_jwt& decoded) const; virtual std::string download_key(const std::string& key_url) const; std::string get_app(const jwt::decoded_jwt& decoded) const; + + // the verify method is declared const. We make this mutable + // as these caches are modified in the verify method. md5_sum(raw_token) -> + // DecodedToken + mutable LRUCache< std::string, CachedToken > m_cached_tokens; + + // key_id -> signing public key + mutable LRUCache< std::string, std::string > m_cached_keys; }; } // namespace sisl diff --git a/include/sisl/logging/logging.h b/include/sisl/logging/logging.h index a0b7778c..c87a47d3 100644 --- a/include/sisl/logging/logging.h +++ b/include/sisl/logging/logging.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -482,6 +483,7 @@ extern bool is_crash_handler_installed(); extern bool restore_signal_handler(const SignalType sig_num); extern bool restore_signal_handlers(); extern bool send_thread_signal(const pthread_t thr, const SignalType sig_num); +extern std::filesystem::path get_base_dir(); template < typename... Args > std::string format_log_msg(const char* const msg, Args&&... args) { diff --git a/src/auth_manager/CMakeLists.txt b/src/auth_manager/CMakeLists.txt index 3e45f388..9fe278d6 100644 --- a/src/auth_manager/CMakeLists.txt +++ b/src/auth_manager/CMakeLists.txt @@ -42,5 +42,16 @@ if (DEFINED ENABLE_TESTING) GTest::gmock ) add_test(NAME AuthManager COMMAND test_auth_mgr) + + add_executable(test_lru_cache) + target_sources(test_lru_cache PRIVATE + tests/LRUCacheTest.cpp + ) + target_link_libraries(test_lru_cache + sisl + ${COMMON_DEPS} + GTest::gmock + ) + add_test(NAME LRUCache COMMAND test_lru_cache) endif() endif() diff --git a/src/auth_manager/auth_manager.cpp b/src/auth_manager/auth_manager.cpp index 38396cca..6e083cd4 100644 --- a/src/auth_manager/auth_manager.cpp +++ b/src/auth_manager/auth_manager.cpp @@ -2,14 +2,58 @@ #include #include +extern "C" { +#include +} #include "sisl/auth_manager/auth_manager.hpp" namespace sisl { +static std::string md5_sum(std::string const& s) { + unsigned char digest[MD5_DIGEST_LENGTH]; + + MD5(reinterpret_cast< unsigned char* >(const_cast< char* >(s.c_str())), s.length(), + reinterpret_cast< unsigned char* >(&digest)); + + std::ostringstream out; + out << std::hex; + for (int i = 0; i < MD5_DIGEST_LENGTH; i++) { + out << std::setfill('0') << std::setw(2) << std::hex << (int)(unsigned char)digest[i]; + } + return out.str(); +} + +struct incomplete_verification_error : std::exception { + explicit incomplete_verification_error(const std::string& error) : error_(error) {} + const char* what() const noexcept { return error_.c_str(); } + +private: + const std::string error_; +}; + +AuthManager::AuthManager() : + m_cached_tokens(SECURITY_DYNAMIC_CONFIG(auth_manager->auth_token_cache_size)), + m_cached_keys(SECURITY_DYNAMIC_CONFIG(auth_manager->auth_key_cache_size)) {} + AuthVerifyStatus AuthManager::verify(const std::string& token, std::string& msg) const { + // if we have it in cache, just use it to make the decision + auto const token_hash = md5_sum(token); + if (auto const ct = m_cached_tokens.get(token_hash); ct) { + if (ct->valid) { + auto now = std::chrono::system_clock::now(); + if (now > ct->expires_at + std::chrono::seconds(SECURITY_DYNAMIC_CONFIG(auth_manager->leeway))) { + m_cached_tokens.put(token_hash, + CachedToken{AuthVerifyStatus::UNAUTH, "token expired", false, ct->expires_at}); + } + } + msg = ct->msg; + return ct->response_status; + } + + // not found in cache + CachedToken cached_token; std::string app_name; - // TODO: cache tokens for better performance try { // this may throw if token is ill formed const auto decoded{jwt::decode(token)}; @@ -18,34 +62,66 @@ AuthVerifyStatus AuthManager::verify(const std::string& token, std::string& msg) // exception is thrown. verify_decoded(decoded); app_name = get_app(decoded); - } catch (const std::exception& e) { + cached_token.expires_at = decoded.get_expires_at(); + cached_token.set_valid(); + } catch (const incomplete_verification_error& e) { + // verification incomplete, the token validity is not determined, shouldn't + // cache msg = e.what(); return AuthVerifyStatus::UNAUTH; + } catch (const std::exception& e) { + cached_token.set_invalid(AuthVerifyStatus::UNAUTH, e.what()); + m_cached_tokens.put(token_hash, cached_token); + msg = cached_token.msg; + return cached_token.response_status; } // check client application if (SECURITY_DYNAMIC_CONFIG(auth_manager->auth_allowed_apps) != "all") { if (SECURITY_DYNAMIC_CONFIG(auth_manager->auth_allowed_apps).find(app_name) == std::string::npos) { - msg = fmt::format("application '{}' is not allowed to perform the request", app_name); - return AuthVerifyStatus::FORBIDDEN; + cached_token.set_invalid(AuthVerifyStatus::FORBIDDEN, + fmt::format("application '{}' is not allowed to perform the request", app_name)); } } - return AuthVerifyStatus::OK; + m_cached_tokens.put(token_hash, cached_token); + msg = cached_token.msg; + return cached_token.response_status; } + void AuthManager::verify_decoded(const jwt::decoded_jwt& decoded) const { const auto alg{decoded.get_algorithm()}; if (alg != "RS256") throw std::runtime_error(fmt::format("unsupported algorithm: {}", alg)); - if (!decoded.has_header_claim("x5u")) throw std::runtime_error("no indication of verification key"); + std::string signing_key; + std::string key_id; + auto should_cache_key = true; - auto key_url = decoded.get_header_claim("x5u").as_string(); + if (decoded.has_key_id()) { + key_id = decoded.get_key_id(); + auto cached_key = m_cached_keys.get(key_id); + if (cached_key) { + signing_key = *cached_key; + should_cache_key = false; + } + } else { + should_cache_key = false; + } - if (key_url.rfind(SECURITY_DYNAMIC_CONFIG(auth_manager->tf_token_url), 0) != 0) { - throw std::runtime_error(fmt::format("key url {} is not trusted", key_url)); + if (signing_key.empty()) { + if (!decoded.has_header_claim("x5u")) throw std::runtime_error("no indication of verification key"); + + auto key_url = decoded.get_header_claim("x5u").as_string(); + + if (key_url.rfind(SECURITY_DYNAMIC_CONFIG(auth_manager->tf_token_url), 0) != 0) { + throw std::runtime_error(fmt::format("key url {} is not trusted", key_url)); + } + signing_key = download_key(key_url); } - const std::string signing_key{download_key(key_url)}; + + if (should_cache_key) { m_cached_keys.put(key_id, signing_key); } + const auto verifier{jwt::verify() .with_issuer(SECURITY_DYNAMIC_CONFIG(auth_manager->issuer)) .allow_algorithm(jwt::algorithm::rs256(signing_key)) diff --git a/src/auth_manager/security_config.fbs b/src/auth_manager/security_config.fbs index e560455b..20cbec5a 100644 --- a/src/auth_manager/security_config.fbs +++ b/src/auth_manager/security_config.fbs @@ -33,6 +33,10 @@ table AuthManager { // ssl verification for the signing key download url verify: bool = true; + + // LRUCache sizes + auth_token_cache_size: uint32 = 2000; + auth_key_cache_size: uint32 = 100; } table SecuritySettings { diff --git a/src/auth_manager/tests/AuthTest.cpp b/src/auth_manager/tests/AuthTest.cpp index 7447a346..79ba44ac 100644 --- a/src/auth_manager/tests/AuthTest.cpp +++ b/src/auth_manager/tests/AuthTest.cpp @@ -210,12 +210,12 @@ TEST_F(AuthTest, trf_allow_valid_token) { EXPECT_EQ(mock_auth_mgr->verify(mock_trf_client.get_token()), AuthVerifyStatus::OK); // use the acces_token saved from the previous call - EXPECT_CALL(*mock_auth_mgr, download_key(_)).Times(1).WillOnce(Return(rsa_pub_key)); + EXPECT_CALL(*mock_auth_mgr, download_key(_)).Times(0); EXPECT_EQ(mock_auth_mgr->verify(mock_trf_client.get_token()), AuthVerifyStatus::OK); // set token to be expired invoking request_with_grant_token mock_trf_client.set_expiry(std::chrono::system_clock::now() - std::chrono::seconds(100)); - EXPECT_CALL(*mock_auth_mgr, download_key(_)).Times(1).WillOnce(Return(rsa_pub_key)); + EXPECT_CALL(*mock_auth_mgr, download_key(_)).Times(0); EXPECT_EQ(mock_auth_mgr->verify(mock_trf_client.get_token()), AuthVerifyStatus::OK); } diff --git a/src/auth_manager/tests/LRUCacheTest.cpp b/src/auth_manager/tests/LRUCacheTest.cpp new file mode 100644 index 00000000..cdb901f4 --- /dev/null +++ b/src/auth_manager/tests/LRUCacheTest.cpp @@ -0,0 +1,73 @@ +#include "sisl/auth_manager/LRUCache.h" +#include +#include +#include +#include + +SISL_OPTIONS_ENABLE(logging) + +namespace sisl::testing { + +using namespace ::testing; + +TEST(LRUTest, basic) { + auto lru = LRUCache< int, int >(3); + + EXPECT_EQ(0, lru.size()); + EXPECT_FALSE(lru.exists(1)); + + lru.put(0, 0); + lru.put(1, 1); + EXPECT_EQ(2, lru.size()); + EXPECT_TRUE(lru.exists(0)); + EXPECT_TRUE(lru.exists(1)); + + lru.put(2, 2); + + // this will evict 0 from cache + lru.put(3, 3); + + EXPECT_EQ(3, lru.size()); + + EXPECT_FALSE(lru.exists(0)); + EXPECT_TRUE(lru.exists(1)); + EXPECT_TRUE(lru.exists(2)); + EXPECT_TRUE(lru.exists(3)); + + // current elements in cache are 3, 2, 1 + // let's re-insert 1, this will move 1 to the head of cache + lru.put(1, 1); + + // insert another new key, this will evict 2 + lru.put(4, 4); + + EXPECT_EQ(3, lru.size()); + EXPECT_FALSE(lru.exists(2)); + EXPECT_TRUE(lru.exists(1)); + EXPECT_TRUE(lru.exists(3)); + EXPECT_TRUE(lru.exists(4)); +} + +TEST(LRUTest, get) { + auto lru = LRUCache< std::string, std::string >(3); + + lru.put("key1", "value1"); + EXPECT_EQ("value1", lru.get("key1")); + auto v = lru.get("no-such-key"); + EXPECT_EQ(std::nullopt, v); + + // use variable as key, to test the perfect forwarding + std::string key{"key2"}; + std::string value{"value2"}; + lru.put(key, value); + ASSERT_TRUE(lru.get(key)); + EXPECT_EQ(value, lru.get(key)); +} + +} // namespace sisl::testing + +int main(int argc, char* argv[]) { + testing::InitGoogleMock(&argc, argv); + SISL_OPTIONS_LOAD(argc, argv, logging) + return RUN_ALL_TESTS(); +} \ No newline at end of file diff --git a/src/file_watcher/file_watcher.cpp b/src/file_watcher/file_watcher.cpp index 05145ba1..94989052 100644 --- a/src/file_watcher/file_watcher.cpp +++ b/src/file_watcher/file_watcher.cpp @@ -119,9 +119,11 @@ bool FileWatcher::unregister_listener(const std::string& file_path, const std::s } bool FileWatcher::remove_watcher(FileInfo& file_info) { - if (auto err = inotify_rm_watch(m_inotify_fd, file_info.m_wd); err != 0) { return false; } + bool success = true; + if (auto err = inotify_rm_watch(m_inotify_fd, file_info.m_wd); err != 0) { success = false; } + // remove the file from the map regardless of the inotify_rm_watch result m_files.erase(file_info.m_filepath); - return true; + return success; } bool FileWatcher::stop() { diff --git a/src/logging/logging.cpp b/src/logging/logging.cpp index df32d266..41fe1f9a 100644 --- a/src/logging/logging.cpp +++ b/src/logging/logging.cpp @@ -135,31 +135,25 @@ std::shared_ptr< spdlog::logger >& GetCriticalLogger() { return logger_thread_ctx.m_critical_logger; } -static std::filesystem::path get_base_dir() { - namespace fs = std::filesystem; - const auto cwd = fs::current_path(); - const auto log_dir{cwd / "logs"}; - - // Construct a unique directory path based on the current time - auto const current_time{std::chrono::system_clock::now()}; - auto const current_t{std::chrono::system_clock::to_time_t(current_time)}; - auto const current_tm{std::localtime(¤t_t)}; - std::array< char, PATH_MAX > c_time; - if (std::strftime(c_time.data(), c_time.size(), "%F_%R", current_tm)) { - const fs::path cur_log_dir = log_dir / c_time.data(); - fs::create_directories(cur_log_dir); - - const fs::path sym_path = log_dir / "latest"; - try { - if (fs::is_symlink(sym_path)) { fs::remove(sym_path); } - fs::create_directory_symlink(cur_log_dir, sym_path); - } catch (std::exception& e) { - LOGINFO("Unable to create latest symlink={} to log dir={}, ignoring symlink creation\n", sym_path, log_dir); +static std::filesystem::path g_base_dir; + +std::filesystem::path get_base_dir() { + static std::once_flag one_base_dir; + std::call_once(one_base_dir, [] { + const auto cwd{std::filesystem::current_path()}; + g_base_dir = cwd / "logs"; + // Construct a unique directory path based on the current time + auto const current_time{std::chrono::system_clock::now()}; + auto const current_t{std::chrono::system_clock::to_time_t(current_time)}; + auto const current_tm{std::localtime(¤t_t)}; + std::array< char, PATH_MAX > c_time; + if (std::strftime(c_time.data(), c_time.size(), "%F_%R", current_tm)) { + g_base_dir /= c_time.data(); + std::filesystem::create_directories(g_base_dir); } - return cur_log_dir; - } else { - return log_dir; - } + }); + + return g_base_dir; } static std::filesystem::path log_path(std::string const& name) { @@ -167,8 +161,7 @@ static std::filesystem::path log_path(std::string const& name) { if (0 < SISL_OPTIONS.count("logfile")) { p = std::filesystem::path{SISL_OPTIONS["logfile"].as< std::string >()}; } else { - static std::filesystem::path base_dir{get_base_dir()}; - p = base_dir / std::filesystem::path{name}.filename(); + p = get_base_dir() / std::filesystem::path{name}.filename(); } return p; } diff --git a/src/logging/stacktrace.cpp b/src/logging/stacktrace.cpp index 1ac07443..ac65d5d1 100644 --- a/src/logging/stacktrace.cpp +++ b/src/logging/stacktrace.cpp @@ -131,7 +131,7 @@ static bool dumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, static void bt_dumper([[maybe_unused]] const SignalType signal_number) { #if defined(__linux__) - google_breakpad::ExceptionHandler::WriteMinidump("./", dumpCallback, nullptr); + google_breakpad::ExceptionHandler::WriteMinidump(get_base_dir().string(), dumpCallback, nullptr); #endif }