Commit Graph

5 Commits (dependabot/github_actions/github/codeql-action-4.31.9)

Author SHA1 Message Date
Nick Khyl 2917ea8d0e ipn/ipnauth, safesocket: defer named pipe client's token retrieval until ipnserver needs it
An error returned by net.Listener.Accept() causes the owning http.Server to shut down.
With the deprecation of net.Error.Temporary(), there's no way for the http.Server to test
whether the returned error is temporary / retryable or not (see golang/go#66252).

Because of that, errors returned by (*safesocket.winIOPipeListener).Accept() cause the LocalAPI
server (aka ipnserver.Server) to shut down, and tailscaled process to exit.

While this might be acceptable in the case of non-recoverable errors, such as programmer errors,
we shouldn't shut down the entire tailscaled process for client- or connection-specific errors,
such as when we couldn't obtain the client's access token because the client attempts to connect
at the Anonymous impersonation level. Instead, the LocalAPI server should gracefully handle
these errors by denying access and returning a 401 Unauthorized to the client.

In tailscale/tscert#15, we fixed a known bug where Caddy and other apps using tscert would attempt
to connect at the Anonymous impersonation level and fail. However, we should also fix this on the tailscaled
side to prevent a potential DoS, where a local app could deliberately open the Tailscale LocalAPI named pipe
at the Anonymous impersonation level and cause tailscaled to exit.

In this PR, we defer token retrieval until (*WindowsClientConn).Token() is called and propagate the returned token
or error via ipnauth.GetConnIdentity() to ipnserver, which handles it the same way as other ipnauth-related errors.

Fixes #18212
Fixes tailscale/tscert#13

Signed-off-by: Nick Khyl <nickk@tailscale.com>
3 weeks ago
Brad Fitzpatrick 7c1d6e35a5 all: use Go 1.22 range-over-int
Updates #11058

Change-Id: I35e7ef9b90e83cac04ca93fd964ad00ed5b48430
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Andrew Lytvynov 2e956713de
safesocket: remove ConnectionStrategy (#10662)
This type seems to be a migration shim for TCP tailscaled sockets
(instead of unix/windows pipes). The `port` field was never set, so it
was effectively used as a string (`path` field).
Remove the whole type and simplify call sites to pass the socket path
directly to `safesocket.Connect`.

Updates #cleanup

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
2 years ago
Brad Fitzpatrick b4be4f089f safesocket: make clear which net.Conns are winio types
Follow-up to earlier #9049.

Updates #9049

Change-Id: I121fbd2468770233a23ab5ee3df42698ca1dabc2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
James Tucker f844791e15 safesocket: enable test to run on Windows unpriviliged
I manually tested that the code path that relaxes pipe permissions is
not executed when run with elevated priviliges, and the test also passes
in that case.

Updates #7876

Signed-off-by: James Tucker <jftucker@gmail.com>
3 years ago