extractToken: recognize the __Host-session cookie name (fix session regression)
`extractToken` only ever looked up `cookieValue(cookie, "session")`. After the
M-1 fix switched cookie parsing from substring to exact-name matching
(fafee12), a consumer that sets its session cookie under the hardened
`__Host-session` name (fewo-webapp, #535) stopped resolving — every browser
cookie request extracted an empty token and 401'd. The bare-`session`
substring used to incidentally match inside `__Host-session=`; exact matching
correctly no longer does.
Fix: add `sessionCookieToken(cookieHeader)` which tries the bare `session`
name and falls back to `__Host-session` (the `__Host-` prefix is strictly more
secure, so recognizing it is safe), and route `extractToken` through it. The
bare name is preferred when both are present. `cookieValue` keeps its exact
generic semantics unchanged. Backward-compatible: consumers using `session=`
are unaffected.
Tests: new `test_session_cookie_token` covering both names, precedence, and
substring traps. All 20 ctest targets pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9976efe1de
commit
0436139c87
2 changed files with 46 additions and 4 deletions
|
|
@ -47,17 +47,36 @@ inline std::string cookieValue(const std::string& cookieHeader, const std::strin
|
||||||
return "";
|
return "";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Read the session token from a `Cookie` header value under either the
|
||||||
|
* bare `session` name or the hardened `__Host-session` name.
|
||||||
|
*
|
||||||
|
* The `__Host-` cookie prefix (RFC 6265bis) is strictly *more* secure — it
|
||||||
|
* forces `Secure`, `Path=/` and host-only scoping — so a consumer that sets
|
||||||
|
* `__Host-session` must still be recognised here. `cookieValue` matches names
|
||||||
|
* exactly (see M-1), so the bare-`session` lookup does not match a
|
||||||
|
* `__Host-session` cookie; we fall back to the prefixed name explicitly. The
|
||||||
|
* bare name is preferred when both are present. This closes a regression where
|
||||||
|
* the M-1 exact-name parse silently stopped resolving `__Host-session`
|
||||||
|
* sessions (consumers renaming their cookie to `__Host-session` got 401s).
|
||||||
|
*/
|
||||||
|
inline std::string sessionCookieToken(const std::string& cookieHeader) {
|
||||||
|
std::string tok = cookieValue(cookieHeader, "session");
|
||||||
|
if (tok.empty()) tok = cookieValue(cookieHeader, "__Host-session");
|
||||||
|
return tok;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @brief Pull the session token from an incoming request.
|
* @brief Pull the session token from an incoming request.
|
||||||
*
|
*
|
||||||
* Order of precedence: `Cookie: session=...` → `Authorization: Bearer ...`.
|
* Order of precedence: `Cookie: session=...` (or `__Host-session=...`) →
|
||||||
* Returns "" when no token is present. Does not validate the token — callers
|
* `Authorization: Bearer ...`. Returns "" when no token is present. Does not
|
||||||
* hash it and look it up in their session store.
|
* validate the token — callers hash it and look it up in their session store.
|
||||||
*/
|
*/
|
||||||
inline std::string extractToken(const std::shared_ptr<IncomingRequest>& request) {
|
inline std::string extractToken(const std::shared_ptr<IncomingRequest>& request) {
|
||||||
auto cookie = request->getHeader("Cookie");
|
auto cookie = request->getHeader("Cookie");
|
||||||
if (cookie && !cookie->empty()) {
|
if (cookie && !cookie->empty()) {
|
||||||
std::string tok = cookieValue(*cookie, "session");
|
std::string tok = sessionCookieToken(*cookie);
|
||||||
if (!tok.empty()) return tok;
|
if (!tok.empty()) return tok;
|
||||||
}
|
}
|
||||||
auto auth = request->getHeader("Authorization");
|
auto auth = request->getHeader("Authorization");
|
||||||
|
|
|
||||||
|
|
@ -47,6 +47,28 @@ void test_cookie_exact_name_match() {
|
||||||
REQUIRE(cookieValue("__Host-session=tok", "session") == "");
|
REQUIRE(cookieValue("__Host-session=tok", "session") == "");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void test_session_cookie_token() {
|
||||||
|
// Bare `session` name resolves.
|
||||||
|
REQUIRE(sessionCookieToken("session=abc") == "abc");
|
||||||
|
REQUIRE(sessionCookieToken("other=1; session=abc; more=2") == "abc");
|
||||||
|
|
||||||
|
// Regression: the `__Host-session` hardened name must also resolve — the
|
||||||
|
// exact-name parse (M-1) otherwise silently 401s consumers that renamed
|
||||||
|
// their session cookie to `__Host-session`.
|
||||||
|
REQUIRE(sessionCookieToken("__Host-session=tok") == "tok");
|
||||||
|
REQUIRE(sessionCookieToken("other=1; __Host-session=tok") == "tok");
|
||||||
|
REQUIRE(sessionCookieToken("__Host-session=tok; x=1") == "tok");
|
||||||
|
|
||||||
|
// The bare name is preferred when both are somehow present.
|
||||||
|
REQUIRE(sessionCookieToken("session=real; __Host-session=other") == "real");
|
||||||
|
|
||||||
|
// Substring traps still don't match either recognised name.
|
||||||
|
REQUIRE(sessionCookieToken("x__Host-session=evil") == "");
|
||||||
|
REQUIRE(sessionCookieToken("xsession=evil") == "");
|
||||||
|
REQUIRE(sessionCookieToken("") == "");
|
||||||
|
REQUIRE(sessionCookieToken("foo=bar") == "");
|
||||||
|
}
|
||||||
|
|
||||||
void test_is_valid_ip() {
|
void test_is_valid_ip() {
|
||||||
REQUIRE(isValidIp("192.168.1.1"));
|
REQUIRE(isValidIp("192.168.1.1"));
|
||||||
REQUIRE(isValidIp("::1"));
|
REQUIRE(isValidIp("::1"));
|
||||||
|
|
@ -61,6 +83,7 @@ void test_is_valid_ip() {
|
||||||
|
|
||||||
int main() {
|
int main() {
|
||||||
test_cookie_exact_name_match();
|
test_cookie_exact_name_match();
|
||||||
|
test_session_cookie_token();
|
||||||
test_is_valid_ip();
|
test_is_valid_ip();
|
||||||
std::printf("%s (%d failures)\n", g_failures ? "FAIL" : "OK", g_failures);
|
std::printf("%s (%d failures)\n", g_failures ? "FAIL" : "OK", g_failures);
|
||||||
return g_failures ? 1 : 0;
|
return g_failures ? 1 : 0;
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue