mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-01 12:31:42 +02:00
## Describe your changes
getIPsFromResolver passed a nil context to ExchangeWithFallback, which
net.Dialer.DialContext rejects with panic("nil context"). On iOS this
crashed the whole network extension (SIGABRT) ~2 seconds after connect
whenever the network map contained a domain-based (dynamic) route, as
the resolver goroutine panicked on its first DNS query.
Passing nil used to be a documented input of ExchangeWithFallback ("If
the passed context is nil, this will use Exchange instead of
ExchangeContext") since #3632. 9ed2e2a5b (#5971) removed the nil-context
branch, but this iOS-only caller was not updated — it never fails CI
since route_ios.go only builds with GOOS=ios. Broken since v0.71.1.
Pass a context bounded by the existing dialTimeout instead, matching the
dnsinterceptor pattern (context.Background() + timeout).
Captured panic (netbird.err):
panic: nil context
net.(*Dialer).DialContext -> miekg/dns ExchangeContext ->
nbdns.ExchangeWithFallback(nil, ...) ->
dynamic.(*Route).getIPsFromResolver
route_ios.go:35
## Issue ticket number and link
## Stack
<!-- branch-stack -->
### Checklist
- [x] Is it a bug fix
- [ ] Is a typo/documentation fix
- [ ] Is a feature enhancement
- [ ] It is a refactor
- [ ] Created tests that fail without the change (if possible)
- [ ] This change does **not** modify the public API, gRPC protocols,
functionality behavior, CLI / service flags, or introduce a new feature
— **OR** I have discussed it with the NetBird team beforehand (link the
issue / Slack thread in the description). See
[CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first).
> By submitting this pull request, you confirm that you have read and
agree to the terms of the [Contributor License
Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md).
## Documentation
Select exactly one:
- [ ] I added/updated documentation for this change
- [x] Documentation is **not needed** for this change (explain why)
### Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
<!-- codesmith:footer -->
---
<a
href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6848"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img
alt="View with Codesmith"
src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a>
<a
href="https://backend.blacksmith.sh/track/enable-autofix?expires=1787223752&installation_model_id=427504&pr_number=6848&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6848&signature=881d3b30aa4bd15001562ad417ff946b70c7c108856a1b3808b776c12b7022a9"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img
alt="Autofix with Codesmith"
src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a>
<sup>Need help on this PR? Tag <code>/codesmith</code> with what you
need. Autofix is disabled.</sup>
<!-- codesmith:autofix:disabled -->
<!-- /codesmith:footer -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Fixed iOS dynamic DNS resolution by ensuring DNS queries use a proper
resolver context instead of a nil one.
* Improved DNS resolution reliability by propagating
cancellation/timeouts through all domain IP lookups, including fallback
system resolver queries.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
400 lines
11 KiB
Go
400 lines
11 KiB
Go
package dynamic
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"net"
|
|
"net/netip"
|
|
"strings"
|
|
"sync"
|
|
"time"
|
|
|
|
"github.com/hashicorp/go-multierror"
|
|
log "github.com/sirupsen/logrus"
|
|
|
|
nberrors "github.com/netbirdio/netbird/client/errors"
|
|
"github.com/netbirdio/netbird/client/internal/peer"
|
|
"github.com/netbirdio/netbird/client/internal/routemanager/common"
|
|
"github.com/netbirdio/netbird/client/internal/routemanager/iface"
|
|
"github.com/netbirdio/netbird/client/internal/routemanager/refcounter"
|
|
"github.com/netbirdio/netbird/client/internal/routemanager/util"
|
|
"github.com/netbirdio/netbird/route"
|
|
"github.com/netbirdio/netbird/shared/management/domain"
|
|
)
|
|
|
|
const (
|
|
DefaultInterval = time.Minute
|
|
|
|
minInterval = 2 * time.Second
|
|
failureInterval = 5 * time.Second
|
|
|
|
addAllowedIP = "add allowed IP %s: %w"
|
|
)
|
|
|
|
type domainMap map[domain.Domain][]netip.Prefix
|
|
|
|
type resolveResult struct {
|
|
domain domain.Domain
|
|
prefix netip.Prefix
|
|
err error
|
|
}
|
|
|
|
type Route struct {
|
|
route *route.Route
|
|
routeRefCounter *refcounter.RouteRefCounter
|
|
allowedIPsRefcounter *refcounter.AllowedIPsRefCounter
|
|
interval time.Duration
|
|
dynamicDomains domainMap
|
|
mu sync.Mutex
|
|
currentPeerKey string
|
|
cancel context.CancelFunc
|
|
statusRecorder *peer.Status
|
|
wgInterface iface.WGIface
|
|
resolverAddr netip.AddrPort
|
|
}
|
|
|
|
func NewRoute(params common.HandlerParams, resolverAddr netip.AddrPort) *Route {
|
|
return &Route{
|
|
route: params.Route,
|
|
routeRefCounter: params.RouteRefCounter,
|
|
allowedIPsRefcounter: params.AllowedIPsRefCounter,
|
|
interval: params.DnsRouterInterval,
|
|
statusRecorder: params.StatusRecorder,
|
|
wgInterface: params.WgInterface,
|
|
resolverAddr: resolverAddr,
|
|
dynamicDomains: domainMap{},
|
|
}
|
|
}
|
|
|
|
func (r *Route) String() string {
|
|
return r.route.Domains.SafeString()
|
|
}
|
|
|
|
func (r *Route) AddRoute(ctx context.Context) error {
|
|
r.mu.Lock()
|
|
defer r.mu.Unlock()
|
|
|
|
if r.cancel != nil {
|
|
r.cancel()
|
|
}
|
|
|
|
ctx, r.cancel = context.WithCancel(ctx)
|
|
|
|
go r.startResolver(ctx)
|
|
|
|
return nil
|
|
}
|
|
|
|
// RemoveRoute will stop the dynamic resolver and remove all dynamic routes.
|
|
// It doesn't touch allowed IPs, these should be removed separately and before calling this method.
|
|
func (r *Route) RemoveRoute() error {
|
|
r.mu.Lock()
|
|
defer r.mu.Unlock()
|
|
|
|
if r.cancel != nil {
|
|
r.cancel()
|
|
}
|
|
|
|
var merr *multierror.Error
|
|
for domain, prefixes := range r.dynamicDomains {
|
|
for _, prefix := range prefixes {
|
|
if _, err := r.routeRefCounter.Decrement(prefix); err != nil {
|
|
merr = multierror.Append(merr, fmt.Errorf("remove dynamic route for IP %s: %w", prefix, err))
|
|
}
|
|
}
|
|
log.Debugf("Removed dynamic route(s) for [%s]: %s", domain.SafeString(), strings.ReplaceAll(fmt.Sprintf("%s", prefixes), " ", ", "))
|
|
|
|
r.statusRecorder.DeleteResolvedDomainsStates(domain)
|
|
}
|
|
|
|
r.dynamicDomains = domainMap{}
|
|
|
|
return nberrors.FormatErrorOrNil(merr)
|
|
}
|
|
|
|
func (r *Route) AddAllowedIPs(peerKey string) error {
|
|
r.mu.Lock()
|
|
defer r.mu.Unlock()
|
|
|
|
var merr *multierror.Error
|
|
for domain, domainPrefixes := range r.dynamicDomains {
|
|
for _, prefix := range domainPrefixes {
|
|
if err := r.incrementAllowedIP(domain, prefix, peerKey); err != nil {
|
|
merr = multierror.Append(merr, fmt.Errorf(addAllowedIP, prefix, err))
|
|
}
|
|
}
|
|
}
|
|
r.currentPeerKey = peerKey
|
|
return nberrors.FormatErrorOrNil(merr)
|
|
}
|
|
|
|
func (r *Route) RemoveAllowedIPs() error {
|
|
r.mu.Lock()
|
|
defer r.mu.Unlock()
|
|
|
|
var merr *multierror.Error
|
|
for _, domainPrefixes := range r.dynamicDomains {
|
|
for _, prefix := range domainPrefixes {
|
|
if _, err := r.allowedIPsRefcounter.Decrement(prefix); err != nil {
|
|
merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %w", prefix, err))
|
|
}
|
|
}
|
|
}
|
|
|
|
r.currentPeerKey = ""
|
|
return nberrors.FormatErrorOrNil(merr)
|
|
}
|
|
|
|
func (r *Route) startResolver(ctx context.Context) {
|
|
log.Debugf("Starting dynamic route resolver for domains [%v]", r)
|
|
|
|
interval := r.interval
|
|
if interval < minInterval {
|
|
interval = minInterval
|
|
log.Warnf("Dynamic route resolver interval %s is too low, setting to minimum value %s", r.interval, minInterval)
|
|
}
|
|
|
|
ticker := time.NewTicker(interval)
|
|
defer ticker.Stop()
|
|
|
|
if err := r.update(ctx); err != nil {
|
|
log.Errorf("Failed to resolve domains for route [%v]: %v", r, err)
|
|
if interval > failureInterval {
|
|
ticker.Reset(failureInterval)
|
|
}
|
|
}
|
|
|
|
for {
|
|
select {
|
|
case <-ctx.Done():
|
|
log.Debugf("Stopping dynamic route resolver for domains [%v]", r)
|
|
return
|
|
case <-ticker.C:
|
|
if err := r.update(ctx); err != nil {
|
|
log.Errorf("Failed to resolve domains for route [%v]: %v", r, err)
|
|
// Use a lower ticker interval if the update fails
|
|
if interval > failureInterval {
|
|
ticker.Reset(failureInterval)
|
|
}
|
|
} else if interval > failureInterval {
|
|
// Reset to the original interval if the update succeeds
|
|
ticker.Reset(interval)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
func (r *Route) update(ctx context.Context) error {
|
|
resolved, err := r.resolveDomains(ctx)
|
|
if err != nil {
|
|
if len(resolved) == 0 {
|
|
return fmt.Errorf("resolve domains: %w", err)
|
|
}
|
|
log.Warnf("Failed to resolve domains: %v", err)
|
|
}
|
|
if err := r.updateDynamicRoutes(ctx, resolved); err != nil {
|
|
return fmt.Errorf("update dynamic routes: %w", err)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
func (r *Route) resolveDomains(ctx context.Context) (domainMap, error) {
|
|
results := make(chan resolveResult)
|
|
go r.resolve(ctx, results)
|
|
|
|
resolved := domainMap{}
|
|
var merr *multierror.Error
|
|
|
|
for result := range results {
|
|
if result.err != nil {
|
|
merr = multierror.Append(merr, result.err)
|
|
} else {
|
|
resolved[result.domain] = append(resolved[result.domain], result.prefix)
|
|
}
|
|
}
|
|
|
|
return resolved, nberrors.FormatErrorOrNil(merr)
|
|
}
|
|
|
|
func (r *Route) resolve(ctx context.Context, results chan resolveResult) {
|
|
var wg sync.WaitGroup
|
|
|
|
for _, d := range r.route.Domains {
|
|
wg.Add(1)
|
|
go func(domain domain.Domain) {
|
|
defer wg.Done()
|
|
|
|
ips, err := r.getIPsFromResolver(ctx, domain)
|
|
if err != nil {
|
|
log.Tracef("Failed to resolve domain %s with private resolver: %v", domain.SafeString(), err)
|
|
ips, err = lookupHostIPs(ctx, domain)
|
|
if err != nil {
|
|
results <- resolveResult{domain: domain, err: fmt.Errorf("resolve d %s: %w", domain.SafeString(), err)}
|
|
return
|
|
}
|
|
}
|
|
|
|
for _, ip := range ips {
|
|
prefix, err := util.GetPrefixFromIP(ip)
|
|
if err != nil {
|
|
results <- resolveResult{domain: domain, err: fmt.Errorf("get prefix from IP %s: %w", ip.String(), err)}
|
|
return
|
|
}
|
|
results <- resolveResult{domain: domain, prefix: prefix}
|
|
}
|
|
}(d)
|
|
}
|
|
|
|
wg.Wait()
|
|
close(results)
|
|
}
|
|
|
|
func (r *Route) updateDynamicRoutes(ctx context.Context, newDomains domainMap) error {
|
|
r.mu.Lock()
|
|
defer r.mu.Unlock()
|
|
|
|
if ctx.Err() != nil {
|
|
return ctx.Err()
|
|
}
|
|
|
|
var merr *multierror.Error
|
|
|
|
for domain, newPrefixes := range newDomains {
|
|
oldPrefixes := r.dynamicDomains[domain]
|
|
toAdd, toRemove := determinePrefixChanges(oldPrefixes, newPrefixes)
|
|
|
|
addedPrefixes, err := r.addRoutes(domain, toAdd)
|
|
if err != nil {
|
|
merr = multierror.Append(merr, err)
|
|
} else if len(addedPrefixes) > 0 {
|
|
log.Debugf("Added dynamic route(s) for [%s]: %s", domain.SafeString(), strings.ReplaceAll(fmt.Sprintf("%s", addedPrefixes), " ", ", "))
|
|
}
|
|
|
|
removedPrefixes, err := r.removeRoutes(toRemove)
|
|
if err != nil {
|
|
merr = multierror.Append(merr, err)
|
|
} else if len(removedPrefixes) > 0 {
|
|
log.Debugf("Removed dynamic route(s) for [%s]: %s", domain.SafeString(), strings.ReplaceAll(fmt.Sprintf("%s", removedPrefixes), " ", ", "))
|
|
}
|
|
|
|
updatedPrefixes := combinePrefixes(oldPrefixes, removedPrefixes, addedPrefixes)
|
|
r.dynamicDomains[domain] = updatedPrefixes
|
|
|
|
r.statusRecorder.UpdateResolvedDomainsStates(domain, domain, updatedPrefixes, r.route.GetResourceID())
|
|
}
|
|
|
|
return nberrors.FormatErrorOrNil(merr)
|
|
}
|
|
|
|
func (r *Route) addRoutes(domain domain.Domain, prefixes []netip.Prefix) ([]netip.Prefix, error) {
|
|
var addedPrefixes []netip.Prefix
|
|
var merr *multierror.Error
|
|
|
|
for _, prefix := range prefixes {
|
|
if _, err := r.routeRefCounter.Increment(prefix, struct{}{}); err != nil {
|
|
merr = multierror.Append(merr, fmt.Errorf("add dynamic route for IP %s: %w", prefix, err))
|
|
continue
|
|
}
|
|
if r.currentPeerKey != "" {
|
|
if err := r.incrementAllowedIP(domain, prefix, r.currentPeerKey); err != nil {
|
|
merr = multierror.Append(merr, fmt.Errorf(addAllowedIP, prefix, err))
|
|
}
|
|
}
|
|
addedPrefixes = append(addedPrefixes, prefix)
|
|
}
|
|
|
|
return addedPrefixes, merr.ErrorOrNil()
|
|
}
|
|
|
|
func (r *Route) removeRoutes(prefixes []netip.Prefix) ([]netip.Prefix, error) {
|
|
if r.route.KeepRoute {
|
|
return nil, nil
|
|
}
|
|
|
|
var removedPrefixes []netip.Prefix
|
|
var merr *multierror.Error
|
|
|
|
for _, prefix := range prefixes {
|
|
if _, err := r.routeRefCounter.Decrement(prefix); err != nil {
|
|
merr = multierror.Append(merr, fmt.Errorf("remove dynamic route for IP %s: %w", prefix, err))
|
|
}
|
|
if r.currentPeerKey != "" {
|
|
if _, err := r.allowedIPsRefcounter.Decrement(prefix); err != nil {
|
|
merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %w", prefix, err))
|
|
}
|
|
}
|
|
removedPrefixes = append(removedPrefixes, prefix)
|
|
}
|
|
|
|
return removedPrefixes, merr.ErrorOrNil()
|
|
}
|
|
|
|
func (r *Route) incrementAllowedIP(domain domain.Domain, prefix netip.Prefix, peerKey string) error {
|
|
if ref, err := r.allowedIPsRefcounter.Increment(prefix, peerKey); err != nil {
|
|
return fmt.Errorf(addAllowedIP, prefix, err)
|
|
} else if ref.Count > 1 && ref.Out != peerKey {
|
|
log.Warnf("IP [%s] for domain [%s] is already routed by peer [%s]. HA routing disabled",
|
|
prefix.Addr(),
|
|
domain.SafeString(),
|
|
ref.Out,
|
|
)
|
|
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func determinePrefixChanges(oldPrefixes, newPrefixes []netip.Prefix) (toAdd, toRemove []netip.Prefix) {
|
|
prefixSet := make(map[netip.Prefix]bool)
|
|
for _, prefix := range oldPrefixes {
|
|
prefixSet[prefix] = false
|
|
}
|
|
for _, prefix := range newPrefixes {
|
|
if _, exists := prefixSet[prefix]; exists {
|
|
prefixSet[prefix] = true
|
|
} else {
|
|
toAdd = append(toAdd, prefix)
|
|
}
|
|
}
|
|
for prefix, inUse := range prefixSet {
|
|
if !inUse {
|
|
toRemove = append(toRemove, prefix)
|
|
}
|
|
}
|
|
return
|
|
}
|
|
|
|
// lookupHostIPs resolves d via the system resolver, honoring ctx cancellation.
|
|
func lookupHostIPs(ctx context.Context, d domain.Domain) ([]net.IP, error) {
|
|
addrs, err := net.DefaultResolver.LookupIPAddr(ctx, d.PunycodeString())
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
ips := make([]net.IP, 0, len(addrs))
|
|
for _, addr := range addrs {
|
|
ips = append(ips, addr.IP)
|
|
}
|
|
return ips, nil
|
|
}
|
|
|
|
func combinePrefixes(oldPrefixes, removedPrefixes, addedPrefixes []netip.Prefix) []netip.Prefix {
|
|
prefixSet := make(map[netip.Prefix]struct{})
|
|
for _, prefix := range oldPrefixes {
|
|
prefixSet[prefix] = struct{}{}
|
|
}
|
|
for _, prefix := range removedPrefixes {
|
|
delete(prefixSet, prefix)
|
|
}
|
|
for _, prefix := range addedPrefixes {
|
|
prefixSet[prefix] = struct{}{}
|
|
}
|
|
|
|
var combinedPrefixes []netip.Prefix
|
|
for prefix := range prefixSet {
|
|
combinedPrefixes = append(combinedPrefixes, prefix)
|
|
}
|
|
|
|
return combinedPrefixes
|
|
}
|