[management] Compare agent network cluster addresses case-insensitively

Proxies store their cluster address as they declared it, while
proxy_address is normalised lowercase before validation. The capability
and ownership lookups match cluster_address exactly, so feeding them the
normalised form asked about a spelling the store may never have seen: a
private cluster declared with capitals came back unproven and was
refused, and — worse — another account's cluster declared with capitals
came back as "never declared" and let the pin through.

Compare identity on the normalised form but keep the stored spellings,
and read the capability under each of them, any-true, the same way it
aggregates over a cluster's proxies. Ownership gets the same treatment at
the source: hostnames are case-insensitive, so two spellings of one host
are one cluster and must conflict rather than being claimable side by
side, which also closes the same gap in the proxy-registration
availability check that shares the query.

The e2e's wait for a stopped cluster to leave the active list now allows
for the active window rather than 90s: a proxy that dies without closing
its stream is only dropped once its last heartbeat ages past
proxyActiveThreshold, so the old budget could fail the test on the slow
path alone.
This commit is contained in:
mlsmaycon
2026-09-03 07:21:44 +00:00
parent 0a33ad8979
commit d911649158
4 changed files with 90 additions and 19 deletions
@@ -143,10 +143,16 @@ func waitClusterPrivate(ctx context.Context, t *testing.T, c *harness.Combined,
// 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.
//
// The budget has to clear the active window, not just the disconnect: a proxy
// that closes its stream cleanly is marked disconnected at once, but one that
// dies without that is only dropped when its last heartbeat ages past
// proxyActiveThreshold (2 minutes), so a 90s deadline could fail the test on
// the slow path alone.
func waitClusterAbsent(ctx context.Context, t *testing.T, c *harness.Combined, clusterAddr string) {
t.Helper()
deadline := time.Now().Add(90 * time.Second)
deadline := time.Now().Add(3 * time.Minute)
var last string
for time.Now().Before(deadline) {
domains, err := c.API().ReverseProxyDomains.List(ctx)
@@ -889,12 +889,12 @@ func (m *managerImpl) bootstrapSelfAddressed(ctx context.Context, settings *type
// 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)
declared, err := m.accountClusterSpellings(ctx, accountID, clusterAddr)
if err != nil {
return err
}
if !known {
if len(declared) == 0 {
// 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
@@ -916,26 +916,41 @@ func (m *managerImpl) validateGatewayCluster(ctx context.Context, accountID, clu
// 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)
//
// The capability is read per declared spelling and taken as any-true, the
// same way it aggregates over a cluster's proxies: the store matches
// cluster_address exactly, so a host two proxies spelled differently must
// not come back unproven just because it was asked about under one of them.
for _, address := range declared {
if private := m.store.GetClusterSupportsPrivate(ctx, address); private != nil && *private {
return nil
}
}
return nil
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)
}
// 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) {
// accountClusterSpellings returns every proxy cluster address in the account's
// view — its own (BYOP) clusters plus the shared ones — that names the same
// host as clusterAddr. Empty means management holds no proxy row for that host
// in this account's view.
//
// Addresses are stored as the proxy declared them and hostnames are
// case-insensitive, so identity is compared on the normalised form while the
// stored spellings are what comes back: the capability lookups match
// cluster_address exactly, and handing one a normalised address it never
// stored would silently find nothing. The cluster listing is not gated on
// heartbeats, so this answer does not change while a cluster's proxies are
// merely offline.
func (m *managerImpl) accountClusterSpellings(ctx context.Context, accountID, clusterAddr string) ([]string, error) {
clusters, err := m.store.GetProxyClusters(ctx, accountID)
if err != nil {
return false, fmt.Errorf("list proxy clusters: %w", err)
return nil, fmt.Errorf("list proxy clusters: %w", err)
}
var spellings []string
for _, cluster := range clusters {
normalized, err := types.NormalizeHostname(cluster.Address)
if err != nil {
@@ -945,10 +960,10 @@ func (m *managerImpl) accountKnowsCluster(ctx context.Context, accountID, cluste
continue
}
if normalized == clusterAddr {
return true, nil
spellings = append(spellings, cluster.Address)
}
}
return false, nil
return spellings, nil
}
// bootstrapLabeled allocates a labeled endpoint one label beneath the given
@@ -379,3 +379,48 @@ func TestCreateSettingsAcceptsOwnPrivateCluster(t *testing.T) {
require.NoError(t, err, "the account's own private cluster must be accepted")
assert.Equal(t, "byop.account1.example.com", created.ProxyAddress)
}
// TestCreateSettingsMatchesClusterCasing pins hostname case-insensitivity
// across the whole check. Proxies declare their cluster address verbatim while
// proxy_address is normalised lowercase, so a cluster spelled with capitals is
// the same cluster: its own private capability must still be found (an exact
// lookup under the normalised spelling finds nothing and would refuse a
// perfectly good cluster), and another account's must still be recognised as
// theirs (a lookup that misses would read as "never declared" and let the pin
// through).
func TestCreateSettingsMatchesClusterCasing(t *testing.T) {
ctx := context.Background()
t.Run("own private cluster is found", func(t *testing.T) {
f := newBootstrapFixture(t)
f.seedProxy(t, "proxy1", "", "EU.Proxy.Example.com", ptrTo(true))
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
created, err := f.createSettings(ctx, "account1", "user1", "eu.proxy.example.com", "")
require.NoError(t, err, "a private cluster declared with capitals must still be accepted")
assert.Equal(t, "eu.proxy.example.com", created.ProxyAddress)
})
t.Run("foreign cluster is still foreign", func(t *testing.T) {
f := newBootstrapFixture(t)
f.seedProxy(t, "proxy1", "account2", "BYOP.Account2.Example.com", ptrTo(true))
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
_, err := f.createSettings(ctx, "account1", "user1", "byop.account2.example.com", "")
require.Error(t, err, "another account's cluster must be refused whatever its casing")
var sErr *status.Error
require.ErrorAs(t, err, &sErr)
assert.Equal(t, status.InvalidArgument, sErr.Type())
assert.Contains(t, err.Error(), "not available to this account")
})
t.Run("non-private cluster is still refused", func(t *testing.T) {
f := newBootstrapFixture(t)
f.seedProxy(t, "proxy1", "", "Central.Example.com", ptrTo(false))
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
_, err := f.createSettings(ctx, "account1", "user1", "central.example.com", "")
require.Error(t, err, "casing must not become a way past the capability check")
assert.Contains(t, err.Error(), "embedded proxy")
})
}
+6 -1
View File
@@ -6392,11 +6392,16 @@ func (s *SqlStore) HasActiveProxyAtClusterAddress(ctx context.Context, clusterAd
return count > 0, nil
}
// IsClusterAddressConflicting reports whether the address is already declared
// by a proxy outside the account — a shared proxy or another account's. The
// comparison is case-insensitive: cluster addresses are hostnames, stored as
// the proxy declared them, so two spellings of one host are one cluster and
// must conflict rather than being claimable side by side.
func (s *SqlStore) IsClusterAddressConflicting(ctx context.Context, clusterAddress, accountID string) (bool, error) {
var count int64
result := s.db.
Model(&proxy.Proxy{}).
Where("cluster_address = ? AND (account_id IS NULL OR account_id != ?)", clusterAddress, accountID).
Where("LOWER(cluster_address) = LOWER(?) AND (account_id IS NULL OR account_id != ?)", clusterAddress, accountID).
Count(&count)
if result.Error != nil {
return false, status.Errorf(status.Internal, "check cluster address conflict: %v", result.Error)