From 71691ee34fe9eedb6f91d8f85d66f061f1d4a366 Mon Sep 17 00:00:00 2001 From: Pallab Pain Date: Mon, 24 Jun 2024 13:33:46 +0530 Subject: [PATCH] refactor(policy): removes support for YAML files BREAKING CHANGE: This commit removes the support for YAML files for defining ACL policies. The only supported format henceforth is going to be HuJSON. --- hscontrol/app.go | 2 +- hscontrol/db/node_test.go | 9 +++-- hscontrol/grpcv1.go | 2 +- hscontrol/policy/acls.go | 44 ++++++++------------- hscontrol/policy/acls_test.go | 64 ++++++++++++------------------- hscontrol/policy/acls_types.go | 70 ++++++++++++---------------------- integration/cli_test.go | 3 +- 7 files changed, 72 insertions(+), 122 deletions(-) diff --git a/hscontrol/app.go b/hscontrol/app.go index 179188f4f79..a4995d09cf0 100644 --- a/hscontrol/app.go +++ b/hscontrol/app.go @@ -1040,7 +1040,7 @@ func (h *Headscale) loadACLPolicy() error { return fmt.Errorf("failed to get policy from database: %w", err) } - pol, err = policy.LoadACLPolicyFromBytes([]byte(p.Data), "hujson") + pol, err = policy.LoadACLPolicyFromBytes([]byte(p.Data)) if err != nil { return fmt.Errorf("failed to parse policy: %w", err) } diff --git a/hscontrol/db/node_test.go b/hscontrol/db/node_test.go index e95ee4ae333..f1762a44844 100644 --- a/hscontrol/db/node_test.go +++ b/hscontrol/db/node_test.go @@ -8,13 +8,14 @@ import ( "testing" "time" - "github.com/juanfont/headscale/hscontrol/policy" - "github.com/juanfont/headscale/hscontrol/types" - "github.com/juanfont/headscale/hscontrol/util" "github.com/puzpuzpuz/xsync/v3" "gopkg.in/check.v1" "tailscale.com/tailcfg" "tailscale.com/types/key" + + "github.com/juanfont/headscale/hscontrol/policy" + "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util" ) func (s *Suite) TestGetNode(c *check.C) { @@ -545,7 +546,7 @@ func (s *Suite) TestAutoApproveRoutes(c *check.C) { } `) - pol, err := policy.LoadACLPolicyFromBytes(acl, "hujson") + pol, err := policy.LoadACLPolicyFromBytes(acl) c.Assert(err, check.IsNil) c.Assert(pol, check.NotNil) diff --git a/hscontrol/grpcv1.go b/hscontrol/grpcv1.go index 97e89c68b03..a351048fabf 100644 --- a/hscontrol/grpcv1.go +++ b/hscontrol/grpcv1.go @@ -720,7 +720,7 @@ func (api headscaleV1APIServer) SetPolicy( p := request.GetPolicy() - valid, err := policy.LoadACLPolicyFromBytes([]byte(p), "hujson") + valid, err := policy.LoadACLPolicyFromBytes([]byte(p)) if err != nil { return nil, err } diff --git a/hscontrol/policy/acls.go b/hscontrol/policy/acls.go index 1196995d098..2f2b8cd4b37 100644 --- a/hscontrol/policy/acls.go +++ b/hscontrol/policy/acls.go @@ -7,18 +7,17 @@ import ( "io" "net/netip" "os" - "path/filepath" "strconv" "strings" "time" - "github.com/juanfont/headscale/hscontrol/types" - "github.com/juanfont/headscale/hscontrol/util" "github.com/rs/zerolog/log" "github.com/tailscale/hujson" "go4.org/netipx" - "gopkg.in/yaml.v3" "tailscale.com/tailcfg" + + "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util" ) var ( @@ -108,35 +107,22 @@ func LoadACLPolicyFromPath(path string) (*ACLPolicy, error) { Bytes("file", policyBytes). Msg("Loading ACLs") - switch filepath.Ext(path) { - case ".yml", ".yaml": - return LoadACLPolicyFromBytes(policyBytes, "yaml") - } - - return LoadACLPolicyFromBytes(policyBytes, "hujson") + return LoadACLPolicyFromBytes(policyBytes) } -func LoadACLPolicyFromBytes(acl []byte, format string) (*ACLPolicy, error) { +func LoadACLPolicyFromBytes(acl []byte) (*ACLPolicy, error) { var policy ACLPolicy - switch format { - case "yaml": - err := yaml.Unmarshal(acl, &policy) - if err != nil { - return nil, err - } - default: - ast, err := hujson.Parse(acl) - if err != nil { - return nil, err - } + ast, err := hujson.Parse(acl) + if err != nil { + return nil, err + } - ast.Standardize() - acl = ast.Pack() - err = json.Unmarshal(acl, &policy) - if err != nil { - return nil, err - } + ast.Standardize() + acl = ast.Pack() + + if err := json.Unmarshal(acl, &policy); err != nil { + return nil, err } if policy.IsZero() { @@ -848,7 +834,7 @@ func (pol *ACLPolicy) expandIPsFromUser( // shortcurcuit if we have no nodes to get ips from. if len(filteredNodes) == 0 { - return nil, nil //nolint + return nil, nil // nolint } for _, node := range filteredNodes { diff --git a/hscontrol/policy/acls_test.go b/hscontrol/policy/acls_test.go index b0cafe105b9..03aae7220a8 100644 --- a/hscontrol/policy/acls_test.go +++ b/hscontrol/policy/acls_test.go @@ -6,14 +6,15 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/juanfont/headscale/hscontrol/types" - "github.com/juanfont/headscale/hscontrol/util" "github.com/rs/zerolog/log" "github.com/spf13/viper" "github.com/stretchr/testify/assert" "go4.org/netipx" "gopkg.in/check.v1" "tailscale.com/tailcfg" + + "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util" ) var iap = func(ipStr string) *netip.Addr { @@ -321,44 +322,27 @@ func TestParsing(t *testing.T) { wantErr: false, }, { - name: "port-wildcard-yaml", - format: "yaml", + name: "ipv6", + format: "hujson", acl: ` ---- -hosts: - host-1: 100.100.100.100/32 - subnet-1: 100.100.101.100/24 -acls: - - action: accept - src: - - "*" - dst: - - host-1:* -`, - want: []tailcfg.FilterRule{ - { - SrcIPs: []string{"0.0.0.0/0", "::/0"}, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.100.100.100/32", Ports: tailcfg.PortRangeAny}, - }, - }, - }, - wantErr: false, - }, +{ + "hosts": { + "host-1": "100.100.100.100/32", + "subnet-1": "100.100.101.100/24", + }, + + "acls": [ { - name: "ipv6-yaml", - format: "yaml", - acl: ` ---- -hosts: - host-1: 100.100.100.100/32 - subnet-1: 100.100.101.100/24 -acls: - - action: accept - src: - - "*" - dst: - - host-1:* + "action": "accept", + "src": [ + "*", + ], + "dst": [ + "host-1:*", + ], + }, + ], +} `, want: []tailcfg.FilterRule{ { @@ -374,7 +358,7 @@ acls: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - pol, err := LoadACLPolicyFromBytes([]byte(tt.acl), tt.format) + pol, err := LoadACLPolicyFromBytes([]byte(tt.acl)) if tt.wantErr && err == nil { t.Errorf("parsing() error = %v, wantErr %v", err, tt.wantErr) @@ -544,7 +528,7 @@ func (s *Suite) TestRuleInvalidGeneration(c *check.C) { ], } `) - pol, err := LoadACLPolicyFromBytes(acl, "hujson") + pol, err := LoadACLPolicyFromBytes(acl) c.Assert(pol.ACLs, check.HasLen, 6) c.Assert(err, check.IsNil) diff --git a/hscontrol/policy/acls_types.go b/hscontrol/policy/acls_types.go index e9c44909d16..25f02f16b5a 100644 --- a/hscontrol/policy/acls_types.go +++ b/hscontrol/policy/acls_types.go @@ -6,26 +6,25 @@ import ( "strings" "github.com/tailscale/hujson" - "gopkg.in/yaml.v3" ) // ACLPolicy represents a Tailscale ACL Policy. type ACLPolicy struct { - Groups Groups `json:"groups" yaml:"groups"` - Hosts Hosts `json:"hosts" yaml:"hosts"` - TagOwners TagOwners `json:"tagOwners" yaml:"tagOwners"` - ACLs []ACL `json:"acls" yaml:"acls"` - Tests []ACLTest `json:"tests" yaml:"tests"` - AutoApprovers AutoApprovers `json:"autoApprovers" yaml:"autoApprovers"` - SSHs []SSH `json:"ssh" yaml:"ssh"` + Groups Groups `json:"groups" ` + Hosts Hosts `json:"hosts"` + TagOwners TagOwners `json:"tagOwners"` + ACLs []ACL `json:"acls"` + Tests []ACLTest `json:"tests"` + AutoApprovers AutoApprovers `json:"autoApprovers"` + SSHs []SSH `json:"ssh"` } // ACL is a basic rule for the ACL Policy. type ACL struct { - Action string `json:"action" yaml:"action"` - Protocol string `json:"proto" yaml:"proto"` - Sources []string `json:"src" yaml:"src"` - Destinations []string `json:"dst" yaml:"dst"` + Action string `json:"action"` + Protocol string `json:"proto"` + Sources []string `json:"src"` + Destinations []string `json:"dst"` } // Groups references a series of alias in the ACL rules. @@ -37,27 +36,27 @@ type Hosts map[string]netip.Prefix // TagOwners specify what users (users?) are allow to use certain tags. type TagOwners map[string][]string -// ACLTest is not implemented, but should be use to check if a certain rule is allowed. +// ACLTest is not implemented, but should be used to check if a certain rule is allowed. type ACLTest struct { - Source string `json:"src" yaml:"src"` - Accept []string `json:"accept" yaml:"accept"` - Deny []string `json:"deny,omitempty" yaml:"deny,omitempty"` + Source string `json:"src"` + Accept []string `json:"accept"` + Deny []string `json:"deny,omitempty"` } // AutoApprovers specify which users (users?), groups or tags have their advertised routes // or exit node status automatically enabled. type AutoApprovers struct { - Routes map[string][]string `json:"routes" yaml:"routes"` - ExitNode []string `json:"exitNode" yaml:"exitNode"` + Routes map[string][]string `json:"routes"` + ExitNode []string `json:"exitNode"` } // SSH controls who can ssh into which machines. type SSH struct { - Action string `json:"action" yaml:"action"` - Sources []string `json:"src" yaml:"src"` - Destinations []string `json:"dst" yaml:"dst"` - Users []string `json:"users" yaml:"users"` - CheckPeriod string `json:"checkPeriod,omitempty" yaml:"checkPeriod,omitempty"` + Action string `json:"action"` + Sources []string `json:"src"` + Destinations []string `json:"dst"` + Users []string `json:"users"` + CheckPeriod string `json:"checkPeriod,omitempty"` } // UnmarshalJSON allows to parse the Hosts directly into netip objects. @@ -89,27 +88,6 @@ func (hosts *Hosts) UnmarshalJSON(data []byte) error { return nil } -// UnmarshalYAML allows to parse the Hosts directly into netip objects. -func (hosts *Hosts) UnmarshalYAML(data []byte) error { - newHosts := Hosts{} - hostIPPrefixMap := make(map[string]string) - - err := yaml.Unmarshal(data, &hostIPPrefixMap) - if err != nil { - return err - } - for host, prefixStr := range hostIPPrefixMap { - prefix, err := netip.ParsePrefix(prefixStr) - if err != nil { - return err - } - newHosts[host] = prefix - } - *hosts = newHosts - - return nil -} - // IsZero is perhaps a bit naive here. func (pol ACLPolicy) IsZero() bool { if len(pol.Groups) == 0 && len(pol.Hosts) == 0 && len(pol.ACLs) == 0 { @@ -119,7 +97,7 @@ func (pol ACLPolicy) IsZero() bool { return false } -// Returns the list of autoApproving users, groups or tags for a given IPPrefix. +// GetRouteApprovers returns the list of autoApproving users, groups or tags for a given IPPrefix. func (autoApprovers *AutoApprovers) GetRouteApprovers( prefix netip.Prefix, ) ([]string, error) { @@ -127,7 +105,7 @@ func (autoApprovers *AutoApprovers) GetRouteApprovers( return autoApprovers.ExitNode, nil // 0.0.0.0/0, ::/0 or equivalent } - approverAliases := []string{} + approverAliases := make([]string, 0) for autoApprovedPrefix, autoApproverAliases := range autoApprovers.Routes { autoApprovedPrefix, err := netip.ParsePrefix(autoApprovedPrefix) diff --git a/integration/cli_test.go b/integration/cli_test.go index 57edf58ef0d..d78c7b1650c 100644 --- a/integration/cli_test.go +++ b/integration/cli_test.go @@ -7,11 +7,12 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + v1 "github.com/juanfont/headscale/gen/go/headscale/v1" "github.com/juanfont/headscale/hscontrol/policy" "github.com/juanfont/headscale/integration/hsic" "github.com/juanfont/headscale/integration/tsic" - "github.com/stretchr/testify/assert" ) func executeAndUnmarshal[T any](headscale ControlServer, command []string, result T) error {