tailcat: add Server.AllowClient hook for admitting clients - #120
Open
jormenjanssen wants to merge 1 commit into
Open
jormenjanssen wants to merge 1 commit into
jormenjanssen wants to merge 1 commit into
Conversation
Server.AllowedClients only admits a fixed list of node keys, so a program that decides at runtime which clients may connect, from a database or a directory service say, has to know every key before Start or keep the list current itself with AddAllowedClient, and can never turn a key down once it is on the list. Add Server.AllowClient, a func(key.NodePublic) bool in the shape of AllowProxy, consulted when a client that is not already connected and not in AllowedClients announces itself. A listed key is admitted without the hook; any other key is admitted if the hook approves it; with neither configured the server stays open to all clients, as before. A rejected client is ignored like an unlisted one and asked about again when it retries its meow. The hook runs under the backend's mutex, which peerByIP and peerForIP on the data path also take, so it must return quickly and not do I/O. That keeps this change small. A blocking-capable variant would need the check moved out of the lock, a context canceled at Close, and deduplication of the client's once-a-second meow retries while a decision is pending; a program gating clients on another service can instead decide ahead of time and have the hook look the decision up. onMeow now recognizes an already connected client before the admission check rather than after it, so a connected client's later pings never reach the hook. For the list this changes nothing, since a key can't be removed from it. Updates tailscale#119 Signed-off-by: Jormen Janssen <j.janssen@riwo.eu>
jormenjanssen
force-pushed
the
jormen/allow-client-callback
branch
from
September 16, 2026 12:29
45a0a2b to
a95bb65
Compare
bradfitz
reviewed
Sep 16, 2026
Comment on lines
449
to
+470
| // AllowedClients, if non-empty, restricts which client node keys | ||
| // may connect; all others are silently ignored. If empty, all | ||
| // clients are allowed. See [Server.AddAllowedClient] to add more | ||
| // at runtime. | ||
| // at runtime, and AllowClient to decide per client instead of | ||
| // from a fixed list. | ||
| AllowedClients []key.NodePublic | ||
|
|
||
| // AllowClient, if non-nil, reports whether the client with node | ||
| // key k may connect. It is consulted when a client that is not | ||
| // already connected and not in AllowedClients announces itself; | ||
| // a client it rejects is silently ignored, like an unlisted one, | ||
| // and is asked about again if it retries. If AllowClient is nil | ||
| // and AllowedClients is empty, all clients are allowed. | ||
| // | ||
| // It is called with the server's internal lock held, so it must | ||
| // return quickly and must not block on I/O: a slow call stalls | ||
| // every connected client's traffic. A program gating clients on | ||
| // another service should decide ahead of time and have AllowClient | ||
| // look the decision up. | ||
| // | ||
| // It must be set before calling Start. | ||
| AllowClient func(k key.NodePublic) bool |
Member
There was a problem hiding this comment.
I think we need to tighten the docs on the precedence here about when we fail open.
Actually, maybe we should just ditch the slice and only use the func? We can provide a tiny little adapter for people migrating from the slice style (probably very few people)
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
2 participants
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.
Adds
Server.AllowClient, afunc(key.NodePublic) boolhook in the shape ofAllowProxy, for programs embedding tailcat that decide at runtime which clients may connect and can't list every node key inAllowedClientsup front.Semantics: a key in
AllowedClientsis admitted without consulting the hook, any other key is admitted if the hook approves it, and with neither configured the server stays open to all clients as before. Rejected clients are ignored the same way unlisted ones are, and are asked about again when they retry their meow.onMeownow recognizes an already connected client before the admission check, so a connected client's later pings never reach the hook.Design choice for review. The hook runs under the backend's mutex, which
peerByIPandpeerForIPalso take, so it is documented as non-blocking: return quickly, no I/O, cache decisions from an external service for the hook to look up. That kept the change small. The alternative is a blocking-capablefunc(ctx context.Context, k key.NodePublic) bool, which needs the check moved out of the lock, a ctx canceled atServer.Close, and deduplication of the client's once-a-second meow retries while a decision is pending (the client's 10 second ping timeout bounds it either way). Happy to rework to that variant if you'd rather have it; say which and I'll update this PR.Tests:
TestAllowClientcovers a listed key admitted without the hook, an approved key admitted with exactly one hook call across two pings, and a rejected key that gets no meowed and does reach the hook.TestAllowClientOnlychecks that a hook without anyAllowedClientsstill closes the server to rejected clients. Both run against the local DERP and STUN fromtstest/integrationlike the other server tests.go test ./...,go vet ./...andmake tidyare clean on Windows; the CI matrix covers the rest.No CLI change;
--allowis untouched.Updates #119