mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-25 08:09:07 +02:00
[management] Refuse pins onto a host another account's gateway already claims
A host can be claimed by a pin as well as by a proxy row, and the proxy-row check cannot see that. Another account's labeled pin beneath a host makes the host its cluster, so a self-addressed endpoint on it would never be served; another account's self-addressed endpoint on a host makes the proxy declaring it theirs, so a label beneath it would never be served either. Both bootstrap paths now refuse those shapes before the insert. Neither question touches the shared-cluster shape: labeled pins under one cluster are asked about in neither direction, so any number of accounts still pin beneath a shared cluster. Two self-addressed endpoints on one hostname stay the domain unique index's conflict to refuse. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sa3DsBDP3VciAi4PPG17L6
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
09cc91886f
commit
0df2eb7b25
@@ -1039,6 +1039,12 @@ func (m *managerImpl) bootstrapSelfAddressed(ctx context.Context, settings *type
|
||||
if err := m.requireHostNotForeign(ctx, settings.AccountID, hostname); err != nil {
|
||||
return err
|
||||
}
|
||||
// Another account's labeled pin beneath this hostname makes it their
|
||||
// cluster: a proxy serving them there would never serve this endpoint.
|
||||
// The domain unique index already arbitrates two endpoints on one name.
|
||||
if err := m.requireNotClaimedByOtherAccount(ctx, settings.AccountID, hostname, m.store.HasGatewayClusterPinnedByOtherAccount); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
settings.Domain = hostname
|
||||
settings.ProxyAddress = hostname
|
||||
@@ -1071,6 +1077,13 @@ func (m *managerImpl) bootstrapLabeled(ctx context.Context, settings *types.Sett
|
||||
if err := m.requireHostNotForeign(ctx, settings.AccountID, parent); err != nil {
|
||||
return err
|
||||
}
|
||||
// Another account's endpoint at this exact hostname means the proxy that
|
||||
// declares it is theirs, so nothing would serve a label beneath it. Other
|
||||
// accounts' labeled pins under the same cluster are not asked about: a
|
||||
// shared cluster carries many of them by design.
|
||||
if err := m.requireNotClaimedByOtherAccount(ctx, settings.AccountID, parent, m.store.HasGatewayEndpointByOtherAccount); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
for attempt := 1; attempt <= maxDomainAllocationAttempts; attempt++ {
|
||||
label := labelgen.PickTuple()
|
||||
@@ -1130,11 +1143,28 @@ func (m *managerImpl) requireHostNotForeign(ctx context.Context, accountID, host
|
||||
return fmt.Errorf("check proxy host ownership: %w", err)
|
||||
}
|
||||
if foreign {
|
||||
return status.Errorf(status.InvalidArgument, "proxy cluster %s is not available to this account", host)
|
||||
return errHostNotAvailable(host)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// requireNotClaimedByOtherAccount refuses the pin when another account's
|
||||
// gateway settings already claim the host in the shape claimed answers for.
|
||||
func (m *managerImpl) requireNotClaimedByOtherAccount(ctx context.Context, accountID, host string, claimed func(context.Context, string, string) (bool, error)) error {
|
||||
taken, err := claimed(ctx, host, accountID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("check agent network gateway claims at host: %w", err)
|
||||
}
|
||||
if taken {
|
||||
return errHostNotAvailable(host)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func errHostNotAvailable(host string) error {
|
||||
return status.Errorf(status.InvalidArgument, "proxy cluster %s is not available to this account", host)
|
||||
}
|
||||
|
||||
// isUniqueConstraintError reports whether err is a database unique-constraint
|
||||
// violation, matched on the driver message because CreateAgentNetworkSettings
|
||||
// deliberately returns the driver error unwrapped.
|
||||
|
||||
@@ -372,3 +372,45 @@ func TestCreateSettingsUnknownHostIsPinnable(t *testing.T) {
|
||||
require.NoError(t, err, "a host no proxy has declared must stay pinnable")
|
||||
assert.Equal(t, "future.example.com", created.ProxyAddress)
|
||||
}
|
||||
|
||||
// TestCreateSettingsRejectsHostAnotherAccountPinned covers claims made by pins
|
||||
// rather than proxies, which the proxy-row check cannot see. A labeled pin
|
||||
// beneath a host makes that host the other account's cluster, so a
|
||||
// self-addressed endpoint on it would never be served; a self-addressed
|
||||
// endpoint on a host makes the proxy declaring it theirs, so a label beneath
|
||||
// it would never be served either. Neither is a shared-cluster shape: many
|
||||
// labeled pins under one cluster are asked about in neither direction.
|
||||
func TestCreateSettingsRejectsHostAnotherAccountPinned(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
t.Run("self-addressed onto another account's cluster", func(t *testing.T) {
|
||||
f := newBootstrapFixture(t)
|
||||
f.expectPermission("account2", "user2", modules.AgentNetworkSettings, operations.Create, true)
|
||||
_, err := f.createSettings(ctx, "account2", "user2", "gw.example.com", "")
|
||||
require.NoError(t, err, "account2's labeled pin beneath the host must go through first")
|
||||
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
|
||||
_, err = f.createSettings(ctx, "account1", "user1", "", "gw.example.com")
|
||||
f.requireForeignClusterRefusal(t, err, "account1")
|
||||
})
|
||||
|
||||
t.Run("labeled beneath another account's endpoint", func(t *testing.T) {
|
||||
f := newBootstrapFixture(t)
|
||||
f.expectPermission("account2", "user2", modules.AgentNetworkSettings, operations.Create, true)
|
||||
_, err := f.createSettings(ctx, "account2", "user2", "", "gw.example.com")
|
||||
require.NoError(t, err, "account2's self-addressed endpoint must go through first")
|
||||
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
|
||||
_, err = f.createSettings(ctx, "account1", "user1", "gw.example.com", "")
|
||||
f.requireForeignClusterRefusal(t, err, "account1")
|
||||
})
|
||||
|
||||
t.Run("labeled beside another account's labeled pin stays allowed", func(t *testing.T) {
|
||||
f := newBootstrapFixture(t)
|
||||
for _, account := range []string{"account1", "account2"} {
|
||||
f.expectPermission(account, "user", modules.AgentNetworkSettings, operations.Create, true)
|
||||
_, err := f.createSettings(ctx, account, "user", "eu.proxy.netbird.io", "")
|
||||
require.NoError(t, err, "labeled pins under one cluster are the shared-cluster shape and must not refuse each other")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user