From ce4421d87aca8c5a2a944bc2e2681a74ce0af9fa Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:04:01 +0000 Subject: [PATCH] fix(console): mark the session cookie Secure when the server serves TLS The console set its session cookie, and the cookie that clears it on logout, with HttpOnly, SameSite=Strict and Path=/console but without Secure. A browser attaches such a cookie to a plain http:// request to the same host. The login response also sends HSTS, but a browser does not record HSTS for a host given as an IP address or over a connection with certificate errors, which includes the self-signed certificate extenddb init creates. On such a host, an http:// request the browser is led to send carries the session token in clear text, ahead of the server's redirect to HTTPS. ConsoleState gains secure_cookies, set from AppState.tls_enabled, whose doc comment already said it affects the cookie's Secure flag. Both cookies come from session_cookie, which appends Secure when it is set. A dev-mode build serves plain HTTP, where Secure protects nothing and an HTTP client that does not treat the loopback origin as secure, such as the Python requests client the console tests use, would not send the cookie back, so it leaves Secure off. The security model guide lists the attribute. Unit tests check both cookies with and without Secure. The console login and logout tests check that Secure is present exactly when the endpoint is HTTPS. Against a TLS SQLite build and a dev-mode build on 127.0.0.1, the console auth and role tests pass. With secure_cookies forced false, the login and logout tests fail on the TLS build. With it forced true, they fail on the dev-mode build, and so does the dashboard test, because the client no longer sends the cookie over HTTP. Fixes #687. --- crates/server/src/console/mod.rs | 3 + crates/server/src/console/pages/auth_pages.rs | 78 +++++++++++++++++-- crates/server/src/lib.rs | 1 + docs/manuals/10-security-model.md | 2 +- tests/test_console_integration.py | 8 ++ 5 files changed, 86 insertions(+), 6 deletions(-) diff --git a/crates/server/src/console/mod.rs b/crates/server/src/console/mod.rs index 76cc752c7..3ff5fd6a4 100755 --- a/crates/server/src/console/mod.rs +++ b/crates/server/src/console/mod.rs @@ -55,6 +55,9 @@ pub struct ConsoleState { /// `/console/metrics-data` response with latency segments (same data the /// top-level `/metrics` handler adds). pub metrics: Arc, + /// Whether the session cookie carries `Secure`: true when the server + /// serves TLS, false in dev mode, which serves plain HTTP. + pub secure_cookies: bool, } /// Build the console router. diff --git a/crates/server/src/console/pages/auth_pages.rs b/crates/server/src/console/pages/auth_pages.rs index b869e5434..9919b67a8 100755 --- a/crates/server/src/console/pages/auth_pages.rs +++ b/crates/server/src/console/pages/auth_pages.rs @@ -71,9 +71,7 @@ pub async fn login_submit( }; let (token, _csrf) = state.sessions.create(identity).await; - let cookie = format!( - "extenddb_session={token}; Path=/console; HttpOnly; SameSite=Strict; Max-Age=28800" - ); + let cookie = session_cookie(&token, 28800, state.secure_cookies); ( StatusCode::SEE_OTHER, @@ -87,8 +85,7 @@ pub async fn logout(State(state): State>, headers: HeaderMap) if let Some(token) = extract_session_token(&headers) { state.sessions.remove(&token).await; } - let cookie = - "extenddb_session=; Path=/console; HttpOnly; SameSite=Strict; Max-Age=0".to_string(); + let cookie = session_cookie("", 0, state.secure_cookies); ( StatusCode::SEE_OTHER, [("location", "/console/login"), ("set-cookie", &cookie)], @@ -96,6 +93,16 @@ pub async fn logout(State(state): State>, headers: HeaderMap) .into_response() } +/// The `Set-Cookie` value for the console session. With `secure`, the cookie +/// carries `Secure`, so a browser does not attach it to a plain `http://` +/// request to the host (browsers exempt `localhost` from this rule). +fn session_cookie(token: &str, max_age_secs: u32, secure: bool) -> String { + let secure = if secure { "; Secure" } else { "" }; + format!( + "extenddb_session={token}; Path=/console; HttpOnly; SameSite=Strict; Max-Age={max_age_secs}{secure}" + ) +} + fn login_html(error: Option<&str>, listen_url: &str) -> String { let error_html = error.map(html::alert_error).unwrap_or_default(); @@ -173,3 +180,64 @@ async fn try_iam_login( None } } + +#[cfg(test)] +mod session_cookie_tests { + use super::session_cookie; + + /// The `name=value` pair, which must come first, and the attributes after + /// it, sorted, since their order carries no meaning. + fn parts(cookie: &str) -> (&str, Vec<&str>) { + let mut parts = cookie.split(';').map(str::trim); + let pair = parts.next().unwrap_or_default(); + let mut attributes: Vec<&str> = parts.collect(); + attributes.sort_unstable(); + (pair, attributes) + } + + #[test] + fn a_tls_server_marks_the_session_cookie_secure() { + let login = session_cookie("t0k", 28800, true); + let (pair, attributes) = parts(&login); + assert_eq!(pair, "extenddb_session=t0k"); + assert_eq!( + attributes, + [ + "HttpOnly", + "Max-Age=28800", + "Path=/console", + "SameSite=Strict", + "Secure" + ] + ); + let logout = session_cookie("", 0, true); + let (pair, attributes) = parts(&logout); + assert_eq!(pair, "extenddb_session="); + assert_eq!( + attributes, + [ + "HttpOnly", + "Max-Age=0", + "Path=/console", + "SameSite=Strict", + "Secure" + ] + ); + } + + /// Dev mode serves plain HTTP, where `Secure` protects nothing, and an + /// HTTP client that does not treat the loopback origin as secure, such as + /// the Python `requests` client the console tests use, would not send the + /// cookie back. + #[test] + fn a_plain_http_server_leaves_secure_off() { + for cookie in [ + session_cookie("t0k", 28800, false), + session_cookie("", 0, false), + ] { + let (_, attributes) = parts(&cookie); + assert!(!attributes.contains(&"Secure"), "{cookie}"); + assert!(attributes.contains(&"HttpOnly") && attributes.contains(&"SameSite=Strict")); + } + } +} diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index f608c74d7..035c35996 100755 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -204,6 +204,7 @@ pub async fn start_server( docs_store: docs_store.clone(), auth_cache: auth_cache_for_console, metrics: metrics_for_console, + secure_cookies: tls_enabled, }); let console_router = console::router().with_state(console_state); app = app diff --git a/docs/manuals/10-security-model.md b/docs/manuals/10-security-model.md index bb7a1c75d..97c619277 100755 --- a/docs/manuals/10-security-model.md +++ b/docs/manuals/10-security-model.md @@ -169,7 +169,7 @@ Replace the certificate and key files, then restart extenddb. There is no hot-re The management web console (`/console/*`) implements: - **CSRF protection**: Unique tokens generated per session, injected into all forms via JavaScript, validated on every POST handler -- **Session management**: Server-side sessions with 8-hour expiry, HttpOnly cookies with `SameSite=Strict` and `Path=/console` +- **Session management**: Server-side sessions with 8-hour expiry, HttpOnly and `Secure` cookies with `SameSite=Strict` and `Path=/console` (a dev-mode build serves plain HTTP and leaves `Secure` off) - **Security headers**: X-Content-Type-Options (nosniff), X-Frame-Options (DENY), Referrer-Policy (strict-origin-when-cross-origin) - **HSTS**: Sent automatically (TLS is always enabled) - **Login rate limiting**: Failed login attempts are tracked per user. Excessive failures trigger account lockout. diff --git a/tests/test_console_integration.py b/tests/test_console_integration.py index 402d9b2d4..e4ca9acc4 100755 --- a/tests/test_console_integration.py +++ b/tests/test_console_integration.py @@ -141,12 +141,19 @@ def setup(self, auth_env): self.admin_user = auth_env[1] self.admin_pass = auth_env[2] + def _assert_secure_matches_scheme(self, set_cookie: str) -> None: + """The session cookie carries Secure exactly when the console is served + over HTTPS (dev mode serves plain HTTP).""" + attrs = [a.strip() for a in set_cookie.split(";")] + assert ("Secure" in attrs) == self.endpoint.startswith("https://"), set_cookie + def test_login_redirects_to_dashboard(self): client = ConsoleClient(self.endpoint) resp = client.login(self.admin_user, self.admin_pass) assert resp.status_code == 303 assert "/console" in resp.headers.get("location", "") assert "extenddb_session=" in resp.headers.get("set-cookie", "") + self._assert_secure_matches_scheme(resp.headers.get("set-cookie", "")) def test_login_invalid_credentials(self): client = ConsoleClient(self.endpoint) @@ -161,6 +168,7 @@ def test_logout_clears_session(self): resp = client.logout() assert resp.status_code == 303 assert "/console/login" in resp.headers.get("location", "") + self._assert_secure_matches_scheme(resp.headers.get("set-cookie", "")) # After logout, accessing dashboard should redirect to login. resp = client.get("") assert resp.status_code == 303