diff --git a/ipn/ipnauth/ipnauth_windows.go b/ipn/ipnauth/ipnauth_windows.go index 1138bc23d..e3ea448a8 100644 --- a/ipn/ipnauth/ipnauth_windows.go +++ b/ipn/ipnauth/ipnauth_windows.go @@ -25,6 +25,12 @@ func GetConnIdentity(logf logger.Logf, c net.Conn) (ci *ConnIdentity, err error) if !ok { return nil, fmt.Errorf("not a WindowsClientConn: %T", c) } + if err := wcc.CheckToken(); err != nil { + // Failure to obtain a token means the client cannot be authenticated. + // We don't care about the exact error, but it typically means the client + // attempted to connect at the Anonymous impersonation level. + return nil, fmt.Errorf("authentication failed: %w", err) + } ci.pid, err = wcc.ClientPID() if err != nil { return nil, err @@ -169,26 +175,13 @@ func (t *token) IsUID(uid ipn.WindowsUserID) bool { // WindowsToken returns the WindowsToken representing the security context // of the connection's client. func (ci *ConnIdentity) WindowsToken() (WindowsToken, error) { - var wcc *safesocket.WindowsClientConn - var ok bool - if wcc, ok = ci.conn.(*safesocket.WindowsClientConn); !ok { + wcc, ok := ci.conn.(*safesocket.WindowsClientConn) + if !ok { return nil, fmt.Errorf("not a WindowsClientConn: %T", ci.conn) } - - // We duplicate the token's handle so that the WindowsToken we return may have - // a lifetime independent from the original connection. - var h windows.Handle - if err := windows.DuplicateHandle( - windows.CurrentProcess(), - windows.Handle(wcc.Token()), - windows.CurrentProcess(), - &h, - 0, - false, - windows.DUPLICATE_SAME_ACCESS, - ); err != nil { + token, err := wcc.Token() + if err != nil { return nil, err } - - return newToken(windows.Token(h)), nil + return newToken(token), nil } diff --git a/safesocket/pipe_windows.go b/safesocket/pipe_windows.go index 582834165..2968542f2 100644 --- a/safesocket/pipe_windows.go +++ b/safesocket/pipe_windows.go @@ -10,6 +10,7 @@ "fmt" "net" "runtime" + "sync" "time" "github.com/tailscale/go-winio" @@ -49,7 +50,9 @@ func listen(path string) (net.Listener, error) { // embedded net.Conn must be a go-winio PipeConn. type WindowsClientConn struct { winioPipeConn - token windows.Token + tokenOnce sync.Once + token windows.Token // or zero, if we couldn't obtain the client's token + tokenErr error } // winioPipeConn is a subset of the interface implemented by the go-winio's @@ -79,12 +82,63 @@ func (conn *WindowsClientConn) ClientPID() (int, error) { return int(pid), nil } -// Token returns the Windows access token of the client user. -func (conn *WindowsClientConn) Token() windows.Token { - return conn.token +// CheckToken returns an error if the client user's access token could not be retrieved, +// for example when the client opens the pipe with an anonymous impersonation level. +// +// Deprecated: use [WindowsClientConn.Token] instead. +func (conn *WindowsClientConn) CheckToken() error { + _, err := conn.getToken() + return err +} + +// getToken returns the Windows access token of the client user, +// or an error if the token could not be retrieved, for example +// when the client opens the pipe with an anonymous impersonation level. +// +// The connection retains ownership of the returned token handle; +// the caller must not close it. +// +// TODO(nickkhyl): Remove this, along with [WindowsClientConn.CheckToken], +// once [ipnauth.ConnIdentity] is removed in favor of [ipnauth.Actor]. +func (conn *WindowsClientConn) getToken() (windows.Token, error) { + conn.tokenOnce.Do(func() { + conn.token, conn.tokenErr = clientUserAccessToken(conn.winioPipeConn) + }) + return conn.token, conn.tokenErr +} + +// Token returns the Windows access token of the client user, +// or an error if the token could not be retrieved, for example +// when the client opens the pipe with an anonymous impersonation level. +// +// The caller is responsible for closing the returned token handle. +func (conn *WindowsClientConn) Token() (windows.Token, error) { + token, err := conn.getToken() + if err != nil { + return 0, err + } + + var dupToken windows.Handle + if err := windows.DuplicateHandle( + windows.CurrentProcess(), + windows.Handle(token), + windows.CurrentProcess(), + &dupToken, + 0, + false, + windows.DUPLICATE_SAME_ACCESS, + ); err != nil { + return 0, err + } + return windows.Token(dupToken), nil } func (conn *WindowsClientConn) Close() error { + // Either wait for any pending [WindowsClientConn.Token] calls to complete, + // or ensure that the token will never be opened. + conn.tokenOnce.Do(func() { + conn.tokenErr = net.ErrClosed + }) if conn.token != 0 { conn.token.Close() conn.token = 0 @@ -110,17 +164,7 @@ func (lw *winIOPipeListener) Accept() (net.Conn, error) { conn.Close() return nil, fmt.Errorf("unexpected type %T from winio.ListenPipe listener (itself a %T)", conn, lw.Listener) } - - token, err := clientUserAccessToken(pipeConn) - if err != nil { - conn.Close() - return nil, err - } - - return &WindowsClientConn{ - winioPipeConn: pipeConn, - token: token, - }, nil + return &WindowsClientConn{winioPipeConn: pipeConn}, nil } func clientUserAccessToken(pc winioPipeConn) (windows.Token, error) { diff --git a/safesocket/pipe_windows_test.go b/safesocket/pipe_windows_test.go index 054781f23..8d9cbd19b 100644 --- a/safesocket/pipe_windows_test.go +++ b/safesocket/pipe_windows_test.go @@ -58,9 +58,14 @@ func TestExpectedWindowsTypes(t *testing.T) { if wcc.winioPipeConn.Fd() == 0 { t.Error("accepted conn had unexpected zero fd") } - if wcc.token == 0 { + tok, err := wcc.Token() + if err != nil { + t.Errorf("failed to retrieve client token: %v", err) + } + if tok == 0 { t.Error("accepted conn had unexpected zero token") } + tok.Close() s.Write([]byte("hello"))