mirror of
https://github.com/juanfont/headscale.git
synced 2026-10-23 23:06:49 +09:00
mapper: send an empty SSH policy when compiling it fails
Fail closed; nil kept the client's previous rules. Updates #3508
This commit is contained in:
1 parent
2a0823ca41
commit
d7b333d2aa
3 files changed
+63
-6
No files matched your search
@@ -1,6 +1,7 @@
|
|||||||
package mapper
|
package mapper
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/netip"
|
"net/netip"
|
||||||
@@ -13,6 +14,7 @@ import (
|
|||||||
|
|
||||||
"github.com/juanfont/headscale/hscontrol/db"
|
"github.com/juanfont/headscale/hscontrol/db"
|
||||||
"github.com/juanfont/headscale/hscontrol/derp"
|
"github.com/juanfont/headscale/hscontrol/derp"
|
||||||
|
"github.com/juanfont/headscale/hscontrol/policy"
|
||||||
"github.com/juanfont/headscale/hscontrol/state"
|
"github.com/juanfont/headscale/hscontrol/state"
|
||||||
"github.com/juanfont/headscale/hscontrol/types"
|
"github.com/juanfont/headscale/hscontrol/types"
|
||||||
"github.com/juanfont/headscale/hscontrol/types/change"
|
"github.com/juanfont/headscale/hscontrol/types/change"
|
||||||
@@ -2409,6 +2411,55 @@ func TestAddWorkFullUpdateUnaffectedByEmpty(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var errInjectedSSHPolicy = errors.New("injected SSH policy failure")
|
||||||
|
|
||||||
|
type failingSSHPolicyManager struct {
|
||||||
|
policy.PolicyManager
|
||||||
|
}
|
||||||
|
|
||||||
|
func (failingSSHPolicyManager) SSHPolicy(string, types.NodeView) (*tailcfg.SSHPolicy, error) {
|
||||||
|
return nil, errInjectedSSHPolicy
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSSHPolicyEmptyOnWire pins that a node without SSH rules, or whose SSH
|
||||||
|
// policy fails to compile, gets "SSHPolicy":{"rules":[]}. A nil SSHPolicy
|
||||||
|
// leaves the client's previous rules in force.
|
||||||
|
// https://github.com/juanfont/headscale/issues/3508
|
||||||
|
func TestSSHPolicyEmptyOnWire(t *testing.T) {
|
||||||
|
for name, failCompile := range map[string]bool{
|
||||||
|
"no ssh rules": false,
|
||||||
|
"compile error": true,
|
||||||
|
} {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
testData, cleanup := setupBatcherWithTestData(t, NewBatcherAndMapper, 1, 2, normalBufferSize)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
if failCompile {
|
||||||
|
testData.State.WrapPolicyManagerForTest(
|
||||||
|
func(pm policy.PolicyManager) policy.PolicyManager {
|
||||||
|
return failingSSHPolicyManager{PolicyManager: pm}
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
self := testData.Nodes[0].n.ID
|
||||||
|
mc := newMockNodeConnection(self)
|
||||||
|
|
||||||
|
require.NoError(t, handleNodeChange(mc, testData.Batcher.mapper, change.PolicyChange()))
|
||||||
|
require.NoError(t, handleNodeChange(mc, testData.Batcher.mapper, change.FullSelf(self)))
|
||||||
|
|
||||||
|
sent := mc.getSent()
|
||||||
|
require.Len(t, sent, 2)
|
||||||
|
|
||||||
|
for _, resp := range sent {
|
||||||
|
wire, err := json.Marshal(resp)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Contains(t, string(wire), `"SSHPolicy":{"rules":[]}`)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestAddWorkPeersRemovedNotTreatedAsEmpty ensures the empty filter does
|
// TestAddWorkPeersRemovedNotTreatedAsEmpty ensures the empty filter does
|
||||||
// not swallow a PeersRemoved change — deletion cleanup must still run, and
|
// not swallow a PeersRemoved change — deletion cleanup must still run, and
|
||||||
// surviving recipients must still see the removal.
|
// surviving recipients must still see the removal.
|
||||||
|
|||||||
@@ -151,16 +151,15 @@ func (b *MapResponseBuilder) WithSSHPolicy() *MapResponseBuilder {
|
|||||||
|
|
||||||
sshPolicy, err := b.mapper.state.SSHPolicy(node)
|
sshPolicy, err := b.mapper.state.SSHPolicy(node)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// SSH policy is optional for a node to function. Rather than fail the
|
// Don't fail the whole map over SSH, but fail closed: a nil
|
||||||
// whole map (leaving the node unable to connect), log and continue
|
// SSHPolicy would leave the client's previous rules in force.
|
||||||
// without it; the node still receives a usable netmap.
|
log.Error().Caller().
|
||||||
log.Warn().Caller().
|
|
||||||
Err(err).
|
Err(err).
|
||||||
Uint64(zf.NodeID, node.ID().Uint64()).
|
Uint64(zf.NodeID, node.ID().Uint64()).
|
||||||
Str(zf.NodeHostname, node.Hostname()).
|
Str(zf.NodeHostname, node.Hostname()).
|
||||||
Msg("building map response: skipping SSH policy for node; node will receive a map without SSH rules")
|
Msg("building map response: compiling SSH policy failed; sending empty SSH policy")
|
||||||
|
|
||||||
return b
|
sshPolicy = &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{}}
|
||||||
}
|
}
|
||||||
|
|
||||||
b.resp.SSHPolicy = sshPolicy
|
b.resp.SSHPolicy = sshPolicy
|
||||||
|
|||||||
@@ -1624,6 +1624,13 @@ func (s *State) DeleteNodeFromStoreForTest(id types.NodeID) {
|
|||||||
s.nodeStore.DeleteNode(id)
|
s.nodeStore.DeleteNode(id)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// WrapPolicyManagerForTest replaces the policy manager with wrap(current) so
|
||||||
|
// tests can fail a single method. The [NodeStore] keeps the original manager,
|
||||||
|
// so wrap must delegate to its argument. Call before any map is served.
|
||||||
|
func (s *State) WrapPolicyManagerForTest(wrap func(policy.PolicyManager) policy.PolicyManager) {
|
||||||
|
s.polMan = wrap(s.polMan)
|
||||||
|
}
|
||||||
|
|
||||||
// CreateRegisteredNodeForTest creates a test node with allocated IPs. This is a convenience wrapper around the database layer.
|
// CreateRegisteredNodeForTest creates a test node with allocated IPs. This is a convenience wrapper around the database layer.
|
||||||
func (s *State) CreateRegisteredNodeForTest(user *types.User, hostname ...string) *types.Node {
|
func (s *State) CreateRegisteredNodeForTest(user *types.User, hostname ...string) *types.Node {
|
||||||
return s.db.CreateRegisteredNodeForTest(user, hostname...)
|
return s.db.CreateRegisteredNodeForTest(user, hostname...)
|
||||||
|
|||||||
Reference in new issue
Block a user