diff --git a/examples/config.yml b/examples/config.yml index f81d72ee..c3603446 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -39,8 +39,9 @@ static_host_map: # public address. Other valid options are "ip6" and "ip" (returns both.) #network: ip4 - # lookup_timeout is the DNS query timeout. - #lookup_timeout: 250ms + # lookup_timeout is the DNS query timeout. A failed lookup keeps the last known good addresses, only a not found + # answer removes them. + #lookup_timeout: 5s lighthouse: # am_lighthouse is used to enable lighthouse functionality for a node. This should ONLY be true on nodes diff --git a/handshake_manager.go b/handshake_manager.go index b68c0c38..11522b1f 100644 --- a/handshake_manager.go +++ b/handshake_manager.go @@ -405,6 +405,8 @@ func (hm *HandshakeManager) StartHandshake(vpnAddr netip.Addr, cacheCb func(*Han hm.Unlock() hm.lightHouse.QueryServer(vpnAddr) + // If this host came from a static_map hostname its addresses may have moved, retries will pick up anything new + hostinfo.remotes.ResolveHostnames() return hostinfo } diff --git a/lighthouse.go b/lighthouse.go index 49cf8b14..a9c3dc83 100644 --- a/lighthouse.go +++ b/lighthouse.go @@ -452,7 +452,7 @@ func getStaticMapCadence(c *config.C) (time.Duration, error) { } func getStaticMapLookupTimeout(c *config.C) (time.Duration, error) { - lookupTimeout := c.GetString("static_map.lookup_timeout", "250ms") + lookupTimeout := c.GetString("static_map.lookup_timeout", "5s") d, err := time.ParseDuration(lookupTimeout) if err != nil { return 0, err @@ -630,7 +630,8 @@ func (lh *LightHouse) addStaticRemotes(i int, d time.Duration, network string, t ctx := lh.ctx lh.Unlock() - hr, err := NewHostnameResults(ctx, lh.l, d, network, timeout, toAddrs, func() { + // On reload am.hr is the previous resolver, its last known good addresses carry over in case DNS is down right now + hr, err := NewHostnameResults(ctx, lh.l, d, network, timeout, toAddrs, am.hr, func() { // This callback runs whenever the DNS hostname resolver finds a different set of addr's // in its resolution for hostnames. am.Lock() @@ -642,7 +643,8 @@ func (lh *LightHouse) addStaticRemotes(i int, d time.Duration, network string, t } am.unlockedSetHostnamesResults(hr) - for _, addrPort := range hr.GetAddrs() { + // Only literals belong in the cache, anything from DNS has to stay in hr where a not found answer can remove it + for _, addrPort := range hr.literalAddrs() { if !lh.shouldAdd([]netip.Addr{vpnAddr}, addrPort.Addr()) { continue } diff --git a/remote_list.go b/remote_list.go index 90406b47..17f1372b 100644 --- a/remote_list.go +++ b/remote_list.go @@ -2,6 +2,7 @@ package nebula import ( "context" + "errors" "log/slog" "net" "net/netip" @@ -58,6 +59,8 @@ type cacheV6 struct { type hostnamePort struct { name string port uint16 + // addr is set when name is an ip literal, these never need a lookup + addr netip.Addr } type hostnamesResults struct { @@ -65,15 +68,20 @@ type hostnamesResults struct { network string lookupTimeout time.Duration cancelFn func() + resolveNow chan struct{} l *slog.Logger ips atomic.Pointer[map[netip.AddrPort]struct{}] + // known is the last known good addresses per hostname, kept so a reload can carry them forward + known atomic.Pointer[map[hostnamePort][]netip.AddrPort] } -func NewHostnameResults(ctx context.Context, l *slog.Logger, d time.Duration, network string, timeout time.Duration, hostPorts []string, onUpdate func()) (*hostnamesResults, error) { +// NewHostnameResults starts resolving hostPorts. Any hostnames also in prev start with its last known good addresses. +func NewHostnameResults(ctx context.Context, l *slog.Logger, d time.Duration, network string, timeout time.Duration, hostPorts []string, prev *hostnamesResults, onUpdate func()) (*hostnamesResults, error) { r := &hostnamesResults{ hostnames: make([]hostnamePort, len(hostPorts)), network: network, lookupTimeout: timeout, + resolveNow: make(chan struct{}, 1), l: l, } @@ -81,6 +89,13 @@ func NewHostnameResults(ctx context.Context, l *slog.Logger, d time.Duration, ne // DNS lookups for hostnames that aren't hardcoded IP's will happen in a background goroutine. performBackgroundLookup := false ips := map[netip.AddrPort]struct{}{} + known := make([][]netip.AddrPort, len(hostPorts)) + var prevKnown map[hostnamePort][]netip.AddrPort + if prev != nil && prev.network == network { + if p := prev.known.Load(); p != nil { + prevKnown = *p + } + } for idx, hostPort := range hostPorts { rIp, sPort, err := net.SplitHostPort(hostPort) @@ -98,13 +113,19 @@ func NewHostnameResults(ctx context.Context, l *slog.Logger, d time.Duration, ne if err != nil { // This address is a hostname, not an IP address performBackgroundLookup = true + known[idx] = prevKnown[r.hostnames[idx]] + for _, a := range known[idx] { + ips[a] = struct{}{} + } continue } // Save the IP address immediately + r.hostnames[idx].addr = addr ips[netip.AddrPortFrom(addr, uint16(iPort))] = struct{}{} } r.ips.Store(&ips) + r.storeKnown(known) // Time for the DNS lookup goroutine if performBackgroundLookup { @@ -114,21 +135,12 @@ func NewHostnameResults(ctx context.Context, l *slog.Logger, d time.Duration, ne go func() { defer ticker.Stop() for { + r.resolve(newCtx, known) + r.storeKnown(known) netipAddrs := map[netip.AddrPort]struct{}{} - for _, hostPort := range r.hostnames { - timeoutCtx, timeoutCancel := context.WithTimeout(ctx, r.lookupTimeout) - addrs, err := net.DefaultResolver.LookupNetIP(timeoutCtx, r.network, hostPort.name) - timeoutCancel() - if err != nil { - l.Error("DNS resolution failed for static_map host", - "hostname", hostPort.name, - "network", r.network, - "error", err, - ) - continue - } + for _, addrs := range known { for _, a := range addrs { - netipAddrs[netip.AddrPortFrom(a.Unmap(), hostPort.port)] = struct{}{} + netipAddrs[a] = struct{}{} } } origSet := r.ips.Load() @@ -159,7 +171,7 @@ func NewHostnameResults(ctx context.Context, l *slog.Logger, d time.Duration, ne case <-newCtx.Done(): return case <-ticker.C: - continue + case <-r.resolveNow: } } }() @@ -168,6 +180,84 @@ func NewHostnameResults(ctx context.Context, l *slog.Logger, d time.Duration, ne return r, nil } +// resolve looks up every hostname in parallel and folds the results into known +func (hr *hostnamesResults) resolve(ctx context.Context, known [][]netip.AddrPort) { + var wg sync.WaitGroup + for i, hostPort := range hr.hostnames { + if hostPort.addr.IsValid() { + known[i] = []netip.AddrPort{netip.AddrPortFrom(hostPort.addr, hostPort.port)} + continue + } + + wg.Go(func() { + timeoutCtx, timeoutCancel := context.WithTimeout(ctx, hr.lookupTimeout) + addrs, err := net.DefaultResolver.LookupNetIP(timeoutCtx, hr.network, hostPort.name) + timeoutCancel() + known[i] = applyLookup(known[i], hostPort.port, addrs, err) + // A canceled ctx means we are reloading or shutting down, not that DNS failed + if err != nil && ctx.Err() == nil { + hr.l.Error("DNS resolution failed for static_map host", + "hostname", hostPort.name, + "network", hr.network, + "error", err, + "usingAddrs", known[i], + ) + } + }) + } + wg.Wait() +} + +// applyLookup returns the addresses to use for a hostname given the previous ones and a lookup result. +// Only a not found answer clears them, a stale lighthouse address costs a handshake but an empty set costs everything. +// darwin also reports not found while offline, that's harmless since nothing is reachable and the next handshake +// triggers a fresh lookup once we're back. +func applyLookup(prev []netip.AddrPort, port uint16, addrs []netip.Addr, err error) []netip.AddrPort { + if err != nil { + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) && dnsErr.IsNotFound { + return nil + } + return prev + } + + out := make([]netip.AddrPort, 0, len(addrs)) + for _, a := range addrs { + out = append(out, netip.AddrPortFrom(a.Unmap(), port)) + } + return out +} + +func (hr *hostnamesResults) storeKnown(known [][]netip.AddrPort) { + byName := make(map[hostnamePort][]netip.AddrPort, len(known)) + for i, addrs := range known { + byName[hr.hostnames[i]] = addrs + } + hr.known.Store(&byName) +} + +// literalAddrs returns the entries that were configured as ip addresses rather than hostnames +func (hr *hostnamesResults) literalAddrs() []netip.AddrPort { + var out []netip.AddrPort + for _, hostPort := range hr.hostnames { + if hostPort.addr.IsValid() { + out = append(out, netip.AddrPortFrom(hostPort.addr, hostPort.port)) + } + } + return out +} + +// ResolveNow asks the background goroutine to look up the hostnames again without waiting for the next tick +func (hr *hostnamesResults) ResolveNow() { + if hr == nil || hr.resolveNow == nil { + return + } + select { + case hr.resolveNow <- struct{}{}: + default: + } +} + func (hr *hostnamesResults) Cancel() { if hr != nil && hr.cancelFn != nil { hr.cancelFn() @@ -256,6 +346,13 @@ func (r *RemoteList) ResetForOwner(ownerVpnAddr netip.Addr) { r.shouldRebuild = true } +// ResolveHostnames kicks a fresh DNS lookup of any static_map hostnames for this host +func (r *RemoteList) ResolveHostnames() { + r.RLock() + defer r.RUnlock() + r.hr.ResolveNow() +} + // ClearHostnameResults cancels the in-flight DNS resolver goroutine (if any) and drops the resolved IP cache. func (r *RemoteList) ClearHostnameResults() { r.Lock() diff --git a/remote_list_test.go b/remote_list_test.go index 38e98b6b..6be8c4b1 100644 --- a/remote_list_test.go +++ b/remote_list_test.go @@ -1,11 +1,15 @@ package nebula import ( + "context" "encoding/binary" "fmt" + "net" "net/netip" "testing" + "time" + "github.com/slackhq/nebula/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -329,6 +333,55 @@ func BenchmarkSortRebuild(b *testing.B) { }) } +func TestApplyLookup(t *testing.T) { + prev := []netip.AddrPort{netip.MustParseAddrPort("1.1.1.1:4242")} + + // A successful lookup replaces the previous addresses + got := applyLookup(prev, 4242, []netip.Addr{netip.MustParseAddr("::ffff:2.2.2.2")}, nil) + assert.Equal(t, []netip.AddrPort{netip.MustParseAddrPort("2.2.2.2:4242")}, got) + + // A not found answer means the record is gone + assert.Empty(t, applyLookup(prev, 4242, nil, &net.DNSError{Err: "no such host", IsNotFound: true})) + + // Anything else is not new information, keep what we had + assert.Equal(t, prev, applyLookup(prev, 4242, nil, &net.DNSError{Err: "i/o timeout", IsTimeout: true})) + assert.Equal(t, prev, applyLookup(prev, 4242, nil, &net.DNSError{Err: "server misbehaving", IsTemporary: true})) + assert.Equal(t, prev, applyLookup(prev, 4242, nil, context.DeadlineExceeded)) +} + +func TestNewHostnameResults_CarriesKnown(t *testing.T) { + // A canceled context keeps the background lookup from learning anything, as if DNS were down + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + known := netip.MustParseAddrPort("1.1.1.1:4242") + prev := &hostnamesResults{network: "ip4"} + prev.known.Store(&map[hostnamePort][]netip.AddrPort{{name: "lh.invalid", port: 4242}: {known}}) + + hostPorts := []string{"lh.invalid:4242", "10.0.0.1:4242"} + literal := netip.MustParseAddrPort("10.0.0.1:4242") + + hr, err := NewHostnameResults(ctx, test.NewLogger(), time.Hour, "ip4", time.Second, hostPorts, prev, func() {}) + require.NoError(t, err) + assert.ElementsMatch(t, []netip.AddrPort{known, literal}, hr.GetAddrs()) + assert.Equal(t, []netip.AddrPort{literal}, hr.literalAddrs()) + + // And again from the new one, before it has finished a lookup of its own + hr2, err := NewHostnameResults(ctx, test.NewLogger(), time.Hour, "ip4", time.Second, hostPorts, hr, func() {}) + require.NoError(t, err) + assert.ElementsMatch(t, []netip.AddrPort{known, literal}, hr2.GetAddrs()) + + // A port change is a different entry + hr3, err := NewHostnameResults(ctx, test.NewLogger(), time.Hour, "ip4", time.Second, []string{"lh.invalid:4243"}, prev, func() {}) + require.NoError(t, err) + assert.Empty(t, hr3.GetAddrs()) + + // So is a network change + hr4, err := NewHostnameResults(ctx, test.NewLogger(), time.Hour, "ip6", time.Second, hostPorts, prev, func() {}) + require.NoError(t, err) + assert.Equal(t, []netip.AddrPort{literal}, hr4.GetAddrs()) +} + func newIp4AndPortFromString(s string) *V4AddrPort { a := netip.MustParseAddrPort(s) v4Addr := a.Addr().As4()