Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for autoApprovers ACL #763

Merged
merged 13 commits into from
Sep 23, 2022
43 changes: 38 additions & 5 deletions acls_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import (

// 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"`
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"`
}

// ACL is a basic rule for the ACL Policy.
Expand All @@ -42,6 +43,13 @@ type ACLTest struct {
Deny []string `json:"deny,omitempty" yaml:"deny,omitempty"`
}

// AutoApprovers specify which users (namespaces?), 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"`
}

// UnmarshalJSON allows to parse the Hosts directly into netip objects.
func (hosts *Hosts) UnmarshalJSON(data []byte) error {
newHosts := Hosts{}
Expand Down Expand Up @@ -100,3 +108,28 @@ func (policy ACLPolicy) IsZero() bool {

return false
}

// Returns the list of autoApproving namespaces, groups or tags for a given IPPrefix.
func (autoApprovers *AutoApprovers) GetRouteApprovers(
prefix netip.Prefix,
) ([]string, error) {
if prefix.Bits() == 0 {
return autoApprovers.ExitNode, nil // 0.0.0.0/0, ::/0 or equivalent
}

approverAliases := []string{}

for autoApprovedPrefix, autoApproverAliases := range autoApprovers.Routes {
autoApprovedPrefix, err := netip.ParsePrefix(autoApprovedPrefix)
if err != nil {
return nil, err
}

if autoApprovedPrefix.Bits() >= prefix.Bits() &&
Copy link

@samcday samcday Oct 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic might be the wrong way around?

i.e if I have an auto approved prefix of 10.0.0.0/8 and then I try to advertise 10.1.0.0/24, I would expect autoApprovedPrefix.Bits() == 8 and prefix.Bits() == 24, or am I misunderstanding this?

I ask because I tried out the parent prefix thing and it doesn't seem to work for me. For now I'm going to add all 255 /24s I need to my ACL to move on xD. If I get some time later I might try forking and inverting this logic and see if it fixes my issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, bugger but thanks for the find.

@juanfont @kradalby I'll write and submit a PR to fix this with a regression test

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, thanks for the speedy response. And also thanks for shipping this very hype feature :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np, nothing good on tv right now 😂 should be fixed in #862 hopefully

autoApprovedPrefix.Contains(prefix.Masked().Addr()) {
approverAliases = append(approverAliases, autoApproverAliases...)
}
}

return approverAliases, nil
}
58 changes: 58 additions & 0 deletions machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,64 @@ func (h *Headscale) EnableRoutes(machine *Machine, routeStrs ...string) error {
return nil
}

// Enabled any routes advertised by a machine that match the ACL autoApprovers policy.
func (h *Headscale) EnableAutoApprovedRoutes(machine *Machine) {
if len(machine.IPAddresses) == 0 {
return // This machine has no IPAddresses, so can't possibly match any autoApprovers ACLs
}

approvedRoutes := make([]netip.Prefix, 0, len(machine.HostInfo.RoutableIPs))
thisMachine := []Machine{*machine}

for _, advertisedRoute := range machine.HostInfo.RoutableIPs {
if contains(machine.EnabledRoutes, advertisedRoute) {
continue // Skip routes that are already enabled for the node
}

routeApprovers, err := h.aclPolicy.AutoApprovers.GetRouteApprovers(
advertisedRoute,
)
if err != nil {
log.Err(err).
Str("advertisedRoute", advertisedRoute.String()).
Uint64("machineId", machine.ID).
Msg("Failed to resolve autoApprovers for advertised route")

return
}

for _, approvedAlias := range routeApprovers {
if approvedAlias == machine.Namespace.Name {
approvedRoutes = append(approvedRoutes, advertisedRoute)
} else {
approvedIps, err := expandAlias(thisMachine, *h.aclPolicy, approvedAlias, h.cfg.OIDC.StripEmaildomain)
if err != nil {
log.Err(err).
Str("alias", approvedAlias).
Msg("Failed to expand alias when processing autoApprovers policy")

return
}

// approvedIPs should contain all of machine's IPs if it matches the rule, so check for first
if contains(approvedIps, machine.IPAddresses[0].String()) {
approvedRoutes = append(approvedRoutes, advertisedRoute)
}
}
}
}

for _, approvedRoute := range approvedRoutes {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one concern here, but feel free to let me know if I have missed it:

If a node advertise a route, and at the time it is accepted by the ACL autoapprove, what happens if it is removed from the ACL?

I believe the current behaviour would result in the route being left active, which is probably ok since the feature is Auto approve and not implying that it will keep the settings in sync. However we probably should call this out somewhere, not sure where tho.

@juanfont @restanrm

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right - if a route advertised by a node is autoApproved under the current policy, and that policy changes, the route will remain approved. As far as I can gather this is similar to the SaaS offering's behaviour: once a route is auto-approved its up to the user to manage it from that point.

Let me know if there's a good place to docco it - another option could be (if my other preauthkey related PR is merged) writing a doc on how to use both features together and call out this behaviour there?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this resembles the upstream behaviour, which the name implies, that it will be approved and is not "state management", then it is fine.

You can document them together.

Another quick question, if a route is already announced by a node, but not approved, and then the autoApprove rule is added, will it get approved? Basically, is this evaluated when a node joins/sends an update, etc.

if !contains(machine.EnabledRoutes, approvedRoute) {
log.Info().
Str("route", approvedRoute.String()).
Uint64("client", machine.ID).
Msg("Enabling autoApproved route for client")
machine.EnabledRoutes = append(machine.EnabledRoutes, approvedRoute)
}
}
}

func (machine *Machine) RoutesToProto() *v1.Routes {
availableRoutes := machine.GetAdvertisedRoutes()

Expand Down
41 changes: 41 additions & 0 deletions machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1050,3 +1050,44 @@ func TestHeadscale_GenerateGivenName(t *testing.T) {
})
}
}

func (s *Suite) TestAutoApproveRoutes(c *check.C) {
err := app.LoadACLPolicy("./tests/acls/acl_policy_autoapprovers.hujson")
c.Assert(err, check.IsNil)

namespace, err := app.CreateNamespace("test")
c.Assert(err, check.IsNil)

pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil)
c.Assert(err, check.IsNil)

nodeKey := key.NewNode()

defaultRoute := netip.MustParsePrefix("0.0.0.0/0")
route1 := netip.MustParsePrefix("10.10.0.0/16")
route2 := netip.MustParsePrefix("10.11.0.0/16")

machine := Machine{
ID: 0,
MachineKey: "foo",
NodeKey: NodePublicKeyStripPrefix(nodeKey.Public()),
DiscoKey: "faa",
Hostname: "test",
NamespaceID: namespace.ID,
RegisterMethod: RegisterMethodAuthKey,
AuthKeyID: uint(pak.ID),
HostInfo: HostInfo{
RequestTags: []string{"tag:exit"},
RoutableIPs: []netip.Prefix{defaultRoute, route1, route2},
},
IPAddresses: []netip.Addr{netip.MustParseAddr("100.64.0.1")},
}

app.db.Save(&machine)

machine0ByID, err := app.GetMachineByID(0)
c.Assert(err, check.IsNil)

app.EnableAutoApprovedRoutes(machine0ByID)
c.Assert(machine0ByID.GetEnabledRoutes(), check.HasLen, 3)
}
4 changes: 4 additions & 0 deletions protocol_common_poll.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ func (h *Headscale) handlePollCommon(
Str("machine", machine.Hostname).
Err(err)
}

// update routes with peer information
h.EnableAutoApprovedRoutes(machine)
}

// From Tailscale client:
//
// ReadOnly is whether the client just wants to fetch the MapResponse,
Expand Down
24 changes: 24 additions & 0 deletions tests/acls/acl_policy_autoapprovers.hujson
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// This ACL validates autoApprovers support for
// exit nodes and advertised routes

{
"tagOwners": {
"tag:exit": ["test"],
},

"groups": {
"group:test": ["test"]
},

"acls": [
{"action": "accept", "users": ["*"], "ports": ["*:*"]},
],

"autoApprovers": {
"exitNode": ["tag:exit"],
"routes": {
"10.10.0.0/16": ["group:test"],
"10.11.0.0/16": ["test"],
}
}
}