From babf9470c25eb9b8703ccf28c2caa70c0ed2feba Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Thu, 4 Aug 2022 10:42:47 +0200 Subject: [PATCH 1/3] fix(acl): fix issue with groups in excludeCorretlyTaggedNodes This commit fix issue #563 --- acls.go | 5 +++- acls_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/acls.go b/acls.go index b485ce3..426c63d 100644 --- a/acls.go +++ b/acls.go @@ -367,7 +367,7 @@ func expandAlias( // if alias is a namespace nodes := filterMachinesByNamespace(machines, alias) - nodes = excludeCorrectlyTaggedNodes(aclPolicy, nodes, alias) + nodes = excludeCorrectlyTaggedNodes(aclPolicy, nodes, alias, stripEmailDomain) for _, n := range nodes { ips = append(ips, n.IPAddresses.ToStringSlice()...) @@ -405,10 +405,13 @@ func excludeCorrectlyTaggedNodes( aclPolicy ACLPolicy, nodes []Machine, namespace string, + stripEmailDomain bool, ) []Machine { out := []Machine{} tags := []string{} for tag, ns := range aclPolicy.TagOwners { + owners, _ := expandTagOwners(aclPolicy, namespace, stripEmailDomain) + ns = append(owners, namespace) if contains(ns, namespace) { tags = append(tags, tag) } diff --git a/acls_test.go b/acls_test.go index 9e24f99..f8af1b7 100644 --- a/acls_test.go +++ b/acls_test.go @@ -1201,9 +1201,10 @@ func Test_expandAlias(t *testing.T) { func Test_excludeCorrectlyTaggedNodes(t *testing.T) { type args struct { - aclPolicy ACLPolicy - nodes []Machine - namespace string + aclPolicy ACLPolicy + nodes []Machine + namespace string + stripEmailDomain bool } tests := []struct { name string @@ -1247,7 +1248,57 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { Namespace: Namespace{Name: "joe"}, }, }, - namespace: "joe", + namespace: "joe", + stripEmailDomain: true, + }, + want: []Machine{ + { + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, + Namespace: Namespace{Name: "joe"}, + }, + }, + }, + { + name: "exclude nodes with valid tags, and owner is in a group", + args: args{ + aclPolicy: ACLPolicy{ + Groups: Groups{ + "group:accountant": []string{"joe", "bar"}, + }, + TagOwners: TagOwners{"tag:accountant-webserver": []string{"group:accountant"}}, + }, + nodes: []Machine{ + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, + HostInfo: HostInfo{ + OS: "centos", + Hostname: "foo", + RequestTags: []string{"tag:accountant-webserver"}, + }, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "joe"}, + HostInfo: HostInfo{ + OS: "centos", + Hostname: "foo", + RequestTags: []string{"tag:accountant-webserver"}, + }, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "joe"}, + }, + }, + namespace: "joe", + stripEmailDomain: true, }, want: []Machine{ { @@ -1288,7 +1339,8 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { Namespace: Namespace{Name: "joe"}, }, }, - namespace: "joe", + namespace: "joe", + stripEmailDomain: true, }, want: []Machine{ { @@ -1333,7 +1385,8 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { Namespace: Namespace{Name: "joe"}, }, }, - namespace: "joe", + namespace: "joe", + stripEmailDomain: true, }, want: []Machine{ { @@ -1373,6 +1426,7 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { test.args.aclPolicy, test.args.nodes, test.args.namespace, + test.args.stripEmailDomain, ) if !reflect.DeepEqual(got, test.want) { t.Errorf("excludeCorrectlyTaggedNodes() = %v, want %v", got, test.want) From 79688e618732525777a37f47fcfdf2df0800f5f8 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Thu, 4 Aug 2022 10:47:00 +0200 Subject: [PATCH 2/3] chore(all): apply formater --- acls.go | 18 +++++++++++--- acls_test.go | 40 +++++++++++++++++++++++++------ acls_types.go | 10 ++++---- api.go | 11 +++++++-- app.go | 22 ++++++++++++----- cmd/headscale/cli/api_key.go | 4 +++- cmd/headscale/cli/preauthkeys.go | 4 +++- cmd/headscale/cli/utils.go | 5 +++- docs/build-headscale-container.md | 2 ++ grpcv1.go | 2 +- integration_cli_test.go | 8 ++++++- integration_common_test.go | 8 ++++--- integration_embedded_derp_test.go | 8 ++++++- integration_test.go | 8 ++++++- machine_test.go | 12 ++++++++-- oidc.go | 6 ++++- platform_config.go | 7 ++++-- 17 files changed, 137 insertions(+), 38 deletions(-) diff --git a/acls.go b/acls.go index 426c63d..4a1e0cc 100644 --- a/acls.go +++ b/acls.go @@ -162,7 +162,12 @@ func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { destPorts := []tailcfg.NetPortRange{} for innerIndex, dest := range acl.Destinations { - dests, err := h.generateACLPolicyDest(machines, *h.aclPolicy, dest, needsWildcard) + dests, err := h.generateACLPolicyDest( + machines, + *h.aclPolicy, + dest, + needsWildcard, + ) if err != nil { log.Error(). Msgf("Error parsing ACL %d, Destination %d", index, innerIndex) @@ -255,7 +260,12 @@ func (h *Headscale) generateACLPolicyDest( func parseProtocol(protocol string) ([]int, bool, error) { switch protocol { case "": - return []int{protocolICMP, protocolIPv6ICMP, protocolTCP, protocolUDP}, false, nil + return []int{ + protocolICMP, + protocolIPv6ICMP, + protocolTCP, + protocolUDP, + }, false, nil case "igmp": return []int{protocolIGMP}, true, nil case "ipv4", "ip-in-ip": @@ -284,7 +294,9 @@ func parseProtocol(protocol string) ([]int, bool, error) { if err != nil { return nil, false, err } - needsWildcard := protocolNumber != protocolTCP && protocolNumber != protocolUDP && protocolNumber != protocolSCTP + needsWildcard := protocolNumber != protocolTCP && + protocolNumber != protocolUDP && + protocolNumber != protocolSCTP return []int{protocolNumber}, needsWildcard, nil } diff --git a/acls_test.go b/acls_test.go index f8af1b7..fe2217c 100644 --- a/acls_test.go +++ b/acls_test.go @@ -62,7 +62,11 @@ func (s *Suite) TestBasicRule(c *check.C) { func (s *Suite) TestInvalidAction(c *check.C) { app.aclPolicy = &ACLPolicy{ ACLs: []ACL{ - {Action: "invalidAction", Sources: []string{"*"}, Destinations: []string{"*:*"}}, + { + Action: "invalidAction", + Sources: []string{"*"}, + Destinations: []string{"*:*"}, + }, }, } err := app.UpdateACLRules() @@ -77,7 +81,11 @@ func (s *Suite) TestInvalidGroupInGroup(c *check.C) { "group:error": []string{"foo", "group:test"}, }, ACLs: []ACL{ - {Action: "accept", Sources: []string{"group:error"}, Destinations: []string{"*:*"}}, + { + Action: "accept", + Sources: []string{"group:error"}, + Destinations: []string{"*:*"}, + }, }, } err := app.UpdateACLRules() @@ -88,7 +96,11 @@ func (s *Suite) TestInvalidTagOwners(c *check.C) { // this ACL is wrong because no tagOwners own the requested tag for the server app.aclPolicy = &ACLPolicy{ ACLs: []ACL{ - {Action: "accept", Sources: []string{"tag:foo"}, Destinations: []string{"*:*"}}, + { + Action: "accept", + Sources: []string{"tag:foo"}, + Destinations: []string{"*:*"}, + }, }, } err := app.UpdateACLRules() @@ -131,7 +143,11 @@ func (s *Suite) TestValidExpandTagOwnersInSources(c *check.C) { Groups: Groups{"group:test": []string{"user1", "user2"}}, TagOwners: TagOwners{"tag:test": []string{"user3", "group:test"}}, ACLs: []ACL{ - {Action: "accept", Sources: []string{"tag:test"}, Destinations: []string{"*:*"}}, + { + Action: "accept", + Sources: []string{"tag:test"}, + Destinations: []string{"*:*"}, + }, }, } err = app.UpdateACLRules() @@ -177,7 +193,11 @@ func (s *Suite) TestValidExpandTagOwnersInDestinations(c *check.C) { Groups: Groups{"group:test": []string{"user1", "user2"}}, TagOwners: TagOwners{"tag:test": []string{"user3", "group:test"}}, ACLs: []ACL{ - {Action: "accept", Sources: []string{"*"}, Destinations: []string{"tag:test:*"}}, + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"tag:test:*"}, + }, }, } err = app.UpdateACLRules() @@ -222,7 +242,11 @@ func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { app.aclPolicy = &ACLPolicy{ TagOwners: TagOwners{"tag:test": []string{"user1"}}, ACLs: []ACL{ - {Action: "accept", Sources: []string{"user1"}, Destinations: []string{"*:*"}}, + { + Action: "accept", + Sources: []string{"user1"}, + Destinations: []string{"*:*"}, + }, }, } err = app.UpdateACLRules() @@ -1265,7 +1289,9 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { Groups: Groups{ "group:accountant": []string{"joe", "bar"}, }, - TagOwners: TagOwners{"tag:accountant-webserver": []string{"group:accountant"}}, + TagOwners: TagOwners{ + "tag:accountant-webserver": []string{"group:accountant"}, + }, }, nodes: []Machine{ { diff --git a/acls_types.go b/acls_types.go index 1c952e2..0f73d6f 100644 --- a/acls_types.go +++ b/acls_types.go @@ -21,9 +21,9 @@ type ACLPolicy struct { // 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"` + Protocol string `json:"proto" yaml:"proto"` + Sources []string `json:"src" yaml:"src"` + Destinations []string `json:"dst" yaml:"dst"` } // Groups references a series of alias in the ACL rules. @@ -37,8 +37,8 @@ type TagOwners map[string][]string // ACLTest is not implemented, but should be use to check if a certain rule is allowed. type ACLTest struct { - Source string `json:"src" yaml:"src"` - Accept []string `json:"accept" yaml:"accept"` + Source string `json:"src" yaml:"src"` + Accept []string `json:"accept" yaml:"accept"` Deny []string `json:"deny,omitempty" yaml:"deny,omitempty"` } diff --git a/api.go b/api.go index ff0de0c..21b85be 100644 --- a/api.go +++ b/api.go @@ -271,7 +271,8 @@ func (h *Headscale) RegistrationHandler( if machine.NodeKey == NodePublicKeyStripPrefix(registerRequest.NodeKey) { // The client sends an Expiry in the past if the client is requesting to expire the key (aka logout) // https://github.com/tailscale/tailscale/blob/main/tailcfg/tailcfg.go#L648 - if !registerRequest.Expiry.IsZero() && registerRequest.Expiry.UTC().Before(now) { + if !registerRequest.Expiry.IsZero() && + registerRequest.Expiry.UTC().Before(now) { h.handleMachineLogOut(writer, req, machineKey, *machine) return @@ -289,7 +290,13 @@ func (h *Headscale) RegistrationHandler( // The NodeKey we have matches OldNodeKey, which means this is a refresh after a key expiration if machine.NodeKey == NodePublicKeyStripPrefix(registerRequest.OldNodeKey) && !machine.isExpired() { - h.handleMachineRefreshKey(writer, req, machineKey, registerRequest, *machine) + h.handleMachineRefreshKey( + writer, + req, + machineKey, + registerRequest, + *machine, + ) return } diff --git a/app.go b/app.go index bd88ded..84ca86c 100644 --- a/app.go +++ b/app.go @@ -418,16 +418,20 @@ func (h *Headscale) createRouter(grpcMux *runtime.ServeMux) *mux.Router { router.HandleFunc("/health", h.HealthHandler).Methods(http.MethodGet) router.HandleFunc("/key", h.KeyHandler).Methods(http.MethodGet) router.HandleFunc("/register", h.RegisterWebAPI).Methods(http.MethodGet) - router.HandleFunc("/machine/{mkey}/map", h.PollNetMapHandler).Methods(http.MethodPost) + router.HandleFunc("/machine/{mkey}/map", h.PollNetMapHandler). + Methods(http.MethodPost) router.HandleFunc("/machine/{mkey}", h.RegistrationHandler).Methods(http.MethodPost) router.HandleFunc("/oidc/register/{mkey}", h.RegisterOIDC).Methods(http.MethodGet) router.HandleFunc("/oidc/callback", h.OIDCCallback).Methods(http.MethodGet) router.HandleFunc("/apple", h.AppleConfigMessage).Methods(http.MethodGet) - router.HandleFunc("/apple/{platform}", h.ApplePlatformConfig).Methods(http.MethodGet) + router.HandleFunc("/apple/{platform}", h.ApplePlatformConfig). + Methods(http.MethodGet) router.HandleFunc("/windows", h.WindowsConfigMessage).Methods(http.MethodGet) - router.HandleFunc("/windows/tailscale.reg", h.WindowsRegConfig).Methods(http.MethodGet) + router.HandleFunc("/windows/tailscale.reg", h.WindowsRegConfig). + Methods(http.MethodGet) router.HandleFunc("/swagger", SwaggerUI).Methods(http.MethodGet) - router.HandleFunc("/swagger/v1/openapiv2.json", SwaggerAPIv1).Methods(http.MethodGet) + router.HandleFunc("/swagger/v1/openapiv2.json", SwaggerAPIv1). + Methods(http.MethodGet) if h.cfg.DERP.ServerEnabled { router.HandleFunc("/derp", h.DERPHandler) @@ -692,7 +696,10 @@ func (h *Headscale) Serve() error { h.pollNetMapStreamWG.Wait() // Gracefully shut down servers - ctx, cancel := context.WithTimeout(context.Background(), HTTPShutdownTimeout) + ctx, cancel := context.WithTimeout( + context.Background(), + HTTPShutdownTimeout, + ) if err := promHTTPServer.Shutdown(ctx); err != nil { log.Error().Err(err).Msg("Failed to shutdown prometheus http") } @@ -819,7 +826,10 @@ func (h *Headscale) setLastStateChangeToNow(namespaces ...string) { if len(namespaces) == 0 { namespaces, err = h.ListNamespacesStr() if err != nil { - log.Error().Caller().Err(err).Msg("failed to fetch all namespaces, failing to update last changed state.") + log.Error(). + Caller(). + Err(err). + Msg("failed to fetch all namespaces, failing to update last changed state.") } } diff --git a/cmd/headscale/cli/api_key.go b/cmd/headscale/cli/api_key.go index d97cefa..5756db4 100644 --- a/cmd/headscale/cli/api_key.go +++ b/cmd/headscale/cli/api_key.go @@ -134,7 +134,9 @@ If you loose a key, create a new one and revoke (expire) the old one.`, expiration := time.Now().UTC().Add(time.Duration(duration)) - log.Trace().Dur("expiration", time.Duration(duration)).Msg("expiration has been set") + log.Trace(). + Dur("expiration", time.Duration(duration)). + Msg("expiration has been set") request.Expiration = timestamppb.New(expiration) diff --git a/cmd/headscale/cli/preauthkeys.go b/cmd/headscale/cli/preauthkeys.go index ffa1a81..8d8e209 100644 --- a/cmd/headscale/cli/preauthkeys.go +++ b/cmd/headscale/cli/preauthkeys.go @@ -164,7 +164,9 @@ var createPreAuthKeyCmd = &cobra.Command{ expiration := time.Now().UTC().Add(time.Duration(duration)) - log.Trace().Dur("expiration", time.Duration(duration)).Msg("expiration has been set") + log.Trace(). + Dur("expiration", time.Duration(duration)). + Msg("expiration has been set") request.Expiration = timestamppb.New(expiration) diff --git a/cmd/headscale/cli/utils.go b/cmd/headscale/cli/utils.go index c07e3a2..ae89c4f 100644 --- a/cmd/headscale/cli/utils.go +++ b/cmd/headscale/cli/utils.go @@ -24,7 +24,10 @@ const ( func getHeadscaleApp() (*headscale.Headscale, error) { cfg, err := headscale.GetHeadscaleConfig() if err != nil { - return nil, fmt.Errorf("failed to load configuration while creating headscale instance: %w", err) + return nil, fmt.Errorf( + "failed to load configuration while creating headscale instance: %w", + err, + ) } app, err := headscale.NewHeadscale(cfg) diff --git a/docs/build-headscale-container.md b/docs/build-headscale-container.md index b022016..ec0dac5 100644 --- a/docs/build-headscale-container.md +++ b/docs/build-headscale-container.md @@ -5,6 +5,7 @@ The Dockerfiles included in the repository are using the [buildx plugin](https:/ # Build native To build the container on the native arch you can just use: + ``` $ sudo docker buildx build -t headscale:custom-arch . ``` @@ -14,6 +15,7 @@ For example: This will build a amd64(x86_64) container if your hostsystem is amd # Build cross platform To build a arm64 container on a amd64 hostsystem you could use: + ``` $ sudo docker buildx build --platform linux/arm64 -t headscale:custom-arm64 . diff --git a/grpcv1.go b/grpcv1.go index b1e5c1e..452ac21 100644 --- a/grpcv1.go +++ b/grpcv1.go @@ -199,7 +199,7 @@ func (api headscaleV1APIServer) SetTags( err := validateTag(tag) if err != nil { return &v1.SetTagsResponse{ - Machine: nil, + Machine: nil, }, status.Error(codes.InvalidArgument, err.Error()) } } diff --git a/integration_cli_test.go b/integration_cli_test.go index 2f58e71..4ba997d 100644 --- a/integration_cli_test.go +++ b/integration_cli_test.go @@ -70,7 +70,13 @@ func (s *IntegrationCLITestSuite) SetupTest() { err = s.pool.RemoveContainerByName(headscaleHostname) if err != nil { - s.FailNow(fmt.Sprintf("Could not remove existing container before building test: %s", err), "") + s.FailNow( + fmt.Sprintf( + "Could not remove existing container before building test: %s", + err, + ), + "", + ) } fmt.Println("Creating headscale container") diff --git a/integration_common_test.go b/integration_common_test.go index 4ee2d3b..862d02e 100644 --- a/integration_common_test.go +++ b/integration_common_test.go @@ -226,7 +226,6 @@ func getIPs( func getDNSNames( headscale *dockertest.Resource, ) ([]string, error) { - listAllResult, err := ExecuteCommand( headscale, []string{ @@ -260,7 +259,6 @@ func getDNSNames( func getMagicFQDN( headscale *dockertest.Resource, ) ([]string, error) { - listAllResult, err := ExecuteCommand( headscale, []string{ @@ -285,7 +283,11 @@ func getMagicFQDN( hostnames := make([]string, len(listAll)) for index := range listAll { - hostnames[index] = fmt.Sprintf("%s.%s.headscale.net", listAll[index].GetGivenName(), listAll[index].GetNamespace().GetName()) + hostnames[index] = fmt.Sprintf( + "%s.%s.headscale.net", + listAll[index].GetGivenName(), + listAll[index].GetNamespace().GetName(), + ) } return hostnames, nil diff --git a/integration_embedded_derp_test.go b/integration_embedded_derp_test.go index ecca8ba..7872f61 100644 --- a/integration_embedded_derp_test.go +++ b/integration_embedded_derp_test.go @@ -131,7 +131,13 @@ func (s *IntegrationDERPTestSuite) SetupSuite() { err = s.pool.RemoveContainerByName(headscaleHostname) if err != nil { - s.FailNow(fmt.Sprintf("Could not remove existing container before building test: %s", err), "") + s.FailNow( + fmt.Sprintf( + "Could not remove existing container before building test: %s", + err, + ), + "", + ) } log.Println("Creating headscale container") diff --git a/integration_test.go b/integration_test.go index 2214b89..21df45b 100644 --- a/integration_test.go +++ b/integration_test.go @@ -248,7 +248,13 @@ func (s *IntegrationTestSuite) SetupSuite() { err = s.pool.RemoveContainerByName(headscaleHostname) if err != nil { - s.FailNow(fmt.Sprintf("Could not remove existing container before building test: %s", err), "") + s.FailNow( + fmt.Sprintf( + "Could not remove existing container before building test: %s", + err, + ), + "", + ) } log.Println("Creating headscale container") diff --git a/machine_test.go b/machine_test.go index 35c3eed..53d065f 100644 --- a/machine_test.go +++ b/machine_test.go @@ -188,8 +188,16 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { Hosts: map[string]netaddr.IPPrefix{}, TagOwners: map[string][]string{}, ACLs: []ACL{ - {Action: "accept", Sources: []string{"admin"}, Destinations: []string{"*:*"}}, - {Action: "accept", Sources: []string{"test"}, Destinations: []string{"test:*"}}, + { + Action: "accept", + Sources: []string{"admin"}, + Destinations: []string{"*:*"}, + }, + { + Action: "accept", + Sources: []string{"test"}, + Destinations: []string{"test:*"}, + }, }, Tests: []ACLTest{}, } diff --git a/oidc.go b/oidc.go index 8b5f024..d1d6291 100644 --- a/oidc.go +++ b/oidc.go @@ -351,7 +351,11 @@ func (h *Headscale) OIDCCallback( Caller(). Err(err). Msg("Failed to refresh machine") - http.Error(writer, "Failed to refresh machine", http.StatusInternalServerError) + http.Error( + writer, + "Failed to refresh machine", + http.StatusInternalServerError, + ) return } diff --git a/platform_config.go b/platform_config.go index 6bb195c..7bceb0c 100644 --- a/platform_config.go +++ b/platform_config.go @@ -325,7 +325,9 @@ func (h *Headscale) ApplePlatformConfig( default: writer.Header().Set("Content-Type", "text/plain; charset=utf-8") writer.WriteHeader(http.StatusBadRequest) - _, err := writer.Write([]byte("Invalid platform, only ios and macos is supported")) + _, err := writer.Write( + []byte("Invalid platform, only ios and macos is supported"), + ) if err != nil { log.Error(). Caller(). @@ -362,7 +364,8 @@ func (h *Headscale) ApplePlatformConfig( return } - writer.Header().Set("Content-Type", "application/x-apple-aspen-config; charset=utf-8") + writer.Header(). + Set("Content-Type", "application/x-apple-aspen-config; charset=utf-8") writer.WriteHeader(http.StatusOK) _, err = writer.Write(content.Bytes()) if err != nil { From bce59345e42ba7d9e11bba894ff5ce6cc00469a1 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Thu, 4 Aug 2022 10:51:06 +0200 Subject: [PATCH 3/3] docs: add entry in changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9a3a13..bec8dc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## 0.17.0 (2022-xx-xx) +### Changes + +- Fix missing group expansion in function `excludeCorretlyTaggedNodes` [#563](https://github.com/juanfont/headscale/issues/563) + ## 0.16.0 (2022-07-25) **Note:** Take a backup of your database before upgrading.