diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index 53380b2aa..948000a3d 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -6,8 +6,10 @@ import ( "fmt" "io" "net/netip" + "os" "os/exec" "slices" + "strconv" "strings" "syscall" "time" @@ -34,10 +36,16 @@ var ( // Registry locations of the host DNS configuration this package programs, // exported so a diagnostic reader reports the same locations that are written. const ( - // NRPTKeyPrefix starts the name of every NRPT rule key this client creates. - // Older versions used different layouts under the same prefix: a single - // unsuffixed key, then one key per domain, now one key per batch of domains. - NRPTKeyPrefix = "NetBird-Match" + // NRPTKeyPrefix starts the name of every NRPT rule key this client creates: + // the match rules, the catch-all, and the .local exemption. Cleanup + // enumerates by this prefix, so a new kind of rule is removed by existing + // code as long as its key starts here. + NRPTKeyPrefix = "NetBird-" + + // nrptMatchKeyName names the match-domain rules. Older versions used + // different layouts under the same name: a single unsuffixed key, then one + // key per domain, now one key per batch of domains. + nrptMatchKeyName = NRPTKeyPrefix + "Match" // DNSPolicyConfigRoot holds the NRPT rules of the local policy store. DNSPolicyConfigRoot = `SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig` @@ -53,8 +61,24 @@ const ( ) const ( - dnsPolicyConfigMatchPath = DNSPolicyConfigRoot + `\` + NRPTKeyPrefix - gpoDnsPolicyConfigMatchPath = GPODNSPolicyConfigRoot + `\` + NRPTKeyPrefix + dnsPolicyConfigMatchPath = DNSPolicyConfigRoot + `\` + nrptMatchKeyName + gpoDnsPolicyConfigMatchPath = GPODNSPolicyConfigRoot + `\` + nrptMatchKeyName + + dnsPolicyConfigExemptLocalPath = DNSPolicyConfigRoot + `\` + NRPTKeyPrefix + `ExemptLocal` + gpoDnsPolicyConfigExemptLocalPath = GPODNSPolicyConfigRoot + `\` + NRPTKeyPrefix + `ExemptLocal` + + nrptCatchAllNamespace = "." + // nrptLocalNamespace is reserved for multicast DNS by RFC 6762: a unicast + // resolver must not answer for it. The catch-all rule would hand it to us + // anyway, so it gets an exemption rule of its own. + nrptLocalNamespace = ".local" + + // envLegacyDNSResolution restores the pre-catch-all behaviour: the adapter's + // NameServer alone, leaving the OS free to query other adapters' resolvers in + // parallel. An escape hatch for setups that depend on a resolver of theirs + // still being reachable while connected, at the cost of the leak and of the + // race the catch-all rule exists to close. + envLegacyDNSResolution = "NB_USE_LEGACY_DNS_RESOLUTION" dnsPolicyConfigVersionKey = "Version" dnsPolicyConfigVersionValue = 2 @@ -293,6 +317,13 @@ func (r *registryConfigurator) disableWINSForInterface() error { } func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error { + // Clear every rule the previous apply installed before installing any new + // one, including a leftover catch-all: removal is unconditional so a rule + // from an earlier run cannot survive into a config that no longer wants it. + if err := r.removeDNSMatchPolicies(); err != nil { + log.Errorf("cleanup old dns match policies: %s", err) + } + if config.RouteAll { if err := r.addDNSSetupForAll(config.ServerIP); err != nil { return fmt.Errorf("add dns setup: %w", err) @@ -318,8 +349,22 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager matchDomains = append(matchDomains, "."+strings.TrimSuffix(dConf.Domain, ".")) } - if err := r.removeDNSMatchPolicies(); err != nil { - log.Errorf("cleanup old dns match policies: %s", err) + // The root namespace is a match domain like any other: it just happens to + // match every name. Without it the adapter's NameServer only adds one more + // resolver to the set Windows queries in parallel, keeping whichever answer + // comes back first — which leaks every query to the local network and lets a + // resolver other than ours answer for a name we are authoritative for. + if config.RouteAll { + if parseBoolEnv(envLegacyDNSResolution) { + log.Infof("%s is set, leaving DNS resolution shared with the other adapters' resolvers instead of forcing it through %s", envLegacyDNSResolution, config.ServerIP) + } else { + matchDomains = append(matchDomains, nrptCatchAllNamespace) + log.Infof("routing every namespace through %s: DNS resolution is now exclusive to NetBird", config.ServerIP) + + if err := r.addDNSExemptLocalPolicy(); err != nil { + return fmt.Errorf("add dns exempt policy: %w", err) + } + } } if len(matchDomains) != 0 { @@ -397,6 +442,42 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr return nil } +// addDNSExemptLocalPolicy carves .local back out of the catch-all. RFC 6762 +// reserves it for multicast DNS, so forwarding those names to a unicast +// upstream answers NXDOMAIN for hosts that do exist - printers, NAS boxes, and +// anything else announcing itself on the link - and the answer is authoritative +// enough that Windows stops looking. A rule naming the namespace with no +// servers hands it back to the DNS client untouched. A more specific rule still +// wins, so a match domain under .local keeps going through us. +func (r *registryConfigurator) addDNSExemptLocalPolicy() error { + var noServers netip.Addr + + if err := r.configureDNSPolicy(dnsPolicyConfigExemptLocalPath, []string{nrptLocalNamespace}, noServers); err != nil { + return fmt.Errorf("configure exempt policy for %s: %w", nrptLocalNamespace, err) + } + + if r.gpo { + if err := r.configureDNSPolicy(gpoDnsPolicyConfigExemptLocalPath, []string{nrptLocalNamespace}, noServers); err != nil { + return fmt.Errorf("configure gpo exempt policy for %s: %w", nrptLocalNamespace, err) + } + if err := refreshGroupPolicy(); err != nil { + log.Warnf("failed to refresh group policy: %v", err) + } + } + + log.Infof("added NRPT exemption for %s, leaving it to the OS resolver", nrptLocalNamespace) + return nil +} + +// configureDNSPolicy writes one NRPT rule. An invalid ip writes an exemption +// rule: the namespace with an empty server list, which tells the DNS client to +// resolve those names the way it would without any rule at all. +// +// The empty string is the whole difference, and it has to be written: dropping +// the value and clearing ConfigOptions instead produces a rule Windows treats +// as a no-op, keeps out of Get-DnsClientNrptPolicy -Effective, and ignores in +// favour of the catch-all. 0x8 says the server list is the meaningful part of +// the rule, and an empty list then means "no server, resolve normally". func (r *registryConfigurator) configureDNSPolicy(policyPath string, domains []string, ip netip.Addr) error { if err := removeRegistryKeyFromDNSPolicyConfig(policyPath); err != nil { return fmt.Errorf("remove existing dns policy: %w", err) @@ -416,7 +497,11 @@ func (r *registryConfigurator) configureDNSPolicy(policyPath string, domains []s return fmt.Errorf("set %s: %w", dnsPolicyConfigNameKey, err) } - if err := regKey.SetStringValue(dnsPolicyConfigGenericDNSServersKey, ip.String()); err != nil { + var servers string + if ip.IsValid() { + servers = ip.String() + } + if err := regKey.SetStringValue(dnsPolicyConfigGenericDNSServersKey, servers); err != nil { return fmt.Errorf("set %s: %w", dnsPolicyConfigGenericDNSServersKey, err) } @@ -514,8 +599,11 @@ func (r *registryConfigurator) getInterfaceRegistryKey() (registry.Key, error) { } func (r *registryConfigurator) restoreHostDNS() error { + // Propagated, unlike in applyDNSConfig: there we are about to write fresh + // rules over whatever survived, here we are leaving, and a rule left behind + // keeps sending every query to an address that is about to disappear. if err := r.removeDNSMatchPolicies(); err != nil { - log.Errorf("remove dns match policies: %s", err) + return fmt.Errorf("remove dns match policies: %w", err) } if err := r.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey); err != nil { @@ -598,9 +686,17 @@ func listNRPTRuleKeys(root string) ([]string, error) { func removeRegistryKeyFromDNSPolicyConfig(regKeyPath string) error { k, err := registry.OpenKey(registry.LOCAL_MACHINE, regKeyPath, registry.QUERY_VALUE) - if err != nil { - log.Debugf("failed to open HKEY_LOCAL_MACHINE\\%s: %v", regKeyPath, err) + switch { + case errors.Is(err, registry.ErrNotExist), errors.Is(err, syscall.ERROR_PATH_NOT_FOUND): + // nothing to remove, which is the normal case for a rule this config + // never installed + log.Debugf("HKEY_LOCAL_MACHINE\\%s does not exist", regKeyPath) return nil + case err != nil: + // anything else has to reach the caller: reporting success here would + // leave the rule in force while claiming it was removed, which is how a + // stale rule outlives the interface it points at + return fmt.Errorf("open HKEY_LOCAL_MACHINE\\%s: %w", regKeyPath, err) } closer(k) @@ -636,6 +732,20 @@ func refreshGroupPolicy() error { return nil } +func parseBoolEnv(key string) bool { + val := os.Getenv(key) + if val == "" { + return false + } + + parsed, err := strconv.ParseBool(val) + if err != nil { + log.Warnf("failed to parse %s=%q: %v", key, val, err) + return false + } + return parsed +} + func closer(closer io.Closer) { if err := closer.Close(); err != nil { log.Errorf("failed to close: %s", err) diff --git a/client/internal/dns/host_windows_test.go b/client/internal/dns/host_windows_test.go index 861613c95..7aef64590 100644 --- a/client/internal/dns/host_windows_test.go +++ b/client/internal/dns/host_windows_test.go @@ -94,6 +94,145 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) { assert.False(t, exists, "NRPT rule 2 should NOT exist after reducing to 75 domains") } +// TestNRPTCatchAllRule verifies that RouteAll adds the root namespace to the +// match rule instead of a rule of its own, that .local is carved back out with +// an empty server list, and that both go away when RouteAll is cleared or the +// host DNS is restored. +func TestNRPTCatchAllRule(t *testing.T) { + if testing.Short() { + t.Skip("skipping registry integration test in short mode") + } + + defer cleanupRegistryKeys(t) + cleanupRegistryKeys(t) + + testIP := netip.MustParseAddr("100.64.0.1") + testGUID := "{12345678-1234-1234-1234-123456789ABC}" + interfacePath := InterfaceConfigPath + `\` + testGUID + testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) + require.NoError(t, err, "Should create test interface registry key") + require.NoError(t, testKey.Close(), "close test interface registry key") + defer func() { + assert.NoError(t, registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath), "delete test interface registry key") + }() + + cfg := ®istryConfigurator{guid: testGUID} + + matchOnly := HostDNSConfig{ + ServerIP: testIP, + Domains: []DomainConfig{{Domain: "example.com", MatchOnly: true}}, + } + primary := HostDNSConfig{ + ServerIP: testIP, + RouteAll: true, + Domains: []DomainConfig{{Domain: "example.com", MatchOnly: true}}, + } + firstRule := fmt.Sprintf("%s-0", dnsPolicyConfigMatchPath) + + // The root namespace is not a rule of its own: it rides in the match rule, + // which is the point of it not being a special case. + require.NoError(t, cfg.applyDNSConfig(matchOnly, nil)) + names := ruleNamespaces(t, firstRule) + assert.Contains(t, names, ".example.com") + assert.NotContains(t, names, nrptCatchAllNamespace, "a match-only config must not claim every namespace") + + require.NoError(t, cfg.applyDNSConfig(primary, nil)) + names = ruleNamespaces(t, firstRule) + assert.Contains(t, names, ".example.com") + assert.Contains(t, names, nrptCatchAllNamespace, "RouteAll should add the root namespace to the match rule") + + k, err := registry.OpenKey(registry.LOCAL_MACHINE, firstRule, registry.QUERY_VALUE) + require.NoError(t, err) + servers, _, err := k.GetStringValue(dnsPolicyConfigGenericDNSServersKey) + require.NoError(t, err) + assert.Equal(t, testIP.String(), servers, "every namespace in the rule resolves through our resolver") + require.NoError(t, k.Close(), "close match rule key") + + // .local is carved back out: RFC 6762 reserves it for mDNS, so it needs a + // rule of its own — it is the one rule with a different server list. + ek, err := registry.OpenKey(registry.LOCAL_MACHINE, dnsPolicyConfigExemptLocalPath, registry.QUERY_VALUE) + require.NoError(t, err, "exemption rule should exist once the root namespace is claimed") + + exemptNames, _, err := ek.GetStringsValue(dnsPolicyConfigNameKey) + require.NoError(t, err) + assert.Equal(t, []string{nrptLocalNamespace}, exemptNames, "the exemption should name only the mDNS namespace") + + exemptServers, _, err := ek.GetStringValue(dnsPolicyConfigGenericDNSServersKey) + require.NoError(t, err, "the value has to be present, empty: without it Windows drops the rule") + assert.Empty(t, exemptServers, "an exemption rule lists no servers") + + exemptOpts, _, err := ek.GetIntegerValue(dnsPolicyConfigConfigOptionsKey) + require.NoError(t, err) + assert.EqualValues(t, dnsPolicyConfigConfigOptionsValue, exemptOpts, "same options as a normal rule; the empty server list is what makes it an exemption") + require.NoError(t, ek.Close(), "close exemption rule key") + + require.NoError(t, cfg.applyDNSConfig(matchOnly, nil)) + names = ruleNamespaces(t, firstRule) + assert.NotContains(t, names, nrptCatchAllNamespace, "clearing RouteAll should drop the root namespace") + + exists, err := registryKeyExists(dnsPolicyConfigExemptLocalPath) + require.NoError(t, err) + assert.False(t, exists, "exemption rule should go with the namespace it carves out of") + + require.NoError(t, cfg.applyDNSConfig(primary, nil)) + require.NoError(t, cfg.restoreHostDNS()) + exists, err = registryKeyExists(firstRule) + require.NoError(t, err) + assert.False(t, exists, "restore should leave no rule behind") +} + +// ruleNamespaces returns the namespaces an NRPT rule key claims. +func ruleNamespaces(t *testing.T, path string) []string { + t.Helper() + k, err := registry.OpenKey(registry.LOCAL_MACHINE, path, registry.QUERY_VALUE) + require.NoError(t, err, "rule key %s should exist", path) + defer k.Close() + + names, _, err := k.GetStringsValue(dnsPolicyConfigNameKey) + require.NoError(t, err) + return names +} + +// TestNRPTCatchAllRuleLegacyEnv verifies that NB_USE_LEGACY_DNS_RESOLUTION +// leaves the root namespace unclaimed, so no rule is written for a RouteAll +// config that carries no match domains. +func TestNRPTCatchAllRuleLegacyEnv(t *testing.T) { + if testing.Short() { + t.Skip("skipping registry integration test in short mode") + } + + defer cleanupRegistryKeys(t) + cleanupRegistryKeys(t) + + t.Setenv(envLegacyDNSResolution, "true") + + testGUID := "{12345678-1234-1234-1234-123456789ABC}" + interfacePath := InterfaceConfigPath + `\` + testGUID + testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) + require.NoError(t, err, "Should create test interface registry key") + require.NoError(t, testKey.Close(), "close test interface registry key") + defer func() { + assert.NoError(t, registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath), "delete test interface registry key") + }() + + cfg := ®istryConfigurator{guid: testGUID} + config := HostDNSConfig{ + ServerIP: netip.MustParseAddr("100.64.0.1"), + RouteAll: true, + } + + require.NoError(t, cfg.applyDNSConfig(config, nil)) + + // RouteAll with no match domains and the switch set leaves nothing to write. + exists, err := registryKeyExists(fmt.Sprintf("%s-0", dnsPolicyConfigMatchPath)) + require.NoError(t, err) + assert.False(t, exists, "no rule should be written when the legacy env var is set") + + exists, err = registryKeyExists(dnsPolicyConfigExemptLocalPath) + require.NoError(t, err) + assert.False(t, exists, "no exemption without a claimed root namespace") +} + func registryKeyExists(path string) (bool, error) { k, err := registry.OpenKey(registry.LOCAL_MACHINE, path, registry.QUERY_VALUE) if err != nil { diff --git a/client/ios/NetBirdSDK/client.go b/client/ios/NetBirdSDK/client.go index 8373e498a..bbbb969c9 100644 --- a/client/ios/NetBirdSDK/client.go +++ b/client/ios/NetBirdSDK/client.go @@ -4,12 +4,14 @@ package NetBirdSDK import ( "context" + "errors" "fmt" "net/netip" "os" "sort" "strings" "sync" + "sync/atomic" "time" log "github.com/sirupsen/logrus" @@ -37,6 +39,8 @@ const ( AnonymizeLevelStrict = nbAnonymize.LevelStrictString ) +var errClientAlreadyRunning = errors.New("client is already running") + // RouteListener export internal RouteListener for mobile type NetworkChangeListener interface { listener.NetworkChangeListener @@ -74,15 +78,13 @@ type Client struct { cacheDir string logFilePath string recorder *peer.Status - ctxCancel context.CancelFunc - ctxCancelLock *sync.Mutex deviceName string osName string osVersion string networkChangeListener listener.NetworkChangeListener onHostDnsFn func([]string) dnsManager dns.IosDnsManager - loginComplete bool + loginComplete atomic.Bool // netMgr outlives engine restarts: it mirrors the OS connectivity, not // the engine lifecycle. Run injects its state and sweeper into each new // ConnectClient. @@ -90,9 +92,16 @@ type Client struct { // preloadedConfig holds config loaded from JSON (used on tvOS where file writes are blocked) preloadedConfig *profilemanager.Config + // stateMu guards the run lifecycle as one unit: the cancel installed by + // the current run, the channel it closes on exit, and the state it + // published. One run at a time: startRun refuses a second Run while the + // previous one has not exited, and the platform serializes Stop before + // Start, so no generation tracking is needed. stateMu sync.RWMutex connectClient *internal.ConnectClient config *profilemanager.Config + runDone chan struct{} + ctxCancel context.CancelFunc } // NewClient instantiate a new Client @@ -107,7 +116,6 @@ func NewClient(cfgFile, stateFile, cacheDir, logFilePath, deviceName string, osV osName: osName, osVersion: osVersion, recorder: recorder, - ctxCancelLock: &sync.Mutex{}, networkChangeListener: networkChangeListener, dnsManager: dnsManager, netMgr: netevents.NewManager(recorder), @@ -156,17 +164,21 @@ func (c *Client) Run(fd int32, interfaceName string, envList *EnvList) error { c.recorder.UpdateManagementAddress(cfg.ManagementURL.String()) c.recorder.UpdateRosenpass(cfg.RosenpassEnabled, cfg.RosenpassPermissive) - var ctx context.Context //nolint ctxWithValues := context.WithValue(context.Background(), system.DeviceNameCtxKey, c.deviceName) //nolint ctxWithValues = context.WithValue(ctxWithValues, system.OsNameCtxKey, c.osName) //nolint ctxWithValues = context.WithValue(ctxWithValues, system.OsVersionCtxKey, c.osVersion) - c.ctxCancelLock.Lock() - ctx, c.ctxCancel = context.WithCancel(ctxWithValues) - defer c.ctxCancel() - c.ctxCancelLock.Unlock() + runCtx, runCancel := context.WithCancel(ctxWithValues) + defer runCancel() + + done, err := c.startRun(runCancel) + if err != nil { + return err + } + defer c.finishRun(done) + ctx := runCtx // No login pre-flight here. The engine's own loginToManagement (connect.go) performs // the authoritative Login immediately before the first Sync, so a LoginSync() call at @@ -215,16 +227,40 @@ func (c *Client) NotifyNetworkChange() { c.netMgr.NotifyNetworkChange() } -// Stop the internal client and free the resources +// Stop cancels the running client and waits for the run loop to exit, so a +// caller that restarts immediately cannot race the outgoing teardown. func (c *Client) Stop() { - c.ctxCancelLock.Lock() - defer c.ctxCancelLock.Unlock() - if c.ctxCancel == nil { + done := c.cancelRun() + if done == nil { return } - c.ctxCancel() - c.setState(nil, nil) + select { + case <-done: + case <-time.After(stopRunWaitTimeout): + log.Warnf("Stop: timed out waiting for the run loop to exit") + } +} + +// StopWithoutWait cancels the running client without waiting for the run loop. +// Use it where the caller is on a deadline the wait could overrun, such as +// NEPacketTunnelProvider.stopTunnel, which iOS gives only a few seconds +// before it kills the extension. +func (c *Client) StopWithoutWait() { + c.cancelRun() +} + +func (c *Client) cancelRun() chan struct{} { + c.stateMu.RLock() + done := c.runDone + cancel := c.ctxCancel + c.stateMu.RUnlock() + + if cancel != nil { + cancel() + } + + return done } // DebugBundle generates a debug bundle, uploads it and returns the upload key. @@ -376,16 +412,14 @@ func (c *Client) IsLoginRequiredCached() bool { } func (c *Client) IsLoginRequired() bool { - var ctx context.Context //nolint ctxWithValues := context.WithValue(context.Background(), system.DeviceNameCtxKey, c.deviceName) //nolint ctxWithValues = context.WithValue(ctxWithValues, system.OsNameCtxKey, c.osName) //nolint ctxWithValues = context.WithValue(ctxWithValues, system.OsVersionCtxKey, c.osVersion) - c.ctxCancelLock.Lock() - defer c.ctxCancelLock.Unlock() - ctx, c.ctxCancel = context.WithCancel(ctxWithValues) + ctx, cancel := context.WithCancel(ctxWithValues) + defer cancel() var cfg *profilemanager.Config var err error @@ -433,17 +467,22 @@ func (c *Client) IsLoginRequired() bool { // loginForMobileAuthTimeout is the timeout for requesting auth info from the server const loginForMobileAuthTimeout = 30 * time.Second +const stopRunWaitTimeout = 20 * time.Second + func (c *Client) LoginForMobile() string { - var ctx context.Context //nolint ctxWithValues := context.WithValue(context.Background(), system.DeviceNameCtxKey, c.deviceName) //nolint ctxWithValues = context.WithValue(ctxWithValues, system.OsNameCtxKey, c.osName) //nolint ctxWithValues = context.WithValue(ctxWithValues, system.OsVersionCtxKey, c.osVersion) - c.ctxCancelLock.Lock() - defer c.ctxCancelLock.Unlock() - ctx, c.ctxCancel = context.WithCancel(ctxWithValues) + ctx, cancel := context.WithCancel(ctxWithValues) + loginDone := false + defer func() { + if !loginDone { + cancel() + } + }() // Use DirectUpdateOrCreateConfig to avoid atomic file operations (temp file + rename) // which are blocked by the tvOS sandbox in App Group containers @@ -470,7 +509,9 @@ func (c *Client) LoginForMobile() string { } // This could cause a potential race condition with loading the extension which need to be handled on swift side + loginDone = true go func() { + defer cancel() tokenInfo, err := oAuthFlow.WaitToken(ctx, flowInfo) if err != nil { log.Errorf("LoginForMobile: WaitToken failed: %v", err) @@ -487,18 +528,18 @@ func (c *Client) LoginForMobile() string { log.Errorf("LoginForMobile: Login failed: %v", err) return } - c.loginComplete = true + c.loginComplete.Store(true) }() return flowInfo.VerificationURIComplete } func (c *Client) IsLoginComplete() bool { - return c.loginComplete + return c.loginComplete.Load() } func (c *Client) ClearLoginComplete() { - c.loginComplete = false + c.loginComplete.Store(false) } func (c *Client) GetRoutesSelectionDetails() (*RoutesSelectionDetails, error) { @@ -718,13 +759,36 @@ func (c *Client) DeselectRoute(id string) error { return nil } -// setState stores the running engine state so DebugBundle can reuse the live -// config and ConnectClient. It is cleared on Stop. -func (c *Client) setState(cfg *profilemanager.Config, cc *internal.ConnectClient) { +func (c *Client) startRun(cancel context.CancelFunc) (chan struct{}, error) { c.stateMu.Lock() defer c.stateMu.Unlock() + + if c.runDone != nil { + return nil, errClientAlreadyRunning + } + + done := make(chan struct{}) + c.runDone = done + c.ctxCancel = cancel + return done, nil +} + +func (c *Client) finishRun(done chan struct{}) { + c.stateMu.Lock() + c.connectClient = nil + c.config = nil + c.runDone = nil + c.ctxCancel = nil + c.stateMu.Unlock() + + close(done) +} + +func (c *Client) setState(cfg *profilemanager.Config, cc *internal.ConnectClient) { + c.stateMu.Lock() c.config = cfg c.connectClient = cc + c.stateMu.Unlock() } // stateSnapshot returns the current config and ConnectClient under the lock. diff --git a/infrastructure_files/getting-started-enterprise.sh b/infrastructure_files/getting-started-enterprise.sh index 7418cb8e8..3f7cf6357 100755 --- a/infrastructure_files/getting-started-enterprise.sh +++ b/infrastructure_files/getting-started-enterprise.sh @@ -15,16 +15,25 @@ NETBIRD_EULA_URL="https://netbird.io/self-hosted-EULA" # server trusts X-Forwarded-* headers from this address only. TRAEFIK_IP="172.30.0.10" +LICENSE_VERDICT="unknown" +LICENSE_LOG_LINES="" + check_docker_compose() { - if command -v docker-compose &> /dev/null; then - echo "docker-compose" - return + if ! command -v docker &> /dev/null && ! command -v docker-compose &> /dev/null; then + echo "Docker is not installed or not in PATH. Please follow the steps from the official guide: https://docs.docker.com/engine/install/" > /dev/stderr + exit 1 fi - if docker compose --help &> /dev/null; then + + if docker compose version &> /dev/null; then echo "docker compose" return fi - echo "docker-compose is not installed or not in PATH. See https://docs.docker.com/engine/install/" > /dev/stderr + if command -v docker-compose &> /dev/null && docker-compose version &> /dev/null; then + echo "docker-compose" + return + fi + + echo "Docker Compose is not installed or not in PATH. Please follow the steps from the official guide: https://docs.docker.com/compose/install/" > /dev/stderr exit 1 } @@ -221,6 +230,90 @@ wait_postgres() { set -e } +wait_for_license_verdict() { + local counter=0 + local logs="" + + echo -n "Waiting for the server to validate the license" + while [[ $counter -lt 60 ]]; do + logs=$($DOCKER_COMPOSE_COMMAND logs --no-color --tail=all netbird-server 2>/dev/null || true) + + if grep -qi "license invalidated" <<< "$logs"; then + echo " rejected" + LICENSE_VERDICT="rejected" + LICENSE_LOG_LINES=$(grep -i "license" <<< "$logs" | tail -n 5 || true) + return 0 + fi + + if grep -qi "license validated" <<< "$logs"; then + echo " ok" + LICENSE_VERDICT="ok" + return 0 + fi + + echo -n " ." + sleep 2 + counter=$((counter + 1)) + done + + echo " no verdict in 120s" + LICENSE_VERDICT="unknown" + LICENSE_LOG_LINES=$(grep -iE "failed to validate license|error validating license" <<< "$logs" | tail -n 3 || true) + return 0 +} + +report_license_verdict() { + if [[ "$LICENSE_VERDICT" == "ok" ]]; then + return 0 + fi + + if [[ "$LICENSE_VERDICT" == "unknown" ]]; then + echo "" + echo " ⚠ The server logged no license verdict within 120s." + if [[ -n "$LICENSE_LOG_LINES" ]]; then + echo " It was still reporting validation errors:" + while IFS= read -r line; do + [[ -n "$line" ]] && echo " $line" + done <<< "$LICENSE_LOG_LINES" + fi + echo "" + echo " Check the verdict with:" + echo "" + echo " $DOCKER_COMPOSE_COMMAND logs netbird-server | grep -i license" + return 0 + fi + + local unreachable="false" + if grep -qi "couldn't be validated with the license server" <<< "$LICENSE_LOG_LINES"; then + unreachable="true" + fi + + echo "" + if [[ "$unreachable" == "true" ]]; then + echo " ⚠ The server could not validate the license:" + else + echo " ⚠ The server rejected the license key:" + fi + while IFS= read -r line; do + [[ -n "$line" ]] && echo " $line" + done <<< "$LICENSE_LOG_LINES" + echo "" + echo " The stack is up, and only the license check did not pass." + echo "" + if [[ "$unreachable" == "true" ]]; then + echo " The license server could not be reached, so the key itself was" + echo " never checked. Confirm this host has outbound access to the" + echo " license server, then restart:" + else + echo " Check the reason the server gave above, verify that" + echo " NETBIRD_LICENSE_KEY in .env matches the key you were issued," + echo " then restart:" + fi + echo "" + echo " $DOCKER_COMPOSE_COMMAND up -d" + return 0 +} + init_environment() { check_openssl DOCKER_COMPOSE_COMMAND=$(check_docker_compose) @@ -299,6 +392,9 @@ init_environment() { echo "Starting remaining services ..." $DOCKER_COMPOSE_COMMAND up -d + echo "" + wait_for_license_verdict + echo "" echo "Done." echo "" @@ -309,6 +405,12 @@ init_environment() { echo "" echo "Tail logs:" echo " cd $(pwd) && $DOCKER_COMPOSE_COMMAND logs -f netbird-server traefik" + + report_license_verdict + + if [[ "$LICENSE_VERDICT" == "rejected" ]]; then + exit 1 + fi } # ------------------------------------------------------------------ diff --git a/infrastructure_files/getting-started.sh b/infrastructure_files/getting-started.sh index 0fc5b23c5..5efc0181e 100755 --- a/infrastructure_files/getting-started.sh +++ b/infrastructure_files/getting-started.sh @@ -60,18 +60,21 @@ check_docker_sock_perms() { } check_docker_compose() { - if command -v docker-compose &> /dev/null - then - echo "docker-compose" - return - fi - if docker compose --help &> /dev/null - then - echo "docker compose" - return + if ! command -v docker &> /dev/null && ! command -v docker-compose &> /dev/null; then + echo "Docker is not installed or not in PATH. Please follow the steps from the official guide: https://docs.docker.com/engine/install/" > /dev/stderr + exit 1 fi - echo "docker-compose is not installed or not in PATH. Please follow the steps from the official guide: https://docs.docker.com/engine/install/" > /dev/stderr + if docker compose version &> /dev/null; then + echo "docker compose" + return + fi + if command -v docker-compose &> /dev/null && docker-compose version &> /dev/null; then + echo "docker-compose" + return + fi + + echo "Docker Compose is not installed or not in PATH. Please follow the steps from the official guide: https://docs.docker.com/compose/install/" > /dev/stderr exit 1 } @@ -98,19 +101,39 @@ get_main_ip_address() { } check_nb_domain() { - DOMAIN=$1 - if [[ "$DOMAIN-x" == "-x" ]]; then + local domain="$1" + + if [[ -z "$domain" ]]; then echo "The NETBIRD_DOMAIN variable cannot be empty." > /dev/stderr return 1 fi - - if [[ "$DOMAIN" == "netbird.example.com" ]]; then + if [[ "$domain" == "use-ip" ]]; then + return 0 + fi + if [[ "$domain" == "netbird.example.com" ]]; then echo "The NETBIRD_DOMAIN cannot be netbird.example.com" > /dev/stderr return 1 fi + if [[ "$domain" =~ ^[0-9.]+$ ]]; then + echo "'$domain' is an IP address. Use 'use-ip' to install on this host's IP over HTTP, or an FQDN to get a TLS certificate." > /dev/stderr + return 1 + fi + if [[ ! "$domain" =~ ^[A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])?(\.[A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])?)+$ ]]; then + echo "'$domain' is not a valid FQDN. It needs at least one dot (e.g. netbird.my-domain.com), with no scheme, port or trailing dot." > /dev/stderr + return 1 + fi return 0 } +check_domain_resolves() { + local domain="$1" + if command -v getent &> /dev/null && getent hosts "$domain" &> /dev/null; then return 0; fi + if command -v host &> /dev/null && host "$domain" &> /dev/null; then return 0; fi + if command -v dig &> /dev/null && [[ -n "$(dig +short "$domain" 2>/dev/null)" ]]; then return 0; fi + if command -v nslookup &> /dev/null && nslookup "$domain" &> /dev/null; then return 0; fi + return 1 +} + # Non-interactive configuration # ------------------------------ # Every prompt below can be pre-answered with an environment variable, so the @@ -170,7 +193,22 @@ read_nb_domain() { read -r READ_NETBIRD_DOMAIN < /dev/tty if ! check_nb_domain "$READ_NETBIRD_DOMAIN"; then read_nb_domain + return fi + + if [[ "$READ_NETBIRD_DOMAIN" != "use-ip" ]] && ! check_domain_resolves "$READ_NETBIRD_DOMAIN"; then + local confirm="" + echo "" > /dev/stderr + echo "Warning: '$READ_NETBIRD_DOMAIN' does not resolve via DNS from this host." > /dev/stderr + echo "TLS certificate issuance and client connections will fail until it does." > /dev/stderr + echo -n "Continue anyway? [y/N]: " > /dev/stderr + read -r confirm < /dev/tty + if [[ ! "$confirm" =~ ^[Yy]$ ]]; then + read_nb_domain + return + fi + fi + echo "$READ_NETBIRD_DOMAIN" return 0 } @@ -439,12 +477,23 @@ configure_domain() { # Domain is validated (not a free-form value), so it keeps its own guard # rather than going through resolve(): a valid NETBIRD_DOMAIN is used as-is, # otherwise we prompt, or abort when there is no terminal to prompt on. + local prompted="false" if ! check_nb_domain "$NETBIRD_DOMAIN"; then if ! tty_available; then - echo "NETBIRD_DOMAIN is required for a non-interactive install." > /dev/stderr + if [[ -n "$NETBIRD_DOMAIN" ]]; then + echo "NETBIRD_DOMAIN='$NETBIRD_DOMAIN' cannot be used for a non-interactive install." > /dev/stderr + else + echo "NETBIRD_DOMAIN is required for a non-interactive install." > /dev/stderr + fi exit 1 fi NETBIRD_DOMAIN=$(read_nb_domain) + prompted="true" + fi + + if [[ "$prompted" == "false" && "$NETBIRD_DOMAIN" != "use-ip" ]] && ! check_domain_resolves "$NETBIRD_DOMAIN"; then + echo "Warning: '$NETBIRD_DOMAIN' does not resolve via DNS from this host." > /dev/stderr + echo "TLS certificate issuance and client connections will fail until it does." > /dev/stderr fi if [[ "$NETBIRD_DOMAIN" == "use-ip" ]]; then diff --git a/infrastructure_files/migrate-to-enterprise.sh b/infrastructure_files/migrate-to-enterprise.sh index 744ba5375..2b10250c9 100755 --- a/infrastructure_files/migrate-to-enterprise.sh +++ b/infrastructure_files/migrate-to-enterprise.sh @@ -40,6 +40,10 @@ ENTERPRISE_CONFIG_FILE="config.yaml.enterprise" # completed successfully. ROLLBACK_STATE="disarmed" ENV_EXISTED="unknown" +# Verdict the server logs about the license key on startup: ok, rejected, or +# unknown when neither line appeared before the timeout. +LICENSE_VERDICT="unknown" +LICENSE_LOG_LINES="" ENV_BACKUP="" PG_VOLUME_NAME="" BACKUP_DIR="" @@ -59,15 +63,21 @@ ENTERPRISE_CONFIG="no" NETBIRD_EULA_URL="https://netbird.io/self-hosted-EULA" check_docker_compose() { - if command -v docker-compose &> /dev/null; then - echo "docker-compose" - return + if ! command -v docker &> /dev/null && ! command -v docker-compose &> /dev/null; then + echo "Docker is not installed or not in PATH. Please follow the steps from the official guide: https://docs.docker.com/engine/install/" > /dev/stderr + exit 1 fi - if docker compose --help &> /dev/null; then + + if docker compose version &> /dev/null; then echo "docker compose" return fi - echo "docker-compose is not installed or not in PATH." > /dev/stderr + if command -v docker-compose &> /dev/null && docker-compose version &> /dev/null; then + echo "docker-compose" + return + fi + + echo "Docker Compose is not installed or not in PATH. Please follow the steps from the official guide: https://docs.docker.com/compose/install/" > /dev/stderr exit 1 } @@ -1000,6 +1010,39 @@ init_migration() { check_stale_postgres_volume } +wait_for_license_verdict() { + local counter=0 + local logs="" + + echo -n "Waiting for the server to validate the license" + while [[ $counter -lt 60 ]]; do + + logs=$($DOCKER_COMPOSE_COMMAND logs --no-color --tail=all "$COMBINED_SERVICE" 2>/dev/null || true) + + if grep -qi "license invalidated" <<< "$logs"; then + echo " rejected" + LICENSE_VERDICT="rejected" + LICENSE_LOG_LINES=$(grep -i "license" <<< "$logs" | tail -n 5 || true) + return 0 + fi + + if grep -qi "license validated" <<< "$logs"; then + echo " ok" + LICENSE_VERDICT="ok" + return 0 + fi + + echo -n " ." + sleep 2 + counter=$((counter + 1)) + done + + echo " no verdict in 120s" + LICENSE_VERDICT="unknown" + LICENSE_LOG_LINES=$(grep -iE "failed to validate license|error validating license" <<< "$logs" | tail -n 3 || true) + return 0 +} + apply_changes() { # From here on a failure must roll the deployment back. ROLLBACK_STATE="armed" @@ -1100,9 +1143,57 @@ apply_changes() { echo "Bringing up all services ..." $DOCKER_COMPOSE_COMMAND up -d + echo "" + wait_for_license_verdict + echo "" echo "Migration complete." + if [[ "$LICENSE_VERDICT" == "rejected" ]]; then + local unreachable="false" + if grep -qi "couldn't be validated with the license server" <<< "$LICENSE_LOG_LINES"; then + unreachable="true" + fi + + echo "" + if [[ "$unreachable" == "true" ]]; then + echo " ⚠ The server could not validate the license:" + else + echo " ⚠ The server rejected the license key:" + fi + while IFS= read -r line; do + [[ -n "$line" ]] && echo " $line" + done <<< "$LICENSE_LOG_LINES" + echo "" + echo " The migration itself completed: the images and any migrated data" + echo " are in place, and only the license check did not pass." + echo "" + if [[ "$unreachable" == "true" ]]; then + echo " The license server could not be reached, so the key itself was" + echo " never checked. Confirm this host has outbound access to the" + echo " license server, then restart:" + else + echo " Check the reason the server gave above, verify that" + echo " NB_LICENSE_KEY in .env matches the key you were issued, then" + echo " restart:" + fi + echo "" + echo " $DOCKER_COMPOSE_COMMAND up -d" + elif [[ "$LICENSE_VERDICT" == "unknown" ]]; then + echo "" + echo " ⚠ The server logged no license verdict within 120s." + if [[ -n "$LICENSE_LOG_LINES" ]]; then + echo " It was still reporting validation errors:" + while IFS= read -r line; do + [[ -n "$line" ]] && echo " $line" + done <<< "$LICENSE_LOG_LINES" + fi + echo "" + echo " Check the verdict with:" + echo "" + echo " $DOCKER_COMPOSE_COMMAND logs $COMBINED_SERVICE | grep -i license" + fi + # Nothing left to undo. ROLLBACK_STATE="disarmed" } @@ -1122,6 +1213,11 @@ print_summary() { fi [[ "$ENABLE_FLOW" == "yes" ]] && echo " Traffic flow: enabled" [[ "$ENABLE_FLOW" != "yes" ]] && echo " Traffic flow: disabled" + case "$LICENSE_VERDICT" in + ok) echo " License: validated by the server" ;; + rejected) echo " License: REJECTED - see above, the install is not usable yet" ;; + *) echo " License: not confirmed (no verdict in the logs yet)" ;; + esac echo "" echo " Generated files (next to your docker-compose.yml):" echo " $OVERRIDE_FILE" @@ -1176,3 +1272,10 @@ trap 'exit 130' INT TERM init_migration apply_changes print_summary + +# A rejected license leaves a migrated but unusable install. Say so in the exit +# code too, or a wrapper script reads this run as a clean success. +if [[ "$LICENSE_VERDICT" == "rejected" ]]; then + exit 1 +fi +exit 0 diff --git a/management/internals/controllers/network_map/controller/controller.go b/management/internals/controllers/network_map/controller/controller.go index e74b17638..d72ba439d 100644 --- a/management/internals/controllers/network_map/controller/controller.go +++ b/management/internals/controllers/network_map/controller/controller.go @@ -566,39 +566,38 @@ func NetworkMapFromData(ctx context.Context, nmData *networkmap.NetworkMapData, return nm } -// peerPostureChecksFromData mirrors getPeerPostureChecks on the twin store. The -// sync response only encodes process-check file paths, so only ProcessCheck is -// converted back to the server posture type. -func peerPostureChecksFromData(nmData *networkmap.NetworkMapData, peerID string) []*posture.Checks { +// peerPostureChecksFromData mirrors getPeerPostureChecks on the twin store. +func peerPostureChecksFromData(nmData *networkmap.NetworkMapData, peerID string) []*nmdata.PostureChecks { if len(nmData.PostureChecks) == 0 { return nil } - peerPostureChecks := make(map[string]*posture.Checks) + peerPostureChecks := make(map[string]*nmdata.PostureChecks) for _, policy := range nmData.Policies { if policy == nil || !policy.Enabled || len(policy.SourcePostureChecks) == 0 { continue } - if !isPeerInPolicySourceGroupsFromData(nmData, peerID, policy) { + if !isPeerInPolicySourcesFromData(nmData, peerID, policy) { continue } for _, checkID := range policy.SourcePostureChecks { - twin := nmData.PostureChecks[checkID] - if twin == nil { - continue + if twin := nmData.PostureChecks[checkID]; twin != nil { + peerPostureChecks[checkID] = twin } - peerPostureChecks[checkID] = postureChecksFromTwin(twin) } } return maps.Values(peerPostureChecks) } -func isPeerInPolicySourceGroupsFromData(nmData *networkmap.NetworkMapData, peerID string, policy *nmdata.Policy) bool { +func isPeerInPolicySourcesFromData(nmData *networkmap.NetworkMapData, peerID string, policy *nmdata.Policy) bool { for _, rule := range policy.Rules { if rule == nil || !rule.Enabled { continue } + if rule.SourceResource.Type == string(types.ResourceTypePeer) && rule.SourceResource.ID == peerID { + return true + } for _, groupID := range rule.Sources { if group := nmData.Groups[groupID]; group != nil && slices.Contains(group.Peers, peerID) { return true @@ -608,18 +607,6 @@ func isPeerInPolicySourceGroupsFromData(nmData *networkmap.NetworkMapData, peerI return false } -func postureChecksFromTwin(twin *nmdata.PostureChecks) *posture.Checks { - checks := &posture.Checks{ID: twin.ID} - if twin.Checks.ProcessCheck != nil { - processes := make([]posture.Process, 0, len(twin.Checks.ProcessCheck.Processes)) - for _, p := range twin.Checks.ProcessCheck.Processes { - processes = append(processes, posture.Process{LinuxPath: p.LinuxPath, MacPath: p.MacPath, WindowsPath: p.WindowsPath}) - } - checks.Checks.ProcessCheck = &posture.ProcessCheck{Processes: processes} - } - return checks -} - func (c *Controller) perAccountOrGlobalSupportedSyncMessageVersions(accountId string) sharedgrpc.SyncMessageVersion { if perAccount, ok := c.perAccountServerSupportedSyncMessageVersions[accountId]; ok { return perAccount @@ -967,7 +954,7 @@ func (c *Controller) BufferUpdateAccountPeers(ctx context.Context, accountID str // data the legacy server folds in via NetworkMap.Merge). The gRPC layer // encodes both into the wire envelope. Callers must gate on capability // themselves before dispatching here — this method does NOT branch on it. -func (c *Controller) GetValidatedPeerWithComponents(ctx context.Context, isRequiresApproval bool, accountID string, peer *nbpeer.Peer) (*nbpeer.Peer, *types.NetworkMapComponents, *types.NetworkMap, []*posture.Checks, int64, error) { +func (c *Controller) GetValidatedPeerWithComponents(ctx context.Context, isRequiresApproval bool, accountID string, peer *nbpeer.Peer) (*nbpeer.Peer, *types.NetworkMapComponents, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { if isRequiresApproval { network, err := c.repo.GetAccountNetwork(ctx, accountID) if err != nil { @@ -1032,7 +1019,7 @@ func (c *Controller) GetValidatedPeerWithComponents(ctx context.Context, isRequi // getValidatedPeerWithComponentsFromData is the account-free variant of // GetValidatedPeerWithComponents. The proxy network map fragment is omitted // like on the other nmdata paths. -func (c *Controller) getValidatedPeerWithComponentsFromData(ctx context.Context, accountID string, peer *nbpeer.Peer, nmData *networkmap.NetworkMapData) (*nbpeer.Peer, *types.NetworkMapComponents, *types.NetworkMap, []*posture.Checks, int64, error) { +func (c *Controller) getValidatedPeerWithComponentsFromData(ctx context.Context, accountID string, peer *nbpeer.Peer, nmData *networkmap.NetworkMapData) (*nbpeer.Peer, *types.NetworkMapComponents, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { postureChecks := peerPostureChecksFromData(nmData, peer.ID) dnsDomain := c.getDNSDomainFromData(nmData.AccountSettings) @@ -1142,7 +1129,7 @@ func (b *bufferAffectedUpdate) setTimer(d time.Duration, f func()) { b.next.Reset(d) } -func (c *Controller) GetValidatedPeerWithMap(ctx context.Context, isRequiresApproval bool, accountID string, peerID string) (*types.NetworkMap, []*posture.Checks, int64, error) { +func (c *Controller) GetValidatedPeerWithMap(ctx context.Context, isRequiresApproval bool, accountID string, peerID string) (*types.NetworkMap, []*nmdata.PostureChecks, int64, error) { if isRequiresApproval { network, err := c.repo.GetAccountNetwork(ctx, accountID) if err != nil { @@ -1209,7 +1196,7 @@ func (c *Controller) GetValidatedPeerWithMap(ctx context.Context, isRequiresAppr // getValidatedPeerWithMapFromData is the account-free variant of // GetValidatedPeerWithMap. The proxy network map fragment is omitted like on // the other nmdata paths. -func (c *Controller) getValidatedPeerWithMapFromData(ctx context.Context, accountID string, peerID string, nmData *networkmap.NetworkMapData) (*types.NetworkMap, []*posture.Checks, int64, error) { +func (c *Controller) getValidatedPeerWithMapFromData(ctx context.Context, accountID string, peerID string, nmData *networkmap.NetworkMapData) (*types.NetworkMap, []*nmdata.PostureChecks, int64, error) { postureChecks := peerPostureChecksFromData(nmData, peerID) dnsDomain := c.getDNSDomainFromData(nmData.AccountSettings) @@ -1234,7 +1221,7 @@ func (c *Controller) GetDNSDomain(settings *types.Settings) string { } // getPeerPostureChecks returns the posture checks applied for a given peer. -func (c *Controller) getPeerPostureChecks(account *types.Account, peerID string) ([]*posture.Checks, error) { +func (c *Controller) getPeerPostureChecks(account *types.Account, peerID string) ([]*nmdata.PostureChecks, error) { peerPostureChecks := make(map[string]*posture.Checks) if len(account.PostureChecks) == 0 { @@ -1251,7 +1238,7 @@ func (c *Controller) getPeerPostureChecks(account *types.Account, peerID string) } } - return maps.Values(peerPostureChecks), nil + return types.TwinPostureChecksList(maps.Values(peerPostureChecks)), nil } func (c *Controller) StartWarmup(ctx context.Context) { @@ -1330,7 +1317,7 @@ func computeForwarderPortFromVersions(wtVersions []string, requiredVersion strin // addPolicyPostureChecks adds posture checks from a policy to the peer posture checks map if the peer is in the policy's source groups. func addPolicyPostureChecks(account *types.Account, peerID string, policy *types.Policy, peerPostureChecks map[string]*posture.Checks) error { - isInGroup, err := isPeerInPolicySourceGroups(account, peerID, policy) + isInGroup, err := isPeerInPolicySources(account, peerID, policy) if err != nil { return err } @@ -1350,13 +1337,17 @@ func addPolicyPostureChecks(account *types.Account, peerID string, policy *types return nil } -// isPeerInPolicySourceGroups checks if a peer is present in any of the policy rule source groups. -func isPeerInPolicySourceGroups(account *types.Account, peerID string, policy *types.Policy) (bool, error) { +// isPeerInPolicySources checks if a peer is a source of the policy, directly or through a source group. +func isPeerInPolicySources(account *types.Account, peerID string, policy *types.Policy) (bool, error) { for _, rule := range policy.Rules { if !rule.Enabled { continue } + if rule.SourceResource.Type == types.ResourceTypePeer && rule.SourceResource.ID == peerID { + return true, nil + } + for _, sourceGroup := range rule.Sources { group := account.GetGroup(sourceGroup) if group == nil { diff --git a/management/internals/controllers/network_map/controller/posture_twin_test.go b/management/internals/controllers/network_map/controller/posture_twin_test.go new file mode 100644 index 000000000..98e0991d0 --- /dev/null +++ b/management/internals/controllers/network_map/controller/posture_twin_test.go @@ -0,0 +1,68 @@ +package controller + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/shared/management/networkmap" + "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" + "github.com/netbirdio/netbird/shared/management/types" +) + +func postureSelectionData(policies ...*nmdata.Policy) *networkmap.NetworkMapData { + return &networkmap.NetworkMapData{ + Groups: map[string]*nmdata.Group{"g-src": {ID: "g-src", Peers: []string{"peer-group"}}}, + Policies: policies, + PostureChecks: map[string]*nmdata.PostureChecks{ + "pc1": {ID: "pc1", Checks: nmdata.ChecksDefinition{NBVersionCheck: &nmdata.NBVersionCheck{MinVersion: "0.30.0"}}}, + }, + } +} + +func gatedPolicy(id string, rule *nmdata.PolicyRule, checkIDs ...string) *nmdata.Policy { + return &nmdata.Policy{ID: id, Enabled: true, SourcePostureChecks: checkIDs, Rules: []*nmdata.PolicyRule{rule}} +} + +func checkIDs(checks []*nmdata.PostureChecks) []string { + ids := make([]string, 0, len(checks)) + for _, c := range checks { + ids = append(ids, c.ID) + } + return ids +} + +func TestPeerPostureChecksFromData_SelectsPolicySourcePeers(t *testing.T) { + groupRule := &nmdata.PolicyRule{ID: "r-group", Enabled: true, Sources: []string{"g-src"}, Destinations: []string{"g-dst"}} + directRule := &nmdata.PolicyRule{ID: "r-direct", Enabled: true, SourceResource: nmdata.Resource{ID: "peer-direct", Type: string(types.ResourceTypePeer)}, Destinations: []string{"g-dst"}} + + t.Run("source group member and direct source peer both get the checks", func(t *testing.T) { + nmData := postureSelectionData(gatedPolicy("p1", groupRule, "pc1"), gatedPolicy("p2", directRule, "pc1")) + + assert.Equal(t, []string{"pc1"}, checkIDs(peerPostureChecksFromData(nmData, "peer-group"))) + assert.Equal(t, []string{"pc1"}, checkIDs(peerPostureChecksFromData(nmData, "peer-direct"))) + assert.Empty(t, peerPostureChecksFromData(nmData, "peer-elsewhere")) + }) + + t.Run("source resource of a non-peer type never matches a peer", func(t *testing.T) { + hostRule := &nmdata.PolicyRule{ID: "r-host", Enabled: true, SourceResource: nmdata.Resource{ID: "peer-direct", Type: string(types.ResourceTypeHost)}, Destinations: []string{"g-dst"}} + nmData := postureSelectionData(gatedPolicy("p1", hostRule, "pc1")) + + assert.Empty(t, peerPostureChecksFromData(nmData, "peer-direct")) + }) + + t.Run("same check through two policies is returned once", func(t *testing.T) { + nmData := postureSelectionData(gatedPolicy("p1", groupRule, "pc1"), gatedPolicy("p2", groupRule, "pc1")) + + assert.Equal(t, []string{"pc1"}, checkIDs(peerPostureChecksFromData(nmData, "peer-group"))) + }) + + t.Run("disabled policy, disabled rule and dangling check are ignored", func(t *testing.T) { + disabledPolicy := gatedPolicy("p-off", groupRule, "pc1") + disabledPolicy.Enabled = false + disabledRule := &nmdata.PolicyRule{ID: "r-off", Enabled: false, Sources: []string{"g-src"}} + nmData := postureSelectionData(disabledPolicy, gatedPolicy("p-rule-off", disabledRule, "pc1"), gatedPolicy("p-dangling", groupRule, "pc-missing")) + + assert.Empty(t, peerPostureChecksFromData(nmData, "peer-group")) + }) +} diff --git a/management/internals/controllers/network_map/interface.go b/management/internals/controllers/network_map/interface.go index b535321d1..1e8c219b3 100644 --- a/management/internals/controllers/network_map/interface.go +++ b/management/internals/controllers/network_map/interface.go @@ -7,8 +7,8 @@ import ( nbdns "github.com/netbirdio/netbird/dns" nbpeer "github.com/netbirdio/netbird/management/server/peer" - "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" ) const ( @@ -23,8 +23,8 @@ type Controller interface { BufferUpdateAffectedPeers(ctx context.Context, accountID string, peerIDs []string, reason types.UpdateReason) error UpdateAccountPeer(ctx context.Context, accountId string, peerId string) error BufferUpdateAccountPeers(ctx context.Context, accountID string, reason types.UpdateReason) error - GetValidatedPeerWithMap(ctx context.Context, isRequiresApproval bool, accountID string, peerID string) (*types.NetworkMap, []*posture.Checks, int64, error) - GetValidatedPeerWithComponents(ctx context.Context, isRequiresApproval bool, accountID string, p *nbpeer.Peer) (*nbpeer.Peer, *types.NetworkMapComponents, *types.NetworkMap, []*posture.Checks, int64, error) + GetValidatedPeerWithMap(ctx context.Context, isRequiresApproval bool, accountID string, peerID string) (*types.NetworkMap, []*nmdata.PostureChecks, int64, error) + GetValidatedPeerWithComponents(ctx context.Context, isRequiresApproval bool, accountID string, p *nbpeer.Peer) (*nbpeer.Peer, *types.NetworkMapComponents, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) GetDNSDomain(settings *types.Settings) string StartWarmup(context.Context) GetNetworkMap(ctx context.Context, peerID string) (*types.NetworkMap, error) diff --git a/management/internals/controllers/network_map/interface_mock.go b/management/internals/controllers/network_map/interface_mock.go index 42051f172..8b104dfa0 100644 --- a/management/internals/controllers/network_map/interface_mock.go +++ b/management/internals/controllers/network_map/interface_mock.go @@ -14,8 +14,8 @@ import ( reflect "reflect" peer "github.com/netbirdio/netbird/management/server/peer" - posture "github.com/netbirdio/netbird/management/server/posture" types "github.com/netbirdio/netbird/management/server/types" + nmdata "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" gomock "go.uber.org/mock/gomock" ) @@ -127,13 +127,13 @@ func (mr *MockControllerMockRecorder) GetNetworkMap(ctx, peerID any) *gomock.Cal } // GetValidatedPeerWithComponents mocks base method. -func (m *MockController) GetValidatedPeerWithComponents(ctx context.Context, isRequiresApproval bool, accountID string, p *peer.Peer) (*peer.Peer, *types.NetworkMapComponents, *types.NetworkMap, []*posture.Checks, int64, error) { +func (m *MockController) GetValidatedPeerWithComponents(ctx context.Context, isRequiresApproval bool, accountID string, p *peer.Peer) (*peer.Peer, *types.NetworkMapComponents, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetValidatedPeerWithComponents", ctx, isRequiresApproval, accountID, p) ret0, _ := ret[0].(*peer.Peer) ret1, _ := ret[1].(*types.NetworkMapComponents) ret2, _ := ret[2].(*types.NetworkMap) - ret3, _ := ret[3].([]*posture.Checks) + ret3, _ := ret[3].([]*nmdata.PostureChecks) ret4, _ := ret[4].(int64) ret5, _ := ret[5].(error) return ret0, ret1, ret2, ret3, ret4, ret5 @@ -146,11 +146,11 @@ func (mr *MockControllerMockRecorder) GetValidatedPeerWithComponents(ctx, isRequ } // GetValidatedPeerWithMap mocks base method. -func (m *MockController) GetValidatedPeerWithMap(ctx context.Context, isRequiresApproval bool, accountID, peerID string) (*types.NetworkMap, []*posture.Checks, int64, error) { +func (m *MockController) GetValidatedPeerWithMap(ctx context.Context, isRequiresApproval bool, accountID, peerID string) (*types.NetworkMap, []*nmdata.PostureChecks, int64, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetValidatedPeerWithMap", ctx, isRequiresApproval, accountID, peerID) ret0, _ := ret[0].(*types.NetworkMap) - ret1, _ := ret[1].([]*posture.Checks) + ret1, _ := ret[1].([]*nmdata.PostureChecks) ret2, _ := ret[2].(int64) ret3, _ := ret[3].(error) return ret0, ret1, ret2, ret3 diff --git a/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/case.json b/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/case.json new file mode 100644 index 000000000..cdf31c413 --- /dev/null +++ b/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/case.json @@ -0,0 +1,5 @@ +{ + "description": "A peer named directly as a rule source or destination is subject to approval exactly like a group member: unvalidated peer-b is neither a source for peer-c nor a destination for peer-a, while the validated direct source peer-a reaches peer-c. Legacy mode is excluded: the frozen legacynmap copy still carries the direct-peer bypass.", + "peers": ["peer-a", "peer-c"], + "modes": ["full", "envelope"] +} diff --git a/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/golden/peer-a.json b/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/golden/peer-a.json new file mode 100644 index 000000000..e0605525f --- /dev/null +++ b/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/golden/peer-a.json @@ -0,0 +1,64 @@ +{ + "Serial": "22", + "peerConfig": { + "address": "100.64.0.1/10", + "sshConfig": {}, + "fqdn": "peer-a.netbird.test", + "autoUpdate": {} + }, + "remotePeers": [ + { + "wgPubKey": "4deEImv8zGvsyBmmfC2G0eQkbyMzyGuz/YK7pcYETwM=", + "allowedIps": [ + "100.64.0.3/32" + ], + "sshConfig": {}, + "fqdn": "peer-c.netbird.test", + "agentVersion": "0.60.0" + } + ], + "DNSConfig": { + "ServiceEnable": true, + "CustomZones": [ + { + "Domain": "netbird.test.", + "Records": [ + { + "Name": "peer-a.netbird.test", + "Type": "1", + "Class": "IN", + "TTL": "300", + "RData": "100.64.0.1" + }, + { + "Name": "peer-c.netbird.test", + "Type": "1", + "Class": "IN", + "TTL": "300", + "RData": "100.64.0.3" + } + ] + } + ], + "ForwarderPort": "22054" + }, + "FirewallRules": [ + { + "PeerIP": "100.64.0.3", + "Protocol": "TCP", + "Port": "443", + "PolicyID": "cG9sLWRpcmVjdC1vaw==" + }, + { + "PeerIP": "100.64.0.3", + "Direction": "OUT", + "Protocol": "TCP", + "Port": "443", + "PolicyID": "cG9sLWRpcmVjdC1vaw==" + } + ], + "routesFirewallRulesIsEmpty": true, + "sshAuth": { + "UserIDClaim": "sub" + } +} diff --git a/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/golden/peer-c.json b/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/golden/peer-c.json new file mode 100644 index 000000000..f2b3e9357 --- /dev/null +++ b/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/golden/peer-c.json @@ -0,0 +1,64 @@ +{ + "Serial": "22", + "peerConfig": { + "address": "100.64.0.3/10", + "sshConfig": {}, + "fqdn": "peer-c.netbird.test", + "autoUpdate": {} + }, + "remotePeers": [ + { + "wgPubKey": "vblMc9U8RAI6cVopcKEMTVT6lVC3D9nTTMSwot5d3L4=", + "allowedIps": [ + "100.64.0.1/32" + ], + "sshConfig": {}, + "fqdn": "peer-a.netbird.test", + "agentVersion": "0.60.0" + } + ], + "DNSConfig": { + "ServiceEnable": true, + "CustomZones": [ + { + "Domain": "netbird.test.", + "Records": [ + { + "Name": "peer-a.netbird.test", + "Type": "1", + "Class": "IN", + "TTL": "300", + "RData": "100.64.0.1" + }, + { + "Name": "peer-c.netbird.test", + "Type": "1", + "Class": "IN", + "TTL": "300", + "RData": "100.64.0.3" + } + ] + } + ], + "ForwarderPort": "22054" + }, + "FirewallRules": [ + { + "PeerIP": "100.64.0.1", + "Protocol": "TCP", + "Port": "443", + "PolicyID": "cG9sLWRpcmVjdC1vaw==" + }, + { + "PeerIP": "100.64.0.1", + "Direction": "OUT", + "Protocol": "TCP", + "Port": "443", + "PolicyID": "cG9sLWRpcmVjdC1vaw==" + } + ], + "routesFirewallRulesIsEmpty": true, + "sshAuth": { + "UserIDClaim": "sub" + } +} diff --git a/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/nmdata.json b/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/nmdata.json new file mode 100644 index 000000000..283df304c --- /dev/null +++ b/management/internals/controllers/network_map/nmaptest/testdata/cases/policy-direct-peer-unvalidated/nmdata.json @@ -0,0 +1,63 @@ +{ + "Network": {"Serial": 22}, + "Peers": { + "peer-a": {"IP": "100.64.0.1", "Meta": {"WtVersion": "0.60.0"}}, + "peer-b": {"IP": "100.64.0.2", "Meta": {"WtVersion": "0.60.0"}}, + "peer-c": {"IP": "100.64.0.3", "Meta": {"WtVersion": "0.60.0"}} + }, + "ValidatedPeers": {"peer-a": {}, "peer-c": {}}, + "Groups": { + "grp-dev": {"Peers": ["peer-a"]}, + "grp-ops": {"Peers": ["peer-c"]} + }, + "Policies": [ + { + "ID": "pol-direct-ok", + "PublicID": "pol-direct-ok-pub", + "Enabled": true, + "Rules": [ + { + "Enabled": true, + "Action": "accept", + "Protocol": "tcp", + "Ports": ["443"], + "Bidirectional": true, + "SourceResource": {"ID": "peer-a", "Type": "peer"}, + "Destinations": ["grp-ops"] + } + ] + }, + { + "ID": "pol-src-unval", + "PublicID": "pol-src-unval-pub", + "Enabled": true, + "Rules": [ + { + "Enabled": true, + "Action": "accept", + "Protocol": "tcp", + "Ports": ["8443"], + "Bidirectional": true, + "SourceResource": {"ID": "peer-b", "Type": "peer"}, + "Destinations": ["grp-ops"] + } + ] + }, + { + "ID": "pol-dst-unval", + "PublicID": "pol-dst-unval-pub", + "Enabled": true, + "Rules": [ + { + "Enabled": true, + "Action": "accept", + "Protocol": "tcp", + "Ports": ["9443"], + "Bidirectional": true, + "Sources": ["grp-dev"], + "DestinationResource": {"ID": "peer-b", "Type": "peer"} + } + ] + } + ] +} diff --git a/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/case.json b/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/case.json new file mode 100644 index 000000000..8d7460721 --- /dev/null +++ b/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/case.json @@ -0,0 +1,5 @@ +{ + "description": "A peer named directly as a rule source is gated by the policy's posture checks exactly like a group member: peer-b (0.40.0) fails the 0.45.0 minimum, so it gets no connectivity and peer-c must not see it, while the compliant direct source peer-a reaches peer-c. Legacy mode is excluded: the frozen legacynmap copy still carries the direct-peer bypass.", + "peers": ["peer-b", "peer-c"], + "modes": ["full", "envelope"] +} diff --git a/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/golden/peer-b.json b/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/golden/peer-b.json new file mode 100644 index 000000000..240358e40 --- /dev/null +++ b/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/golden/peer-b.json @@ -0,0 +1,33 @@ +{ + "Serial": "21", + "peerConfig": { + "address": "100.64.0.2/10", + "sshConfig": {}, + "fqdn": "peer-b.netbird.test", + "autoUpdate": {} + }, + "remotePeersIsEmpty": true, + "DNSConfig": { + "ServiceEnable": true, + "CustomZones": [ + { + "Domain": "netbird.test.", + "Records": [ + { + "Name": "peer-b.netbird.test", + "Type": "1", + "Class": "IN", + "TTL": "300", + "RData": "100.64.0.2" + } + ] + } + ], + "ForwarderPort": "5353" + }, + "firewallRulesIsEmpty": true, + "routesFirewallRulesIsEmpty": true, + "sshAuth": { + "UserIDClaim": "sub" + } +} diff --git a/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/golden/peer-c.json b/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/golden/peer-c.json new file mode 100644 index 000000000..85573ed35 --- /dev/null +++ b/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/golden/peer-c.json @@ -0,0 +1,64 @@ +{ + "Serial": "21", + "peerConfig": { + "address": "100.64.0.3/10", + "sshConfig": {}, + "fqdn": "peer-c.netbird.test", + "autoUpdate": {} + }, + "remotePeers": [ + { + "wgPubKey": "vblMc9U8RAI6cVopcKEMTVT6lVC3D9nTTMSwot5d3L4=", + "allowedIps": [ + "100.64.0.1/32" + ], + "sshConfig": {}, + "fqdn": "peer-a.netbird.test", + "agentVersion": "0.60.0" + } + ], + "DNSConfig": { + "ServiceEnable": true, + "CustomZones": [ + { + "Domain": "netbird.test.", + "Records": [ + { + "Name": "peer-a.netbird.test", + "Type": "1", + "Class": "IN", + "TTL": "300", + "RData": "100.64.0.1" + }, + { + "Name": "peer-c.netbird.test", + "Type": "1", + "Class": "IN", + "TTL": "300", + "RData": "100.64.0.3" + } + ] + } + ], + "ForwarderPort": "5353" + }, + "FirewallRules": [ + { + "PeerIP": "100.64.0.1", + "Protocol": "TCP", + "Port": "443", + "PolicyID": "cG9sLWRpcmVjdC1vaw==" + }, + { + "PeerIP": "100.64.0.1", + "Direction": "OUT", + "Protocol": "TCP", + "Port": "443", + "PolicyID": "cG9sLWRpcmVjdC1vaw==" + } + ], + "routesFirewallRulesIsEmpty": true, + "sshAuth": { + "UserIDClaim": "sub" + } +} diff --git a/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/nmdata.json b/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/nmdata.json new file mode 100644 index 000000000..e6b99bfdd --- /dev/null +++ b/management/internals/controllers/network_map/nmaptest/testdata/cases/posture-direct-source/nmdata.json @@ -0,0 +1,51 @@ +{ + "Network": {"Serial": 21}, + "Peers": { + "peer-a": {"IP": "100.64.0.1", "Meta": {"WtVersion": "0.60.0"}}, + "peer-b": {"IP": "100.64.0.2", "Meta": {"WtVersion": "0.40.0"}}, + "peer-c": {"IP": "100.64.0.3", "Meta": {"WtVersion": "0.60.0"}} + }, + "Groups": { + "grp-ops": {"Peers": ["peer-c"]} + }, + "PostureChecks": { + "chk-ver": {"Checks": {"NBVersionCheck": {"MinVersion": "0.45.0"}}} + }, + "PostureCheckXIDToPublicID": {"chk-ver": "chk-ver-pub"}, + "Policies": [ + { + "ID": "pol-direct-ok", + "PublicID": "pol-direct-ok-pub", + "Enabled": true, + "SourcePostureChecks": ["chk-ver"], + "Rules": [ + { + "Enabled": true, + "Action": "accept", + "Protocol": "tcp", + "Ports": ["443"], + "Bidirectional": true, + "SourceResource": {"ID": "peer-a", "Type": "peer"}, + "Destinations": ["grp-ops"] + } + ] + }, + { + "ID": "pol-direct-denied", + "PublicID": "pol-direct-denied-pub", + "Enabled": true, + "SourcePostureChecks": ["chk-ver"], + "Rules": [ + { + "Enabled": true, + "Action": "accept", + "Protocol": "tcp", + "Ports": ["8443"], + "Bidirectional": true, + "SourceResource": {"ID": "peer-b", "Type": "peer"}, + "Destinations": ["grp-ops"] + } + ] + } + ] +} diff --git a/management/internals/shared/grpc/components_envelope_response.go b/management/internals/shared/grpc/components_envelope_response.go index 049049db2..ff1b8787f 100644 --- a/management/internals/shared/grpc/components_envelope_response.go +++ b/management/internals/shared/grpc/components_envelope_response.go @@ -6,7 +6,6 @@ import ( integrationsConfig "github.com/netbirdio/management-integrations/integrations/config" nbconfig "github.com/netbirdio/netbird/management/internals/server/config" - "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/types" sharedgrpc "github.com/netbirdio/netbird/shared/management/grpc" "github.com/netbirdio/netbird/shared/management/networkmap" @@ -37,7 +36,7 @@ func ToComponentSyncResponse( components *types.NetworkMapComponents, proxyPatch *types.NetworkMap, dnsName string, - checks []*posture.Checks, + checks []*nmdata.PostureChecks, settings *nmdata.AccountSettingsInfo, extraSettings *types.ExtraSettings, peerGroups []string, diff --git a/management/internals/shared/grpc/conversion.go b/management/internals/shared/grpc/conversion.go index b5ef61c01..f2da977a2 100644 --- a/management/internals/shared/grpc/conversion.go +++ b/management/internals/shared/grpc/conversion.go @@ -18,7 +18,6 @@ import ( "github.com/netbirdio/netbird/management/internals/controllers/network_map/controller/cache" nbconfig "github.com/netbirdio/netbird/management/internals/server/config" - "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/management/networkmap" "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" @@ -151,7 +150,7 @@ func toPeerConfig(peer *nmdata.Peer, network *nmdata.Network, dnsName string, se return peerConfig } -func ToSyncResponse(ctx context.Context, config *nbconfig.Config, httpConfig *nbconfig.HttpServerConfig, deviceFlowConfig *nbconfig.DeviceAuthorizationFlow, peer *nmdata.Peer, turnCredentials *Token, relayCredentials *Token, networkMap *types.NetworkMap, dnsName string, checks []*posture.Checks, dnsCache *cache.DNSConfigCache, settings *nmdata.AccountSettingsInfo, extraSettings *types.ExtraSettings, peerGroups []string, dnsFwdPort int64) *proto.SyncResponse { +func ToSyncResponse(ctx context.Context, config *nbconfig.Config, httpConfig *nbconfig.HttpServerConfig, deviceFlowConfig *nbconfig.DeviceAuthorizationFlow, peer *nmdata.Peer, turnCredentials *Token, relayCredentials *Token, networkMap *types.NetworkMap, dnsName string, checks []*nmdata.PostureChecks, dnsCache *cache.DNSConfigCache, settings *nmdata.AccountSettingsInfo, extraSettings *types.ExtraSettings, peerGroups []string, dnsFwdPort int64) *proto.SyncResponse { // IPv6 data in AllowedIPs and SourcePrefixes wildcard expansion depends on // whether the target peer supports IPv6. Routes and firewall rules are already // filtered at the source (network map builder). diff --git a/management/internals/shared/grpc/server.go b/management/internals/shared/grpc/server.go index bbd309322..3c2ef387b 100644 --- a/management/internals/shared/grpc/server.go +++ b/management/internals/shared/grpc/server.go @@ -42,10 +42,10 @@ import ( "github.com/netbirdio/netbird/management/server/auth" nbContext "github.com/netbirdio/netbird/management/server/context" nbpeer "github.com/netbirdio/netbird/management/server/peer" - "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/settings" "github.com/netbirdio/netbird/management/server/telemetry" "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" "github.com/netbirdio/netbird/shared/management/proto" internalStatus "github.com/netbirdio/netbird/shared/management/status" ) @@ -903,7 +903,7 @@ func (s *Server) ExtendAuthSession(ctx context.Context, req *proto.EncryptedMess }, nil } -func (s *Server) prepareLoginResponse(ctx context.Context, peer *nbpeer.Peer, network *types.Network, postureChecks []*posture.Checks, enableSSH bool) (*proto.LoginResponse, error) { +func (s *Server) prepareLoginResponse(ctx context.Context, peer *nbpeer.Peer, network *types.Network, postureChecks []*nmdata.PostureChecks, enableSSH bool) (*proto.LoginResponse, error) { var relayToken *Token var err error if s.config.Relay != nil && len(s.config.Relay.Addresses) > 0 { @@ -991,7 +991,7 @@ func (s *Server) IsHealthy(ctx context.Context, req *proto.Empty) (*proto.Empty, } // sendInitialSync sends initial proto.SyncResponse to the peer requesting synchronization -func (s *Server) sendInitialSync(ctx context.Context, peerKey wgtypes.Key, peer *nbpeer.Peer, networkMap *types.NetworkMap, postureChecks []*posture.Checks, srv proto.ManagementService_SyncServer, dnsFwdPort int64) error { +func (s *Server) sendInitialSync(ctx context.Context, peerKey wgtypes.Key, peer *nbpeer.Peer, networkMap *types.NetworkMap, postureChecks []*nmdata.PostureChecks, srv proto.ManagementService_SyncServer, dnsFwdPort int64) error { var err error var turnToken *Token @@ -1302,7 +1302,7 @@ func (s *Server) Logout(ctx context.Context, req *proto.EncryptedMessage) (*prot } // toProtocolChecks converts posture checks to protocol checks. -func toProtocolChecks(ctx context.Context, postureChecks []*posture.Checks) []*proto.Checks { +func toProtocolChecks(ctx context.Context, postureChecks []*nmdata.PostureChecks) []*proto.Checks { protoChecks := make([]*proto.Checks, 0, len(postureChecks)) for _, postureCheck := range postureChecks { check := toProtocolCheck(postureCheck) @@ -1314,8 +1314,8 @@ func toProtocolChecks(ctx context.Context, postureChecks []*posture.Checks) []*p return protoChecks } -// toProtocolCheck converts a posture.Checks to a proto.Checks. -func toProtocolCheck(postureCheck *posture.Checks) *proto.Checks { +// toProtocolCheck converts posture checks to a proto.Checks. +func toProtocolCheck(postureCheck *nmdata.PostureChecks) *proto.Checks { protoCheck := &proto.Checks{} if check := postureCheck.Checks.ProcessCheck; check != nil { diff --git a/management/server/account.go b/management/server/account.go index 700dfa04d..4fe0e5338 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -52,6 +52,7 @@ import ( "github.com/netbirdio/netbird/management/server/util" "github.com/netbirdio/netbird/route" nbdomain "github.com/netbirdio/netbird/shared/management/domain" + "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" "github.com/netbirdio/netbird/shared/management/status" ) @@ -1920,7 +1921,7 @@ func domainIsUpToDate(domain string, domainCategory string, userAuth auth.UserAu // derived from syncTime (the moment the gRPC stream opened). Any // concurrent stream that started earlier loses the optimistic-lock race // in MarkPeerConnected and bails without writing. -func (am *DefaultAccountManager) SyncAndMarkPeer(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) { +func (am *DefaultAccountManager) SyncAndMarkPeer(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { peer, netMap, postureChecks, dnsfwdPort, err := am.SyncPeer(ctx, types.PeerSync{WireGuardPubKey: peerPubKey, Meta: meta, RealIP: realIP}, accountID) if err != nil { return nil, nil, nil, 0, fmt.Errorf("error syncing peer: %w", err) diff --git a/management/server/account/manager.go b/management/server/account/manager.go index f4b0408cf..154c9ab18 100644 --- a/management/server/account/manager.go +++ b/management/server/account/manager.go @@ -23,6 +23,7 @@ import ( "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/route" "github.com/netbirdio/netbird/shared/management/domain" + "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" ) type ExternalCacheManager nbcache.UserDataCache @@ -70,7 +71,7 @@ type Manager interface { UpdatePeerIPv6(ctx context.Context, accountID, userID, peerID string, newIPv6 netip.Addr) error GetNetworkMap(ctx context.Context, peerID string) (*types.NetworkMap, error) GetPeerNetwork(ctx context.Context, peerID string) (*types.Network, error) - AddPeer(ctx context.Context, accountID, setupKey, userID string, p *nbpeer.Peer, temporary bool) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) + AddPeer(ctx context.Context, accountID, setupKey, userID string, p *nbpeer.Peer, temporary bool) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) CreatePAT(ctx context.Context, accountID string, initiatorUserID string, targetUserID string, tokenName string, expiresIn int) (*types.PersonalAccessTokenGenerated, error) DeletePAT(ctx context.Context, accountID string, initiatorUserID string, targetUserID string, tokenID string) error GetPAT(ctx context.Context, accountID string, initiatorUserID string, targetUserID string, tokenID string) (*types.PersonalAccessToken, error) @@ -109,9 +110,9 @@ type Manager interface { GetPeer(ctx context.Context, accountID, peerID, userID string) (*nbpeer.Peer, error) UpdateAccountSettings(ctx context.Context, accountID, userID string, newSettings *types.Settings) (*types.Settings, error) UpdateAccountOnboarding(ctx context.Context, accountID, userID string, newOnboarding *types.AccountOnboarding) (*types.AccountOnboarding, error) - LoginPeer(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) // used by peer gRPC API - ExtendPeerSession(ctx context.Context, peerPubKey, userID string) (time.Time, error) // used by peer gRPC API for ExtendAuthSession - SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) // used by peer gRPC API + LoginPeer(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) // used by peer gRPC API + ExtendPeerSession(ctx context.Context, peerPubKey, userID string) (time.Time, error) // used by peer gRPC API for ExtendAuthSession + SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) // used by peer gRPC API GetExternalCacheManager() ExternalCacheManager GetPostureChecks(ctx context.Context, accountID, postureChecksID, userID string) (*posture.Checks, error) SavePostureChecks(ctx context.Context, accountID, userID string, postureChecks *posture.Checks, create bool) (*posture.Checks, error) @@ -121,7 +122,7 @@ type Manager interface { UpdateIntegratedValidator(ctx context.Context, accountID, userID, validator string, groups []string) error GroupValidation(ctx context.Context, accountId string, groups []string) (bool, error) GetValidatedPeers(ctx context.Context, accountID string) (map[string]struct{}, map[string]string, error) - SyncAndMarkPeer(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) + SyncAndMarkPeer(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) OnPeerDisconnected(ctx context.Context, accountID string, peerPubKey string, streamStartTime time.Time) error SyncPeerMeta(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP) error FindExistingPostureCheck(accountID string, checks *posture.ChecksDefinition) (*posture.Checks, error) diff --git a/management/server/account/manager_mock.go b/management/server/account/manager_mock.go index 9ac10cba0..f31f63d0e 100644 --- a/management/server/account/manager_mock.go +++ b/management/server/account/manager_mock.go @@ -29,6 +29,7 @@ import ( route "github.com/netbirdio/netbird/route" auth "github.com/netbirdio/netbird/shared/auth" domain "github.com/netbirdio/netbird/shared/management/domain" + nmdata "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" gomock "go.uber.org/mock/gomock" ) @@ -86,12 +87,12 @@ func (mr *MockManagerMockRecorder) AccountExists(ctx, accountID any) *gomock.Cal } // AddPeer mocks base method. -func (m *MockManager) AddPeer(ctx context.Context, accountID, setupKey, userID string, p *peer.Peer, temporary bool) (*peer.Peer, *types.Network, []*posture.Checks, bool, error) { +func (m *MockManager) AddPeer(ctx context.Context, accountID, setupKey, userID string, p *peer.Peer, temporary bool) (*peer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddPeer", ctx, accountID, setupKey, userID, p, temporary) ret0, _ := ret[0].(*peer.Peer) ret1, _ := ret[1].(*types.Network) - ret2, _ := ret[2].([]*posture.Checks) + ret2, _ := ret[2].([]*nmdata.PostureChecks) ret3, _ := ret[3].(bool) ret4, _ := ret[4].(error) return ret0, ret1, ret2, ret3, ret4 @@ -1323,12 +1324,12 @@ func (mr *MockManagerMockRecorder) ListUsers(ctx, accountID any) *gomock.Call { } // LoginPeer mocks base method. -func (m *MockManager) LoginPeer(ctx context.Context, login types.PeerLogin) (*peer.Peer, *types.Network, []*posture.Checks, bool, error) { +func (m *MockManager) LoginPeer(ctx context.Context, login types.PeerLogin) (*peer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "LoginPeer", ctx, login) ret0, _ := ret[0].(*peer.Peer) ret1, _ := ret[1].(*types.Network) - ret2, _ := ret[2].([]*posture.Checks) + ret2, _ := ret[2].([]*nmdata.PostureChecks) ret3, _ := ret[3].(bool) ret4, _ := ret[4].(error) return ret0, ret1, ret2, ret3, ret4 @@ -1568,12 +1569,12 @@ func (mr *MockManagerMockRecorder) StoreEvent(ctx, initiatorID, targetID, accoun } // SyncAndMarkPeer mocks base method. -func (m *MockManager) SyncAndMarkPeer(ctx context.Context, accountID, peerPubKey string, meta peer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*peer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) { +func (m *MockManager) SyncAndMarkPeer(ctx context.Context, accountID, peerPubKey string, meta peer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*peer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SyncAndMarkPeer", ctx, accountID, peerPubKey, meta, realIP, syncTime) ret0, _ := ret[0].(*peer.Peer) ret1, _ := ret[1].(*types.NetworkMap) - ret2, _ := ret[2].([]*posture.Checks) + ret2, _ := ret[2].([]*nmdata.PostureChecks) ret3, _ := ret[3].(int64) ret4, _ := ret[4].(error) return ret0, ret1, ret2, ret3, ret4 @@ -1586,12 +1587,12 @@ func (mr *MockManagerMockRecorder) SyncAndMarkPeer(ctx, accountID, peerPubKey, m } // SyncPeer mocks base method. -func (m *MockManager) SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*peer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) { +func (m *MockManager) SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*peer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SyncPeer", ctx, sync, accountID) ret0, _ := ret[0].(*peer.Peer) ret1, _ := ret[1].(*types.NetworkMap) - ret2, _ := ret[2].([]*posture.Checks) + ret2, _ := ret[2].([]*nmdata.PostureChecks) ret3, _ := ret[3].(int64) ret4, _ := ret[4].(error) return ret0, ret1, ret2, ret3, ret4 diff --git a/management/server/account_test.go b/management/server/account_test.go index a5a484c1a..b462cc2a6 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -10,16 +10,17 @@ import ( "os" "reflect" "strconv" + "strings" "sync" "testing" "time" - "go.uber.org/mock/gomock" "github.com/prometheus/client_golang/prometheus/push" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/metric/noop" + "go.uber.org/mock/gomock" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain" @@ -37,6 +38,8 @@ import ( "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" reverseproxymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service/manager" "github.com/netbirdio/netbird/management/internals/modules/zones" + networkmapdb "github.com/netbirdio/netbird/management/internals/network_map_db" + networkmapdbfactory "github.com/netbirdio/netbird/management/internals/network_map_db/factory" "github.com/netbirdio/netbird/management/internals/server/config" nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc" nbAccount "github.com/netbirdio/netbird/management/server/account" @@ -3293,13 +3296,33 @@ func createManager(t testing.TB) (*DefaultAccountManager, *update_channel.PeersU if err != nil { return nil, nil, err } - eventStore := &activity.InMemoryEventStore{} + return buildTestManager(t, store, nil) +} - metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) - if err != nil { - return nil, nil, err +// createManagerWithNetworkMapStore builds a manager whose network map controller +// reads the twin (nmdata) store, the production path on sqlite and postgres. +func createManagerWithNetworkMapStore(t testing.TB) (*DefaultAccountManager, *update_channel.PeersUpdateManager) { + t.Helper() + + if engine := os.Getenv("NETBIRD_STORE_ENGINE"); engine != "" && !strings.EqualFold(engine, string(types.SqliteStoreEngine)) { + t.Skipf("network map store test needs the sqlite engine, got %s", engine) } + dataDir := t.TempDir() + store, err := createStoreAt(t, dataDir) + require.NoError(t, err) + + nmdataStore, err := networkmapdbfactory.NewNetworkMapDBStore(context.Background(), types.SqliteStoreEngine, dataDir, MockIntegratedValidator{}, newSettingsMockManager(t)) + require.NoError(t, err) + + manager, updateManager, err := buildTestManager(t, store, nmdataStore) + require.NoError(t, err) + return manager, updateManager +} + +func newSettingsMockManager(t testing.TB) *settings.MockManager { + t.Helper() + ctrl := gomock.NewController(t) t.Cleanup(ctrl.Finish) @@ -3312,6 +3335,23 @@ func createManager(t testing.TB) (*DefaultAccountManager, *update_channel.PeersU UpdateExtraSettings(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(false, nil). AnyTimes() + return settingsMockManager +} + +func buildTestManager(t testing.TB, store store.Store, nmdataStore *networkmapdb.NetworkMapDBStoreImpl) (*DefaultAccountManager, *update_channel.PeersUpdateManager, error) { + t.Helper() + + eventStore := &activity.InMemoryEventStore{} + + metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) + if err != nil { + return nil, nil, err + } + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + settingsMockManager := newSettingsMockManager(t) permissionsManager := permissions.NewManager(store) peersManager := peers.NewManager(store, permissionsManager) @@ -3331,7 +3371,7 @@ func createManager(t testing.TB) (*DefaultAccountManager, *update_channel.PeersU updateManager := update_channel.NewPeersUpdateManager(metrics) requestBuffer := NewAccountRequestBuffer(ctx, store) - networkMapController := controller.NewController(ctx, store, metrics, updateManager, requestBuffer, MockIntegratedValidator{}, settingsMockManager, "netbird.cloud", port_forwarding.NewControllerMock(), ephemeral_manager.NewEphemeralManager(store, peers.NewManager(store, permissionsManager)), &config.Config{}, nil) + networkMapController := controller.NewController(ctx, store, metrics, updateManager, requestBuffer, MockIntegratedValidator{}, settingsMockManager, "netbird.cloud", port_forwarding.NewControllerMock(), ephemeral_manager.NewEphemeralManager(store, peers.NewManager(store, permissionsManager)), &config.Config{}, nmdataStore) manager, err := BuildManager(ctx, &config.Config{}, store, networkMapController, job.NewJobManager(nil, store, peersManager), nil, "", eventStore, nil, false, MockIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManager, false, cacheStore) if err != nil { return nil, nil, err @@ -3349,7 +3389,11 @@ func createManager(t testing.TB) (*DefaultAccountManager, *update_channel.PeersU func createStore(t testing.TB) (store.Store, error) { t.Helper() - dataDir := t.TempDir() + return createStoreAt(t, t.TempDir()) +} + +func createStoreAt(t testing.TB, dataDir string) (store.Store, error) { + t.Helper() store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "", dataDir) if err != nil { return nil, err diff --git a/management/server/affected_peers_router_paths_test.go b/management/server/affected_peers_router_paths_test.go index 5d83367fd..d5868a5c1 100644 --- a/management/server/affected_peers_router_paths_test.go +++ b/management/server/affected_peers_router_paths_test.go @@ -12,6 +12,7 @@ import ( resourceTypes "github.com/netbirdio/netbird/management/server/networks/resources/types" routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types" "github.com/netbirdio/netbird/management/server/posture" + "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" ) @@ -145,7 +146,7 @@ func TestAffectedPeers_GroupAddResource_RefreshesRoutingPeer(t *testing.T) { assert.NotContains(t, affected, s.unrelatedPeerID, "unrelated peer must not be affected") } -func (s *routerScenario) createPostureCheckGatedPolicy(t *testing.T, ctx context.Context) string { +func (s *routerScenario) createPostureCheckGatedPolicy(t *testing.T, ctx context.Context, policy *types.Policy) string { t.Helper() check, err := s.manager.SavePostureChecks(ctx, s.accountID, userID, &posture.Checks{ @@ -156,7 +157,6 @@ func (s *routerScenario) createPostureCheckGatedPolicy(t *testing.T, ctx context }, true) require.NoError(t, err) - policy := peerToResourcePolicyByGroup(s.sourceGroupID, s.resourceGroupID) policy.SourcePostureChecks = []string{check.ID} _, err = s.manager.SavePolicy(ctx, s.accountID, userID, policy, true) require.NoError(t, err) @@ -168,7 +168,7 @@ func TestAffectedPeers_E2E_SavePostureCheck_RefreshesRoutingPeer(t *testing.T) { s := setupRouterScenario(t, true) ctx := context.Background() - checkID := s.createPostureCheckGatedPolicy(t, ctx) + checkID := s.createPostureCheckGatedPolicy(t, ctx, peerToResourcePolicyByGroup(s.sourceGroupID, s.resourceGroupID)) srcCh := s.updateManager.CreateChannel(ctx, s.sourcePeerID) routerCh := s.updateManager.CreateChannel(ctx, s.routerPeerID) @@ -338,3 +338,61 @@ func TestAffectedPeers_PeerChange_RouterInOtherNetworkNotAffected(t *testing.T) assert.NotContains(t, affected, second.routerPeerID, "a router in an unrelated network must not be affected by a source-peer change for another resource") } + +// TestAffectedPeers_E2E_PostureFlip_RefreshesRoutingPeer drives the customer path +// on the twin store: the source peer's metadata flips a posture verdict on sync, +// and the routing peer serving the gated resource must be refreshed in both +// directions. Without the flip detection the deny direction takes the nmap +// shortcut (the denied peer's map holds no router) and the allow direction +// depends on which meta field moved, leaving the routers with a stale map. +func TestAffectedPeers_E2E_PostureFlip_RefreshesRoutingPeer(t *testing.T) { + runPostureFlipRefreshesRoutingPeer(t, func(s *routerScenario) *types.Policy { + return peerToResourcePolicyByGroup(s.sourceGroupID, s.resourceGroupID) + }) +} + +// TestAffectedPeers_E2E_PostureFlip_DirectSourcePeer_RefreshesRoutingPeer is the same +// scenario with the source peer named directly in the rule: it must receive its posture +// checks and have its flips detected exactly like a group member. +func TestAffectedPeers_E2E_PostureFlip_DirectSourcePeer_RefreshesRoutingPeer(t *testing.T) { + runPostureFlipRefreshesRoutingPeer(t, func(s *routerScenario) *types.Policy { + return peerToResourcePolicyByPeer(s.sourcePeerID, s.resourceGroupID) + }) +} + +func runPostureFlipRefreshesRoutingPeer(t *testing.T, policyFor func(s *routerScenario) *types.Policy) { + t.Helper() + + manager, updateManager := createManagerWithNetworkMapStore(t) + s := buildRouterScenario(t, manager, updateManager, true) + ctx := context.Background() + + s.createPostureCheckGatedPolicy(t, ctx, policyFor(s)) + + source, err := s.manager.Store.GetPeerByID(ctx, store.LockingStrengthNone, s.accountID, s.sourcePeerID) + require.NoError(t, err) + + syncWithVersion := func(version string) { + meta := source.Meta + meta.WtVersion = version + _, _, _, _, err := s.manager.SyncPeer(ctx, types.PeerSync{WireGuardPubKey: source.Key, Meta: meta}, s.accountID) + require.NoError(t, err) + } + syncWithVersion("0.31.0") + + routerCh := s.updateManager.CreateChannel(ctx, s.routerPeerID) + unrelatedCh := s.updateManager.CreateChannel(ctx, s.unrelatedPeerID) + t.Cleanup(func() { + s.updateManager.CloseChannel(ctx, s.routerPeerID) + s.updateManager.CloseChannel(ctx, s.unrelatedPeerID) + }) + settleAffectedUpdates(routerCh, unrelatedCh) + + syncWithVersion("0.29.0") + peerShouldReceiveUpdate(t, routerCh) + peerShouldNotReceiveUpdate(t, unrelatedCh) + + syncWithVersion("0.31.0") + peerShouldReceiveUpdate(t, routerCh) + peerShouldNotReceiveUpdate(t, unrelatedCh) +} diff --git a/management/server/affected_peers_router_test.go b/management/server/affected_peers_router_test.go index cc9df0a6a..7e3f02b27 100644 --- a/management/server/affected_peers_router_test.go +++ b/management/server/affected_peers_router_test.go @@ -60,6 +60,12 @@ func setupRouterScenario(t *testing.T, directRouterPeer bool) *routerScenario { manager, updateManager, err := createManager(t) require.NoError(t, err) + return buildRouterScenario(t, manager, updateManager, directRouterPeer) +} + +func buildRouterScenario(t *testing.T, manager *DefaultAccountManager, updateManager *update_channel.PeersUpdateManager, directRouterPeer bool) *routerScenario { + t.Helper() + ctx := context.Background() account, err := createAccount(manager, "router_scenario", userID, "") @@ -167,6 +173,23 @@ func peerToResourcePolicyByGroup(sourceGroupID, resourceGroupID string) *types.P } } +// peerToResourcePolicyByPeer builds a policy naming the source peer directly via +// SourceResource rather than through a group. +func peerToResourcePolicyByPeer(sourcePeerID, resourceGroupID string) *types.Policy { + return &types.Policy{ + Enabled: true, + Name: "peer-to-resource-by-peer", + Rules: []*types.PolicyRule{ + { + Enabled: true, + SourceResource: types.Resource{ID: sourcePeerID, Type: types.ResourceTypePeer}, + Destinations: []string{resourceGroupID}, + Action: types.PolicyTrafficActionAccept, + }, + }, + } +} + // peerToResourcePolicyByResource builds a policy referencing the resource // directly via DestinationResource rather than its group. func peerToResourcePolicyByResource(sourceGroupID, resourceID string) *types.Policy { diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 071e3771b..2f871c3e2 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -24,6 +24,7 @@ import ( "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/route" "github.com/netbirdio/netbird/shared/management/domain" + "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" ) var _ account.Manager = (*MockAccountManager)(nil) @@ -41,11 +42,11 @@ type MockAccountManager struct { GetPeersFunc func(ctx context.Context, accountID, userID, nameFilter, ipFilter string) ([]*nbpeer.Peer, error) MarkPeerConnectedFunc func(ctx context.Context, peerKey string, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error MarkPeerDisconnectedFunc func(ctx context.Context, peerKey string, accountID string, sessionStartedAt int64) error - SyncAndMarkPeerFunc func(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) + SyncAndMarkPeerFunc func(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) DeletePeerFunc func(ctx context.Context, accountID, peerKey, userID string) error GetNetworkMapFunc func(ctx context.Context, peerKey string) (*types.NetworkMap, error) GetPeerNetworkFunc func(ctx context.Context, peerKey string) (*types.Network, error) - AddPeerFunc func(ctx context.Context, accountID string, setupKey string, userId string, peer *nbpeer.Peer, temporary bool) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) + AddPeerFunc func(ctx context.Context, accountID string, setupKey string, userId string, peer *nbpeer.Peer, temporary bool) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) GetGroupFunc func(ctx context.Context, accountID, groupID, userID string) (*types.Group, error) GetAllGroupsFunc func(ctx context.Context, accountID, userID string) ([]*types.Group, error) GetGroupByNameFunc func(ctx context.Context, groupName, accountID, userID string) (*types.Group, error) @@ -98,9 +99,9 @@ type MockAccountManager struct { SaveDNSSettingsFunc func(ctx context.Context, accountID, userID string, dnsSettingsToSave *types.DNSSettings) error GetPeerFunc func(ctx context.Context, accountID, peerID, userID string) (*nbpeer.Peer, error) UpdateAccountSettingsFunc func(ctx context.Context, accountID, userID string, newSettings *types.Settings) (*types.Settings, error) - LoginPeerFunc func(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) + LoginPeerFunc func(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) ExtendPeerSessionFunc func(ctx context.Context, peerPubKey, userID string) (time.Time, error) - SyncPeerFunc func(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) + SyncPeerFunc func(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) InviteUserFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserEmail string) error ApproveUserFunc func(ctx context.Context, accountID, initiatorUserID, targetUserID string) (*types.UserInfo, error) RejectUserFunc func(ctx context.Context, accountID, initiatorUserID, targetUserID string) error @@ -230,7 +231,7 @@ func (am *MockAccountManager) DeleteSetupKey(ctx context.Context, accountID, use return status.Errorf(codes.Unimplemented, "method DeleteSetupKey is not implemented") } -func (am *MockAccountManager) SyncAndMarkPeer(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) { +func (am *MockAccountManager) SyncAndMarkPeer(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { if am.SyncAndMarkPeerFunc != nil { return am.SyncAndMarkPeerFunc(ctx, accountID, peerPubKey, meta, realIP, syncTime) } @@ -424,7 +425,7 @@ func (am *MockAccountManager) AddPeer( userId string, peer *nbpeer.Peer, temporary bool, -) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) { +) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) { if am.AddPeerFunc != nil { return am.AddPeerFunc(ctx, accountID, setupKey, userId, peer, temporary) } @@ -862,7 +863,7 @@ func (am *MockAccountManager) UpdateAccountSettings(ctx context.Context, account } // LoginPeer mocks LoginPeer of the AccountManager interface -func (am *MockAccountManager) LoginPeer(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) { +func (am *MockAccountManager) LoginPeer(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) { if am.LoginPeerFunc != nil { return am.LoginPeerFunc(ctx, login) } @@ -878,7 +879,7 @@ func (am *MockAccountManager) ExtendPeerSession(ctx context.Context, peerPubKey, } // SyncPeer mocks SyncPeer of the AccountManager interface -func (am *MockAccountManager) SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) { +func (am *MockAccountManager) SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { if am.SyncPeerFunc != nil { return am.SyncPeerFunc(ctx, sync, accountID) } diff --git a/management/server/peer.go b/management/server/peer.go index 579ff2708..07619f51e 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -23,7 +23,6 @@ import ( "github.com/netbirdio/netbird/shared/management/domain" "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" - "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" @@ -741,7 +740,7 @@ func (am *DefaultAccountManager) handleSetupKeyAddedPeer(ctx context.Context, en // to it. We also add the User ID to the peer metadata to identify registrant. If no userID provided, then fail with status.PermissionDenied // Each new Peer will be assigned a new next net.IP from the Account.Network and Account.Network.LastIP will be updated (IP's are not reused). // The peer property is just a placeholder for the Peer properties to pass further -func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKey, userID string, peer *nbpeer.Peer, temporary bool) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) { +func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKey, userID string, peer *nbpeer.Peer, temporary bool) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) { if setupKey == "" && userID == "" && !peer.ProxyMeta.Embedded { // no auth method provided => reject access return nil, nil, nil, false, status.ErrNoAuthMethodProvided @@ -1001,7 +1000,7 @@ func getPeerIPDNSLabel(ip netip.Addr, peerHostName string) (string, error) { } // SyncPeer checks whether peer is eligible for receiving NetworkMap (authenticated) and returns its NetworkMap if eligible -func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) { +func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*nmdata.PostureChecks, int64, error) { var peer *nbpeer.Peer var ipv6CapabilityChanged bool var metaDiff nbpeer.MetaDiff @@ -1065,7 +1064,7 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSy return nil, nil, nil, 0, err } - metaDiffAffectsPosture := posture.AffectsPosture(ctx, &metaDiff, resPostureChecks) + metaDiffAffectsPosture := metaDiffAffectsPosture(&metaDiff, resPostureChecks) if requiresPeerUpdate(ctx, isStatusChanged, sync.UpdateAccountPeers, ipv6CapabilityChanged, metaDiffAffectsPosture, metaDiff.VersionChanged(), metaDiff.HostnameChanged()) { changedPeerIDs := []string{peer.ID} affectedPeerIDs := am.syncPeerAffectedPeers(ctx, accountID, peer.ID, nmap, peerNotValid, metaDiffAffectsPosture) @@ -1077,6 +1076,14 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSy return peer, nmap, resPostureChecks, dnsFwdPort, nil } +// metaDiffAffectsPosture reports whether the meta change flips the verdict of any of +// the peer's posture checks, replaying them against the old and new state. +func metaDiffAffectsPosture(diff *nbpeer.MetaDiff, checks []*nmdata.PostureChecks) bool { + oldPeer := types.TwinPeer(&nbpeer.Peer{Meta: diff.OldMeta, Location: diff.OldLocation}) + newPeer := types.TwinPeer(&nbpeer.Peer{Meta: diff.NewMeta, Location: diff.NewLocation}) + return nmdata.PostureVerdictChanged(checks, oldPeer, newPeer) +} + func requiresPeerUpdate(ctx context.Context, isStatusChanged, updateAccountPeers, ipv6CapabilityChanged, metaDiffAffectsPosture, versionChanged, hostname bool) bool { var reason string switch { @@ -1128,7 +1135,7 @@ func (am *DefaultAccountManager) markConnectedAffectedPeers(ctx context.Context, return affectedPeerIDsFromNetworkMap(nmap, peerID) } -func (am *DefaultAccountManager) handlePeerLoginNotFound(ctx context.Context, login types.PeerLogin, err error) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) { +func (am *DefaultAccountManager) handlePeerLoginNotFound(ctx context.Context, login types.PeerLogin, err error) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) { if errStatus, ok := status.FromError(err); ok && errStatus.Type() == status.NotFound { // we couldn't find this peer by its public key which can mean that peer hasn't been registered yet. // Try registering it. @@ -1149,7 +1156,7 @@ func (am *DefaultAccountManager) handlePeerLoginNotFound(ctx context.Context, lo // LoginPeer logs in or registers a peer. // If peer doesn't exist the function checks whether a setup key or a user is present and registers a new peer if so. -func (am *DefaultAccountManager) LoginPeer(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.Network, []*posture.Checks, bool, error) { +func (am *DefaultAccountManager) LoginPeer(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.Network, []*nmdata.PostureChecks, bool, error) { accountID, err := am.Store.GetAccountIDByPeerPubKey(ctx, login.WireGuardPubKey) if err != nil { return am.handlePeerLoginNotFound(ctx, login, err) @@ -1322,7 +1329,7 @@ func (am *DefaultAccountManager) ExtendPeerSession(ctx context.Context, peerPubK // getPeerLoginInfo computes the login/register response data (network, posture // checks, SSH) from the store without building the peer's full network map. -func getPeerLoginInfo(ctx context.Context, transaction store.Store, accountID string, peer *nbpeer.Peer, isValid bool) (*types.Network, []*posture.Checks, bool, error) { +func getPeerLoginInfo(ctx context.Context, transaction store.Store, accountID string, peer *nbpeer.Peer, isValid bool) (*types.Network, []*nmdata.PostureChecks, bool, error) { network, err := transaction.GetAccountNetwork(ctx, store.LockingStrengthNone, accountID) if err != nil { return nil, nil, false, fmt.Errorf("get account network: %w", err) @@ -1342,7 +1349,7 @@ func getPeerLoginInfo(ctx context.Context, transaction store.Store, accountID st return nil, nil, false, err } - postureChecks, err := getPeerPostureChecks(ctx, transaction, accountID, peerGroupIDs, policies) + postureChecks, err := getPeerPostureChecks(ctx, transaction, accountID, peer.ID, peerGroupIDs, policies) if err != nil { return nil, nil, false, err } @@ -1364,7 +1371,7 @@ func isPeerSSHEnabled(ctx context.Context, peer *nbpeer.Peer, policies []*types. } // getPeerPostureChecks returns the posture checks for the peer. -func getPeerPostureChecks(ctx context.Context, transaction store.Store, accountID string, peerGroupIDs []string, policies []*types.Policy) ([]*posture.Checks, error) { +func getPeerPostureChecks(ctx context.Context, transaction store.Store, accountID, peerID string, peerGroupIDs []string, policies []*types.Policy) ([]*nmdata.PostureChecks, error) { if len(policies) == 0 { return nil, nil } @@ -1376,7 +1383,7 @@ func getPeerPostureChecks(ctx context.Context, transaction store.Store, accountI continue } - postureChecksIDs := processPeerPostureChecks(policy, peerGroupIDs) + postureChecksIDs := processPeerPostureChecks(policy, peerID, peerGroupIDs) peerPostureChecksIDs = append(peerPostureChecksIDs, postureChecksIDs...) } @@ -1385,16 +1392,20 @@ func getPeerPostureChecks(ctx context.Context, transaction store.Store, accountI return nil, err } - return maps.Values(peerPostureChecks), nil + return types.TwinPostureChecksList(maps.Values(peerPostureChecks)), nil } -// processPeerPostureChecks checks if the peer is in the source group of the policy and returns the posture checks. -func processPeerPostureChecks(policy *types.Policy, peerGroupIDs []string) []string { +// processPeerPostureChecks returns the policy's posture checks when the peer is a source of the policy, directly or through a source group. +func processPeerPostureChecks(policy *types.Policy, peerID string, peerGroupIDs []string) []string { for _, rule := range policy.Rules { if !rule.Enabled { continue } + if rule.SourceResource.Type == types.ResourceTypePeer && rule.SourceResource.ID == peerID { + return policy.SourcePostureChecks + } + for _, sourceGroup := range rule.Sources { if slices.Contains(peerGroupIDs, sourceGroup) { return policy.SourcePostureChecks diff --git a/management/server/peer_posture_test.go b/management/server/peer_posture_test.go new file mode 100644 index 000000000..88662e2fa --- /dev/null +++ b/management/server/peer_posture_test.go @@ -0,0 +1,203 @@ +package server + +import ( + "net" + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + + nbpeer "github.com/netbirdio/netbird/management/server/peer" + "github.com/netbirdio/netbird/management/server/posture" + "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" +) + +func diffFrom(oldMeta, newMeta nbpeer.PeerSystemMeta, oldLoc, newLoc nbpeer.Location) *nbpeer.MetaDiff { + return &nbpeer.MetaDiff{ + OldMeta: oldMeta, + NewMeta: newMeta, + OldLocation: oldLoc, + NewLocation: newLoc, + } +} + +func postureBundle(def nmdata.ChecksDefinition) []*nmdata.PostureChecks { + return []*nmdata.PostureChecks{{Checks: def}} +} + +func TestMetaDiffAffectsPosture_NBVersion(t *testing.T) { + c := postureBundle(nmdata.ChecksDefinition{NBVersionCheck: &nmdata.NBVersionCheck{MinVersion: "1.2.0"}}) + + tests := []struct { + name string + oldVer, newVer string + want bool + }{ + {"both above min, no flip", "1.3.0", "1.4.0", false}, + {"both below min, no flip", "1.0.0", "1.1.0", false}, + {"crosses up below->above", "1.1.0", "1.3.0", true}, + {"crosses down above->below", "1.3.0", "1.1.0", true}, + {"unparsable old only -> flip", "garbage", "1.3.0", true}, + {"unparsable both -> no flip", "garbage", "junk", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + diff := diffFrom( + nbpeer.PeerSystemMeta{WtVersion: tt.oldVer}, + nbpeer.PeerSystemMeta{WtVersion: tt.newVer}, + nbpeer.Location{}, nbpeer.Location{}, + ) + assert.Equal(t, tt.want, metaDiffAffectsPosture(diff, c)) + }) + } +} + +func TestMetaDiffAffectsPosture_OSVersion_KernelBumpWithinMin(t *testing.T) { + c := postureBundle(nmdata.ChecksDefinition{OSVersionCheck: &nmdata.OSVersionCheck{ + Linux: &nmdata.MinKernelVersionCheck{MinKernelVersion: "5.0.0"}, + }}) + + withinMin := diffFrom( + nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "5.10.0-arch1"}, + nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "5.15.0-arch2"}, + nbpeer.Location{}, nbpeer.Location{}, + ) + assert.False(t, metaDiffAffectsPosture(withinMin, c)) + + crossesDown := diffFrom( + nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "5.10.0-arch1"}, + nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "4.19.0-arch1"}, + nbpeer.Location{}, nbpeer.Location{}, + ) + assert.True(t, metaDiffAffectsPosture(crossesDown, c)) +} + +func TestMetaDiffAffectsPosture_OSVersion_GoOSSwitchFlipsVerdict(t *testing.T) { + c := postureBundle(nmdata.ChecksDefinition{OSVersionCheck: &nmdata.OSVersionCheck{ + Linux: &nmdata.MinKernelVersionCheck{MinKernelVersion: "6.0.0"}, + }}) + + diff := diffFrom( + nbpeer.PeerSystemMeta{GoOS: "freebsd"}, + nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "4.19.0"}, + nbpeer.Location{}, nbpeer.Location{}, + ) + assert.True(t, metaDiffAffectsPosture(diff, c)) +} + +func TestMetaDiffAffectsPosture_Process_GoOSSwitchFlipsVerdict(t *testing.T) { + c := postureBundle(nmdata.ChecksDefinition{ProcessCheck: &nmdata.ProcessCheck{ + Processes: []nmdata.Process{{LinuxPath: "/usr/bin/foo"}}, + }}) + + files := []nbpeer.File{{Path: "/usr/bin/foo", ProcessIsRunning: true}} + diff := diffFrom( + nbpeer.PeerSystemMeta{GoOS: "linux", Files: files}, + nbpeer.PeerSystemMeta{GoOS: "windows", Files: files}, + nbpeer.Location{}, nbpeer.Location{}, + ) + assert.True(t, metaDiffAffectsPosture(diff, c)) +} + +func TestMetaDiffAffectsPosture_Process_UnrelatedFileChange(t *testing.T) { + c := postureBundle(nmdata.ChecksDefinition{ProcessCheck: &nmdata.ProcessCheck{ + Processes: []nmdata.Process{{LinuxPath: "/usr/bin/foo"}}, + }}) + + diff := diffFrom( + nbpeer.PeerSystemMeta{GoOS: "linux", Files: []nbpeer.File{ + {Path: "/usr/bin/foo", ProcessIsRunning: true}, + }}, + nbpeer.PeerSystemMeta{GoOS: "linux", Files: []nbpeer.File{ + {Path: "/usr/bin/foo", ProcessIsRunning: true}, + {Path: "/usr/bin/bar", ProcessIsRunning: true}, + }}, + nbpeer.Location{}, nbpeer.Location{}, + ) + assert.False(t, metaDiffAffectsPosture(diff, c)) +} + +func TestMetaDiffAffectsPosture_GeoLocation(t *testing.T) { + c := postureBundle(nmdata.ChecksDefinition{GeoLocationCheck: &nmdata.GeoLocationCheck{ + Action: posture.CheckActionAllow, + Locations: []nmdata.GeoLocation{{CountryCode: "DE"}}, + }}) + + stayAllowed := diffFrom( + nbpeer.PeerSystemMeta{}, nbpeer.PeerSystemMeta{}, + nbpeer.Location{CountryCode: "DE", CityName: "Berlin"}, + nbpeer.Location{CountryCode: "DE", CityName: "Munich"}, + ) + assert.False(t, metaDiffAffectsPosture(stayAllowed, c)) + + moveOut := diffFrom( + nbpeer.PeerSystemMeta{}, nbpeer.PeerSystemMeta{}, + nbpeer.Location{CountryCode: "DE"}, + nbpeer.Location{CountryCode: "FR"}, + ) + assert.True(t, metaDiffAffectsPosture(moveOut, c)) +} + +func TestMetaDiffAffectsPosture_PeerNetworkRange_ConnectionIP(t *testing.T) { + c := postureBundle(nmdata.ChecksDefinition{PeerNetworkRangeCheck: &nmdata.PeerNetworkRangeCheck{ + Action: posture.CheckActionAllow, + Ranges: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, + }}) + + movesOutOfRange := diffFrom( + nbpeer.PeerSystemMeta{}, nbpeer.PeerSystemMeta{}, + nbpeer.Location{ConnectionIP: net.ParseIP("10.1.2.3")}, + nbpeer.Location{ConnectionIP: net.ParseIP("8.8.8.8")}, + ) + assert.True(t, metaDiffAffectsPosture(movesOutOfRange, c)) + + staysInRange := diffFrom( + nbpeer.PeerSystemMeta{}, nbpeer.PeerSystemMeta{}, + nbpeer.Location{ConnectionIP: net.ParseIP("10.1.2.3")}, + nbpeer.Location{ConnectionIP: net.ParseIP("10.9.9.9")}, + ) + assert.False(t, metaDiffAffectsPosture(staysInRange, c)) +} + +func TestMetaDiffAffectsPosture_IrrelevantFieldChange(t *testing.T) { + c := postureBundle(nmdata.ChecksDefinition{ + NBVersionCheck: &nmdata.NBVersionCheck{MinVersion: "1.0.0"}, + GeoLocationCheck: &nmdata.GeoLocationCheck{Action: posture.CheckActionAllow, Locations: []nmdata.GeoLocation{{CountryCode: "DE"}}}, + }) + + diff := diffFrom( + nbpeer.PeerSystemMeta{Hostname: "old", WtVersion: "1.5.0"}, + nbpeer.PeerSystemMeta{Hostname: "new", WtVersion: "1.5.0"}, + nbpeer.Location{CountryCode: "DE"}, nbpeer.Location{CountryCode: "DE"}, + ) + assert.False(t, metaDiffAffectsPosture(diff, c)) +} + +func TestMetaDiffAffectsPosture_NoChecks(t *testing.T) { + diff := diffFrom( + nbpeer.PeerSystemMeta{WtVersion: "1.0.0"}, + nbpeer.PeerSystemMeta{WtVersion: "2.0.0"}, + nbpeer.Location{}, nbpeer.Location{}, + ) + assert.False(t, metaDiffAffectsPosture(diff, nil)) +} + +func TestProcessPeerPostureChecks(t *testing.T) { + policy := &types.Policy{ + Enabled: true, + SourcePostureChecks: []string{"pc1"}, + Rules: []*types.PolicyRule{ + {Enabled: false, Sources: []string{"g-disabled"}, SourceResource: types.Resource{ID: "peer-disabled", Type: types.ResourceTypePeer}}, + {Enabled: true, Sources: []string{"g-src"}, Destinations: []string{"g-dst"}}, + {Enabled: true, SourceResource: types.Resource{ID: "peer-direct", Type: types.ResourceTypePeer}, Destinations: []string{"g-dst"}}, + {Enabled: true, SourceResource: types.Resource{ID: "peer-as-host", Type: types.ResourceTypeHost}, Destinations: []string{"g-dst"}}, + }, + } + + assert.Equal(t, []string{"pc1"}, processPeerPostureChecks(policy, "peer-in-group", []string{"g-src"}), "source group member") + assert.Equal(t, []string{"pc1"}, processPeerPostureChecks(policy, "peer-direct", nil), "direct source peer") + assert.Empty(t, processPeerPostureChecks(policy, "peer-elsewhere", []string{"g-dst"}), "destination-only peer") + assert.Empty(t, processPeerPostureChecks(policy, "peer-disabled", []string{"g-disabled"}), "disabled rule") + assert.Empty(t, processPeerPostureChecks(policy, "peer-as-host", nil), "source resource of a non-peer type") +} diff --git a/management/server/peer_test.go b/management/server/peer_test.go index 9a662bdbf..22f2b9b6f 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -16,11 +16,11 @@ import ( "testing" "time" - "go.uber.org/mock/gomock" "github.com/rs/xid" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "golang.org/x/exp/maps" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" @@ -1170,11 +1170,11 @@ func TestToSyncResponse(t *testing.T) { }, } dnsName := "example.com" - checks := []*posture.Checks{ + checks := []*nmdata.PostureChecks{ { - Checks: posture.ChecksDefinition{ - ProcessCheck: &posture.ProcessCheck{ - Processes: []posture.Process{{LinuxPath: "/usr/bin/netbird"}}, + Checks: nmdata.ChecksDefinition{ + ProcessCheck: &nmdata.ProcessCheck{ + Processes: []nmdata.Process{{LinuxPath: "/usr/bin/netbird"}}, }, }, }, diff --git a/management/server/posture/affects_posture_test.go b/management/server/posture/affects_posture_test.go deleted file mode 100644 index 6aa54d892..000000000 --- a/management/server/posture/affects_posture_test.go +++ /dev/null @@ -1,202 +0,0 @@ -package posture - -import ( - "context" - "net" - "net/netip" - "testing" - - "github.com/stretchr/testify/assert" - - nbpeer "github.com/netbirdio/netbird/management/server/peer" -) - -// diffFrom builds a MetaDiff from the old/new snapshots AffectsPosture replays against. -func diffFrom(oldMeta, newMeta nbpeer.PeerSystemMeta, oldLoc, newLoc nbpeer.Location) *nbpeer.MetaDiff { - return &nbpeer.MetaDiff{ - OldMeta: oldMeta, - NewMeta: newMeta, - OldLocation: oldLoc, - NewLocation: newLoc, - } -} - -func checks(def ChecksDefinition) []*Checks { - return []*Checks{{Checks: def}} -} - -func TestAffectsPosture_NilDiff(t *testing.T) { - assert.False(t, AffectsPosture(context.Background(), nil, checks(ChecksDefinition{ - NBVersionCheck: &NBVersionCheck{MinVersion: "1.0.0"}, - }))) -} - -func TestAffectsPosture_NBVersion(t *testing.T) { - c := checks(ChecksDefinition{NBVersionCheck: &NBVersionCheck{MinVersion: "1.2.0"}}) - - tests := []struct { - name string - oldVer, newVer string - want bool - }{ - {"both above min, no flip", "1.3.0", "1.4.0", false}, - {"both below min, no flip", "1.0.0", "1.1.0", false}, - {"crosses up below->above", "1.1.0", "1.3.0", true}, - {"crosses down above->below", "1.3.0", "1.1.0", true}, - {"unparsable old only -> flip", "garbage", "1.3.0", true}, - {"unparsable both -> no flip", "garbage", "junk", false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - diff := diffFrom( - nbpeer.PeerSystemMeta{WtVersion: tt.oldVer}, - nbpeer.PeerSystemMeta{WtVersion: tt.newVer}, - nbpeer.Location{}, nbpeer.Location{}, - ) - assert.Equal(t, tt.want, AffectsPosture(context.Background(), diff, c)) - }) - } -} - -func TestAffectsPosture_OSVersion_KernelBumpWithinMin(t *testing.T) { - c := checks(ChecksDefinition{OSVersionCheck: &OSVersionCheck{ - Linux: &MinKernelVersionCheck{MinKernelVersion: "5.0.0"}, - }}) - - // Kernel moves but stays above the minimum: verdict stays pass -> not affected. - withinMin := diffFrom( - nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "5.10.0-arch1"}, - nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "5.15.0-arch2"}, - nbpeer.Location{}, nbpeer.Location{}, - ) - assert.False(t, AffectsPosture(context.Background(), withinMin, c)) - - // Kernel drops below the minimum: verdict flips pass -> fail -> affected. - crossesDown := diffFrom( - nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "5.10.0-arch1"}, - nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "4.19.0-arch1"}, - nbpeer.Location{}, nbpeer.Location{}, - ) - assert.True(t, AffectsPosture(context.Background(), crossesDown, c)) -} - -func TestAffectsPosture_OSVersion_GoOSSwitchFlipsVerdict(t *testing.T) { - // Only Linux is constrained. An OS outside the switch (freebsd) passes; switching to a - // failing linux kernel flips the verdict pass -> fail. - c := checks(ChecksDefinition{OSVersionCheck: &OSVersionCheck{ - Linux: &MinKernelVersionCheck{MinKernelVersion: "6.0.0"}, - }}) - - diff := diffFrom( - nbpeer.PeerSystemMeta{GoOS: "freebsd"}, - nbpeer.PeerSystemMeta{GoOS: "linux", KernelVersion: "4.19.0"}, - nbpeer.Location{}, nbpeer.Location{}, - ) - assert.True(t, AffectsPosture(context.Background(), diff, c)) -} - -func TestAffectsPosture_Process_GoOSSwitchFlipsVerdict(t *testing.T) { - // Process runs at a linux path. Switching GoOS to windows (no WindowsPath configured) - // flips the verdict. - c := checks(ChecksDefinition{ProcessCheck: &ProcessCheck{ - Processes: []Process{{LinuxPath: "/usr/bin/foo"}}, - }}) - - files := []nbpeer.File{{Path: "/usr/bin/foo", ProcessIsRunning: true}} - diff := diffFrom( - nbpeer.PeerSystemMeta{GoOS: "linux", Files: files}, - nbpeer.PeerSystemMeta{GoOS: "windows", Files: files}, - nbpeer.Location{}, nbpeer.Location{}, - ) - assert.True(t, AffectsPosture(context.Background(), diff, c)) -} - -func TestAffectsPosture_Process_UnrelatedFileChange(t *testing.T) { - // A tracked process stays running while an unrelated file is added: the verdict does - // not move, so posture is not affected. - c := checks(ChecksDefinition{ProcessCheck: &ProcessCheck{ - Processes: []Process{{LinuxPath: "/usr/bin/foo"}}, - }}) - - diff := diffFrom( - nbpeer.PeerSystemMeta{GoOS: "linux", Files: []nbpeer.File{ - {Path: "/usr/bin/foo", ProcessIsRunning: true}, - }}, - nbpeer.PeerSystemMeta{GoOS: "linux", Files: []nbpeer.File{ - {Path: "/usr/bin/foo", ProcessIsRunning: true}, - {Path: "/usr/bin/bar", ProcessIsRunning: true}, - }}, - nbpeer.Location{}, nbpeer.Location{}, - ) - assert.False(t, AffectsPosture(context.Background(), diff, c)) -} - -func TestAffectsPosture_GeoLocation(t *testing.T) { - c := checks(ChecksDefinition{GeoLocationCheck: &GeoLocationCheck{ - Action: CheckActionAllow, - Locations: []Location{{CountryCode: "DE"}}, - }}) - - // Moving within allowed countries keeps the verdict; moving out flips it. - stayAllowed := diffFrom( - nbpeer.PeerSystemMeta{}, nbpeer.PeerSystemMeta{}, - nbpeer.Location{CountryCode: "DE", CityName: "Berlin"}, - nbpeer.Location{CountryCode: "DE", CityName: "Munich"}, - ) - assert.False(t, AffectsPosture(context.Background(), stayAllowed, c)) - - moveOut := diffFrom( - nbpeer.PeerSystemMeta{}, nbpeer.PeerSystemMeta{}, - nbpeer.Location{CountryCode: "DE"}, - nbpeer.Location{CountryCode: "FR"}, - ) - assert.True(t, AffectsPosture(context.Background(), moveOut, c)) -} - -func TestAffectsPosture_PeerNetworkRange_ConnectionIP(t *testing.T) { - // The check reads the connection IP. Moving out of the allowed range flips the verdict; - // moving within it does not. - _, allowed, _ := net.ParseCIDR("10.0.0.0/8") - c := checks(ChecksDefinition{PeerNetworkRangeCheck: &PeerNetworkRangeCheck{ - Action: CheckActionAllow, - Ranges: []netip.Prefix{netip.MustParsePrefix(allowed.String())}, - }}) - - movesOutOfRange := diffFrom( - nbpeer.PeerSystemMeta{}, nbpeer.PeerSystemMeta{}, - nbpeer.Location{ConnectionIP: net.ParseIP("10.1.2.3")}, - nbpeer.Location{ConnectionIP: net.ParseIP("8.8.8.8")}, - ) - assert.True(t, AffectsPosture(context.Background(), movesOutOfRange, c)) - - staysInRange := diffFrom( - nbpeer.PeerSystemMeta{}, nbpeer.PeerSystemMeta{}, - nbpeer.Location{ConnectionIP: net.ParseIP("10.1.2.3")}, - nbpeer.Location{ConnectionIP: net.ParseIP("10.9.9.9")}, - ) - assert.False(t, AffectsPosture(context.Background(), staysInRange, c)) -} - -func TestAffectsPosture_IrrelevantFieldChange(t *testing.T) { - // Hostname changes but no check reads it: not affected even with checks present. - c := checks(ChecksDefinition{ - NBVersionCheck: &NBVersionCheck{MinVersion: "1.0.0"}, - GeoLocationCheck: &GeoLocationCheck{Action: CheckActionAllow, Locations: []Location{{CountryCode: "DE"}}}, - }) - - diff := diffFrom( - nbpeer.PeerSystemMeta{Hostname: "old", WtVersion: "1.5.0"}, - nbpeer.PeerSystemMeta{Hostname: "new", WtVersion: "1.5.0"}, - nbpeer.Location{CountryCode: "DE"}, nbpeer.Location{CountryCode: "DE"}, - ) - assert.False(t, AffectsPosture(context.Background(), diff, c)) -} - -func TestAffectsPosture_NoChecks(t *testing.T) { - diff := diffFrom( - nbpeer.PeerSystemMeta{WtVersion: "1.0.0"}, - nbpeer.PeerSystemMeta{WtVersion: "2.0.0"}, - nbpeer.Location{}, nbpeer.Location{}, - ) - assert.False(t, AffectsPosture(context.Background(), diff, nil)) -} diff --git a/management/server/posture/checks.go b/management/server/posture/checks.go index 72b719252..c38136d1c 100644 --- a/management/server/posture/checks.go +++ b/management/server/posture/checks.go @@ -7,7 +7,6 @@ import ( "regexp" "github.com/hashicorp/go-version" - log "github.com/sirupsen/logrus" nbpeer "github.com/netbirdio/netbird/management/server/peer" "github.com/netbirdio/netbird/shared/management/http/api" @@ -55,46 +54,6 @@ type Checks struct { Checks ChecksDefinition `gorm:"serializer:json"` } -// AffectsPosture reports whether the change in diff flips the verdict of any check. It -// replays each check against the peer's old and new state and compares verdicts, so a -// change that moves a field but stays the right side of a threshold (e.g. a kernel bump -// still above the minimum) does not force a re-evaluation. See verdictChanged for how an -// evaluation error counts. -func AffectsPosture(ctx context.Context, diff *nbpeer.MetaDiff, checks []*Checks) bool { - if diff == nil { - return false - } - - oldPeer := nbpeer.Peer{Meta: diff.OldMeta, Location: diff.OldLocation} - newPeer := nbpeer.Peer{Meta: diff.NewMeta, Location: diff.NewLocation} - - for _, c := range checks { - for _, check := range c.GetChecks() { - if verdictChanged(ctx, check, oldPeer, newPeer) { - return true - } - } - } - return false -} - -// verdictChanged replays check against old and new state and reports whether the verdict -// differs. Like callers, it treats an evaluation error as deny: two errors are the same -// verdict (no change), an error on one side only is a flip. -func verdictChanged(ctx context.Context, check Check, oldPeer, newPeer nbpeer.Peer) bool { - oldPass, oldErr := check.Check(ctx, oldPeer) - newPass, newErr := check.Check(ctx, newPeer) - - oldVerdict := oldPass && (oldErr == nil) - newVerdict := newPass && (newErr == nil) - changed := oldVerdict != newVerdict - - log.WithContext(ctx).Tracef("posture check %s replay: verdict %t -> %t (changed=%t), errs: %v -> %v", - check.Name(), oldVerdict, newVerdict, changed, oldErr, newErr) - - return changed -} - // ChecksDefinition contains definition of actual check type ChecksDefinition struct { NBVersionCheck *NBVersionCheck `json:",omitempty"` diff --git a/management/server/types/account.go b/management/server/types/account.go index b54b3d01b..0bdf2df66 100644 --- a/management/server/types/account.go +++ b/management/server/types/account.go @@ -1111,8 +1111,17 @@ func ruleHasDestination(rule *PolicyRule, peerID string, peerGroupIDs map[string // Important: Posture checks are applicable only to source group peers, // for destination group peers, call this method with an empty list of sourcePostureChecksIDs func (a *Account) getAllPeersFromGroups(ctx context.Context, groups []string, peerID string, sourcePostureChecksIDs []string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, bool) { + return a.filterPolicyPeers(ctx, a.getUniquePeerIDsFromGroupsIDs(ctx, groups), peerID, sourcePostureChecksIDs, validatedPeersMap) +} + +// getPeerFromResource resolves a rule side that names a peer directly, admitting it +// like a member of a group holding only that peer. +func (a *Account) getPeerFromResource(ctx context.Context, resource Resource, peerID string, sourcePostureChecksIDs []string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, bool) { + return a.filterPolicyPeers(ctx, []string{resource.ID}, peerID, sourcePostureChecksIDs, validatedPeersMap) +} + +func (a *Account) filterPolicyPeers(ctx context.Context, uniquePeerIDs []string, peerID string, sourcePostureChecksIDs []string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, bool) { peerInGroups := false - uniquePeerIDs := a.getUniquePeerIDsFromGroupsIDs(ctx, groups) filteredPeers := make([]*nbpeer.Peer, 0, len(uniquePeerIDs)) for _, p := range uniquePeerIDs { peer, ok := a.Peers[p] diff --git a/management/server/types/account_networkmapdata.go b/management/server/types/account_networkmapdata.go index e0305be7c..a89cf8cb1 100644 --- a/management/server/types/account_networkmapdata.go +++ b/management/server/types/account_networkmapdata.go @@ -93,7 +93,7 @@ func (a *Account) toNetworkMapData( } for _, pc := range a.PostureChecks { if pc != nil { - nmd.PostureChecks[pc.ID] = twinPostureChecks(pc) + nmd.PostureChecks[pc.ID] = TwinPostureChecks(pc) nmd.PostureCheckXIDToPublicID[pc.ID] = pc.PublicID } } @@ -393,7 +393,17 @@ func TwinNetwork(n *Network) *nmdata.Network { } } -func twinPostureChecks(pc *posture.Checks) *nmdata.PostureChecks { +// TwinPostureChecksList converts posture checks to their slim nmdata twins. +func TwinPostureChecksList(checks []*posture.Checks) []*nmdata.PostureChecks { + out := make([]*nmdata.PostureChecks, 0, len(checks)) + for _, pc := range checks { + out = append(out, TwinPostureChecks(pc)) + } + return out +} + +// TwinPostureChecks converts posture checks to their slim nmdata twin. +func TwinPostureChecks(pc *posture.Checks) *nmdata.PostureChecks { if pc == nil { return nil } diff --git a/management/server/types/networkmap_components_correctness_test.go b/management/server/types/networkmap_components_correctness_test.go index eb3e4fe3b..35b5f7149 100644 --- a/management/server/types/networkmap_components_correctness_test.go +++ b/management/server/types/networkmap_components_correctness_test.go @@ -875,6 +875,89 @@ func TestComponents_PeerAsSourceResource(t *testing.T) { assert.True(t, has443, "peer-0 as source resource should have port 443 rule") } +func hasFirewallRuleTo(nm *types.NetworkMap, peerIP, port string) bool { + for _, rule := range nm.FirewallRules { + if rule.PeerIP == peerIP && rule.Port == port { + return true + } + } + return false +} + +// TestComponents_PeerAsSourceResource_PostureChecks verifies that a directly referenced +// source peer is gated by the policy's posture checks like a member of a group holding only +// that peer: peer-1 (0.25.0) fails the 0.26.0 minimum, peer-2 (0.40.0) passes. +func TestComponents_PeerAsSourceResource_PostureChecks(t *testing.T) { + account, validatedPeers := scalableTestAccountWithoutDefaultPolicy(20, 2) + + for _, sourcePeerID := range []string{"peer-1", "peer-2"} { + account.Policies = append(account.Policies, &types.Policy{ + ID: "policy-peer-src-" + sourcePeerID, Name: "Peer Source " + sourcePeerID, Enabled: true, AccountID: "test-account", + SourcePostureChecks: []string{"posture-check-ver"}, + Rules: []*types.PolicyRule{{ + ID: "rule-peer-src-" + sourcePeerID, Enabled: true, + Action: types.PolicyTrafficActionAccept, + Protocol: types.PolicyRuleProtocolTCP, + Bidirectional: true, + Ports: []string{"9443"}, + SourceResource: types.Resource{ID: sourcePeerID, Type: types.ResourceTypePeer}, + Destinations: []string{"group-0"}, + }}, + }) + } + + nm0 := componentsNetworkMap(account, "peer-0", validatedPeers) + require.NotNil(t, nm0) + assert.False(t, hasFirewallRuleTo(nm0, "100.64.0.1", "9443"), "destination must not see the direct source peer failing the posture check") + assert.True(t, hasFirewallRuleTo(nm0, "100.64.0.2", "9443"), "destination must see the direct source peer passing the posture check") + + nm1 := componentsNetworkMap(account, "peer-1", validatedPeers) + require.NotNil(t, nm1) + assert.False(t, hasFirewallRuleTo(nm1, "100.64.0.0", "9443"), "a direct source peer failing the posture check gets no policy connectivity") + + nm2 := componentsNetworkMap(account, "peer-2", validatedPeers) + require.NotNil(t, nm2) + assert.True(t, hasFirewallRuleTo(nm2, "100.64.0.0", "9443"), "a direct source peer passing the posture check gets policy connectivity") +} + +// TestComponents_PeerAsResource_Unvalidated verifies that a directly referenced peer is +// subject to approval like a group member, whether it is the rule's source or destination. +func TestComponents_PeerAsResource_Unvalidated(t *testing.T) { + account, validatedPeers := scalableTestAccountWithoutDefaultPolicy(20, 2) + delete(validatedPeers, "peer-2") + + account.Policies = append(account.Policies, + &types.Policy{ + ID: "policy-unval-src", Name: "Unvalidated Source", Enabled: true, AccountID: "test-account", + Rules: []*types.PolicyRule{{ + ID: "rule-unval-src", Enabled: true, + Action: types.PolicyTrafficActionAccept, Protocol: types.PolicyRuleProtocolTCP, Bidirectional: true, + Ports: []string{"9443"}, + SourceResource: types.Resource{ID: "peer-2", Type: types.ResourceTypePeer}, + Destinations: []string{"group-0"}, + }}, + }, + &types.Policy{ + ID: "policy-unval-dst", Name: "Unvalidated Destination", Enabled: true, AccountID: "test-account", + Rules: []*types.PolicyRule{{ + ID: "rule-unval-dst", Enabled: true, + Action: types.PolicyTrafficActionAccept, Protocol: types.PolicyRuleProtocolTCP, Bidirectional: true, + Ports: []string{"9444"}, + Sources: []string{"group-0"}, + DestinationResource: types.Resource{ID: "peer-2", Type: types.ResourceTypePeer}, + }}, + }, + ) + + nm0 := componentsNetworkMap(account, "peer-0", validatedPeers) + require.NotNil(t, nm0) + assert.False(t, hasFirewallRuleTo(nm0, "100.64.0.2", "9443"), "an unvalidated direct source peer must not be admitted") + assert.False(t, hasFirewallRuleTo(nm0, "100.64.0.2", "9444"), "an unvalidated direct destination peer must not be admitted") + for _, p := range nm0.Peers { + assert.NotEqual(t, "peer-2", p.ID, "an unvalidated direct peer must not be shipped as a remote peer") + } +} + // TestComponents_PeerAsDestinationResource verifies that a policy with DestinationResource.Type=Peer // targets only that specific peer as the destination. func TestComponents_PeerAsDestinationResource(t *testing.T) { diff --git a/shared/management/networkmap/networkmapcompute.go b/shared/management/networkmap/networkmapcompute.go index 1cf7aeef4..65e76d097 100644 --- a/shared/management/networkmap/networkmapcompute.go +++ b/shared/management/networkmap/networkmapcompute.go @@ -324,19 +324,13 @@ func (nmd *NetworkMapData) getPeersGroupsPoliciesRoutes( var peerInSources, peerInDestinations bool if rule.SourceResource.Type == string(types.ResourceTypePeer) && rule.SourceResource.ID != "" { - sourcePeers = []string{rule.SourceResource.ID} - if rule.SourceResource.ID == peerID { - peerInSources = true - } + sourcePeers, peerInSources = nmd.getPeerFromResource(rule.SourceResource, peerID, policy.SourcePostureChecks, postureFailedPeers) } else { sourcePeers, peerInSources = nmd.getPeersFromGroups(rule.Sources, peerID, policy.SourcePostureChecks, postureFailedPeers) } if rule.DestinationResource.Type == string(types.ResourceTypePeer) && rule.DestinationResource.ID != "" { - destinationPeers = []string{rule.DestinationResource.ID} - if rule.DestinationResource.ID == peerID { - peerInDestinations = true - } + destinationPeers, peerInDestinations = nmd.getPeerFromResource(rule.DestinationResource, peerID, nil, postureFailedPeers) } else { destinationPeers, peerInDestinations = nmd.getPeersFromGroups(rule.Destinations, peerID, nil, postureFailedPeers) } @@ -403,30 +397,16 @@ func (nmd *NetworkMapData) getPeersFromGroups(groups []string, peerID string, so filteredPeerIDs = make([]string, 0, len(group.Peers)) peerInGroups = false for _, pid := range group.Peers { - peer, ok := nmd.Peers[pid] - if !ok || peer == nil { + if !nmd.admitPolicyPeer(pid, sourcePostureChecksIDs, postureFailedPeers) { continue } - if _, ok := nmd.ValidatedPeers[peer.ID]; !ok { - continue - } - - isValid, pname := nmd.validatePostureChecksOnPeerGetFailed(sourcePostureChecksIDs, peer.ID) - if !isValid && len(pname) > 0 { - if _, ok := (*postureFailedPeers)[pname]; !ok { - (*postureFailedPeers)[pname] = make(map[string]struct{}) - } - (*postureFailedPeers)[pname][peer.ID] = struct{}{} - continue - } - - if peer.ID == peerID { + if pid == peerID { peerInGroups = true continue } - filteredPeerIDs = append(filteredPeerIDs, peer.ID) + filteredPeerIDs = append(filteredPeerIDs, pid) } return filteredPeerIDs, peerInGroups } @@ -436,36 +416,59 @@ func (nmd *NetworkMapData) getPeersFromGroups(groups []string, peerID string, so continue } seenPeerIds[pid] = struct{}{} - peer, ok := nmd.Peers[pid] - if !ok || peer == nil { + if !nmd.admitPolicyPeer(pid, sourcePostureChecksIDs, postureFailedPeers) { continue } - if _, ok := nmd.ValidatedPeers[peer.ID]; !ok { - continue - } - - isValid, pname := nmd.validatePostureChecksOnPeerGetFailed(sourcePostureChecksIDs, peer.ID) - if !isValid && len(pname) > 0 { - if _, ok := (*postureFailedPeers)[pname]; !ok { - (*postureFailedPeers)[pname] = make(map[string]struct{}) - } - (*postureFailedPeers)[pname][peer.ID] = struct{}{} - continue - } - - if peer.ID == peerID { + if pid == peerID { peerInGroups = true continue } - filteredPeerIDs = append(filteredPeerIDs, peer.ID) + filteredPeerIDs = append(filteredPeerIDs, pid) } } return filteredPeerIDs, peerInGroups } +// getPeerFromResource resolves a rule side that names a peer directly, admitting it +// like a member of a group holding only that peer. +func (nmd *NetworkMapData) getPeerFromResource(resource nmdata.Resource, peerID string, sourcePostureChecksIDs []string, + postureFailedPeers *map[string]map[string]struct{}) ([]string, bool) { + if !nmd.admitPolicyPeer(resource.ID, sourcePostureChecksIDs, postureFailedPeers) { + return nil, false + } + if resource.ID == peerID { + return nil, true + } + return []string{resource.ID}, false +} + +// admitPolicyPeer applies the per-peer admission of a rule side: the peer must exist, +// be validated and pass the rule's posture checks. A failed check is recorded in +// postureFailedPeers. +func (nmd *NetworkMapData) admitPolicyPeer(pid string, sourcePostureChecksIDs []string, postureFailedPeers *map[string]map[string]struct{}) bool { + peer, ok := nmd.Peers[pid] + if !ok || peer == nil { + return false + } + + if _, ok := nmd.ValidatedPeers[pid]; !ok { + return false + } + + isValid, pname := nmd.validatePostureChecksOnPeerGetFailed(sourcePostureChecksIDs, pid) + if !isValid && len(pname) > 0 { + if _, ok := (*postureFailedPeers)[pname]; !ok { + (*postureFailedPeers)[pname] = make(map[string]struct{}) + } + (*postureFailedPeers)[pname][pid] = struct{}{} + return false + } + return true +} + func (nmd *NetworkMapData) validatePostureChecksOnPeerGetFailed(sourcePostureChecksID []string, peerID string) (bool, string) { peer, ok := nmd.Peers[peerID] if !ok || peer == nil { diff --git a/shared/management/networkmap/networkmapcompute_test.go b/shared/management/networkmap/networkmapcompute_test.go index 8c9add8c1..ad21fd70f 100644 --- a/shared/management/networkmap/networkmapcompute_test.go +++ b/shared/management/networkmap/networkmapcompute_test.go @@ -448,10 +448,9 @@ func TestGetPeerNetworkMapComponents_PeerResourceRules(t *testing.T) { assert.ElementsMatch(t, []string{targetID, remote.ID}, peerIDSet(c.Peers)) }) - // Legacy parity: directly referenced peers bypass the ValidatedPeers gate - // and posture checks that group-derived peers go through; the client-side - // Calculate shares this behavior via getPeerFromResource. - t.Run("unvalidated source resource peer still connects", func(t *testing.T) { + // A directly referenced peer is admitted like a member of a group holding only + // that peer: the ValidatedPeers gate and the posture checks apply equally. + t.Run("unvalidated source resource peer is excluded", func(t *testing.T) { target := newPeer(targetID, 1) unval := newPeer("peer-unval", 2) nmd := newNMD(target, unval) @@ -463,10 +462,10 @@ func TestGetPeerNetworkMapComponents_PeerResourceRules(t *testing.T) { c := compute(nmd, targetID) - assert.ElementsMatch(t, []string{targetID, unval.ID}, peerIDSet(c.Peers)) + assert.ElementsMatch(t, []string{targetID}, peerIDSet(c.Peers)) }) - t.Run("source resource peer bypasses posture checks", func(t *testing.T) { + t.Run("source resource peer failing posture checks is excluded", func(t *testing.T) { target := newPeer(targetID, 1) failing := newPeer("peer-failing", 2) failing.Meta.WtVersion = failingVersion @@ -481,10 +480,65 @@ func TestGetPeerNetworkMapComponents_PeerResourceRules(t *testing.T) { c := compute(nmd, targetID) - assert.ElementsMatch(t, []string{targetID, failing.ID}, peerIDSet(c.Peers)) + assert.ElementsMatch(t, []string{targetID}, peerIDSet(c.Peers)) assert.Empty(t, c.PostureFailedPeers) }) + t.Run("direct source peer failure recorded when connected via another policy", func(t *testing.T) { + target := newPeer(targetID, 1) + failing := newPeer("peer-failing", 2) + failing.Meta.WtVersion = failingVersion + nmd := newNMD(target, failing) + addVersionCheck(nmd, "pc-1", postureMinVersion) + addGroup(nmd, "g-dst", targetID) + checkedRule := newRule(nil, []string{"g-dst"}) + checkedRule.SourceResource = peerResource(failing.ID) + checked := newPolicy("p-checked", checkedRule) + checked.SourcePostureChecks = []string{"pc-1"} + openRule := newRule(nil, []string{"g-dst"}) + openRule.SourceResource = peerResource(failing.ID) + nmd.Policies = []*nmdata.Policy{checked, newPolicy("p-open", openRule)} + + c := compute(nmd, targetID) + + assert.ElementsMatch(t, []string{targetID, failing.ID}, peerIDSet(c.Peers)) + assert.Equal(t, map[string]map[string]struct{}{"pc-1": {failing.ID: {}}}, c.PostureFailedPeers) + }) + + t.Run("target as source resource failing posture checks gets no policy", func(t *testing.T) { + target := newPeer(targetID, 1) + target.Meta.WtVersion = failingVersion + dst := newPeer("peer-dst", 2) + nmd := newNMD(target, dst) + addVersionCheck(nmd, "pc-1", postureMinVersion) + addGroup(nmd, "g-dst", dst.ID) + rule := newRule(nil, []string{"g-dst"}) + rule.SourceResource = peerResource(targetID) + p := newPolicy("p-1", rule) + p.SourcePostureChecks = []string{"pc-1"} + nmd.Policies = []*nmdata.Policy{p} + + c := compute(nmd, targetID) + + assert.Empty(t, policyIDs(c.Policies)) + assert.ElementsMatch(t, []string{targetID}, peerIDSet(c.Peers)) + }) + + t.Run("unvalidated destination resource peer is excluded", func(t *testing.T) { + target := newPeer(targetID, 1) + unval := newPeer("peer-unval", 2) + nmd := newNMD(target, unval) + delete(nmd.ValidatedPeers, unval.ID) + addGroup(nmd, "g-src", targetID) + rule := newRule([]string{"g-src"}, nil) + rule.DestinationResource = peerResource(unval.ID) + nmd.Policies = []*nmdata.Policy{newPolicy("p-1", rule)} + + c := compute(nmd, targetID) + + assert.ElementsMatch(t, []string{targetID}, peerIDSet(c.Peers)) + }) + t.Run("unrelated peer resource rule ignored", func(t *testing.T) { target := newPeer(targetID, 1) a := newPeer("peer-a", 2) diff --git a/shared/management/networkmap/nmdata/posture.go b/shared/management/networkmap/nmdata/posture.go index dc1753791..6a6b028c7 100644 --- a/shared/management/networkmap/nmdata/posture.go +++ b/shared/management/networkmap/nmdata/posture.go @@ -45,6 +45,22 @@ func PassesChecks(checks []Check, peer *Peer) bool { return true } +// PostureVerdictChanged reports whether any check in the bundles gives a different +// verdict for newPeer than for oldPeer. Checks are replayed one by one, so a change +// that moves a field but stays on the same side of a threshold does not count. An +// evaluation error is a deny, like in PassesChecks. +func PostureVerdictChanged(checks []*PostureChecks, oldPeer, newPeer *Peer) bool { + for _, pc := range checks { + for _, c := range pc.GetChecks() { + single := []Check{c} + if PassesChecks(single, oldPeer) != PassesChecks(single, newPeer) { + return true + } + } + } + return false +} + // GetChecks returns the initialized checks in the same order as posture.Checks.GetChecks. func (pc *PostureChecks) GetChecks() []Check { var checks []Check diff --git a/shared/management/networkmap/nmdata/posture_test.go b/shared/management/networkmap/nmdata/posture_test.go new file mode 100644 index 000000000..13e5f268e --- /dev/null +++ b/shared/management/networkmap/nmdata/posture_test.go @@ -0,0 +1,54 @@ +package nmdata + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func bundle(def ChecksDefinition) []*PostureChecks { + return []*PostureChecks{{Checks: def}} +} + +func TestPostureVerdictChanged_ErrorCountsAsDeny(t *testing.T) { + c := bundle(ChecksDefinition{NBVersionCheck: &NBVersionCheck{MinVersion: "1.2.0"}}) + + tests := []struct { + name string + oldVer, newVer string + want bool + }{ + {"both above min, no flip", "1.3.0", "1.4.0", false}, + {"crosses up below->above", "1.1.0", "1.3.0", true}, + {"unparsable old only -> flip", "garbage", "1.3.0", true}, + {"unparsable both -> no flip", "garbage", "junk", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + oldPeer := &Peer{Meta: PeerSystemMeta{WtVersion: tt.oldVer}} + newPeer := &Peer{Meta: PeerSystemMeta{WtVersion: tt.newVer}} + assert.Equal(t, tt.want, PostureVerdictChanged(c, oldPeer, newPeer)) + }) + } +} + +func TestPostureVerdictChanged_ReplaysEachCheck(t *testing.T) { + // Old fails the version check, new fails the kernel check: the bundle denies on + // both sides, yet every single check flipped, so the posture must be re-evaluated. + c := bundle(ChecksDefinition{ + NBVersionCheck: &NBVersionCheck{MinVersion: "1.0.0"}, + OSVersionCheck: &OSVersionCheck{Linux: &MinKernelVersionCheck{MinKernelVersion: "5.0.0"}}, + }) + oldPeer := &Peer{Meta: PeerSystemMeta{WtVersion: "0.9.0", GoOS: "linux", KernelVersion: "6.0.0"}} + newPeer := &Peer{Meta: PeerSystemMeta{WtVersion: "1.1.0", GoOS: "linux", KernelVersion: "4.0.0"}} + + assert.False(t, c[0].Passes(oldPeer)) + assert.False(t, c[0].Passes(newPeer)) + assert.True(t, PostureVerdictChanged(c, oldPeer, newPeer)) +} + +func TestPostureVerdictChanged_NoChecks(t *testing.T) { + oldPeer := &Peer{Meta: PeerSystemMeta{WtVersion: "1.0.0"}} + newPeer := &Peer{Meta: PeerSystemMeta{WtVersion: "2.0.0"}} + assert.False(t, PostureVerdictChanged(nil, oldPeer, newPeer)) +} diff --git a/shared/management/types/networkmap_components.go b/shared/management/types/networkmap_components.go index dc1e7c53c..2940ed492 100644 --- a/shared/management/types/networkmap_components.go +++ b/shared/management/types/networkmap_components.go @@ -370,8 +370,21 @@ func (c *NetworkMapComponents) connResourcesGenerator(targetPeer *nmdata.Peer) ( } func (c *NetworkMapComponents) getAllPeersFromGroups(groups []string, peerID string, sourcePostureChecksIDs []string) ([]*nmdata.Peer, bool) { + return c.filterPolicyPeers(c.getUniquePeerIDsFromGroupsIDs(groups), peerID, sourcePostureChecksIDs) +} + +// getPeerFromResource resolves a rule side that names a peer directly. The peer is +// subject to the same admission as a group member, so a direct peer behaves exactly +// like a group holding only that peer. +func (c *NetworkMapComponents) getPeerFromResource(resource nmdata.Resource, peerID string, sourcePostureChecksIDs []string) ([]*nmdata.Peer, bool) { + return c.filterPolicyPeers([]string{resource.ID}, peerID, sourcePostureChecksIDs) +} + +// filterPolicyPeers admits the peers of one rule side: known to the components and +// passing the rule's posture checks. It reports the admitted peers other than peerID +// and whether peerID itself is admitted on that side. +func (c *NetworkMapComponents) filterPolicyPeers(uniquePeerIDs []string, peerID string, sourcePostureChecksIDs []string) ([]*nmdata.Peer, bool) { peerInGroups := false - uniquePeerIDs := c.getUniquePeerIDsFromGroupsIDs(groups) filteredPeers := make([]*nmdata.Peer, 0, len(uniquePeerIDs)) for _, p := range uniquePeerIDs { @@ -424,25 +437,6 @@ func (c *NetworkMapComponents) getUniquePeerIDsFromGroupsIDs(groups []string) [] return ids } -func (c *NetworkMapComponents) getPeerFromResource(resource nmdata.Resource, peerID string, postureChecks []string) ([]*nmdata.Peer, bool) { - if resource.ID == peerID { - if len(postureChecks) > 0 && !c.ValidatePostureChecksOnPeer(peerID, postureChecks) { - return []*nmdata.Peer{}, false - } - return []*nmdata.Peer{}, true - } - - peerInfo := c.GetPeerInfo(resource.ID) - if peerInfo == nil { - return []*nmdata.Peer{}, false - } - if len(postureChecks) > 0 && !c.ValidatePostureChecksOnPeer(resource.ID, postureChecks) { - return []*nmdata.Peer{}, false - } - - return []*nmdata.Peer{peerInfo}, false -} - func (c *NetworkMapComponents) filterPeersByLoginExpiration(aclPeers []*nmdata.Peer) ([]*nmdata.Peer, []*nmdata.Peer) { peersToConnect := make([]*nmdata.Peer, 0, len(aclPeers)) var expiredPeers []*nmdata.Peer