use tsaddr library and cleanups (#2150)

* resuse tsaddr code instead of handrolled

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* ensure we dont give out internal tailscale IPs

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* use prefix instead of string for routes

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* remove old custom compare func

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* trim unused util code

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

---------

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
This commit is contained in:
Kristoffer Dalby 2024-10-02 09:06:09 +02:00 committed by GitHub
parent 63035cdb5a
commit 3964dec1c6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 123 additions and 153 deletions

View file

@ -14,6 +14,7 @@ import (
"github.com/rs/zerolog/log"
"go4.org/netipx"
"gorm.io/gorm"
"tailscale.com/net/tsaddr"
)
// IPAllocator is a singleton responsible for allocating
@ -190,8 +191,9 @@ func (i *IPAllocator) next(prev netip.Addr, prefix *netip.Prefix) (*netip.Addr,
return nil, ErrCouldNotAllocateIP
}
// Check if the IP has already been allocated.
if set.Contains(ip) {
// Check if the IP has already been allocated
// or if it is a IP reserved by Tailscale.
if set.Contains(ip) || isTailscaleReservedIP(ip) {
switch i.strategy {
case types.IPAllocationStrategySequential:
ip = ip.Next()
@ -248,6 +250,12 @@ func randomNext(pfx netip.Prefix) (netip.Addr, error) {
return ip, nil
}
func isTailscaleReservedIP(ip netip.Addr) bool {
return tsaddr.ChromeOSVMRange().Contains(ip) ||
tsaddr.TailscaleServiceIP() == ip ||
tsaddr.TailscaleServiceIPv6() == ip
}
// BackfillNodeIPs will take a database transaction, and
// iterate through all of the current nodes in headscale
// and ensure it has IP addresses according to the current

View file

@ -12,6 +12,9 @@ import (
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/stretchr/testify/assert"
"tailscale.com/net/tsaddr"
"tailscale.com/types/ptr"
)
var mpp = func(pref string) *netip.Prefix {
@ -514,3 +517,26 @@ func TestBackfillIPAddresses(t *testing.T) {
})
}
}
func TestIPAllocatorNextNoReservedIPs(t *testing.T) {
alloc, err := NewIPAllocator(db, ptr.To(tsaddr.CGNATRange()), ptr.To(tsaddr.TailscaleULARange()), types.IPAllocationStrategySequential)
if err != nil {
t.Fatalf("failed to set up ip alloc: %s", err)
}
// Validate that we do not give out 100.100.100.100
nextQuad100, err := alloc.next(na("100.100.100.99"), ptr.To(tsaddr.CGNATRange()))
assert.NoError(t, err)
assert.Equal(t, na("100.100.100.101"), *nextQuad100)
// Validate that we do not give out fd7a:115c:a1e0::53
nextQuad100v6, err := alloc.next(na("fd7a:115c:a1e0::52"), ptr.To(tsaddr.TailscaleULARange()))
assert.NoError(t, err)
assert.Equal(t, na("fd7a:115c:a1e0::54"), *nextQuad100v6)
// Validate that we do not give out fd7a:115c:a1e0::53
nextChrome, err := alloc.next(na("100.115.91.255"), ptr.To(tsaddr.CGNATRange()))
t.Logf("chrome: %s", nextChrome.String())
assert.NoError(t, err)
assert.Equal(t, na("100.115.94.0"), *nextChrome)
}

View file

@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"net/netip"
"slices"
"sort"
"sync"
"time"
@ -215,7 +216,7 @@ func SetTags(
var newTags types.StringList
for _, tag := range tags {
if !util.StringOrPrefixListContains(newTags, tag) {
if !slices.Contains(newTags, tag) {
newTags = append(newTags, tag)
}
}
@ -538,34 +539,24 @@ func IsRoutesEnabled(tx *gorm.DB, node *types.Node, routeStr string) bool {
func (hsdb *HSDatabase) enableRoutes(
node *types.Node,
routeStrs ...string,
newRoutes ...netip.Prefix,
) (*types.StateUpdate, error) {
return Write(hsdb.DB, func(tx *gorm.DB) (*types.StateUpdate, error) {
return enableRoutes(tx, node, routeStrs...)
return enableRoutes(tx, node, newRoutes...)
})
}
// enableRoutes enables new routes based on a list of new routes.
func enableRoutes(tx *gorm.DB,
node *types.Node, routeStrs ...string,
node *types.Node, newRoutes ...netip.Prefix,
) (*types.StateUpdate, error) {
newRoutes := make([]netip.Prefix, len(routeStrs))
for index, routeStr := range routeStrs {
route, err := netip.ParsePrefix(routeStr)
if err != nil {
return nil, err
}
newRoutes[index] = route
}
advertisedRoutes, err := GetAdvertisedRoutes(tx, node)
if err != nil {
return nil, err
}
for _, newRoute := range newRoutes {
if !util.StringOrPrefixListContains(advertisedRoutes, newRoute) {
if !slices.Contains(advertisedRoutes, newRoute) {
return nil, fmt.Errorf(
"route (%s) is not available on node %s: %w",
node.Hostname,
@ -607,12 +598,6 @@ func enableRoutes(tx *gorm.DB,
node.Routes = nRoutes
log.Trace().
Caller().
Str("node", node.Hostname).
Strs("routes", routeStrs).
Msg("enabling routes")
return &types.StateUpdate{
Type: types.StatePeerChanged,
ChangeNodes: []types.NodeID{node.ID},

View file

@ -6,7 +6,6 @@ import (
"math/big"
"net/netip"
"regexp"
"sort"
"strconv"
"sync"
"testing"
@ -20,6 +19,7 @@ import (
"github.com/stretchr/testify/assert"
"gopkg.in/check.v1"
"gorm.io/gorm"
"tailscale.com/net/tsaddr"
"tailscale.com/tailcfg"
"tailscale.com/types/key"
"tailscale.com/types/ptr"
@ -528,16 +528,16 @@ func TestAutoApproveRoutes(t *testing.T) {
}
}`,
routes: []netip.Prefix{
netip.MustParsePrefix("0.0.0.0/0"),
netip.MustParsePrefix("::/0"),
tsaddr.AllIPv4(),
tsaddr.AllIPv6(),
netip.MustParsePrefix("10.10.0.0/16"),
netip.MustParsePrefix("10.11.0.0/24"),
},
want: []netip.Prefix{
netip.MustParsePrefix("::/0"),
netip.MustParsePrefix("10.11.0.0/24"),
tsaddr.AllIPv4(),
netip.MustParsePrefix("10.10.0.0/16"),
netip.MustParsePrefix("0.0.0.0/0"),
netip.MustParsePrefix("10.11.0.0/24"),
tsaddr.AllIPv6(),
},
},
}
@ -594,9 +594,7 @@ func TestAutoApproveRoutes(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, enabledRoutes, len(tt.want))
sort.Slice(enabledRoutes, func(i, j int) bool {
return util.ComparePrefix(enabledRoutes[i], enabledRoutes[j]) > 0
})
tsaddr.SortPrefixes(enabledRoutes)
if diff := cmp.Diff(tt.want, enabledRoutes, util.Comparers...); diff != "" {
t.Errorf("unexpected enabled routes (-want +got):\n%s", diff)

View file

@ -11,6 +11,7 @@ import (
"github.com/puzpuzpuz/xsync/v3"
"github.com/rs/zerolog/log"
"gorm.io/gorm"
"tailscale.com/net/tsaddr"
"tailscale.com/util/set"
)
@ -117,12 +118,12 @@ func EnableRoute(tx *gorm.DB, id uint64) (*types.StateUpdate, error) {
return enableRoutes(
tx,
&route.Node,
types.ExitRouteV4.String(),
types.ExitRouteV6.String(),
tsaddr.AllIPv4(),
tsaddr.AllIPv6(),
)
}
return enableRoutes(tx, &route.Node, netip.Prefix(route.Prefix).String())
return enableRoutes(tx, &route.Node, netip.Prefix(route.Prefix))
}
func DisableRoute(tx *gorm.DB,

View file

@ -27,6 +27,10 @@ var smap = func(m map[types.NodeID]bool) *xsync.MapOf[types.NodeID, bool] {
return s
}
var mp = func(p string) netip.Prefix {
return netip.MustParsePrefix(p)
}
func (s *Suite) TestGetRoutes(c *check.C) {
user, err := db.CreateUser("test")
c.Assert(err, check.IsNil)
@ -64,10 +68,10 @@ func (s *Suite) TestGetRoutes(c *check.C) {
c.Assert(len(advertisedRoutes), check.Equals, 1)
// TODO(kradalby): check state update
_, err = db.enableRoutes(&node, "192.168.0.0/24")
_, err = db.enableRoutes(&node, mp("192.168.0.0/24"))
c.Assert(err, check.NotNil)
_, err = db.enableRoutes(&node, "10.0.0.0/24")
_, err = db.enableRoutes(&node, mp("10.0.0.0/24"))
c.Assert(err, check.IsNil)
}
@ -119,10 +123,10 @@ func (s *Suite) TestGetEnableRoutes(c *check.C) {
c.Assert(err, check.IsNil)
c.Assert(len(noEnabledRoutes), check.Equals, 0)
_, err = db.enableRoutes(&node, "192.168.0.0/24")
_, err = db.enableRoutes(&node, mp("192.168.0.0/24"))
c.Assert(err, check.NotNil)
_, err = db.enableRoutes(&node, "10.0.0.0/24")
_, err = db.enableRoutes(&node, mp("10.0.0.0/24"))
c.Assert(err, check.IsNil)
enabledRoutes, err := db.GetEnabledRoutes(&node)
@ -130,14 +134,14 @@ func (s *Suite) TestGetEnableRoutes(c *check.C) {
c.Assert(len(enabledRoutes), check.Equals, 1)
// Adding it twice will just let it pass through
_, err = db.enableRoutes(&node, "10.0.0.0/24")
_, err = db.enableRoutes(&node, mp("10.0.0.0/24"))
c.Assert(err, check.IsNil)
enableRoutesAfterDoubleApply, err := db.GetEnabledRoutes(&node)
c.Assert(err, check.IsNil)
c.Assert(len(enableRoutesAfterDoubleApply), check.Equals, 1)
_, err = db.enableRoutes(&node, "150.0.10.0/25")
_, err = db.enableRoutes(&node, mp("150.0.10.0/25"))
c.Assert(err, check.IsNil)
enabledRoutesWithAdditionalRoute, err := db.GetEnabledRoutes(&node)
@ -183,10 +187,10 @@ func (s *Suite) TestIsUniquePrefix(c *check.C) {
c.Assert(err, check.IsNil)
c.Assert(sendUpdate, check.Equals, false)
_, err = db.enableRoutes(&node1, route.String())
_, err = db.enableRoutes(&node1, route)
c.Assert(err, check.IsNil)
_, err = db.enableRoutes(&node1, route2.String())
_, err = db.enableRoutes(&node1, route2)
c.Assert(err, check.IsNil)
hostInfo2 := tailcfg.Hostinfo{
@ -206,7 +210,7 @@ func (s *Suite) TestIsUniquePrefix(c *check.C) {
c.Assert(err, check.IsNil)
c.Assert(sendUpdate, check.Equals, false)
_, err = db.enableRoutes(&node2, route2.String())
_, err = db.enableRoutes(&node2, route2)
c.Assert(err, check.IsNil)
enabledRoutes1, err := db.GetEnabledRoutes(&node1)
@ -267,10 +271,10 @@ func (s *Suite) TestDeleteRoutes(c *check.C) {
c.Assert(err, check.IsNil)
c.Assert(sendUpdate, check.Equals, false)
_, err = db.enableRoutes(&node1, prefix.String())
_, err = db.enableRoutes(&node1, prefix)
c.Assert(err, check.IsNil)
_, err = db.enableRoutes(&node1, prefix2.String())
_, err = db.enableRoutes(&node1, prefix2)
c.Assert(err, check.IsNil)
routes, err := db.GetNodeRoutes(&node1)