diff --git a/hardware/PCB/.gitignore b/hardware/PCB/.gitignore index a0acfff..4a2c23a 100644 --- a/hardware/PCB/.gitignore +++ b/hardware/PCB/.gitignore @@ -5,3 +5,4 @@ *-backups/ *.bak __pycache__/ +.history/ diff --git a/src/admin_command_gate.h b/src/admin_command_gate.h index 26325d8..28cd90d 100644 --- a/src/admin_command_gate.h +++ b/src/admin_command_gate.h @@ -9,6 +9,8 @@ extern "C" { #endif +/* Blocking recursive task-context gate. Pair each successful take with a give + * on the same task; this does not replace the canonical command dispatcher. */ esp_err_t admin_command_gate_take(void); void admin_command_gate_give(void); diff --git a/src/admin_ssh_console.c b/src/admin_ssh_console.c index f0e663e..d1e8555 100644 --- a/src/admin_ssh_console.c +++ b/src/admin_ssh_console.c @@ -190,6 +190,8 @@ static bool session_is_current(const admin_ssh_console_token_t *token, if (owner == NULL) { return false; } + /* Database and transport-owner checks run outside the console spinlock; + * neither an open console slot nor an unchanged account alone is sufficient. */ bool account_current = false; bool current = principal->role == USER_ROLE_ADMIN && user_database_principal_is_current(principal, &account_current) == ESP_OK && @@ -773,6 +775,8 @@ static void worker_task(void *context) continue; } + /* Queue admission is not execution authority: revalidate the remote + * principal and reserve its exact slot before invoking a handler. */ bool current = session_is_current(&request.token, &request.principal); bool active; taskENTER_CRITICAL(&s_lock); @@ -877,6 +881,8 @@ static void control_task(void *context) if (xQueueReceive(s_control_queue, &request, portMAX_DELAY) != pdTRUE) { continue; } + /* Drain waiting stays off the dispatcher so it cannot stall other + * commands; empty application buffers do not prove peer receipt. */ TickType_t deadline = xTaskGetTickCount() + pdMS_TO_TICKS(10000U); bool drained = false; while ((int32_t)(xTaskGetTickCount() - deadline) < 0) { @@ -1127,6 +1133,8 @@ void admin_ssh_console_close(const admin_ssh_console_token_t *token) session->prompt_length = 0U; wake_prompt = true; } + /* Invalidate immediately, but retain an executing slot's identity until + * the worker returns and wipes it; admission must not reuse it meanwhile. */ session->active = false; if (!session->executing) { secure_wipe(session, sizeof(*session)); diff --git a/src/admin_ssh_console.h b/src/admin_ssh_console.h index c581373..0e1a832 100644 --- a/src/admin_ssh_console.h +++ b/src/admin_ssh_console.h @@ -129,6 +129,7 @@ esp_err_t admin_ssh_console_start_uart_frontend(void); /* Valid only while a registered command callback runs on the dispatcher task. */ bool admin_ssh_console_dispatch_is_remote(void); bool admin_ssh_console_dispatch_is_web(void); +/* Borrowed until this command returns; NULL outside remote dispatch. */ const user_principal_t *admin_ssh_console_dispatch_principal(void); /* Revalidate account, originating owner/session and token before side effects. * False outside the dispatcher; UART0 dispatch remains physically trusted. */ @@ -146,6 +147,8 @@ void admin_ssh_console_close(const admin_ssh_console_token_t *token); /* Called by the session owner. Returns false when input must be backpressured. */ bool admin_ssh_console_accepts_input(const admin_ssh_console_token_t *token); +/* With valid arguments, consumed is the accepted prefix even on a short feed. + * A true result does not imply all bytes were consumed; retain the remainder. */ bool admin_ssh_console_feed_input(const admin_ssh_console_token_t *token, const uint8_t *data, size_t length, size_t *consumed); diff --git a/src/console_completion.h b/src/console_completion.h index 72cc0a2..a6326d8 100644 --- a/src/console_completion.h +++ b/src/console_completion.h @@ -15,6 +15,7 @@ extern "C" { /* Install late-terminal upgrade handling and project-specific completion. */ void console_completion_install(void); +/* Called synchronously with a borrowed static string; false stops visitation. */ typedef bool (*console_completion_visitor_t)(const char *candidate, void *context); /* Visit the same matching hint candidates used by both UART0 and admin SSH. */ diff --git a/src/console_input.h b/src/console_input.h index 29200a5..b49c58c 100644 --- a/src/console_input.h +++ b/src/console_input.h @@ -8,6 +8,10 @@ #include "esp_err.h" +/* Blocking command-handler helpers: remote dispatch uses its session prompt; + * otherwise UART0 is read directly, flushing pending input before the prompt. + * Capacity includes the trailing NUL; output_length excludes it. Callers own + * the output and must wipe secrets after use, including hidden input. */ esp_err_t console_input_read_hidden(const char *prompt, uint8_t *output, size_t capacity, size_t minimum_length, size_t maximum_length, diff --git a/src/local_boot_animation.h b/src/local_boot_animation.h index 3756692..5481c96 100644 --- a/src/local_boot_animation.h +++ b/src/local_boot_animation.h @@ -5,5 +5,7 @@ #include "esp_err.h" -/* Plays the OLED-only startup identity animation; a missing display is nonfatal. */ +/* Synchronous animation with task delays; call after display start and before + * the status renderer. Returns display/frame errors, including an absent panel; + * the boot caller must treat these as nonfatal to other services. */ esp_err_t local_boot_animation_play(void); diff --git a/src/local_display.c b/src/local_display.c index a56f584..e7c2a39 100644 --- a/src/local_display.c +++ b/src/local_display.c @@ -204,6 +204,7 @@ static esp_err_t flush_dirty_locked(void) if (error != ESP_OK) { return error; } + /* Retire only fully transmitted pages; failures leave remaining work dirty. */ s_dirty_pages &= (uint8_t)~page_mask; } return ESP_OK; @@ -324,6 +325,7 @@ static esp_err_t initialize_locked(uint8_t address) s_inverted = false; memset(s_framebuffer, 0, sizeof(s_framebuffer)); s_dirty_pages = (uint8_t)((1U << LOCAL_DISPLAY_PAGE_COUNT) - 1U); + /* Clear panel RAM before enabling output so startup cannot expose stale pixels. */ error = flush_dirty_locked(); if (error == ESP_OK) { error = send_command_locked(0xafU); @@ -543,6 +545,7 @@ esp_err_t local_display_frame_begin(void) give_lock(); return ESP_ERR_INVALID_STATE; } + /* Keep the mutex across drawing and commit/cancel; only this task may finish. */ s_frame_active = true; s_frame_owner = xTaskGetCurrentTaskHandle(); return ESP_OK; @@ -555,6 +558,7 @@ esp_err_t local_display_frame_end(void) } esp_err_t error = flush_dirty_locked(); if (error != ESP_OK) { + /* A partial frame requires panel reinitialization before further drawing. */ s_initialized = false; } set_last_error(error); @@ -566,6 +570,7 @@ esp_err_t local_display_frame_end(void) void local_display_frame_cancel(void) { + /* Cancel releases ownership without I2C; framebuffer edits are not rolled back. */ if (s_frame_active && s_frame_owner == xTaskGetCurrentTaskHandle()) { s_frame_active = false; s_frame_owner = NULL; diff --git a/src/local_display.h b/src/local_display.h index 50e596d..79e2fe3 100644 --- a/src/local_display.h +++ b/src/local_display.h @@ -51,7 +51,10 @@ esp_err_t local_display_stop(void); esp_err_t local_display_get_snapshot(local_display_snapshot_t *snapshot); esp_err_t local_display_probe_expected(uint8_t *address_7bit); -/* Bounded scan of usable 7-bit addresses 0x08 through 0x77. */ +/* Synchronous scan after bus init; ESP_ERR_NOT_FOUND means no responders. + * Callback and responding_count may be NULL. Callbacks run in the caller's + * task after releasing the display mutex; context is borrowed only until return. + * Scans usable 7-bit addresses 0x08 through 0x77. */ esp_err_t local_display_scan(local_display_scan_callback_t callback, void *context, size_t *responding_count); @@ -63,13 +66,16 @@ esp_err_t local_display_set_inverted(bool inverted); * A frame holds only the display's own mutex and is owned by the task that * begins it. Callers must never retain a service/broker mutex while beginning * or ending a frame. Only the owning task may end or cancel it; end sends only - * modified 8-pixel pages and releases the display mutex on all outcomes. + * modified 8-pixel pages and releases the owning task's mutex even on IO error. + * A failed flush requires panel restart before another frame can begin. + * Cancel releases ownership without IO; framebuffer edits are not rolled back. */ esp_err_t local_display_frame_begin(void); esp_err_t local_display_frame_end(void); void local_display_frame_cancel(void); -/* Drawing coordinates are panel-local and are clipped to the selected panel. */ +/* Drawing requires a frame owned by the calling task; otherwise it is a no-op. + * Coordinates are panel-local and are clipped to the selected panel. */ void local_display_frame_clear(local_display_panel_t panel); void local_display_frame_clear_all(void); void local_display_frame_set_pixel(local_display_panel_t panel, diff --git a/src/local_status_ui.c b/src/local_status_ui.c index 327361a..01ef223 100644 --- a/src/local_status_ui.c +++ b/src/local_status_ui.c @@ -394,6 +394,8 @@ static void draw_content_item(local_status_icon_t icon, uint8_t y, const char *t static void collect_snapshot(local_status_snapshot_t *snapshot) { + /* Gather independent service observations before taking the display lock; + * this is a render copy, not an atomic snapshot across services. */ memset(snapshot, 0, sizeof(*snapshot)); snapshot->serial_running = serial_service_is_running(); @@ -863,6 +865,7 @@ static bool render_ui(const local_status_ui_state_t *state, if (local_display_frame_begin() != ESP_OK) { return false; } + /* A diagnostic hold may have arrived while frame_begin waited for I2C. */ if (diagnostic_hold_is_active(xTaskGetTickCount())) { local_display_frame_cancel(); return false; @@ -1340,6 +1343,7 @@ static void local_status_ui_task(void *context) rendered = render_ui(&state, &snapshot); last_render = now; } + /* Allow result feedback, but do not let an unavailable panel block reboot. */ if (state.restart_pending && (rendered || (now - state.mode_since) >= pdMS_TO_TICKS(2000U))) { vTaskDelay(pdMS_TO_TICKS(500U)); @@ -1410,6 +1414,8 @@ esp_err_t local_status_ui_update_settings(local_ui_settings_action_t action, portEXIT_CRITICAL(&s_timing_mux); if (error != ESP_OK) return error; + /* The busy reservation spans NVS work without holding the timing spinlock; + * reset defaults become visible only after persistence succeeds. */ bool stored = true; if (action == LOCAL_UI_SETTINGS_LOAD) error = local_ui_config_load(&candidate, &stored); if (action == LOCAL_UI_SETTINGS_DEFAULTS || action == LOCAL_UI_SETTINGS_RESET) diff --git a/src/local_status_ui.h b/src/local_status_ui.h index 464d1e0..8d22ff6 100644 --- a/src/local_status_ui.h +++ b/src/local_status_ui.h @@ -13,12 +13,14 @@ extern "C" { /* * Starts the low-priority status renderer and local recovery controls. The * task never becomes a broker client or serial writer. The OLED and buttons - * are optional, so a missing display is not an error. + * are optional, so a missing display is not an error. Copies config before + * returning; success reports task creation, not completion of hardware setup. */ esp_err_t local_status_ui_start(const local_ui_config_t *config); /* Runtime settings are copied atomically and never expose display-frame ownership. */ esp_err_t local_status_ui_get_config(local_ui_config_t *config); +/* RAM-only copy; rendering observes the change asynchronously, without NVS IO. */ esp_err_t local_status_ui_apply_config(const local_ui_config_t *config); typedef enum { @@ -33,6 +35,9 @@ esp_err_t local_status_ui_get_settings(local_ui_config_t *config, uint32_t *gene * Nonzero stale generations return ESP_ERR_INVALID_STATE; contention returns * ESP_ERR_TIMEOUT. Load retains the canonical default fallback. Reset commits * defaults before publishing RAM, so a storage failure needs no RAM rollback. + * config is required only for APPLY and is copied, never retained. Optional + * loaded_defaults is true only after a successful LOAD that fell back to defaults. + * DEFAULTS changes RAM only; SAVE persists current RAM; RESET does both. * No display IO occurs here; successful RAM changes signal renderer activity. */ esp_err_t local_status_ui_update_settings(local_ui_settings_action_t action, uint32_t expected_generation, const local_ui_config_t *config, bool *loaded_defaults); diff --git a/src/local_ui_config.h b/src/local_ui_config.h index c702d7f..54bac5a 100644 --- a/src/local_ui_config.h +++ b/src/local_ui_config.h @@ -18,13 +18,18 @@ typedef struct { uint32_t version; - /* Zero disables the corresponding inactivity transition. */ + /* Zero disables the corresponding inactivity transition. If both are + * enabled, off must be strictly later than dim; each is at most 86400 s. */ uint32_t dim_timeout_seconds; uint32_t off_timeout_seconds; } local_ui_config_t; void local_ui_config_defaults(local_ui_config_t *config); esp_err_t local_ui_config_validate(const local_ui_config_t *config); +/* Both outputs are required. Missing/incompatible data yields ESP_OK with + * defaults and used_stored_config=false; other storage errors propagate. + * Loading never repairs storage or updates the running UI. */ esp_err_t local_ui_config_load(local_ui_config_t *config, bool *used_stored_config); esp_err_t local_ui_config_save(const local_ui_config_t *config); +/* Commit defaults to NVS, rather than erase the key; does not change live RAM. */ esp_err_t local_ui_config_reset_storage(void); diff --git a/src/local_ui_console.h b/src/local_ui_console.h index d658710..cd59912 100644 --- a/src/local_ui_console.h +++ b/src/local_ui_console.h @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-3.0-only */ -/* UART0 administration commands for local UI aging settings. */ +/* Shared administration command registration for local UI aging settings. */ #pragma once diff --git a/src/main.c b/src/main.c index 91cb528..88977d3 100644 --- a/src/main.c +++ b/src/main.c @@ -240,6 +240,7 @@ void app_main(void) } wifi_config_secure_wipe(&wifi_config, sizeof(wifi_config)); + /* Gate each network listener independently; HTTPS failure must not suppress SSH. */ if (wifi_error == ESP_OK && web_security_error == ESP_OK && web_runtime_error == ESP_OK) { esp_err_t start_error = web_server_start(); @@ -314,6 +315,7 @@ void app_main(void) ESP_ERROR_CHECK(admin_ssh_console_register_commands()); /* Upgrade late UART terminals safely and add nested completion. */ console_completion_install(); + /* Admit commands only after the shared registry and completion are ready. */ ESP_ERROR_CHECK(admin_ssh_console_start_uart_frontend()); ESP_LOGI(TAG, "Shared UART0/SSH administration console ready at %d baud", diff --git a/src/mdns_config.h b/src/mdns_config.h index 0a165b6..9d81688 100644 --- a/src/mdns_config.h +++ b/src/mdns_config.h @@ -14,16 +14,23 @@ #define MDNS_CONFIG_NVS_NAMESPACE "mdns_cfg" #define MDNS_CONFIG_NVS_BLOB_KEY "config" +/* Fixed NVS blob layout; initialize with defaults before editing fields. */ typedef struct { uint32_t schema_version; uint16_t blob_size; uint8_t suffix_len; uint8_t reserved; + /* Nonempty lowercase a-z/0-9/hyphen suffix, with no edge hyphens. + * NUL-terminate and zero-pad; excludes the sak- prefix and .local domain. */ char suffix[MDNS_CONFIG_SUFFIX_MAX_LEN + 1U]; } mdns_config_t; void mdns_config_defaults(mdns_config_t *config); esp_err_t mdns_config_validate(const mdns_config_t *config); +/* Both outputs required. Missing/incompatible blobs select MAC-derived defaults + * with ESP_OK and used_stored_config=false, without rewriting storage. + * MAC/default-generation or other storage failures still propagate. */ esp_err_t mdns_config_load(mdns_config_t *config, bool *used_stored_config); esp_err_t mdns_config_save(const mdns_config_t *config); +/* Commit MAC-derived defaults; does not change the running mDNS configuration. */ esp_err_t mdns_config_reset_storage(void); diff --git a/src/mdns_service.c b/src/mdns_service.c index 6cfd736..8ac137c 100644 --- a/src/mdns_service.c +++ b/src/mdns_service.c @@ -55,6 +55,7 @@ static esp_err_t reconcile_record(const char *type, uint16_t port, esp_err_t error = available ? mdns_service_add(NULL, type, "_tcp", port, NULL, 0) : mdns_service_remove(type, "_tcp"); + /* Keep failed changes pending for the next owner reconciliation. */ if (error == ESP_OK) *registered = available; return error; } @@ -130,6 +131,8 @@ esp_err_t mdns_service_reconcile(void) bool https_available = s_https_available; bool ssh_available = s_ssh_available; portEXIT_CRITICAL(&s_availability_mux); + /* Component calls run outside the availability lock; attempt both records + * even after a family/record failure, retaining the first error. */ esp_err_t https_error = reconcile_record("_https", 443, https_available, &s_https_registered); if (error == ESP_OK) error = https_error; esp_err_t ssh_error = reconcile_record("_ssh", 22, ssh_available, &s_ssh_registered); @@ -308,6 +311,8 @@ void mdns_service_stop(void) if (s_mutex == NULL) { return; } + /* Retain the responder and records; offline reconciliation disables its + * address families instead of tearing down component state. */ lock_service(); s_announced = false; unlock_service(); @@ -329,6 +334,8 @@ esp_err_t mdns_service_reannounce(void) uint32_t generation = s_config_generation; unlock_service(); + /* Apply the copied generation, not a newer config staged during the call; + * a failed hostname update leaves this generation eligible for retry. */ esp_err_t error = ESP_OK; if (generation != s_applied_generation) { char hostname[MDNS_CONFIG_SUFFIX_MAX_LEN + 5U] = {0}; diff --git a/src/mdns_service.h b/src/mdns_service.h index aacda81..b44b5f1 100644 --- a/src/mdns_service.h +++ b/src/mdns_service.h @@ -17,8 +17,11 @@ typedef struct { esp_err_t last_error; } mdns_service_snapshot_t; +/* One-time RAM initialization: copies config but does not start announcements. */ esp_err_t mdns_service_init(const mdns_config_t *config); esp_err_t mdns_service_get_config(mdns_config_t *config); +/* Copy into RAM only; no persistence or immediate reannouncement. Queue + * wifi_manager_mdns_reannounce() separately to request the network update. */ esp_err_t mdns_service_set_config(const mdns_config_t *config); esp_err_t mdns_service_get_snapshot(mdns_service_snapshot_t *snapshot); diff --git a/src/rs232_port_owner.h b/src/rs232_port_owner.h index 8817953..46c6028 100644 --- a/src/rs232_port_owner.h +++ b/src/rs232_port_owner.h @@ -2,6 +2,8 @@ #include "esp_err.h" +/* Cooperative UART1/MAX3243 arbitration, not a task identity or driver lock. + * Owners must perform hardware setup/cleanup themselves while holding a claim. */ typedef enum { RS232_PORT_OWNER_NONE, RS232_PORT_OWNER_PHASE0, @@ -9,9 +11,17 @@ typedef enum { RS232_PORT_OWNER_FAULT, } rs232_port_owner_t; +/* Initialize once before concurrent use; subsequent operations are task-context. */ esp_err_t rs232_port_owner_init(void); +/* Claim PHASE0 or SERVICE only. An occupied port (even by the same owner) + * returns ESP_ERR_INVALID_STATE; this does not wait for ownership to become free. */ esp_err_t rs232_port_claim(rs232_port_owner_t owner); +/* Release only the matching owner after safe cleanup; no hardware IO is done. */ esp_err_t rs232_port_release(rs232_port_owner_t owner); +/* Replace previous_owner only if still current. Unsafe cleanup must leave FAULT + * in place until reboot; ordinary owners must not release it as if cleanup passed. */ void rs232_port_mark_fault(rs232_port_owner_t previous_owner); +/* Observation only, not a claim; returns FAULT before initialization. */ rs232_port_owner_t rs232_port_get_owner(void); +/* Returns a static string; caller must not free it. */ const char *rs232_port_owner_to_string(rs232_port_owner_t owner); diff --git a/src/secure_random.c b/src/secure_random.c index 3964a91..740015e 100644 --- a/src/secure_random.c +++ b/src/secure_random.c @@ -82,6 +82,9 @@ esp_err_t secure_random_fill(void *output, size_t length) unsigned char *cursor = (unsigned char *)output; esp_err_t error = ESP_OK; + /* Hold ownership across all chunks so DRBG state and the lifetime call + * budget advance together. Failure may leave a generated prefix in output; + * callers must not use the buffer unless the whole request succeeds. */ xSemaphoreTake(s_random_mutex, portMAX_DELAY); while (length > 0U) { /* CTR_DRBG limits each request even though the public API need not. */ @@ -120,6 +123,7 @@ int secure_random_mbedtls(void *context, unsigned char *output, size_t length) void secure_wipe(void *data, size_t size) { + /* Volatile stores keep secret erasure from becoming a dead-store removal. */ volatile uint8_t *byte = (volatile uint8_t *)data; if (byte == NULL) { diff --git a/src/secure_random.h b/src/secure_random.h index 8fa438d..9c47588 100644 --- a/src/secure_random.h +++ b/src/secure_random.h @@ -18,7 +18,9 @@ extern "C" { */ esp_err_t secure_random_init(void); -/* Fill output from the already-seeded, mutex-protected device DRBG. */ +/* Fill output from the already-seeded, mutex-protected device DRBG. + * Failure may leave a generated prefix; use output only on ESP_OK and wipe + * sensitive buffers after use. A zero-length request succeeds without init. */ esp_err_t secure_random_fill(void *output, size_t length); /* Mbed TLS-compatible adapter: zero means success, negative means failure. */ diff --git a/src/serial_config.h b/src/serial_config.h index 1e3a1a5..1888e6d 100644 --- a/src/serial_config.h +++ b/src/serial_config.h @@ -71,6 +71,8 @@ const char *serial_config_dtr_behavior_to_string(serial_config_dtr_behavior_t va /* Storage operations initialize the default NVS partition before use. */ esp_err_t serial_config_storage_init(void); +/* Missing or incompatible records return ESP_OK with defaults and false. + * Other storage errors propagate; valid output pointers still receive defaults. */ esp_err_t serial_config_load(serial_config_t *config, bool *used_stored_config); esp_err_t serial_config_save(const serial_config_t *config); esp_err_t serial_config_reset_storage(void); diff --git a/src/serial_service.c b/src/serial_service.c index c8d730a..f637ff7 100644 --- a/src/serial_service.c +++ b/src/serial_service.c @@ -284,6 +284,7 @@ static void serial_event_task(void *context) poll_modem_state(); } + /* Account for software TX abandoned on stop before acknowledging quiescence. */ size_t discarded = (pending_size - pending_offset) + xStreamBufferBytesAvailable(s_tx_stream); if (discarded > 0) { @@ -431,6 +432,7 @@ static esp_err_t stop_locked(bool restore_static_mode) return ESP_OK; } + /* Keep the driver and buffers intact unless the I/O task acknowledges stop. */ s_stop_requested = true; if (xSemaphoreTake(s_task_stopped, pdMS_TO_TICKS(SERIAL_STOP_TIMEOUT_MS)) != pdTRUE) { ESP_LOGE(TAG, "UART service task did not quiesce within %d ms", SERIAL_STOP_TIMEOUT_MS); @@ -561,6 +563,7 @@ esp_err_t serial_service_stop(void) xSemaphoreTake(s_state_mutex, portMAX_DELAY); esp_err_t result = stop_locked(true); + /* Diagnostics may reclaim the port only after driver removal and safe GPIO restoration. */ if (!s_running && !uart_is_driver_installed(RS232_UART_PORT) && rs232_port_get_owner() == RS232_PORT_OWNER_SERVICE) { @@ -594,6 +597,7 @@ esp_err_t serial_service_apply_config(const serial_config_t *config) serial_config_t previous = s_config; bool restart = s_running; + /* Retain port ownership across restart; service buffers are discarded, not broker sessions. */ esp_err_t result = ESP_OK; if (restart) { result = stop_locked(false); @@ -602,6 +606,7 @@ esp_err_t serial_service_apply_config(const serial_config_t *config) s_config = *config; if (restart) { result = start_locked(false); + /* Retry the old configuration only while cleanup has not faulted ownership. */ if (result != ESP_OK && rs232_port_get_owner() == RS232_PORT_OWNER_SERVICE) { esp_err_t original_error = result; diff --git a/src/serial_service.h b/src/serial_service.h index e00f630..fcc57e1 100644 --- a/src/serial_service.h +++ b/src/serial_service.h @@ -59,7 +59,9 @@ esp_err_t serial_service_get_snapshot(serial_service_snapshot_t *snapshot); /* * Access is intentionally nonblocking. The session broker is the sole * logical RX consumer and TX producer; calls are serialized internally to - * satisfy FreeRTOS stream-buffer concurrency rules. + * satisfy FreeRTOS stream-buffer concurrency rules. Counts may be short or + * zero (including lock contention or a stopped service). write counts bytes + * copied into the TX queue, not bytes delivered to the UART or peer. */ size_t serial_service_read(uint8_t *data, size_t size); size_t serial_service_write(const uint8_t *data, size_t size); diff --git a/src/session_broker.c b/src/session_broker.c index 823ae0d..2bfb8c9 100644 --- a/src/session_broker.c +++ b/src/session_broker.c @@ -81,6 +81,7 @@ static session_broker_slot_t *find_slot_locked(session_broker_client_id_t client return NULL; } + /* Slot bits locate storage; the full ID also rejects stale connection generations. */ session_broker_slot_t *slot = &s_slots[slot_index]; if (!slot->connected || slot->id != client_id) { return NULL; @@ -122,6 +123,7 @@ static void broadcast_event_locked(session_broker_event_type_t type, continue; } + /* Notifications are advisory: a full queue must not block ownership changes. */ if (xQueueSend(slot->events, &event, 0) == pdTRUE) { ++slot->counters.events_queued; ++s_counters.events_queued; @@ -483,6 +485,7 @@ static esp_err_t broker_force_writer(session_broker_client_id_t client_id, } xSemaphoreTake(s_mutex, portMAX_DELAY); + /* Compare the lease generation and target ID under the same lock as reassignment. */ if (expected_generation && (expected_generation == UINT32_MAX || expected_generation != s_writer_generation)) { xSemaphoreGive(s_mutex); @@ -641,6 +644,7 @@ esp_err_t session_broker_write(session_broker_client_id_t client_id, return ESP_ERR_INVALID_STATE; } + /* Keep authorization and enqueue atomic with respect to writer revocation. */ size_t queued = serial_service_write(data, size); size_t rejected = size - queued; slot->counters.tx_accepted_bytes += queued; diff --git a/src/session_broker.h b/src/session_broker.h index 1f98799..4514548 100644 --- a/src/session_broker.h +++ b/src/session_broker.h @@ -41,6 +41,8 @@ typedef enum { * Events contain no pointers, so transports can encode them without lifetime * concerns. sequence is broker-global and strictly increases per event. * client_id identifies the subject; writer_id is the writer after the event. + * Delivery is advisory and can drop on full queues; use snapshots to reconcile + * current ownership rather than treating event history as authoritative. */ typedef struct { uint64_t sequence; diff --git a/src/ssh_security.h b/src/ssh_security.h index aec31ab..25bf7f8 100644 --- a/src/ssh_security.h +++ b/src/ssh_security.h @@ -34,7 +34,9 @@ typedef struct { /* NVS and secure_random must be ready. Existing malformed material is not replaced. */ esp_err_t ssh_security_init(ssh_security_load_result_t *load_result); -/* Query with output NULL/capacity zero; the required length is always returned. */ +/* Query with output NULL/capacity zero. When material is ready, returns the + * required length even for an undersized buffer; no partial key is copied. + * Caller owns the private DER copy: never log it, and wipe it after use. */ esp_err_t ssh_security_copy_private_key(uint8_t *output, size_t capacity, size_t *output_length); esp_err_t ssh_security_get_metadata(ssh_security_metadata_t *metadata); diff --git a/src/ssh_transport.c b/src/ssh_transport.c index 633a137..f12cdde 100644 --- a/src/ssh_transport.c +++ b/src/ssh_transport.c @@ -479,6 +479,8 @@ static int authenticate_public_key(ssh_slot_t *slot, : WOLFSSH_USERAUTH_FAILURE; } + /* A stored-key match is not proof of possession; only the result callback + * may promote this candidate after signature verification and a currentness check. */ if (public_key->hasSignature != 0U) { slot->pending_principal = principal; slot->pending_principal_valid = true; @@ -598,6 +600,8 @@ static bool cleanup_slot(ssh_slot_t *slot) } close_socket(&slot->socket_fd); + /* Keep the slot unavailable until its broker client is retired, even when + * network teardown has already completed; failed disconnects are retried. */ if (slot->broker_client_id != SESSION_BROKER_NO_CLIENT) { esp_err_t error = session_broker_disconnect(slot->broker_client_id); if (error != ESP_OK && error != ESP_ERR_NOT_FOUND) { @@ -1029,6 +1033,8 @@ static esp_err_t connect_broker(ssh_slot_t *slot, size_t slot_index) return error; } add_counter(&s_counters.broker_connections, 1U); + /* Broker calls do not hold the account lock. Recheck across admission and + * writer acquisition, undoing the client if its principal became stale. */ if (!slot_principal_is_current(slot)) { disconnect_failed_admission(slot); return ESP_ERR_INVALID_STATE; @@ -1082,6 +1088,8 @@ static void process_handshake(ssh_slot_t *slot, size_t slot_index) return; } + /* Routes are exclusive: an administrative shell gets no serial broker + * client, and a serial user never enters the command dispatcher. */ esp_err_t error; if (slot->principal.role == USER_ROLE_USER) { error = connect_broker(slot, slot_index); diff --git a/src/ssh_transport.h b/src/ssh_transport.h index 58448ae..5d89d7f 100644 --- a/src/ssh_transport.h +++ b/src/ssh_transport.h @@ -141,7 +141,9 @@ esp_err_t ssh_transport_replace_host_key(bool reset); esp_err_t ssh_transport_get_snapshot(ssh_transport_snapshot_t *snapshot); esp_err_t ssh_transport_clear_counters(void); -/* Close one session, one account's sessions, or every transport session. */ +/* Request owner-task closure of one session, an account's sessions, or all + * sessions. ESP_OK is not a cleanup barrier; revocation also succeeds when + * no matching sessions exist. */ esp_err_t ssh_transport_disconnect(uint32_t session_id); esp_err_t ssh_transport_revoke_user(const uint8_t *username, size_t username_length); diff --git a/src/status_led.h b/src/status_led.h index c3ba028..6756063 100644 --- a/src/status_led.h +++ b/src/status_led.h @@ -2,6 +2,7 @@ #include "esp_err.h" +/* Diagnostic indication, not aggregate service health: blue/amber/green/red. */ typedef enum { STATUS_LED_IDLE, STATUS_LED_RUNNING, @@ -9,5 +10,8 @@ typedef enum { STATUS_LED_FAIL, } status_led_state_t; +/* Create the board's RMT-backed LED once at startup; no deinit API is provided. */ esp_err_t status_led_init(void); +/* Requires successful init; refreshes the LED synchronously. Callers serialize + * updates: this module provides no mutex or background state arbitration. */ esp_err_t status_led_set(status_led_state_t state); diff --git a/src/usb_cdc_transport.c b/src/usb_cdc_transport.c index 27a95c3..2dfa4d3 100644 --- a/src/usb_cdc_transport.c +++ b/src/usb_cdc_transport.c @@ -315,6 +315,7 @@ static void discard_host_input(usb_cdc_pending_buffer_t *pending) pending->size = 0U; pending->offset = 0U; + /* Drain rather than reset a stream with a live callback writer; bound refill work. */ uint8_t data[USB_CDC_IO_CHUNK_SIZE]; size_t drain_budget = USB_CDC_HOST_RX_STREAM_SIZE; while (drain_budget > 0U) { @@ -498,6 +499,7 @@ static void transport_task(void *context) bool service_attempted = false; bool connect_attempted = false; bool reconciled = false; + /* Only this task owns broker mediation and the unaccepted tail in each direction. */ usb_cdc_pending_buffer_t host_pending = {0}; usb_cdc_pending_buffer_t usb_pending = {0}; const TickType_t poll_ticks = milliseconds_to_ticks(USB_CDC_POLL_MS); @@ -683,6 +685,7 @@ static void reset_uninitialized_state(void) static esp_err_t cleanup_init_allocations(bool cdc_initialized, bool driver_installed) { + /* Retire callback producers before freeing the storage they can still access. */ if (cdc_initialized) { esp_err_t error = tinyusb_cdcacm_deinit(TINYUSB_CDC_ACM_0); if (error != ESP_OK) { @@ -835,6 +838,7 @@ static esp_err_t enqueue_control_request(usb_cdc_control_t control) return ESP_ERR_INVALID_STATE; } + /* Queue admission is not a lease grant; the owner task resolves current broker state. */ if (xQueueSend(s_control_queue, &control, 0U) != pdTRUE) { add_counter(&s_counters.control_drops, 1U); return ESP_ERR_TIMEOUT; diff --git a/src/usb_cdc_transport.h b/src/usb_cdc_transport.h index 1a610e7..7597427 100644 --- a/src/usb_cdc_transport.h +++ b/src/usb_cdc_transport.h @@ -55,6 +55,7 @@ typedef struct { bool rts; session_broker_client_id_t broker_client_id; bool writer; + /* Host-requested diagnostics only; does not configure UART1. */ usb_cdc_transport_line_coding_t line_coding; usb_cdc_transport_counters_t counters; } usb_cdc_transport_snapshot_t; @@ -67,7 +68,8 @@ esp_err_t usb_cdc_transport_init(void); esp_err_t usb_cdc_transport_get_snapshot(usb_cdc_transport_snapshot_t *snapshot); -/* Requests are asynchronous and execute in the transport task. */ +/* Requests are asynchronous and execute in the transport task. + * ESP_OK means queued, not granted/released; inspect a later snapshot. */ esp_err_t usb_cdc_transport_request_writer(void); esp_err_t usb_cdc_transport_release_writer(void); diff --git a/src/user_database.c b/src/user_database.c index dd96d52..4c2a290 100644 --- a/src/user_database.c +++ b/src/user_database.c @@ -480,6 +480,8 @@ static esp_err_t commit_candidate_locked(void) } nvs_close(handle); } + /* Publish only committed state; on failure the live database stays intact. + * The candidate contains verifiers and is wiped on either outcome. */ if (error == ESP_OK) { secure_wipe(&s_database, sizeof(s_database)); s_database = *s_candidate; @@ -776,11 +778,15 @@ esp_err_t user_database_authenticate_password( memcpy(salt, user->password_salt, sizeof(salt)); memcpy(expected_hash, user->password_hash, sizeof(expected_hash)); } else { + /* Invalid or unknown credentials still incur KDF work, without making + * a dummy-verifier match eligible to authenticate. */ memcpy(salt, s_dummy_salt, sizeof(salt)); memcpy(expected_hash, s_dummy_hash, sizeof(expected_hash)); } xSemaphoreGive(s_mutex); + /* Run the expensive KDF outside the database lock. A match authorizes only + * the same account identity, credential generation and role seen above. */ esp_err_t error = derive_password(kdf_password, kdf_password_length, salt, iterations, derived_hash); bool matched = error == ESP_OK && @@ -836,6 +842,8 @@ esp_err_t user_database_authorize_ssh_public_key( key->blob_length == key_blob_length && memcmp(key->type, key_type, key_type_length) == 0 && constant_time_equal(key->blob, key_blob, key_blob_length)) { + /* This authorizes the key only; the SSH layer must verify the + * signature before treating the principal as authenticated. */ fill_principal(user, USER_AUTH_METHOD_SSH_PUBLIC_KEY, principal); *authorized = true; break; diff --git a/src/user_database.h b/src/user_database.h index 0d21692..b061ebe 100644 --- a/src/user_database.h +++ b/src/user_database.h @@ -41,6 +41,8 @@ typedef enum { USER_DATABASE_LOAD_EMPTY, } user_database_load_result_t; +/* Secret-free identity copy, not a permanent authorization grant. Recheck + * principal currentness at protected boundaries after account changes. */ typedef struct { uint32_t user_id; uint32_t auth_generation; @@ -50,6 +52,8 @@ typedef struct { char username[USER_DATABASE_USERNAME_CAPACITY + 1U]; } user_principal_t; +/* Caller-owned plaintext secret; keep out of routine status/logs and wipe + * the entire structure after the intended credential handoff. */ typedef struct { size_t password_length; uint8_t password[USER_DATABASE_PASSWORD_CAPACITY + 1U]; @@ -122,15 +126,21 @@ esp_err_t user_database_set_password_current(const user_database_account_t *expe * owns/wipes successful output; failures clear it. Same generator as CLI. */ esp_err_t user_database_generate_password_value(user_database_generated_password_t *generated); +/* ESP_OK alone is not authentication: require authenticated == true before + * using principal. Credential rejection normally returns ESP_OK with false. */ esp_err_t user_database_authenticate_password( const uint8_t *username, size_t username_length, const uint8_t *password, size_t password_length, user_principal_t *principal, bool *authenticated); +/* Likewise require authorized == true. This checks key membership only; + * the SSH layer must verify proof of private-key possession separately. */ esp_err_t user_database_authorize_ssh_public_key( const uint8_t *username, size_t username_length, const uint8_t *key_type, size_t key_type_length, const uint8_t *key_blob, size_t key_blob_length, user_principal_t *principal, bool *authorized); +/* Account identity/generation/role check only, not transport-session liveness. + * ESP_OK reports a completed check; inspect current for the decision. */ esp_err_t user_database_principal_is_current(const user_principal_t *principal, bool *current); diff --git a/src/web_account_settings.h b/src/web_account_settings.h index 0863bbc..6d6af42 100644 --- a/src/web_account_settings.h +++ b/src/web_account_settings.h @@ -3,8 +3,14 @@ #include #include "esp_http_server.h" -/* One session-bound pending/result slot. Dispatcher execution only; completed - * results are replaceable, not durable history or an idempotent retry API. */ +/* HTTPD authenticates/admin-checks and copies input; only an ID is queued to + * the existing dispatcher. HTTP 202 means admission, not mutation success. + * One global pending/result slot; only the original login can read its result + * (logging in again as the same account does not recover it). Completed results + * are replaceable, not durable history or an idempotent retry API. + * Execution rechecks the login and 30-second admission deadline; admitted DB + * work may finish after session loss. Pending create/password input also has + * periodic expiry; executing input is wiped on return, not by that timer. */ esp_err_t web_account_settings_handler(httpd_req_t *request); void web_account_settings_execute(uint32_t id); /* POST /api/settings/accounts/keys: admin cookie + Origin/CSRF, JSON exactly diff --git a/src/web_broker_settings.h b/src/web_broker_settings.h index 3a7e856..39c23ff 100644 --- a/src/web_broker_settings.h +++ b/src/web_broker_settings.h @@ -3,7 +3,13 @@ #include #include "esp_http_server.h" -/* Optional admin-only snapshot and login-isolated typed assignment/results. */ +/* HTTPD handlers enforce admin admission; POST also requires Origin/CSRF/JSON. + * HTTP 202 admits a copied assignment, not a completed writer transfer. One + * global slot stays busy through execution; its replaceable result is visible + * only to the original login, not a later login by the same account. */ esp_err_t web_broker_settings_handler(httpd_req_t *request); esp_err_t web_broker_operation_handler(httpd_req_t *request); +/* Existing dispatcher only. Rechecks login and 30-second admission deadline; + * broker compares target ID and lease generation atomically. Expiry is checked + * on dequeue, not by a slot-release timer; admitted work is not revoked. */ void web_broker_settings_execute(uint32_t id); diff --git a/src/web_console.h b/src/web_console.h index f0a9874..04df064 100644 --- a/src/web_console.h +++ b/src/web_console.h @@ -5,4 +5,6 @@ #include "esp_err.h" +/* Register during console setup, before admitting commands. Registration does + * not start HTTPS or require it to be running; recovery must work while offline. */ esp_err_t web_console_register_commands(void); diff --git a/src/web_cookie_auth.c b/src/web_cookie_auth.c index 5c9dbb1..3e8e7ee 100644 --- a/src/web_cookie_auth.c +++ b/src/web_cookie_auth.c @@ -177,6 +177,7 @@ static esp_err_t require(httpd_req_t *r, bool mutation, bool upgrade, size_t bod web_session_view_t *view, bool *allowed) { char canonical[129] = {0}, token[65] = {0}, csrf[65] = {0}; + /* A denial response can return ESP_OK; only allowed grants admission. */ *allowed = false; memset(view, 0, sizeof(*view)); if (!web_httpd_headers_valid(r) || !cookies_valid(r) || (!upgrade && strchr(r->uri, '?')) || @@ -324,6 +325,8 @@ esp_err_t web_cookie_auth_handler(httpd_req_t *r) status = "403 Forbidden"; code = "csrf"; goto deny; } } + /* Consume a matching login challenge before body IO or password work; + * later failures require a new challenge rather than permitting replay. */ bool has_cookie = cookie(r, PRELOGIN_COOKIE, token); if (has_cookie && mbedtls_sha256((const uint8_t *)token, 64, digest, 0)) goto deny; taskENTER_CRITICAL(&s_lock); @@ -375,6 +378,7 @@ esp_err_t web_cookie_auth_handler(httpd_req_t *r) received += (size_t)count; } if (received != r->content_len || !web_auth_parse_login(body, received, &credentials)) goto deny; + /* Charge the shared attempt budget before entering password verification. */ now = esp_timer_get_time(); unsigned attempts; int64_t retry; @@ -415,6 +419,7 @@ esp_err_t web_cookie_auth_handler(httpd_req_t *r) deny: result = failure(r, status, code); cleanup: + /* Roll back only this response's new challenge, not a reused/replaced one. */ if (challenge_published && result != ESP_OK) { taskENTER_CRITICAL(&s_lock); if (epoch == s_epoch && selected >= 0 && diff --git a/src/web_cookie_auth.h b/src/web_cookie_auth.h index 788ee23..c98a235 100644 --- a/src/web_cookie_auth.h +++ b/src/web_cookie_auth.h @@ -12,7 +12,11 @@ typedef struct { } web_cookie_auth_snapshot_t; void web_cookie_auth_get_snapshot(web_cookie_auth_snapshot_t *snapshot); void web_cookie_auth_clear_counters(void); -/* Sends an error on denial, with allowed=false. View is caller-wiped. */ +/* HTTPD handler context, before body reads/responses; all pointers required. + * Admission requires ESP_OK AND allowed=true: a sent denial can return ESP_OK. + * mutation selects POST plus CSRF; otherwise GET. mutation/upgrade require Origin. + * This variant rejects bodies. Role checks remain with the caller; a successful + * view is not a lease for later actions. Wipe the view after use, even on denial. */ esp_err_t web_cookie_auth_require(httpd_req_t *request, bool mutation, bool upgrade, web_session_view_t *view, bool *allowed); diff --git a/src/web_display_settings.h b/src/web_display_settings.h index 2516dba..3260ec1 100644 --- a/src/web_display_settings.h +++ b/src/web_display_settings.h @@ -3,7 +3,13 @@ #include #include "esp_http_server.h" -/* Optional admin-only RAM snapshot and typed dispatcher admission/results. */ +/* HTTPD handlers enforce admin admission; POST also requires Origin/CSRF/JSON. + * HTTP 202 means queued, not applied or persisted. One global slot stays busy + * through execution; completed results are replaceable and readable only by + * the original login, not a later login by the same account. */ esp_err_t web_display_settings_handler(httpd_req_t *request); esp_err_t web_display_operation_handler(httpd_req_t *request); +/* Existing dispatcher only. Rechecks login and 30-second admission deadline; + * the UI owner compares the configuration generation before mutation. This is + * not a completion timeout: admitted RAM/NVS work can outlive the login. */ void web_display_settings_execute(uint32_t id); diff --git a/src/web_firmware_update.c b/src/web_firmware_update.c index b6d30b4..e918ad0 100644 --- a/src/web_firmware_update.c +++ b/src/web_firmware_update.c @@ -133,6 +133,8 @@ esp_err_t web_firmware_update_handler(httpd_req_t *request) REJECT("400 Bad Request", "invalid_request"); if (request->content_len < PREFIX_SIZE) REJECT("400 Bad Request", "invalid_firmware"); + /* Acquire reboot, service and identity exclusion before allocating upload + * resources or touching flash; cleanup releases only acquired ownership. */ if (web_firmware_update_reserve_reboot() != ESP_OK) REJECT("503 Service Unavailable", "busy"); gate_owned = true; @@ -193,6 +195,8 @@ esp_err_t web_firmware_update_handler(httpd_req_t *request) esp_partition_pos_t position = {.offset = target->address, .size = target->size}; if (esp_image_get_metadata(&position, metadata) != ESP_OK || metadata->image_len != received) REJECT("400 Bad Request", "invalid_firmware"); + /* Streaming may outlive login authority. Reauthorize after validation and + * before changing the boot target, even though flash was already written. */ bool current = false; if (web_session_store_check_principal(view.id, &view.principal, ¤t) != ESP_OK || !current) REJECT("401 Unauthorized", "authentication_required"); diff --git a/src/web_firmware_update.h b/src/web_firmware_update.h index 1dcdec6..3fe0a9b 100644 --- a/src/web_firmware_update.h +++ b/src/web_firmware_update.h @@ -4,6 +4,11 @@ #define WEB_FIRMWARE_UPDATE_URI "/api/firmware" +/* Synchronous HTTPD POST handler; enforces admin cookie/Origin/CSRF itself. + * Streams a raw application/octet-stream image into the inactive OTA slot and + * rechecks session authority before boot selection. ESP_OK can mean a sent error + * response, not a committed update. Selection survives a failed success response: + * never blindly retry; further uploads require reboot. A sent success schedules it. */ esp_err_t web_firmware_update_handler(httpd_req_t *request); /* Atomically exclude uploads until reset. Call immediately before an ordinary diff --git a/src/web_httpd_adapter.h b/src/web_httpd_adapter.h index d7aef9e..b8b4781 100644 --- a/src/web_httpd_adapter.h +++ b/src/web_httpd_adapter.h @@ -23,6 +23,8 @@ bool web_httpd_unread_body(httpd_req_t *request); /* After the final response/lookup: preserve only unread pipelined data on a * keepalive connection. Closing requests may discard pending data entirely. */ void web_httpd_wipe_request(httpd_req_t *request, bool closing); +/* Caller must complete authorization/admission first; this only validates the + * handshake, sends 101 and installs the frame callback on success. HTTPD-owner only. */ esp_err_t web_httpd_upgrade(httpd_req_t *request, esp_err_t (*handler)(httpd_req_t *)); diff --git a/src/web_lifecycle_settings.h b/src/web_lifecycle_settings.h index d6f84e9..babcb3a 100644 --- a/src/web_lifecycle_settings.h +++ b/src/web_lifecycle_settings.h @@ -3,9 +3,19 @@ #include #include "esp_http_server.h" +/* HTTPD handlers enforce admin admission; POST also requires Origin/CSRF/JSON. + * One global slot remains reserved through execution. HTTP 202 is not completion: + * only successful response send followed by HTTPD handoff can queue the ID; + * send success does not prove peer receipt. Handoff has a two-second deadline, + * and execution rechecks the original login and 30-second admission deadline. + * Results are replaceable and original-login-only, never recoverable by logging + * in again. Stop/restart may invalidate that login; lost responses or failed + * results do not prove absence of side effects. Do not automatically retry. */ esp_err_t web_lifecycle_settings_handler(httpd_req_t *request); esp_err_t web_lifecycle_operation_handler(httpd_req_t *request); /* Existing dispatcher only; callbacks submit IDs, never execute lifecycle work. */ void web_lifecycle_settings_execute(uint32_t id); -/* Only after successful HTTPD destruction, before another server can start. */ +/* Only after successful HTTPD destruction, before another server can start. + * server is an identity for comparison, not dereferenced here. Retires its ACK + * handoff and cancels pending work; executing work retains its slot. */ void web_lifecycle_settings_stopped(httpd_handle_t server); diff --git a/src/web_login_ui.h b/src/web_login_ui.h index 062c2ce..59af786 100644 --- a/src/web_login_ui.h +++ b/src/web_login_ui.h @@ -5,5 +5,7 @@ #include "esp_http_server.h" /* Standalone public login document. Rendering only: authentication, route - * registration and bounded challenge allocation belong to web_cookie_auth. */ + * registration and bounded challenge allocation belong to web_cookie_auth. + * Call with a live HTTPD request; sends synchronously without retaining it. + * Success reports response submission, not a successful login. */ esp_err_t web_login_ui_send_response(httpd_req_t *request); diff --git a/src/web_network_settings.h b/src/web_network_settings.h index 507cfc9..4e7be93 100644 --- a/src/web_network_settings.h +++ b/src/web_network_settings.h @@ -15,6 +15,8 @@ * ID enters the existing dispatcher. A periodic one-second ESP timer wipes and * cancels non-executing input at 30 seconds plus scheduling latency. Executing * locals wipe on return; already-admitted work can finish after session loss. + * HTTP 202 means dispatcher admission, not the final operation result. Polling + * is restricted to the original login; logging in again cannot recover it. * 'accepted' means RAM/owner queue admission, NEVER association or DHCP success. * * SSID JSON is a BYTE string: raw printable ASCII, standard single-character @@ -31,4 +33,5 @@ */ esp_err_t web_network_snapshot_handler(httpd_req_t *request); esp_err_t web_network_operation_handler(httpd_req_t *request); +/* Existing admin dispatcher only; no request pointer survives HTTPD admission. */ void web_network_settings_execute(uint32_t id); diff --git a/src/web_security.h b/src/web_security.h index 688451f..33e6748 100644 --- a/src/web_security.h +++ b/src/web_security.h @@ -59,6 +59,9 @@ esp_err_t web_security_init(web_security_load_result_t *load_result); /* * Query with both outputs NULL/capacities zero. Certificate and key are copied * under one lock so a concurrent rotation can never produce a mismatched pair. + * Both length pointers are required; ready material reports required lengths even + * on INVALID_SIZE, without copying either buffer. Caller owns the copies and + * must wipe private-key storage after its consumer has finished with it. */ esp_err_t web_security_copy_tls_material( uint8_t *certificate, size_t certificate_capacity, diff --git a/src/web_serial_settings.h b/src/web_serial_settings.h index 8ac12c6..0a0d7de 100644 --- a/src/web_serial_settings.h +++ b/src/web_serial_settings.h @@ -7,6 +7,8 @@ * One global slot rejects mutations while pending (including execution). GET * exposes only the caller's session result; a later admitted operation replaces * that result, so this is not a durable history or an idempotent retry API. + * A new login by the same account cannot retrieve it. HTTP 202 means queued, + * not applied/persisted; response loss does not cancel an admitted operation. * The 30-second deadline is checked when dequeued, not a completion deadline or * a timer that frees the slot. Revocation/expiry cancels before operation * admission; admitted serial/NVS work may finish after the session is gone. */ diff --git a/src/web_serial_transport.c b/src/web_serial_transport.c index 0c1dfa8..b17d3cc 100644 --- a/src/web_serial_transport.c +++ b/src/web_serial_transport.c @@ -709,6 +709,8 @@ static esp_err_t connect_websocket(httpd_req_t *request, int socket_fd, goto cleanup; } + /* Broker creation ran outside the slot lock; recheck authority before + * requesting a writer lease and retire the unpublished client on failure. */ principal_current = false; result = identity_is_current(&principal, web_session_id, &principal_current); if (result != ESP_OK || !principal_current) { @@ -1011,6 +1013,8 @@ static void web_serial_send_work(void *argument) bool retired = false; taskENTER_CRITICAL(&s_lock); + /* Retire canceled work only when it still owns this slot incarnation; + * an old callback must not clear another session's pending frame. */ bool owned_work = slot->work_pending && work == &slot->work && work->generation == slot->generation && work->server == slot->server && @@ -1047,6 +1051,8 @@ static void web_serial_send_work(void *argument) esp_err_t result = ESP_FAIL; int64_t send_start = 0, send_end = 0; bool send_called = false; + /* In HTTPD context, verify socket ownership as well as the captured slot + * generation before sending: a file descriptor alone is reusable. */ void *current_context = httpd_sess_get_ctx(server, socket_fd); if (current_context == slot && httpd_ws_get_fd_info(server, socket_fd) == @@ -1149,6 +1155,8 @@ static esp_err_t queue_slot_frame(web_serial_slot_t *slot, uint32_t generation, esp_err_t result = httpd_queue_work(server, web_serial_send_work, &slot->work); #endif + /* Failed submission has no callback to release the frame; close only + * this session rather than retrying data already drained from the broker. */ if (result != ESP_OK) { taskENTER_CRITICAL(&s_lock); if (slot->work_pending && slot->generation == generation) { diff --git a/src/web_serial_transport.h b/src/web_serial_transport.h index 96a1c11..64c909a 100644 --- a/src/web_serial_transport.h +++ b/src/web_serial_transport.h @@ -132,6 +132,8 @@ esp_err_t web_serial_transport_session_ws_handler(httpd_req_t *request, * cleanup. Safe to repeat after either store or transport slot reuse. */ esp_err_t web_serial_transport_revoke_web_session(web_session_id_t id); +/* Copied transport state only: principal_valid marks an active slot, not a fresh + * database/session check; writer is cached, not an authoritative broker lease. */ esp_err_t web_serial_transport_get_snapshot( web_serial_transport_snapshot_t *snapshot); diff --git a/src/web_server.c b/src/web_server.c index c9d25c4..83daee0 100644 --- a/src/web_server.c +++ b/src/web_server.c @@ -663,6 +663,7 @@ static esp_err_t start_server(bool reserved) xSemaphoreGive(s_server_mutex); return ESP_ERR_INVALID_STATE; } + /* Reserve lifecycle ownership before releasing the mutex for slow startup. */ s_transitioning = true; if (s_generation != UINT32_MAX) ++s_generation; serial_transport_ready = s_serial_transport_initialized; @@ -833,6 +834,7 @@ static esp_err_t stop_server(uint32_t expected_generation, bool restart, bool re mdns_service_set_https_available(false); xSemaphoreGive(s_server_mutex); + /* Revoke admission first; teardown failures must not leave usable logins. */ web_cookie_auth_stop(); esp_err_t idle_error = web_httpd_idle_detach(server); if (idle_error != ESP_OK) { @@ -924,6 +926,7 @@ esp_err_t web_server_replace_identity(uint32_t expected_service_generation, error = web_security_replace_reserved(token); } if (error == ESP_OK) { + /* Report identity commit independently of the subsequent restart result. */ *committed = true; if (running) error = stop_server(0, true, true); else if (reset) error = start_server(true); diff --git a/src/web_session_store.c b/src/web_session_store.c index a1c7318..197c08e 100644 --- a/src/web_session_store.c +++ b/src/web_session_store.c @@ -134,6 +134,8 @@ esp_err_t web_session_store_init(void) uint64_t epoch = s_state.epoch; taskEXIT_CRITICAL(&s_lock); + /* Probe RNG outside the lock; the epoch check prevents a concurrent stop + * from being undone when initialization returns. */ uint8_t probe[WEB_SESSION_STORE_SECRET_BYTES] = {0}; esp_err_t error = secure_random_fill(probe, sizeof(probe)); secure_wipe(probe, sizeof(probe)); @@ -296,6 +298,8 @@ esp_err_t web_session_store_issue( duplicate = true; } } + /* Publish only into free capacity under the captured revocation epoch; + * never evict a live login or recycle an exhausted session identity. */ if (!s_state.ready || epoch != s_state.epoch || s_state.next_id == UINT64_MAX || now < 0 || now > INT64_MAX - WEB_SESSION_STORE_LIFETIME_US) { @@ -358,6 +362,8 @@ esp_err_t web_session_store_lookup( } } taskEXIT_CRITICAL(&s_lock); + /* Digest matching identifies a candidate, not admission: resolve must + * still enforce expiry, store readiness and current account authority. */ error = resolve(id, view, NULL); } secure_wipe(token_hash, sizeof(token_hash)); diff --git a/src/web_session_store.h b/src/web_session_store.h index 3753cb2..45c52be 100644 --- a/src/web_session_store.h +++ b/src/web_session_store.h @@ -52,13 +52,18 @@ void web_session_store_stop(void); esp_err_t web_session_store_issue( const user_principal_t *principal, const char *origin, size_t origin_length, char token[WEB_SESSION_STORE_TOKEN_LENGTH + 1U], web_session_view_t *view); +/* Exact lowercase-hex token span and the same canonical origin used at issue. + * Does not refresh the absolute expiry. Clears view on failure; caller wipes it + * after success. NOT_FOUND covers absent, expired or stale/mismatched sessions. */ esp_err_t web_session_store_lookup( const char *token, size_t token_length, const char *origin, size_t origin_length, web_session_view_t *view); /* Trusted transport identity check, not a replacement for HTTP cookie/origin * authorization. Every successful lookup/check revalidates the principal. - * No API result is a lease: recheck at later sensitive boundaries. */ + * No API result is a lease: recheck at later sensitive boundaries. + * current is required and set false on failure; ESP_OK means true. NOT_FOUND + * means no current match; store/database errors also deny authority. */ esp_err_t web_session_store_is_current(web_session_id_t id, bool *current); /* Also verifies that the transport's copied principal belongs to this ID. */ esp_err_t web_session_store_check_principal(web_session_id_t id, diff --git a/src/web_ssh_settings.h b/src/web_ssh_settings.h index e9316fb..4d019e3 100644 --- a/src/web_ssh_settings.h +++ b/src/web_ssh_settings.h @@ -3,7 +3,13 @@ #include #include "esp_http_server.h" -/* Optional admin-only SSH status and login-isolated ordinary controls. */ +/* HTTPD handlers enforce admin admission; POST also requires Origin/CSRF/JSON. + * HTTP 202 means queued, not completed. One global slot stays busy through + * execution; its replaceable result belongs to the original login, not the + * account across logins. A lost response is not grounds for automatic retry. */ esp_err_t web_ssh_settings_handler(httpd_req_t *request); esp_err_t web_ssh_operation_handler(httpd_req_t *request); +/* Existing dispatcher only. Rechecks login and 30-second admission deadline + * before generation-checked owner calls; admitted work may outlive the login. + * Rotation failure does not imply that the new identity was not committed. */ void web_ssh_settings_execute(uint32_t id); diff --git a/src/web_ui.h b/src/web_ui.h index 62f17eb..496e0b8 100644 --- a/src/web_ui.h +++ b/src/web_ui.h @@ -22,6 +22,9 @@ typedef enum { /* * Send one UI resource after the caller has authenticated the request. * This module deliberately performs no authentication or URI dispatch. + * Call within the HTTPD handler: the request is borrowed only for this + * synchronous send; resource storage remains module-owned. Send success does + * not establish a browser session or confirm that the browser loaded the UI. */ esp_err_t web_ui_send_response(httpd_req_t *request, web_ui_resource_t resource); diff --git a/src/wifi_config.h b/src/wifi_config.h index 0a54d7a..6f70eee 100644 --- a/src/wifi_config.h +++ b/src/wifi_config.h @@ -5,6 +5,8 @@ * The public structures below are also the version-1 NVS wire format. Keep * every field fixed-width and introduce a new schema version for layout * changes; do not silently reinterpret an existing blob. + * Config copies contain plaintext PSKs: never use them for routine status/logs, + * and wipe transient copies with wifi_config_secure_wipe() when finished. */ #pragma once @@ -77,6 +79,7 @@ typedef struct { uint8_t ap_channel; uint8_t reserved[5]; + /* As for station profiles, lengths are authoritative; no NUL is required. */ uint8_t ap_ssid[WIFI_CONFIG_SSID_MAX_LEN]; uint8_t ap_psk[WIFI_CONFIG_PSK_MAX_LEN]; uint8_t reserved_tail[1]; diff --git a/src/wifi_console.h b/src/wifi_console.h index cef6730..5eb3d6a 100644 --- a/src/wifi_console.h +++ b/src/wifi_console.h @@ -4,5 +4,6 @@ #include "esp_err.h" -/* Register Wi-Fi configuration, lifecycle, and diagnostic commands on UART0. */ +/* Register Wi-Fi configuration, lifecycle, and diagnostics in the shared + * administration registry; frontend policy controls remote command access. */ esp_err_t wifi_console_register_commands(void); diff --git a/src/wifi_manager.c b/src/wifi_manager.c index abcaaf6..0d8b9b8 100644 --- a/src/wifi_manager.c +++ b/src/wifi_manager.c @@ -547,6 +547,7 @@ static void schedule_cycle_retry(manager_runtime_t *runtime) } } + /* Back off between whole profile cycles, after attempting fallback AP recovery. */ uint32_t delay_seconds = runtime->next_backoff_seconds; if (delay_seconds < WIFI_MANAGER_INITIAL_BACKOFF_SECONDS) { delay_seconds = WIFI_MANAGER_INITIAL_BACKOFF_SECONDS; @@ -582,6 +583,8 @@ static void start_next_profile(manager_runtime_t *runtime) wifi_app_config_t config; copy_working_config(&config); + /* Immediate setup failures advance here; an accepted connect hands progress + * to events and the attempt deadline before another profile is configured. */ while (runtime->next_profile < runtime->profile_count) { uint8_t slot = runtime->profile_order[runtime->next_profile++]; const wifi_config_sta_profile_t *profile = &config.profiles[slot]; @@ -644,6 +647,8 @@ static void start_next_profile_after_current(manager_runtime_t *runtime) break; } } + /* Rotate the cycle rather than omit the current profile: it becomes the + * final candidate if every alternative fails. */ for (uint8_t index = 0U; index < count; ++index) { uint8_t source = found_active ? (uint8_t)((active_index + 1U + index) % count) : index; @@ -1301,6 +1306,8 @@ static void handle_expired_deadlines(manager_runtime_t *runtime) static void manager_task(void *context) { (void)context; + /* Event callbacks and command callers feed the queue; policy deadlines and + * transition bookkeeping remain private to this task. */ manager_runtime_t runtime; memset(&runtime, 0, sizeof(runtime)); runtime.next_backoff_seconds = WIFI_MANAGER_INITIAL_BACKOFF_SECONDS; diff --git a/src/wifi_manager.h b/src/wifi_manager.h index fe9bc92..050ddf5 100644 --- a/src/wifi_manager.h +++ b/src/wifi_manager.h @@ -94,13 +94,15 @@ typedef struct { */ esp_err_t wifi_manager_init(const wifi_app_config_t *config); -/* Returns a copy of the RAM working configuration, including credentials. */ +/* Returns a caller-owned copy of the RAM configuration, including plaintext + * credentials. Do not log it; wipe the copy on every exit path after use. */ esp_err_t wifi_manager_get_working_config(wifi_app_config_t *config); /* * Replaces the RAM working configuration. Disabled-profile-only edits do not * interrupt a running radio; changes to effective station/AP policy are - * applied asynchronously by restarting with the newest generation. + * applied asynchronously by restarting with the newest generation. Input is + * copied, not retained; success is not radio readiness and does not persist NVS. */ esp_err_t wifi_manager_apply_working_config(const wifi_app_config_t *config); @@ -150,7 +152,10 @@ esp_err_t wifi_manager_save_current(uint32_t generation); /* Stored-only load: never generates or installs unknown default credentials. */ esp_err_t wifi_manager_load_current(uint32_t generation); -/* Lifecycle requests are asynchronous and serialized by the manager task. */ +/* Lifecycle requests are asynchronous and serialized by the manager task. + * ESP_OK means queue admission, not completion; observe runtime via snapshots. + * Queue-full returns ESP_ERR_TIMEOUT. Accepted start/stop also set the RAM + * enabled_at_boot flag to 1/0 respectively; persistence still requires save. */ esp_err_t wifi_manager_start(void); esp_err_t wifi_manager_stop(void); esp_err_t wifi_manager_reconnect(void);