diff --git a/e2e/agentnetwork/settings_cluster_validation_test.go b/e2e/agentnetwork/settings_cluster_validation_test.go new file mode 100644 index 000000000..af0382ad5 --- /dev/null +++ b/e2e/agentnetwork/settings_cluster_validation_test.go @@ -0,0 +1,173 @@ +//go:build e2e + +package agentnetwork + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/e2e/harness" + "github.com/netbirdio/netbird/shared/management/http/api" +) + +// TestSettingsBootstrapValidatesProxyCluster covers the bootstrap-time check +// on the picked cluster, end to end against a real proxy. +// +// The synthesised gateway service is always private: agents reach it over the +// WireGuard tunnel and are authorised by their peer identity. Only a proxy +// running embedded in a netbird client (`netbird proxy --private`) can serve +// that, and management reports it per cluster as the `private` capability — +// the same supports_private flag the dashboard reads to decide which clusters +// it may offer. The endpoint assigned at bootstrap is immutable, so pinning +// to a cluster that cannot serve it has to be refused up front rather than +// leaving the account with a dead gateway. +// +// One combined server and one cluster address, walked through three states: +// a live centralised proxy (refused), that proxy stopped so nothing in the +// cluster is live any more (still refused — the record of what the cluster is +// outlives its heartbeats), and finally an embedded proxy (accepted, the +// capability being any-true across the cluster's live proxies). Same account, +// same address, so nothing but the cluster's state accounts for the different +// answers. +func TestSettingsBootstrapValidatesProxyCluster(t *testing.T) { + ctx := context.Background() + + fresh, err := harnessStartFresh(ctx, t) + require.NoError(t, err, "start dedicated combined server") + + proxyToken, err := fresh.CreateProxyTokenCLI(ctx, "e2e-cluster-validation") + require.NoError(t, err, "mint proxy token via CLI") + + const cluster = harness.AgentNetworkCluster + + // A centralised proxy: connected and serving the cluster, but not + // embedded in a netbird client, so it cannot authenticate tunnel peers. + central, err := harness.StartProxy(ctx, fresh, proxyToken, map[string]string{ + "NB_PROXY_PRIVATE": "false", + }) + require.NoError(t, err, "start centralised proxy") + // Terminated mid-test; the cleanup only covers an early failure. + t.Cleanup(func() { _ = central.Terminate(context.Background()) }) + + waitClusterPrivate(ctx, t, fresh, cluster, false) + + _, err = fresh.CreateSettings(ctx, api.AgentNetworkSettingsCreateRequest{ + ProxyAddress: ptr(cluster), + }) + require.Error(t, err, "bootstrap onto a cluster with no embedded proxy must be refused") + requireClientError(t, err) + assert.Contains(t, err.Error(), "embedded proxy", + "the refusal must name what the cluster is missing: %v", err) + + after, err := fresh.GetSettings(ctx) + require.NoError(t, err, "settings must still read after a refused bootstrap") + assert.Empty(t, after.Endpoint, "a refused bootstrap must not assign an endpoint") + assert.Empty(t, after.ProxyAddress, "a refused bootstrap must not pin a cluster") + + // Stopping the centralised proxy must not turn the refusal into an + // acceptance: the cluster's proxy rows outlive their heartbeats (only the + // hourly stale reaper removes them), so the cluster is still on record as + // one that cannot serve the gateway. Judging on liveness instead would + // make "wait for the proxy to go quiet" a way to pin the account's + // immutable endpoint to a cluster that can never serve it. + require.NoError(t, central.Terminate(ctx), "stop the centralised proxy") + waitClusterAbsent(ctx, t, fresh, cluster) + + _, err = fresh.CreateSettings(ctx, api.AgentNetworkSettingsCreateRequest{ + ProxyAddress: ptr(cluster), + }) + require.Error(t, err, "an offline cluster with no embedded proxy on record must stay refused") + requireClientError(t, err) + + // Add an embedded proxy to the same cluster: now it can serve a private + // service, and the very same request must go through. + embedded, err := harness.StartProxy(ctx, fresh, proxyToken) + require.NoError(t, err, "start embedded proxy") + t.Cleanup(func() { _ = embedded.Terminate(context.Background()) }) + + waitClusterPrivate(ctx, t, fresh, cluster, true) + + bootstrapped, err := fresh.CreateSettings(ctx, api.AgentNetworkSettingsCreateRequest{ + ProxyAddress: ptr(cluster), + }) + require.NoError(t, err, "bootstrap onto a private-capable cluster must succeed") + assert.Equal(t, cluster, bootstrapped.ProxyAddress, "the pinned cluster is the requested one") + assert.True(t, strings.HasSuffix(bootstrapped.Endpoint, "."+cluster), + "the endpoint must hang one label beneath the cluster: %s", bootstrapped.Endpoint) +} + +// waitClusterPrivate polls the domains endpoint — the list the dashboard picks +// its bootstrap cluster from — until the free domain for clusterAddr reports +// supports_private == want. A proxy's capabilities land when it registers, so +// this is the barrier between starting a proxy and asserting on what +// management thinks its cluster can do. +func waitClusterPrivate(ctx context.Context, t *testing.T, c *harness.Combined, clusterAddr string, want bool) { + t.Helper() + + deadline := time.Now().Add(90 * time.Second) + var last string + for time.Now().Before(deadline) { + domains, err := c.API().ReverseProxyDomains.List(ctx) + if err != nil { + last = "list domains: " + err.Error() + } else { + last = "cluster not listed" + for _, d := range domains { + if d.Domain != clusterAddr { + continue + } + if d.SupportsPrivate == nil { + last = "supports_private not reported yet" + break + } + if *d.SupportsPrivate == want { + return + } + last = "supports_private is not the expected value" + break + } + } + if !waitBeforeRetry(ctx, 2*time.Second) { + break + } + } + t.Fatalf("cluster %s never reported supports_private=%v: %s", clusterAddr, want, last) +} + +// waitClusterAbsent polls the domains endpoint until clusterAddr is no longer +// offered, i.e. management sees no live proxy in it. The free-domain list is +// built from the active clusters, so this is how a proxy going away becomes +// observable — while the cluster's rows, and so its capability record, remain. +func waitClusterAbsent(ctx context.Context, t *testing.T, c *harness.Combined, clusterAddr string) { + t.Helper() + + deadline := time.Now().Add(90 * time.Second) + var last string + for time.Now().Before(deadline) { + domains, err := c.API().ReverseProxyDomains.List(ctx) + if err != nil { + last = "list domains: " + err.Error() + } else { + listed := false + for _, d := range domains { + if d.Domain == clusterAddr { + listed = true + break + } + } + if !listed { + return + } + last = "cluster still listed as active" + } + if !waitBeforeRetry(ctx, 2*time.Second) { + break + } + } + t.Fatalf("cluster %s never dropped out of the active list: %s", clusterAddr, last) +} diff --git a/management/internals/modules/agentnetwork/handlers/handlers_test.go b/management/internals/modules/agentnetwork/handlers/handlers_test.go index 6d1be3562..4f19afcc0 100644 --- a/management/internals/modules/agentnetwork/handlers/handlers_test.go +++ b/management/internals/modules/agentnetwork/handlers/handlers_test.go @@ -9,6 +9,7 @@ import ( "runtime" "strings" "testing" + "time" "go.uber.org/mock/gomock" "github.com/gorilla/mux" @@ -17,6 +18,7 @@ import ( "github.com/netbirdio/netbird/management/internals/modules/agentnetwork" agentNetworkTypes "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types" + rpproxy "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy" "github.com/netbirdio/netbird/management/server/account" nbcontext "github.com/netbirdio/netbird/management/server/context" "github.com/netbirdio/netbird/management/server/permissions" @@ -29,6 +31,9 @@ import ( const ( testAccountID = "acc-1" testUserID = "user-bob" + // testClusterAddress is the shared proxy cluster the settings tests pin + // their gateway to; the fixture seeds a connected embedded proxy for it. + testClusterAddress = "eu.proxy.netbird.io" ) // agentNetworkHandlerFixture builds a real agentnetwork.Manager with @@ -75,6 +80,12 @@ func newAgentNetworkHandlerFixture(t *testing.T) *agentNetworkHandlerFixture { manager := agentnetwork.NewManager(st, perms, accounts, nil) h := &handler{manager: manager} + // The labeled bootstrap validates its proxy_address against the live + // clusters, so seed the shared cluster these tests pin to as a real, + // private-capable one — the wire-shape assertions then run through the + // validated path rather than the "nothing connected yet" carve-out. + seedSharedEmbeddedCluster(t, st, testClusterAddress) + router := mux.NewRouter() router.HandleFunc("/agent-network/providers", h.createProvider).Methods("POST") router.HandleFunc("/agent-network/providers/{providerId}", h.getProvider).Methods("GET") @@ -268,3 +279,21 @@ func TestConsumptionHandler_PopulatedAccountListsRows(t *testing.T) { assert.Equal(t, groupRow.WindowStartUtc, userRow.WindowStartUtc, "rows recorded in the same window must share the aligned window_start_utc") } + +// seedSharedEmbeddedCluster registers a connected, NetBird-operated proxy +// running embedded in a netbird client (the `private` capability) so +// clusterAddr is a cluster any account may pin its agent-network gateway to. +func seedSharedEmbeddedCluster(t *testing.T, st store.Store, clusterAddr string) { + t.Helper() + private := true + now := time.Now().UTC() + require.NoError(t, st.SaveProxy(context.Background(), &rpproxy.Proxy{ + ID: "shared-proxy-" + clusterAddr, + SessionID: "shared-session", + ClusterAddress: clusterAddr, + LastSeen: now, + ConnectedAt: &now, + Status: rpproxy.StatusConnected, + Capabilities: rpproxy.Capabilities{Private: &private}, + }), "seeding the shared proxy cluster must succeed") +} diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go index 41789195e..7954cb46e 100644 --- a/management/internals/modules/agentnetwork/manager.go +++ b/management/internals/modules/agentnetwork/manager.go @@ -860,18 +860,117 @@ func (m *managerImpl) bootstrapSelfAddressed(ctx context.Context, settings *type return nil } +// validateGatewayCluster rejects a labeled bootstrap pinned to a cluster that +// cannot serve the account's gateway. +// +// The synthesised gateway service is unconditionally private +// (buildAccountService): agents reach it over the WireGuard tunnel and are +// authorised by ValidateTunnelPeer against the policies' source groups, and +// its single target is the cluster itself with DirectUpstream. Only a proxy +// running embedded in a netbird client (`netbird proxy`) can serve that — a +// centralised proxy has no tunnel identity to authenticate against and no +// WireGuard endpoint to be reached on. Management reports that per cluster as +// the `private` capability, the same flag the dashboard renders as +// supports_private when it gates NetBird-only services. +// +// Without this check the bootstrap happily pins to any hostname the caller +// names, including a cluster the account cannot use or one with no embedded +// proxy — and the endpoint it allocates is immutable, so the account is left +// with a dead gateway that only a DeleteSettings/re-bootstrap can undo. +// +// Whether management knows the cluster is decided on the proxy rows +// themselves, never on how fresh their heartbeats are: a cluster's rows +// outlive its proxies' liveness (only the stale-proxy reaper removes them), so +// a cluster that exists stays judged as one. Judging on liveness instead would +// make the same centralised cluster pass or fail depending on whether its +// proxies happened to have heartbeated in the last couple of minutes. +// +// The single opening left is a cluster management holds no proxy row for at +// all: pinning ahead of a proxy's first connection is a legitimate order — the +// dedicated path claims an address the same way, before any proxy declares it. +func (m *managerImpl) validateGatewayCluster(ctx context.Context, accountID, clusterAddr string) error { + known, err := m.accountKnowsCluster(ctx, accountID, clusterAddr) + if err != nil { + return err + } + + if !known { + // Not in the account's view. A shared cluster would have been in it, + // so a proxy row elsewhere for this address can only be another + // account's BYOP cluster: its proxies filter foreign mappings out on + // delivery, making the pin dead on arrival. + foreign, err := m.store.IsClusterAddressConflicting(ctx, clusterAddr, accountID) + if err != nil { + return fmt.Errorf("check proxy cluster ownership: %w", err) + } + if foreign { + return status.Errorf(status.InvalidArgument, + "proxy cluster %s is not available to this account", clusterAddr) + } + // No proxy has ever declared this address: an address-first pin. + return nil + } + + // A cluster management knows has to prove it can serve the gateway, and + // only a live embedded proxy proves that. Both an explicit false and an + // unreported capability (nothing live in the cluster, or proxies predating + // capability reporting) fail here: unusable and unproven are the same + // answer for a decision that cannot be revisited later. + if private := m.store.GetClusterSupportsPrivate(ctx, clusterAddr); private == nil || !*private { + return status.Errorf(status.InvalidArgument, + "proxy cluster %s cannot serve the agent network gateway: the gateway is reachable only from connected peers, "+ + "which needs at least one connected embedded proxy (netbird proxy) in the cluster", clusterAddr) + } + return nil +} + +// accountKnowsCluster reports whether clusterAddr is a proxy cluster in the +// account's view — one of its own (BYOP) clusters or a shared one. The cluster +// listing is not gated on heartbeats, so this answer does not change while a +// cluster's proxies are merely offline. Addresses are stored as the proxy +// declared them, so both sides are normalised: hostnames are case-insensitive +// and the pin must not be sidesteppable by casing. +func (m *managerImpl) accountKnowsCluster(ctx context.Context, accountID, clusterAddr string) (bool, error) { + clusters, err := m.store.GetProxyClusters(ctx, accountID) + if err != nil { + return false, fmt.Errorf("list proxy clusters: %w", err) + } + + for _, cluster := range clusters { + normalized, err := types.NormalizeHostname(cluster.Address) + if err != nil { + // An address declared in a shape we cannot normalise is not one an + // endpoint can be allocated beneath. + log.WithContext(ctx).Debugf("skipping unusable proxy cluster address %q: %s", cluster.Address, err) + continue + } + if normalized == clusterAddr { + return true, nil + } + } + return false, nil +} + // bootstrapLabeled allocates a labeled endpoint one label beneath the given // cluster address: Domain =