From 70f59b64f546e6205158778bb555ca0fe39c98d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 07:12:05 +0000 Subject: [PATCH 1/3] httpd: four login sessions instead of one Logging in from a second browser ended the first one's session at its next request: two browsers open on the switch logged each other out every few seconds. Up to four sessions now, each with its own cookie and expiry; a fifth login replaces the least recently used. The expiry check is a leaf function so its 32-bit scratch stays out of internal RAM (stack still 159 bytes with HEALTH). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Tx2W7xVecMoKbBCkD2i8PG (cherry picked from commit 03330f4077ef8872d861d951c8e44ece3c573935) --- httpd/httpd.c | 64 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/httpd/httpd.c b/httpd/httpd.c index 688513f8..93828d34 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -75,10 +75,15 @@ __xdata uint8_t postbody_endpoint; __xdata uint16_t postbody_start; __xdata char passwd[21]; -__xdata char session_id[SESSION_ID_LENGTH + 1]; +/* Up to SESSIONS logins at once, each with its own cookie and its own + * expiry. With a single one, logging in from a second browser (or a phone) + * ended the first browser's session at its next request. */ +#define SESSIONS 4 +__xdata char session_id[SESSIONS][SESSION_ID_LENGTH + 1]; +__xdata uint32_t last_session_use[SESSIONS]; +__xdata uint8_t session_slot; __xdata uint8_t authenticated; __xdata uint32_t now; -__xdata uint32_t last_session_use; #define TSTATE_NONE 0 #define TSTATE_TX 1 @@ -109,6 +114,33 @@ void httpd_init(void) __banked // handling one, and nothing has set it yet at init time. for (uint8_t i = 0; i < UIP_CONNS; i++) uip_conns[i].appstate.tstate = TSTATE_CLOSED; + for (session_slot = 0; session_slot < SESSIONS; session_slot++) + session_id[session_slot][0] = NUL; +} + +/* Leaf functions, so the compiler keeps their 32-bit scratch in the overlay + * rather than in internal RAM. */ +static uint8_t session_stale(void) +{ + if (now - last_session_use[session_slot] > session_timeout) + return 1; + return 0; +} + +/* For a new login: a free or expired slot, else the least recently used. */ +static void session_pick(void) +{ + __xdata uint8_t i; + __xdata uint8_t oldest = 0; + + for (i = 0; i < SESSIONS; i++) { + session_slot = i; + if (!session_id[i][0] || session_stale()) + return; + if (last_session_use[i] < last_session_use[oldest]) + oldest = i; + } + session_slot = oldest; } @@ -343,15 +375,17 @@ __xdata uint8_t *scan_header(__xdata uint8_t * __xdata p) read_reg_timer(&now); - if (session && session_id[0]) { - if (now - last_session_use > session_timeout) { - dbg_string("Session expired\n"); - } else { - if (is_word_x(session, session_id)) { + if (session) { + for (session_slot = 0; session_slot < SESSIONS; session_slot++) { + if (!session_id[session_slot][0]) + continue; + if (session_stale()) { + dbg_string("Session expired\n"); + session_id[session_slot][0] = NUL; + } else if (is_word_x(session, session_id[session_slot])) { authenticated = 1; - last_session_use = now; - } else { - dbg_string("Invalid session cookie!\n"); + last_session_use[session_slot] = now; + break; } } } @@ -675,13 +709,15 @@ static void run_login_body(__xdata uint8_t *body) { if (strstart(body, "pwd=") && is_url_word_x(body + 4, passwd)) { dbg_string("Password accepted!\n"); - read_reg_timer(&last_session_use); - gen_random_hex_chars(session_id, SESSION_ID_LENGTH); - session_id[SESSION_ID_LENGTH] = NUL; + read_reg_timer(&now); + session_pick(); + last_session_use[session_slot] = now; + gen_random_hex_chars(session_id[session_slot], SESSION_ID_LENGTH); + session_id[session_slot][SESSION_ID_LENGTH] = NUL; slen = strtox(outbuf, "HTTP/1.1 302 Found\r\nConnection: close\r\nLocation: index.html\r\n" \ "Set-Cookie: session="); for (uint8_t i = 0; i < SESSION_ID_LENGTH; i++) - outbuf[slen++] = session_id[i]; + outbuf[slen++] = session_id[session_slot][i]; slen += strtox(outbuf + slen, "; SameSite=Strict\r\n\r\n"); } else { dbg_string("Password invalid!\n"); From a7db0ca8b6ced18cd839ad3d309d20c3724eb2fd Mon Sep 17 00:00:00 2001 From: Brunoz78 Date: Wed, 21 Oct 2026 10:13:39 +0200 Subject: [PATCH 2/3] httpd: keep the login sessions in bank 3 The web server's bank had no room left for the four sessions once the original firmware's more robust header handling came in. Picking a slot, checking a cookie and starting a session move to sessions.c in bank 3; httpd.c only calls session_init(), session_check() and session_new(). The random id is written straight into the reply, which drops gen_random_hex_chars() from bank 1. Co-Authored-By: Claude Opus 5.5 --- Makefile | 1 + httpd/httpd.c | 78 +++--------------------------------- sessions.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++ sessions.h | 19 +++++++++ test/Makefile | 2 + 5 files changed, 137 insertions(+), 72 deletions(-) create mode 100644 sessions.c create mode 100644 sessions.h diff --git a/Makefile b/Makefile index 90fbcebf..882a1c16 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,7 @@ SRCS = \ sfp.c \ dns.c \ dns_cli.c \ + sessions.c \ ntp.c \ ntp_cli.c \ syslog.c \ diff --git a/httpd/httpd.c b/httpd/httpd.c index 93828d34..b383d8de 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -6,12 +6,12 @@ #include "cmd_parser.h" #include "rtl837x_flash.h" #include "uip.h" +#include "sessions.h" #include "html_data.h" // #define DEBUG #include "debug.h" -#define SESSION_ID_LENGTH 12 #define SESSION_TIMEOUT_DEFAULT 200 /* Seconds of inactivity after which a session cookie stops validating. @@ -75,13 +75,7 @@ __xdata uint8_t postbody_endpoint; __xdata uint16_t postbody_start; __xdata char passwd[21]; -/* Up to SESSIONS logins at once, each with its own cookie and its own - * expiry. With a single one, logging in from a second browser (or a phone) - * ended the first browser's session at its next request. */ -#define SESSIONS 4 -__xdata char session_id[SESSIONS][SESSION_ID_LENGTH + 1]; -__xdata uint32_t last_session_use[SESSIONS]; -__xdata uint8_t session_slot; +// Up to four logins at once, see sessions.c __xdata uint8_t authenticated; __xdata uint32_t now; @@ -114,33 +108,7 @@ void httpd_init(void) __banked // handling one, and nothing has set it yet at init time. for (uint8_t i = 0; i < UIP_CONNS; i++) uip_conns[i].appstate.tstate = TSTATE_CLOSED; - for (session_slot = 0; session_slot < SESSIONS; session_slot++) - session_id[session_slot][0] = NUL; -} - -/* Leaf functions, so the compiler keeps their 32-bit scratch in the overlay - * rather than in internal RAM. */ -static uint8_t session_stale(void) -{ - if (now - last_session_use[session_slot] > session_timeout) - return 1; - return 0; -} - -/* For a new login: a free or expired slot, else the least recently used. */ -static void session_pick(void) -{ - __xdata uint8_t i; - __xdata uint8_t oldest = 0; - - for (i = 0; i < SESSIONS; i++) { - session_slot = i; - if (!session_id[i][0] || session_stale()) - return; - if (last_session_use[i] < last_session_use[oldest]) - oldest = i; - } - session_slot = oldest; + session_init(); } @@ -375,40 +343,11 @@ __xdata uint8_t *scan_header(__xdata uint8_t * __xdata p) read_reg_timer(&now); - if (session) { - for (session_slot = 0; session_slot < SESSIONS; session_slot++) { - if (!session_id[session_slot][0]) - continue; - if (session_stale()) { - dbg_string("Session expired\n"); - session_id[session_slot][0] = NUL; - } else if (is_word_x(session, session_id[session_slot])) { - authenticated = 1; - last_session_use[session_slot] = now; - break; - } - } - } + if (session) + session_check(); return p; } -/* - * Generate random HEX-chars at the buffer location. - */ -void gen_random_hex_chars(__xdata uint8_t * b, __xdata uint8_t bytes) -{ - uint8_t i = 0; - while (bytes) { - if (!i) - get_random_32(); - b[--bytes] = itohex(sfr_data[i]); - if (!bytes) { break; } - b[--bytes] = itohex(sfr_data[i] >> 4 | sfr_data[i] << 4); - i = (i + 1) & 0x3; - } -} - - /* 0: body incomplete, 1: configuration stored, 2: malformed */ static uint8_t config_take(void) { @@ -710,14 +649,9 @@ static void run_login_body(__xdata uint8_t *body) if (strstart(body, "pwd=") && is_url_word_x(body + 4, passwd)) { dbg_string("Password accepted!\n"); read_reg_timer(&now); - session_pick(); - last_session_use[session_slot] = now; - gen_random_hex_chars(session_id[session_slot], SESSION_ID_LENGTH); - session_id[session_slot][SESSION_ID_LENGTH] = NUL; slen = strtox(outbuf, "HTTP/1.1 302 Found\r\nConnection: close\r\nLocation: index.html\r\n" \ "Set-Cookie: session="); - for (uint8_t i = 0; i < SESSION_ID_LENGTH; i++) - outbuf[slen++] = session_id[session_slot][i]; + session_new(); slen += strtox(outbuf + slen, "; SameSite=Strict\r\n\r\n"); } else { dbg_string("Password invalid!\n"); diff --git a/sessions.c b/sessions.c new file mode 100644 index 00000000..b8954cc8 --- /dev/null +++ b/sessions.c @@ -0,0 +1,109 @@ +/* + * Login sessions of the web interface. + * + * Up to SESSIONS logins at once, each with its own cookie and its own expiry. + * With a single one, logging in from a second browser (or a phone) ended the + * first browser's session at its next request. + * + * This sits in bank 3: the web server's own bank is full, so httpd.c only + * calls in here. It shares three things with httpd.c: the cookie value it + * parsed (session), the time read for the request (now) and the result + * (authenticated); the reply is written to outbuf at slen. + */ + +#pragma codeseg BANK3 +#pragma constseg BANK3 + +#include +#include "rtl837x_common.h" +#include "sessions.h" + +extern volatile __xdata uint8_t sfr_data[4]; +extern __xdata uint16_t session_timeout; +extern __xdata uint32_t now; +extern __xdata uint8_t authenticated; +extern __xdata uint8_t * __xdata session; +extern __xdata uint8_t outbuf[]; +extern __xdata uint16_t slen; + +__xdata char session_id[SESSIONS][SESSION_ID_LENGTH + 1]; +__xdata uint32_t last_session_use[SESSIONS]; + +static __xdata uint8_t ss_slot, ss_i, ss_oldest, ss_c; + +static __code const char ss_hex[] = "0123456789abcdef"; +/* What may follow a cookie value: ';' separates cookies in a Cookie header */ +static __code const char ss_end[] = " \t:?=\n\r;"; + +/* Leaf function, so the compiler keeps its 32-bit scratch in the overlay + * rather than in internal RAM. */ +static uint8_t ss_stale(void) +{ + if (now - last_session_use[ss_slot] > session_timeout) + return 1; + return 0; +} + +/* Whether the cookie value is the id of slot ss_slot */ +static uint8_t ss_match(void) +{ + for (ss_i = 0; ss_i < SESSION_ID_LENGTH; ss_i++) { + if (session[ss_i] != session_id[ss_slot][ss_i]) + return 0; + } + ss_c = session[SESSION_ID_LENGTH]; + if (!ss_c) + return 1; + for (ss_i = 0; ss_end[ss_i]; ss_i++) { + if (ss_c == ss_end[ss_i]) + return 1; + } + return 0; +} + +void session_init(void) __banked +{ + for (ss_slot = 0; ss_slot < SESSIONS; ss_slot++) + session_id[ss_slot][0] = NUL; +} + +void session_check(void) __banked +{ + for (ss_slot = 0; ss_slot < SESSIONS; ss_slot++) { + if (!session_id[ss_slot][0]) + continue; + if (ss_stale()) { + session_id[ss_slot][0] = NUL; + } else if (ss_match()) { + authenticated = 1; + last_session_use[ss_slot] = now; + return; + } + } +} + +void session_new(void) __banked +{ + /* A free or expired slot, else the least recently used */ + ss_oldest = 0; + for (ss_slot = 0; ss_slot < SESSIONS; ss_slot++) { + if (!session_id[ss_slot][0] || ss_stale()) + break; + if (last_session_use[ss_slot] < last_session_use[ss_oldest]) + ss_oldest = ss_slot; + } + if (ss_slot == SESSIONS) + ss_slot = ss_oldest; + + last_session_use[ss_slot] = now; + for (ss_i = 0; ss_i < SESSION_ID_LENGTH; ss_i++) { + if (!(ss_i & 7)) + get_random_32(); + ss_c = sfr_data[(ss_i >> 1) & 3]; + if (ss_i & 1) + ss_c >>= 4; + session_id[ss_slot][ss_i] = ss_hex[ss_c & 0xf]; + outbuf[slen++] = ss_hex[ss_c & 0xf]; + } + session_id[ss_slot][SESSION_ID_LENGTH] = NUL; +} diff --git a/sessions.h b/sessions.h new file mode 100644 index 00000000..66577a4c --- /dev/null +++ b/sessions.h @@ -0,0 +1,19 @@ +#ifndef _SESSIONS_H_ +#define _SESSIONS_H_ + +#include + +#define SESSIONS 4 +#define SESSION_ID_LENGTH 12 + +extern __xdata char session_id[SESSIONS][SESSION_ID_LENGTH + 1]; +extern __xdata uint32_t last_session_use[SESSIONS]; + +/* No session at boot */ +void session_init(void) __banked; +/* Sets authenticated if the cookie in session belongs to a live session */ +void session_check(void) __banked; +/* Starts a session at now and writes its id to outbuf at slen */ +void session_new(void) __banked; + +#endif diff --git a/test/Makefile b/test/Makefile index 6b145fb1..388d06bd 100644 --- a/test/Makefile +++ b/test/Makefile @@ -72,6 +72,8 @@ $(BUILDDIR)/test_lacp: test_lacp.c $(SRCROOT)/rtl837x_lacp.c | $(BUILDDIR) # this test carries the environment below them itself. $(BUILDDIR)/test_httpd_tx: test_httpd_tx.c $(SRCROOT)/httpd/httpd.c $(SRCROOT)/uip/uip.c | $(BUILDDIR) $(CC) $(CFLAGS) $(filter %.c,$^) -o $@ $(LDFLAGS) +# the login sessions httpd.c calls into +$(BUILDDIR)/test_httpd_tx: $(SRCROOT)/sessions.c # CPU VLAN initialization with full and reduced user-port ranges. $(BUILDDIR)/test_cpu_vlan_restricted: CFLAGS += -DMACHINE_MAX_PORT=7 From e1c79ec4cd761b6cc8461092f6d3c2e52bb288ae Mon Sep 17 00:00:00 2001 From: Brunoz78 Date: Sun, 25 Oct 2026 10:35:12 +0100 Subject: [PATCH 3/3] httpd: reuse the slot of a valid cookie, end sessions on passwd Review feedback on #560 (DrDoof): - A login that came with a cookie that is still valid took a second slot. The first one looked fresh, so the next login could push out someone who is using the switch. The slot of the cookie now gets the new id. - Changing the password leaves no old login valid: session_init() from parse_passwd(). With one session the next login pushed a leftover one out, with several it would stay valid until the next restart. - The 32-bit comparison of the last use times moves into a leaf function, so the compiler does not give it a 4-byte scratch in internal RAM. Co-Authored-By: Claude Sonnet 5.5 --- cmd_parser.c | 4 +++ sessions.c | 37 +++++++++++++++----- test/test_httpd_tx.c | 81 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 110 insertions(+), 12 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index ab3a5894..0da626e4 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -24,6 +24,7 @@ #include "sfp.h" #include "dhcp.h" #include "syslog.h" +#include "sessions.h" #include "dns.h" #include "ntp.h" #include "uip/uip.h" @@ -1479,6 +1480,9 @@ void parse_passwd(void) passwd[j++] = c; } while (c != NUL && j < 20); passwd[j] = NUL; + /* Logins made with the old password end here. With one session the next + * login pushed a leftover one out; with several it would stay valid. */ + session_init(); return; } cmd_error("Missing password\n"); diff --git a/sessions.c b/sessions.c index b8954cc8..6be20b53 100644 --- a/sessions.c +++ b/sessions.c @@ -30,6 +30,9 @@ __xdata char session_id[SESSIONS][SESSION_ID_LENGTH + 1]; __xdata uint32_t last_session_use[SESSIONS]; static __xdata uint8_t ss_slot, ss_i, ss_oldest, ss_c; +/* The slot whose cookie the last request came with; only meaningful while + * authenticated is set, which only session_check() does */ +static __xdata uint8_t ss_hit; static __code const char ss_hex[] = "0123456789abcdef"; /* What may follow a cookie value: ';' separates cookies in a Cookie header */ @@ -44,6 +47,14 @@ static uint8_t ss_stale(void) return 0; } +/* Likewise a leaf: the 32-bit comparison needs no stack of its own this way */ +static uint8_t ss_older(void) +{ + if (last_session_use[ss_slot] < last_session_use[ss_oldest]) + return 1; + return 0; +} + /* Whether the cookie value is the id of slot ss_slot */ static uint8_t ss_match(void) { @@ -76,6 +87,7 @@ void session_check(void) __banked session_id[ss_slot][0] = NUL; } else if (ss_match()) { authenticated = 1; + ss_hit = ss_slot; last_session_use[ss_slot] = now; return; } @@ -84,16 +96,23 @@ void session_check(void) __banked void session_new(void) __banked { - /* A free or expired slot, else the least recently used */ - ss_oldest = 0; - for (ss_slot = 0; ss_slot < SESSIONS; ss_slot++) { - if (!session_id[ss_slot][0] || ss_stale()) - break; - if (last_session_use[ss_slot] < last_session_use[ss_oldest]) - ss_oldest = ss_slot; + if (authenticated && session_id[ss_hit][0]) { + /* The login came with a cookie that is still valid: its slot gets the + * new id. A second slot for the same browser would leave the old one + * looking fresh and push out someone else at the next login. */ + ss_slot = ss_hit; + } else { + /* A free or expired slot, else the least recently used */ + ss_oldest = 0; + for (ss_slot = 0; ss_slot < SESSIONS; ss_slot++) { + if (!session_id[ss_slot][0] || ss_stale()) + break; + if (ss_older()) + ss_oldest = ss_slot; + } + if (ss_slot == SESSIONS) + ss_slot = ss_oldest; } - if (ss_slot == SESSIONS) - ss_slot = ss_oldest; last_session_use[ss_slot] = now; for (ss_i = 0; ss_i < SESSION_ID_LENGTH; ss_i++) { diff --git a/test/test_httpd_tx.c b/test/test_httpd_tx.c index 0397e3de..c978d669 100644 --- a/test/test_httpd_tx.c +++ b/test/test_httpd_tx.c @@ -144,7 +144,8 @@ void clear_command_history(void) { } void udp_callbacks(void) { } void tcpip_output(void) { } void get_random_32(void) { } -void read_reg_timer(uint32_t *tmr) { *tmr = 0; } +static uint32_t fake_now; +void read_reg_timer(uint32_t *tmr) { *tmr = fake_now; } /* uip-conf.h asks uIP not to define the packet buffer: on the switch it lives * at a fixed XDATA address, so the bench provides the storage itself. */ @@ -193,9 +194,9 @@ void send_lldp(void) {} extern uint8_t authenticated; extern char session_id[]; +extern uint32_t last_session_use[]; extern char passwd[21]; extern uint16_t session_timeout; -extern uint32_t last_session_use; /* uip.c defines the listen table but no header declares it; the bench reads it * to confirm the port httpd_init() asked for is the one uIP is watching. */ @@ -879,6 +880,77 @@ static void scenario_split_get_header_terminator(void) "fragmented GET terminator: CRLFCRLF may span receive events"); } +/* The sessions of the login tests below: four in use, slot 1 the least recently used */ +#define ID_STRIDE 13 /* SESSION_ID_LENGTH + 1 */ + +static const char *slot_id(int i) +{ + return session_id + i * ID_STRIDE; +} + +static void fill_sessions(void) +{ + static const char *const ids[4] = { "0123456789ab", "aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc" }; + static const uint32_t used[4] = { 50, 10, 60, 70 }; + + fake_now = 100; + for (int i = 0; i < 4; i++) { + strcpy(session_id + i * ID_STRIDE, ids[i]); + last_session_use[i] = used[i]; + } +} + +static int others_kept(int first, int second, int third) +{ + static const char *const ids[4] = { "0123456789ab", "aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc" }; + + return !strcmp(slot_id(first), ids[first]) && !strcmp(slot_id(second), ids[second]) && + !strcmp(slot_id(third), ids[third]); +} + +static void scenario_login_with_valid_cookie_reuses_its_slot(void) +{ + static const char post[] = + "POST /login HTTP/1.1\r\n" + "Cookie: session=0123456789ab\r\n" + "Content-Length: 8\r\n" + "Content-Type: application/x-www-form-urlencoded\r\n" + "\r\n" + "pwd=1234"; + + login_session_start(); + fill_sessions(); + client_send(TCP_ACK | TCP_PSH, post, (int)strlen(post)); + CHECK(stream_contains("Location: index.html\r\n"), + "login with a valid cookie: accepted"); + CHECK(strcmp(slot_id(0), "0123456789ab") != 0, + "login with a valid cookie: its own slot gets the new id"); + CHECK(others_kept(1, 2, 3), + "login with a valid cookie: nobody else is pushed out"); + fake_now = 0; +} + +static void scenario_login_without_cookie_replaces_least_recently_used(void) +{ + static const char post[] = + "POST /login HTTP/1.1\r\n" + "Content-Length: 8\r\n" + "Content-Type: application/x-www-form-urlencoded\r\n" + "\r\n" + "pwd=1234"; + + login_session_start(); + fill_sessions(); + client_send(TCP_ACK | TCP_PSH, post, (int)strlen(post)); + CHECK(stream_contains("Location: index.html\r\n"), + "login without a cookie: accepted"); + CHECK(strcmp(slot_id(1), "aaaaaaaaaaaa") != 0, + "login without a cookie: the least recently used slot is taken"); + CHECK(others_kept(0, 2, 3), + "login without a cookie: the other three stay"); + fake_now = 0; +} + static void scenario_fragmented_get_without_cookie(void) { static const char first[] = @@ -1043,7 +1115,7 @@ static void scenario_upload(const char *name, const char *part_type, int split) session_start(MSS_FULL); strcpy(session_id, "0123456789ab"); session_timeout = 200; - last_session_use = 0; /* read_reg_timer() returns zero in this bench. */ + last_session_use[0] = 0; /* read_reg_timer() returns zero in this bench. */ flash_size = 0x100000; flash_written_len = 0; flash_write_bad = 0; @@ -1116,6 +1188,9 @@ int main(int argc, char **argv) scenario_upload("MacBinary, split part terminator", "Content-Type: application/macbinary\r\n", 1); scenario_upload("no part Content-Type", "", 0); + scenario_login_with_valid_cookie_reuses_its_slot(); + scenario_login_without_cookie_replaces_least_recently_used(); + printf("\n%s (%d failure%s)\n", failures ? "BENCH: FAILURES" : "BENCH: ALL PASS", failures, failures == 1 ? "" : "s");