diff --git a/cmd/headscale/cli/oauth_client.go b/cmd/headscale/cli/oauth_client.go index aeb56646..3d74b825 100644 --- a/cmd/headscale/cli/oauth_client.go +++ b/cmd/headscale/cli/oauth_client.go @@ -23,7 +23,7 @@ func init() { oauthClientsCmd.AddCommand(listOAuthClientsCmd) createOAuthClientCmd.Flags(). - StringArrayP("scope", "s", nil, "Scope the client's tokens are granted (repeatable): auth_keys, oauth_keys, devices:core, devices:routes, policy_file, feature_settings (each with a :read variant), or all/all:read") + StringArrayP("scope", "s", nil, "Scope the client's tokens are granted (repeatable): auth_keys, oauth_keys, devices:core, devices:routes, policy_file, feature_settings, users (each with a :read variant), or all/all:read") createOAuthClientCmd.Flags(). StringArrayP("tag", "t", nil, "Tag the client's tokens may assign to devices (repeatable), e.g. tag:k8s-operator") createOAuthClientCmd.Flags().StringP("description", "d", "", "Human-readable description") diff --git a/hscontrol/api/v2/errors.go b/hscontrol/api/v2/errors.go index cf5110e2..3a918eda 100644 --- a/hscontrol/api/v2/errors.go +++ b/hscontrol/api/v2/errors.go @@ -72,6 +72,7 @@ func mapError(msg string, err error) error { case errors.Is(err, db.ErrPreAuthKeyNotTaggedOrOwned), errors.Is(err, db.ErrPreAuthKeyACLTagInvalid), + errors.Is(err, db.ErrOAuthClientScopeInvalid), errors.Is(err, state.ErrGivenNameInvalid), errors.Is(err, state.ErrGivenNameTaken), errors.Is(err, state.ErrNodeNameNotUnique), diff --git a/hscontrol/db/oauth.go b/hscontrol/db/oauth.go index 07c234f3..dbbda311 100644 --- a/hscontrol/db/oauth.go +++ b/hscontrol/db/oauth.go @@ -11,9 +11,11 @@ import ( "strings" "time" + "github.com/juanfont/headscale/hscontrol/scope" "github.com/juanfont/headscale/hscontrol/types" "golang.org/x/crypto/argon2" "gorm.io/gorm" + "tailscale.com/util/multierr" "tailscale.com/util/rands" "tailscale.com/util/set" ) @@ -37,6 +39,7 @@ var ( ErrOAuthClientNotFound = fmt.Errorf("oauth client not found: %w", gorm.ErrRecordNotFound) ErrOAuthClientFailedToParse = errors.New("failed to parse oauth client secret") ErrOAuthClientRevoked = errors.New("oauth client revoked") + ErrOAuthClientScopeInvalid = errors.New("oauth client scope invalid") ErrAccessTokenNotFound = fmt.Errorf("oauth access token not found: %w", gorm.ErrRecordNotFound) ErrAccessTokenFailedToParse = errors.New("failed to parse oauth access token") @@ -132,6 +135,22 @@ func verifySecret(encoded []byte, secret string) error { return nil } +// validateScopes rejects any value outside the vocabulary in [scope.Known]. An +// unknown scope satisfies no requirement, so storing one would silently mint a +// client with fewer permissions than asked for. Every offending scope is +// reported so the caller can correct them all in one pass. +func validateScopes(scopes []string) error { + var errs []error + + for _, s := range scopes { + if !scope.Scope(s).Valid() { + errs = append(errs, fmt.Errorf("%w: %q", ErrOAuthClientScopeInvalid, s)) + } + } + + return multierr.New(errs...) +} + // CreateOAuthClient creates a new [types.OAuthClient] and returns the plaintext // secret (shown ONCE) alongside the stored client. creatorUserID is the user who // created it (informational), or nil. @@ -145,6 +164,11 @@ func (hsdb *HSDatabase) CreateOAuthClient( return "", nil, err } + err = validateScopes(scopes) + if err != nil { + return "", nil, err + } + scopes = set.SetOf(scopes).Slice() slices.Sort(scopes) diff --git a/hscontrol/db/oauth_test.go b/hscontrol/db/oauth_test.go index 7d662959..1d983929 100644 --- a/hscontrol/db/oauth_test.go +++ b/hscontrol/db/oauth_test.go @@ -250,3 +250,61 @@ func TestAccessTokenRejectedWhenClientGone(t *testing.T) { _, err = db.AuthenticateAccessToken(tokenStr2) require.ErrorIs(t, err, ErrAccessTokenClientRevoked) } + +// TestOAuthClientCreateRejectsUnknownScopes asserts scopes are validated against +// the known vocabulary the same way tags are, instead of being stored verbatim. +func TestOAuthClientCreateRejectsUnknownScopes(t *testing.T) { + tests := []struct { + name string + scopes []string + valid bool + }{ + {name: "known write and read scopes", scopes: []string{"auth_keys", "devices:core:read"}, valid: true}, + {name: "super scopes", scopes: []string{"all", "all:read"}, valid: true}, + {name: "arbitrary scope", scopes: []string{"EVIL:superuser"}}, + {name: "path traversal", scopes: []string{"../../etc/passwd"}}, + {name: "wildcard", scopes: []string{"*::*"}}, + {name: "empty scope", scopes: []string{""}}, + {name: "wrong case", scopes: []string{"AUTH_KEYS"}}, + {name: "one unknown among known", scopes: []string{"auth_keys", "devices:everything"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + _, client, err := db.CreateOAuthClient(tt.scopes, []string{"tag:ci"}, "", nil) + if tt.valid { + require.NoError(t, err) + assert.ElementsMatch(t, tt.scopes, client.Scopes) + + return + } + + require.ErrorIs(t, err, ErrOAuthClientScopeInvalid) + assert.Nil(t, client) + + clients, err := db.ListOAuthClients() + require.NoError(t, err) + assert.Empty(t, clients, "rejected client must not be stored") + }) + } +} + +// TestOAuthClientCreateReportsEveryUnknownScope asserts validation does not stop +// at the first bad scope, so a caller can fix them all in one pass. +func TestOAuthClientCreateReportsEveryUnknownScope(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + _, _, err = db.CreateOAuthClient( + []string{"auth_keys", "devices:everything", "EVIL:superuser"}, + []string{"tag:ci"}, + "", + nil, + ) + require.ErrorIs(t, err, ErrOAuthClientScopeInvalid) + assert.Contains(t, err.Error(), "devices:everything") + assert.Contains(t, err.Error(), "EVIL:superuser") +} diff --git a/hscontrol/scope/scope.go b/hscontrol/scope/scope.go index 1a1a0a6d..b909bc21 100644 --- a/hscontrol/scope/scope.go +++ b/hscontrol/scope/scope.go @@ -8,7 +8,10 @@ // hscontrol/api/v2, so it can be tested exhaustively on its own. package scope -import "strings" +import ( + "slices" + "strings" +) // Scope is an OAuth capability an operation requires and a token grants. The names // mirror Tailscale's API scopes; a "...:read" scope is the read-only subset of its @@ -62,6 +65,13 @@ func Known() []Scope { } } +// Valid reports whether s is part of the known scope vocabulary. Scopes outside +// it never satisfy any requirement, so they are rejected at creation time +// rather than stored as permanently inert grants. +func (s Scope) Valid() bool { + return slices.Contains(Known(), s) +} + // IsRead reports whether s is a read-only scope (its name ends with ":read"). func (s Scope) IsRead() bool { return strings.HasSuffix(string(s), readSuffix) diff --git a/hscontrol/scope/scope_test.go b/hscontrol/scope/scope_test.go index 0b59aa42..af20145d 100644 --- a/hscontrol/scope/scope_test.go +++ b/hscontrol/scope/scope_test.go @@ -232,3 +232,17 @@ func TestKnownIsComplete(t *testing.T) { t.Errorf("Known() has %d scopes, want 16", len(known)) } } + +func TestValid(t *testing.T) { + for _, s := range Known() { + if !s.Valid() { + t.Errorf("Known scope %q reported invalid", s) + } + } + + for _, s := range []Scope{"", "EVIL:superuser", "*::*", "../../etc/passwd", "AUTH_KEYS", "devices", "all:write"} { + if s.Valid() { + t.Errorf("unknown scope %q reported valid", s) + } + } +}