diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ad7062a..8505e5a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## 0.17.0 (2022-xx-xx) - Updated dependencies (including the library that lacked armhf support) [#722](https://github.com/juanfont/headscale/pull/722) +- Fix missing group expansion in function `excludeCorretlyTaggedNodes` [#563](https://github.com/juanfont/headscale/issues/563) ## 0.16.0 (2022-07-25) diff --git a/acls.go b/acls.go index b485ce30..4a1e0ccf 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 } @@ -367,7 +379,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 +417,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 9e24f99f..fe2217c3 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() @@ -1201,9 +1225,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 +1272,59 @@ 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 +1365,8 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { Namespace: Namespace{Name: "joe"}, }, }, - namespace: "joe", + namespace: "joe", + stripEmailDomain: true, }, want: []Machine{ { @@ -1333,7 +1411,8 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { Namespace: Namespace{Name: "joe"}, }, }, - namespace: "joe", + namespace: "joe", + stripEmailDomain: true, }, want: []Machine{ { @@ -1373,6 +1452,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) diff --git a/acls_types.go b/acls_types.go index 1c952e22..0f73d6fd 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 ff0de0c4..21b85be4 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 bd88dedf..84ca86c0 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 d97cefa9..5756db48 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 ffa1a81d..8d8e2093 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 c07e3a25..ae89c4fe 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/grpcv1.go b/grpcv1.go index b1e5c1ee..452ac21d 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 bc7d1c1e..f2066021 100644 --- a/integration_cli_test.go +++ b/integration_cli_test.go @@ -69,7 +69,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 for CLI tests") diff --git a/integration_common_test.go b/integration_common_test.go index b0c3dbb8..0235f25b 100644 --- a/integration_common_test.go +++ b/integration_common_test.go @@ -227,7 +227,6 @@ func getIPs( func getDNSNames( headscale *dockertest.Resource, ) ([]string, error) { - listAllResult, err := ExecuteCommand( headscale, []string{ @@ -261,7 +260,6 @@ func getDNSNames( func getMagicFQDN( headscale *dockertest.Resource, ) ([]string, error) { - listAllResult, err := ExecuteCommand( headscale, []string{ @@ -286,7 +284,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 29f6ea1d..37ce82c8 100644 --- a/integration_embedded_derp_test.go +++ b/integration_embedded_derp_test.go @@ -129,7 +129,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 for DERP integration tests") diff --git a/integration_general_test.go b/integration_general_test.go index 8ee52651..d4c64c38 100644 --- a/integration_general_test.go +++ b/integration_general_test.go @@ -246,7 +246,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 for core integration tests") diff --git a/machine_test.go b/machine_test.go index 35c3eed9..53d065ff 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 d995c976..4c1bf5a7 100644 --- a/oidc.go +++ b/oidc.go @@ -474,7 +474,11 @@ func (h *Headscale) validateMachineForOIDCCallback( 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 nil, true, err } diff --git a/platform_config.go b/platform_config.go index 6bb195c7..7bceb0c9 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 {