diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index 4d23e589..eb8fcd02 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -244,8 +244,6 @@ jobs: - TestACLDynamicUnknownUserRemoval - TestAPIAuthenticationBypass - TestAPIAuthenticationBypassCurl - - TestGRPCAuthenticationBypass - - TestCLIWithConfigAuthenticationBypass - TestAuthKeyLogoutAndReloginSameUser - TestAuthKeyLogoutAndReloginNewUser - TestAuthKeyLogoutAndReloginSameUserExpiredKey diff --git a/cmd/dev/main.go b/cmd/dev/main.go index b3c7a840..93d65136 100644 --- a/cmd/dev/main.go +++ b/cmd/dev/main.go @@ -28,17 +28,14 @@ var errHealthTimeout = errors.New("health check timed out") var errEmptyAuthKey = errors.New("empty auth key in response") -// maxDevPort is the highest --port value that keeps both the derived -// metrics port (port+1010) and gRPC port (port+42363) inside the valid -// 1..65535 TCP range. -const maxDevPort = 23172 +// maxDevPort is the highest --port value that keeps the derived metrics port +// (port+1010) inside the valid 1..65535 TCP range. +const maxDevPort = 64525 const devConfig = `--- server_url: http://127.0.0.1:%d listen_addr: 127.0.0.1:%d metrics_listen_addr: 127.0.0.1:%d -grpc_listen_addr: 127.0.0.1:%d -grpc_allow_insecure: true noise: private_key_path: %s/noise_private.key @@ -83,7 +80,7 @@ func main() { if *port < 1 || *port > maxDevPort { log.Fatalf( - "--port must be in 1..%d (higher values overflow the derived gRPC port); got %d", + "--port must be in 1..%d (higher values overflow the derived metrics port); got %d", maxDevPort, *port, ) } @@ -101,7 +98,6 @@ func main() { func run() error { metricsPort := *port + 1010 // default 9090 - grpcPort := *port + 42363 // default 50443 tmpDir, err := os.MkdirTemp("", "headscale-dev-") if err != nil { @@ -114,8 +110,9 @@ func run() error { // Write config. configPath := filepath.Join(tmpDir, "config.yaml") - configContent := fmt.Sprintf(devConfig, - *port, *port, metricsPort, grpcPort, + configContent := fmt.Sprintf( + devConfig, + *port, *port, metricsPort, tmpDir, tmpDir, tmpDir, ) @@ -193,7 +190,8 @@ func run() error { } // Print banner. - fmt.Printf(` + fmt.Printf( + ` === Headscale Dev Environment === Server: http://127.0.0.1:%d Metrics: http://127.0.0.1:%d diff --git a/config-example.yaml b/config-example.yaml index 22b9f349..28fc617d 100644 --- a/config-example.yaml +++ b/config-example.yaml @@ -23,22 +23,6 @@ listen_addr: 127.0.0.1:8080 # Use an empty value to disable the metrics listener. metrics_listen_addr: 127.0.0.1:9090 -# Address to listen for gRPC. -# gRPC is used for controlling a headscale server -# remotely with the CLI -# Note: Remote access _only_ works if you have -# valid certificates. -# -# For production: -# grpc_listen_addr: 0.0.0.0:50443 -grpc_listen_addr: 127.0.0.1:50443 - -# Allow the gRPC admin interface to run in INSECURE -# mode. This is not recommended as the traffic will -# be unencrypted. Only enable if you know what you -# are doing. -grpc_allow_insecure: false - # CIDR(s) of reverse proxies (e.g. 127.0.0.1/32) whose # True-Client-IP, X-Real-IP and X-Forwarded-For headers should # be honoured. Empty (default) ignores those headers; setting diff --git a/hscontrol/app.go b/hscontrol/app.go index 35c196fa..6817060d 100644 --- a/hscontrol/app.go +++ b/hscontrol/app.go @@ -482,7 +482,7 @@ func (h *Headscale) createRouter(apiV1 http.Handler) *chi.Mux { return r } -// Serve launches the HTTP and gRPC server service Headscale and the API. +// Serve launches the HTTP server serving Headscale and the v1 API. // //nolint:gocyclo // complex server startup function func (h *Headscale) Serve() error { @@ -606,12 +606,12 @@ func (h *Headscale) Serve() error { socketListener, err := new(net.ListenConfig).Listen(context.Background(), "unix", h.cfg.UnixSocket) if err != nil { - return fmt.Errorf("setting up gRPC socket: %w", err) + return fmt.Errorf("setting up unix socket: %w", err) } // Change socket permissions if err := os.Chmod(h.cfg.UnixSocket, h.cfg.UnixSocketPermission); err != nil { //nolint:noinlineerr - return fmt.Errorf("changing gRPC socket permission: %w", err) + return fmt.Errorf("changing unix socket permission: %w", err) } // Build the v1 API handler and HTTP router once; both the local unix diff --git a/hscontrol/auth_tags_test.go b/hscontrol/auth_tags_test.go index 3c55c667..57940a3c 100644 --- a/hscontrol/auth_tags_test.go +++ b/hscontrol/auth_tags_test.go @@ -179,9 +179,10 @@ func TestReAuthDoesNotReapplyTags(t *testing.T) { assert.Equal(t, 1, allNodes.Len(), "Should have exactly one node") } -// NOTE: TestSetTagsOnUserOwnedNode functionality is covered by gRPC tests in grpcv1_test.go -// which properly handle ACL policy setup. The test verifies that [headscaleV1APIServer.SetTags] can convert -// user-owned nodes to tagged nodes while preserving UserID. +// NOTE: SetTags functionality is covered by the HTTP-API tests in +// hscontrol/servertest/apiv1_nodes_test.go, which handle ACL policy setup and +// verify that SetTags converts user-owned nodes to tagged nodes while +// preserving UserID. // TestCannotRemoveAllTags tests that attempting to remove all tags from a // tagged node fails with ErrCannotRemoveAllTags. Once a node is tagged, diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index 18fcd939..852c2b62 100644 --- a/hscontrol/types/config.go +++ b/hscontrol/types/config.go @@ -98,8 +98,6 @@ type Config struct { ServerURL string Addr string MetricsAddr string - GRPCAddr string - GRPCAllowInsecure bool TrustedProxies []netip.Prefix Node NodeConfig PrefixV4 *netip.Prefix @@ -418,9 +416,6 @@ func LoadConfig(path string, isFile bool) error { viper.SetDefault("unix_socket", "/var/run/headscale/headscale.sock") viper.SetDefault("unix_socket_permission", "0o770") - viper.SetDefault("grpc_listen_addr", ":50443") - viper.SetDefault("grpc_allow_insecure", false) - viper.SetDefault("cli.timeout", "5s") viper.SetDefault("cli.insecure", false) @@ -1182,8 +1177,6 @@ func LoadServerConfig() (*Config, error) { ServerURL: serverURL, Addr: viper.GetString("listen_addr"), MetricsAddr: viper.GetString("metrics_listen_addr"), - GRPCAddr: viper.GetString("grpc_listen_addr"), - GRPCAllowInsecure: viper.GetBool("grpc_allow_insecure"), TrustedProxies: trusted, DisableUpdateCheck: false, diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index bec3550a..ea0400d9 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -524,10 +524,9 @@ func (node *Node) AnnouncedRoutes() []netip.Prefix { // of the subnet-router-as-source identity. // // IMPORTANT: This method is used for internal data structures and should NOT be -// used for the gRPC Proto conversion. For Proto, SubnetRoutes must be populated -// manually with PrimaryRoutes to ensure it includes only routes actively served -// by the node. See the comment in [Node.Proto] method and the implementation in -// grpcv1.go/nodesToProto. +// used to populate the API SubnetRoutes field. For the API, SubnetRoutes must be +// populated manually with PrimaryRoutes to ensure it includes only routes +// actively served by the node. See hscontrol/api/v1/nodes.go. func (node *Node) SubnetRoutes() []netip.Prefix { var routes []netip.Prefix diff --git a/hscontrol/util/net.go b/hscontrol/util/net.go index b159f765..a746ba0c 100644 --- a/hscontrol/util/net.go +++ b/hscontrol/util/net.go @@ -1,8 +1,6 @@ package util import ( - "context" - "net" "net/netip" "sync" @@ -10,12 +8,6 @@ import ( "tailscale.com/net/tsaddr" ) -func GrpcSocketDialer(ctx context.Context, addr string) (net.Conn, error) { - var d net.Dialer - - return d.DialContext(ctx, "unix", addr) -} - func PrefixesToString(prefixes []netip.Prefix) []string { ret := make([]string, 0, len(prefixes)) for _, prefix := range prefixes { diff --git a/integration/api_auth_test.go b/integration/api_auth_test.go index cc7a0b45..5c1ac7bd 100644 --- a/integration/api_auth_test.go +++ b/integration/api_auth_test.go @@ -4,7 +4,6 @@ import ( "context" "crypto/tls" "encoding/json" - "fmt" "io" "net/http" "strings" @@ -414,286 +413,3 @@ func TestAPIAuthenticationBypassCurl(t *testing.T) { assert.Len(t, users, 2, "Should have 2 users") }) } - -// TestGRPCAuthenticationBypass tests that the gRPC authentication interceptor -// properly blocks unauthorized requests. -// This test verifies that the gRPC API does not have the same bypass issue -// as the HTTP API middleware. -func TestGRPCAuthenticationBypass(t *testing.T) { - IntegrationSkip(t) - - spec := ScenarioSpec{ - Users: []string{"grpcuser1", "grpcuser2"}, - } - - scenario, err := NewScenario(spec) - - require.NoError(t, err) - defer scenario.ShutdownAssertNoPanics(t) - - // We need TLS for remote gRPC connections - err = scenario.CreateHeadscaleEnv( - []tsic.Option{}, - hsic.WithTestName("grpcauthtest"), - hsic.WithConfigEnv(map[string]string{ - // Enable gRPC on the standard port - "HEADSCALE_GRPC_LISTEN_ADDR": "0.0.0.0:50443", - }), - ) - require.NoError(t, err) - - headscale, err := scenario.Headscale() - require.NoError(t, err) - - // Create a valid API key - apiKeyOutput, err := headscale.Execute( - []string{ - "headscale", - "apikeys", - "create", - "--expiration", - "24h", - }, - ) - require.NoError(t, err) - - validAPIKey := strings.TrimSpace(apiKeyOutput) - - // Get the gRPC endpoint - // For gRPC, we need to use the hostname and port 50443 - grpcAddress := headscale.GetHostname() + ":50443" - - t.Run("gRPC_NoAPIKey", func(t *testing.T) { - // Test 1: Try to use CLI without API key (should fail) - // When HEADSCALE_CLI_ADDRESS is set but HEADSCALE_CLI_API_KEY is not set, - // the CLI should fail immediately - _, err := headscale.Execute( - []string{ - "sh", "-c", - fmt.Sprintf("HEADSCALE_CLI_ADDRESS=%s HEADSCALE_CLI_INSECURE=true headscale users list --output json 2>&1", grpcAddress), - }, - ) - - // Should fail - CLI exits when API key is missing - assert.Error(t, err, - "gRPC connection without API key should fail") - }) - - t.Run("gRPC_InvalidAPIKey", func(t *testing.T) { - // Test 2: Try to use CLI with invalid API key (should fail with auth error) - output, err := headscale.Execute( - []string{ - "sh", "-c", - fmt.Sprintf("HEADSCALE_CLI_ADDRESS=%s HEADSCALE_CLI_API_KEY=invalid-key-12345 HEADSCALE_CLI_INSECURE=true headscale users list --output json 2>&1", grpcAddress), - }, - ) - - // Should fail with authentication error - require.Error(t, err, - "gRPC connection with invalid API key should fail") - - // Should contain authentication error message - outputStr := strings.ToLower(output) - assert.True(t, - strings.Contains(outputStr, "unauthenticated") || - strings.Contains(outputStr, "invalid token") || - strings.Contains(outputStr, "validating token") || - strings.Contains(outputStr, "authentication"), - "Error should indicate authentication failure, got: %s", output) - - // Should NOT leak user data - assert.NotContains(t, output, "grpcuser1", - "SECURITY ISSUE: gRPC should not leak user data with invalid auth") - assert.NotContains(t, output, "grpcuser2", - "SECURITY ISSUE: gRPC should not leak user data with invalid auth") - }) - - t.Run("gRPC_ValidAPIKey", func(t *testing.T) { - // Test 3: Use CLI with valid API key (should succeed) - output, err := headscale.Execute( - []string{ - "sh", "-c", - fmt.Sprintf("HEADSCALE_CLI_ADDRESS=%s HEADSCALE_CLI_API_KEY=%s HEADSCALE_CLI_INSECURE=true headscale users list --output json", grpcAddress, validAPIKey), - }, - ) - - // Should succeed - require.NoError(t, err, - "gRPC connection with valid API key should succeed, output: %s", output) - - // CLI outputs the users array directly as JSON - // Parse as JSON array (CLI uses [json.Marshal], not protojson) - var users []*apiv1.User - - err = json.Unmarshal([]byte(output), &users) - require.NoError(t, err, "Response should be valid JSON array") - assert.Len(t, users, 2, "Should have 2 users") - - userNames := make([]string, len(users)) - for i, u := range users { - userNames[i] = u.GetName().Or("") - } - - assert.Contains(t, userNames, "grpcuser1") - assert.Contains(t, userNames, "grpcuser2") - }) -} - -// TestCLIWithConfigAuthenticationBypass tests that the headscale CLI -// with --config flag does not have authentication bypass issues when -// connecting to a remote server. -// Note: When using --config with local unix socket, no auth is needed. -// This test focuses on remote gRPC connections which require API keys. -func TestCLIWithConfigAuthenticationBypass(t *testing.T) { - IntegrationSkip(t) - - spec := ScenarioSpec{ - Users: []string{"cliuser1", "cliuser2"}, - } - - scenario, err := NewScenario(spec) - - require.NoError(t, err) - defer scenario.ShutdownAssertNoPanics(t) - - err = scenario.CreateHeadscaleEnv( - []tsic.Option{}, - hsic.WithTestName("cliconfigauth"), - hsic.WithConfigEnv(map[string]string{ - "HEADSCALE_GRPC_LISTEN_ADDR": "0.0.0.0:50443", - }), - ) - require.NoError(t, err) - - headscale, err := scenario.Headscale() - require.NoError(t, err) - - // Create a valid API key - apiKeyOutput, err := headscale.Execute( - []string{ - "headscale", - "apikeys", - "create", - "--expiration", - "24h", - }, - ) - require.NoError(t, err) - - validAPIKey := strings.TrimSpace(apiKeyOutput) - - grpcAddress := headscale.GetHostname() + ":50443" - - // Create a config file for testing - configWithoutKey := fmt.Sprintf(` -cli: - address: %s - timeout: 5s - insecure: true -`, grpcAddress) - - configWithInvalidKey := fmt.Sprintf(` -cli: - address: %s - api_key: invalid-key-12345 - timeout: 5s - insecure: true -`, grpcAddress) - - configWithValidKey := fmt.Sprintf(` -cli: - address: %s - api_key: %s - timeout: 5s - insecure: true -`, grpcAddress, validAPIKey) - - t.Run("CLI_Config_NoAPIKey", func(t *testing.T) { - // Create config file without API key - err := headscale.WriteFile("/tmp/config_no_key.yaml", []byte(configWithoutKey)) - require.NoError(t, err) - - // Try to use CLI with config that has no API key - _, err = headscale.Execute( - []string{ - "headscale", - "--config", "/tmp/config_no_key.yaml", - "users", "list", - "--output", "json", - }, - ) - - // Should fail - assert.Error(t, err, - "CLI with config missing API key should fail") - }) - - t.Run("CLI_Config_InvalidAPIKey", func(t *testing.T) { - // Create config file with invalid API key - err := headscale.WriteFile("/tmp/config_invalid_key.yaml", []byte(configWithInvalidKey)) - require.NoError(t, err) - - // Try to use CLI with invalid API key - output, err := headscale.Execute( - []string{ - "sh", "-c", - "headscale --config /tmp/config_invalid_key.yaml users list --output json 2>&1", - }, - ) - - // Should fail - require.Error(t, err, - "CLI with invalid API key should fail") - - // Should indicate authentication failure - outputStr := strings.ToLower(output) - assert.True(t, - strings.Contains(outputStr, "unauthenticated") || - strings.Contains(outputStr, "invalid token") || - strings.Contains(outputStr, "validating token") || - strings.Contains(outputStr, "authentication"), - "Error should indicate authentication failure, got: %s", output) - - // Should NOT leak user data - assert.NotContains(t, output, "cliuser1", - "SECURITY ISSUE: CLI should not leak user data with invalid auth") - assert.NotContains(t, output, "cliuser2", - "SECURITY ISSUE: CLI should not leak user data with invalid auth") - }) - - t.Run("CLI_Config_ValidAPIKey", func(t *testing.T) { - // Create config file with valid API key - err := headscale.WriteFile("/tmp/config_valid_key.yaml", []byte(configWithValidKey)) - require.NoError(t, err) - - // Use CLI with valid API key - output, err := headscale.Execute( - []string{ - "headscale", - "--config", "/tmp/config_valid_key.yaml", - "users", "list", - "--output", "json", - }, - ) - - // Should succeed - require.NoError(t, err, - "CLI with valid API key should succeed") - - // CLI outputs the users array directly as JSON - // Parse as JSON array (CLI uses [json.Marshal], not protojson) - var users []*apiv1.User - - err = json.Unmarshal([]byte(output), &users) - require.NoError(t, err, "Response should be valid JSON array") - assert.Len(t, users, 2, "Should have 2 users") - - userNames := make([]string, len(users)) - for i, u := range users { - userNames[i] = u.GetName().Or("") - } - - assert.Contains(t, userNames, "cliuser1") - assert.Contains(t, userNames, "cliuser2") - }) -}