Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[client] Cleanup dns and route states on startup #2757

Merged
merged 18 commits into from
Oct 24, 2024
Merged
1 change: 1 addition & 0 deletions client/firewall/nftables/state.go
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package nftables
12 changes: 5 additions & 7 deletions client/internal/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,6 @@ func (c *ConnectClient) run(

log.Infof("starting NetBird client version %s on %s/%s", version.NetbirdVersion(), runtime.GOOS, runtime.GOARCH)

// Check if client was not shut down in a clean way and restore DNS config if required.
// Otherwise, we might not be able to connect to the management server to retrieve new config.
if err := dns.CheckUncleanShutdown(c.config.WgIface); err != nil {
log.Errorf("checking unclean shutdown error: %s", err)
}

backOff := &backoff.ExponentialBackOff{
InitialInterval: time.Second,
RandomizationFactor: 1,
Expand Down Expand Up @@ -358,7 +352,11 @@ func (c *ConnectClient) Stop() error {
if c.engine == nil {
return nil
}
return c.engine.Stop()
if err := c.engine.Stop(); err != nil {
return fmt.Errorf("stop engine: %w", err)
}

return nil
}

func (c *ConnectClient) isContextCancelled() bool {
Expand Down
3 changes: 1 addition & 2 deletions client/internal/dns/consts_freebsd.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package dns

const (
fileUncleanShutdownResolvConfLocation = "/var/db/netbird/resolv.conf"
fileUncleanShutdownManagerTypeLocation = "/var/db/netbird/manager"
fileUncleanShutdownResolvConfLocation = "/var/db/netbird/resolv.conf"
)
3 changes: 1 addition & 2 deletions client/internal/dns/consts_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
package dns

const (
fileUncleanShutdownResolvConfLocation = "/var/lib/netbird/resolv.conf"
fileUncleanShutdownManagerTypeLocation = "/var/lib/netbird/manager"
fileUncleanShutdownResolvConfLocation = "/var/lib/netbird/resolv.conf"
)
8 changes: 5 additions & 3 deletions client/internal/dns/file_repair_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (

"github.com/fsnotify/fsnotify"
log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/client/internal/statemanager"
)

var (
Expand All @@ -20,7 +22,7 @@ var (
}
)

type repairConfFn func([]string, string, *resolvConf) error
type repairConfFn func([]string, string, *resolvConf, *statemanager.Manager) error

type repair struct {
operationFile string
Expand All @@ -40,7 +42,7 @@ func newRepair(operationFile string, updateFn repairConfFn) *repair {
}
}

func (f *repair) watchFileChanges(nbSearchDomains []string, nbNameserverIP string) {
func (f *repair) watchFileChanges(nbSearchDomains []string, nbNameserverIP string, stateManager *statemanager.Manager) {
if f.inotify != nil {
return
}
Expand Down Expand Up @@ -81,7 +83,7 @@ func (f *repair) watchFileChanges(nbSearchDomains []string, nbNameserverIP strin
log.Errorf("failed to rm inotify watch for resolv.conf: %s", err)
}

err = f.updateFn(nbSearchDomains, nbNameserverIP, rConf)
err = f.updateFn(nbSearchDomains, nbNameserverIP, rConf, stateManager)
if err != nil {
log.Errorf("failed to repair resolv.conf: %v", err)
}
Expand Down
9 changes: 5 additions & 4 deletions client/internal/dns/file_repair_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"
"time"

"github.com/netbirdio/netbird/client/internal/statemanager"
"github.com/netbirdio/netbird/util"
)

Expand Down Expand Up @@ -104,14 +105,14 @@ nameserver 8.8.8.8`,

var changed bool
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
updateFn := func([]string, string, *resolvConf) error {
updateFn := func([]string, string, *resolvConf, *statemanager.Manager) error {
changed = true
cancel()
return nil
}

r := newRepair(operationFile, updateFn)
r.watchFileChanges([]string{"netbird.cloud"}, "10.0.0.1")
r.watchFileChanges([]string{"netbird.cloud"}, "10.0.0.1", nil)

err = os.WriteFile(operationFile, []byte(tt.touchedConfContent), 0755)
if err != nil {
Expand Down Expand Up @@ -151,14 +152,14 @@ searchdomain netbird.cloud something`

var changed bool
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
updateFn := func([]string, string, *resolvConf) error {
updateFn := func([]string, string, *resolvConf, *statemanager.Manager) error {
changed = true
cancel()
return nil
}

r := newRepair(tmpLink, updateFn)
r.watchFileChanges([]string{"netbird.cloud"}, "10.0.0.1")
r.watchFileChanges([]string{"netbird.cloud"}, "10.0.0.1", nil)

err = os.WriteFile(tmpLink, []byte(modifyContent), 0755)
if err != nil {
Expand Down
24 changes: 9 additions & 15 deletions client/internal/dns/file_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"time"

log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/client/internal/statemanager"
)

const (
Expand All @@ -36,7 +38,7 @@ type fileConfigurator struct {
nbNameserverIP string
}

func newFileConfigurator() (hostManager, error) {
func newFileConfigurator() (*fileConfigurator, error) {
fc := &fileConfigurator{}
fc.repair = newRepair(defaultResolvConfPath, fc.updateConfig)
return fc, nil
Expand All @@ -46,7 +48,7 @@ func (f *fileConfigurator) supportCustomPort() bool {
return false
}

func (f *fileConfigurator) applyDNSConfig(config HostDNSConfig) error {
func (f *fileConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
backupFileExist := f.isBackupFileExist()
if !config.RouteAll {
if backupFileExist {
Expand Down Expand Up @@ -76,15 +78,15 @@ func (f *fileConfigurator) applyDNSConfig(config HostDNSConfig) error {

f.repair.stopWatchFileChanges()

err = f.updateConfig(nbSearchDomains, f.nbNameserverIP, resolvConf)
err = f.updateConfig(nbSearchDomains, f.nbNameserverIP, resolvConf, stateManager)
if err != nil {
return err
}
f.repair.watchFileChanges(nbSearchDomains, f.nbNameserverIP)
f.repair.watchFileChanges(nbSearchDomains, f.nbNameserverIP, stateManager)
return nil
}

func (f *fileConfigurator) updateConfig(nbSearchDomains []string, nbNameserverIP string, cfg *resolvConf) error {
func (f *fileConfigurator) updateConfig(nbSearchDomains []string, nbNameserverIP string, cfg *resolvConf, stateManager *statemanager.Manager) error {
searchDomainList := mergeSearchDomains(nbSearchDomains, cfg.searchDomains)
nameServers := generateNsList(nbNameserverIP, cfg)

Expand All @@ -107,7 +109,7 @@ func (f *fileConfigurator) updateConfig(nbSearchDomains []string, nbNameserverIP
log.Infof("created a NetBird managed %s file with the DNS settings. Added %d search domains. Search list: %s", defaultResolvConfPath, len(searchDomainList), searchDomainList)

// create another backup for unclean shutdown detection right after overwriting the original resolv.conf
if err := createUncleanShutdownIndicator(fileDefaultResolvConfBackupLocation, fileManager, nbNameserverIP); err != nil {
if err := createUncleanShutdownIndicator(fileDefaultResolvConfBackupLocation, nbNameserverIP, stateManager); err != nil {
log.Errorf("failed to create unclean shutdown resolv.conf backup: %s", err)
}

Expand Down Expand Up @@ -145,10 +147,6 @@ func (f *fileConfigurator) restore() error {
return fmt.Errorf("restoring %s from %s: %w", defaultResolvConfPath, fileDefaultResolvConfBackupLocation, err)
}

if err := removeUncleanShutdownIndicator(); err != nil {
log.Errorf("failed to remove unclean shutdown resolv.conf backup: %s", err)
}

return os.RemoveAll(fileDefaultResolvConfBackupLocation)
}

Expand Down Expand Up @@ -176,7 +174,7 @@ func (f *fileConfigurator) restoreUncleanShutdownDNS(storedDNSAddress *netip.Add
return restoreResolvConfFile()
}

log.Info("restoring unclean shutdown: first current nameserver differs from saved nameserver pre-netbird: not restoring")
log.Infof("restoring unclean shutdown: first current nameserver differs from saved nameserver pre-netbird: %s (current) vs %s (stored): not restoring", currentDNSAddress, storedDNSAddress)
return nil
}

Expand All @@ -192,10 +190,6 @@ func restoreResolvConfFile() error {
return fmt.Errorf("restoring %s from %s: %w", defaultResolvConfPath, fileUncleanShutdownResolvConfLocation, err)
}

if err := removeUncleanShutdownIndicator(); err != nil {
log.Errorf("failed to remove unclean shutdown resolv.conf file: %s", err)
}

return nil
}

Expand Down
19 changes: 6 additions & 13 deletions client/internal/dns/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import (
"net/netip"
"strings"

"github.com/netbirdio/netbird/client/internal/statemanager"
nbdns "github.com/netbirdio/netbird/dns"
)

type hostManager interface {
applyDNSConfig(config HostDNSConfig) error
applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error
restoreHostDNS() error
supportCustomPort() bool
restoreUncleanShutdownDNS(storedDNSAddress *netip.Addr) error
}

type SystemDNSSettings struct {
Expand All @@ -35,15 +35,15 @@ type DomainConfig struct {
}

type mockHostConfigurator struct {
applyDNSConfigFunc func(config HostDNSConfig) error
applyDNSConfigFunc func(config HostDNSConfig, stateManager *statemanager.Manager) error
restoreHostDNSFunc func() error
supportCustomPortFunc func() bool
restoreUncleanShutdownDNSFunc func(*netip.Addr) error
}

func (m *mockHostConfigurator) applyDNSConfig(config HostDNSConfig) error {
func (m *mockHostConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
if m.applyDNSConfigFunc != nil {
return m.applyDNSConfigFunc(config)
return m.applyDNSConfigFunc(config, stateManager)
}
return fmt.Errorf("method applyDNSSettings is not implemented")
}
Expand All @@ -62,16 +62,9 @@ func (m *mockHostConfigurator) supportCustomPort() bool {
return false
}

func (m *mockHostConfigurator) restoreUncleanShutdownDNS(storedDNSAddress *netip.Addr) error {
if m.restoreUncleanShutdownDNSFunc != nil {
return m.restoreUncleanShutdownDNSFunc(storedDNSAddress)
}
return fmt.Errorf("method restoreUncleanShutdownDNS is not implemented")
}

func newNoopHostMocker() hostManager {
return &mockHostConfigurator{
applyDNSConfigFunc: func(config HostDNSConfig) error { return nil },
applyDNSConfigFunc: func(config HostDNSConfig, stateManager *statemanager.Manager) error { return nil },
restoreHostDNSFunc: func() error { return nil },
supportCustomPortFunc: func() bool { return true },
restoreUncleanShutdownDNSFunc: func(*netip.Addr) error { return nil },
Expand Down
12 changes: 5 additions & 7 deletions client/internal/dns/host_android.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
package dns

import "net/netip"
import (
"github.com/netbirdio/netbird/client/internal/statemanager"
)

type androidHostManager struct {
}

func newHostManager() (hostManager, error) {
func newHostManager() (*androidHostManager, error) {
return &androidHostManager{}, nil
}

func (a androidHostManager) applyDNSConfig(config HostDNSConfig) error {
func (a androidHostManager) applyDNSConfig(HostDNSConfig, *statemanager.Manager) error {
return nil
}

Expand All @@ -20,7 +22,3 @@ func (a androidHostManager) restoreHostDNS() error {
func (a androidHostManager) supportCustomPort() bool {
return false
}

func (a androidHostManager) restoreUncleanShutdownDNS(*netip.Addr) error {
return nil
}
18 changes: 7 additions & 11 deletions client/internal/dns/host_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ import (
"fmt"
"io"
"net"
"net/netip"
"os/exec"
"strconv"
"strings"

log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/client/internal/statemanager"
)

const (
Expand All @@ -37,7 +38,7 @@ type systemConfigurator struct {
systemDNSSettings SystemDNSSettings
}

func newHostManager() (hostManager, error) {
func newHostManager() (*systemConfigurator, error) {
return &systemConfigurator{
createdKeys: make(map[string]struct{}),
}, nil
Expand All @@ -47,12 +48,11 @@ func (s *systemConfigurator) supportCustomPort() bool {
return true
}

func (s *systemConfigurator) applyDNSConfig(config HostDNSConfig) error {
func (s *systemConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
var err error

// create a file for unclean shutdown detection
if err := createUncleanShutdownIndicator(); err != nil {
log.Errorf("failed to create unclean shutdown file: %s", err)
if err := stateManager.UpdateState(&ShutdownState{}); err != nil {
log.Errorf("failed to update shutdown state: %s", err)
}

var (
Expand Down Expand Up @@ -123,10 +123,6 @@ func (s *systemConfigurator) restoreHostDNS() error {
}
}

if err := removeUncleanShutdownIndicator(); err != nil {
log.Errorf("failed to remove unclean shutdown file: %s", err)
}

return nil
}

Expand Down Expand Up @@ -320,7 +316,7 @@ func (s *systemConfigurator) getPrimaryService() (string, string, error) {
return primaryService, router, nil
}

func (s *systemConfigurator) restoreUncleanShutdownDNS(*netip.Addr) error {
func (s *systemConfigurator) restoreUncleanShutdownDNS() error {
if err := s.restoreHostDNS(); err != nil {
return fmt.Errorf("restoring dns via scutil: %w", err)
}
Expand Down
11 changes: 4 additions & 7 deletions client/internal/dns/host_ios.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,24 @@ package dns
import (
"encoding/json"
"fmt"
"net/netip"

log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/client/internal/statemanager"
)

type iosHostManager struct {
dnsManager IosDnsManager
config HostDNSConfig
}

func newHostManager(dnsManager IosDnsManager) (hostManager, error) {
func newHostManager(dnsManager IosDnsManager) (*iosHostManager, error) {
return &iosHostManager{
dnsManager: dnsManager,
}, nil
}

func (a iosHostManager) applyDNSConfig(config HostDNSConfig) error {
func (a iosHostManager) applyDNSConfig(config HostDNSConfig, _ *statemanager.Manager) error {
jsonData, err := json.Marshal(config)
if err != nil {
return fmt.Errorf("marshal: %w", err)
Expand All @@ -37,7 +38,3 @@ func (a iosHostManager) restoreHostDNS() error {
func (a iosHostManager) supportCustomPort() bool {
return false
}

func (a iosHostManager) restoreUncleanShutdownDNS(*netip.Addr) error {
return nil
}
Loading
Loading