diff --git a/client/web/auth.go b/client/web/auth.go index 35312fde0..f4ed2aced 100644 --- a/client/web/auth.go +++ b/client/web/auth.go @@ -4,14 +4,10 @@ package web import ( - "bytes" "context" "crypto/rand" "encoding/base64" - "encoding/json" "errors" - "fmt" - "io" "net/http" "time" @@ -152,7 +148,7 @@ func (s *Server) getSession(r *http.Request) (*browserSession, *apitype.WhoIsRes // and stores it back to the session cache. Creating of a new session includes // generating a new auth URL from the control server. func (s *Server) newSession(ctx context.Context, src *apitype.WhoIsResponse) (*browserSession, error) { - d, err := s.getOrAwaitAuth(ctx, "", src.Node.ID) + a, err := s.newAuthURL(ctx, src.Node.ID) if err != nil { return nil, err } @@ -164,8 +160,8 @@ func (s *Server) newSession(ctx context.Context, src *apitype.WhoIsResponse) (*b ID: sid, SrcNode: src.Node.ID, SrcUser: src.UserProfile.ID, - AuthID: d.ID, - AuthURL: d.URL, + AuthID: a.ID, + AuthURL: a.URL, Created: s.timeNow(), } s.browserSessions.Store(sid, session) @@ -179,7 +175,7 @@ func (s *Server) awaitUserAuth(ctx context.Context, session *browserSession) err if session.isAuthorized(s.timeNow()) { return nil // already authorized } - d, err := s.getOrAwaitAuth(ctx, session.AuthID, session.SrcNode) + a, err := s.waitAuthURL(ctx, session.AuthID, session.SrcNode) if err != nil { // Clean up the session. Doing this on any error from control // server to avoid the user getting stuck with a bad session @@ -187,52 +183,13 @@ func (s *Server) awaitUserAuth(ctx context.Context, session *browserSession) err s.browserSessions.Delete(session.ID) return err } - if d.Complete { - session.Authenticated = d.Complete + if a.Complete { + session.Authenticated = a.Complete s.browserSessions.Store(session.ID, session) } return nil } -// getOrAwaitAuth connects to the control server for user auth, -// with the following behavior: -// -// 1. If authID is provided empty, a new auth URL is created on the control -// server and reported back here, which can then be used to redirect the -// user on the frontend. -// 2. If authID is provided non-empty, the connection to control blocks until -// the user has completed authenticating the associated auth URL, -// or until ctx is canceled. -func (s *Server) getOrAwaitAuth(ctx context.Context, authID string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) { - type data struct { - ID string - Src tailcfg.NodeID - } - var b bytes.Buffer - if err := json.NewEncoder(&b).Encode(data{ID: authID, Src: src}); err != nil { - return nil, err - } - url := "http://" + apitype.LocalAPIHost + "/localapi/v0/debug-web-client" - req, err := http.NewRequestWithContext(ctx, "POST", url, &b) - if err != nil { - return nil, err - } - resp, err := s.lc.DoLocalRequest(req) - if err != nil { - return nil, err - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("failed request: %s", body) - } - var authResp *tailcfg.WebClientAuthResponse - if err := json.Unmarshal(body, &authResp); err != nil { - return nil, err - } - return authResp, nil -} - func (s *Server) newSessionID() (string, error) { raw := make([]byte, 16) for i := 0; i < 5; i++ { diff --git a/client/web/web.go b/client/web/web.go index 3ecde836a..9457a564b 100644 --- a/client/web/web.go +++ b/client/web/web.go @@ -71,6 +71,13 @@ type Server struct { // The map provides a lookup of the session by cookie value // (browserSession.ID => browserSession). browserSessions sync.Map + + // newAuthURL creates a new auth URL that can be used to validate + // a browser session to manage this web client. + newAuthURL func(ctx context.Context, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) + // waitWebClientAuthURL blocks until the associated auth URL has + // been completed by its user, or until ctx is canceled. + waitAuthURL func(ctx context.Context, id string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) } // ServerMode specifies the mode of a running web.Server. @@ -121,6 +128,20 @@ type ServerOpts struct { // Logf optionally provides a logger function. // log.Printf is used as default. Logf logger.Logf + + // The following two fields are required and used exclusively + // in ManageServerMode to facilitate the control server login + // check step for authorizing browser sessions. + + // NewAuthURL should be provided as a function that generates + // a new tailcfg.WebClientAuthResponse. + // This field is required for ManageServerMode mode. + NewAuthURL func(ctx context.Context, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) + // WaitAuthURL should be provided as a function that blocks until + // the associated tailcfg.WebClientAuthResponse has been marked + // as completed. + // This field is required for ManageServerMode mode. + WaitAuthURL func(ctx context.Context, id string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) } // NewServer constructs a new Tailscale web client server. @@ -140,13 +161,23 @@ func NewServer(opts ServerOpts) (s *Server, err error) { opts.LocalClient = &tailscale.LocalClient{} } s = &Server{ - mode: opts.Mode, - logf: opts.Logf, - devMode: envknob.Bool("TS_DEBUG_WEB_CLIENT_DEV"), - lc: opts.LocalClient, - cgiMode: opts.CGIMode, - pathPrefix: opts.PathPrefix, - timeNow: opts.TimeNow, + mode: opts.Mode, + logf: opts.Logf, + devMode: envknob.Bool("TS_DEBUG_WEB_CLIENT_DEV"), + lc: opts.LocalClient, + cgiMode: opts.CGIMode, + pathPrefix: opts.PathPrefix, + timeNow: opts.TimeNow, + newAuthURL: opts.NewAuthURL, + waitAuthURL: opts.WaitAuthURL, + } + if s.mode == ManageServerMode { + if opts.NewAuthURL == nil { + return nil, fmt.Errorf("must provide a NewAuthURL implementation") + } + if opts.WaitAuthURL == nil { + return nil, fmt.Errorf("must provide WaitAuthURL implementation") + } } if s.timeNow == nil { s.timeNow = time.Now diff --git a/client/web/web_test.go b/client/web/web_test.go index 3a1e6b763..bd256bdac 100644 --- a/client/web/web_test.go +++ b/client/web/web_test.go @@ -4,6 +4,7 @@ package web import ( + "context" "encoding/json" "errors" "fmt" @@ -446,9 +447,11 @@ func TestServeAuth(t *testing.T) { sixtyDaysAgo := timeNow.Add(-sessionCookieExpiry * 2) s := &Server{ - mode: ManageServerMode, - lc: &tailscale.LocalClient{Dial: lal.Dial}, - timeNow: func() time.Time { return timeNow }, + mode: ManageServerMode, + lc: &tailscale.LocalClient{Dial: lal.Dial}, + timeNow: func() time.Time { return timeNow }, + newAuthURL: mockNewAuthURL, + waitAuthURL: mockWaitAuthURL, } successCookie := "ts-cookie-success" @@ -797,33 +800,24 @@ func mockLocalAPI(t *testing.T, whoIs map[string]*apitype.WhoIsResponse, self fu case "/localapi/v0/status": writeJSON(w, ipnstate.Status{Self: self()}) return - case "/localapi/v0/debug-web-client": // used by TestServeTailscaleAuth - type reqData struct { - ID string - Src tailcfg.NodeID - } - var data reqData - if err := json.NewDecoder(r.Body).Decode(&data); err != nil { - http.Error(w, "invalid JSON body", http.StatusBadRequest) - return - } - if data.Src == 0 { - http.Error(w, "missing Src node", http.StatusBadRequest) - return - } - var resp *tailcfg.WebClientAuthResponse - if data.ID == "" { - resp = &tailcfg.WebClientAuthResponse{ID: testAuthPath, URL: testControlURL + testAuthPath} - } else if data.ID == testAuthPathSuccess { - resp = &tailcfg.WebClientAuthResponse{Complete: true} - } else if data.ID == testAuthPathError { - http.Error(w, "authenticated as wrong user", http.StatusUnauthorized) - return - } - writeJSON(w, resp) - return default: t.Fatalf("unhandled localapi test endpoint %q, add to localapi handler func in test", r.URL.Path) } })} } + +func mockNewAuthURL(_ context.Context, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) { + // Create new dummy auth URL. + return &tailcfg.WebClientAuthResponse{ID: testAuthPath, URL: testControlURL + testAuthPath}, nil +} + +func mockWaitAuthURL(_ context.Context, id string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) { + switch id { + case testAuthPathSuccess: // successful auth URL + return &tailcfg.WebClientAuthResponse{Complete: true}, nil + case testAuthPathError: // error auth URL + return nil, errors.New("authenticated as wrong user") + default: + return nil, errors.New("unknown id") + } +} diff --git a/ipn/ipnlocal/web_client.go b/ipn/ipnlocal/web_client.go index ba000d575..ee919d976 100644 --- a/ipn/ipnlocal/web_client.go +++ b/ipn/ipnlocal/web_client.go @@ -7,8 +7,10 @@ package ipnlocal import ( "context" + "encoding/json" "errors" "fmt" + "io" "net" "net/http" "net/netip" @@ -19,6 +21,7 @@ import ( "tailscale.com/client/web" "tailscale.com/logtail/backoff" "tailscale.com/net/netutil" + "tailscale.com/tailcfg" "tailscale.com/types/logger" "tailscale.com/util/mak" ) @@ -67,6 +70,8 @@ func (b *LocalBackend) webClientGetOrInit() (s *web.Server, err error) { Mode: web.ManageServerMode, LocalClient: b.webClient.lc, Logf: b.logf, + NewAuthURL: b.newWebClientAuthURL, + WaitAuthURL: b.waitWebClientAuthURL, }); err != nil { return nil, fmt.Errorf("web.NewServer: %w", err) } @@ -144,3 +149,57 @@ func (b *LocalBackend) newWebClientListener(ctx context.Context, ap netip.AddrPo bo: backoff.NewBackoff("webclient-listener", logf, 30*time.Second), } } + +// newWebClientAuthURL talks to the control server to create a new auth +// URL that can be used to validate a browser session to manage this +// tailscaled instance via the web client. +func (b *LocalBackend) newWebClientAuthURL(ctx context.Context, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) { + return b.doWebClientNoiseRequest(ctx, "", src) +} + +// waitWebClientAuthURL connects to the control server and blocks +// until the associated auth URL has been completed by its user, +// or until ctx is canceled. +func (b *LocalBackend) waitWebClientAuthURL(ctx context.Context, id string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) { + return b.doWebClientNoiseRequest(ctx, id, src) +} + +// doWebClientNoiseRequest handles making the "/machine/webclient" +// noise requests to the control server for web client user auth. +// +// It either creates a new control auth URL or waits for an existing +// one to be completed, based on the presence or absence of the +// provided id value. +func (b *LocalBackend) doWebClientNoiseRequest(ctx context.Context, id string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) { + nm := b.NetMap() + if nm == nil || !nm.SelfNode.Valid() { + return nil, errors.New("[unexpected] no self node") + } + dst := nm.SelfNode.ID() + var noiseURL string + if id != "" { + noiseURL = fmt.Sprintf("https://unused/machine/webclient/wait/%d/to/%d/%s", src, dst, id) + } else { + noiseURL = fmt.Sprintf("https://unused/machine/webclient/init/%d/to/%d", src, dst) + } + + req, err := http.NewRequestWithContext(ctx, "POST", noiseURL, nil) + if err != nil { + return nil, err + } + resp, err := b.DoNoiseRequest(req) + if err != nil { + return nil, err + } + + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("failed request: %s", body) + } + var authResp *tailcfg.WebClientAuthResponse + if err := json.Unmarshal(body, &authResp); err != nil { + return nil, err + } + return authResp, nil +} diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index f4bcc4143..7d2a3c440 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -86,7 +86,6 @@ var handler = map[string]localAPIHandler{ "debug-peer-endpoint-changes": (*Handler).serveDebugPeerEndpointChanges, "debug-capture": (*Handler).serveDebugCapture, "debug-log": (*Handler).serveDebugLog, - "debug-web-client": (*Handler).serveDebugWebClient, "derpmap": (*Handler).serveDERPMap, "dev-set-state-store": (*Handler).serveDevSetStateStore, "set-push-device-token": (*Handler).serveSetPushDeviceToken, @@ -2300,65 +2299,6 @@ func (h *Handler) serveQueryFeature(w http.ResponseWriter, r *http.Request) { } } -// serveDebugWebClient is for use by the web client to communicate with -// the control server for browser auth sessions. -// -// This is an unsupported localapi endpoint and restricted to flagged -// domains on the control side. TODO(tailscale/#14335): Rename this handler. -func (h *Handler) serveDebugWebClient(w http.ResponseWriter, r *http.Request) { - if !h.PermitWrite { - http.Error(w, "access denied", http.StatusForbidden) - return - } - if r.Method != "POST" { - http.Error(w, "POST required", http.StatusMethodNotAllowed) - return - } - - type reqData struct { - ID string - Src tailcfg.NodeID - } - var data reqData - if err := json.NewDecoder(r.Body).Decode(&data); err != nil { - http.Error(w, "invalid JSON body", 400) - return - } - nm := h.b.NetMap() - if nm == nil || !nm.SelfNode.Valid() { - http.Error(w, "[unexpected] no self node", 400) - return - } - dst := nm.SelfNode.ID() - - var noiseURL string - if data.ID != "" { - noiseURL = fmt.Sprintf("https://unused/machine/webclient/wait/%d/to/%d/%s", data.Src, dst, data.ID) - } else { - noiseURL = fmt.Sprintf("https://unused/machine/webclient/init/%d/to/%d", data.Src, dst) - } - - req, err := http.NewRequestWithContext(r.Context(), "POST", noiseURL, nil) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - resp, err := h.b.DoNoiseRequest(req) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() - if resp.StatusCode != http.StatusOK { - http.Error(w, string(body), resp.StatusCode) - return - } - w.Write(body) - w.Header().Set("Content-Type", "application/json") -} - func defBool(a string, def bool) bool { if a == "" { return def