Skip to content

Commit 4eea31f

Browse files
Claudeclaude
andcommitted
fix(M17): expire admin_mode after idle timeout, refresh on activity
admin_mode was cleared only by explicit logout and USB disconnect, never by time. On a permanently attached host stdio_usb_connected() stays true, so one login persisted indefinitely and anyone later reaching the host or cable inherited admin. Record an absolute time_us_64() deadline = now + ADMIN_IDLE_TIMEOUT_US (5 min) whenever admin is granted (new commands_admin_grant(), called from every cmd_login grant path). The dispatcher expires a stale session before honoring any admin-gated command (clears admin_mode, refuses) and refreshes the deadline after each successful admin command so active use extends the window. USB disconnect now also clears the deadline. A zero deadline is treated as never-expiring so direct admin_mode toggles (host harness) are not spuriously logged out. harness_commands gains an injectable time_us_64() and an assertion that an admin command is allowed within the window (refreshing it) and refused + auto-cleared after it. Host asan/valgrind/coverage/ci-check all 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a94d9f7 commit 4eea31f

4 files changed

Lines changed: 105 additions & 9 deletions

File tree

serial/commands.c

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
#include "hardware/buzzer.h"
44
#include "storage/storage.h"
55
#include "pico/stdlib.h"
6+
#include "pico/time.h"
7+
#include <stdint.h>
68
#include <stdio.h>
79
#include <string.h>
810
#include <stdbool.h>
@@ -13,13 +15,37 @@
1315

1416
bool admin_mode = false;
1517

18+
// Absolute time_us_64() deadline at which an idle admin session auto-expires.
19+
// 0 means "no active session / no deadline armed" - the expiry check treats
20+
// that as never-expiring so code (or a host harness) that toggles admin_mode
21+
// directly without a login is not spuriously logged out.
22+
static uint64_t admin_deadline_us = 0;
23+
1624
bool commands_is_admin(void) {
1725
return admin_mode;
1826
}
1927

28+
void commands_admin_grant(void) {
29+
admin_mode = true;
30+
admin_deadline_us = time_us_64() + ADMIN_IDLE_TIMEOUT_US;
31+
}
32+
33+
// Expire a stale admin session: if admin mode is active, a deadline is armed,
34+
// and it has passed, clear admin mode (auto-logout) and report true so the
35+
// caller can refuse. Returns false if the session is still valid or inactive.
36+
static bool admin_session_expired(void) {
37+
if (admin_mode && admin_deadline_us != 0 && time_us_64() >= admin_deadline_us) {
38+
admin_mode = false;
39+
admin_deadline_us = 0;
40+
return true;
41+
}
42+
return false;
43+
}
44+
2045
void commands_on_disconnect(void) {
2146
if (admin_mode) {
22-
admin_mode = false;
47+
admin_mode = false;
48+
admin_deadline_us = 0;
2349
printf("[console] USB disconnected - admin session ended\r\n");
2450
}
2551
}
@@ -123,10 +149,19 @@ void commands_dispatch(int argc, char **argv) {
123149

124150
const command_t *cmd = &COMMANDS[i];
125151

126-
if (cmd->requires_admin && !admin_mode) {
127-
printf("error: '%s' requires admin mode - use login <otp>\r\n", cmd->name);
128-
buzzer_play_command_ack();
129-
return;
152+
if (cmd->requires_admin) {
153+
// Drop a session that has been idle too long before honoring it, so
154+
// a long-attached host cannot inherit an old login.
155+
if (admin_session_expired()) {
156+
printf("error: admin session expired - use login <otp>\r\n");
157+
buzzer_play_command_ack();
158+
return;
159+
}
160+
if (!admin_mode) {
161+
printf("error: '%s' requires admin mode - use login <otp>\r\n", cmd->name);
162+
buzzer_play_command_ack();
163+
return;
164+
}
130165
}
131166

132167
int user_args = argc - 1;
@@ -137,6 +172,10 @@ void commands_dispatch(int argc, char **argv) {
137172
}
138173

139174
HANDLERS[i](argc, argv);
175+
176+
// Successful admin activity refreshes the idle window.
177+
if (cmd->requires_admin && admin_mode)
178+
commands_admin_grant();
140179
return;
141180
}
142181

serial/commands.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,21 @@
33

44
#include <stdbool.h>
55

6+
// Admin sessions auto-expire after this many microseconds of inactivity. The
7+
// deadline is armed on login and refreshed by every successful admin command,
8+
// so an unattended-but-still-connected host cannot keep one login alive forever.
9+
#define ADMIN_IDLE_TIMEOUT_US (5ULL * 60 * 1000000ULL)
10+
611
// Dispatch a parsed command line
712
void commands_dispatch(int argc, char **argv);
813

914
// Returns true if currently in admin mode
1015
bool commands_is_admin(void);
1116

17+
// Enter (or renew) admin mode and (re)arm the idle-timeout deadline. Called by
18+
// cmd_login on every successful grant and by the dispatcher on admin activity.
19+
void commands_admin_grant(void);
20+
1221
// Called by console on USB disconnect - auto-logout
1322
void commands_on_disconnect(void);
1423

serial/commands_system.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include "commands.h"
12
#include "commands_handlers.h"
23
#include "commands.h"
34
#include "hardware/buzzer.h"
@@ -154,7 +155,7 @@ void cmd_login(int argc, char **argv) {
154155
wifi_config_t wifi;
155156
if (storage_is_mounted() && !storage_wifi_get(&wifi)) {
156157
printf("warning: wifi not configured - open mode\r\n");
157-
admin_mode = true;
158+
commands_admin_grant();
158159
printf("login: admin mode enabled\r\n");
159160
buzzer_play_command_ack();
160161
return;
@@ -168,7 +169,7 @@ void cmd_login(int argc, char **argv) {
168169
if (!clock_get_unix_time(&now_unix)) {
169170
if (time_us_64() >= BOOT_BYPASS_WINDOW_US) {
170171
printf("warning: RTC not set - open mode\r\n");
171-
admin_mode = true;
172+
commands_admin_grant();
172173
printf("login: admin mode enabled\r\n");
173174
buzzer_play_command_ack();
174175
} else {
@@ -183,7 +184,7 @@ void cmd_login(int argc, char **argv) {
183184
// No admin keys - allow any credentials (bootstrap mode)
184185
if (!any_admin) {
185186
printf("warning: no admin keys configured - bootstrap mode\r\n");
186-
admin_mode = true;
187+
commands_admin_grant();
187188
printf("login: admin mode enabled\r\n");
188189
buzzer_play_command_ack();
189190
return;
@@ -213,7 +214,7 @@ void cmd_login(int argc, char **argv) {
213214
return;
214215
}
215216

216-
admin_mode = true;
217+
commands_admin_grant();
217218
printf("login: admin mode enabled\r\n");
218219
secure_wipe(&key, sizeof(key));
219220
buzzer_play_command_ack();

test/harness_commands.c

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <assert.h>
2626
#include <fcntl.h>
2727
#include <stdbool.h>
28+
#include <stdint.h>
2829
#include <stdio.h>
2930
#include <string.h>
3031
#include <unistd.h>
@@ -33,6 +34,14 @@
3334
// test the admin gate without going through the real cmd_login/TOTP path.
3435
extern bool admin_mode;
3536

37+
// Injectable host clock. commands.c calls time_us_64() to arm and check the
38+
// admin idle-timeout deadline; driving it here lets the idle-timeout test step
39+
// time deterministically instead of sleeping.
40+
static uint64_t g_now_us = 0;
41+
uint64_t time_us_64(void) {
42+
return g_now_us;
43+
}
44+
3645
// ---------------------------------------------------------------------------
3746
// Spies
3847
// ---------------------------------------------------------------------------
@@ -261,6 +270,43 @@ static void test_on_disconnect_clears_admin(void) {
261270
assert(!commands_is_admin());
262271
}
263272

273+
static void test_admin_session_idle_timeout(void) {
274+
// A login arms a ~5-min idle window; admin commands inside it run and each
275+
// refreshes the deadline, but a command issued after the window has elapsed
276+
// is refused and the session is auto-cleared - so a permanently attached
277+
// host cannot inherit an old login.
278+
g_now_us = 1000;
279+
commands_admin_grant(); // login path: enters admin + arms the deadline
280+
assert(commands_is_admin());
281+
282+
// Just inside the window: an admin command runs (and refreshes the deadline).
283+
g_now_us = 1000 + ADMIN_IDLE_TIMEOUT_US - 1;
284+
run("list-keys", (const char *)NULL);
285+
assert(g_last_handler != NULL && strcmp(g_last_handler, "cmd_list_keys") == 0);
286+
assert(commands_is_admin());
287+
288+
// The refresh moved the deadline to now+timeout, so a second command another
289+
// (almost) full window later is STILL allowed - total elapsed time already
290+
// exceeds one window, proving activity extends the session.
291+
g_now_us += ADMIN_IDLE_TIMEOUT_US - 1;
292+
run("list-keys", (const char *)NULL);
293+
assert(g_last_handler != NULL && strcmp(g_last_handler, "cmd_list_keys") == 0);
294+
assert(commands_is_admin());
295+
296+
// Now go idle past the (refreshed) deadline: the next admin command is
297+
// refused and admin mode is auto-cleared.
298+
g_now_us += ADMIN_IDLE_TIMEOUT_US + 1;
299+
run("list-keys", (const char *)NULL);
300+
assert(g_last_handler == NULL); // handler did NOT run
301+
assert(!commands_is_admin()); // auto-logged out
302+
assert(out_has("expired"));
303+
304+
// Reset the injected clock to 0 so any deadline a later test's refresh arms
305+
// stays in the future (the other tests toggle admin_mode directly).
306+
g_now_us = 0;
307+
admin_mode = false;
308+
}
309+
264310
int main(void) {
265311
test_routes_public_command();
266312
test_help_and_alias_route_same_handler();
@@ -275,6 +321,7 @@ int main(void) {
275321
test_admin_command_with_args_routes_when_admin();
276322
test_admin_gate_precedes_argc_check();
277323
test_on_disconnect_clears_admin();
324+
test_admin_session_idle_timeout();
278325

279326
printf("harness_commands: all assertions passed\n");
280327
return 0;

0 commit comments

Comments
 (0)