feat: enforce per-device encryption keys - #96
Closed
quechau wants to merge 3 commits into
Closed
Conversation
Introduce a package that owns one decision: which AES key Rubix CE uses for a LoRa device, and what that implies for how strictly its frames are checked. Keys are produced outside this system - at manufacturing, or by whoever commissions the device - and typed into both ends: the device (AT+AES= on STM32, param_set 0x0220 on ESP32) and the "Device Key" box in Rubix CE. CE only needs to know the key and enforce it, so there is no key generation and no provisioning state machine here. Key selection is byte-identical to the previous getEncryptionKey: Device.Manufacture when set, otherwise the module default key. Three behaviours worth calling out: 1. Pasting the shared key into the Device Key box is reported as SHARED_KEY, not PER_DEVICE_KEY. It looks like provisioning but protects nothing, and treating it as a real per-device key would also flip the device into encryption-required and break a device that was working fine. 2. Dash-separated hex is accepted. The ESP32 console prints keys as AA-BB-CC-..., and an operator copying from param_get 0x0220 should not have to strip separators by hand. 3. An unparseable mode meta tag falls back to the inferred mode rather than failing, so corrupt storage can never silently downgrade a device. Key material carries a redacting String() and a Zero() wipe so it cannot be logged by accident. Tests: 17 cases covering selection, error paths, format variants, the shared-key-paste trap, explicit UNENCRYPTED opt-in and key hygiene.
…wn key A device address travels in cleartext at the start of every LoRaRAW frame. Before this change, a device with its own key still accepted a well-formed PLAINTEXT frame whenever its model allowed plaintext in general (Rubix, ZipHydroTap). Anyone who overheard a single frame learned the address and could then bypass encryption entirely. Reject plaintext for any device whose mode is PER_DEVICE_KEY, whatever the model allows. A factory-reset device lands in the same branch - it is back on the shared key while CE still expects its own - so it now fails closed with a message telling the operator to re-enter the key on both sides, rather than being silently downgraded. Route key selection through keymgmt.Resolver so key handling lives in one place, and add a diagnostic that identifies the device's key by elimination when a frame fails to decrypt. The device's key is never on the wire (a frame carries only ciphertext and a CMAC), so CE probes the keys it knows and reports one of three conclusions: the device is on the shared key, it is on another device's key, or it holds a key CE has never seen. Each conclusion names the fix. The probe is a diagnostic, not part of the data path: it is guarded against a nil marshaller and nil config so it can never panic or fail a frame. A unit test caught exactly that panic before it shipped. Tests: 7 downgrade cases. Three of them guard against regression - shared-key devices keep accepting plaintext, an explicit UNENCRYPTED opt-in still works, and pasting the shared key does not flip a device into encryption-required.
Per-device keying only contains a compromise if the keys are actually distinct. Until now nothing stopped the same key being entered on several devices, which quietly gives up that property while still displaying as PER_DEVICE_KEY. It is easy to do by accident: pasting one value into several Device Key boxes, or copying an example out of documentation. Both happened during bench testing. Check at the point of entry - device create and update - and reject with the address of the device already holding that key, rather than letting the operator discover it later from dropped frames. Comparison normalises case, dashes and spaces, so the same key typed in STM32 form (plain hex) and ESP32 form (dash-separated) is recognised as one key. An empty key is never a duplicate: it means "use the shared key", which many devices legitimately do. If the device lookup itself fails, configuration is allowed to proceed - a transient lookup error should not block commissioning, and the RX-path diagnostics still surface reuse if it slips through. Tests: 6 cases including self-edit, format variants and records that cannot be named as an owner.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes per-device AES keys actually protect the device they belong to. Keys are produced outside this system - at manufacturing, or by whoever commissions the device - and typed into both ends: the device (
AT+AES=on STM32,param_set 0x0220on ESP32) and the "Device Key" field in Rubix CE.The key-selection mechanism has existed since
26597487. This PR closes the gaps around it.Operator-facing procedure is in
Per-Key-Loraraw.md(#95).The security problem this fixes
A device address travels in cleartext at the start of every LoRaRAW frame - it has to, because the receiver must know which device sent a frame before it can pick a key.
Before this change, a device with its own key still accepted a well-formed plaintext frame whenever its model allowed plaintext in general (
Rubix,ZipHydroTap). So anyone who overheard a single frame learned the address, and could then forge plaintext for that address and have it accepted - bypassing encryption entirely.A test written against the current
masterdemonstrates this:What changed
1. Plaintext is rejected for a device that has its own key, whatever its model allows in general. A factory-reset device lands in the same branch - it is back on the shared key while CE still expects its own - so it now fails closed with a message pointing at re-entering the key, rather than being silently downgraded.
2. Pasting the shared key into the Device Key box is reported as
SHARED_KEY, notPER_DEVICE_KEY. It looks like provisioning but protects nothing. Treating it as a real per-device key would also flip the device into encryption-required and break a device that was working fine. This happened during bench testing.3. A Device Key already used by another device is refused at the point of entry, naming the device that holds it. Per-device keying only contains a compromise if the keys are actually distinct; nothing previously stopped the same value being pasted into several devices. This also happened during bench testing.
4. Dash-separated keys are accepted. The ESP32 console prints keys as
AA-BB-CC-..., and an operator copying fromparam_get 0x0220should not have to strip separators by hand.5. When a frame fails to decrypt, the log identifies the device's key by elimination. The key is never on the wire - a frame carries only ciphertext and a CMAC - so CE probes the keys it knows and reports one of three conclusions: the device is on the shared key, it is on another device's key, or it holds a key CE has never seen. Each names the fix.
Compatibility
Devices on the shared key are unaffected - three of the seven downgrade tests exist specifically to pin that down. Verified on a live board with a mixed fleet: devices with their own key and devices on the shared key running side by side, no false rejections, no panics.
Notes for review
logKeyMismatchprints raw key material at ERROR level. It is commissioning tooling and is taggedUNSAFE-DEBUG; grep that string to find every such site. Worth deciding whether it ships as-is or goes behind a flag.Tests
56 pass, 0 fail. 17 cases in
keymgmt, 7 covering the downgrade guard, 6 covering duplicate-key detection, the rest pre-existing.gofmtclean,go vetclean.