mirror of
https://github.com/tailscale/tailscale.git
synced 2026-02-08 17:21:33 +01:00
ssh/tailssh: skip policy compliance check pre-auth
When the OnPolicyChange handler is called before authentication has completed, the localUser field on the connection may not have been populated yet, which leads to a nil pointer dereference when OnPolicyChange tries to read it. In order to avoid this, we skip the validation when the localUser is empty (something we're already doing when the info field is empty). The authentication will check compliance with the policy once it completes. Updates tailscale/corp#36268 Signed-off-by: Gesa Stupperich <gesa@tailscale.com>
This commit is contained in:
parent
1183f7a191
commit
cd66071731
@ -192,9 +192,12 @@ func (srv *server) OnPolicyChange() {
|
||||
srv.mu.Lock()
|
||||
defer srv.mu.Unlock()
|
||||
for c := range srv.activeConns {
|
||||
if c.info == nil {
|
||||
// c.info is nil when the connection hasn't been authenticated yet.
|
||||
// In that case, the connection will be terminated when it is.
|
||||
// move info and localUser to be protected by conn mutex?
|
||||
if c.info == nil || c.localUser == nil {
|
||||
// c.info or c.localUser are nil when the connection hasn't been
|
||||
// authenticated yet. We will continue here, but the connection will
|
||||
// be rechecked once it is authenticated. If it no longer conforms
|
||||
// with the SSH access policy at that point, it will be terminated.
|
||||
continue
|
||||
}
|
||||
go c.checkStillValid()
|
||||
|
||||
@ -31,6 +31,7 @@ import (
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
"time"
|
||||
|
||||
gossh "golang.org/x/crypto/ssh"
|
||||
@ -1317,6 +1318,27 @@ func TestStdOsUserUserAssumptions(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestOnPolicyChangeHandlesNilLocalUser(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
srv := &server{
|
||||
logf: tstest.WhileTestRunningLogger(t),
|
||||
lb: &localState{
|
||||
sshEnabled: true,
|
||||
matchingRule: newSSHRule(&tailcfg.SSHAction{Accept: true}),
|
||||
},
|
||||
}
|
||||
c := &conn{
|
||||
srv: srv,
|
||||
info: &sshConnInfo{sshUser: "alice"},
|
||||
}
|
||||
srv.activeConns = map[*conn]bool{c: true}
|
||||
|
||||
srv.OnPolicyChange()
|
||||
|
||||
synctest.Wait()
|
||||
})
|
||||
}
|
||||
|
||||
func mockRecordingServer(t *testing.T, handleRecord http.HandlerFunc) *httptest.Server {
|
||||
t.Helper()
|
||||
mux := http.NewServeMux()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user