diff --git a/src/scitokens_internal.cpp b/src/scitokens_internal.cpp index 34f3688..a02b5dd 100644 --- a/src/scitokens_internal.cpp +++ b/src/scitokens_internal.cpp @@ -699,39 +699,138 @@ std::string rs256_from_coords(const std::string &e_str, * '/a/b/c' */ std::string normalize_absolute_path(const std::string &path) { - if ((path == "//") || (path == "/") || (path == "")) { - return "/"; - } - std::vector path_components; - auto path_iter = path.begin(); - while (path_iter != path.end()) { - while (*path_iter == '/') { - path_iter++; - } - auto next_path_iter = std::find(path_iter, path.end(), '/'); - std::string component; - component.reserve(std::distance(path_iter, next_path_iter)); - component.assign(path_iter, next_path_iter); - path_components.push_back(component); - path_iter = next_path_iter; - } std::vector path_components_filtered; - path_components_filtered.reserve(path_components.size()); - for (const auto &component : path_components) { + std::stringstream path_stream(path); + std::string component; + while (std::getline(path_stream, component, '/')) { if (component == "..") { - path_components_filtered.pop_back(); + if (!path_components_filtered.empty()) { + path_components_filtered.pop_back(); + } } else if (!component.empty() && component != ".") { path_components_filtered.push_back(component); } } - std::stringstream ss; - for (const auto &component : path_components_filtered) { - ss << "/" << component; + + std::stringstream normalized_path; + for (const auto &normalized_component : path_components_filtered) { + normalized_path << "/" << normalized_component; } - std::string result = ss.str(); + + std::string result = normalized_path.str(); return result.empty() ? "/" : result; } +std::string percent_decode_once(const std::string &input, bool &changed) { + std::string output; + output.reserve(input.size()); + changed = false; + + auto hex_to_int = [](char value) { + if (value >= '0' && value <= '9') { + return value - '0'; + } + if (value >= 'a' && value <= 'f') { + return 10 + (value - 'a'); + } + if (value >= 'A' && value <= 'F') { + return 10 + (value - 'A'); + } + return -1; + }; + + for (size_t idx = 0; idx < input.size(); ++idx) { + if (input[idx] == '%' && idx + 2 < input.size()) { + int upper = hex_to_int(input[idx + 1]); + int lower = hex_to_int(input[idx + 2]); + if (upper >= 0 && lower >= 0) { + output.push_back(static_cast((upper << 4) | lower)); + changed = true; + idx += 2; + continue; + } + } + output.push_back(input[idx]); + } + + return output; +} + +bool normalize_scope_path_strict(const std::string &raw_path, + std::string &normalized_path) { + constexpr size_t MAX_SCOPE_DECODE_PASSES = 4; + + std::vector path_components_filtered; + std::stringstream path_stream(raw_path); + std::string raw_component; + while (std::getline(path_stream, raw_component, '/')) { + std::string decoded_component = raw_component; + for (size_t pass = 0; pass < MAX_SCOPE_DECODE_PASSES; ++pass) { + bool changed = false; + decoded_component = percent_decode_once(decoded_component, changed); + if (decoded_component == "..") { + return false; + } + if (!changed) { + break; + } + } + + // If we still have percent-encodings after the allowed decode passes, + // reject the component to prevent downstream decoders from turning it + // into a traversal such as ".." later. + if (decoded_component.find('%') != std::string::npos) { + return false; + } + + // Scope paths are attacker-controlled token input and must not gain + // broader access while being decoded and canonicalized. + + // After decoding, components must not contain path separators or NULs. + // Otherwise, an encoded slash could be reintroduced as a real path + // segment (e.g., "foo%2F..%2Fbar" -> "foo/../bar"), or "%00" could + // produce a NUL that causes later C-string truncation. + if (decoded_component.find('/') != std::string::npos || + decoded_component.find('\\') != std::string::npos || + decoded_component.find('\0') != std::string::npos) { + return false; + } + + if (decoded_component == "..") { + return false; + } + if (!decoded_component.empty() && decoded_component != ".") { + path_components_filtered.push_back(decoded_component); + } + } + + std::stringstream canonical_path; + for (const auto &component : path_components_filtered) { + canonical_path << "/" << component; + } + normalized_path = canonical_path.str(); + if (normalized_path.empty()) { + normalized_path = "/"; + } + return true; +} + +bool path_matches_scope(const std::string &requested_path, + const std::string &authorized_path) { + // Scope matching must respect path segment boundaries so "/john" does not + // authorize sibling paths such as "/johnathan". + if (authorized_path == "/") { + return true; + } + if (requested_path == authorized_path) { + return true; + } + return requested_path.size() > authorized_path.size() && + requested_path.compare(0, authorized_path.size(), authorized_path) == + 0 && + requested_path[authorized_path.size()] == '/'; +} + } // namespace // static std::unordered_map @@ -1329,7 +1428,9 @@ bool scitokens::Enforcer::scope_validator(const jwt::claim &claim, } else { path = full_authz.substr((++sep_iter)); } - path = normalize_absolute_path(path); + if (!normalize_scope_path_strict(path, path)) { + return false; + } // If we are in compatibility mode and this is a WLCG token, then // translate the authorization names to utilize the SciToken-style @@ -1363,7 +1464,7 @@ bool scitokens::Enforcer::scope_validator(const jwt::claim &claim, me->m_gen_acls.emplace_back(alt_authz, path); } else if (((me->m_test_authz == authz) || (!alt_authz.empty() && (me->m_test_authz == alt_authz))) && - (requested_path.substr(0, path.size()) == path)) { + path_matches_scope(requested_path, path)) { return true; } @@ -1376,7 +1477,7 @@ bool scitokens::Enforcer::scope_validator(const jwt::claim &claim, if (me->m_test_authz.empty()) { me->m_gen_acls.emplace_back("condor", "/WRITE"); } else if ((me->m_test_authz == "condor") && - (requested_path.substr(0, 6) == "/WRITE")) { + path_matches_scope(requested_path, "/WRITE")) { return true; } } diff --git a/test/main.cpp b/test/main.cpp index af2bbb5..61fffa2 100644 --- a/test/main.cpp +++ b/test/main.cpp @@ -396,6 +396,265 @@ TEST_F(SerializeTest, EnforcerScopeTest) { ASSERT_TRUE(found_write); } +TEST_F(SerializeTest, EnforcerScopeRespectsPathBoundaries) { + char *err_msg = nullptr; + + auto rv = scitoken_set_claim_string( + m_token.get(), "aud", "https://demo.scitokens.org/", &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + auto enforcer = enforcer_create("https://demo.scitokens.org/gtest", + &m_audiences_array[0], &err_msg); + ASSERT_TRUE(enforcer != nullptr) << err_msg; + + rv = scitoken_set_claim_string(m_token.get(), "scope", "read:/john", + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + rv = scitoken_set_claim_string(m_token.get(), "ver", "scitoken:2.0", + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + char *token_value = nullptr; + rv = scitoken_serialize(m_token.get(), &token_value, &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + std::unique_ptr token_value_ptr(token_value, free); + + rv = scitoken_deserialize_v2(token_value, m_read_token.get(), nullptr, + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + free(err_msg); + err_msg = nullptr; + + auto expect_access = [&](const char *resource, bool allowed) { + Acl acl; + acl.authz = "read"; + acl.resource = resource; + + int acl_rv = + enforcer_test(enforcer, m_read_token.get(), &acl, &err_msg); + if (allowed) { + EXPECT_EQ(acl_rv, 0) << (err_msg ? err_msg : "unexpected denial"); + EXPECT_EQ(err_msg, nullptr); + } else { + EXPECT_EQ(acl_rv, -1); + ASSERT_NE(err_msg, nullptr); + EXPECT_STREQ(err_msg, "token verification failed: 'scope' claim " + "verification failed."); + } + free(err_msg); + err_msg = nullptr; + }; + + expect_access("/john", true); + expect_access("/john/file", true); + expect_access("/johnathan", false); + expect_access("/johnny", false); + + enforcer_destroy(enforcer); +} + +TEST_F(SerializeTest, EnforcerRootScopeAllowsNormalizedAbsolutePaths) { + char *err_msg = nullptr; + + auto rv = scitoken_set_claim_string( + m_token.get(), "aud", "https://demo.scitokens.org/", &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + auto enforcer = enforcer_create("https://demo.scitokens.org/gtest", + &m_audiences_array[0], &err_msg); + ASSERT_TRUE(enforcer != nullptr) << err_msg; + + rv = scitoken_set_claim_string(m_token.get(), "scope", "read:/", &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + rv = scitoken_set_claim_string(m_token.get(), "ver", "scitoken:2.0", + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + char *token_value = nullptr; + rv = scitoken_serialize(m_token.get(), &token_value, &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + std::unique_ptr token_value_ptr(token_value, free); + + rv = scitoken_deserialize_v2(token_value, m_read_token.get(), nullptr, + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + free(err_msg); + err_msg = nullptr; + + Acl acl; + acl.authz = "read"; + acl.resource = "/users/alice/project/file.txt"; + + rv = enforcer_test(enforcer, m_read_token.get(), &acl, &err_msg); + EXPECT_EQ(rv, 0) << (err_msg ? err_msg : "unexpected denial"); + EXPECT_EQ(err_msg, nullptr); + free(err_msg); + + enforcer_destroy(enforcer); +} + +TEST_F(SerializeTest, EnforcerCompatWriteScopeRespectsPathBoundaries) { + char *err_msg = nullptr; + + auto rv = scitoken_set_claim_string( + m_token.get(), "aud", "https://demo.scitokens.org/", &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + auto enforcer = enforcer_create("https://demo.scitokens.org/gtest", + &m_audiences_array[0], &err_msg); + ASSERT_TRUE(enforcer != nullptr) << err_msg; + + scitoken_set_serialize_profile(m_token.get(), SciTokenProfile::WLCG_1_0); + rv = scitoken_set_claim_string(m_token.get(), "scope", + "compute.modify compute.create " + "compute.cancel", + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + char *token_value = nullptr; + rv = scitoken_serialize(m_token.get(), &token_value, &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + std::unique_ptr token_value_ptr(token_value, free); + + rv = scitoken_deserialize_v2(token_value, m_read_token.get(), nullptr, + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + free(err_msg); + err_msg = nullptr; + + auto expect_access = [&](const char *resource, bool allowed) { + Acl acl; + acl.authz = "condor"; + acl.resource = resource; + + int acl_rv = + enforcer_test(enforcer, m_read_token.get(), &acl, &err_msg); + if (allowed) { + EXPECT_EQ(acl_rv, 0) << (err_msg ? err_msg : "unexpected denial"); + EXPECT_EQ(err_msg, nullptr); + } else { + EXPECT_EQ(acl_rv, -1); + ASSERT_NE(err_msg, nullptr); + EXPECT_STREQ(err_msg, "token verification failed: 'scope' claim " + "verification failed."); + } + free(err_msg); + err_msg = nullptr; + }; + + expect_access("/WRITE", true); + expect_access("/WRITE/job/123", true); + expect_access("/WRITEUP", false); + + enforcer_destroy(enforcer); +} + +TEST_F(SerializeTest, EnforcerRejectsTraversalInScopePaths) { + char *err_msg = nullptr; + + auto rv = scitoken_set_claim_string( + m_token.get(), "aud", "https://demo.scitokens.org/", &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + auto enforcer = enforcer_create("https://demo.scitokens.org/gtest", + &m_audiences_array[0], &err_msg); + ASSERT_TRUE(enforcer != nullptr) << err_msg; + + rv = scitoken_set_claim_string(m_token.get(), "ver", "scitoken:2.0", + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + auto expect_scope_access = [&](const char *scope, const char *resource, + bool allowed) { + int local_rv = + scitoken_set_claim_string(m_token.get(), "scope", scope, &err_msg); + ASSERT_TRUE(local_rv == 0) << err_msg; + + char *token_value = nullptr; + local_rv = scitoken_serialize(m_token.get(), &token_value, &err_msg); + ASSERT_TRUE(local_rv == 0) << err_msg; + std::unique_ptr token_value_ptr(token_value, + free); + + local_rv = scitoken_deserialize_v2(token_value, m_read_token.get(), + nullptr, &err_msg); + ASSERT_TRUE(local_rv == 0) << err_msg; + free(err_msg); + err_msg = nullptr; + + Acl acl; + acl.authz = "read"; + acl.resource = resource; + + local_rv = enforcer_test(enforcer, m_read_token.get(), &acl, &err_msg); + if (allowed) { + EXPECT_EQ(local_rv, 0) << (err_msg ? err_msg : "unexpected denial"); + EXPECT_EQ(err_msg, nullptr); + } else { + EXPECT_EQ(local_rv, -1); + ASSERT_NE(err_msg, nullptr); + EXPECT_STREQ(err_msg, "token verification failed: 'scope' claim " + "verification failed."); + } + free(err_msg); + err_msg = nullptr; + }; + + expect_scope_access("read:/home/user1/..", "/home/user2", false); + expect_scope_access("read:/anything/..", "/etc/passwd", false); + expect_scope_access("read:/foo/%2e%2e/bar", "/bar", false); + expect_scope_access("read:/foo/%2E%2e/bar", "/bar", false); + expect_scope_access("read:/foo/%252e%252e/bar", "/bar", false); + expect_scope_access("read:/home/user1/docs", "/home/user1/docs/file.txt", + true); + + enforcer_destroy(enforcer); +} + +TEST_F(SerializeTest, EnforcerSafelyNormalizesRequestedPathTraversal) { + char *err_msg = nullptr; + + auto rv = scitoken_set_claim_string( + m_token.get(), "aud", "https://demo.scitokens.org/", &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + auto enforcer = enforcer_create("https://demo.scitokens.org/gtest", + &m_audiences_array[0], &err_msg); + ASSERT_TRUE(enforcer != nullptr) << err_msg; + + rv = scitoken_set_claim_string(m_token.get(), "scope", "read:/safe", + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + rv = scitoken_set_claim_string(m_token.get(), "ver", "scitoken:2.0", + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + + char *token_value = nullptr; + rv = scitoken_serialize(m_token.get(), &token_value, &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + std::unique_ptr token_value_ptr(token_value, free); + + rv = scitoken_deserialize_v2(token_value, m_read_token.get(), nullptr, + &err_msg); + ASSERT_TRUE(rv == 0) << err_msg; + free(err_msg); + err_msg = nullptr; + + Acl acl; + acl.authz = "read"; + acl.resource = "/../../safe/file.txt"; + + rv = enforcer_test(enforcer, m_read_token.get(), &acl, &err_msg); + EXPECT_EQ(rv, 0) << (err_msg ? err_msg : "unexpected denial"); + EXPECT_EQ(err_msg, nullptr); + free(err_msg); + + enforcer_destroy(enforcer); +} + } // namespace TEST_F(SerializeTest, DeserializeAsyncTest) {