Skip to content

tailcat: replace AllowedClients with an AllowClient hook, add KeySet and DisconnectClient - #134

Merged
bradfitz merged 1 commit into
mainfrom
bradfitz/allowclient
Sep 26, 2026
Merged

bradfitz merged 1 commit into
mainfrom
bradfitz/allowclient

Conversation

@bradfitz

@bradfitz bradfitz commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Server.AllowedClients and Server.AddAllowedClient are replaced by a
single Server.AllowClient func(key.NodePublic) bool, asked about each
client as it connects. A nil hook allows all clients, which is the one
fail-open rule. The old slice's "empty means open" semantics forced
the CLI's --allow=none to insert a zero key as a sentinel; an empty
KeySet now simply allows nobody.

The hook runs with the backend mutex released, so unlike the hook
proposed in #120 it may block on an external lookup and may call other
Server methods. onMeow already runs in its own goroutine per meow
ping, so a slow answer delays only that client. A pendingAllow map
keeps a client's once-a-second meow retries from starting a second
call for the same key while one is in flight.

The new KeySet type is a mutex-guarded set of node keys whose Contains
method is the ready-made hook for a fixed or hand-maintained list, so
the runtime add that AddAllowedClient offered is still available, and
removal comes with it. The new Server.DisconnectClient drops a
connected client from the network map so its traffic is blackholed in
both directions; it does not reset its connections. Revoking a client
is then two orthogonal steps the caller composes: make the hook reject
the key, then disconnect it.

Client IDs now come from a counter that is never reused, since after a
removal len(clients)+2 could collide with a live peer.

Supersedes #120 and #125, which each added a second admission form
alongside the slice.

Fixes #119
Fixes #124

Co-authored-by: Jormen Janssen (@jormenjanssen)
Co-authored-by: Yann (@ybaelli)

…and DisconnectClient

Server.AllowedClients and Server.AddAllowedClient are replaced by a
single Server.AllowClient func(key.NodePublic) bool, asked about each
client as it connects. A nil hook allows all clients, which is the one
fail-open rule. The old slice's "empty means open" semantics forced
the CLI's --allow=none to insert a zero key as a sentinel; an empty
KeySet now simply allows nobody.

The hook runs with the backend mutex released, so unlike the hook
proposed in #120 it may block on an external lookup and may call other
Server methods. onMeow already runs in its own goroutine per meow
ping, so a slow answer delays only that client. A pendingAllow map
keeps a client's once-a-second meow retries from starting a second
call for the same key while one is in flight.

The new KeySet type is a mutex-guarded set of node keys whose Contains
method is the ready-made hook for a fixed or hand-maintained list, so
the runtime add that AddAllowedClient offered is still available, and
removal comes with it. The new Server.DisconnectClient drops a
connected client from the network map so its traffic is blackholed in
both directions; it does not reset its connections. Revoking a client
is then two orthogonal steps the caller composes: make the hook reject
the key, then disconnect it.

Client IDs now come from a counter that is never reused, since after a
removal len(clients)+2 could collide with a live peer.

Supersedes #120 and #125, which each added a second admission form
alongside the slice.

Fixes #119
Fixes #124

Co-authored-by: Jormen Janssen <j.janssen@riwo.eu>
Co-authored-by: Yann <y.baelli05@gmail.com>
@bradfitz

Copy link
Copy Markdown
Member Author

cc @jormenjanssen @ybaelli

@ybaelli

ybaelli commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

it solves my problem, so I'm completely fine with it, I appreciate the mention to my work aswell 😄

First open source contribution ever 🎉

@bradfitz

Copy link
Copy Markdown
Member Author

@ybaelli, congrats! :) Sorry I couldn't take your change more directly, but we had ~3 things coming in all in conflict with each other, so I had to kinda merge them together in some coherent form.

@ybaelli

ybaelli commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

No problem at all, I had a quick look, it's solving the problem I had, all good !

@bradfitz

Copy link
Copy Markdown
Member Author

@jormenjanssen also approves in #119 (comment)

@bradfitz
bradfitz merged commit 7d80847 into main Sep 26, 2026
9 checks passed
rsc pushed a commit to rsc/cmd that referenced this pull request Sep 26, 2026
Bump github.com/tailscale/tailcat to 7d80847ede15, which replaces
Server.AllowedClients with a Server.AllowClient hook and adds the
KeySet type:

tailscale/tailcat@7d80847ede15

mote now builds a KeySet from allowed.txt and installs its Contains
method as the hook when the file has any keys. With no keys the hook
stays nil, so the server still answers everyone, as before. The
"answering N allowed client keys" log line moves into startTailcat,
where the count is known, since the server no longer exposes the list.

Updates tailscale/tailcat#119
Updates tailscale/tailcat#134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants