Skip to content

gorums: authenticate peer identities with WithPeerVerifier - #359

Open
meling wants to merge 9 commits into
masterfrom
feat/325/peer-identity
Open

meling wants to merge 9 commits into
masterfrom
feat/325/peer-identity

Conversation

@meling

@meling meling commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Closes #325.

Stacked on #361 (per-server options for NewLocalServers and the gorumstest.LocalServers change), which must merge first.

Summary

Servers configured with WithPeers trusted the node ID that a peer claimed in its stream metadata, and whoever answered at a peer's address.
Any client that could reach a server could pose as a configured peer.
Under WithStreamDedup, a forged claim for a lower ID also redirected the server's own calls to the attacker.

This PR lets a server authenticate its peers:

  • WithPeerVerifier(verify PeerVerifier) checks the identity of each peer configured by WithPeers on every stream between the two servers, in both directions.
    • Inbound: a stream that claims a peer's node ID is checked before it is attached to the peer's node. A failed check, or a claim for an ID outside the peer configuration, ends the stream with codes.Unauthenticated.
    • Outbound: a stream that the server opens to a peer is checked against the dialed node's ID before any request is sent on it.
  • TLSPeerVerifier(identities map[ID]string) binds each node ID to a SAN of the peer's mTLS certificate that the TLS handshake verified against a trusted CA.
    An identity with a URI scheme (for example spiffe://relab/node/2) matches only a URI SAN in the same canonical form; any other identity matches only a DNS SAN, without regard to case.
    Wildcard DNS SANs, IP SANs, the Common Name, and empty identities never match, and two node IDs with the same identity make it panic.
  • With the option on, ServerContext.SenderID is authenticated for every configured peer, with and without stream deduplication.
  • Without the option, behavior is unchanged.

Design and decisions: see the design comment on #325.

API Changes

  • New: PeerVerifier, WithPeerVerifier, TLSPeerVerifier.
  • Internal: conn.NewInboundManager takes the verifier; stream.OutboundOptions gains Verify.

Documentation

  • User guide: a new "Authenticating Peers" section (both checks, what a refused peer sees, the SenderID guarantee, client authentication with TLS settings and interceptors, and certificate and CA requirements for TLSPeerVerifier); "Finding the Sender of a Request" is updated.
  • Developer guide: the checks in the stream and connectivity layers and in the inbound request path.

Known Limitation

A peer that a server refuses resends its pending calls on new streams until their contexts end, so those calls fail with the context's error rather than codes.Unauthenticated, and the retries are not paced.
This retry policy predates this PR; the PR documents it, and a fix follows in a stacked PR.

Testing

  • Unit tests per layer: the dial-side check in internal/stream, the inbound check and dial-side wiring in internal/conn, TLSPeerVerifier matching rules, construction errors, and the wiring in both directions with and without stream deduplication.
  • End-to-end tests in internal/tests/tls with a test CA issuing per-replica certificates: peers connect and handlers see the right SenderID; a server presenting another node's certificate is refused by both checks; an impostor claiming a lower peer's ID cannot take over its stream; clients without certificates are still served.
  • make test and make integrationtest pass; the touched packages pass under -race; make modernize changes nothing and make goplscheck is clean.

Copilot AI balanced review requested due to automatic review settings October 11, 2026 18:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@meling

meling commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

I have only used 78% of my AI credits!! Please learn to compute!

@meling
meling force-pushed the feat/325/peer-identity branch from f8677d9 to 4cc9887 Compare October 11, 2026 20:40
@meling
meling changed the base branch from master to feat/325/local-server-options October 11, 2026 20:40
@meling
meling added this pull request to stack #362 October 11, 2026 20:40
@meling
meling force-pushed the feat/325/peer-identity branch from 4cc9887 to 38665fe Compare October 12, 2026 20:10
Base automatically changed from feat/325/local-server-options to master October 12, 2026 20:21
An outbound channel trusted whichever server answered at the dialed
address. Add PeerVerifier and OutboundOptions.Verify: when set, the
channel checks the remote end of each new stream against its node ID,
using the stream's context, before it sends requests on the stream. A
failed check closes the stream and fails the waiting request with
codes.Unauthenticated; the channel then retries as for any failed
stream.
A server attached an inbound stream to peer X whenever the stream's
metadata claimed X, so any client could pose as a configured peer.
NewInboundManager now takes an optional PeerVerifier. With it,
AcceptPeer attaches a stream that claims a peer's node ID only after
the verifier accepts the claim, and refuses claims for unknown node
IDs, both with codes.Unauthenticated. The verifier runs without the
manager's lock. The outbound manager hands the same verifier to the
channels of the configured peers that the server dials.
Servers trusted the node ID that a peer claimed when it connected and
whoever answered at a peer's address, so ServerContext.SenderID was not
authenticated. WithPeerVerifier installs a PeerVerifier that checks
each configured peer on every stream between the two servers, in both
directions, before the stream carries calls. NewServer panics for a
nil verifier and for a verifier without WithPeers. The user guide gains
an Authenticating Peers section.
Add a PeerVerifier that binds each node ID to a SAN of the peer's
verified mTLS certificate. It reads the certificate only from the
verified chains, so a certificate that no trusted CA signed never
passes. An identity with a URI scheme matches only a URI SAN with the
same canonical form; any other identity matches only a DNS SAN,
without regard to case. Wildcard DNS SANs, IP SANs, the Common Name,
and empty identities never match. The user guide shows the
certificate and TLS configuration that replicas need.
Run peer groups whose servers check each other with TLSPeerVerifier
and certificates from a test CA, with and without stream deduplication.
The tests cover peers that connect, a server that presents another
node's certificate (refused by the dial-side and the inbound check),
an impostor that claims a lower peer's ID and must not take over its
stream, and a client without a certificate that is still served.
A verifier on a dialing node runs on the channel's goroutine, so one
that blocks delays the channel's sends and its Close. State that it
should return promptly and when its context ends, in the stream
package and in the root PeerVerifier alias.

State the invariant behind calling the stream's Context before the
check: it commits the stream to its transport, so requests reach the
checked peer, and turns off gRPC's transparent retry for the stream.

Add a test that pins re-verification of the replacement stream after
the server kills the first one.
If two node IDs have the same identity, one certificate matches both,
so either replica could claim the other's ID. TLSPeerVerifier now
panics at construction in that case. Two identities are the same when
one SAN matches both: equal canonical URIs, or DNS names that are
equal without regard to case. IDs are compared in increasing order so
the panic message names the same pair on every run.
A refused peer does not see codes.Unauthenticated on its calls: it
resends them on new streams until their contexts end, and the refusal
with the verifier's text appears in Node.LastErr. State this in the
WithPeerVerifier docs and the user guide, with the need for deadlines
and the brief effect on ConnectedPeers and WaitForAll. Add a subtest
that pins the refusal text in the refused peer's Node.LastErr.

State that TLSPeerVerifier is only as strong as the CA pools, and
which TLS settings verify no chain. Tabulate which SenderID values are
authenticated, state that an unknown claimed ID is refused under a
verifier, and show an interceptor check with Config.Contains, since a
nonzero SenderID also covers unauthenticated back-channel clients.
* Make the cross-type duplicate test case able to fail by using a
  DNS identity that strings.EqualFold equates with a URI identity.
* Document that TLSPeerVerifier panics on duplicate identities.
* Fix wording: rewrap the SenderID comment, say that only two-way
  calls are resent, say "every peer stream is refused", and split a
  paragraph in the user guide.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: stronger peer identity guarantees

2 participants