From efc48b0578739538cd808ebcb458b1e94731f04c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 10 Mar 2022 10:28:42 -0800 Subject: [PATCH] ssh/tailssh, ipnlocal, controlclient: fetch next SSHAction from network Updates #3802 Change-Id: I08e98805ab86d6bbabb6c365ed4526f54742fd8e Signed-off-by: Brad Fitzpatrick --- control/controlclient/auto.go | 5 +++ control/controlclient/client.go | 4 ++ control/controlclient/direct.go | 8 ++++ ipn/ipnlocal/local.go | 12 +++++ ipn/ipnlocal/state_test.go | 5 +++ ssh/tailssh/tailssh.go | 78 ++++++++++++++++++++++++++++----- tailcfg/tailcfg.go | 12 +++-- 7 files changed, 108 insertions(+), 16 deletions(-) diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index 292229ca8..cecf95c60 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -7,6 +7,7 @@ import ( "context" "fmt" + "net/http" "sync" "time" @@ -725,3 +726,7 @@ func (c *Auto) TestOnlyTimeNow() time.Time { func (c *Auto) SetDNS(ctx context.Context, req *tailcfg.SetDNSRequest) error { return c.direct.SetDNS(ctx, req) } + +func (c *Auto) DoNoiseRequest(req *http.Request) (*http.Response, error) { + return c.direct.DoNoiseRequest(req) +} diff --git a/control/controlclient/client.go b/control/controlclient/client.go index 3aff039d2..8b2f7c7bd 100644 --- a/control/controlclient/client.go +++ b/control/controlclient/client.go @@ -11,6 +11,7 @@ import ( "context" + "net/http" "time" "tailscale.com/tailcfg" @@ -82,6 +83,9 @@ type Client interface { // SetDNS sends the SetDNSRequest request to the control plane server, // requesting a DNS record be created or updated. SetDNS(context.Context, *tailcfg.SetDNSRequest) error + // DoNoiseRequest sends an HTTP request to the control plane + // over the Noise transport. + DoNoiseRequest(*http.Request) (*http.Response, error) } // UserVisibleError is an error that should be shown to users. diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 5c9fd21f4..1d0496e6a 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -1428,6 +1428,14 @@ func (c *Direct) SetDNS(ctx context.Context, req *tailcfg.SetDNSRequest) (err er return nil } +func (c *Direct) DoNoiseRequest(req *http.Request) (*http.Response, error) { + nc, err := c.getNoiseClient() + if err != nil { + return nil, err + } + return nc.Do(req) +} + // tsmpPing sends a Ping to pr.IP, and sends an http request back to pr.URL // with ping response data. func tsmpPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pinger Pinger) error { diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 793da4973..c17ffe7f9 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3241,3 +3241,15 @@ func (b *LocalBackend) magicConn() (*magicsock.Conn, error) { } return mc, nil } + +// DoNoiseRequest sends a request to URL over the the control plane +// Noise connection. +func (b *LocalBackend) DoNoiseRequest(req *http.Request) (*http.Response, error) { + b.mu.Lock() + cc := b.cc + b.mu.Unlock() + if cc == nil { + return nil, errors.New("no client") + } + return cc.DoNoiseRequest(req) +} diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 8d79cfb2c..ba3fb5c22 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -6,6 +6,7 @@ import ( "context" + "net/http" "sync" "testing" "time" @@ -266,6 +267,10 @@ func (*mockControl) SetDNS(context.Context, *tailcfg.SetDNSRequest) error { panic("unexpected SetDNS call") } +func (*mockControl) DoNoiseRequest(*http.Request) (*http.Response, error) { + panic("unexpected DoNoiseRequest call") +} + // A very precise test of the sequence of function calls generated by // ipnlocal.Local into its controlclient instance, and the events it // produces upstream into the UI. diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index fb37b5922..6e872a36a 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -15,6 +15,7 @@ "fmt" "io" "net" + "net/http" "os" "os/exec" "os/user" @@ -26,6 +27,7 @@ "inet.af/netaddr" "tailscale.com/envknob" "tailscale.com/ipn/ipnlocal" + "tailscale.com/logtail/backoff" "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "tailscale.com/types/logger" @@ -200,19 +202,40 @@ func (srv *server) handleSSH(s ssh.Session) { s.Exit(1) return } - if action.Message != "" { - io.WriteString(s.Stderr(), strings.Replace(action.Message, "\n", "\r\n", -1)) - } - if action.Reject { - logf("ssh: access denied for %q from %v", ci.uprof.LoginName, ci.src.IP()) - s.Exit(1) - return - } - if !action.Accept || action.HoldAndDelegate != "" { - fmt.Fprintf(s, "TODO: other SSHAction outcomes") - s.Exit(1) - return + + // Loop processing/fetching Actions until one reaches a + // terminal state (Accept, Reject, or invalid Action), or + // until fetchSSHAction times out due to the context being + // done (client disconnect) or its 30 minute timeout passes. + // (Which is a long time for somebody to see login + // instructions and go to a URL to do something.) +ProcessAction: + for { + if action.Message != "" { + io.WriteString(s.Stderr(), strings.Replace(action.Message, "\n", "\r\n", -1)) + } + if action.Reject { + logf("ssh: access denied for %q from %v", ci.uprof.LoginName, ci.src.IP()) + s.Exit(1) + return + } + if action.Accept { + break ProcessAction + } + url := action.HoldAndDelegate + if url == "" { + logf("ssh: access denied; SSHAction has neither Reject, Accept, or next step URL") + s.Exit(1) + return + } + action, err = srv.fetchSSHAction(s.Context(), url) + if err != nil { + logf("ssh: fetching SSAction from %s: %v", url, err) + s.Exit(1) + return + } } + lu, err := user.Lookup(localUser) if err != nil { logf("ssh: user Lookup %q: %v", localUser, err) @@ -235,6 +258,37 @@ func (srv *server) handleSSH(s ssh.Session) { srv.handleAcceptedSSH(ctx, s, ci, lu) } +func (srv *server) fetchSSHAction(ctx context.Context, url string) (*tailcfg.SSHAction, error) { + ctx, cancel := context.WithTimeout(ctx, 30*time.Minute) + defer cancel() + bo := backoff.NewBackoff("fetch-ssh-action", srv.logf, 10*time.Second) + for { + if err := ctx.Err(); err != nil { + return nil, err + } + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + if err != nil { + return nil, err + } + res, err := srv.lb.DoNoiseRequest(req) + if err != nil { + bo.BackOff(ctx, err) + continue + } + if res.StatusCode != 200 { + res.Body.Close() + bo.BackOff(ctx, fmt.Errorf("unexpected status: %v", res.Status)) + continue + } + a := new(tailcfg.SSHAction) + if err := json.NewDecoder(res.Body).Decode(a); err != nil { + bo.BackOff(ctx, err) + continue + } + return a, nil + } +} + func (srv *server) handleSessionTermination(ctx context.Context, s ssh.Session, ci *sshConnInfo, cmd *exec.Cmd, exitOnce *sync.Once) { <-ctx.Done() // Either the process has already existed, in which case this does nothing. diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 1ba5a3e27..5305dea4f 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -1615,10 +1615,14 @@ type SSHAction struct { // before being forcefully terminated. SesssionDuration time.Duration `json:"sessionDuration,omitempty"` - // HoldAndDelegate, if non-empty, is a URL that serves an outcome verdict. - // The connection will be accepted and will block until the - // provided long-polling URL serves a new SSHAction JSON - // value. + // HoldAndDelegate, if non-empty, is a URL that serves an + // outcome verdict. The connection will be accepted and will + // block until the provided long-polling URL serves a new + // SSHAction JSON value. The URL must be fetched using the + // Noise transport (in package control/control{base,http}). + // If the long poll breaks before returning a complete HTTP + // response, it should be re-fetched as long as the SSH + // session is open. HoldAndDelegate string `json:"holdAndDelegate,omitempty"` }