Compare commits

...

8 Commits

Author SHA1 Message Date
kari-ts
b0e1bbb62e VERSION.txt: this is v1.58.2
Signed-off-by: kari-ts <kari@tailscale.com>
2024-01-23 13:54:48 -08:00
Andrew Dunham
80b20dc60f net/portmapper: handle cases where we have no supported clients
This no longer results in a nil pointer exception when we get a valid
UPnP response with no supported clients.

Updates #10911

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I6e3715a49a193ff5261013871ad7fff197a4d77e
(cherry picked from commit b45089ad85)
2024-01-23 13:53:08 -08:00
kari-ts
916de26363 VERSION.txt: this is v1.58.1
Signed-off-by: kari-ts <kari@tailscale.com>
2024-01-23 10:18:58 -08:00
James Tucker
d1810a7e10 appc: add test to ensure that individual IPs are not removed during route updates
If control advised the connector to advertise a route that had already
been discovered by DNS it would be incorrectly removed. Now those routes
are preserved.

Updates tailscale/corp#16833

Signed-off-by: James Tucker <james@tailscale.com>
(cherry picked from commit 0e2cb76abe)
2024-01-23 10:18:26 -08:00
Charlotte Brandhorst-Satzkorn
8ffcd9a11b appc,ipn/ipnlocal: optimize preference adjustments when routes update
This change allows us to perform batch modification for new route
advertisements and route removals. Additionally, we now handle the case
where newly added routes are covered by existing ranges.

This change also introduces a new appctest package that contains some
shared functions used for testing.

Updates tailscale/corp#16833

Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
(cherry picked from commit ce4553b988)
2024-01-23 10:18:07 -08:00
James Tucker
d304bdcae8 ipn/ipnlocal: make app connector configuration concurrent
If there are routes changes as a side effect of an app connector
configuration update, the connector configuration may want to reenter a
lock, so must be started asynchronously.

Updates tailscale/corp#16833
Signed-off-by: James Tucker <james@tailscale.com>
(cherry picked from commit 8250582fe6)
2024-01-23 10:15:11 -08:00
James Tucker
36e3c418aa control/controlclient,util/execqueue: extract execqueue into a package
This is a useful primitive for asynchronous execution of ordered work I
want to use in another change.

Updates tailscale/corp#16833
Signed-off-by: James Tucker <james@tailscale.com>
(cherry picked from commit 38a1cf748a)
2024-01-23 10:14:58 -08:00
kari-ts
dfc5715d94 VERSION.txt: this is v1.58.0
Signed-off-by: kari-ts <kari@tailscale.com>
2024-01-18 12:04:42 -08:00
13 changed files with 630 additions and 217 deletions

View File

@@ -1 +1 @@
1.57.0
1.58.2

View File

@@ -10,6 +10,7 @@
package appc
import (
"context"
"net/netip"
"slices"
"strings"
@@ -20,17 +21,18 @@ import (
"tailscale.com/types/logger"
"tailscale.com/types/views"
"tailscale.com/util/dnsname"
"tailscale.com/util/execqueue"
)
// RouteAdvertiser is an interface that allows the AppConnector to advertise
// newly discovered routes that need to be served through the AppConnector.
type RouteAdvertiser interface {
// AdvertiseRoute adds a new route advertisement if the route is not already
// being advertised.
AdvertiseRoute(netip.Prefix) error
// AdvertiseRoute adds one or more route advertisements skipping any that
// are already advertised.
AdvertiseRoute(...netip.Prefix) error
// UnadvertiseRoute removes a route advertisement.
UnadvertiseRoute(netip.Prefix) error
// UnadvertiseRoute removes any matching route advertisements.
UnadvertiseRoute(...netip.Prefix) error
}
// AppConnector is an implementation of an AppConnector that performs
@@ -58,6 +60,9 @@ type AppConnector struct {
// wildcards is the list of domain strings that match subdomains.
wildcards []string
// queue provides ordering for update operations
queue execqueue.ExecQueue
}
// NewAppConnector creates a new AppConnector.
@@ -68,11 +73,33 @@ func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *AppConn
}
}
// UpdateDomains replaces the current set of configured domains with the
// supplied set of domains. Domains must not contain a trailing dot, and should
// be lower case. If the domain contains a leading '*' label it matches all
// subdomains of a domain.
// UpdateDomainsAndRoutes starts an asynchronous update of the configuration
// given the new domains and routes.
func (e *AppConnector) UpdateDomainsAndRoutes(domains []string, routes []netip.Prefix) {
e.queue.Add(func() {
// Add the new routes first.
e.updateRoutes(routes)
e.updateDomains(domains)
})
}
// UpdateDomains asynchronously replaces the current set of configured domains
// with the supplied set of domains. Domains must not contain a trailing dot,
// and should be lower case. If the domain contains a leading '*' label it
// matches all subdomains of a domain.
func (e *AppConnector) UpdateDomains(domains []string) {
e.queue.Add(func() {
e.updateDomains(domains)
})
}
// Wait waits for the currently scheduled asynchronous configuration changes to
// complete.
func (e *AppConnector) Wait(ctx context.Context) {
e.queue.Wait(ctx)
}
func (e *AppConnector) updateDomains(domains []string) {
e.mu.Lock()
defer e.mu.Unlock()
@@ -104,11 +131,11 @@ func (e *AppConnector) UpdateDomains(domains []string) {
e.logf("handling domains: %v and wildcards: %v", xmaps.Keys(e.domains), e.wildcards)
}
// UpdateRoutes merges the supplied routes into the currently configured routes. The routes supplied
// updateRoutes merges the supplied routes into the currently configured routes. The routes supplied
// by control for UpdateRoutes are supplemental to the routes discovered by DNS resolution, but are
// also more often whole ranges. UpdateRoutes will remove any single address routes that are now
// covered by new ranges.
func (e *AppConnector) UpdateRoutes(routes []netip.Prefix) {
func (e *AppConnector) updateRoutes(routes []netip.Prefix) {
e.mu.Lock()
defer e.mu.Unlock()
@@ -117,26 +144,30 @@ func (e *AppConnector) UpdateRoutes(routes []netip.Prefix) {
return
}
if err := e.routeAdvertiser.AdvertiseRoute(routes...); err != nil {
e.logf("failed to advertise routes: %v: %v", routes, err)
return
}
var toRemove []netip.Prefix
nextRoute:
for _, r := range routes {
if err := e.routeAdvertiser.AdvertiseRoute(r); err != nil {
e.logf("failed to advertise route: %v: %v", r, err)
continue
}
for _, addr := range e.domains {
for _, a := range addr {
if r.Contains(a) {
if r.Contains(a) && netip.PrefixFrom(a, a.BitLen()) != r {
pfx := netip.PrefixFrom(a, a.BitLen())
if err := e.routeAdvertiser.UnadvertiseRoute(pfx); err != nil {
e.logf("failed to unadvertise route: %v: %v", pfx, err)
}
toRemove = append(toRemove, pfx)
continue nextRoute
}
}
}
}
if err := e.routeAdvertiser.UnadvertiseRoute(toRemove...); err != nil {
e.logf("failed to unadvertise routes: %v: %v", toRemove, err)
}
e.controlRoutes = routes
}

View File

@@ -4,6 +4,7 @@
package appc
import (
"context"
"net/netip"
"reflect"
"slices"
@@ -11,13 +12,17 @@ import (
xmaps "golang.org/x/exp/maps"
"golang.org/x/net/dns/dnsmessage"
"tailscale.com/appc/appctest"
"tailscale.com/util/mak"
"tailscale.com/util/must"
)
func TestUpdateDomains(t *testing.T) {
ctx := context.Background()
a := NewAppConnector(t.Logf, nil)
a.UpdateDomains([]string{"example.com"})
a.Wait(ctx)
if got, want := a.Domains().AsSlice(), []string{"example.com"}; !slices.Equal(got, want) {
t.Errorf("got %v; want %v", got, want)
}
@@ -25,6 +30,7 @@ func TestUpdateDomains(t *testing.T) {
addr := netip.MustParseAddr("192.0.0.8")
a.domains["example.com"] = append(a.domains["example.com"], addr)
a.UpdateDomains([]string{"example.com"})
a.Wait(ctx)
if got, want := a.domains["example.com"], []netip.Addr{addr}; !slices.Equal(got, want) {
t.Errorf("got %v; want %v", got, want)
@@ -32,39 +38,66 @@ func TestUpdateDomains(t *testing.T) {
// domains are explicitly downcased on set.
a.UpdateDomains([]string{"UP.EXAMPLE.COM"})
a.Wait(ctx)
if got, want := xmaps.Keys(a.domains), []string{"up.example.com"}; !slices.Equal(got, want) {
t.Errorf("got %v; want %v", got, want)
}
}
func TestUpdateRoutes(t *testing.T) {
rc := &routeCollector{}
ctx := context.Background()
rc := &appctest.RouteCollector{}
a := NewAppConnector(t.Logf, rc)
routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}
a.UpdateRoutes(routes)
a.updateDomains([]string{"*.example.com"})
if !slices.EqualFunc(routes, rc.routes, prefixEqual) {
t.Fatalf("got %v, want %v", rc.routes, routes)
// This route should be collapsed into the range
a.ObserveDNSResponse(dnsResponse("a.example.com.", "192.0.2.1"))
a.Wait(ctx)
if !slices.Equal(rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) {
t.Fatalf("got %v, want %v", rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")})
}
// This route should not be collapsed or removed
a.ObserveDNSResponse(dnsResponse("b.example.com.", "192.0.0.1"))
a.Wait(ctx)
routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24"), netip.MustParsePrefix("192.0.0.1/32")}
a.updateRoutes(routes)
slices.SortFunc(rc.Routes(), prefixCompare)
rc.SetRoutes(slices.Compact(rc.Routes()))
slices.SortFunc(routes, prefixCompare)
// Ensure that the non-matching /32 is preserved, even though it's in the domains table.
if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) {
t.Errorf("added routes: got %v, want %v", rc.Routes(), routes)
}
// Ensure that the contained /32 is removed, replaced by the /24.
wantRemoved := []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}
if !slices.EqualFunc(rc.RemovedRoutes(), wantRemoved, prefixEqual) {
t.Fatalf("unexpected removed routes: %v", rc.RemovedRoutes())
}
}
func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) {
rc := &routeCollector{}
rc := &appctest.RouteCollector{}
a := NewAppConnector(t.Logf, rc)
mak.Set(&a.domains, "example.com", []netip.Addr{netip.MustParseAddr("192.0.2.1")})
rc.routes = []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}
rc.SetRoutes([]netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")})
routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}
a.UpdateRoutes(routes)
a.updateRoutes(routes)
if !slices.EqualFunc(routes, rc.routes, prefixEqual) {
t.Fatalf("got %v, want %v", rc.routes, routes)
if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) {
t.Fatalf("got %v, want %v", rc.Routes(), routes)
}
}
func TestDomainRoutes(t *testing.T) {
rc := &routeCollector{}
rc := &appctest.RouteCollector{}
a := NewAppConnector(t.Logf, rc)
a.UpdateDomains([]string{"example.com"})
a.updateDomains([]string{"example.com"})
a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
want := map[string][]netip.Addr{
@@ -77,43 +110,43 @@ func TestDomainRoutes(t *testing.T) {
}
func TestObserveDNSResponse(t *testing.T) {
rc := &routeCollector{}
rc := &appctest.RouteCollector{}
a := NewAppConnector(t.Logf, rc)
// a has no domains configured, so it should not advertise any routes
a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
if got, want := rc.routes, ([]netip.Prefix)(nil); !slices.Equal(got, want) {
if got, want := rc.Routes(), ([]netip.Prefix)(nil); !slices.Equal(got, want) {
t.Errorf("got %v; want %v", got, want)
}
wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}
a.UpdateDomains([]string{"example.com"})
a.updateDomains([]string{"example.com"})
a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
if got, want := rc.routes, wantRoutes; !slices.Equal(got, want) {
if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) {
t.Errorf("got %v; want %v", got, want)
}
wantRoutes = append(wantRoutes, netip.MustParsePrefix("2001:db8::1/128"))
a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1"))
if got, want := rc.routes, wantRoutes; !slices.Equal(got, want) {
if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) {
t.Errorf("got %v; want %v", got, want)
}
// don't re-advertise routes that have already been advertised
a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1"))
if !slices.Equal(rc.routes, wantRoutes) {
t.Errorf("rc.routes: got %v; want %v", rc.routes, wantRoutes)
if !slices.Equal(rc.Routes(), wantRoutes) {
t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes)
}
// don't advertise addresses that are already in a control provided route
pfx := netip.MustParsePrefix("192.0.2.0/24")
a.UpdateRoutes([]netip.Prefix{pfx})
a.updateRoutes([]netip.Prefix{pfx})
wantRoutes = append(wantRoutes, pfx)
a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.2.1"))
if !slices.Equal(rc.routes, wantRoutes) {
t.Errorf("rc.routes: got %v; want %v", rc.routes, wantRoutes)
if !slices.Equal(rc.Routes(), wantRoutes) {
t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes)
}
if !slices.Contains(a.domains["example.com"], netip.MustParseAddr("192.0.2.1")) {
t.Errorf("missing %v from %v", "192.0.2.1", a.domains["exmaple.com"])
@@ -121,19 +154,19 @@ func TestObserveDNSResponse(t *testing.T) {
}
func TestWildcardDomains(t *testing.T) {
rc := &routeCollector{}
rc := &appctest.RouteCollector{}
a := NewAppConnector(t.Logf, rc)
a.UpdateDomains([]string{"*.example.com"})
a.updateDomains([]string{"*.example.com"})
a.ObserveDNSResponse(dnsResponse("foo.example.com.", "192.0.0.8"))
if got, want := rc.routes, []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}; !slices.Equal(got, want) {
if got, want := rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}; !slices.Equal(got, want) {
t.Errorf("routes: got %v; want %v", got, want)
}
if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) {
t.Errorf("wildcards: got %v; want %v", got, want)
}
a.UpdateDomains([]string{"*.example.com", "example.com"})
a.updateDomains([]string{"*.example.com", "example.com"})
if _, ok := a.domains["foo.example.com"]; !ok {
t.Errorf("expected foo.example.com to be preserved in domains due to wildcard")
}
@@ -142,7 +175,7 @@ func TestWildcardDomains(t *testing.T) {
}
// There was an early regression where the wildcard domain was added repeatedly, this guards against that.
a.UpdateDomains([]string{"*.example.com", "example.com"})
a.updateDomains([]string{"*.example.com", "example.com"})
if len(a.wildcards) != 1 {
t.Errorf("expected only one wildcard domain, got %v", a.wildcards)
}
@@ -185,30 +218,13 @@ func dnsResponse(domain, address string) []byte {
return must.Get(b.Finish())
}
// routeCollector is a test helper that collects the list of routes advertised
type routeCollector struct {
routes []netip.Prefix
}
// routeCollector implements RouteAdvertiser
var _ RouteAdvertiser = (*routeCollector)(nil)
func (rc *routeCollector) AdvertiseRoute(pfx netip.Prefix) error {
rc.routes = append(rc.routes, pfx)
return nil
}
func (rc *routeCollector) UnadvertiseRoute(pfx netip.Prefix) error {
routes := rc.routes
rc.routes = rc.routes[:0]
for _, r := range routes {
if r != pfx {
rc.routes = append(rc.routes, r)
}
}
return nil
}
func prefixEqual(a, b netip.Prefix) bool {
return a.Addr().Compare(b.Addr()) == 0 && a.Bits() == b.Bits()
return a == b
}
func prefixCompare(a, b netip.Prefix) int {
if a.Addr().Compare(b.Addr()) == 0 {
return a.Bits() - b.Bits()
}
return a.Addr().Compare(b.Addr())
}

49
appc/appctest/appctest.go Normal file
View File

@@ -0,0 +1,49 @@
// Copyright (c) Tailscale Inc & AUTHORS
// SPDX-License-Identifier: BSD-3-Clause
package appctest
import (
"net/netip"
"slices"
)
// RouteCollector is a test helper that collects the list of routes advertised
type RouteCollector struct {
routes []netip.Prefix
removedRoutes []netip.Prefix
}
func (rc *RouteCollector) AdvertiseRoute(pfx ...netip.Prefix) error {
rc.routes = append(rc.routes, pfx...)
return nil
}
func (rc *RouteCollector) UnadvertiseRoute(toRemove ...netip.Prefix) error {
routes := rc.routes
rc.routes = rc.routes[:0]
for _, r := range routes {
if !slices.Contains(toRemove, r) {
rc.routes = append(rc.routes, r)
} else {
rc.removedRoutes = append(rc.removedRoutes, r)
}
}
return nil
}
// RemovedRoutes returns the list of routes that were removed.
func (rc *RouteCollector) RemovedRoutes() []netip.Prefix {
return rc.removedRoutes
}
// Routes returns the ordered list of routes that were added, including
// possible duplicates.
func (rc *RouteCollector) Routes() []netip.Prefix {
return rc.routes
}
func (rc *RouteCollector) SetRoutes(routes []netip.Prefix) error {
rc.routes = routes
return nil
}

View File

@@ -348,6 +348,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
💣 tailscale.com/util/deephash from tailscale.com/ipn/ipnlocal+
L 💣 tailscale.com/util/dirwalk from tailscale.com/metrics+
tailscale.com/util/dnsname from tailscale.com/hostinfo+
tailscale.com/util/execqueue from tailscale.com/control/controlclient+
tailscale.com/util/goroutines from tailscale.com/ipn/ipnlocal
tailscale.com/util/groupmember from tailscale.com/ipn/ipnauth+
💣 tailscale.com/util/hashx from tailscale.com/util/deephash

View File

@@ -22,6 +22,7 @@ import (
"tailscale.com/types/netmap"
"tailscale.com/types/persist"
"tailscale.com/types/structs"
"tailscale.com/util/execqueue"
)
type LoginGoal struct {
@@ -118,7 +119,7 @@ type Auto struct {
closed bool
updateCh chan struct{} // readable when we should inform the server of a change
observer Observer // called to update Client status; always non-nil
observerQueue execQueue
observerQueue execqueue.ExecQueue
unregisterHealthWatch func()
@@ -675,7 +676,7 @@ func (c *Auto) Shutdown() {
direct := c.direct
if !closed {
c.closed = true
c.observerQueue.shutdown()
c.observerQueue.Shutdown()
c.cancelAuthCtxLocked()
c.cancelMapCtxLocked()
for _, w := range c.unpauseWaiters {
@@ -696,7 +697,7 @@ func (c *Auto) Shutdown() {
}
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
c.observerQueue.wait(ctx)
c.observerQueue.Wait(ctx)
c.logf("Client.Shutdown done.")
}
}
@@ -737,95 +738,3 @@ func (c *Auto) DoNoiseRequest(req *http.Request) (*http.Response, error) {
func (c *Auto) GetSingleUseNoiseRoundTripper(ctx context.Context) (http.RoundTripper, *tailcfg.EarlyNoise, error) {
return c.direct.GetSingleUseNoiseRoundTripper(ctx)
}
type execQueue struct {
mu sync.Mutex
closed bool
inFlight bool // whether a goroutine is running q.run
doneWaiter chan struct{} // non-nil if waiter is waiting, then closed
queue []func()
}
func (q *execQueue) Add(f func()) {
q.mu.Lock()
defer q.mu.Unlock()
if q.closed {
return
}
if q.inFlight {
q.queue = append(q.queue, f)
} else {
q.inFlight = true
go q.run(f)
}
}
// RunSync waits for the queue to be drained and then synchronously runs f.
// It returns an error if the queue is closed before f is run or ctx expires.
func (q *execQueue) RunSync(ctx context.Context, f func()) error {
for {
if err := q.wait(ctx); err != nil {
return err
}
q.mu.Lock()
if q.inFlight {
q.mu.Unlock()
continue
}
defer q.mu.Unlock()
if q.closed {
return errors.New("closed")
}
f()
return nil
}
}
func (q *execQueue) run(f func()) {
f()
q.mu.Lock()
for len(q.queue) > 0 && !q.closed {
f := q.queue[0]
q.queue[0] = nil
q.queue = q.queue[1:]
q.mu.Unlock()
f()
q.mu.Lock()
}
q.inFlight = false
q.queue = nil
if q.doneWaiter != nil {
close(q.doneWaiter)
q.doneWaiter = nil
}
q.mu.Unlock()
}
func (q *execQueue) shutdown() {
q.mu.Lock()
defer q.mu.Unlock()
q.closed = true
}
// wait waits for the queue to be empty.
func (q *execQueue) wait(ctx context.Context) error {
q.mu.Lock()
waitCh := q.doneWaiter
if q.inFlight && waitCh == nil {
waitCh = make(chan struct{})
q.doneWaiter = waitCh
}
q.mu.Unlock()
if waitCh == nil {
return nil
}
select {
case <-waitCh:
return nil
case <-ctx.Done():
return ctx.Err()
}
}

View File

@@ -3460,8 +3460,7 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
slices.SortFunc(routes, func(i, j netip.Prefix) int { return i.Addr().Compare(j.Addr()) })
domains = slices.Compact(domains)
routes = slices.Compact(routes)
b.appConnector.UpdateRoutes(routes)
b.appConnector.UpdateDomains(domains)
b.appConnector.UpdateDomainsAndRoutes(domains, routes)
}
// authReconfig pushes a new configuration into wgengine, if engine
@@ -5791,45 +5790,73 @@ var ErrDisallowedAutoRoute = errors.New("route is not allowed")
// AdvertiseRoute implements the appc.RouteAdvertiser interface. It sets a new
// route advertisement if one is not already present in the existing routes.
// If the route is disallowed, ErrDisallowedAutoRoute is returned.
func (b *LocalBackend) AdvertiseRoute(ipp netip.Prefix) error {
if !allowedAutoRoute(ipp) {
return ErrDisallowedAutoRoute
func (b *LocalBackend) AdvertiseRoute(ipps ...netip.Prefix) error {
finalRoutes := b.Prefs().AdvertiseRoutes().AsSlice()
newRoutes := false
for _, ipp := range ipps {
if !allowedAutoRoute(ipp) {
continue
}
if slices.Contains(finalRoutes, ipp) {
continue
}
// If the new prefix is already contained by existing routes, skip it.
if coveredRouteRange(finalRoutes, ipp) {
continue
}
finalRoutes = append(finalRoutes, ipp)
newRoutes = true
}
currentRoutes := b.Prefs().AdvertiseRoutes()
if currentRoutes.ContainsFunc(func(r netip.Prefix) bool {
// TODO(raggi): add support for subset checks and avoid subset route creations.
return ipp.IsSingleIP() && r.Contains(ipp.Addr()) || r == ipp
}) {
if !newRoutes {
return nil
}
routes := append(currentRoutes.AsSlice(), ipp)
_, err := b.EditPrefs(&ipn.MaskedPrefs{
Prefs: ipn.Prefs{
AdvertiseRoutes: routes,
AdvertiseRoutes: finalRoutes,
},
AdvertiseRoutesSet: true,
})
return err
}
// coveredRouteRange checks if a route is already included in a slice of
// prefixes.
func coveredRouteRange(finalRoutes []netip.Prefix, ipp netip.Prefix) bool {
for _, r := range finalRoutes {
if ipp.IsSingleIP() {
if r.Contains(ipp.Addr()) {
return true
}
} else {
if r.Contains(ipp.Addr()) && r.Contains(netipx.PrefixLastIP(ipp)) {
return true
}
}
}
return false
}
// UnadvertiseRoute implements the appc.RouteAdvertiser interface. It removes
// a route advertisement if one is present in the existing routes.
func (b *LocalBackend) UnadvertiseRoute(ipp netip.Prefix) error {
func (b *LocalBackend) UnadvertiseRoute(toRemove ...netip.Prefix) error {
currentRoutes := b.Prefs().AdvertiseRoutes().AsSlice()
if !slices.Contains(currentRoutes, ipp) {
return nil
}
finalRoutes := currentRoutes[:0]
newRoutes := currentRoutes[:0]
for _, r := range currentRoutes {
if r != ipp {
newRoutes = append(newRoutes, r)
for _, ipp := range currentRoutes {
if slices.Contains(toRemove, ipp) {
continue
}
finalRoutes = append(finalRoutes, ipp)
}
_, err := b.EditPrefs(&ipn.MaskedPrefs{
Prefs: ipn.Prefs{
AdvertiseRoutes: newRoutes,
AdvertiseRoutes: finalRoutes,
},
AdvertiseRoutesSet: true,
})

View File

@@ -18,6 +18,7 @@ import (
"go4.org/netipx"
"golang.org/x/net/dns/dnsmessage"
"tailscale.com/appc"
"tailscale.com/appc/appctest"
"tailscale.com/control/controlclient"
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
@@ -1204,14 +1205,52 @@ func TestObserveDNSResponse(t *testing.T) {
// ensure no error when no app connector is configured
b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
rc := &routeCollector{}
rc := &appctest.RouteCollector{}
b.appConnector = appc.NewAppConnector(t.Logf, rc)
b.appConnector.UpdateDomains([]string{"example.com"})
b.appConnector.Wait(context.Background())
b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
b.appConnector.Wait(context.Background())
wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}
if !slices.Equal(rc.routes, wantRoutes) {
t.Fatalf("got routes %v, want %v", rc.routes, wantRoutes)
if !slices.Equal(rc.Routes(), wantRoutes) {
t.Fatalf("got routes %v, want %v", rc.Routes(), wantRoutes)
}
}
func TestCoveredRouteRange(t *testing.T) {
tests := []struct {
existingRoute netip.Prefix
newRoute netip.Prefix
want bool
}{
{
existingRoute: netip.MustParsePrefix("192.0.0.1/32"),
newRoute: netip.MustParsePrefix("192.0.0.1/32"),
want: true,
},
{
existingRoute: netip.MustParsePrefix("192.0.0.1/32"),
newRoute: netip.MustParsePrefix("192.0.0.2/32"),
want: false,
},
{
existingRoute: netip.MustParsePrefix("192.0.0.0/24"),
newRoute: netip.MustParsePrefix("192.0.0.1/32"),
want: true,
},
{
existingRoute: netip.MustParsePrefix("192.0.0.0/16"),
newRoute: netip.MustParsePrefix("192.0.0.0/24"),
want: true,
},
}
for _, tt := range tests {
got := coveredRouteRange([]netip.Prefix{tt.existingRoute}, tt.newRoute)
if got != tt.want {
t.Errorf("coveredRouteRange(%v, %v) = %v, want %v", tt.existingRoute, tt.newRoute, got, tt.want)
}
}
}
@@ -1250,6 +1289,7 @@ func TestReconfigureAppConnector(t *testing.T) {
}).View()
b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
b.appConnector.Wait(context.Background())
want := []string{"example.com"}
if !slices.Equal(b.appConnector.Domains().AsSlice(), want) {
@@ -1349,27 +1389,6 @@ func dnsResponse(domain, address string) []byte {
return must.Get(b.Finish())
}
// routeCollector is a test helper that collects the list of routes advertised
type routeCollector struct {
routes []netip.Prefix
}
func (rc *routeCollector) AdvertiseRoute(pfx netip.Prefix) error {
rc.routes = append(rc.routes, pfx)
return nil
}
func (rc *routeCollector) UnadvertiseRoute(pfx netip.Prefix) error {
routes := rc.routes
rc.routes = rc.routes[:0]
for _, r := range routes {
if r != pfx {
rc.routes = append(rc.routes, r)
}
}
return nil
}
type errorSyspolicyHandler struct {
t *testing.T
err error

View File

@@ -23,6 +23,7 @@ import (
"go4.org/netipx"
"golang.org/x/net/dns/dnsmessage"
"tailscale.com/appc"
"tailscale.com/appc/appctest"
"tailscale.com/client/tailscale/apitype"
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
@@ -685,10 +686,11 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) {
}
func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
ctx := context.Background()
var h peerAPIHandler
h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
rc := &routeCollector{}
rc := &appctest.RouteCollector{}
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
h.ps = &peerAPIServer{
@@ -700,6 +702,7 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
},
}
h.ps.b.appConnector.UpdateDomains([]string{"example.com"})
h.ps.b.appConnector.Wait(ctx)
h.ps.resolver = &fakeResolver{}
f := filter.NewAllowAllForTest(logger.Discard)
@@ -717,10 +720,11 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
if w.Code != http.StatusOK {
t.Errorf("unexpected status code: %v", w.Code)
}
h.ps.b.appConnector.Wait(ctx)
wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}
if !slices.Equal(rc.routes, wantRoutes) {
t.Errorf("got %v; want %v", rc.routes, wantRoutes)
if !slices.Equal(rc.Routes(), wantRoutes) {
t.Errorf("got %v; want %v", rc.Routes(), wantRoutes)
}
}

View File

@@ -252,7 +252,8 @@ func getUPnPRootDevice(ctx context.Context, logf logger.Logf, debug DebugKnobs,
}
// selectBestService picks the "best" service from the given UPnP root device
// to use to create a port mapping.
// to use to create a port mapping. It may return (nil, nil) if no supported
// service was found in the provided *goupnp.RootDevice.
//
// loc is the parsed location that was used to fetch the given RootDevice.
//
@@ -559,6 +560,20 @@ func (c *Client) tryUPnPPortmapWithDevice(
return netip.AddrPort{}, nil, err
}
// If we have no client, we cannot continue; this can happen if we get
// a valid UPnP response that does not contain any of the service types
// that we know how to use.
if client == nil {
// For debugging, print all available services that we aren't
// using because they're not supported; use c.vlogf so we don't
// spam the logs unless verbose debugging is turned on.
rootDev.Device.VisitServices(func(s *goupnp.Service) {
c.vlogf("unsupported UPnP service: Type=%q ID=%q ControlURL=%q", s.ServiceType, s.ServiceId, s.ControlURL.Str)
})
return netip.AddrPort{}, nil, fmt.Errorf("no supported UPnP clients")
}
// Start by trying to make a temporary lease with a duration.
var newPort uint16
newPort, err = addAnyPortMapping(

View File

@@ -165,6 +165,172 @@ const (
</device>
<disabledForTestURLBase>http://10.0.0.1:2828</disabledForTestURLBase>
</root>
`
// Huawei, https://github.com/tailscale/tailscale/issues/10911
huaweiRootDescXML = `<?xml version="1.0"?>
<root xmlns="urn:schemas-upnp-org:device-1-0">
<specVersion>
<major>1</major>
<minor>0</minor>
</specVersion>
<device>
<deviceType>urn:dslforum-org:device:InternetGatewayDevice:1</deviceType>
<friendlyName>HG531 V1</friendlyName>
<manufacturer>Huawei Technologies Co., Ltd.</manufacturer>
<manufacturerURL>http://www.huawei.com</manufacturerURL>
<modelDescription>Huawei Home Gateway</modelDescription>
<modelName>HG531 V1</modelName>
<modelNumber>Huawei Model</modelNumber>
<modelURL>http://www.huawei.com</modelURL>
<serialNumber>G6J8W15326003974</serialNumber>
<UDN>uuid:00e0fc37-2626-2828-2600-587f668bdd9a</UDN>
<UPC>000000000001</UPC>
<serviceList>
<service>
<serviceType>urn:www-huawei-com:service:DeviceConfig:1</serviceType>
<serviceId>urn:www-huawei-com:serviceId:DeviceConfig1</serviceId>
<SCPDURL>/desc/DevCfg.xml</SCPDURL>
<controlURL>/ctrlt/DeviceConfig_1</controlURL>
<eventSubURL>/evt/DeviceConfig_1</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:LANConfigSecurity:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:LANConfigSecurity1</serviceId>
<SCPDURL>/desc/LANSec.xml</SCPDURL>
<controlURL>/ctrlt/LANConfigSecurity_1</controlURL>
<eventSubURL>/evt/LANConfigSecurity_1</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:Layer3Forwarding:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:Layer3Forwarding1</serviceId>
<SCPDURL>/desc/L3Fwd.xml</SCPDURL>
<controlURL>/ctrlt/Layer3Forwarding_1</controlURL>
<eventSubURL>/evt/Layer3Forwarding_1</eventSubURL>
</service>
</serviceList>
<deviceList>
<device>
<deviceType>urn:dslforum-org:device:WANDevice:1</deviceType>
<friendlyName>WANDevice</friendlyName>
<manufacturer>Huawei Technologies Co., Ltd.</manufacturer>
<manufacturerURL>http://www.huawei.com</manufacturerURL>
<modelDescription>Huawei Home Gateway</modelDescription>
<modelName>HG531 V1</modelName>
<modelNumber>Huawei Model</modelNumber>
<modelURL>http://www.huawei.com</modelURL>
<serialNumber>G6J8W15326003974</serialNumber>
<UDN>uuid:00e0fc37-2626-2828-2601-587f668bdd9a</UDN>
<UPC>000000000001</UPC>
<serviceList>
<service>
<serviceType>urn:dslforum-org:service:WANDSLInterfaceConfig:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WANDSLInterfaceConfig1</serviceId>
<SCPDURL>/desc/WanDslIfCfg.xml</SCPDURL>
<controlURL>/ctrlt/WANDSLInterfaceConfig_1</controlURL>
<eventSubURL>/evt/WANDSLInterfaceConfig_1</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:WANCommonInterfaceConfig:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WANCommonInterfaceConfig1</serviceId>
<SCPDURL>/desc/WanCommonIfc1.xml</SCPDURL>
<controlURL>/ctrlt/WANCommonInterfaceConfig_1</controlURL>
<eventSubURL>/evt/WANCommonInterfaceConfig_1</eventSubURL>
</service>
</serviceList>
<deviceList>
<device>
<deviceType>urn:dslforum-org:device:WANConnectionDevice:1</deviceType>
<friendlyName>WANConnectionDevice</friendlyName>
<manufacturer>Huawei Technologies Co., Ltd.</manufacturer>
<manufacturerURL>http://www.huawei.com</manufacturerURL>
<modelDescription>Huawei Home Gateway</modelDescription>
<modelName>HG531 V1</modelName>
<modelNumber>Huawei Model</modelNumber>
<modelURL>http://www.huawei.com</modelURL>
<serialNumber>G6J8W15326003974</serialNumber>
<UDN>uuid:00e0fc37-2626-2828-2603-587f668bdd9a</UDN>
<UPC>000000000001</UPC>
<serviceList>
<service>
<serviceType>urn:dslforum-org:service:WANPPPConnection:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WANPPPConnection1</serviceId>
<SCPDURL>/desc/WanPppConn.xml</SCPDURL>
<controlURL>/ctrlt/WANPPPConnection_1</controlURL>
<eventSubURL>/evt/WANPPPConnection_1</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:WANEthernetConnectionManagement:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WANEthernetConnectionManagement1</serviceId>
<SCPDURL>/desc/WanEthConnMgt.xml</SCPDURL>
<controlURL>/ctrlt/WANEthernetConnectionManagement_1</controlURL>
<eventSubURL>/evt/WANEthernetConnectionManagement_1</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:WANDSLLinkConfig:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WANDSLLinkConfig1</serviceId>
<SCPDURL>/desc/WanDslLink.xml</SCPDURL>
<controlURL>/ctrlt/WANDSLLinkConfig_1</controlURL>
<eventSubURL>/evt/WANDSLLinkConfig_1</eventSubURL>
</service>
</serviceList>
</device>
</deviceList>
</device>
<device>
<deviceType>urn:dslforum-org:device:LANDevice:1</deviceType>
<friendlyName>LANDevice</friendlyName>
<manufacturer>Huawei Technologies Co., Ltd.</manufacturer>
<manufacturerURL>http://www.huawei.com</manufacturerURL>
<modelDescription>Huawei Home Gateway</modelDescription>
<modelName>HG531 V1</modelName>
<modelNumber>Huawei Model</modelNumber>
<modelURL>http://www.huawei.com</modelURL>
<serialNumber>G6J8W15326003974</serialNumber>
<UDN>uuid:00e0fc37-2626-2828-2602-587f668bdd9a</UDN>
<UPC>000000000001</UPC>
<serviceList>
<service>
<serviceType>urn:dslforum-org:service:WLANConfiguration:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WLANConfiguration4</serviceId>
<SCPDURL>/desc/WLANCfg.xml</SCPDURL>
<controlURL>/ctrlt/WLANConfiguration_4</controlURL>
<eventSubURL>/evt/WLANConfiguration_4</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:WLANConfiguration:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WLANConfiguration3</serviceId>
<SCPDURL>/desc/WLANCfg.xml</SCPDURL>
<controlURL>/ctrlt/WLANConfiguration_3</controlURL>
<eventSubURL>/evt/WLANConfiguration_3</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:WLANConfiguration:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WLANConfiguration2</serviceId>
<SCPDURL>/desc/WLANCfg.xml</SCPDURL>
<controlURL>/ctrlt/WLANConfiguration_2</controlURL>
<eventSubURL>/evt/WLANConfiguration_2</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:WLANConfiguration:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:WLANConfiguration1</serviceId>
<SCPDURL>/desc/WLANCfg.xml</SCPDURL>
<controlURL>/ctrlt/WLANConfiguration_1</controlURL>
<eventSubURL>/evt/WLANConfiguration_1</eventSubURL>
</service>
<service>
<serviceType>urn:dslforum-org:service:LANHostConfigManagement:1</serviceType>
<serviceId>urn:dslforum-org:serviceId:LANHostConfigManagement1</serviceId>
<SCPDURL>/desc/LanHostCfgMgmt.xml</SCPDURL>
<controlURL>/ctrlt/LANHostConfigManagement_1</controlURL>
<eventSubURL>/evt/LANHostConfigManagement_1</eventSubURL>
</service>
</serviceList>
</device>
</deviceList>
<presentationURL>http://127.0.0.1</presentationURL>
</device>
</root>
`
)
@@ -233,6 +399,14 @@ func TestGetUPnPClient(t *testing.T) {
"*internetgateway2.WANIPConnection1",
"saw UPnP type WANIPConnection1 at http://127.0.0.1:NNN/rootDesc.xml; MikroTik Router (MikroTik), method=none\n",
},
{
"huawei",
huaweiRootDescXML,
// services not supported and thus returns nil, but shouldn't crash
"<nil>",
"",
},
// TODO(bradfitz): find a PPP one in the wild
}
for _, tt := range tests {
@@ -375,6 +549,48 @@ func TestGetUPnPPortMapping(t *testing.T) {
}
}
// TestGetUPnPPortMapping_NoValidServices tests that getUPnPPortMapping doesn't
// crash when a valid UPnP response with no supported services is discovered
// and parsed.
//
// See https://github.com/tailscale/tailscale/issues/10911
func TestGetUPnPPortMapping_NoValidServices(t *testing.T) {
igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
if err != nil {
t.Fatal(err)
}
defer igd.Close()
igd.SetUPnPHandler(&upnpServer{
t: t,
Desc: huaweiRootDescXML,
})
c := newTestClient(t, igd)
defer c.Close()
c.debug.VerboseLogs = true
ctx := context.Background()
res, err := c.Probe(ctx)
if err != nil {
t.Fatalf("Probe: %v", err)
}
if !res.UPnP {
t.Errorf("didn't detect UPnP")
}
gw, myIP, ok := c.gatewayAndSelfIP()
if !ok {
t.Fatalf("could not get gateway and self IP")
}
// This shouldn't panic
_, ok = c.getUPnPPortMapping(ctx, gw, netip.AddrPortFrom(myIP, 12345), 0)
if ok {
t.Fatal("did not expect to get UPnP port mapping")
}
}
func TestGetUPnPPortMappingNoResponses(t *testing.T) {
igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
if err != nil {

104
util/execqueue/execqueue.go Normal file
View File

@@ -0,0 +1,104 @@
// Copyright (c) Tailscale Inc & AUTHORS
// SPDX-License-Identifier: BSD-3-Clause
// Package execqueue implements an ordered asynchronous queue for executing functions.
package execqueue
import (
"context"
"errors"
"sync"
)
type ExecQueue struct {
mu sync.Mutex
closed bool
inFlight bool // whether a goroutine is running q.run
doneWaiter chan struct{} // non-nil if waiter is waiting, then closed
queue []func()
}
func (q *ExecQueue) Add(f func()) {
q.mu.Lock()
defer q.mu.Unlock()
if q.closed {
return
}
if q.inFlight {
q.queue = append(q.queue, f)
} else {
q.inFlight = true
go q.run(f)
}
}
// RunSync waits for the queue to be drained and then synchronously runs f.
// It returns an error if the queue is closed before f is run or ctx expires.
func (q *ExecQueue) RunSync(ctx context.Context, f func()) error {
for {
if err := q.Wait(ctx); err != nil {
return err
}
q.mu.Lock()
if q.inFlight {
q.mu.Unlock()
continue
}
defer q.mu.Unlock()
if q.closed {
return errors.New("closed")
}
f()
return nil
}
}
func (q *ExecQueue) run(f func()) {
f()
q.mu.Lock()
for len(q.queue) > 0 && !q.closed {
f := q.queue[0]
q.queue[0] = nil
q.queue = q.queue[1:]
q.mu.Unlock()
f()
q.mu.Lock()
}
q.inFlight = false
q.queue = nil
if q.doneWaiter != nil {
close(q.doneWaiter)
q.doneWaiter = nil
}
q.mu.Unlock()
}
// Shutdown asynchronously signals the queue to stop.
func (q *ExecQueue) Shutdown() {
q.mu.Lock()
defer q.mu.Unlock()
q.closed = true
}
// Wait waits for the queue to be empty.
func (q *ExecQueue) Wait(ctx context.Context) error {
q.mu.Lock()
waitCh := q.doneWaiter
if q.inFlight && waitCh == nil {
waitCh = make(chan struct{})
q.doneWaiter = waitCh
}
q.mu.Unlock()
if waitCh == nil {
return nil
}
select {
case <-waitCh:
return nil
case <-ctx.Done():
return ctx.Err()
}
}

View File

@@ -0,0 +1,22 @@
// Copyright (c) Tailscale Inc & AUTHORS
// SPDX-License-Identifier: BSD-3-Clause
package execqueue
import (
"context"
"sync/atomic"
"testing"
)
func TestExecQueue(t *testing.T) {
ctx := context.Background()
var n atomic.Int32
q := &ExecQueue{}
defer q.Shutdown()
q.Add(func() { n.Add(1) })
q.Wait(ctx)
if got := n.Load(); got != 1 {
t.Errorf("n=%d; want 1", got)
}
}