mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-04 12:39:25 +09:00
policy/v2: refuse an empty policy
SetPolicy returned a nil error for empty input it ignored, so the v1 API stored it as valid. Reload skips only the nil "no policy" case.
This commit is contained in:
1 parent
df7eb934a0
commit
89b37fde7d
5 files changed
+88
-7
No files matched your search
@@ -97,6 +97,24 @@ func TestAPIV1PolicySet(t *testing.T) {
|
||||
assert.NotEmpty(t, got.UpdatedAt)
|
||||
})
|
||||
|
||||
t.Run("empty rejected", func(t *testing.T) {
|
||||
h := newAPIV1Harness(t)
|
||||
|
||||
body, err := json.Marshal(map[string]string{"policy": validPolicy})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, http.StatusOK,
|
||||
h.callHuma(http.MethodPut, "/api/v1/policy", body).status)
|
||||
|
||||
for _, body := range []string{`{"policy":""}`, `{}`} {
|
||||
res := h.callHuma(http.MethodPut, "/api/v1/policy", []byte(body))
|
||||
assertStatus(t, res, http.StatusBadRequest)
|
||||
}
|
||||
|
||||
stored, err := h.app.state.GetPolicy()
|
||||
require.NoError(t, err)
|
||||
assert.JSONEq(t, validPolicy, stored.Data)
|
||||
})
|
||||
|
||||
t.Run("not-db mode rejected", func(t *testing.T) {
|
||||
// Policy updates are only valid in DB mode; file mode rejects with 400.
|
||||
// createTestApp is DB mode, so build a file-mode app here.
|
||||
@@ -132,6 +150,15 @@ func TestAPIV1PolicyCheck(t *testing.T) {
|
||||
assertStatus(t, res, http.StatusBadRequest)
|
||||
})
|
||||
|
||||
t.Run("empty rejected", func(t *testing.T) {
|
||||
h := newAPIV1Harness(t)
|
||||
|
||||
res := h.callHuma(
|
||||
http.MethodPost, "/api/v1/policy/check", []byte(`{"policy":""}`),
|
||||
)
|
||||
assertStatus(t, res, http.StatusBadRequest)
|
||||
})
|
||||
|
||||
t.Run("valid returns empty object", func(t *testing.T) {
|
||||
h := newAPIV1Harness(t)
|
||||
|
||||
|
||||
@@ -193,8 +193,6 @@ var apiv2CompatSkipReasons = map[string]string{
|
||||
|
||||
"policy-api/P7/0/body": hujsonFormatted,
|
||||
"policy-api/P7/1/body": hujsonFormatted,
|
||||
|
||||
"policy-api/P2/0/v1-verdict": v1CheckAcceptsEmpty,
|
||||
}
|
||||
|
||||
// apiv2CompatNotCompared lists, by key prefix, checks left out: SaaS's
|
||||
@@ -232,7 +230,6 @@ const (
|
||||
taggedAllClientAnyTag = "TAGGED_ALL_CLIENT_ANY_TAG: SaaS lets a tagged all-scope client's token ask only for the client's tags; headscale lets it ask for any tag"
|
||||
retagRemovesUnownedTag = "RETAG_REMOVES_UNOWNED_TAG: SaaS refuses a token's retag of a device holding a tag outside the token's grant with 400 cannot remove current tags; headscale replaces the tags"
|
||||
sshTestHostnames = "SSH_TEST_HOSTNAMES: headscale names the nodes behind a failing sshTests destination; SaaS names the destination as written"
|
||||
v1CheckAcceptsEmpty = "V1_CHECK_ACCEPTS_EMPTY: v1 policy check accepts a zero-length policy; SaaS's validate refuses it"
|
||||
)
|
||||
|
||||
// Differences no planned change removes yet.
|
||||
|
||||
@@ -29,6 +29,9 @@ import (
|
||||
// ErrInvalidTagOwner is returned when a tag owner is not an [Alias] type.
|
||||
var ErrInvalidTagOwner = errors.New("tag owner is not an Alias")
|
||||
|
||||
// ErrPolicyEmpty is returned by [PolicyManager.SetPolicy] for a zero-length policy.
|
||||
var ErrPolicyEmpty = errors.New(`policy is empty, use "{}" to allow all`)
|
||||
|
||||
type PolicyManager struct {
|
||||
// RWMutex, not Mutex, so concurrent map generation does not serialise on
|
||||
// reads. The per-node caches are xsync.Maps so a read can fill them without
|
||||
@@ -658,7 +661,7 @@ func (pm *PolicyManager) sshRuleAllowsUser(rule SSH, srcNode types.NodeView, loc
|
||||
|
||||
func (pm *PolicyManager) SetPolicy(polB []byte) (bool, error) {
|
||||
if len(polB) == 0 {
|
||||
return false, nil
|
||||
return false, ErrPolicyEmpty
|
||||
}
|
||||
|
||||
pol, err := unmarshalPolicy(polB)
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"maps"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"slices"
|
||||
@@ -18,6 +19,7 @@ import (
|
||||
|
||||
"github.com/juanfont/headscale/hscontrol/db"
|
||||
"github.com/juanfont/headscale/hscontrol/policy"
|
||||
policyv2 "github.com/juanfont/headscale/hscontrol/policy/v2"
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
"github.com/juanfont/headscale/hscontrol/types/change"
|
||||
"github.com/juanfont/headscale/hscontrol/types/testcapture"
|
||||
@@ -1312,6 +1314,54 @@ func TestReloadPolicyOverlappingWrites(t *testing.T) {
|
||||
assert.Equal(t, want, got, "the policy in force must be the one stored last")
|
||||
}
|
||||
|
||||
// TestReloadPolicyEmpty: with no policy configured a reload has nothing to
|
||||
// install; an emptied policy file is refused. Both keep the live policy.
|
||||
func TestReloadPolicyEmpty(t *testing.T) {
|
||||
const pol = `{"acls": [{"action": "accept", "src": ["persist-user@"], "dst": ["persist-user@:22"]}]}`
|
||||
|
||||
t.Run("no policy configured", func(t *testing.T) {
|
||||
_, s, _ := persistTestSetup(t)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
_, err := s.SetPolicy([]byte(pol))
|
||||
require.NoError(t, err)
|
||||
|
||||
before, _ := s.Filter()
|
||||
|
||||
_, err = s.ReloadPolicy()
|
||||
require.NoError(t, err)
|
||||
|
||||
after, _ := s.Filter()
|
||||
assert.Equal(t, before, after)
|
||||
})
|
||||
|
||||
t.Run("empty policy file", func(t *testing.T) {
|
||||
_, s, _ := persistTestSetup(t)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
s.cfg.Policy = types.PolicyConfig{
|
||||
Mode: types.PolicyModeFile,
|
||||
Path: filepath.Join(t.TempDir(), "policy.hujson"),
|
||||
}
|
||||
|
||||
require.NoError(t, os.WriteFile(s.cfg.Policy.Path, []byte(pol), 0o600))
|
||||
|
||||
_, err := s.ReloadPolicy()
|
||||
require.NoError(t, err)
|
||||
|
||||
before, _ := s.Filter()
|
||||
|
||||
// os.ReadFile gives an empty, non-nil slice here.
|
||||
require.NoError(t, os.WriteFile(s.cfg.Policy.Path, nil, 0o600))
|
||||
|
||||
_, err = s.ReloadPolicy()
|
||||
require.ErrorIs(t, err, policyv2.ErrPolicyEmpty)
|
||||
|
||||
after, _ := s.Filter()
|
||||
assert.Equal(t, before, after)
|
||||
})
|
||||
}
|
||||
|
||||
// TestNodeWriteChangeWhenPolicyRefreshFails fails both the NodeStore
|
||||
// writer's SetNodes and the caller's. The write still reached the
|
||||
// NodeStore, so the change returned with the error must resend the node to
|
||||
|
||||
@@ -411,9 +411,13 @@ func (s *State) ReloadPolicy() ([]change.Change, error) {
|
||||
return nil, fmt.Errorf("loading policy: %w", err)
|
||||
}
|
||||
|
||||
policyChanged, err := s.polMan.SetPolicy(pol)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("setting policy: %w", err)
|
||||
// nil means no policy is configured: nothing to install.
|
||||
var policyChanged bool
|
||||
if pol != nil {
|
||||
policyChanged, err = s.polMan.SetPolicy(pol)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("setting policy: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Clear SSH check auth times when policy changes to ensure stale
|
||||
|
||||
Reference in new issue
Block a user