diff --git a/enterprise/tailnet/pgcoord_test.go b/enterprise/tailnet/pgcoord_test.go index 3ec874ad17..728387e3d9 100644 --- a/enterprise/tailnet/pgcoord_test.go +++ b/enterprise/tailnet/pgcoord_test.go @@ -120,7 +120,7 @@ func TestPGCoordinatorSingle_AgentInvalidIP(t *testing.T) { // The agent connection should be closed immediately after sending an invalid addr agent.AssertEventuallyResponsesClosed( - agpl.AuthorizationError{Wrapped: agpl.InvalidNodeAddressError{Addr: prefix.Addr().String()}}.Error()) + agpl.AuthorizationError{Wrapped: xerrors.Errorf("Addresses: %w", agpl.InvalidNodeAddressError{Addr: prefix.Addr().String()})}.Error()) assertEventuallyLost(ctx, t, store, agent.ID) } @@ -146,7 +146,37 @@ func TestPGCoordinatorSingle_AgentInvalidIPBits(t *testing.T) { // The agent connection should be closed immediately after sending an invalid addr agent.AssertEventuallyResponsesClosed( - agpl.AuthorizationError{Wrapped: agpl.InvalidAddressBitsError{Bits: 64}}.Error()) + agpl.AuthorizationError{Wrapped: xerrors.Errorf("Addresses: %w", agpl.InvalidAddressBitsError{Bits: 64})}.Error()) + assertEventuallyLost(ctx, t, store, agent.ID) +} + +func TestPGCoordinatorSingle_AgentInvalidAllowedIP(t *testing.T) { + t.Parallel() + + store, ps := dbtestutil.NewDB(t) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitSuperLong) + defer cancel() + logger := testutil.Logger(t) + coordinator, err := tailnet.NewPGCoord(ctx, logger, ps, store) + require.NoError(t, err) + defer coordinator.Close() + + agent := agpltest.NewAgent(ctx, t, coordinator, "agent") + defer agent.Close(ctx) + // A valid self-address paired with an AllowedIP belonging to a different + // (victim) agent must be rejected. + victim := agpl.TailscaleServicePrefix.PrefixFromUUID(uuid.New()) + agent.UpdateNode(&proto.Node{ + Addresses: []string{ + agpl.TailscaleServicePrefix.PrefixFromUUID(agent.ID).String(), + }, + AllowedIps: []string{victim.String()}, + PreferredDerp: 10, + }) + + // The agent connection should be closed after sending an invalid AllowedIP. + agent.AssertEventuallyResponsesClosed( + agpl.AuthorizationError{Wrapped: xerrors.Errorf("AllowedIps: %w", agpl.InvalidNodeAddressError{Addr: victim.Addr().String()})}.Error()) assertEventuallyLost(ctx, t, store, agent.ID) } diff --git a/tailnet/coordinator_test.go b/tailnet/coordinator_test.go index fb7ecc73dc..e35d0c3fa8 100644 --- a/tailnet/coordinator_test.go +++ b/tailnet/coordinator_test.go @@ -7,6 +7,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "cdr.dev/slog/v3" "cdr.dev/slog/v3/sloggers/slogtest" @@ -102,7 +103,32 @@ func TestCoordinator(t *testing.T) { PreferredDerp: 10, }) agent.AssertEventuallyResponsesClosed( - tailnet.AuthorizationError{Wrapped: tailnet.InvalidNodeAddressError{Addr: prefix.Addr().String()}}.Error()) + tailnet.AuthorizationError{Wrapped: xerrors.Errorf("Addresses: %w", tailnet.InvalidNodeAddressError{Addr: prefix.Addr().String()})}.Error()) + }) + + t.Run("AgentWithoutClients_InvalidAllowedIP", func(t *testing.T) { + t.Parallel() + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + ctx := testutil.Context(t, testutil.WaitShort) + coordinator := tailnet.NewCoordinator(logger) + defer func() { + err := coordinator.Close() + require.NoError(t, err) + }() + agent := test.NewAgent(ctx, t, coordinator, "agent") + defer agent.Close(ctx) + // A valid self-address paired with an AllowedIP belonging to a different + // (victim) agent must be rejected. + victim := tailnet.TailscaleServicePrefix.PrefixFromUUID(uuid.New()) + agent.UpdateNode(&proto.Node{ + Addresses: []string{ + tailnet.TailscaleServicePrefix.PrefixFromUUID(agent.ID).String(), + }, + AllowedIps: []string{victim.String()}, + PreferredDerp: 10, + }) + agent.AssertEventuallyResponsesClosed( + tailnet.AuthorizationError{Wrapped: xerrors.Errorf("AllowedIps: %w", tailnet.InvalidNodeAddressError{Addr: victim.Addr().String()})}.Error()) }) t.Run("AgentWithoutClients_InvalidBits", func(t *testing.T) { @@ -124,7 +150,7 @@ func TestCoordinator(t *testing.T) { PreferredDerp: 10, }) agent.AssertEventuallyResponsesClosed( - tailnet.AuthorizationError{Wrapped: tailnet.InvalidAddressBitsError{Bits: 64}}.Error()) + tailnet.AuthorizationError{Wrapped: xerrors.Errorf("Addresses: %w", tailnet.InvalidAddressBitsError{Bits: 64})}.Error()) }) t.Run("AgentWithClient", func(t *testing.T) { diff --git a/tailnet/tunnel.go b/tailnet/tunnel.go index 75a943ba13..c575d0b986 100644 --- a/tailnet/tunnel.go +++ b/tailnet/tunnel.go @@ -71,21 +71,38 @@ func (a AgentCoordinateeAuth) Authorize(_ context.Context, req *proto.Coordinate } if upd := req.GetUpdateSelf(); upd != nil { - for _, addrStr := range upd.Node.Addresses { - pre, err := netip.ParsePrefix(addrStr) - if err != nil { - return xerrors.Errorf("parse node address: %w", err) - } + // Both Addresses and AllowedIPs are installed into the WireGuard peer + // config and drive routing, so an agent may only advertise prefixes + // derived from its own UUID. Without this an agent could claim a victim + // agent's IP and have traffic routed to it. + if err := a.authorizeNodePrefixes(upd.Node.Addresses); err != nil { + return xerrors.Errorf("Addresses: %w", err) + } + if err := a.authorizeNodePrefixes(upd.Node.AllowedIps); err != nil { + return xerrors.Errorf("AllowedIps: %w", err) + } + } - if pre.Bits() != 128 { - return InvalidAddressBitsError{pre.Bits()} - } + return nil +} - if TailscaleServicePrefix.AddrFromUUID(a.ID).Compare(pre.Addr()) != 0 && - CoderServicePrefix.AddrFromUUID(a.ID).Compare(pre.Addr()) != 0 && - legacyWorkspaceAgentIP.Compare(pre.Addr()) != 0 { - return InvalidNodeAddressError{pre.Addr().String()} - } +// authorizeNodePrefixes verifies that every prefix is a /128 address derived +// from the agent's own UUID (or the legacy workspace agent IP). +func (a AgentCoordinateeAuth) authorizeNodePrefixes(prefixes []string) error { + for _, prefixStr := range prefixes { + pre, err := netip.ParsePrefix(prefixStr) + if err != nil { + return xerrors.Errorf("parse node address: %w", err) + } + + if pre.Bits() != 128 { + return InvalidAddressBitsError{pre.Bits()} + } + + if TailscaleServicePrefix.AddrFromUUID(a.ID).Compare(pre.Addr()) != 0 && + CoderServicePrefix.AddrFromUUID(a.ID).Compare(pre.Addr()) != 0 && + legacyWorkspaceAgentIP.Compare(pre.Addr()) != 0 { + return InvalidNodeAddressError{pre.Addr().String()} } }