From d7a503a34effa188e9bb27cb6b0fad2002112fb0 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 14 May 2025 17:32:56 +0300 Subject: [PATCH 1/8] changelog: entry for 0.26 (#2594) * changelog: entry for 0.26 Signed-off-by: Kristoffer Dalby * docs: bump version Signed-off-by: Kristoffer Dalby --------- Signed-off-by: Kristoffer Dalby --- CHANGELOG.md | 16 +++++++++------- mkdocs.yml | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2076acc4..6bca556d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Next +## 0.26.0 (2025-05-14) + ### BREAKING #### Routes @@ -69,17 +71,17 @@ new policy code passes all of our tests. Migration notes when the policy is stored in the database. This section **only** applies if the policy is stored in the database and -Headscale 0.26 doesn't start due to a policy error (`failed to load ACL -policy`). +Headscale 0.26 doesn't start due to a policy error +(`failed to load ACL policy`). -* Start Headscale 0.26 with the environment variable `HEADSCALE_POLICY_V1=1` +- Start Headscale 0.26 with the environment variable `HEADSCALE_POLICY_V1=1` set. You can check that Headscale picked up the environment variable by observing this message during startup: `Using policy manager version: 1` -* Dump the policy to a file: `headscale policy get > policy.json` -* Edit `policy.json` and migrate to policy V2. Use the command +- Dump the policy to a file: `headscale policy get > policy.json` +- Edit `policy.json` and migrate to policy V2. Use the command `headscale policy check --file policy.json` to check for policy errors. -* Load the modified policy: `headscale policy set --file policy.json` -* Restart Headscale **without** the environment variable `HEADSCALE_POLICY_V1`. +- Load the modified policy: `headscale policy set --file policy.json` +- Restart Headscale **without** the environment variable `HEADSCALE_POLICY_V1`. Headscale should now print the message `Using policy manager version: 2` and startup successfully. diff --git a/mkdocs.yml b/mkdocs.yml index dec10d34..84fe2e1c 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -107,7 +107,7 @@ extra: - icon: fontawesome/brands/discord link: https://discord.gg/c84AZQhmpx headscale: - version: 0.25.0 + version: 0.26.0 # Extensions markdown_extensions: From 2dc2f3b3f0e0efcb5257379dec7ad1b4b09f8945 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 14 May 2025 17:45:14 +0300 Subject: [PATCH 2/8] users: harden, test, and add cleaner of identifier (#2593) * users: harden, test, and add cleaner of identifier Signed-off-by: Kristoffer Dalby * db: migrate badly joined provider identifiers Signed-off-by: Kristoffer Dalby --------- Signed-off-by: Kristoffer Dalby --- hscontrol/db/db.go | 23 ++++ hscontrol/types/users.go | 113 +++++++++++++++++- hscontrol/types/users_test.go | 213 ++++++++++++++++++++++++++++++++++ 3 files changed, 344 insertions(+), 5 deletions(-) diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go index 74f51ddc..bab0061e 100644 --- a/hscontrol/db/db.go +++ b/hscontrol/db/db.go @@ -695,6 +695,29 @@ AND auth_key_id NOT IN ( }, Rollback: func(db *gorm.DB) error { return nil }, }, + // Fix the provider identifier for users that have a double slash in the + // provider identifier. + { + ID: "202505141324", + Migrate: func(tx *gorm.DB) error { + users, err := ListUsers(tx) + if err != nil { + return fmt.Errorf("listing users: %w", err) + } + + for _, user := range users { + user.ProviderIdentifier.String = types.CleanIdentifier(user.ProviderIdentifier.String) + + err := tx.Save(user).Error + if err != nil { + return fmt.Errorf("saving user: %w", err) + } + } + + return nil + }, + Rollback: func(db *gorm.DB) error { return nil }, + }, }, ) diff --git a/hscontrol/types/users.go b/hscontrol/types/users.go index 471cb1e5..6cd2c41a 100644 --- a/hscontrol/types/users.go +++ b/hscontrol/types/users.go @@ -194,13 +194,110 @@ type OIDCClaims struct { Username string `json:"preferred_username,omitempty"` } +// Identifier returns a unique identifier string combining the Iss and Sub claims. +// The format depends on whether Iss is a URL or not: +// - For URLs: Joins the URL and sub path (e.g., "https://example.com/sub") +// - For non-URLs: Joins with a slash (e.g., "oidc/sub") +// - For empty Iss: Returns just "sub" +// - For empty Sub: Returns just the Issuer +// - For both empty: Returns empty string +// +// The result is cleaned using CleanIdentifier() to ensure consistent formatting. func (c *OIDCClaims) Identifier() string { - if strings.HasPrefix(c.Iss, "http") { - if i, err := url.JoinPath(c.Iss, c.Sub); err == nil { - return i + // Handle empty components special cases + if c.Iss == "" && c.Sub == "" { + return "" + } + if c.Iss == "" { + return CleanIdentifier(c.Sub) + } + if c.Sub == "" { + return CleanIdentifier(c.Iss) + } + + // We'll use the raw values and let CleanIdentifier handle all the whitespace + issuer := c.Iss + subject := c.Sub + + var result string + // Try to parse as URL to handle URL joining correctly + if u, err := url.Parse(issuer); err == nil && u.Scheme != "" { + // For URLs, use proper URL path joining + if joined, err := url.JoinPath(issuer, subject); err == nil { + result = joined } } - return c.Iss + "/" + c.Sub + + // If URL joining failed or issuer wasn't a URL, do simple string join + if result == "" { + // Default case: simple string joining with slash + issuer = strings.TrimSuffix(issuer, "/") + subject = strings.TrimPrefix(subject, "/") + result = issuer + "/" + subject + } + + // Clean the result and return it + return CleanIdentifier(result) +} + +// CleanIdentifier cleans a potentially malformed identifier by removing double slashes +// while preserving protocol specifications like http://. This function will: +// - Trim all whitespace from the beginning and end of the identifier +// - Remove whitespace within path segments +// - Preserve the scheme (http://, https://, etc.) for URLs +// - Remove any duplicate slashes in the path +// - Remove empty path segments +// - For non-URL identifiers, it joins non-empty segments with a single slash +// - Returns empty string for identifiers with only slashes +// - Normalize URL schemes to lowercase +func CleanIdentifier(identifier string) string { + if identifier == "" { + return identifier + } + + // Trim leading/trailing whitespace + identifier = strings.TrimSpace(identifier) + + // Handle URLs with schemes + u, err := url.Parse(identifier) + if err == nil && u.Scheme != "" { + // Clean path by removing empty segments and whitespace within segments + parts := strings.FieldsFunc(u.Path, func(c rune) bool { return c == '/' }) + for i, part := range parts { + parts[i] = strings.TrimSpace(part) + } + // Remove empty parts after trimming + cleanParts := make([]string, 0, len(parts)) + for _, part := range parts { + if part != "" { + cleanParts = append(cleanParts, part) + } + } + + if len(cleanParts) == 0 { + u.Path = "" + } else { + u.Path = "/" + strings.Join(cleanParts, "/") + } + // Ensure scheme is lowercase + u.Scheme = strings.ToLower(u.Scheme) + return u.String() + } + + // Handle non-URL identifiers + parts := strings.FieldsFunc(identifier, func(c rune) bool { return c == '/' }) + // Clean whitespace from each part + cleanParts := make([]string, 0, len(parts)) + for _, part := range parts { + trimmed := strings.TrimSpace(part) + if trimmed != "" { + cleanParts = append(cleanParts, trimmed) + } + } + if len(cleanParts) == 0 { + return "" + } + return strings.Join(cleanParts, "/") } type OIDCUserInfo struct { @@ -231,7 +328,13 @@ func (u *User) FromClaim(claims *OIDCClaims) { } } - u.ProviderIdentifier = sql.NullString{String: claims.Identifier(), Valid: true} + // Get provider identifier + identifier := claims.Identifier() + // Ensure provider identifier always has a leading slash for backward compatibility + if claims.Iss == "" && !strings.HasPrefix(identifier, "/") { + identifier = "/" + identifier + } + u.ProviderIdentifier = sql.NullString{String: identifier, Valid: true} u.DisplayName = claims.Name u.ProfilePicURL = claims.ProfilePictureURL u.Provider = util.RegisterMethodOIDC diff --git a/hscontrol/types/users_test.go b/hscontrol/types/users_test.go index 12029701..f36489a3 100644 --- a/hscontrol/types/users_test.go +++ b/hscontrol/types/users_test.go @@ -7,6 +7,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/juanfont/headscale/hscontrol/util" + "github.com/stretchr/testify/assert" ) func TestUnmarshallOIDCClaims(t *testing.T) { @@ -76,6 +77,218 @@ func TestUnmarshallOIDCClaims(t *testing.T) { } } +func TestOIDCClaimsIdentifier(t *testing.T) { + tests := []struct { + name string + iss string + sub string + expected string + }{ + { + name: "standard URL with trailing slash", + iss: "https://oidc.example.com/", + sub: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", + expected: "https://oidc.example.com/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", + }, + { + name: "standard URL without trailing slash", + iss: "https://oidc.example.com", + sub: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", + expected: "https://oidc.example.com/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", + }, + { + name: "standard URL with uppercase protocol", + iss: "HTTPS://oidc.example.com/", + sub: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", + expected: "https://oidc.example.com/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", + }, + { + name: "standard URL with path and trailing slash", + iss: "https://login.microsoftonline.com/v2.0/", + sub: "I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", + expected: "https://login.microsoftonline.com/v2.0/I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", + }, + { + name: "standard URL with path without trailing slash", + iss: "https://login.microsoftonline.com/v2.0", + sub: "I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", + expected: "https://login.microsoftonline.com/v2.0/I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", + }, + { + name: "non-URL identifier with slash", + iss: "oidc", + sub: "sub", + expected: "oidc/sub", + }, + { + name: "non-URL identifier with trailing slash", + iss: "oidc/", + sub: "sub", + expected: "oidc/sub", + }, + { + name: "subject with slash", + iss: "oidc/", + sub: "sub/", + expected: "oidc/sub", + }, + { + name: "whitespace", + iss: " oidc/ ", + sub: " sub ", + expected: "oidc/sub", + }, + { + name: "newline", + iss: "\noidc/\n", + sub: "\nsub\n", + expected: "oidc/sub", + }, + { + name: "tab", + iss: "\toidc/\t", + sub: "\tsub\t", + expected: "oidc/sub", + }, + { + name: "empty issuer", + iss: "", + sub: "sub", + expected: "sub", + }, + { + name: "empty subject", + iss: "https://oidc.example.com", + sub: "", + expected: "https://oidc.example.com", + }, + { + name: "both empty", + iss: "", + sub: "", + expected: "", + }, + { + name: "URL with double slash", + iss: "https://login.microsoftonline.com//v2.0", + sub: "I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", + expected: "https://login.microsoftonline.com/v2.0/I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", + }, + { + name: "FTP URL protocol", + iss: "ftp://example.com/directory", + sub: "resource", + expected: "ftp://example.com/directory/resource", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + claims := OIDCClaims{ + Iss: tt.iss, + Sub: tt.sub, + } + result := claims.Identifier() + assert.Equal(t, tt.expected, result) + if diff := cmp.Diff(tt.expected, result); diff != "" { + t.Errorf("Identifier() mismatch (-want +got):\n%s", diff) + } + + // Now clean the identifier and verify it's still the same + cleaned := CleanIdentifier(result) + + // Double-check with cmp.Diff for better error messages + if diff := cmp.Diff(tt.expected, cleaned); diff != "" { + t.Errorf("CleanIdentifier(Identifier()) mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestCleanIdentifier(t *testing.T) { + tests := []struct { + name string + identifier string + expected string + }{ + { + name: "empty identifier", + identifier: "", + expected: "", + }, + { + name: "simple identifier", + identifier: "oidc/sub", + expected: "oidc/sub", + }, + { + name: "double slashes in the middle", + identifier: "oidc//sub", + expected: "oidc/sub", + }, + { + name: "trailing slash", + identifier: "oidc/sub/", + expected: "oidc/sub", + }, + { + name: "multiple double slashes", + identifier: "oidc//sub///id//", + expected: "oidc/sub/id", + }, + { + name: "HTTP URL with proper scheme", + identifier: "http://example.com/path", + expected: "http://example.com/path", + }, + { + name: "HTTP URL with double slashes in path", + identifier: "http://example.com//path///resource", + expected: "http://example.com/path/resource", + }, + { + name: "HTTPS URL with empty segments", + identifier: "https://example.com///path//", + expected: "https://example.com/path", + }, + { + name: "URL with double slashes in domain", + identifier: "https://login.microsoftonline.com//v2.0/I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", + expected: "https://login.microsoftonline.com/v2.0/I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", + }, + { + name: "FTP URL with double slashes", + identifier: "ftp://example.com//resource//", + expected: "ftp://example.com/resource", + }, + { + name: "Just slashes", + identifier: "///", + expected: "", + }, + { + name: "Leading slash without URL", + identifier: "/path//to///resource", + expected: "path/to/resource", + }, + { + name: "Non-standard protocol", + identifier: "ldap://example.org//path//to//resource", + expected: "ldap://example.org/path/to/resource", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CleanIdentifier(tt.identifier) + assert.Equal(t, tt.expected, result) + if diff := cmp.Diff(tt.expected, result); diff != "" { + t.Errorf("CleanIdentifier() mismatch (-want +got):\n%s", diff) + } + }) + } +} + func TestOIDCClaimsJSONToUser(t *testing.T) { tests := []struct { name string From 30525cee0eb14de0d587b1b4168cbf8ee462b99e Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 16 May 2025 11:23:22 +0300 Subject: [PATCH 3/8] goreleaser: always do draft (#2595) Signed-off-by: Kristoffer Dalby --- .goreleaser.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.goreleaser.yml b/.goreleaser.yml index 45eb6e01..ee83cd21 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -7,6 +7,7 @@ before: release: prerelease: auto + draft: true builds: - id: headscale From bd6ed80936d950a1e650af43125928fea74301f3 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 16 May 2025 17:30:47 +0200 Subject: [PATCH 4/8] policy/v2: error on missing or zero port (#2606) * policy/v2: error on missing or zero port Fixes #2605 Signed-off-by: Kristoffer Dalby * changelog: add entry Signed-off-by: Kristoffer Dalby --------- Signed-off-by: Kristoffer Dalby --- CHANGELOG.md | 5 ++++ hscontrol/policy/v2/types.go | 3 +++ hscontrol/policy/v2/types_test.go | 38 +++++++++++++++++++++++++++++++ hscontrol/policy/v2/utils.go | 4 ++++ 4 files changed, 50 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bca556d..e6645ec5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Next +### BREAKING + +- Policy: Zero or empty destination port is no longer allowed + [#2606](https://github.com/juanfont/headscale/pull/2606) + ## 0.26.0 (2025-05-14) ### BREAKING diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index a49f55de..d10136a0 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -3,6 +3,7 @@ package v2 import ( "bytes" "encoding/json" + "errors" "fmt" "net/netip" "strings" @@ -467,6 +468,8 @@ func (ve *AliasWithPorts) UnmarshalJSON(b []byte) error { return err } ve.Ports = ports + } else { + return errors.New(`hostport must contain a colon (":")`) } ve.Alias, err = parseAlias(vs) diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 3808b547..e9fa6263 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -706,6 +706,44 @@ func TestUnmarshalPolicy(t *testing.T) { `, wantErr: `Tag "tag:notdefined" is not defined in the Policy, please define or remove the reference to it`, }, + { + name: "missing-dst-port-is-err", + input: ` + { + "acls": [ + { + "action": "accept", + "src": [ + "*" + ], + "dst": [ + "100.64.0.1" + ] + } + ] +} +`, + wantErr: `hostport must contain a colon (":")`, + }, + { + name: "dst-port-zero-is-err", + input: ` + { + "acls": [ + { + "action": "accept", + "src": [ + "*" + ], + "dst": [ + "100.64.0.1:0" + ] + } + ] +} +`, + wantErr: `first port must be >0, or use '*' for wildcard`, + }, } cmps := append(util.Comparers, cmp.Comparer(func(x, y Prefix) bool { diff --git a/hscontrol/policy/v2/utils.go b/hscontrol/policy/v2/utils.go index 9c962af8..2c551eda 100644 --- a/hscontrol/policy/v2/utils.go +++ b/hscontrol/policy/v2/utils.go @@ -73,6 +73,10 @@ func parsePortRange(portDef string) ([]tailcfg.PortRange, error) { return nil, err } + if port < 1 { + return nil, errors.New("first port must be >0, or use '*' for wildcard") + } + portRanges = append(portRanges, tailcfg.PortRange{First: port, Last: port}) } } From 49b3468845576c3db0970edbb9ac2a9be78ee576 Mon Sep 17 00:00:00 2001 From: Florian Preinstorfer Date: Wed, 14 May 2025 09:21:30 +0200 Subject: [PATCH 5/8] Do not ignore config-example.yml Various tools (e.g ripgrep) skip files ignored by Git. Do not ignore config-example.yml to include it in searches. --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 1662d7f2..2ea56ad7 100644 --- a/.gitignore +++ b/.gitignore @@ -20,9 +20,9 @@ vendor/ dist/ /headscale -config.json config.yaml config*.yaml +!config-example.yaml derp.yaml *.hujson *.key From c15aa541bb9e8d834a3e2d9c12d8edacae7d8502 Mon Sep 17 00:00:00 2001 From: Florian Preinstorfer Date: Wed, 14 May 2025 20:12:37 +0200 Subject: [PATCH 6/8] Document HEADSCALE_CONFIG --- docs/ref/configuration.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/ref/configuration.md b/docs/ref/configuration.md index e11710db..18c8502f 100644 --- a/docs/ref/configuration.md +++ b/docs/ref/configuration.md @@ -5,7 +5,9 @@ - `/etc/headscale` - `$HOME/.headscale` - the current working directory -- Use the command line flag `-c`, `--config` to load the configuration from a different path +- To load the configuration from a different path, use: + - the command line flag `-c`, `--config` + - the environment variable `HEADSCALE_CONFIG` - Validate the configuration file with: `headscale configtest` !!! example "Get the [example configuration from the GitHub repository](https://github.com/juanfont/headscale/blob/main/config-example.yaml)" From b50e10a1be94898e3f237fee35e9f57eea06eda8 Mon Sep 17 00:00:00 2001 From: Florian Preinstorfer Date: Thu, 15 May 2025 07:15:54 +0200 Subject: [PATCH 7/8] Document breaking change for dns.override_local_dns See: #2438 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6645ec5..bfe63f3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,11 @@ working in v1 and not tested might be broken in v2 (and vice versa). [#2542](https://github.com/juanfont/headscale/pull/2542) - Pre auth key API/CLI now uses ID over username [#2542](https://github.com/juanfont/headscale/pull/2542) +- A non-empty list of global nameservers needs to be specified via + `dns.nameservers.global` if the configuration option `dns.override_local_dns` + is enabled or is not specified in the configuration file. This aligns with + behaviour of tailscale.com. + [#2438](https://github.com/juanfont/headscale/pull/2438) ### Changes From 6750414db116aa4b0241c07b2d8f52f13ba045fe Mon Sep 17 00:00:00 2001 From: Vitalij Dovhanyc <45185420+vdovhanych@users.noreply.github.com> Date: Sat, 17 May 2025 11:07:34 +0200 Subject: [PATCH 8/8] feat: add autogroup:member, autogroup:tagged (#2572) --- .github/workflows/test-integration.yaml | 2 + CHANGELOG.md | 2 + docs/about/features.md | 2 +- hscontrol/policy/v2/types.go | 93 ++++++++++++++--- hscontrol/policy/v2/types_test.go | 133 +++++++++++++++++++++++- integration/acl_test.go | 112 ++++++++++++++++++++ 6 files changed, 329 insertions(+), 15 deletions(-) diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index 3c8141c7..61213ea6 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -22,6 +22,8 @@ jobs: - TestACLNamedHostsCanReach - TestACLDevice1CanAccessDevice2 - TestPolicyUpdateWhileRunningWithCLIInDatabase + - TestACLAutogroupMember + - TestACLAutogroupTagged - TestAuthKeyLogoutAndReloginSameUser - TestAuthKeyLogoutAndReloginNewUser - TestAuthKeyLogoutAndReloginSameUserExpiredKey diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe63f3e..91a23a05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -155,6 +155,8 @@ working in v1 and not tested might be broken in v2 (and vice versa). [#2438](https://github.com/juanfont/headscale/pull/2438) - Add documentation for routes [#2496](https://github.com/juanfont/headscale/pull/2496) +- Add support for `autogroup:member`, `autogroup:tagged` + [#2572](https://github.com/juanfont/headscale/pull/2572) ## 0.25.1 (2025-02-25) diff --git a/docs/about/features.md b/docs/about/features.md index 22a4be62..3ee913db 100644 --- a/docs/about/features.md +++ b/docs/about/features.md @@ -23,7 +23,7 @@ provides on overview of Headscale's feature and compatibility with the Tailscale - [x] Access control lists ([GitHub label "policy"](https://github.com/juanfont/headscale/labels/policy%20%F0%9F%93%9D)) - [x] ACL management via API - [x] Some [Autogroups](https://tailscale.com/kb/1396/targets#autogroups), currently: `autogroup:internet`, - `autogroup:nonroot` + `autogroup:nonroot`, `autogroup:member`, `autogroup:tagged` - [x] [Auto approvers](https://tailscale.com/kb/1337/acl-syntax#auto-approvers) for [subnet routers](../ref/routes.md#automatically-approve-routes-of-a-subnet-router) and [exit nodes](../ref/routes.md#automatically-approve-an-exit-node-with-auto-approvers) diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index d10136a0..580a1980 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -384,15 +384,20 @@ type AutoGroup string const ( AutoGroupInternet AutoGroup = "autogroup:internet" + AutoGroupMember AutoGroup = "autogroup:member" AutoGroupNonRoot AutoGroup = "autogroup:nonroot" + AutoGroupTagged AutoGroup = "autogroup:tagged" // These are not yet implemented. - AutoGroupSelf AutoGroup = "autogroup:self" - AutoGroupMember AutoGroup = "autogroup:member" - AutoGroupTagged AutoGroup = "autogroup:tagged" + AutoGroupSelf AutoGroup = "autogroup:self" ) -var autogroups = []AutoGroup{AutoGroupInternet} +var autogroups = []AutoGroup{ + AutoGroupInternet, + AutoGroupMember, + AutoGroupNonRoot, + AutoGroupTagged, +} func (ag AutoGroup) Validate() error { if slices.Contains(autogroups, ag) { @@ -410,13 +415,76 @@ func (ag *AutoGroup) UnmarshalJSON(b []byte) error { return nil } -func (ag AutoGroup) Resolve(_ *Policy, _ types.Users, _ types.Nodes) (*netipx.IPSet, error) { +func (ag AutoGroup) Resolve(p *Policy, users types.Users, nodes types.Nodes) (*netipx.IPSet, error) { + var build netipx.IPSetBuilder + switch ag { case AutoGroupInternet: return util.TheInternet(), nil - } - return nil, nil + case AutoGroupMember: + // autogroup:member represents all untagged devices in the tailnet. + tagMap, err := resolveTagOwners(p, users, nodes) + if err != nil { + return nil, err + } + + for _, node := range nodes { + // Skip if node has forced tags + if len(node.ForcedTags) != 0 { + continue + } + + // Skip if node has any allowed requested tags + hasAllowedTag := false + if node.Hostinfo != nil && len(node.Hostinfo.RequestTags) != 0 { + for _, tag := range node.Hostinfo.RequestTags { + if tagips, ok := tagMap[Tag(tag)]; ok && node.InIPSet(tagips) { + hasAllowedTag = true + break + } + } + } + if hasAllowedTag { + continue + } + + // Node is a member if it has no forced tags and no allowed requested tags + node.AppendToIPSet(&build) + } + + return build.IPSet() + + case AutoGroupTagged: + // autogroup:tagged represents all devices with a tag in the tailnet. + tagMap, err := resolveTagOwners(p, users, nodes) + if err != nil { + return nil, err + } + + for _, node := range nodes { + // Include if node has forced tags + if len(node.ForcedTags) != 0 { + node.AppendToIPSet(&build) + continue + } + + // Include if node has any allowed requested tags + if node.Hostinfo != nil && len(node.Hostinfo.RequestTags) != 0 { + for _, tag := range node.Hostinfo.RequestTags { + if _, ok := tagMap[Tag(tag)]; ok { + node.AppendToIPSet(&build) + break + } + } + } + } + + return build.IPSet() + + default: + return nil, fmt.Errorf("unknown autogroup %q", ag) + } } func (ag *AutoGroup) Is(c AutoGroup) bool { @@ -952,12 +1020,13 @@ type Policy struct { } var ( - autogroupForSrc = []AutoGroup{} - autogroupForDst = []AutoGroup{AutoGroupInternet} - autogroupForSSHSrc = []AutoGroup{} - autogroupForSSHDst = []AutoGroup{} + // TODO(kradalby): Add these checks for tagOwners and autoApprovers + autogroupForSrc = []AutoGroup{AutoGroupMember, AutoGroupTagged} + autogroupForDst = []AutoGroup{AutoGroupInternet, AutoGroupMember, AutoGroupTagged} + autogroupForSSHSrc = []AutoGroup{AutoGroupMember, AutoGroupTagged} + autogroupForSSHDst = []AutoGroup{AutoGroupMember, AutoGroupTagged} autogroupForSSHUser = []AutoGroup{AutoGroupNonRoot} - autogroupNotSupported = []AutoGroup{AutoGroupSelf, AutoGroupMember, AutoGroupTagged} + autogroupNotSupported = []AutoGroup{AutoGroupSelf} ) func validateAutogroupSupported(ag *AutoGroup) error { diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index e9fa6263..3e9de7d7 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -359,7 +359,7 @@ func TestUnmarshalPolicy(t *testing.T) { ], } `, - wantErr: `AutoGroup is invalid, got: "autogroup:invalid", must be one of [autogroup:internet]`, + wantErr: `AutoGroup is invalid, got: "autogroup:invalid", must be one of [autogroup:internet autogroup:member autogroup:nonroot autogroup:tagged]`, }, { name: "undefined-hostname-errors-2490", @@ -998,6 +998,135 @@ func TestResolvePolicy(t *testing.T) { toResolve: Wildcard, want: []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6()}, }, + { + name: "autogroup-member-comprehensive", + toResolve: ptr.To(AutoGroup(AutoGroupMember)), + nodes: types.Nodes{ + // Node with no tags (should be included) + { + User: users["testuser"], + IPv4: ap("100.100.101.1"), + }, + // Node with forced tags (should be excluded) + { + User: users["testuser"], + ForcedTags: []string{"tag:test"}, + IPv4: ap("100.100.101.2"), + }, + // Node with allowed requested tag (should be excluded) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:test"}, + }, + IPv4: ap("100.100.101.3"), + }, + // Node with non-allowed requested tag (should be included) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:notallowed"}, + }, + IPv4: ap("100.100.101.4"), + }, + // Node with multiple requested tags, one allowed (should be excluded) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:test", "tag:notallowed"}, + }, + IPv4: ap("100.100.101.5"), + }, + // Node with multiple requested tags, none allowed (should be included) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:notallowed1", "tag:notallowed2"}, + }, + IPv4: ap("100.100.101.6"), + }, + }, + pol: &Policy{ + TagOwners: TagOwners{ + Tag("tag:test"): Owners{ptr.To(Username("testuser@"))}, + }, + }, + want: []netip.Prefix{ + mp("100.100.101.1/32"), // No tags + mp("100.100.101.4/32"), // Non-allowed requested tag + mp("100.100.101.6/32"), // Multiple non-allowed requested tags + }, + }, + { + name: "autogroup-tagged", + toResolve: ptr.To(AutoGroup(AutoGroupTagged)), + nodes: types.Nodes{ + // Node with no tags (should be excluded) + { + User: users["testuser"], + IPv4: ap("100.100.101.1"), + }, + // Node with forced tag (should be included) + { + User: users["testuser"], + ForcedTags: []string{"tag:test"}, + IPv4: ap("100.100.101.2"), + }, + // Node with allowed requested tag (should be included) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:test"}, + }, + IPv4: ap("100.100.101.3"), + }, + // Node with non-allowed requested tag (should be excluded) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:notallowed"}, + }, + IPv4: ap("100.100.101.4"), + }, + // Node with multiple requested tags, one allowed (should be included) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:test", "tag:notallowed"}, + }, + IPv4: ap("100.100.101.5"), + }, + // Node with multiple requested tags, none allowed (should be excluded) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:notallowed1", "tag:notallowed2"}, + }, + IPv4: ap("100.100.101.6"), + }, + // Node with multiple forced tags (should be included) + { + User: users["testuser"], + ForcedTags: []string{"tag:test", "tag:other"}, + IPv4: ap("100.100.101.7"), + }, + }, + pol: &Policy{ + TagOwners: TagOwners{ + Tag("tag:test"): Owners{ptr.To(Username("testuser@"))}, + }, + }, + want: []netip.Prefix{ + mp("100.100.101.2/31"), // Forced tag and allowed requested tag consecutive IPs are put in 31 prefix + mp("100.100.101.5/32"), // Multiple requested tags, one allowed + mp("100.100.101.7/32"), // Multiple forced tags + }, + }, + { + name: "autogroup-invalid", + toResolve: ptr.To(AutoGroup("autogroup:invalid")), + wantErr: "unknown autogroup", + }, } for _, tt := range tests { @@ -1161,7 +1290,7 @@ func TestResolveAutoApprovers(t *testing.T) { name: "mixed-routes-and-exit-nodes", policy: &Policy{ Groups: Groups{ - "group:testgroup": Usernames{"user1", "user2"}, + "group:testgroup": Usernames{"user1@", "user2@"}, }, AutoApprovers: AutoApproverPolicy{ Routes: map[netip.Prefix]AutoApprovers{ diff --git a/integration/acl_test.go b/integration/acl_test.go index bb18b3b3..116f298d 100644 --- a/integration/acl_test.go +++ b/integration/acl_test.go @@ -1139,3 +1139,115 @@ func TestPolicyUpdateWhileRunningWithCLIInDatabase(t *testing.T) { } } } + +func TestACLAutogroupMember(t *testing.T) { + IntegrationSkip(t) + t.Parallel() + + scenario := aclScenario(t, + &policyv1.ACLPolicy{ + ACLs: []policyv1.ACL{ + { + Action: "accept", + Sources: []string{"autogroup:member"}, + Destinations: []string{"autogroup:member:*"}, + }, + }, + }, + 2, + ) + defer scenario.ShutdownAssertNoPanics(t) + + allClients, err := scenario.ListTailscaleClients() + require.NoError(t, err) + + err = scenario.WaitForTailscaleSync() + require.NoError(t, err) + + // Test that untagged nodes can access each other + for _, client := range allClients { + status, err := client.Status() + require.NoError(t, err) + if status.Self.Tags != nil && status.Self.Tags.Len() > 0 { + continue + } + + for _, peer := range allClients { + if client.Hostname() == peer.Hostname() { + continue + } + + status, err := peer.Status() + require.NoError(t, err) + if status.Self.Tags != nil && status.Self.Tags.Len() > 0 { + continue + } + + fqdn, err := peer.FQDN() + require.NoError(t, err) + + url := fmt.Sprintf("http://%s/etc/hostname", fqdn) + t.Logf("url from %s to %s", client.Hostname(), url) + + result, err := client.Curl(url) + assert.Len(t, result, 13) + require.NoError(t, err) + } + } +} + +func TestACLAutogroupTagged(t *testing.T) { + IntegrationSkip(t) + t.Parallel() + + scenario := aclScenario(t, + &policyv1.ACLPolicy{ + ACLs: []policyv1.ACL{ + { + Action: "accept", + Sources: []string{"autogroup:tagged"}, + Destinations: []string{"autogroup:tagged:*"}, + }, + }, + }, + 2, + ) + defer scenario.ShutdownAssertNoPanics(t) + + allClients, err := scenario.ListTailscaleClients() + require.NoError(t, err) + + err = scenario.WaitForTailscaleSync() + require.NoError(t, err) + + // Test that tagged nodes can access each other + for _, client := range allClients { + status, err := client.Status() + require.NoError(t, err) + if status.Self.Tags == nil || status.Self.Tags.Len() == 0 { + continue + } + + for _, peer := range allClients { + if client.Hostname() == peer.Hostname() { + continue + } + + status, err := peer.Status() + require.NoError(t, err) + if status.Self.Tags == nil || status.Self.Tags.Len() == 0 { + continue + } + + fqdn, err := peer.FQDN() + require.NoError(t, err) + + url := fmt.Sprintf("http://%s/etc/hostname", fqdn) + t.Logf("url from %s to %s", client.Hostname(), url) + + result, err := client.Curl(url) + assert.Len(t, result, 13) + require.NoError(t, err) + } + } +}