Repository navigation
perf(core): remember peer-advert verdicts and imported verification keys - #358
Conversation
A hub re-broadcasts every advert it accepts to every other client and replays the directory to each sender, so one advert reaches a receiver many times and a mesh of N devices re-verifies each advert on the order of N times per receiver. Each check derived a device id, imported the public key and verified the signature, all from scratch. The verdict for a signature over given signed bytes is a pure function of those bytes, so verifyPeerAdvert now remembers it in a byte-budgeted least-recently-used memo held per verifier. Only a verdict the checks reached is kept, a refusal included; a check that threw is retried. The node identity adapter keeps the imported verification key per device so a first sight of a new advert does not re-import a known key.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsFinding details are still loading. Check the individual review comments. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex security review |
1 similar comment
|
@codex security review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 1702fa9182
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
The cache key was built from the signature and signed bytes before the budget was checked, so a peer sending a frame near the relay's size limit made the verifier copy it into a string of the same size before the signature was ever judged. The cache now takes the byte parts and works out the key's length from their lengths alone. An input whose key would exceed the budget is verified directly and never keyed.
|
@codex security review |
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: dd7fb569ea
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
The imported-key cache id is the key's bytes as a string, built before the key was checked, so a gossiped advert carrying a key near the frame limit was copied and encoded in full only to be refused by the import. A key of the wrong size for its algorithm can never import, so it is now refused before the id is built, along with an algorithm no verifier here implements.
|
@codex security review |
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 256e690abb
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
…dget An entry evicted while its verdict was pending left the map but not memory: the computation kept its key string until it settled. A peer splitting one large gossip frame into many adverts, each within the size limit, could hold a key for every one of them at once. Keys of unsettled verdicts are now charged to the budget, and an input that would overflow it is verified directly instead of being keyed.
|
@codex security review |
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
|
🎉 This PR is included in version 4.0.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
A hub re-broadcasts every advert it accepts to every other client and replays the directory to each sender, so with a few dozen devices on one machine each receiver verifies the same advert many times over. A profile of a busy hub process showed
verifyPeerAdvert(key import, two digests, signature check) and its CBOR encoding dominating its non-idle time; see ExaDev/agent-comms#434.The verdict for a given signature over given signed bytes cannot change, so
verifyPeerAdvertnow keeps it in a byte-budgeted LRU held per verifier object. A repeat of an advert already judged costs one encoding. A refusal is kept too; a check that threw is not, since the same bytes may verify on a retry. The node identity adapter also keeps the imported verification key per device, so a new advert from a known device does not re-import its key.The tests count checks run through a counting verifier rather than timing anything: the same advert re-sent repeatedly, a hub fed repeated gossip from several peers, and a session replayed the same directory, each checked once per distinct advert. With the memo and key cache disabled six of them fail.
Refs ExaDev/agent-comms#434