Skip to content

Commit 3e87642

Browse files
Copilotbbockelm
andcommitted
Implement per-issuer lock to prevent thundering herd on new issuers
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
1 parent 9418caf commit 3e87642

File tree

2 files changed

+90
-2
lines changed

2 files changed

+90
-2
lines changed

src/scitokens_internal.cpp

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,44 @@ CurlRaii myCurl;
3333

3434
std::mutex key_refresh_mutex;
3535

36+
// Per-issuer mutex map for preventing thundering herd on new issuers
37+
std::mutex issuer_mutex_map_lock;
38+
std::unordered_map<std::string, std::shared_ptr<std::mutex>> issuer_mutexes;
39+
constexpr size_t MAX_ISSUER_MUTEXES = 1000;
40+
41+
// Get or create a mutex for a specific issuer
42+
std::shared_ptr<std::mutex> get_issuer_mutex(const std::string &issuer) {
43+
std::lock_guard<std::mutex> guard(issuer_mutex_map_lock);
44+
45+
auto it = issuer_mutexes.find(issuer);
46+
if (it != issuer_mutexes.end()) {
47+
return it->second;
48+
}
49+
50+
// Prevent resource exhaustion: limit the number of cached mutexes
51+
if (issuer_mutexes.size() >= MAX_ISSUER_MUTEXES) {
52+
// Remove mutexes that are no longer in use
53+
for (auto iter = issuer_mutexes.begin(); iter != issuer_mutexes.end(); ) {
54+
if (iter->second.use_count() == 1) {
55+
// Only we hold a reference, safe to remove
56+
iter = issuer_mutexes.erase(iter);
57+
} else {
58+
++iter;
59+
}
60+
}
61+
62+
// If still at capacity, clear the oldest entries
63+
// Note: In a production system, we might use an LRU cache
64+
if (issuer_mutexes.size() >= MAX_ISSUER_MUTEXES) {
65+
issuer_mutexes.clear();
66+
}
67+
}
68+
69+
auto mutex_ptr = std::make_shared<std::mutex>();
70+
issuer_mutexes[issuer] = mutex_ptr;
71+
return mutex_ptr;
72+
}
73+
3674
} // namespace
3775

3876
namespace scitokens {
@@ -829,8 +867,22 @@ Validator::get_public_key_pem(const std::string &issuer, const std::string &kid,
829867
}
830868
} else {
831869
// No keys in the DB, or they are expired, so get them from the web.
832-
result = get_public_keys_from_web(
833-
issuer, internal::SimpleCurlGet::default_timeout);
870+
// Use per-issuer lock to prevent thundering herd for new issuers
871+
auto issuer_mutex = get_issuer_mutex(issuer);
872+
std::lock_guard<std::mutex> lock(*issuer_mutex);
873+
874+
// Check again if keys are now in DB (another thread may have fetched them)
875+
if (get_public_keys_from_db(issuer, now, result->m_keys,
876+
result->m_next_update)) {
877+
// Keys are now available, use them
878+
result->m_continue_fetch = false;
879+
result->m_do_store = false;
880+
result->m_done = true;
881+
} else {
882+
// Still no keys, fetch them from the web
883+
result = get_public_keys_from_web(
884+
issuer, internal::SimpleCurlGet::default_timeout);
885+
}
834886
}
835887
result->m_issuer = issuer;
836888
result->m_kid = kid;

test/main.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,42 @@ TEST_F(IssuerSecurityTest, SpecialCharacterIssuer) {
941941
EXPECT_NE(error_message.find("\""), std::string::npos);
942942
}
943943

944+
// Test for thundering herd prevention with per-issuer locks
945+
TEST_F(IssuerSecurityTest, ThunderingHerdPrevention) {
946+
char *err_msg = nullptr;
947+
948+
// Create tokens for a new issuer and pre-populate the cache
949+
std::string test_issuer = "https://thundering-herd-test.example.org/gtest";
950+
951+
auto rv = scitoken_set_claim_string(m_token.get(), "iss",
952+
test_issuer.c_str(), &err_msg);
953+
ASSERT_TRUE(rv == 0) << err_msg;
954+
955+
// Store public key for this issuer in the cache
956+
rv = scitoken_store_public_ec_key(test_issuer.c_str(), "1", ec_public,
957+
&err_msg);
958+
ASSERT_TRUE(rv == 0) << err_msg;
959+
960+
char *token_value = nullptr;
961+
rv = scitoken_serialize(m_token.get(), &token_value, &err_msg);
962+
ASSERT_TRUE(rv == 0) << err_msg;
963+
std::unique_ptr<char, decltype(&free)> token_value_ptr(token_value, free);
964+
965+
// Successfully deserialize - the per-issuer lock should prevent thundering herd
966+
// Since we pre-populated the cache, this should succeed without network access
967+
rv = scitoken_deserialize_v2(token_value, m_read_token.get(), nullptr,
968+
&err_msg);
969+
ASSERT_TRUE(rv == 0) << err_msg;
970+
971+
// Verify the issuer claim
972+
char *value;
973+
rv = scitoken_get_claim_string(m_read_token.get(), "iss", &value, &err_msg);
974+
ASSERT_TRUE(rv == 0) << err_msg;
975+
ASSERT_TRUE(value != nullptr);
976+
std::unique_ptr<char, decltype(&free)> value_ptr(value, free);
977+
EXPECT_STREQ(value, test_issuer.c_str());
978+
}
979+
944980
int main(int argc, char **argv) {
945981
::testing::InitGoogleTest(&argc, argv);
946982
return RUN_ALL_TESTS();

0 commit comments

Comments
 (0)