From c7d377a7cbb1583288bd61339f73275601dd5a5f Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 06:53:19 -0700 Subject: [PATCH] Hold fleet authority after uncertain settings publication --- docs/fleet_management.md | 8 ++ src/helpers/FleetChannel.cpp | 29 ++++++- test/test_fleet_channel_runtime.py | 124 ++++++++++++++++++++++++++++- 3 files changed, 156 insertions(+), 5 deletions(-) diff --git a/docs/fleet_management.md b/docs/fleet_management.md index fa3ca8438..78e3405a8 100644 --- a/docs/fleet_management.md +++ b/docs/fleet_management.md @@ -61,6 +61,14 @@ last accepted sequence. It does not reveal the channel key. `set fleet.channel off` disables reception; `set fleet.controller off` revokes the publisher. Key changes use the existing individually authenticated admin path, never fleet commands. + +If a settings save reports `unconfirmed; restart required`, the durable result +is unknown. Fleet reception and further fleet settings writes are disabled, and +`get fleet.channel` reports `store=error`. Enrollment shown in RAM is not active +authority while held. Restart to load and validate the public file, then check +enrollment before retrying. A `not saved; retry later` error occurred before +publication and does not impose this hold. + `get fleet.stats` includes `parts`, the number of fragments currently buffered across the two bounded, incomplete two-packet command slots. diff --git a/src/helpers/FleetChannel.cpp b/src/helpers/FleetChannel.cpp index 21bbaf725..9564c7d5d 100644 --- a/src/helpers/FleetChannel.cpp +++ b/src/helpers/FleetChannel.cpp @@ -58,10 +58,31 @@ bool FleetChannel::save(bool enabled, const uint8_t* key, const uint8_t* control storage::writeLE32(image + 60, storage::updateCRC32(0xffffffff, image, 60)); #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) AtomicFileWriter writer(fs_, Path); + const bool wrote = writer && writer.write(image, sizeof(image)) == sizeof(image); + const bool saved = writer.commit(wrote); + if (writer.outcome() == AtomicFileWriter::Outcome::Indeterminate) healthy_ = false; + return saved; #else ContactFileTransaction writer(fs_, Path, filePresence); + if (!writer || writer.write(image, sizeof(image)) != sizeof(image)) return false; + ContactFileTransaction::CommitProgress progress; + bool publishing; + do { + publishing = writer.readyToPublish(); + progress = writer.serviceCommit(); + } while (progress == ContactFileTransaction::CommitProgress::Pending); + if (!publishing) return false; // No publication attempt; local retry is safe. + // The fallback can report a late rename error or false success. Its IEEE + // checksum also cannot distinguish valid images carrying an IEEE CRC tail. + // Confirm exact public bytes before accepting new authority or using old RAM. + auto file = openFileRead(fs_, Path); + uint8_t verified[ImageSize]; + healthy_ = file && !file.isDirectory() && file.size() == sizeof(verified) + && file.read(verified, sizeof(verified)) == (int)sizeof(verified) + && memcmp(image, verified, sizeof(image)) == 0; + file.close(); + return healthy_; #endif - return writer && writer.write(image, sizeof(image)) == sizeof(image) && writer.commit(); } bool FleetChannel::handleConfig(const char* command, char* reply, size_t capacity) { @@ -94,7 +115,8 @@ bool FleetChannel::handleConfig(const char* command, char* reply, size_t capacit snprintf(reply, capacity, "Err - use a private random 128-bit key or off"); return true; } if (!save(enabled, key, controller_.pub_key, last_sequence_)) { - snprintf(reply, capacity, "Err - fleet settings not saved; store unavailable"); return true; + snprintf(reply, capacity, "Err - fleet settings %s", healthy_ + ? "not saved; retry later" : "unconfirmed; restart required"); return true; } configured_ = enabled; clearAssembly(); @@ -113,7 +135,8 @@ bool FleetChannel::handleConfig(const char* command, char* reply, size_t capacit snprintf(reply, capacity, "Err - use publisher public key (64 hex) or off"); return true; } if (!save(configured_, channel_.secret, key, last_sequence_)) { - snprintf(reply, capacity, "Err - fleet settings not saved; store unavailable"); return true; + snprintf(reply, capacity, "Err - fleet settings %s", healthy_ + ? "not saved; retry later" : "unconfirmed; restart required"); return true; } memcpy(controller_.pub_key, key, 32); controller_set_ = !zero(key, 32); diff --git a/test/test_fleet_channel_runtime.py b/test/test_fleet_channel_runtime.py index 38487e761..74545ad11 100644 --- a/test/test_fleet_channel_runtime.py +++ b/test/test_fleet_channel_runtime.py @@ -117,6 +117,9 @@ namespace fs { class FS { public: std::map> files; bool fail_open=false, fail_read=false, fail_write=false, fail_rename=false; + bool fail_after_publish=false, late_publish_error=false, fail_rollback=false; + bool false_success_rename=false, false_success_publish=false; + std::vector substitute_temp; unsigned reads=0,writes=0,renames=0; std::string fail_rename_from; void _lockFS() {} void _unlockFS() {} FS* _getFS(){return this;} @@ -125,11 +128,16 @@ class FS { public: bool mkdir(const char*) {return true;} bool rename(const char* old,const char* next) { ++renames; if(fail_rename || fail_rename_from==old || !exists(old)) return false; - files[next]=std::move(files[old]);files.erase(old);return true; + if(fail_rollback&&!strcmp(old,"/fleet_channel.bak"))return false; + if(false_success_rename||(false_success_publish&&!strcmp(old,"/fleet_channel.tmp")))return true; + files[next]=std::move(files[old]);files.erase(old); + if(!strcmp(next,"/fleet_channel")&&fail_after_publish)fail_read=true; + return !(late_publish_error&&!strcmp(old,"/fleet_channel.tmp")); } File open(const char* path,const char* mode="r") { if(!path||fail_open) return {}; if(mode[0]=='w') {files[path].clear();return File(this,path,true);} + if(!strcmp(path,"/fleet_channel.tmp")&&!substitute_temp.empty())files[path]=substitute_temp; return File(this,path,exists(path)); } }; @@ -961,7 +969,104 @@ int main(int argc,char** argv){ if(scenario.rfind("fragments_",0)==0)return fragmentScenario(scenario); fs::FS fs;Mesh mesh;RadioProfileCLI profiles; FleetChannel fleet(&fs);LocalIdentity publisher;bind(fleet,mesh,profiles); - if(scenario=="enrollment"){ + if(scenario=="local_config_indeterminate"){ + for(bool controller:{false,true}){ + fs::FS disk;Mesh transport;RadioProfileCLI radio;FleetChannel receiver(&disk);enroll(receiver,publisher); + disk.fail_after_publish=true; + assert(config(receiver,controller?"set fleet.controller off":"set fleet.channel off").rfind("Err",0)==0); + const auto published=disk.files.at(Path); + assert(controller?std::all_of(published.begin()+24,published.begin()+56,[](uint8_t b){return b==0;}):published[4]==0); + disk.fail_after_publish=false;disk.fail_read=false; + assert(config(receiver,"get fleet.channel").find("store=error")!=std::string::npos); + const unsigned writes=disk.writes,renames=disk.renames,verifies=verify_calls; + Packet packet=command(publisher,transport.clock.now,"get radio2"); + assert(apply(receiver,disk,transport,radio,packet)==0); + const std::string partial=controller?std::string("set fleet.channel ")+Key: + std::string("set fleet.controller ")+publicHex(publisher); + assert(config(receiver,partial).find("unconfirmed")!=std::string::npos); + assert(disk.files.at(Path)==published&&disk.writes==writes&&disk.renames==renames&&verify_calls==verifies); + FleetChannel rebooted(&disk); + assert(config(rebooted,"get fleet.channel").find("store=ok")!=std::string::npos); + assert(apply(rebooted,disk,transport,radio,packet)==0&&disk.files.at(Path)==published); + assert(config(rebooted,controller?std::string("set fleet.controller ")+publicHex(publisher): + std::string("set fleet.channel ")+Key).rfind("OK",0)==0); + assert(apply(rebooted,disk,transport,radio,packet)==1); + } + }else if(scenario=="local_config_prepublication"){ + for(unsigned fault=0;fault<3;++fault){ + fs::FS disk;Mesh transport;RadioProfileCLI radio;FleetChannel receiver(&disk);enroll(receiver,publisher); + const auto previous=disk.files.at(Path); + disk.fail_open=fault==0;disk.fail_write=fault==1;disk.fail_read=fault==2; + assert(config(receiver,"set fleet.controller off").rfind("Err",0)==0); + assert(disk.files.at(Path)==previous); + disk.fail_open=disk.fail_write=disk.fail_read=false; + assert(config(receiver,"get fleet.channel").find("store=ok")!=std::string::npos); + Packet packet=command(publisher,transport.clock.now,"get radio2"); + assert(apply(receiver,disk,transport,radio,packet)==1); + assert(config(receiver,"set fleet.controller off").rfind("OK",0)==0); + ++transport.clock.now;packet=command(publisher,transport.clock.now,"get radio2"); + assert(apply(receiver,disk,transport,radio,packet)==0); + } + }else if(scenario=="local_config_publish_reconciliation"){ + for(bool false_success:{false,true}){ + fs::FS disk;Mesh transport;RadioProfileCLI radio;FleetChannel receiver(&disk);enroll(receiver,publisher); + const auto previous=disk.files.at(Path); + disk.false_success_rename=false_success;disk.late_publish_error=!false_success;disk.fail_rollback=true; + const std::string reply=config(receiver,"set fleet.controller off"); + assert(reply.rfind(false_success?"Err":"OK",0)==0); + disk.false_success_rename=disk.late_publish_error=disk.fail_rollback=false; + assert(config(receiver,"get fleet.channel").find(false_success?"store=error":"store=ok")!=std::string::npos); + Packet packet=command(publisher,transport.clock.now,"get radio2"); + const auto published=disk.files.at(Path);const unsigned writes=disk.writes; + assert(apply(receiver,disk,transport,radio,packet)==0&&disk.writes==writes&&disk.files.at(Path)==published); + assert((published==previous)==false_success); + FleetChannel rebooted(&disk); + assert(apply(rebooted,disk,transport,radio,packet)==unsigned(false_success)); + } + }else if(scenario=="local_config_missing_public"){ + enroll(fleet,publisher);const auto previous=fs.files.at(Path);fs.false_success_publish=true; + assert(config(fleet,"set fleet.controller off").rfind("Err",0)==0); + fs.false_success_publish=false; + assert(config(fleet,"get fleet.channel").find("store=error")!=std::string::npos); +#if defined(RP2040_PLATFORM) + // The fallback moved the old target to backup, then trusted the no-op + // publication and removed that backup. Fleet must quarantine the missing name. + assert(!fs.exists(Path)&&!fs.exists("/fleet_channel.bak")); +#else + assert(fs.files.at(Path)==previous); +#endif + const auto published=fs.files;const unsigned writes=fs.writes,renames=fs.renames; + Packet packet=command(publisher,mesh.clock.now,"get radio2"); + assert(apply(fleet,fs,mesh,profiles,packet)==0); + assert(config(fleet,"set fleet.channel off").find("unconfirmed")!=std::string::npos); + assert(fs.files==published&&fs.writes==writes&&fs.renames==renames); + FleetChannel rebooted(&fs); +#if defined(RP2040_PLATFORM) + assert(config(rebooted,"get fleet.channel").find("off,controller=unset,store=ok")!=std::string::npos); + assert(apply(rebooted,fs,mesh,profiles,packet)==0); +#else + assert(apply(rebooted,fs,mesh,profiles,packet)==1); +#endif + }else if(scenario=="local_config_staged_substitution"){ + enroll(fleet,publisher);const auto previous=fs.files.at(Path); + fs.substitute_temp=previous; + assert(config(fleet,"set fleet.controller off").rfind("Err",0)==0); + assert(fs.files.at(Path)==previous);fs.substitute_temp.clear(); +#if defined(NRF52_PLATFORM) + // CRC32C detects the substituted IEEE-CRC-tail image before publication. + assert(config(fleet,"get fleet.channel").find("store=ok")!=std::string::npos); + assert(config(fleet,"set fleet.controller off").rfind("OK",0)==0); +#else + // The fallback's IEEE whole-image residue matches. Exact public bytes must + // reject the old valid image and block any further stale writes until restart. + assert(config(fleet,"get fleet.channel").find("store=error")!=std::string::npos); + const unsigned writes=fs.writes; + assert(config(fleet,"set fleet.channel off").find("unconfirmed")!=std::string::npos); + Packet packet=command(publisher,mesh.clock.now,"get radio2"); + assert(apply(fleet,fs,mesh,profiles,packet)==0&&fs.writes==writes&&fs.files.at(Path)==previous); + FleetChannel rebooted(&fs);assert(apply(rebooted,fs,mesh,profiles,packet)==1); +#endif + }else if(scenario=="enrollment"){ Packet packet=command(publisher,mesh.clock.now);assert(apply(fleet,fs,mesh,profiles,packet)==0); assert(config(fleet,"get fleet.channel").find("off,controller=unset,store=ok")!=std::string::npos); assert(config(fleet,"set fleet.channel 00000000000000000000000000000000").rfind("Err",0)==0); @@ -1457,6 +1562,21 @@ class SHA256 : public SHA256Base {{ public: def test_failed_configuration_write_preserves_durable_enrollment(self): self.scenario("config_fault") + def test_uncertain_local_revocation_blocks_old_authority_and_partial_writes_until_restart(self): + self.scenario("local_config_indeterminate") + + def test_proven_prepublication_configuration_failures_remain_retryable(self): + self.scenario("local_config_prepublication") + + def test_local_configuration_reconciles_late_errors_and_rejects_false_success(self): + self.scenario("local_config_publish_reconciliation") + + def test_valid_same_crc_tail_staged_substitution_cannot_acknowledge_revocation(self): + self.scenario("local_config_staged_substitution") + + def test_false_success_final_rename_missing_public_image_blocks_live_writes(self): + self.scenario("local_config_missing_public") + def test_corrupt_short_long_unreadable_images_fail_closed_and_backup_recovers(self): self.scenario("images")