Skip to content

Commit

Permalink
Trigger additional DNS probes on peer conn status
Browse files Browse the repository at this point in the history
This patch adds an additional DNS probe trigger by ConnStatus changes to
any peer. This is an attempt to limit the number of DNS changes applied
to Darwin, Windows and BSD hosts. There should be no change whatsoever
on how iOS and Android operate.

How publicly accessible DNS servers get applied to the host should also
remain unchanged.
  • Loading branch information
hurricanehrndz committed Aug 21, 2024
1 parent ddea001 commit 2196243
Show file tree
Hide file tree
Showing 7 changed files with 227 additions and 73 deletions.
4 changes: 3 additions & 1 deletion client/internal/dns/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,15 @@ func dnsConfigToHostDNSConfig(dnsConfig nbdns.Config, ip string, port int) HostD
if len(nsConfig.NameServers) == 0 {
continue
}
if nsConfig.Primary {

if nsConfig.Primary && nsConfig.Enabled {
config.RouteAll = true
}

for _, domain := range nsConfig.Domains {
config.Domains = append(config.Domains, DomainConfig{
Domain: strings.TrimSuffix(domain, "."),
Disabled: !nsConfig.Enabled,
MatchOnly: !nsConfig.SearchDomainsEnabled,
})
}
Expand Down
54 changes: 36 additions & 18 deletions client/internal/dns/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/netip"
"runtime"
"slices"
"strings"
"sync"

Expand Down Expand Up @@ -297,6 +298,11 @@ func (s *DefaultServer) applyConfiguration(update nbdns.Config) error {
s.service.Stop()
}

// update each NameServerGroup config in nbdns.Config base on peerCount and IP.IsPrivate()
if runtime.GOOS != "android" && runtime.GOOS != "ios" {
s.toggleNameServerGroupsOnStatus(update.NameServerGroups)
}

localMuxUpdates, localRecords, err := s.buildLocalHandlerUpdate(update.CustomZones)
if err != nil {
return fmt.Errorf("not applying dns update, error: %v", err)
Expand All @@ -308,6 +314,7 @@ func (s *DefaultServer) applyConfiguration(update nbdns.Config) error {
muxUpdates := append(localMuxUpdates, upstreamMuxUpdates...) //nolint:gocritic

s.updateMux(muxUpdates)

s.updateLocalResolver(localRecords)
s.currentConfig = dnsConfigToHostDNSConfig(update, s.service.RuntimeIP(), s.service.RuntimePort())

Expand Down Expand Up @@ -359,7 +366,6 @@ func (s *DefaultServer) buildLocalHandlerUpdate(customZones []nbdns.CustomZone)
}

func (s *DefaultServer) buildUpstreamHandlerUpdate(nameServerGroups []*nbdns.NameServerGroup) ([]muxUpdate, error) {

var muxUpdates []muxUpdate
for _, nsGroup := range nameServerGroups {
if len(nsGroup.NameServers) == 0 {
Expand Down Expand Up @@ -403,7 +409,7 @@ func (s *DefaultServer) buildUpstreamHandlerUpdate(nameServerGroups []*nbdns.Nam
// contains this upstream settings (temporal deactivation not removed it)
handler.deactivate, handler.reactivate = s.upstreamCallbacks(nsGroup, handler)

if nsGroup.Primary {
if nsGroup.Primary && nsGroup.Enabled {
muxUpdates = append(muxUpdates, muxUpdate{
domain: nbdns.RootZone,
handler: handler,
Expand All @@ -421,6 +427,9 @@ func (s *DefaultServer) buildUpstreamHandlerUpdate(nameServerGroups []*nbdns.Nam
handler.stop()
return nil, fmt.Errorf("received a nameserver group with an empty domain element")
}
if !nsGroup.Enabled {
continue
}
muxUpdates = append(muxUpdates, muxUpdate{
domain: domain,
handler: handler,
Expand Down Expand Up @@ -484,6 +493,19 @@ func (s *DefaultServer) updateLocalResolver(update map[string]nbdns.SimpleRecord
s.localResolver.registeredMap = updatedMap
}

func (s *DefaultServer) toggleNameServerGroupsOnStatus(nameServerGroups []*nbdns.NameServerGroup) {
peerCount := s.statusRecorder.GetConnectedPeersCount()
for _, nsGroup := range nameServerGroups {
var hasPublicNameServer bool
for _, s := range nsGroup.NameServers {
if !s.IP.IsPrivate() {
hasPublicNameServer = true
}
}
nsGroup.Enabled = hasPublicNameServer || (peerCount >= 1)
}
}

func getNSHostPort(ns nbdns.NameServer) string {
return fmt.Sprintf("%s:%d", ns.IP.String(), ns.Port)
}
Expand All @@ -495,29 +517,22 @@ func (s *DefaultServer) upstreamCallbacks(
nsGroup *nbdns.NameServerGroup,
handler dns.Handler,
) (deactivate func(error), reactivate func()) {
var removeIndex map[string]int
deactivate = func(err error) {
s.mux.Lock()
defer s.mux.Unlock()

l := log.WithField("nameservers", nsGroup.NameServers)
l.Info("Temporarily deactivating nameservers group due to timeout")

removeIndex = make(map[string]int)
for _, domain := range nsGroup.Domains {
removeIndex[domain] = -1
}
if nsGroup.Primary {
removeIndex[nbdns.RootZone] = -1
s.currentConfig.RouteAll = false
s.service.DeregisterMux(nbdns.RootZone)
}

for i, item := range s.currentConfig.Domains {
if _, found := removeIndex[item.Domain]; found {
if slices.Contains(nsGroup.Domains, item.Domain) {
s.currentConfig.Domains[i].Disabled = true
s.service.DeregisterMux(item.Domain)
removeIndex[item.Domain] = i
}
}

Expand All @@ -530,18 +545,16 @@ func (s *DefaultServer) upstreamCallbacks(
}

s.updateNSState(nsGroup, err, false)

}
reactivate = func() {
s.mux.Lock()
defer s.mux.Unlock()

for domain, i := range removeIndex {
if i == -1 || i >= len(s.currentConfig.Domains) || s.currentConfig.Domains[i].Domain != domain {
continue
for i, item := range s.currentConfig.Domains {
if slices.Contains(nsGroup.Domains, item.Domain) {
s.currentConfig.Domains[i].Disabled = false
s.service.RegisterMux(item.Domain, handler)
}
s.currentConfig.Domains[i].Disabled = false
s.service.RegisterMux(domain, handler)
}

l := log.WithField("nameservers", nsGroup.NameServers)
Expand Down Expand Up @@ -588,17 +601,22 @@ func (s *DefaultServer) updateNSGroupStates(groups []*nbdns.NameServerGroup) {

for _, group := range groups {
var servers []string
var nsError error
if !group.Enabled {
nsError = fmt.Errorf("no peers connected")
}
for _, ns := range group.NameServers {
servers = append(servers, fmt.Sprintf("%s:%d", ns.IP, ns.Port))
}

// Automatically disbled if peer == 0 and IP is private

Check failure on line 612 in client/internal/dns/server.go

View workflow job for this annotation

GitHub Actions / codespell

disbled ==> disabled
state := peer.NSGroupState{
ID: generateGroupKey(group),
Servers: servers,
Domains: group.Domains,
// The probe will determine the state, default enabled
Enabled: true,
Error: nil,
Enabled: group.Enabled,
Error: nsError,
}
states = append(states, state)
}
Expand Down
17 changes: 16 additions & 1 deletion client/internal/dns/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net/netip"
"os"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -126,10 +127,12 @@ func TestUpdateDNSServer(t *testing.T) {
},
NameServerGroups: []*nbdns.NameServerGroup{
{
Enabled: true,
Domains: []string{"netbird.io"},
NameServers: nameServers,
},
{
Enabled: true,
NameServers: nameServers,
Primary: true,
},
Expand All @@ -154,6 +157,7 @@ func TestUpdateDNSServer(t *testing.T) {
},
NameServerGroups: []*nbdns.NameServerGroup{
{
Enabled: true,
Domains: []string{"netbird.io"},
NameServers: nameServers,
},
Expand Down Expand Up @@ -279,7 +283,16 @@ func TestUpdateDNSServer(t *testing.T) {
t.Log(err)
}
}()
dnsServer, err := NewDefaultServer(context.Background(), wgIface, "", &peer.Status{})
statusRecorder := peer.NewRecorder("https://mgm")
key := "abc"
statusRecorder.AddPeer(key, "abc.netbird")

Check failure on line 288 in client/internal/dns/server_test.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest)

Error return value of `statusRecorder.AddPeer` is not checked (errcheck)

Check failure on line 288 in client/internal/dns/server_test.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

Error return value of `statusRecorder.AddPeer` is not checked (errcheck)

Check failure on line 288 in client/internal/dns/server_test.go

View workflow job for this annotation

GitHub Actions / lint (windows-latest)

Error return value of `statusRecorder.AddPeer` is not checked (errcheck)
statusRecorder.UpdatePeerState(peer.State{

Check failure on line 289 in client/internal/dns/server_test.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest)

Error return value of `statusRecorder.UpdatePeerState` is not checked (errcheck)

Check failure on line 289 in client/internal/dns/server_test.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

Error return value of `statusRecorder.UpdatePeerState` is not checked (errcheck)

Check failure on line 289 in client/internal/dns/server_test.go

View workflow job for this annotation

GitHub Actions / lint (windows-latest)

Error return value of `statusRecorder.UpdatePeerState` is not checked (errcheck)
PubKey: key,
Mux: new(sync.RWMutex),
ConnStatus: peer.StatusConnected,
ConnStatusUpdate: time.Now(),
})
dnsServer, err := NewDefaultServer(context.Background(), wgIface, "", statusRecorder)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -427,10 +440,12 @@ func TestDNSFakeResolverHandleUpdates(t *testing.T) {
{
Domains: []string{"netbird.io"},
NameServers: nameServers,
Enabled: true,
},
{
NameServers: nameServers,
Primary: true,
Enabled: true,
},
},
}
Expand Down
Loading

0 comments on commit 2196243

Please sign in to comment.