Merge pull request #560 from brunoz78/multi-session

httpd: four login sessions instead of one

Makefile

@@ -105,6 +105,7 @@ SRCS = \
 	sfp.c \
 	dns.c \
 	dns_cli.c \
+	sessions.c \
 	ntp.c \
 	ntp_cli.c \
 	syslog.c \

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");

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,10 +75,9 @@ __xdata uint8_t postbody_endpoint;
 __xdata uint16_t postbody_start;
 
 __xdata char passwd[21];
-__xdata char session_id[SESSION_ID_LENGTH + 1];
+// Up to four logins at once, see sessions.c
 __xdata uint8_t authenticated;
 __xdata uint32_t now;
-__xdata uint32_t last_session_use;
 
 #define TSTATE_NONE		0
 #define TSTATE_TX		1
@@ -109,6 +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;
+	session_init();
 }
 
 
@@ -343,38 +343,11 @@ __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)) {
-				authenticated = 1;
-				last_session_use = now;
-			} else {
-				dbg_string("Invalid session cookie!\n");
-			}
-		}
-	}
+	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)
 {
@@ -675,13 +648,10 @@ 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);
 		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];
+		session_new();
 		slen += strtox(outbuf + slen, "; SameSite=Strict\r\n\r\n");
 	} else {
 		dbg_string("Password invalid!\n");

sessions.c

@@ -0,0 +1,128 @@
+/*
+ * 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 <stdint.h>
+#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;
+/* 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 */
+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;
+}
+
+/* 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)
+{
+	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;
+			ss_hit = ss_slot;
+			last_session_use[ss_slot] = now;
+			return;
+		}
+	}
+}
+
+void session_new(void) __banked
+{
+	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;
+	}
+
+	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;
+}

sessions.h

@@ -0,0 +1,19 @@
+#ifndef _SESSIONS_H_
+#define _SESSIONS_H_
+
+#include <stdint.h>
+
+#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

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

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");