Skip to content

server: apply TCP-AO to peer sockets - #3584

Open
rastislavs wants to merge 1 commit into
osrg:masterfrom
rastislavs:tcp-ao-peer
Open

rastislavs wants to merge 1 commit into
osrg:masterfrom
rastislavs:tcp-ao-peer

Conversation

@rastislavs

Copy link
Copy Markdown
Contributor

Part of TCP-AO implementation (#3493) this PR actually applies configured TCP-AO keys to peer sockets and makes TCP-AO work end-to-end.

Dynamic peers are not yet supported, will be covered in a later PR.

@rastislavs rastislavs mentioned this pull request Sep 2, 2026
8 tasks
@rastislavs
rastislavs marked this pull request as ready for review September 2, 2026 07:34
Comment thread pkg/server/server.go Outdated
conn.Close()
return
}
if err := setTcpAoConnectionPreferredKey(conn, tcpAoKeys); err != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we use setTcpAoConnectionRNext() instead? We should not update
current. The peer's next segment moves it back anyway.
I think that we can remove setTcpAoConnectionPreferredKey().

Comment thread pkg/server/fsm.go Outdated
// preferred key is not set on the listener until an inbound connection is accepted and matched to the peer.
// - Active peers install all keys in net.Dialer.Control in connectLoop() and mark the preferred key during installation.
// - Async keychain update causes update of keys directly on the listening sockets and fsm conn sockets of the affected peers.
// This is handled on best effort; keychain update is not rolled back upon individual soket operation errors.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

socket?

Comment thread pkg/server/peer.go
@@ -93,7 +93,7 @@ func newDynamicPeer(g *oc.Global, neighborAddress string, pg *oc.PeerGroup, loc
return nil

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If oc.PeerGroup has AO configurations, newDynamicPeer should fail?

Comment thread pkg/server/server.go
return true
}
}
for _, peer := range s.neighborMap {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tcpAoKeyUsed() can return false for a key that is in use. It only looks at preferredSendID, but the peer chooses our Current_key through RNextKeyID, and it can change that at any time?

Doing this properly needs care about races and partial updates, so I think key deletion can be left to a later PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we kind of already discussed this here: #3493 (comment)

I implemented the discussed best-effort behavior: first perform config-level validation, then update the keychain as the source of truth, attempt to apply the deletion to live sockets, and let users inspect the peer state API to verify which MKTs remain installed.

If a socket deletion returns EBUSY, the removed key may remain installed on that connection until it closes. This remains visible through the peer state API, allowing the user to react—for example, by resetting the peer.

I renamed tcpAoKeyUsed() to tcpAoKeyConfigured() to clarify that this is a configuration check and does not attempt to determine the kernel’s current key.

Preventing all key deletion while any peer references the keychain would be too restrictive operationally IMO. A keychain can contain at most 256 unique send IDs, and expired / invalid keys should be removable without first detaching the keychain or tearing down every affected peer.

@fujita

fujita commented Sep 14, 2026

Copy link
Copy Markdown
Member

How about not supporting key deletion while a peer uses the keychain? There is no safe way to tell whether a key is in use. Current_key is chosen by the peer through RNextKeyID and can change at any time, so any check has a window.

Removing a whole keychain is a different story. tcpAoKeychainUsed() only reads our own state and the kernel never changes it. A keychain that no peer references has no keys on any socket, because deleteNeighbor() removed them.
That is why DeleteTcpAoKeychain() does not need to touch sockets. The same holds for a single key while no peer uses the keychain.

With that, the delete path in updateTcpAoKeychainSockets() is not needed at all, and neither is -EBUSY handling or a per peer replacement key. Adding keys to a used keychain still works, so key rotation still works.

The cost is that a key can only be removed after the peer stops using the keychain. That seems fine if key management lives outside gobgp.

Resolve each peer's configured TCP-AO keychain when the peer is added,
keep a reference on the FSM, and install or remove its keys on listening
and connected sockets.

Passive peers have all configured keys installed on matching
listening sockets before the peer starts; no key is selected
on the listener. Once an inbound connection is accepted
and matched to the peer, select the configured send key
as both CurrentKey and RNextKey before passing the connection to the FSM.
Active peers install all keys from net.Dialer.Control before connect
and mark the preferred key during installation, so the initial SYN uses it.

Attaching, removing, or changing a keychain always uses the existing
peer delete-and-add path and therefore restarts the session.
A send ID change within the same keychain is handled
live by updating RNext on the current connection.

Management operations serialize peer and keychain changes. Socket key material
is copied and cleared after use. Listener updates use their raw socket handles
directly; existing connection updates hold the FSM mutex while reading fsm.conn
and issuing the socket options, avoiding a select-then-use window in this path.

Keychain key additions and deletions update the in-memory keychain first,
then propagate to listeners and established connections.
Per-socket failures are logged rather than rolling back the request,
so propagation is best effort and can be partial. Users can use
state data retrieval to confirm keychain changes have sucesfully
propagated to all affected peers.
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.

2 participants