Remove the management namespace - #183
Conversation
ffc0a70 to
e244ee7
Compare
70b873e to
1eafad3
Compare
1eafad3 to
8cd329d
Compare
maximmaxim345
left a comment
There was a problem hiding this comment.
Previously runtime changes to the pairing config went through management, which needed the admitted connection, so the server that changed the state was also the one that knew the fresh state, and every change had a defined result.
Now local actions can change it at any time, but client/hello is only sent on a new connection and the client has no way to announce a change: a server that got pairing_required never learns that unpaired access was enabled later.
I think we should solve this at least somehow. For connected servers the simplest solution is to require the client to send a client/goodbye with restart after such a change, but I'm not sure if that is the best solution. If a server was previously disconnected through pairing_required, it has no connection left to receive a goodbye...
Also, @arturpragacz should also carefully review this PR before this is merged.
arturpragacz
left a comment
There was a problem hiding this comment.
This should be addressed first: #182 (comment)
Removes the already-paired entry into code-based pairing: a code-based pairing now runs over a Sentinel-keyed connection only, and the long-term PSK no longer admits the `'pairing'` activity. A server has no need to run a pairing round over an existing credential: to establish a fresh one it unpairs (`server/unpair`) and pairs again. The Pairing PSK flow is unaffected - it still enters from any open connection by re-handshaking to the pairing PSK first, since that is an operator-token-driven fresh pairing. Device-presence verification rode exclusively on this entry and is removed with it. If demand appears, it can come back as its own feature. Split out of #183 (see [the pairing.md discussion](#183 (comment))) so it can be discussed on its own. Overlaps textually with #183; whichever merges second needs a rebase. Co-authored-by: Paulus Schoutsen <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]>
Manufacturers will not accept remote server control over client records and pairing configuration. Replace the management commands with a fixed record model: clients store at least 5 pairing records and evict an implementation-chosen record (never one backing an open connection) when a pairing completes at capacity. An evicted server recovers through the Sentinel fallback. Shared-PSK records, record_mode, storage accounting, and the used flag are removed with the namespace. Secret rotation, method enable/disable, the unpaired_access toggle, and pairing-window opening are local manufacturer-defined actions now. Co-Authored-By: Claude Fable 5 <[email protected]>
Without remote configuration, a method shipped disabled with no pairing code provisioned can never be enabled. Static pairing code is for devices without an out-channel. Co-Authored-By: Claude Fable 5 <[email protected]>
The manufacturer-defined action already defines the full behavior. Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Both rules are stated at their point of use: supported_pair_methods omission in messaging.md, candidate-set exclusion in connection.md. Co-Authored-By: Claude Fable 5 <[email protected]>
Every PSK is generated locally from a CSPRNG, so cross-category uniqueness holds by construction. Co-Authored-By: Claude Fable 5 <[email protected]>
Uniqueness holds by construction (all PSKs are CSPRNG-drawn or the Sentinel constant), and category-scoped matching already makes a cross-category psk_id a lookup miss. Co-Authored-By: Claude Fable 5 <[email protected]>
The pairing PSK cannot be set to a chosen value, so a removed record's PSK can never become the pairing PSK. Co-Authored-By: Claude Fable 5 <[email protected]>
supported_pair_methods already says what the client currently offers; how a client manages its own pairing configuration over time is not the spec's concern. Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
How a client manages its own pairing secrets over time is not the spec's concern. The stability semantics and the per-device factory-provisioning requirement stay. Co-Authored-By: Claude Fable 5 <[email protected]>
Remove restatements and stranded remnants: the static-keys-plus-PSK note left over from the deleted shared-PSK model, the stale Sentinel gloss, the record-removal inventory sentence, the duplicate category-mismatch and server-timeout statements, and the token-decoding dispatch sentence. Give the code-based starting-state bullets a lead-in. Fix the worked unauthorized example to a combination a server can actually send. Co-Authored-By: Claude Fable 5 <[email protected]>
With management removed, pairing methods cannot be disabled and the pairing PSK is always among the handshake candidates, so a client that offers no pairing method cannot exist: locked_down was unreachable. Remove the Locked-Down Clients section and the goodbye reason. Co-Authored-By: Claude Fable 5 <[email protected]>
- Make record persistence and replacement an explicit MUST (covers a reinstalled server pairing fresh while the client still holds its old record) - Protect records backing provisional connections from eviction and cap open paired connections below record capacity, so an evictable record always exists - Restore the specific-version rejection rule in pairing-token decoding - Remove the already-paired pairing entry: code-based pairing runs over Sentinel-keyed connections only, the long-term PSK no longer admits ['pairing'], and device-presence verification goes with it - Stop implying the credential-mismatch rule points at an exhaustive record inventory Co-Authored-By: Claude Fable 5 <[email protected]>
Its removal moves to a separate PR so it can be discussed on its own. Co-Authored-By: Claude Fable 5 <[email protected]>
A client that stops admitting unpaired access closes any connection relying on it with client/goodbye reason pairing_required. Co-Authored-By: Claude Fable 5 <[email protected]>
The keep-the-entry revert predates Sendspin#192; replaying it over a main that merged Sendspin#192 resurrected the removed text. Co-Authored-By: Claude Fable 5 <[email protected]>
2a2b9c2 to
e6f7f39
Compare
Co-Authored-By: Claude Fable 5.1 <[email protected]>
|
Just for the future record: I personally am not in favour of this change. |
Fixes #184. Fixes #186. Two changes to pairing.md's Methods section: - Clients may implement at most one pairing-code method: static or dynamic, not both (Pairing PSK stays mandatory). No wire change - `supported_pair_methods` already carries whatever the client offers; this constrains what may appear there. - The steering paragraph (SHOULD implement `dynamic_pairing_code` given an out-channel, SHOULD offer `qr_code` given a display) is dropped. Method choice is the client's decision; the method list already describes the trade-offs (out-channel requirement, static-code MITM caveat). Note: overlaps textually with #183, which edits the same paragraph - whichever lands second needs a trivial rebase. --------- Co-authored-by: Paulus Schoutsen <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]> Co-authored-by: Maxim Raznatovski <[email protected]>
Fixes #182.
Removes the entire
management/*namespace and replaces it with a fixed pairing-record model.Changes
management.mddeleted: allmanagement/*commands,management/result, storage accounting,record_mode, and theusedflag are gone.pairing.md: new Pairing Records section. A record is the long-term PSK bound to aserver_id; re-pairing replaces a server's own record. Clients MUST store at least 5 records. A pairing that completes at capacity MUST succeed by evicting an existing record; the choice is implementation-defined (for example, least recently used), except the record backing a currently-open connection MUST NOT be evicted. Evicted servers recover through the existing Sentinel fallback. Method enable/disable, secret rotation, theunpaired_accesstoggle, and pairing-window opening are local manufacturer-defined actions now.messaging.md:activitiesis now('playback' | 'pairing')[]; the long-term-PSK allowed sets are[],['playback'],['pairing'].server/unpairloses its shared-record carve-out. Themanagement/prefix and the management cases in theunauthorizedandconcurrent_attemptgoodbye reasons are removed.connection.md: connection priority is now'playback'>'pairing'. The shared-PSK storage model is removed; every record storesserver_idand gets the post-match check.psk_iduniqueness is enforced at generation time. Eviction is added to the Sentinel-fallback causes.README.mdregenerated;build-readme.py --checkpasses.Follow-up
The implementations (aiosendspin, sendspin-cpp) and the conformance suite still carry management support and need matching removals once this lands.