From 2a8de4a7eef8e324f7ff644258c754e01493b1ad Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Mon, 27 Jun 2022 12:13:50 -0700 Subject: [PATCH] ssh/tailssh: always use current time for policy evaluation Whenever the SSH policy changes we revaluate all open connections to make sure they still have access. This check was using the wrong timestamp and would match against expired policies, however this really isn't a problem today as we don't have policy that would be impacted by this check. Fixing it for future use. Signed-off-by: Maisem Ali (cherry picked from commit c434e47f2dd345a91ba3f9555d83d818db3b8ec6) --- ssh/tailssh/tailssh.go | 15 ++++++--------- ssh/tailssh/tailssh_test.go | 1 - 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index ba36cde1e..0fbdec1c6 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -69,7 +69,7 @@ type server struct { } func (srv *server) now() time.Time { - if srv.timeNow != nil { + if srv != nil && srv.timeNow != nil { return srv.timeNow() } return time.Now() @@ -152,10 +152,6 @@ type conn struct { insecureSkipTailscaleAuth bool // used by tests. - // now is the time to consider the present moment for the - // purposes of rule evaluation. - now time.Time - connID string // ID that's shared with control action0 *tailcfg.SSHAction // first matching action srv *server @@ -278,8 +274,9 @@ func (srv *server) newConn() (*conn, error) { return nil, gossh.ErrDenied } srv.mu.Unlock() - c := &conn{srv: srv, now: srv.now()} - c.connID = fmt.Sprintf("conn-%s-%02x", c.now.UTC().Format("20060102T150405"), randBytes(5)) + c := &conn{srv: srv} + now := srv.now() + c.connID = fmt.Sprintf("conn-%s-%02x", now.UTC().Format("20060102T150405"), randBytes(5)) c.Server = &ssh.Server{ Version: "Tailscale", Handler: c.handleSessionPostSSHAuth, @@ -751,7 +748,7 @@ func (ss *sshSession) vlogf(format string, args ...interface{}) { } func (c *conn) newSSHSession(s ssh.Session) *sshSession { - sharedID := fmt.Sprintf("sess-%s-%02x", c.now.UTC().Format("20060102T150405"), randBytes(5)) + sharedID := fmt.Sprintf("sess-%s-%02x", c.srv.now().UTC().Format("20060102T150405"), randBytes(5)) c.logf("starting session: %v", sharedID) return &sshSession{ Session: s, @@ -1087,7 +1084,7 @@ func (c *conn) ruleExpired(r *tailcfg.SSHRule) bool { if r.RuleExpires == nil { return false } - return r.RuleExpires.Before(c.now) + return r.RuleExpires.Before(c.srv.now()) } func (c *conn) evalSSHPolicy(pol *tailcfg.SSHPolicy, pubKey gossh.PublicKey) (a *tailcfg.SSHAction, localUser string, ok bool) { diff --git a/ssh/tailssh/tailssh_test.go b/ssh/tailssh/tailssh_test.go index 78bda83e2..ce43be36c 100644 --- a/ssh/tailssh/tailssh_test.go +++ b/ssh/tailssh/tailssh_test.go @@ -179,7 +179,6 @@ func TestMatchRule(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &conn{ - now: time.Unix(200, 0), info: tt.ci, } got, gotUser, err := c.matchRule(tt.rule, nil)