fix(eventstream): gate subscription on TrustChecker (PILOT-251)#24
Closed
matthew-pilot wants to merge 1 commit into
Closed
fix(eventstream): gate subscription on TrustChecker (PILOT-251)#24matthew-pilot wants to merge 1 commit into
matthew-pilot wants to merge 1 commit into
Conversation
Add an auth gate in handleConn: when the trustedagents plugin is registered (deps.Trust != nil), reject subscription attempts from peers whose node ID is NOT on the trusted-agents allowlist. When trustedagents is NOT loaded (deps.Trust == nil), fall back to the previous behavior (allow all) for backward compatibility. Changes: - service.go: broker stores optional TrustChecker, handleConn checks sub.conn.RemoteAddr().Node against trust.IsTrusted before addSub - zz_added_coverage_test.go: +2 tests (reject untrusted, accept trusted) - zz_more_test.go, zz_resilience_test.go: update newBroker call sites Closes PILOT-251
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Collaborator
Author
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.
What failed
Previously, any peer that completed L6 key exchange could subscribe to any eventstream topic by simply connecting and sending a subscribe event. There was no auth gate —
handleConnat service.go:236-244 would add the subscriber to the requested topic without checking whether the peer was allowed to read that topic.A malicious peer could subscribe to any topic to learn who publishes on it and, for non-encrypted topics, read the event content.
Why this fix
When the trustedagents plugin is registered (
deps.Trust != nil),handleConnnow checkssub.conn.RemoteAddr().Nodeagainst the trust allowlist before adding the subscriber. Untrusted peers are rejected with a WARN log.When trustedagents is not loaded (
deps.Trust == nil— e.g., operator did not configure trusted agents), the fallback is the previous behavior: allow all. This ensures backward compatibility for existing deployments that do not use the trust gate.Verification
go build ./...✅go vet ./...✅go test ./... -count=1✅ (0.22s, all tests pass)TestBroker_HandleConn_TrustGateRejectsUntrusted,TestBroker_HandleConn_TrustGateAcceptsTrustedScope
4 files, +156/-19 — medium tier.
Closes https://vulturelabs.atlassian.net/browse/PILOT-251