Add encryption support#84
Conversation
| - `storage_exhausted` (client) - client cannot persist a new pairing record and has no fallback policy | ||
| - `user_cancelled` (client) - operator aborted the pairing through a local UI | ||
|
|
||
| ## Management |
There was a problem hiding this comment.
The management section adds 13 commands plus result handling and persistence. The rest of the spec is optimized to keep client implementations as minimal as possible (requires the server to support everything, opt-in roles, player chooses what audio format it can support).
Only set-name is something a normal user would ever touch. Most of the rest are admin/installer use cases I'm struggling to picture an end-user reaching for. I mean things like list-records, add-record, remove-record, list-pairing-psks are really advanced for an average users.
There was a problem hiding this comment.
Users wouldn't necessarily be exposed to those raw commands directly, they'd be used by servers in the background. There are a number of use cases, for example:
- Disabling PINs (Matter does this automatically after first successful pairing, we could do the same).
- Unpairing servers that are no longer in use.
- Sharing a client with another server (e.g. by adding a Pairing PSK to the client and passing it along).
The general idea is to keep the protocol simple and flexible, providing straightforward mechanisms and leaving policy decisions like the above to the servers.
There was a problem hiding this comment.
Adding 13 commands is anything but keeping the protocol simple.
The 4 commands for just the Pairing PSK need to be implemented by every client. If we want to keep the generate-PSK workflow, I think even a single management/refresh-pairing-token should be enough. What's the use case of having multiple long-lived pairing tokens and being able to revoke them individually?
Same goes for the records family of commands. Do we need all 4 of them? management/add-record is redundant with the existing pairing mechanisms.
We could maybe even reduce it to one or two commands (a management/release-ownership and maybe a management/unpair-other-servers), but that looks a bit less flexible to me.
Do we need management/clear-pin-lockout? Would a user know to search for this in their main server's settings? I for one wouldn't look there for such an option, maybe it's best to just drop it then? Restarting the speaker or using PSKs is still an option to get the speaker paired.
There was a problem hiding this comment.
The number management messages is my biggest hang up at the moment, this is a significant amount of new messages/complexity to the protocol. I think any reduction we can do here is very beneficial.
There was a problem hiding this comment.
I think the message count is fine now.
We could technically remove management/add-record since the same could be achieved without it (by temporarily enabling pairing and sharing the PSK), but keeping it is also fine by me.
| - `storage_exhausted` (client) - client cannot persist a new pairing record and has no fallback policy | ||
| - `user_cancelled` (client) - operator aborted the pairing through a local UI | ||
|
|
||
| ## Management |
There was a problem hiding this comment.
The number management messages is my biggest hang up at the moment, this is a significant amount of new messages/complexity to the protocol. I think any reduction we can do here is very beneficial.
|
|
||
| Clients with a usable out-channel (display, speaker, etc.) SHOULD implement `dynamic_pin` rather than `static_pin`. `static_pin` is intended only for devices that genuinely cannot emit a per-session value. | ||
|
|
||
| ### Unpaired Playback |
There was a problem hiding this comment.
Unpaired playback sounds a bit like it's exclusive to just players, but thats not exactly the case.
I think we can still call it unpaired_playback since its consistent with the group state, but maybe add a sentence here clarifying that it's not only for players.
| Only after receiving the initial `server/activate` should the client send any other messages (including [`client/time`](#client--server-clienttime) and the initial [`client/state`](#client--server-clientstate) message if the client has roles that require state updates). | ||
|
|
||
| ### Server → Client: `server/hello` | ||
| - `activities`: ('playback' | 'pairing' | 'management')[] - the set of currently-active purposes on this connection. May be empty. Members are unordered and unique. |
There was a problem hiding this comment.
We should require the server to keep this minimal, otherwise a server can always include playback (or management), and essentially "break" multi server support by stealing the connection.
| `record_mode` is an object: `{ kind, psk_id? }`. | ||
|
|
||
| - `kind: 'individual'`: a new stored-pubkey record is created with a freshly generated per-server PSK (32 bytes from a CSPRNG). On storage exhaustion: | ||
| - **without `psk_id`**: the pairing fails with [`pair/abort`](#client--server-pairabort) reason `storage_exhausted`. |
There was a problem hiding this comment.
Should we specify a minimum storage requirement for records? Without one, a client could ship with only one or two slots and users would hit storage exhaustion much earlier than expected. Hard to write good UX for "you must free a slot" without knowing how many slots exist.
| **Note:** Each role version may have its own support object (e.g., `player@v1_support`, `player@v2_support`). Application-specific roles or role versions follow the same pattern (e.g., `_myapp_display@v1_support`, `player@_experimental_support`). | ||
|
|
||
| ### Client → Server: `client/time` | ||
| ### Server → Client: `server/activate` |
There was a problem hiding this comment.
What happens to streams when activities transition? Two cases:
['playback']→['playback', 'management']: playback stays in the set, streams continue.['playback', 'management']→['management'](or[]): playback leaves the set, but management sessions don't have streams. So streams have to stop somehow.
Should we require the server to send stream/end for active streams before sending a server/activate that drops 'playback'? Otherwise the client has to handle this edge case itself.
| - **Per-method failure counter.** The client maintains a failure counter for each PIN-pairing method family (`static_pin` and `dynamic_pin` tracked independently). The counter is persisted across reboots. It is not partitioned by `server_id` or source IP: a single per-method counter for the device. | ||
| - **Increment.** The counter for a method increments on each inner-authentication failure observed in that method's flow. | ||
| - **Reset.** The counter for a method resets to zero on a successful pairing finalization for that method. | ||
| - **Terminal lockout.** When a method's counter reaches **10**, the method enters a **terminal lockout** state: the client refuses all pairing attempts for that method indefinitely. Exit requires a deliberate, local operator action (manufacturer-defined), or writing `locked_out: false` for the method via [`management/set-pairing-config`](#server--client-managementset-pairing-config) from an `owner`-trust server; on successful exit the counter resets to zero. The client SHOULD surface the lockout to the operator via a device-local mechanism (LED, on-screen indicator, audible cue). If a server initiates a pairing-mode connection during terminal lockout, the client sends [`pair/abort`](#client--server-pairabort) with reason `locked_out` and closes. |
There was a problem hiding this comment.
This seams a bit dangerous if the client doesn't have an owner to reset the lockout (or the user doesn't know where to find this option).
Also not sure if this should be signaled by an LED or audible cue, especially if Sendspin isn't the only protocol implemented on the device. There will potentially be an always glowing red LED on the device until you do a factory reset.
Can we make this silent and reset on reboot? The default action is to restart hardware if something stops working.
I know that a local action must be provided by the client/device, but having to do a full reset is a pretty bad experience. And requiring manufacturers to create a Sendspin-only reset shortcut is also difficult.
Add encryption support