From 0a33ad89790e4eca1b35d0b64cecc3d1970258f9 Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Wed, 2 Sep 2026 09:59:29 +0000 Subject: [PATCH] [management] Validate the proxy cluster an agent network bootstraps onto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The synthesised agent network gateway service is unconditionally private: agents reach it over the WireGuard tunnel and are authorised by ValidateTunnelPeer against the enabled policies' source groups, and its only target is the cluster itself with DirectUpstream. Only a proxy running embedded in a netbird client can serve that, which management already reports per cluster as the `private` capability. CreateSettings accepted any hostname as proxy_address, so a labeled bootstrap could pin an account to a cluster that cannot serve its gateway — another account's BYOP cluster, or one whose proxies are all centralised. The endpoint assigned at bootstrap is immutable, so the account is then stuck with a dead gateway until someone deletes and re-bootstraps the settings row. Validate the cluster before allocating an endpoint beneath it. Whether management knows a cluster is decided on its proxy rows, never on how fresh their heartbeats are: the rows outlive their proxies' liveness, so a known cluster stays judged as one and has to prove with a live embedded proxy that it can serve the gateway. Deciding on liveness instead would let the same centralised cluster pass or fail depending on whether its proxies had heartbeated in the last couple of minutes, turning "wait for the proxy to go quiet" into a way to pin the endpoint to a cluster that can never serve it. Ownership comes from the same time-independent source, so a foreign cluster stays refused while it is offline. Only a cluster no proxy has ever declared is still pinnable — that is the address-first order the dedicated (self-addressed) path documents, and the one self-hosted setups follow when they configure before deploying. --- .../settings_cluster_validation_test.go | 173 ++++++++++++++++++ .../agentnetwork/handlers/handlers_test.go | 29 +++ .../internals/modules/agentnetwork/manager.go | 99 ++++++++++ .../agentnetwork/settings_bootstrap_test.go | 156 ++++++++++++++++ .../agentnetwork_budgetrule_realstack_test.go | 1 + .../server/agentnetwork_realstack_test.go | 21 +++ 6 files changed, 479 insertions(+) create mode 100644 e2e/agentnetwork/settings_cluster_validation_test.go 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 =