Clock towers + pio testing support#2504
Conversation
Adds an opt-in "trusted clock" list of advert pubkeys whose timestamps
may step the local RTC. Replay-guarded (advert_ts must be strictly newer
than the last accepted from the same key, RAM-only cursor). Steps only
when |advert_ts - local| >= clock.trust.thresh (default 60s, set 0 to
disable). Sources have an optional human-readable name. Last few sync
events are kept in a small RAM ring queryable via the CLI.
Logic lives in TrustedClockTable (no Arduino deps): table ops, replay
predicate, threshold predicate, sync-event ring, and pure
serialize/deserialize. MyMesh holds it as a member and wraps file I/O
around the helper.
CLI under clock.trust:
clock.trust list
clock.trust events
clock.trust add <pubkey> [name]
clock.trust remove <pubkey-or-name>
Persisted via NodePrefs.clock_trust_thresh (set/get clock.trust.thresh)
and /trusted_clocks file (u8 version, u8 count, count*{pubkey, name}).
Adds a PlatformIO native test env so the helper logic can be exercised on the host without touching hardware. 11 Unity test cases cover add/find/remove (incl. dedup, capacity, mid-array shift), replay and threshold predicates at boundaries, sync-event ring eviction order, serialize/deserialize round-trip, bad-version rejection, count clamping, truncated records, and name-length truncation. Layout follows PIO conventions: tests live under test/, run via `pio test -e native` or `make test`. New host-tests.yml workflow runs the same command on PRs/pushes that touch src, examples, test, or platformio.ini.
|
This looks interesting. |
|
Following and will be testing. Appreciate the opt-in model and having to define the trusted clock source. |
|
My idea (not implemented yet) was to make a generic trust list that has a set of trust "scopes" for each trusted public key. Example based on your syntax: This would contribute a generic and extendable trust mechanism. The clock-specific commands would be under a different base command: etc.. |
I am good with that idea if everybody else agrees I can switch to something like that :), would be more reusable as you say :) |
|
Curious if this couldn't use the same ACL used for repeater admin, but just as a new ACL flag (4) for trust-clock-sync or something? Just because the ACL system is already there. Eg, see PR #2438 |
Thanks for the pointer. I will investigate, because this ACL list is for Clients instead of Repeaters. It makes sense TBH, but I wonder if the code is structured in a way that we can reuse it for "repeaters" ACL. I'll report back once I have time to investigate :-) |
i think this would be a very appropriate use case for ACL 4. In addition to region modification the clock trust is there. Maybe we should come up with a good name like "Trusted Third Party" that isn't as specific as "Region Manager" |
|
I'm not sure of the particulars of ACL 4, but I personally would want this separate from repeater admin. I may trust some random repeater with GPS enough to set the time on my own repeaters, but not enough to be an admin on my repeater. I don't know if repeaters themselves could do something malicious but (very unlikely) a malicious repeater owner could gain people's trust then take their repeater's private key and use it on a companion device from which they could do something malicious to repeaters that trust that key. I was actually about to submit a PR very similar to this and I'm glad to see someone else had already been working on it. (I'll check Github better next time 🙂). A few things I had done in addition to this were:
I like your code better overall and mention these in case you find any of the ideas useful. Personally I like accounting for time per hop but I'm guessing most people won't use time sources more than a few hops away anyway. |
|
This would be a great addition that could be connected up with the pymc linux nodes. they could be the trusted clock and keep all other repeaters clocks on sync. |
If the repeater trusts it as a source. We don't want implicit trust. |
yeah i think any clock source should be in the acl somewhere above guest. that way it's explicit trust only. |
Also ACLs are designed to be "trust Client" from this Repeater, not for trusting other repeaters. Although what @Talie5in proposed was using a specific flag to only trust the clock, reusing the same list and extending it for this use case.
Yeah, that sounds like a reasonable default in line with what I have experienced, we could perhaps make it a configurable parameter.
I will add this to the patch and test ASAP. What I have not managed (I believe), is to get any feedback from core developers of meshcore. I'd like to waste a lot of time if this is going into an unwanted direction for them, do you know how to get them have an eye? |
I'm new to MeshCore so I don't know much about the actual project structure yet or how many developers are involved. |
|
@liamcottle I'd be grateful if you could have an eye to this PR, and offer some guidance. We are wondering if we should have a more generic trust mechanism, or if the ACL subsystem could be extended for this purpose. I am happy to finish the patch and implement the received feedback, but I'd love to know what is the preference of the project admins. Best, |
|
Interesting; I’ve implemented a clock sync before that worked by filtering out outliers from incoming packets to get an accurate time. That works until there are malicious actors, of course, but it was fine for some rogue nodes. I don’t think ACL is the right place for this though, since it’s about companion permissions, and personally I find @vaizki 's idea cleaner in the long run. That said, this approach is also fine for what it’s supposed to do. On the other hand… it does add quite a bit of code for something (accurate time) that repeaters don’t actually need to function in the first place. What’s the real use case here? and does that justify the extra code/complexity? |
If someone operates more than one node, especially if they lack an RTC or GNSS module keeping time in sync becomes a bit annoying at times. This applies to both companions and repeaters. Being able to trust certain operators and their timekeeping would make the management of low-cost nodes easier. |
|
Ok, it seems like I may need to rebase the patch at this point… |
9 participants
Implement repeater clock trust update (see #2416 "Clock towers").
Adds an opt-in "trusted clock" list of advert pubkeys whose timestamps
may step the local RTC. Replay-guarded (advert_ts must be strictly newer
than the last accepted from the same key.
Last few sync events are kept in a small RAM ring queryable via the CLI.
Logic lives in TrustedClockTable (no Arduino deps): table ops, replay
predicate, threshold predicate, sync-event ring, and pure
serialize/deserialize. MyMesh holds it as a member and wraps file I/O
around the helper.
CLI under clock.trust:
Also adds a new setting
clock.trust.thresh.To make reviewing easy I have structured the PR in two parts:
This is an example of the firmware running on a local T114 repeater I have at home:
advert from a trusted clock key received (it's another repeater with GPS clock sync):
clock gets updated: 10:54:35 - 15/5/2024 --> 08:31:18 - 8/5/2026