From 45542dcbffce118ce66fe1797823c2fec829aecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Martin?= Date: Sat, 3 Oct 2026 21:48:39 +0200 Subject: [PATCH] OTA: the firmware rolls itself back; the bootloader doesn't A deliberately crashing update looped forever on the device: the prebuilt bootloader ignores ESP_OTA_IMG_PENDING_VERIFY despite the app-side rollback config. UpdateService::bootGuard() now runs first in setup(): it counts starts on Probation in NVS and, on the second unconfirmed start, marks the image invalid and reboots into the previous one. Confirming (or the Wi-Fi rollback) resets the counter. ADR 0003 records the limit: a crash in the first milliseconds still needs USB. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EhqxQ49eCju4CzKYNjZzwT --- .../0003-own-signature-check-not-secure-boot.md | 1 + lib/ota/src/probation.h | 5 +++++ src/main.cpp | 4 +++- src/services/update_service.cpp | 15 +++++++++++++++ src/services/update_service.h | 3 +++ test/test_ota/test_ota.cpp | 7 +++++++ 6 files changed, 34 insertions(+), 1 deletion(-) diff --git a/docs/adr/0003-own-signature-check-not-secure-boot.md b/docs/adr/0003-own-signature-check-not-secure-boot.md index c6ffc07..2fb1644 100644 --- a/docs/adr/0003-own-signature-check-not-secure-boot.md +++ b/docs/adr/0003-own-signature-check-not-secure-boot.md @@ -9,3 +9,4 @@ We chose this over the ESP32's hardware Secure Boot. Secure Boot is enforced by - Someone with physical USB access can still flash anything. Only Wi-Fi and SD card updates are guarded. - **Losing the private key** means the next update has to go over USB, carrying a new public key. - P-256 rather than Ed25519, because the firmware's TLS library (mbedTLS) already verifies it, so it costs no extra code. +- **Rollback is done by the firmware, not the bootloader.** On the device, the prebuilt bootloader that PlatformIO flashes did not roll back a crashing update, even though the app's configuration enables it. So the firmware counts its own boots on Probation, very first thing in `setup()`, and reverts itself on the second unconfirmed start. A crash before that counter is written (the first few milliseconds) would not be caught: recovery is then over USB. diff --git a/lib/ota/src/probation.h b/lib/ota/src/probation.h index 7569f09..d507cfe 100644 --- a/lib/ota/src/probation.h +++ b/lib/ota/src/probation.h @@ -12,6 +12,11 @@ class Probation { static constexpr uint32_t kHealthyAfterMs = 30000; static constexpr uint32_t kWifiDeadlineMs = 180000; + // Checked first thing at boot, before anything that could crash. `attemptsBefore` counts earlier + // boots of this image on Probation; a second start means the first one died before confirming. + // (The prebuilt bootloader doesn't roll back by itself, so the firmware does.) + static bool rollBackAtBoot(bool onProbation, int attemptsBefore) { return onProbation && attemptsBefore >= 1; } + static Verdict judge(uint32_t uptimeMs, bool firstFrameDrawn, bool wifiConfigured, bool wifiConnected) { if (wifiConfigured && !wifiConnected && uptimeMs >= kWifiDeadlineMs) return Verdict::RollBack; if (uptimeMs < kHealthyAfterMs || !firstFrameDrawn) return Verdict::Wait; diff --git a/src/main.cpp b/src/main.cpp index 6f7331c..7ee1b70 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -91,12 +91,14 @@ static StatusInfo currentStatus() { } void setup() { + nvs.begin(); + UpdateService::bootGuard(nvs); // first: before anything that could crash on new firmware + auto cfg = M5.config(); M5Cardputer.begin(cfg, true); M5Cardputer.Display.setRotation(1); Serial.begin(115200); - nvs.begin(); settings.load(); battery = new BatteryService(bus); diff --git a/src/services/update_service.cpp b/src/services/update_service.cpp index 44c498a..e8bcf31 100644 --- a/src/services/update_service.cpp +++ b/src/services/update_service.cpp @@ -91,6 +91,19 @@ void UpdateService::start() { if (!task_) xTaskCreate(taskEntry, "update", 8192, this, 1, &task_); } +void UpdateService::bootGuard(KeyValueStore& store) { + esp_ota_img_states_t state; + bool probation = esp_ota_get_state_partition(esp_ota_get_running_partition(), &state) == ESP_OK && + state == ESP_OTA_IMG_PENDING_VERIFY; + int32_t attempts = 0; + store.getInt("ota_attempts", attempts); + if (Probation::rollBackAtBoot(probation, attempts)) { + store.putInt("ota_attempts", 0); + esp_ota_mark_app_invalid_rollback_and_reboot(); // does not return when there's a previous image + } + store.putInt("ota_attempts", probation ? attempts + 1 : 0); +} + void UpdateService::tick(uint32_t nowMs) { if (!probation_) return; bool wifiConfigured = settings_.getBool(Setting::WifiEnabled) && saved_.count() > 0; @@ -101,10 +114,12 @@ void UpdateService::tick(uint32_t nowMs) { esp_ota_mark_app_valid_cancel_rollback(); probation_ = false; store_.putString("ota_pending", ""); + store_.putInt("ota_attempts", 0); notify(std::string("Updated to ") + versionString(), NotificationLevel::Info); break; case Probation::Verdict::RollBack: // ota_pending still names this version: the previous firmware will report the failure. + store_.putInt("ota_attempts", 0); delay(200); esp_ota_mark_app_invalid_rollback_and_reboot(); break; diff --git a/src/services/update_service.h b/src/services/update_service.h index adbc69d..eab80fa 100644 --- a/src/services/update_service.h +++ b/src/services/update_service.h @@ -20,6 +20,9 @@ class UpdateService : public Service { enum class Phase { Idle, Receiving, Installed, Failed }; static constexpr uint16_t kPort = 3232; + // Call first in setup(): rolls back new firmware that already died once on Probation. + static void bootGuard(KeyValueStore& store); + UpdateService(KeyValueStore& store, WifiService& wifi, SavedNetworks& saved, StorageService& storage, EventBus& bus, const Settings& settings); const char* name() const override { return "update"; } diff --git a/test/test_ota/test_ota.cpp b/test/test_ota/test_ota.cpp index c8af9a8..18d4397 100644 --- a/test/test_ota/test_ota.cpp +++ b/test/test_ota/test_ota.cpp @@ -278,6 +278,12 @@ void test_probation_rolls_back_when_wifi_never_comes() { TEST_ASSERT_EQUAL(kConfirm, judge(500000, true, false, false)); // no Wi-Fi configured: fine } +void test_rollback_at_boot_after_an_unconfirmed_start() { + TEST_ASSERT_FALSE(Probation::rollBackAtBoot(true, 0)); // first start of new firmware + TEST_ASSERT_TRUE(Probation::rollBackAtBoot(true, 1)); // it died before confirming + TEST_ASSERT_FALSE(Probation::rollBackAtBoot(false, 3)); // confirmed firmware: never +} + int main() { UNITY_BEGIN(); RUN_TEST(test_sha256_known_vectors); @@ -298,5 +304,6 @@ int main() { RUN_TEST(test_probation_waits_30_seconds_and_a_first_frame); RUN_TEST(test_probation_needs_wifi_when_it_is_configured); RUN_TEST(test_probation_rolls_back_when_wifi_never_comes); + RUN_TEST(test_rollback_at_boot_after_an_unconfirmed_start); return UNITY_END(); }