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