Gate the PKI-decrypt key on signer-proven, not the opportunistic cache#11116
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesPKI key resolution
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/mesh/Router.cpp (1)
627-631: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the explanatory comment.
As per coding guidelines, code comments should be kept to a maximum of one or two lines, avoiding multi-paragraph explanatory blocks.
♻️ Proposed refactor
- // Resolve the sender's public key only for actual PKI-decrypt candidates. Use - // copyPublicKeyAuthoritative (hot store or warm tier only), not the opportunistic - // copyPublicKey: authenticated DM attribution (pki_encrypted, p->from) must rest on - // authoritative keys, never on the ephemeral TOFU TrafficManagement NodeInfo cache. A - // not-yet-committed key from an in-progress key-verification handshake is tried below. + // Use authoritative sender key for authenticated DM attribution. + // Pending keys from in-progress handshakes are tried below.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mesh/Router.cpp` around lines 627 - 631, Condense the explanatory comment above the sender public-key resolution logic to one or two lines, retaining only that PKI-decrypt attribution must use authoritative keys and not the opportunistic TOFU cache. Remove the detailed implementation and handshake explanation while leaving the code unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/mesh/Router.cpp`:
- Around line 627-631: Condense the explanatory comment above the sender
public-key resolution logic to one or two lines, retaining only that PKI-decrypt
attribution must use authoritative keys and not the opportunistic TOFU cache.
Remove the detailed implementation and handshake explanation while leaving the
code unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6081b298-0ab9-4f31-bb8b-f66d11fceef8
📒 Files selected for processing (1)
src/mesh/Router.cpp
d427030 to
c3da38f
Compare
Resolve the inbound sender key via NodeDB::copyPublicKeyForDecrypt: authoritative (hot/warm) as before, plus a TrafficManagement cold-tier cache key only when it is signer-proven. An unverified TOFU cache key no longer backs pki_encrypted attribution, while a previously-authenticated node evicted to the cache still resolves. Outbound encrypt-to keying is unchanged.
c3da38f to
60738f0
Compare
caveman99
changed the title
|
Switched to option 1 from the review: copyPublicKeyForDecrypt accepts a cold-tier cache key only when signer-proven, keeping the hot/warm TOFU baseline and the outbound opportunistic path unchanged. |
|
warm store doesn't differentiate manually verified & xeddsa properly. There's a piece of work to do to re-allocate the spurious protected flag, and then split it properly. The protected flag is used at migration, but it's unused after that. |
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (30)
Build artifacts expire on 2026-08-21. Updated for |
…horitative-key # Conflicts: # src/mesh/Router.cpp
2 participants
Follow-up to #11050. That PR made NodeDB::copyPublicKey fall through to the ephemeral TrafficManagement NodeInfo cache, and Router::perhapsDecode used it to resolve the sender key on the inbound PKI-decrypt path, so a DM decrypted with an unverified TOFU cache key was stamped pki_encrypted and attributed to p->from.
New NodeDB::copyPublicKeyForDecrypt resolves authoritative keys (hot/warm, TOFU baseline unchanged) and accepts a cold-tier cache key only when it is signer-proven. A previously-authenticated node evicted to the cache still resolves; an unverified TOFU cache entry no longer backs authenticated DM attribution. The outbound encrypt-to path keeps the opportunistic copyPublicKey lookup.
Summary by CodeRabbit