-
Notifications
You must be signed in to change notification settings - Fork 470
Cryptography overhaul for V2 #1450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Everybody loves numbers (SF8, BW62.5, CR8) (edit: corrected the math, initially I did not count the timestamp)Real Message ExamplesNote: Plaintext = message length + 4 bytes (timestamp prefix in TXT_MSG)
Overhead CalculationV1 (AES-ECB + HMAC):
V2 (ChaCha20-Poly1305):
When Each Wins
The crossover point: V2 wins when Security Comparison
Summary
|
|
Store now - decrypt later (post-quantum stuff) notes: To be safe:
|
|
Note on Ascon: Ascon requires a full 128-bit nonce - no compression like with ChaCha. Even with a truncated 64-bit tag, Ascon overhead is 24 bytes vs 12 bytes. (CPU stats, but they doesn't really matter, chacha is 10x faster than AES-ECB already) conclusion: chacha is better, more homogenous, battle-tested |
|
Some tests |
|
With the proper algo we can get >25% average payload compression, but due to the padding required, the air time savings are negligible with v1. Does this version still requires boundary padding ? V2 must include [at least] 2 byte ids to stop the collision problem. |
no padding needed and I've compressed everything I can in the crypto layer maintaining acceptable security. No LZW, I focused on sec enhancements only I've tried to be backwards compatible so it can roll out without real breaking changes, but if the 2byte is needed then the backwards compatibility can be scrubbed from the code the only requirement for this to work right now is for repeaters to forward payload v2. we are now dropping it in src/Mesh.cpp: |
force pushing repeater "first login chacha sensing" too. now repeaters will respond with chacha if chacha decrypt succeeded. All paths are secured. No downgrade possible between two V2-capable nodes. |
|
Unused byte at offset 287 in the contacts file format is used to retain ChaCha capability of a client (after reboot, etc) currently the channel sync is not compatible with the MC app, app does not ignore the trailing byte :) commented it out, need to think it over |
e65abb8 to
321e5ed
Compare
|
CMD_GET_CHANNEL --> Returns 50 bytes (no flags) - for backwards compatibility RESP_CODE_CHANNEL_INFO --> 50 bytes |
|
room server note: thought about data at rest encryption, but... private key is on the device itself so doesn't needed, physical access to a device always game over. protect your private keys guys :) |
|
Just use a plain counter as per the RFC7539, section 2.6. After a reboot, just regenerate a new key. 'Compress'ing stuff through a function to generate a nonce is not going to improve security, it actually might make it worse. ChaCha20 requires you guarantee uniqueness in your nonces for a given key. A plain counter provides this guarantee. On that note, I suspect you may be able to go down to a 16 bit nonce counter but you have to guarantee that you use a new key when it the counter wraps. The nonce is now just an indicator of when a key exchange must take place; you of course do the exchange pro-actively before you get close to the wrap around.
Dragons may lurk here, combining encryption and compression is a box of spiders. OpenSSH looks to have mitigated the issue by delaying compression until after authentication completes whilst TLS 1.3 just removed the footgun altogether. |
- Transmit only 4-byte counter [boot_id(2)][sequence(2)] - Derive full 12-byte nonce internally: SHA256(key || counter)[0:12] - Combined with tag reduction: V2 overhead now 12 bytes (was 24) - Saves ~16.5ms airtime per packet at SF8/BW62.5/CR8 - Security maintained: nonce unpredictable without shared secret
repeater first login chacha sensing contact marking persistence channel sync V2
|
started to rewrite a lot of thing after consulting with @nextgens |
src/Utils.cpp
Outdated
| // Counter format: [epoch (2 bytes random)] [sequence (2 bytes incrementing)] | ||
| // - epoch: random value, regenerated on boot or sequence wraparound | ||
| // - sequence: 0-65535, increments each message | ||
| // This guarantees uniqueness across devices (different epochs) and | ||
| // within long-running sessions (epoch changes on wraparound) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the random number?
Is it not possible to negotiate a new key?
If not, you could store the counter every N messages (say 100 or more) and after a reboot, read back the saved counter and increment it by N+X (where X>=1) or even 2*N?
Some decision will have to be made about what to do once you wrap the counter, though sending ~4bn messages to someone does seem unlikely :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's "raw" meaning I just started to transform the old chacha stuff :) thanks for pointing that out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
negotiation won't happen, airtime is a huge factor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...airtime once per 2^{16 or 32} packets is too much of an overhead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the random number?
The point is that you can't persist stuff reliably. What is proposed is a random boot_id (lifetime is the uptime or wrap-around) combined with a simple counter.
It doesn't depend on state keeping past uptime and the added benefit is that we can do stats on the boot_ids and identify any problem in the RNG at network's scale.
The counter is shared amongst all invocations: it's not per-peer/key
Is it not possible to negotiate a new key?
"negotiate" no, the aim of the PR is to make something backward compatible that keeps the existing packet format/structure. Fancier stuff (ECDH, ...) will require more work. What's proposed in this PR won't have forward secrecy.
That said the plan is to diversify keys (if only because existing keys are already reused elsewhere in the codebase) as that does not cost anything (we're not reusing key schedules) and solves the counter-wrapping problem.
My pet project outgrew itself, so I thought I share it here, maybe someone will have use for the information and stuff I learned along the way in the future.
MeshCore v2 Encryption: ChaCha20-Poly1305 Implementation
Requirements
This implementation requires repeaters to forward PAYLOAD_VER_2 packets! (currently in dev everything is dropped above VER_1)Refactored so it will use PAYLOAD_VER_1
Decisions
I decided to use ChaCha20-Poly1305 because we are using software for decrypt anyway (ESP's can do hardware accelerated stuff, but I tried to be universal, chacha is 10x faster than the current AES-ECB+HMAC anyway, it was designed for "low-end" systems like our embedded variants)
I decided to use 12 byte nonce and 12 byte tag because it's the lowest that adheres to standards. (Nonce can be lowered to 8 to save some minor airtime)
Draft PR, if anyone have anything to teach me more I'd be glad to hear it.
TL;DR
ChaCha20-Poly1305 authenticated encryption, replacing AES-128-ECB scheme for enhanced security.
The implementation maintains backward compatibility with v1 packets while providing stronger cryptographic guarantees for "v2-capable" nodes, as minimal airtime increase as I could squeeze out, no handshakes no nothing "additional" packets.
This was a huge project, I worked on it in "one section at a time" to preserve context windows and try to understand everything I did along the way.
I'm not a crypto expert nor a coding guru, so read everything with this in mind.
I've tested everything I could on my daily driver companion and my repeater (the code is still running on them, I'm using our mesh with this right now, of course v2 encryptes talk just between my repeater's admin commands and my other test companion).
And for the masochist people, the full changelog, I "enchanced" my scribbles with claude:
What Changed
Core Cryptographic Changes
New Encryption Algorithm: ChaCha20-Poly1305 AEAD cipher
Payload Version System
PAYLOAD_VER_1: Legacy AES-128-ECB (unchanged, remains default)PAYLOAD_VER_2: New ChaCha20-Poly1305 encryptionADV_FEAT1_CHACHA_CAPABLE)Channel Tagging
CHANNEL_FLAG_V2toGroupChannel.flagsbytePacket Types Migrated to v2
PAYLOAD_TYPE_REQ(request packets)PAYLOAD_TYPE_RESPONSE(response packets)PAYLOAD_TYPE_TXT_MSG(text messages)PAYLOAD_TYPE_GRP_TXT(group text messages)PAYLOAD_TYPE_GRP_DATA(group data messages)PAYLOAD_TYPE_PATH(path return packets)PAYLOAD_TYPE_ANON_REQ(anonymous request packets)Forwarding Logic Updated
PAYLOAD_VER_2packets (previously dropped)Security Hardening (Additional Fixes)
This PR also includes critical security hardening fixes identified during code audit:
Packet Parsing Hardening (
Packet::readFrom())uint8_ttouint16_tto prevent truncationACK Packet Parsing
payload_len >= 4validation before readingack_crcTRACE Packet Parsing
payload_len >= 9) before reading trace fieldsBridge Compatibility
RS232BridgeandESPNowBridgenow compatible withuint16_tlength parameterDesign Decisions in full
Why ChaCha20-Poly1305?
Why 12-Byte (96-bit) Authentication Tag?
UPDATE: 8 byte tag is enough
rweather/Cryptolibrary supports tag truncation viaCHACHA_TAG_SIZEWhy 12-Byte Nonce?
UPDATE: Compressing nonce to 4 byte via SHA derivation
rweather/Cryptolibrary expectationsUtils::getHardwareRandom()AAD (Additional Authenticated Data) Implementation
AAD protects packet metadata from tampering:
dest_hash,src_hash(orchannel_hash),payload_type,payload_versionHow It Works
Encryption Flow (v2)
peerSupportsCHACHA()orCHANNEL_FLAG_V2)Utils::getHardwareRandom()(platform-specific RNG)Utils::encryptCHACHA()which:[nonce (12)] [ciphertext] [tag (12)]PAYLOAD_VER_2in headerDecryption Flow (v2)
PAYLOAD_VER_2from packet headerUtils::decryptCHACHA()which:secure_compare())Capability Negotiation
ADV_FEAT1_CHACHA_CAPABLEflag inPAYLOAD_TYPE_ADVERTpacketsContactInfo.supports_chachaandClientInfo.supports_chachatrack peer capabilitiesAirtime Comparison (sorry if the table is shifted)
Packet Size Overhead
Airtime (BW62.5, SF8, CR8)
Security Improvements
What v2 Provides
secure_compare()to prevent timing attacksWhat v2 Doesn't Change
PAYLOAD_TYPE_ACKpackets remain unencrypted (by design, for low overhead)Backward Compatibility
- Old Firmware: Will drop v2 packets (expected)Resource Usage (CPU/RAM)
RAM Impact
CPU Impact
Known Limitations / Future Work
Breaking Changes
- Old Repeaters: Will dropPAYLOAD_VER_2packets