From 083906b84d504d44c6ab3b00613a6b1ce392a9db Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 22:56:07 +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 agent network gateway service is synthesised as 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 the 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: a cluster whose live proxies have reported their capabilities must be one the account may route through and must be private-capable. A cluster nothing is connected to is left alone, so claiming an address ahead of the proxy's first connection keeps working — the same address-first order the dedicated (self-addressed) path documents, and the one the e2e suite and self-hosted setups follow. The e2e coverage drives the real thing: one combined server and two proxies in the same cluster — a centralised one that makes the cluster live but unusable, then an embedded one that makes it usable — so both the refusal and the acceptance are exercised against the same account and cluster address, with the domains endpoint (the list the dashboard picks from) as the barrier between starting a proxy and asserting on it. Co-Authored-By: Claude Opus 5 --- .../settings_cluster_validation_test.go | 122 ++++++++++++++++++ .../agentnetwork/handlers/handlers_test.go | 29 +++++ .../internals/modules/agentnetwork/manager.go | 90 +++++++++++++ .../agentnetwork/settings_bootstrap_test.go | 119 +++++++++++++++++ .../agentnetwork_budgetrule_realstack_test.go | 1 + .../server/agentnetwork_realstack_test.go | 21 +++ 6 files changed, 382 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..65ad5f451 --- /dev/null +++ b/e2e/agentnetwork/settings_cluster_validation_test.go @@ -0,0 +1,122 @@ +//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, two proxies in the same cluster: the centralised one +// makes the cluster live-but-unusable, the embedded one added afterwards +// makes it usable (the capability is any-true across the cluster's live +// proxies), so both the refusal and the acceptance are exercised against the +// same account and the same cluster address. +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") + 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") + + // 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) +} 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..d470bd284 100644 --- a/management/internals/modules/agentnetwork/manager.go +++ b/management/internals/modules/agentnetwork/manager.go @@ -860,18 +860,108 @@ 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. +// +// Only a cluster management can actually judge is rejected: one whose live +// proxies have reported their capabilities. A cluster nothing is connected to +// is left alone, because pinning ahead of the 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 { + private := m.store.GetClusterSupportsPrivate(ctx, clusterAddr) + if private == nil { + // No live proxy in the cluster reported its capabilities: either + // nothing is connected there yet, or the proxies predate capability + // reporting. Nothing to judge — let the pin through. + return nil + } + + available, err := m.accountClusterAddresses(ctx, accountID) + if err != nil { + return err + } + if !slices.Contains(available, clusterAddr) { + // Live, but not a cluster this account may route through: another + // account's BYOP cluster. Its proxies filter foreign mappings out on + // delivery, so the pin would be dead on arrival. + return status.Errorf(status.InvalidArgument, + "proxy cluster %s is not available to this account", clusterAddr) + } + + if !*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 +} + +// accountClusterAddresses lists the active proxy cluster addresses the account +// may pin its gateway to: its own (BYOP) clusters plus the shared ones. This +// mirrors the free-domain allow list the dashboard offers as cluster choices, +// so the API accepts exactly what the UI can present. Addresses are stored as +// the proxy declared them; they are normalised here so the comparison against +// a normalised proxy_address is not defeated by case. +func (m *managerImpl) accountClusterAddresses(ctx context.Context, accountID string) ([]string, error) { + byop, err := m.store.GetActiveProxyClusterAddressesForAccount(ctx, accountID) + if err != nil { + return nil, fmt.Errorf("list account proxy clusters: %w", err) + } + shared, err := m.store.GetActiveProxyClusterAddresses(ctx) + if err != nil { + return nil, fmt.Errorf("list shared proxy clusters: %w", err) + } + + addresses := make([]string, 0, len(byop)+len(shared)) + for _, addr := range slices.Concat(byop, shared) { + normalized, err := types.NormalizeHostname(addr) + if err != nil { + // A cluster address the proxy 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", addr, err) + continue + } + addresses = append(addresses, normalized) + } + return addresses, nil +} + // bootstrapLabeled allocates a labeled endpoint one label beneath the given // cluster address: Domain =