Web editor: cap a session at 8 hours and name its cookie __Host-s

firmware/lib/web/web.cpp

@@ -69,26 +69,29 @@ std::string cookieToken(const std::string& cookieHeader) {
         while (start < cookieHeader.size() && cookieHeader[start] == ' ') ++start;
         size_t end = cookieHeader.find(';', start);
         if (end == std::string::npos) end = cookieHeader.size();
-        if (cookieHeader.compare(start, 2, "s=") == 0) {
-            const std::string t = cookieHeader.substr(start + 2, end - start - 2);
+        static const std::string kName = "__Host-s=";
+        if (cookieHeader.compare(start, kName.size(), kName) == 0) {
+            const std::string t = cookieHeader.substr(start + kName.size(), end - start - kName.size());
             return isToken(t) ? t : "";
         }
         start = end + 1;
     }
     return "";
 }
 
-void startSession(Session& s, const std::string& token, uint32_t nowMs) {
+void startSession(Session& s, const std::string& token, uint64_t nowMs) {
     s.token = token;
-    s.lastMs = nowMs;
+    s.lastMs = s.startMs = nowMs;
 }
 
 void endSession(Session& s) { s = Session{}; }
 
-void touch(Session& s, uint32_t nowMs) { s.lastMs = nowMs; }
+void touch(Session& s, uint64_t nowMs) { s.lastMs = nowMs; }
 
-bool sessionValid(const Session& s, const std::string& token, uint32_t nowMs) {
-    if (!isToken(s.token) || !isToken(token) || nowMs - s.lastMs >= kSessionMs) return false;  // wrap-safe
+bool sessionValid(const Session& s, const std::string& token, uint64_t nowMs) {
+    if (!isToken(s.token) || !isToken(token) || nowMs - s.lastMs >= kSessionMs ||
+        nowMs - s.startMs >= kSessionMaxMs)
+        return false;
     unsigned char diff = 0;
     for (size_t i = 0; i < kTokenLen; ++i) diff |= static_cast<unsigned char>(s.token[i] ^ token[i]);
     return diff == 0;

firmware/lib/web/web.h

@@ -7,7 +7,8 @@
 
 namespace web {
 
-constexpr uint32_t kSessionMs = 15u * 60 * 1000;  // ends after 15 minutes without a request
+constexpr uint32_t kSessionMs = 15u * 60 * 1000;          // ends after 15 minutes without a request
+constexpr uint32_t kSessionMaxMs = 8u * 60 * 60 * 1000;    // and 8 hours after login, whatever the requests
 constexpr size_t kTokenLen = 32;                  // 16 random bytes as lowercase hex
 
 // True only for the panel's own address, with or without :443 (DNS rebinding).
@@ -19,20 +20,22 @@ bool originAllowed(const std::string& origin, const std::string& ip);
 // One field of an application/x-www-form-urlencoded body, decoded; empty if missing or malformed.
 std::string formValue(const std::string& body, const std::string& name);
 
-// The "s" cookie's value if it's a well-formed token, else empty.
+// The "__Host-s" cookie's value if it's a well-formed token, else empty.
 std::string cookieToken(const std::string& cookieHeader);
 
 // One session at a time: a new login replaces the old one.
 struct Session {
     std::string token;  // empty: none
-    uint32_t lastMs = 0;
+    uint64_t lastMs = 0;  // a 64-bit clock: a 32-bit one wraps and revives an old cookie
+    uint64_t startMs = 0;
 };
 
-void startSession(Session& s, const std::string& token, uint32_t nowMs);
+void startSession(Session& s, const std::string& token, uint64_t nowMs);
 void endSession(Session& s);
-void touch(Session& s, uint32_t nowMs);
+void touch(Session& s, uint64_t nowMs);
 
-// Constant time in the token; false once kSessionMs have passed since the last touch.
-bool sessionValid(const Session& s, const std::string& token, uint32_t nowMs);
+// Constant time in the token; false once kSessionMs have passed since the last touch, or
+// kSessionMaxMs since login.
+bool sessionValid(const Session& s, const std::string& token, uint64_t nowMs);
 
 }  // namespace web

firmware/src/web_server.cpp

@@ -5,6 +5,7 @@
 #include <WiFi.h>
 #include <esp_https_server.h>
 #include <esp_random.h>
+#include <esp_timer.h>
 #include <freertos/FreeRTOS.h>
 #include <freertos/semphr.h>
 #include <mbedtls/ecp.h>
@@ -150,10 +151,13 @@ std::string ip() { return WiFi.localIP().toString().c_str(); }
 
 bool hostOk(httpd_req_t* req) { return web::hostAllowed(header(req, "Host"), ip()); }
 
+// Milliseconds since boot on a clock that doesn't wrap, for the session (millis() wraps at 49.7 days).
+uint64_t nowMs() { return esp_timer_get_time() / 1000; }
+
 bool loggedIn(httpd_req_t* req) {
     const std::string token = web::cookieToken(header(req, "Cookie"));
-    if (!web::sessionValid(g_session, token, millis())) return false;
-    web::touch(g_session, millis());
+    if (!web::sessionValid(g_session, token, nowMs())) return false;
+    web::touch(g_session, nowMs());
     return true;
 }
 
@@ -242,8 +246,8 @@ esp_err_t postLogin(httpd_req_t* req) {
     esp_fill_random(raw, sizeof raw);
     char token[web::kTokenLen + 1];
     for (size_t i = 0; i < sizeof raw; ++i) snprintf(token + 2 * i, 3, "%02x", raw[i]);
-    web::startSession(g_session, token, millis());
-    const std::string cookie = std::string("s=") + token + "; HttpOnly; Secure; SameSite=Strict; Path=/";
+    web::startSession(g_session, token, nowMs());
+    const std::string cookie = std::string("__Host-s=") + token + "; HttpOnly; Secure; SameSite=Strict; Path=/";
     httpd_resp_set_hdr(req, "Set-Cookie", cookie.c_str());
     httpd_resp_set_hdr(req, "Location", "/");
     return reply(req, "303 See Other", "");
@@ -302,7 +306,7 @@ esp_err_t postLogout(httpd_req_t* req) {
     esp_err_t sent = ESP_OK;
     if (!postOk(req, body, sent)) return sent;
     web::endSession(g_session);
-    httpd_resp_set_hdr(req, "Set-Cookie", "s=; HttpOnly; Secure; SameSite=Strict; Path=/; Max-Age=0");
+    httpd_resp_set_hdr(req, "Set-Cookie", "__Host-s=; HttpOnly; Secure; SameSite=Strict; Path=/; Max-Age=0");
     httpd_resp_set_hdr(req, "Location", "/");
     return reply(req, "303 See Other", "");
 }

firmware/test/test_web/test_web.cpp

@@ -39,14 +39,16 @@ void test_a_form_field_is_decoded() {
     TEST_ASSERT_EQUAL_STRING("", web::formValue("password=", "password").c_str());
 }
 
-// The session cookie: only "s", only 32 lowercase hex.
+// The session cookie: only "__Host-s" (the browser takes it only from the panel's own https
+// origin, Path=/, no Domain), only 32 lowercase hex.
 void test_the_session_token_comes_from_its_cookie() {
     const std::string t = "0123456789abcdef0123456789abcdef";
-    TEST_ASSERT_EQUAL_STRING(t.c_str(), web::cookieToken("s=" + t).c_str());
-    TEST_ASSERT_EQUAL_STRING(t.c_str(), web::cookieToken("a=1; s=" + t + "; b=2").c_str());
-    TEST_ASSERT_EQUAL_STRING("", web::cookieToken("xs=" + t).c_str());
-    TEST_ASSERT_EQUAL_STRING("", web::cookieToken("s=" + t.substr(1)).c_str());
-    TEST_ASSERT_EQUAL_STRING("", web::cookieToken("s=" + t + "0").c_str());
+    TEST_ASSERT_EQUAL_STRING(t.c_str(), web::cookieToken("__Host-s=" + t).c_str());
+    TEST_ASSERT_EQUAL_STRING(t.c_str(), web::cookieToken("a=1; __Host-s=" + t + "; b=2").c_str());
+    TEST_ASSERT_EQUAL_STRING("", web::cookieToken("s=" + t).c_str());  // no prefix: any subdomain could set it
+    TEST_ASSERT_EQUAL_STRING("", web::cookieToken("x__Host-s=" + t).c_str());
+    TEST_ASSERT_EQUAL_STRING("", web::cookieToken("__Host-s=" + t.substr(1)).c_str());
+    TEST_ASSERT_EQUAL_STRING("", web::cookieToken("__Host-s=" + t + "0").c_str());
     TEST_ASSERT_EQUAL_STRING("", web::cookieToken("").c_str());
 }
 
@@ -62,10 +64,37 @@ void test_a_session_matches_its_token_until_it_expires() {
     web::touch(s, 2000);
     TEST_ASSERT_TRUE(web::sessionValid(s, t, 2000 + web::kSessionMs - 1));
     TEST_ASSERT_FALSE(web::sessionValid(s, t, 2000 + web::kSessionMs));
-    web::startSession(s, t, 0xFFFFFF00u);  // millis() wraps
-    TEST_ASSERT_TRUE(web::sessionValid(s, t, 0x100u));
+    web::startSession(s, t, 0xFFFFFF00u);  // across where a 32-bit clock would wrap
+    TEST_ASSERT_TRUE(web::sessionValid(s, t, 0x100000100ull));
     web::endSession(s);
-    TEST_ASSERT_FALSE(web::sessionValid(s, t, 0x100u));
+    TEST_ASSERT_FALSE(web::sessionValid(s, t, 0x100000100ull));
+}
+
+// Requests keep a session alive, but never past kSessionMaxMs from login: a stolen cookie in
+// a tab left open doesn't work forever.
+void test_activity_does_not_keep_a_session_past_its_cap() {
+    web::Session s;
+    const std::string t = "0123456789abcdef0123456789abcdef";
+    const uint64_t start = 0xFFFFF000u;  // past where a 32-bit millis() would wrap
+    web::startSession(s, t, start);
+    for (uint64_t now = start; now - start < web::kSessionMaxMs - 1000; now += web::kSessionMs / 2) {
+        web::touch(s, now);
+        TEST_ASSERT_TRUE(web::sessionValid(s, t, now));
+    }
+    web::touch(s, start + web::kSessionMaxMs - 1);
+    TEST_ASSERT_TRUE(web::sessionValid(s, t, start + web::kSessionMaxMs - 1));
+    TEST_ASSERT_FALSE(web::sessionValid(s, t, start + web::kSessionMaxMs));
+}
+
+// A 32-bit clock comes back around after 49.7 days: a cookie left unused that long must not look
+// fresh again.
+void test_a_session_left_unused_for_49_days_stays_ended() {
+    web::Session s;
+    const std::string t = "0123456789abcdef0123456789abcdef";
+    const uint64_t hour = 60ull * 60 * 1000;
+    web::startSession(s, t, 1000);
+    web::touch(s, 1000 + hour);
+    TEST_ASSERT_FALSE(web::sessionValid(s, t, 1000 + hour + (1ull << 32) + 60000));
 }
 
 int main() {
@@ -75,5 +104,7 @@ int main() {
     RUN_TEST(test_a_form_field_is_decoded);
     RUN_TEST(test_the_session_token_comes_from_its_cookie);
     RUN_TEST(test_a_session_matches_its_token_until_it_expires);
+    RUN_TEST(test_activity_does_not_keep_a_session_past_its_cap);
+    RUN_TEST(test_a_session_left_unused_for_49_days_stays_ended);
     return UNITY_END();
 }