diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index ec3f9de7..bcf94115 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -147,6 +147,7 @@ jobs: python3 -B test/test_elrs_power.py python3 -B test/test_ota_heap_context.py python3 -B test/test_client_acl_spiffs.py + python3 -B test/test_client_acl_cli.py -v - name: Verify ESP32 USB sleep and G3 button wake run: python3 -B test/test_esp32_usb_sleep.py diff --git a/examples/kiss_modem/main.cpp b/examples/kiss_modem/main.cpp index 5efdf091..760d8cd1 100644 --- a/examples/kiss_modem/main.cpp +++ b/examples/kiss_modem/main.cpp @@ -58,15 +58,16 @@ void loadOrCreateIdentity() { bool identity_ready = true; if (needs_identity) { identity_ready = mesh::generateUsableLocalIdentity(identity, radio_new_identity); - if (identity_ready) store.save("_main", identity); + if (identity_ready) identity_ready = store.saveWithRetry("_main", identity); } #if defined(ESP32_PLATFORM) mesh::discardESP32TrueRandom(); #endif if (!identity_ready) { - MESH_DEBUG_PRINTLN("Identity generation exhausted all attempts; rebooting"); + MESH_DEBUG_PRINTLN("Identity generation or persistence failed after retries; rebooting"); board.reboot(); + halt(); // Never let setup continue if a platform's reboot returns. } } diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index df0837e6..494a57bc 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -11974,8 +11974,10 @@ void MyMesh::handleCommand(uint32_t sender_timestamp, ClientInfo* sender, char * uint8_t pubkey[PUB_KEY_SIZE]; if (hex_len > 0 && hex_len <= PUB_KEY_SIZE * 2 && (hex_len & 1) == 0 && mesh::Utils::fromHex(pubkey, (int)(hex_len / 2), hex)) { - uint8_t perms = atoi(sp); - if (acl.applyPermissions(self_id, pubkey, (int)(hex_len / 2), perms)) { + uint32_t perms; + if (!mesh::cli::parseUnsignedIntegerStrict(sp, perms) || perms > UINT8_MAX) { + strcpy(reply, "Err - permissions must be 0-255"); + } else if (acl.applyPermissions(self_id, pubkey, (int)(hex_len / 2), static_cast(perms))) { mesh::scheduleLazyPersistenceMutation( dirty_contacts_expiry, contacts_save_failures, futureMillis(LAZY_CONTACTS_WRITE_DELAY)); diff --git a/examples/simple_repeater/main.cpp b/examples/simple_repeater/main.cpp index a6b8ac30..b03a99cf 100644 --- a/examples/simple_repeater/main.cpp +++ b/examples/simple_repeater/main.cpp @@ -177,14 +177,14 @@ void setup() { if (needs_identity) { MESH_DEBUG_PRINTLN("Generating new keypair"); identity_ready = mesh::generateUsableLocalIdentity(the_mesh.self_id, radio_new_identity); - if (identity_ready) store.save("_main", the_mesh.self_id); + if (identity_ready) identity_ready = store.saveWithRetry("_main", the_mesh.self_id); } #if defined(ESP32_PLATFORM) mesh::discardESP32TrueRandom(); #endif if (!identity_ready) { - MESH_DEBUG_PRINTLN("Identity generation exhausted all attempts; rebooting"); + MESH_DEBUG_PRINTLN("Identity generation or persistence failed after retries; rebooting"); board.reboot(); return; } diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index a8b8ff3f..bee3a640 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -2376,8 +2376,10 @@ void MyMesh::handleCommand(uint32_t sender_timestamp, char *command, char *reply uint8_t pubkey[PUB_KEY_SIZE]; if (hex_len > 0 && hex_len <= PUB_KEY_SIZE * 2 && (hex_len & 1) == 0 && mesh::Utils::fromHex(pubkey, (int)(hex_len / 2), hex)) { - uint8_t perms = atoi(sp); - if (acl.applyPermissions(self_id, pubkey, (int)(hex_len / 2), perms)) { + uint32_t perms; + if (!mesh::cli::parseUnsignedIntegerStrict(sp, perms) || perms > UINT8_MAX) { + strcpy(reply, "Err - permissions must be 0-255"); + } else if (acl.applyPermissions(self_id, pubkey, (int)(hex_len / 2), static_cast(perms))) { mesh::scheduleLazyPersistenceMutation( dirty_contacts_expiry, contacts_save_failures, futureMillis(LAZY_CONTACTS_WRITE_DELAY)); diff --git a/examples/simple_room_server/main.cpp b/examples/simple_room_server/main.cpp index 88ab8c3e..b1bf87b1 100644 --- a/examples/simple_room_server/main.cpp +++ b/examples/simple_room_server/main.cpp @@ -103,14 +103,14 @@ void setup() { bool identity_ready = true; if (needs_identity) { identity_ready = mesh::generateUsableLocalIdentity(the_mesh.self_id, radio_new_identity); - if (identity_ready) store.save("_main", the_mesh.self_id); + if (identity_ready) identity_ready = store.saveWithRetry("_main", the_mesh.self_id); } #if defined(ESP32_PLATFORM) mesh::discardESP32TrueRandom(); #endif if (!identity_ready) { - MESH_DEBUG_PRINTLN("Identity generation exhausted all attempts; rebooting"); + MESH_DEBUG_PRINTLN("Identity generation or persistence failed after retries; rebooting"); board.reboot(); return; } diff --git a/examples/simple_secure_chat/main.cpp b/examples/simple_secure_chat/main.cpp index 787fe58c..76c13250 100644 --- a/examples/simple_secure_chat/main.cpp +++ b/examples/simple_secure_chat/main.cpp @@ -348,14 +348,14 @@ public: identity_ready = mesh::generateUsableLocalIdentity( self_id, [this]() { return mesh::LocalIdentity(getRNG()); }); #endif - if (identity_ready) store.save("_main", self_id); + if (identity_ready) identity_ready = store.saveWithRetry("_main", self_id); } #if defined(ESP32_PLATFORM) mesh::discardESP32TrueRandom(); #endif if (!identity_ready) { - MESH_DEBUG_PRINTLN("Identity generation exhausted all attempts; rebooting"); + MESH_DEBUG_PRINTLN("Identity generation or persistence failed after retries; rebooting"); board.reboot(); return; } diff --git a/examples/simple_sensor/SensorMesh.cpp b/examples/simple_sensor/SensorMesh.cpp index dd4e7924..b3a0c1b4 100644 --- a/examples/simple_sensor/SensorMesh.cpp +++ b/examples/simple_sensor/SensorMesh.cpp @@ -626,8 +626,10 @@ void SensorMesh::handleCommand(uint32_t sender_timestamp, char* command, char* r uint8_t pubkey[PUB_KEY_SIZE]; if (hex_len > 0 && hex_len <= PUB_KEY_SIZE * 2 && (hex_len & 1) == 0 && mesh::Utils::fromHex(pubkey, (int)(hex_len / 2), hex)) { - uint8_t perms = atoi(sp); - if (acl.applyPermissions(self_id, pubkey, (int)(hex_len / 2), perms)) { + uint32_t perms; + if (!mesh::cli::parseUnsignedIntegerStrict(sp, perms) || perms > UINT8_MAX) { + strcpy(reply, "Err - permissions must be 0-255"); + } else if (acl.applyPermissions(self_id, pubkey, (int)(hex_len / 2), static_cast(perms))) { mesh::scheduleLazyPersistenceMutation( dirty_contacts_expiry, contacts_save_failures, futureMillis(LAZY_CONTACTS_WRITE_DELAY)); diff --git a/examples/simple_sensor/main.cpp b/examples/simple_sensor/main.cpp index 3c449a33..e8ff788d 100644 --- a/examples/simple_sensor/main.cpp +++ b/examples/simple_sensor/main.cpp @@ -116,14 +116,14 @@ void setup() { if (needs_identity) { MESH_DEBUG_PRINTLN("Generating new keypair"); identity_ready = mesh::generateUsableLocalIdentity(the_mesh.self_id, radio_new_identity); - if (identity_ready) store.save("_main", the_mesh.self_id); + if (identity_ready) identity_ready = store.saveWithRetry("_main", the_mesh.self_id); } #if defined(ESP32_PLATFORM) mesh::discardESP32TrueRandom(); #endif if (!identity_ready) { - MESH_DEBUG_PRINTLN("Identity generation exhausted all attempts; rebooting"); + MESH_DEBUG_PRINTLN("Identity generation or persistence failed after retries; rebooting"); board.reboot(); return; } diff --git a/src/helpers/ClientACL.cpp b/src/helpers/ClientACL.cpp index 17df9b20..64e92b13 100644 --- a/src/helpers/ClientACL.cpp +++ b/src/helpers/ClientACL.cpp @@ -283,7 +283,9 @@ static bool readClientLoginReplayCeiling( bool* found) { *ceiling = 0; *found = false; - if (!fs->exists(mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH)) return true; + bool present = false; + if (!mesh::filePresence(fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH, present)) return false; + if (!present) return true; if (!validateLoginReplayFileIntegrity( fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH)) { return false; @@ -338,7 +340,9 @@ static bool writeClientLoginReplayCeiling( mesh::ClientLoginReplayReservationAction action) { if (action == mesh::ClientLoginReplayReservationAction::None) return true; #if defined(NRF52_PLATFORM) - if (fs->exists(mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH) + bool primary_exists = false; + if (!mesh::filePresence(fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH, primary_exists)) return false; + if (primary_exists && !validateLoginReplayFileIntegrity( fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH)) { return false; @@ -352,7 +356,9 @@ static bool writeClientLoginReplayCeiling( File source = mesh::emptyFile(fs); size_t record_count = 0; - if (fs->exists(mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH)) { + bool source_exists = false; + if (!mesh::filePresence(fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH, source_exists)) return false; + if (source_exists) { source = openRead(fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH); // A previously present image becoming unreadable is not a first login. // Check again after opening so disappearance/truncation cannot underflow @@ -487,7 +493,11 @@ bool ClientACL::clampLoginReplayTimestamps( if (_fs == NULL || !login_replay_store_available || now == 0) return false; ClientLoginReplayClampResult pending = {}; - if (_fs->exists(mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH)) { + bool primary = false, backup = false, temp = false; + if (!mesh::filePresence(_fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH, primary) + || !mesh::filePresence(_fs, mesh::CLIENT_LOGIN_REPLAY_BACKUP_PATH, backup) + || !mesh::filePresence(_fs, mesh::CLIENT_LOGIN_REPLAY_TEMP_PATH, temp)) return false; + if (primary) { File source = openRead(_fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH); uint32_t original_crc = 0; const bool valid = copyClampedLoginReplay( @@ -546,8 +556,7 @@ bool ClientACL::clampLoginReplayTimestamps( return false; } } - } else if (_fs->exists(mesh::CLIENT_LOGIN_REPLAY_BACKUP_PATH) - || _fs->exists(mesh::CLIENT_LOGIN_REPLAY_TEMP_PATH)) { + } else if (backup || temp) { // Recovery is performed by load(). Do not mistake an interrupted/corrupt // transaction for a never-created store during this explicit operation. login_replay_store_available = false; @@ -585,10 +594,11 @@ void ClientACL::load(FILESYSTEM* fs, const mesh::LocalIdentity& self_id) { // rename. The live image remains authoritative. mesh::removeClientLoginReplayArtifact( _fs, mesh::CLIENT_LOGIN_REPLAY_TEMP_PATH); + bool replay_exists = false; login_replay_store_available = - !_fs->exists(mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH) - || validateLoginReplayFileIntegrity( - _fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH); + mesh::filePresence(_fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH, replay_exists) + && (!replay_exists || validateLoginReplayFileIntegrity( + _fs, mesh::CLIENT_LOGIN_REPLAY_PRIMARY_PATH)); #else login_replay_store_available = mesh::recoverClientLoginReplayFiles( _fs, validateLoginReplayFileIntegrity); diff --git a/src/helpers/ClientLoginPersistence.h b/src/helpers/ClientLoginPersistence.h index 1d7e4742..91066811 100644 --- a/src/helpers/ClientLoginPersistence.h +++ b/src/helpers/ClientLoginPersistence.h @@ -3,6 +3,7 @@ #include #include #include +#include "FilePresence.h" namespace mesh { @@ -162,14 +163,18 @@ inline bool validateClientLoginReplayImage(const uint8_t* image, template bool removeClientLoginReplayArtifact(Filesystem* fs, const char* path) { - if (!fs->exists(path)) return true; - fs->remove(path); - return !fs->exists(path); + bool present = false; + if (!filePresence(fs, path, present)) return false; + if (!present) return true; + return fs->remove(path) && filePresence(fs, path, present) && !present; } template bool recoverClientLoginReplayFiles(Filesystem* fs, Validator is_valid) { - if (fs->exists(CLIENT_LOGIN_REPLAY_PRIMARY_PATH) + bool primary = false, backup = false; + if (!filePresence(fs, CLIENT_LOGIN_REPLAY_PRIMARY_PATH, primary) + || !filePresence(fs, CLIENT_LOGIN_REPLAY_BACKUP_PATH, backup)) return false; + if (primary && is_valid(fs, CLIENT_LOGIN_REPLAY_PRIMARY_PATH)) { // The validated primary is already authoritative. Cleanup failure must // not disable replay protection or make committed state appear absent. @@ -177,7 +182,7 @@ bool recoverClientLoginReplayFiles(Filesystem* fs, Validator is_valid) { removeClientLoginReplayArtifact(fs, CLIENT_LOGIN_REPLAY_BACKUP_PATH); return true; } - if (fs->exists(CLIENT_LOGIN_REPLAY_BACKUP_PATH) + if (backup && is_valid(fs, CLIENT_LOGIN_REPLAY_BACKUP_PATH)) { if (!removeClientLoginReplayArtifact(fs, CLIENT_LOGIN_REPLAY_PRIMARY_PATH) @@ -190,8 +195,7 @@ bool recoverClientLoginReplayFiles(Filesystem* fs, Validator is_valid) { } // A never-created store is the upgrade/first-boot case. An invalid live or // backup image is different: retain it for diagnosis and fail closed. - return !fs->exists(CLIENT_LOGIN_REPLAY_PRIMARY_PATH) - && !fs->exists(CLIENT_LOGIN_REPLAY_BACKUP_PATH) + return !primary && !backup && removeClientLoginReplayArtifact(fs, CLIENT_LOGIN_REPLAY_TEMP_PATH); } @@ -204,9 +208,9 @@ bool publishClientLoginReplayTemp(Filesystem* fs, removeClientLoginReplayArtifact(fs, CLIENT_LOGIN_REPLAY_TEMP_PATH); return false; } - if (fs->exists(CLIENT_LOGIN_REPLAY_BACKUP_PATH)) return false; - - const bool had_primary = fs->exists(CLIENT_LOGIN_REPLAY_PRIMARY_PATH); + bool backup = false, had_primary = false; + if (!filePresence(fs, CLIENT_LOGIN_REPLAY_BACKUP_PATH, backup) || backup + || !filePresence(fs, CLIENT_LOGIN_REPLAY_PRIMARY_PATH, had_primary)) return false; if (had_primary && !fs->rename(CLIENT_LOGIN_REPLAY_PRIMARY_PATH, CLIENT_LOGIN_REPLAY_BACKUP_PATH)) { diff --git a/src/helpers/IdentityStore.cpp b/src/helpers/IdentityStore.cpp index 38531f85..baafa0f8 100644 --- a/src/helpers/IdentityStore.cpp +++ b/src/helpers/IdentityStore.cpp @@ -24,47 +24,55 @@ bool IdentityStore::recover(const char* name) { } bool IdentityStore::load(const char *name, mesh::LocalIdentity& id) { - if (!recover(name)) return false; - bool loaded = false; - char filename[40]; - sprintf(filename, "%s/%s.id", _dir, name); - if (_fs->exists(filename)) { -#if defined(RP2040_PLATFORM) - File file = _fs->open(filename, "r"); -#else - File file = _fs->open(filename); -#endif - if (file) { - loaded = id.readFrom(file); - file.close(); - } - } - return loaded; + return load(name, id, nullptr, 0); } bool IdentityStore::load(const char *name, mesh::LocalIdentity& id, char display_name[], int max_name_sz) { - if (!recover(name)) return false; - bool loaded = false; char filename[40]; - sprintf(filename, "%s/%s.id", _dir, name); - if (_fs->exists(filename)) { + if (snprintf(filename, sizeof(filename), "%s/%s.id", _dir, name) + >= (int)sizeof(filename)) return false; + for (unsigned attempt = 0; attempt < IO_ATTEMPTS; ++attempt) { + if (!recover(name)) continue; + bool present = false; + if (!mesh::filePresence(_fs, filename, present)) continue; + if (!present) return false; #if defined(RP2040_PLATFORM) File file = _fs->open(filename, "r"); #else File file = _fs->open(filename); #endif - if (file) { - loaded = id.readFrom(file); - - int n = max_name_sz; // up to 32 bytes - if (n > 32) n = 32; - file.read((uint8_t *) display_name, n); - display_name[n - 1] = 0; // ensure null terminator - + if (!file || file.isDirectory()) { file.close(); + continue; } + mesh::LocalIdentity loaded; + if (!loaded.readFrom(file)) { + file.close(); + continue; + } + // The legacy display name is optional. A key-only image remains valid; + // neither a short name nor a partial key may leak into the caller's state. + if (display_name != nullptr && max_name_sz > 0) { + const int n = max_name_sz > 32 ? 32 : max_name_sz; + char loaded_name[32]; + if (file.read(reinterpret_cast(loaded_name), n) == n) { + loaded_name[n - 1] = 0; + memcpy(display_name, loaded_name, n); + } + } + file.close(); + id = loaded; + return true; } - return loaded; + // Callers may provision a replacement after bounded recovery is exhausted. + return false; +} + +bool IdentityStore::saveWithRetry(const char* name, const mesh::LocalIdentity& id) { + for (unsigned attempt = 0; attempt < IO_ATTEMPTS; ++attempt) { + if (save(name, id)) return true; + } + return false; } bool IdentityStore::save(const char *name, const mesh::LocalIdentity& id) { diff --git a/src/helpers/IdentityStore.h b/src/helpers/IdentityStore.h index c8f7aefa..2c079bc6 100644 --- a/src/helpers/IdentityStore.h +++ b/src/helpers/IdentityStore.h @@ -15,6 +15,7 @@ class IdentityStore { FILESYSTEM* _fs; const char* _dir; public: + static constexpr unsigned IO_ATTEMPTS = 3; IdentityStore(FILESYSTEM& fs, const char* dir): _fs(&fs), _dir(dir) { } void begin() { @@ -25,5 +26,7 @@ public: bool load(const char *name, mesh::LocalIdentity& id); bool load(const char *name, mesh::LocalIdentity& id, char display_name[], int max_name_sz); bool save(const char *name, const mesh::LocalIdentity& id); + // Startup must not run with a new identity that was never made durable. + bool saveWithRetry(const char *name, const mesh::LocalIdentity& id); bool save(const char *name, const mesh::LocalIdentity& id, const char display_name[]); }; diff --git a/test/fixtures/client_acl_cli/test_client_acl_cli.cpp b/test/fixtures/client_acl_cli/test_client_acl_cli.cpp index 0fe19b73..e85e704d 100644 --- a/test/fixtures/client_acl_cli/test_client_acl_cli.cpp +++ b/test/fixtures/client_acl_cli/test_client_acl_cli.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -32,6 +33,13 @@ static Stream console; Stream& usbConsolePort() { return console; } } +static mesh::LocalIdentity self_id; +static unsigned long dirty_contacts_expiry = 0; +static uint8_t contacts_save_failures = 0; +static const unsigned long LAZY_CONTACTS_WRITE_DELAY = 5000; +static unsigned long futureMillis(unsigned long delay) { return delay + 1; } +static unsigned gps_updates = 0; +static void updateGpsTelemetryPolicy() { ++gps_updates; } #include "production.h" static ClientInfo* add(ClientACL& acl, unsigned index, uint8_t permissions) { @@ -244,6 +252,44 @@ static void repeater_delegation_stays_denied() { } } +static void permissions_validate_before_mutation() { + for (auto handler : {repeaterCommand, roomCommand, sensorCommand}) { + for (uint32_t timestamp : {0U, 12345U}) { + FakeFilesystem fs; + ClientACL acl; + acl.load(&fs, self_id); + auto* admin = add(acl, 0, PERM_ACL_ADMIN); + auto* target = add(acl, 1, PERM_ACL_ADMIN); + uint8_t target_key[PUB_KEY_SIZE]; + memcpy(target_key, target->id.pub_key, sizeof(target_key)); + char key[65]; mesh::Utils::toHex(key, target_key, sizeof(target_key)); + CHECK(acl.save(&fs)); + const auto durable = fs.files; + for (const char* value : {"", "invalid", "-1", "256", "259", "3bad", "3.0", + "+", "4294967296", "99999999999999999999999"}) { + char command[140], reply[160] = {}; + snprintf(command, sizeof(command), "setperm %s %s", key, value); + dirty_contacts_expiry = 0; + gps_updates = 0; + handler(acl, timestamp ? admin : nullptr, timestamp, command, reply); + CHECK(strncmp(reply, "Err", 3) == 0); + CHECK(acl.getNumClients() == 2 && acl.getClient(target_key, PUB_KEY_SIZE)->isAdmin()); + CHECK(dirty_contacts_expiry == 0 && fs.files == durable); + CHECK(gps_updates == 0); + } + for (unsigned value : {1U, 2U, 3U, 5U, 255U, 0U}) { + char command[140], reply[160] = {}; + snprintf(command, sizeof(command), "setperm %s %u", key, value); + dirty_contacts_expiry = 0; + handler(acl, timestamp ? admin : nullptr, timestamp, command, reply); + CHECK(strcmp(reply, "OK") == 0 && dirty_contacts_expiry != 0); + auto* changed = acl.getClient(target_key, PUB_KEY_SIZE); + CHECK(value == 0 ? changed == nullptr : changed && changed->permissions == value); + } + } + } +} + int main() { empty_and_single_entry(); skips_inactive_and_preserves_full_keys(); @@ -254,5 +300,6 @@ int main() { listing_does_not_mutate_acl(); actual_role_dispatch_handles_radio_and_local(); repeater_delegation_stays_denied(); - puts("9 ACL CLI checks passed"); + permissions_validate_before_mutation(); + puts("10 ACL CLI checks passed"); } diff --git a/test/fixtures/client_acl_spiffs/mocks/Arduino.h b/test/fixtures/client_acl_spiffs/mocks/Arduino.h index f0c5ef2c..411290cd 100644 --- a/test/fixtures/client_acl_spiffs/mocks/Arduino.h +++ b/test/fixtures/client_acl_spiffs/mocks/Arduino.h @@ -61,7 +61,10 @@ public: size_t fail_read_open = 0; size_t fail_read_after = 0; - bool exists(const char* path) const { return files.count(path) != 0; } + // ESP32 VFS exists() opens the file; stat/rename use the namespace instead. + bool exists(const char* path) const { + return files.count(path) != 0 && unreadable.count(path) == 0; + } File open(const char* path, const char* mode = "r", bool = false) { const std::string name(path); if (mode[0] == 'r') { @@ -86,7 +89,7 @@ public: } bool rename(const char* from, const char* to) { if (fail_rename_from == from || fail_rename_from_paths.count(from) - || !exists(from) || exists(to)) return false; + || files.count(from) == 0 || files.count(to) != 0) return false; files[to] = files[from]; files.erase(from); return true; diff --git a/test/fixtures/client_acl_spiffs/test_client_acl_spiffs.cpp b/test/fixtures/client_acl_spiffs/test_client_acl_spiffs.cpp index c3b3562f..5353c37d 100644 --- a/test/fixtures/client_acl_spiffs/test_client_acl_spiffs.cpp +++ b/test/fixtures/client_acl_spiffs/test_client_acl_spiffs.cpp @@ -515,10 +515,46 @@ static void incomplete_acl_load_is_never_authoritative() { } } +static void replay_metadata_errors_never_erase_authority() { + for (unsigned fault = 0; fault < 2; ++fault) { + FakeFilesystem fs; + ClientACL acl; + acl.load(&fs, SELF); + CHECK(acl.authorizeLoginTimestamp(KEY, 100, 0, PERM_ACL_ADMIN)); + auto* client = acl.putClient(mesh::Identity(KEY), PERM_ACL_ADMIN); + CHECK(client); + client->last_timestamp = 500; + const auto original = fs.files[PRIMARY]; + if (fault == 0) fs.unreadable.insert(PRIMARY); + else fs.metadata_error = true; + CHECK(!mesh::recoverClientLoginReplayFiles(&fs, validateLoginReplayFileIntegrity)); + uint32_t ceiling; + bool found; + CHECK(!readClientLoginReplayCeiling(&fs, KEY, &ceiling, &found)); + CHECK(!writeClientLoginReplayCeiling(&fs, KEY, 900, + mesh::ClientLoginReplayReservationAction::CreateNew)); + ClientLoginReplayClampResult result{}; + CHECK(!acl.clampLoginReplayTimestamps(KEY, 50, result)); + check_empty_result(result); + CHECK(client->last_timestamp == 500); + CHECK(!acl.authorizeLoginTimestamp(KEY, 99, 0, PERM_ACL_READ_ONLY)); + CHECK(fs.files[PRIMARY] == original); + ClientACL reboot; + reboot.load(&fs, SELF); + CHECK(!reboot.authorizeLoginTimestamp(KEY, 99, 0, PERM_ACL_READ_ONLY)); + CHECK(fs.files[PRIMARY] == original); + fs.unreadable.clear(); fs.metadata_error = false; + reboot.load(&fs, SELF); + CHECK(!reboot.authorizeLoginTimestamp(KEY, 100, 0, PERM_ACL_ADMIN)); + CHECK(reboot.authorizeLoginTimestamp(KEY, 161, 0, PERM_ACL_ADMIN)); + } +} + int main() { const struct { const char* name; void (*run)(); } tests[] = { {"allocation failure preserves clients", allocation_failure_preserves_saved_clients}, {"incomplete ACL is not authoritative", incomplete_acl_load_is_never_authoritative}, + {"replay read and metadata failures stay closed", replay_metadata_errors_never_erase_authority}, {"missing read differs from empty file", missing_read_is_not_empty_file}, {"first admin and monotonic retries", first_admin_and_retries}, {"reboot preserves ceiling", reboot_preserves_ceiling}, @@ -548,5 +584,5 @@ int main() { test.run(); std::printf("PASS: %s\n", test.name); } - std::puts("26 ClientACL SPIFFS checks passed"); + std::puts("27 ClientACL SPIFFS checks passed"); } diff --git a/test/fixtures/companion_uncached_storage/test.cpp b/test/fixtures/companion_uncached_storage/test.cpp index 71cce579..10474f09 100644 --- a/test/fixtures/companion_uncached_storage/test.cpp +++ b/test/fixtures/companion_uncached_storage/test.cpp @@ -22,6 +22,7 @@ public: File(FakeFilesystem* filesystem, const char* name, bool write) : fs(filesystem), path(name), writing(write) {} explicit operator bool() const { return fs != nullptr; } + bool isDirectory() const { return false; } bool open(const char* name, uint8_t mode); size_t read(uint8_t* bytes, size_t length); size_t write(const uint8_t* bytes, size_t length); @@ -41,6 +42,7 @@ public: unsigned fail_open_remaining = std::numeric_limits::max(); int stat_error = 0; unsigned fail_rename = 0, renames = 0, writes = 0; + unsigned read_opens = 0; std::vector snapshots; bool exists(const char* path) const { #if defined(ESP32_PLATFORM) @@ -54,6 +56,7 @@ public: void _unlockFS() {} FakeFilesystem* _getFS() { return this; } File open(const char* path, const char* mode = "r", bool = false) { + if (*mode == 'r') ++read_opens; if (fail_open == path && fail_open_remaining != 0) { --fail_open_remaining; return File(); diff --git a/test/fixtures/contact_cache/mocks/Identity.h b/test/fixtures/contact_cache/mocks/Identity.h index f518766c..d33a1840 100644 --- a/test/fixtures/contact_cache/mocks/Identity.h +++ b/test/fixtures/contact_cache/mocks/Identity.h @@ -12,6 +12,10 @@ class LocalIdentity : public Identity { public: uint8_t private_key[64] = {}; mutable unsigned derivations = 0; + template bool readFrom(Reader& in) { + return in.read(pub_key, sizeof(pub_key)) == sizeof(pub_key) + && in.read(private_key, sizeof(private_key)) == sizeof(private_key); + } size_t writeTo(uint8_t* out, size_t size) const { if (size < 96) return 0; memcpy(out, private_key, 64); diff --git a/test/test_client_acl_cli.py b/test/test_client_acl_cli.py index 9121b001..8143e375 100644 --- a/test/test_client_acl_cli.py +++ b/test/test_client_acl_cli.py @@ -26,6 +26,7 @@ def role_handler(role, source): body = extract_braced(source, f"void {owner}::handleCommand(uint32_t sender_timestamp,") zero_guard = re.search(r"if \([^\n]+sender_timestamp == 0\) sender_timestamp = 1;", body).group() prefix = extract_braced(body, "if (strlen(command) > 4 && command[2] == '|')") + permissions = extract_braced(body, 'if (memcmp(command, "setperm ", 8) == 0)') paged = extract_braced(body, "if (mesh::cli::handleACLGet(") local = extract_braced(body, 'if (sender_timestamp == 0 && strcmp(command, "get acl") == 0)') # The room source opens its next conditional branch's preprocessor guard @@ -46,7 +47,7 @@ static void {role}Command(ClientACL& acl, ClientInfo* sender, {prefix} mesh::cli::normalizeCommandVerb(command); {guard} - {paged} else {local} else strcpy(reply, "unhandled"); + {permissions} else {paged} else {local} else strcpy(reply, "unhandled"); }} """ @@ -61,7 +62,10 @@ class ClientAclCliTest(unittest.TestCase): hex_chars = re.search(r"static const char hex_chars\[\].*;", utils).group() generated = "namespace mesh {\n" + hex_chars + "\n" generated += extract_braced(utils, "void Utils::toHex(") + "\n" - generated += extract_braced(utils, "void Utils::printHex(") + "\n}\n" + for signature in ("void Utils::printHex(", "static uint8_t hexVal(", + "bool Utils::isHexChar(", "bool Utils::fromHex("): + generated += extract_braced(utils, signature) + "\n" + generated += "}\n" for signature in ( "static bool commandFamilyMatches(", "static bool isCommonManagerReadOnlyAllowed(", "static bool isRegionMgrAllowed(", "static bool isFilterMgrAllowed(", @@ -80,6 +84,8 @@ class Stream; namespace mesh { class Utils { public: static void toHex(char*, const uint8_t*, size_t); static void printHex(Stream&, const uint8_t*, size_t); + static bool isHexChar(char); + static bool fromHex(uint8_t*, int, const char*); }; } """) (work / "production.h").write_text(generated) @@ -96,7 +102,7 @@ namespace mesh { class Utils { public: self.assertEqual(result.returncode, 0, result.stdout + result.stderr) checked = subprocess.run([str(binary)], capture_output=True, text=True, timeout=10) self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) - self.assertIn("9 ACL CLI checks passed", checked.stdout) + self.assertIn("10 ACL CLI checks passed", checked.stdout) def test_remote_dispatch_stays_behind_existing_admin_guards(self): for role, path in ROLES.items(): diff --git a/test/test_client_acl_spiffs.py b/test/test_client_acl_spiffs.py index 979d6714..a9933ff3 100644 --- a/test/test_client_acl_spiffs.py +++ b/test/test_client_acl_spiffs.py @@ -25,8 +25,8 @@ class ClientAclSpiffsTest(unittest.TestCase): self.assertEqual(compiled.returncode, 0, compiled.stdout + compiled.stderr) checked = subprocess.run([str(binary)], capture_output=True, text=True, timeout=10) self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) - self.assertIn("26 ClientACL SPIFFS checks passed", checked.stdout) - self.assertEqual(checked.stdout.count("PASS:"), 26) + self.assertIn("27 ClientACL SPIFFS checks passed", checked.stdout) + self.assertEqual(checked.stdout.count("PASS:"), 27) if __name__ == "__main__": diff --git a/test/test_identity_and_settings_recovery.py b/test/test_identity_and_settings_recovery.py index 100c4a77..6c7d0b6d 100644 --- a/test/test_identity_and_settings_recovery.py +++ b/test/test_identity_and_settings_recovery.py @@ -54,11 +54,68 @@ class IdentityAndSettingsRecovery(unittest.TestCase): program += 'namespace mesh { template \n' + extract_braced( presence, 'bool filePresence(') + '\n}\n' for signature in ('bool IdentityStore::recover(', + 'bool IdentityStore::load(const char *name, mesh::LocalIdentity& id)', + 'bool IdentityStore::load(const char *name, mesh::LocalIdentity& id, char display_name[], int max_name_sz)', + 'bool IdentityStore::saveWithRetry(', 'bool IdentityStore::save(const char *name, const mesh::LocalIdentity& id)', 'bool IdentityStore::save(const char *name, const mesh::LocalIdentity& id, const char display_name[])'): program += extract_braced(source, signature) + '\n' program += r''' int main() { + // Startup reads reopen a transiently failing file and never publish a + // partially read key. Exhausted reads still permit provisioning a new key. + for (bool display : {false, true}) { + for (unsigned misses : {0U, 1U, 2U, 3U}) { + filesystem = FakeFilesystem(); + IdentityStore store(filesystem, ""); + mesh::LocalIdentity saved, live; + memset(saved.pub_key, 7, PUB_KEY_SIZE); + memset(saved.private_key, 8, PRV_KEY_SIZE); + memset(live.pub_key, 9, PUB_KEY_SIZE); + assert(store.save("main", saved, "saved name")); + char name[32] = "unchanged"; + filesystem.fail_open = "/main.id"; + filesystem.fail_open_remaining = misses; + filesystem.read_opens = 0; + const bool loaded = display ? store.load("main", live, name, sizeof(name)) + : store.load("main", live); + assert(loaded == (misses < IdentityStore::IO_ATTEMPTS)); + assert(filesystem.read_opens == (misses < 3 ? misses + 1 : 3)); + if (loaded) { + assert(memcmp(live.pub_key, saved.pub_key, PUB_KEY_SIZE) == 0); + assert(memcmp(live.private_key, saved.private_key, PRV_KEY_SIZE) == 0); + if (display) assert(strcmp(name, "saved name") == 0); + } else { + assert(live.pub_key[0] == 9 && strcmp(name, "unchanged") == 0); + assert(store.saveWithRetry("main", live)); + } + } + } + filesystem = FakeFilesystem(); + IdentityStore boot_store(filesystem, ""); + mesh::LocalIdentity old_identity, new_identity; + memset(old_identity.pub_key, 7, PUB_KEY_SIZE); + memset(new_identity.pub_key, 9, PUB_KEY_SIZE); + assert(boot_store.save("main", old_identity)); + const auto previous = filesystem.files["/main.id"]; + filesystem.max_read = 3; + filesystem.read_opens = 0; + char optional_name[32] = "keep default"; + assert(!boot_store.load("main", new_identity, optional_name, sizeof(optional_name))); + assert(filesystem.read_opens == 3 && new_identity.pub_key[0] == 9); + assert(strcmp(optional_name, "keep default") == 0); + filesystem.max_read = std::numeric_limits::max(); + assert(boot_store.load("main", new_identity, optional_name, sizeof(optional_name))); + assert(strcmp(optional_name, "keep default") == 0); // key-only image is valid + memset(new_identity.pub_key, 9, PUB_KEY_SIZE); + filesystem.fail_open = "/main.id.tmp"; + filesystem.fail_open_remaining = 2; + assert(boot_store.saveWithRetry("main", new_identity)); + assert(filesystem.files["/main.id"] != previous); + const auto committed = filesystem.files["/main.id"]; + filesystem.fail_open_remaining = std::numeric_limits::max(); + assert(!boot_store.saveWithRetry("main", old_identity)); + assert(filesystem.files["/main.id"] == committed); for (bool display : {false, true}) { for (unsigned fault = 0; fault < 4; ++fault) { filesystem = FakeFilesystem(); @@ -97,6 +154,16 @@ int main() { ''' compile_run(program, platform) + def test_startup_requires_durable_generated_identity(self): + for role in ('simple_repeater', 'simple_room_server', 'simple_sensor', + 'kiss_modem', 'simple_secure_chat'): + with self.subTest(role=role): + source = (ROOT / 'examples' / role / 'main.cpp').read_text() + self.assertIn('if (identity_ready) identity_ready = store.saveWithRetry(', source) + failed = extract_braced(source, 'if (!identity_ready)') + self.assertIn('board.reboot();', failed) + self.assertIn('halt();' if role == 'kiss_modem' else 'return;', failed) + def test_esp32_preferences_and_channels_recover_or_reset(self): store = (ROOT / 'examples/companion_radio/DataStore.cpp').read_text() prefs = (ROOT / 'examples/companion_radio/NodePrefs.h').read_text()