Preserve pre-existing ipsets and roll back partial route ACL installs

.git-branches.toml

@@ -3,7 +3,7 @@
 [branches]
 main = "main"
 perennials = []
-perennial-regex = "^release-"
+perennial-regex = ""
 
 [create]
 new-branch-type = "feature"

.github/workflows/check-license-dependencies.yml

@@ -2,7 +2,7 @@ name: Check License Dependencies
 
 on:
   push:
-    branches: [main, "release-*"]
+    branches: [main]
     paths:
       - "go.mod"
       - "go.sum"

.github/workflows/frontend-ui.yml

@@ -10,7 +10,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
     paths:
       - "client/ui/frontend/**"
       - "client/ui/i18n/**"

.github/workflows/golang-test-darwin.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
   pull_request:
 
 concurrency:

.github/workflows/golang-test-freebsd.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
   pull_request:
 
 concurrency:

.github/workflows/golang-test-linux.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
   pull_request:
 
 concurrency:

.github/workflows/golang-test-windows.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
   pull_request:
 
 env:

.github/workflows/install-script-test.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
   pull_request:
     paths:
       - "release_files/install.sh"

.github/workflows/mobile-build-validation.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
   pull_request:
 
 concurrency:

.github/workflows/release.yml

@@ -6,7 +6,6 @@ on:
       - "v*"
     branches:
       - main
-      - "release-*"
   pull_request:
 
 env:
@@ -255,23 +254,15 @@ jobs:
         id: tag_and_push_images
         if: |
           (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) ||
-          (github.event_name == 'push' && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release-')))
+          (github.event_name == 'push' && github.ref == 'refs/heads/main')
         run: |
           set -euo pipefail
 
-          # $GITHUB_REF / $GITHUB_EVENT_NAME are read from the runner
-          # environment rather than substituted into this script with the
-          # workflow expression syntax: branch names may legally contain
-          # $(…), and interpolating github.ref would execute it.
           resolve_tags() {
-            if [[ "$GITHUB_EVENT_NAME" == "pull_request" ]]; then
+            if [[ "${{ github.event_name }}" == "pull_request" ]]; then
               echo "pr-${{ github.event.pull_request.number }}"
-            elif [[ "$GITHUB_REF" == "refs/heads/main" ]]; then
-              echo "main sha-$(git rev-parse --short HEAD)"
             else
-              # Release branches get an immutable sha-* tag only — the floating
-              # "main" tag must never move from a release branch.
-              echo "sha-$(git rev-parse --short HEAD)"
+              echo "main sha-$(git rev-parse --short HEAD)"
             fi
           }
 

.github/workflows/sync-tag.yml

@@ -9,9 +9,21 @@ concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}-${{ github.head_ref || github.actor_id }}
   cancel-in-progress: true
 
-# The receiving bump-netbird workflows expect the short tag form
-# (e.g. v0.30.0), not refs/tags/v0.30.0 — github.ref_name, not github.ref.
+# Receiving workflows (cloud sync-tag, mobile bump-netbird) expect the short
+# tag form (e.g. v0.30.0), not refs/tags/v0.30.0 — github.ref_name, not github.ref.
 jobs:
+  trigger_sync_tag:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Trigger release tag sync
+        uses: benc-uk/workflow-dispatch@31e2b3319479a63f0ab15bf800eff9e913504e26 # v1.3.2
+        with:
+          workflow: sync-tag.yml
+          ref: main
+          repo: ${{ secrets.UPSTREAM_REPO }}
+          token: ${{ secrets.NC_GITHUB_TOKEN }}
+          inputs: '{ "tag": "${{ github.ref_name }}" }'
+
   trigger_android_bump:
     runs-on: ubuntu-latest
     if: github.event.created && !github.event.deleted && startsWith(github.ref, 'refs/tags/v') && !contains(github.ref_name, '-')

.github/workflows/test-infrastructure-files.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
   pull_request:
     paths:
       - "infrastructure_files/**"

.github/workflows/wasm-build-validation.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - main
-      - "release-*"
   pull_request:
 
 concurrency:

.goreleaser.yaml

@@ -468,13 +468,6 @@ checksum:
     - glob: ./infrastructure_files/migrate-to-enterprise.sh
 
 release:
-  # The signing pipeline (netbirdio/sign-pipelines, dispatched by
-  # trigger_signer) marks the release latest once the Windows and macOS
-  # artifacts are signed. Without this override goreleaser marks it latest
-  # at publish time, while those artifacts are still unsigned.
-  make_latest: false
-  # Mark x.y.z-rc.* and other prerelease tags as prereleases on GitHub.
-  prerelease: auto
   extra_files:
     - glob: ./infrastructure_files/getting-started-with-zitadel.sh
     - glob: ./release_files/install.sh

.goreleaser_ui.yaml

@@ -144,11 +144,3 @@ uploads:
     target: https://pkgs.wiretrustee.com/yum/{{ .Arch }}{{ if .Arm }}{{ .Arm }}{{ end }}
     username: dev@wiretrustee.com
     method: PUT
-
-release:
-  # Uploads into the release created by the main .goreleaser.yaml run.
-  # make_latest stays false everywhere: the signing pipeline
-  # (netbirdio/sign-pipelines) marks the release latest after the Windows
-  # and macOS artifacts are signed.
-  make_latest: false
-  prerelease: auto

.goreleaser_ui_darwin.yaml

@@ -43,11 +43,3 @@ checksum:
   name_template: "{{ .ProjectName }}_darwin_checksums.txt"
 changelog:
   disable: true
-
-release:
-  # Uploads into the release created by the main .goreleaser.yaml run.
-  # make_latest stays false everywhere: the signing pipeline
-  # (netbirdio/sign-pipelines) marks the release latest after the Windows
-  # and macOS artifacts are signed.
-  make_latest: false
-  prerelease: auto

.goreleaser_ui_gtk3.yaml

@@ -134,11 +134,3 @@ uploads:
     target: https://pkgs.wiretrustee.com/yum/{{ .Arch }}{{ if .Arm }}{{ .Arm }}{{ end }}
     username: dev@wiretrustee.com
     method: PUT
-
-release:
-  # Uploads into the release created by the main .goreleaser.yaml run.
-  # make_latest stays false everywhere: the signing pipeline
-  # (netbirdio/sign-pipelines) marks the release latest after the Windows
-  # and macOS artifacts are signed.
-  make_latest: false
-  prerelease: auto

client/cmd/login.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"os/user"
+	"runtime"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -120,7 +121,7 @@ func doDaemonLogin(ctx context.Context, cmd *cobra.Command, providedSetupKey str
 	loginRequest := proto.LoginRequest{
 		SetupKey:            providedSetupKey,
 		ManagementUrl:       managementURL,
-		IsUnixDesktopClient: util.HasGraphicalSession(),
+		IsUnixDesktopClient: isUnixRunningDesktop(),
 		Hostname:            hostName,
 		DnsLabels:           dnsLabelsReq,
 		ProfileName:         &handle,
@@ -188,8 +189,7 @@ func doExtendSession(ctx context.Context, cmd *cobra.Command) error {
 
 	client := proto.NewDaemonServiceClient(conn)
 
-	// the CLI runs in the user's session, the daemon does not: tell it what we can see
-	req := &proto.RequestExtendAuthSessionRequest{HasGraphicalSession: util.HasGraphicalSession()}
+	req := &proto.RequestExtendAuthSessionRequest{}
 	// Pre-fill the IdP login hint from the active profile so the user
 	// doesn't have to retype their email. Best-effort: we still proceed
 	// without a hint if the lookup fails.
@@ -408,9 +408,9 @@ func foregroundGetTokenInfo(ctx context.Context, cmd *cobra.Command, config *pro
 		hint = profileState.Email
 	}
 
-	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, util.HasGraphicalSession(), false, hint)
+	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, isUnixRunningDesktop(), false, hint)
 	if err != nil {
-		return nil, auth.WithSetupKeyAdvice(err)
+		return nil, err
 	}
 
 	flowInfo, err := oAuthFlow.RequestAuthInfo(context.TODO())
@@ -458,6 +458,14 @@ func openURL(cmd *cobra.Command, verificationURIComplete, userCode string, noBro
 	}
 }
 
+// isUnixRunningDesktop checks if a Linux OS is running desktop environment
+func isUnixRunningDesktop() bool {
+	if runtime.GOOS != "linux" && runtime.GOOS != "freebsd" {
+		return false
+	}
+	return os.Getenv("DESKTOP_SESSION") != "" || os.Getenv("XDG_CURRENT_DESKTOP") != ""
+}
+
 func setEnvAndFlags(cmd *cobra.Command) error {
 	SetFlagsFromEnvVars(rootCmd)
 

client/cmd/up.go

@@ -21,8 +21,8 @@ import (
 	"github.com/netbirdio/netbird/client/internal"
 	"github.com/netbirdio/netbird/client/internal/peer"
 	"github.com/netbirdio/netbird/client/internal/profilemanager"
-	nbnet "github.com/netbirdio/netbird/client/net"
 	"github.com/netbirdio/netbird/client/proto"
+	nbnet "github.com/netbirdio/netbird/client/net"
 	"github.com/netbirdio/netbird/client/server"
 	"github.com/netbirdio/netbird/client/system"
 	"github.com/netbirdio/netbird/shared/management/domain"
@@ -626,7 +626,7 @@ func setupLoginRequest(providedSetupKey string, customDNSAddressConverted []byte
 		NatExternalIPs:      natExternalIPs,
 		CleanNATExternalIPs: natExternalIPs != nil && len(natExternalIPs) == 0,
 		CustomDNSAddress:    customDNSAddressConverted,
-		IsUnixDesktopClient: util.HasGraphicalSession(),
+		IsUnixDesktopClient: isUnixRunningDesktop(),
 		Hostname:            hostName,
 		ExtraIFaceBlacklist: extraIFaceBlackList,
 		DnsLabels:           dnsLabels,

client/firewall/iptables/acl_linux.go

@@ -42,12 +42,12 @@ type aclManager struct {
 	optionalEntries map[string][]entry
 	ipsetStore      *ipsetStore
 	v6              bool
-	ipsetSupported  bool
+	ipsetSupport    *ipsetSupport
 
 	stateManager *statemanager.Manager
 }
 
-func newAclManager(iptablesClient *iptables.IPTables, wgIface iFaceMapper) (*aclManager, error) {
+func newAclManager(iptablesClient *iptables.IPTables, wgIface iFaceMapper, ipsetSupport *ipsetSupport) (*aclManager, error) {
 	return &aclManager{
 		iptablesClient:  iptablesClient,
 		wgIface:         wgIface,
@@ -55,14 +55,13 @@ func newAclManager(iptablesClient *iptables.IPTables, wgIface iFaceMapper) (*acl
 		optionalEntries: make(map[string][]entry),
 		ipsetStore:      newIpsetStore(),
 		v6:              iptablesClient.Proto() == iptables.ProtocolIPv6,
+		ipsetSupport:    ipsetSupport,
 	}, nil
 }
 
 func (m *aclManager) init(stateManager *statemanager.Manager) error {
 	m.stateManager = stateManager
 
-	m.ipsetSupported = m.probeIPSetSupport()
-
 	m.seedInitialEntries()
 	m.seedInitialOptionalEntries()
 
@@ -88,18 +87,77 @@ func (m *aclManager) AddPeerFiltering(
 	action firewall.Action,
 	ipsetName string,
 ) ([]firewall.Rule, error) {
-	chain := chainNameInputRules
+	ipsetName = m.resolveIPSetName(ipsetName, sPort, dPort, action)
+	if ipsetName == "" {
+		return m.addPeerRule(ip, protocol, sPort, dPort, action, "")
+	}
 
+	// A set that is already in the store backs rules installed earlier, so it must
+	// survive this call's failure.
+	_, preexisting := m.ipsetStore.ipset(ipsetName)
+
+	rules, err := m.addPeerRule(ip, protocol, sPort, dPort, action, ipsetName)
+	if err == nil {
+		return rules, nil
+	}
+
+	var unusable *ipsetUnusableError
+	if !errors.As(err, &unusable) {
+		return nil, err
+	}
+
+	// The set could not be created or matched. Drop the one this call created and
+	// retry the rule matching the IP directly; only if that succeeds do we know
+	// ipset was to blame and latch it off for subsequent rules.
+	if !preexisting {
+		m.discardIPSet(ipsetName)
+	}
+
+	rules, retryErr := m.addPeerRule(ip, protocol, sPort, dPort, action, "")
+	if retryErr != nil {
+		return nil, fmt.Errorf("add peer rule (ipset: %w): %w", unusable.cause, retryErr)
+	}
+
+	m.ipsetSupport.markUnsupported(unusable.cause)
+
+	return rules, nil
+}
+
+// resolveIPSetName derives the ipset name for a rule, returning "" when the rule
+// must match the IP directly: either no set was requested or ipset is unusable.
+func (m *aclManager) resolveIPSetName(ipsetName string, sPort, dPort *firewall.Port, action firewall.Action) string {
 	ipsetName = transformIPsetName(ipsetName, sPort, dPort, action)
-	if m.v6 && ipsetName != "" {
+	if ipsetName == "" || !m.ipsetSupport.supported() {
+		return ""
+	}
+
+	if m.v6 {
 		ipsetName += "-v6"
 	}
-	// When the kernel lacks the required ipset hash module, fall back to
-	// per-IP iptables rules (pre-0.68 behavior) so ACLs keep working instead
-	// of silently leaving the chain empty.
-	if ipsetName != "" && !m.ipsetSupported {
-		ipsetName = ""
+
+	return ipsetName
+}
+
+// discardIPSet removes a set that turned out to be unusable, so a later rule
+// does not find it in the store and assume it works.
+func (m *aclManager) discardIPSet(ipsetName string) {
+	m.ipsetStore.deleteIpset(ipsetName)
+
+	if err := m.destroyIPSet(ipsetName); err != nil {
+		log.Debugf("destroy unusable ipset %s: %v", ipsetName, err)
 	}
+}
+
+func (m *aclManager) addPeerRule(
+	ip net.IP,
+	protocol firewall.Protocol,
+	sPort *firewall.Port,
+	dPort *firewall.Port,
+	action firewall.Action,
+	ipsetName string,
+) ([]firewall.Rule, error) {
+	chain := chainNameInputRules
+
 	proto := protoForFamily(protocol, m.v6)
 	specs := filterRuleSpecs(ip, proto, sPort, dPort, action, ipsetName)
 
@@ -114,7 +172,7 @@ func (m *aclManager) AddPeerFiltering(
 	if ipsetName != "" {
 		if ipList, ipsetExists := m.ipsetStore.ipset(ipsetName); ipsetExists {
 			if err := m.addToIPSet(ipsetName, ip); err != nil {
-				return nil, fmt.Errorf("add IP to ipset: %w", err)
+				return nil, ipsetUnusable(fmt.Errorf("add IP to ipset: %w", err))
 			}
 			// if ruleset already exists it means we already have the firewall rule
 			// so we need to update IPs in the ruleset and return new fw.Rule object for ACL manager.
@@ -137,10 +195,10 @@ func (m *aclManager) AddPeerFiltering(
 			}
 		}
 		if err := m.createIPSet(ipsetName); err != nil {
-			return nil, fmt.Errorf("create ipset: %w", err)
+			return nil, ipsetUnusable(fmt.Errorf("create ipset: %w", err))
 		}
 		if err := m.addToIPSet(ipsetName, ip); err != nil {
-			return nil, fmt.Errorf("add IP to ipset: %w", err)
+			return nil, ipsetUnusable(fmt.Errorf("add IP to ipset: %w", err))
 		}
 
 		ipList := newIpList(ip.String())
@@ -149,7 +207,7 @@ func (m *aclManager) AddPeerFiltering(
 
 	ok, err := m.iptablesClient.Exists(tableFilter, chain, specs...)
 	if err != nil {
-		return nil, fmt.Errorf("failed to check rule: %w", err)
+		return nil, maybeIPSetUnusable(ipsetName, fmt.Errorf("check rule: %w", err))
 	}
 	if ok {
 		return nil, fmt.Errorf("rule already exists")
@@ -163,7 +221,7 @@ func (m *aclManager) AddPeerFiltering(
 		err = m.iptablesClient.Append(tableFilter, chain, specs...)
 	}
 	if err != nil {
-		return nil, err
+		return nil, maybeIPSetUnusable(ipsetName, err)
 	}
 
 	if err := m.iptablesClient.Append(tableMangle, chainRTPRE, mangleSpecs...); err != nil {
@@ -507,40 +565,6 @@ func transformIPsetName(ipsetName string, sPort, dPort *firewall.Port, action fi
 	}
 }
 
-// probeIPSetSupport checks whether the kernel can create the ipset type used for
-// ACL rules. On kernels lacking the required ipset hash module, ipset creation
-// fails (e.g. "invalid argument"), which would otherwise leave the ACL chain
-// empty and silently drop all policy-permitted inbound traffic. When unsupported,
-// the manager falls back to per-IP iptables rules.
-func (m *aclManager) probeIPSetSupport() bool {
-	// Use a unique name so concurrent processes don't collide and we only ever
-	// destroy the set we created ourselves. ipset names are limited to 31 chars,
-	// so use a short random suffix.
-	probeName := "nb-probe-" + uuid.New().String()[:8]
-
-	opts := ipset.CreateOptions{
-		Replace: true,
-	}
-	if m.v6 {
-		opts.Family = ipset.FamilyIPV6
-	}
-
-	if err := ipset.Create(probeName, ipset.TypeHashNet, opts); err != nil {
-		log.Warnf("ipset is not available (failed to create probe set: %v); "+
-			"falling back to per-IP iptables ACL rules. Ensure the kernel provides "+
-			"the ipset hash:net module (ip_set_hash_net) for better performance with large rule sets", err)
-		return false
-	}
-
-	defer func() {
-		if err := ipset.Destroy(probeName); err != nil {
-			log.Debugf("destroy ipset probe set %q: %v", probeName, err)
-		}
-	}()
-
-	return true
-}
-
 func (m *aclManager) createIPSet(name string) error {
 	opts := ipset.CreateOptions{
 		Replace: true,

client/firewall/iptables/ipset_support_linux.go

@@ -0,0 +1,75 @@
+package iptables
+
+import (
+	"sync"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// ipsetSupport tracks whether ipset-backed firewall rules can be installed.
+//
+// It starts optimistic and latches to unsupported the first time the kernel
+// proves otherwise: either the hash:net set type is missing (ip_set_hash_net) or
+// iptables cannot match against a set (xt_set). Callers then emit per-IP and
+// per-prefix rules instead. Without the fallback, a rule referencing an unusable
+// set is never installed and the catch-all DROP silently blocks traffic the
+// policy permits.
+//
+// One instance is shared by the ACL managers and routers of both address
+// families, because ipset availability is a property of the kernel rather than
+// of any single table.
+type ipsetSupport struct {
+	mu          sync.RWMutex
+	unsupported bool
+}
+
+func newIPSetSupport() *ipsetSupport {
+	return &ipsetSupport{}
+}
+
+func (s *ipsetSupport) supported() bool {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	return !s.unsupported
+}
+
+// markUnsupported records that ipset cannot be used, logging the reason once.
+func (s *ipsetSupport) markUnsupported(cause error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	if s.unsupported {
+		return
+	}
+	s.unsupported = true
+
+	log.Warnf("ipset is unavailable (%v); falling back to per-IP firewall rules. "+
+		"Ensure the kernel provides ip_set_hash_net and xt_set; without them rule "+
+		"sets are larger and slower to converge on networks with many peers", cause)
+}
+
+// ipsetUnusableError marks a failure attributable to ipset, so the caller can
+// retry the same rule in its per-IP form before latching the capability off.
+type ipsetUnusableError struct {
+	cause error
+}
+
+func (e *ipsetUnusableError) Error() string { return e.cause.Error() }
+
+func (e *ipsetUnusableError) Unwrap() error { return e.cause }
+
+func ipsetUnusable(cause error) error {
+	return &ipsetUnusableError{cause: cause}
+}
+
+// maybeIPSetUnusable marks an iptables failure as ipset-attributable only when the
+// rule actually carried a set match, since the same call can fail for unrelated
+// reasons on a rule that matches addresses directly.
+func maybeIPSetUnusable(ipsetName string, err error) error {
+	if ipsetName == "" {
+		return err
+	}
+
+	return ipsetUnusable(err)
+}

client/firewall/iptables/manager_linux.go

@@ -33,6 +33,10 @@ type Manager struct {
 	router       *router
 	rawSupported bool
 
+	// ipsetSupport is shared by the ACL managers and routers of both families,
+	// so a kernel without usable ipset support degrades them together.
+	ipsetSupport *ipsetSupport
+
 	// IPv6 counterparts, nil when no v6 overlay
 	ipv6Client *iptables.IPTables
 	aclMgr6    *aclManager
@@ -53,16 +57,17 @@ func Create(wgIface iFaceMapper, mtu uint16) (*Manager, error) {
 	}
 
 	m := &Manager{
-		wgIface:    wgIface,
-		ipv4Client: iptablesClient,
+		wgIface:      wgIface,
+		ipv4Client:   iptablesClient,
+		ipsetSupport: newIPSetSupport(),
 	}
 
-	m.router, err = newRouter(iptablesClient, wgIface, mtu)
+	m.router, err = newRouter(iptablesClient, wgIface, mtu, m.ipsetSupport)
 	if err != nil {
 		return nil, fmt.Errorf("create router: %w", err)
 	}
 
-	m.aclMgr, err = newAclManager(iptablesClient, wgIface)
+	m.aclMgr, err = newAclManager(iptablesClient, wgIface, m.ipsetSupport)
 	if err != nil {
 		return nil, fmt.Errorf("create acl manager: %w", err)
 	}
@@ -83,7 +88,7 @@ func (m *Manager) createIPv6Components(wgIface iFaceMapper, mtu uint16) error {
 	}
 	m.ipv6Client = ip6Client
 
-	m.router6, err = newRouter(ip6Client, wgIface, mtu)
+	m.router6, err = newRouter(ip6Client, wgIface, mtu, m.ipsetSupport)
 	if err != nil {
 		return fmt.Errorf("create v6 router: %w", err)
 	}
@@ -92,7 +97,7 @@ func (m *Manager) createIPv6Components(wgIface iFaceMapper, mtu uint16) error {
 	// Forwarding refcounter is per-family but shared between v4 and v6 routers.
 	m.router6.ipFwdState = m.router.ipFwdState
 
-	m.aclMgr6, err = newAclManager(ip6Client, wgIface)
+	m.aclMgr6, err = newAclManager(ip6Client, wgIface, m.ipsetSupport)
 	if err != nil {
 		return fmt.Errorf("create v6 acl manager: %w", err)
 	}

client/firewall/iptables/manager_linux_test.go

@@ -292,39 +292,97 @@ func TestIptablesCreatePerformance(t *testing.T) {
 	}
 }
 
-// TestIptablesACLIPSetFallback verifies that when the kernel lacks ipset support,
-// the ACL manager falls back to per-IP iptables rules (-s <ip>) instead of
-// silently leaving the chain empty. See discussion #6125.
-func TestIptablesACLIPSetFallback(t *testing.T) {
-	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
-	require.NoError(t, err)
+// newACLTestManager returns a started manager. Create()/Init() is used so the
+// router-owned chains (chainRTFWDIN/OUT) exist before the ACL manager's
+// createDefaultChains() references them.
+func newACLTestManager(t *testing.T) *Manager {
+	t.Helper()
 
-	// Use Create()/Init() so the router-owned chains (chainRTFWDIN/OUT) are
-	// created before the ACL manager's createDefaultChains() references them.
 	manager, err := Create(ifaceMock, iface.DefaultMTU)
 	require.NoError(t, err)
 	require.NoError(t, manager.Init(nil))
-
-	aclMgr := manager.aclMgr
-	// Simulate a kernel without the ipset hash module.
-	aclMgr.ipsetSupported = false
-
-	defer func() {
+	t.Cleanup(func() {
 		require.NoError(t, manager.Close(nil))
-	}()
+	})
+
+	return manager
+}
+
+// TestIptablesACLUsesIPSetOnHealthyKernel guards the default: on a kernel that
+// does have ipset, rules must keep matching a set. A regression that reported
+// ipset as unusable would silently move every Linux client to per-IP rules.
+func TestIptablesACLUsesIPSetOnHealthyKernel(t *testing.T) {
+	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	manager := newACLTestManager(t)
 
 	ip := netip.MustParseAddr("10.20.0.42")
 	port := &fw.Port{Values: []uint16{22}}
 
-	rules, err := aclMgr.AddPeerFiltering(nil, ip.AsSlice(), "tcp", nil, port, fw.ActionAccept, "nb0000001")
-	require.NoError(t, err, "AddPeerFiltering should succeed via fallback")
+	rules, err := manager.aclMgr.AddPeerFiltering(nil, ip.AsSlice(), "tcp", nil, port, fw.ActionAccept, "nb0000001")
+	require.NoError(t, err)
+	require.NotEmpty(t, rules)
+
+	rule := rules[0].(*Rule)
+	require.Equal(t, "nb0000001-dport", rule.ipsetName, "healthy kernel must use an ipset")
+	require.Contains(t, rule.specs, "--match-set")
+	require.True(t, manager.ipsetSupport.supported(), "ipset must not be latched off on a healthy kernel")
+
+	checkRuleSpecs(t, ipv4Client, rule.chain, true, rule.specs...)
+}
+
+// TestIptablesACLFallsBackWhenIPSetUnusable drives the real failure path: an
+// oversized set name is rejected by the kernel, which stands in for a kernel
+// without ip_set_hash_net or xt_set. The rule must still land in the chain,
+// matching the IP directly, and the capability must latch off so later rules skip
+// ipset. Before the fallback existed, the rule was dropped and the catch-all DROP
+// silently blocked traffic the policy permits.
+func TestIptablesACLFallsBackWhenIPSetUnusable(t *testing.T) {
+	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	manager := newACLTestManager(t)
+
+	// ipset names are limited to 31 characters, so creating this set fails.
+	unusableName := strings.Repeat("a", 40)
+
+	ip := netip.MustParseAddr("10.20.0.42")
+	port := &fw.Port{Values: []uint16{22}}
+
+	rules, err := manager.aclMgr.AddPeerFiltering(nil, ip.AsSlice(), "tcp", nil, port, fw.ActionAccept, unusableName)
+	require.NoError(t, err, "AddPeerFiltering must succeed by falling back")
 	require.NotEmpty(t, rules)
 
 	rule := rules[0].(*Rule)
 	require.Empty(t, rule.ipsetName, "fallback rule must not reference an ipset")
-	require.Contains(t, strings.Join(rule.specs, " "), "-s 10.20.0.42", "fallback rule must match by source IP")
-	require.NotContains(t, strings.Join(rule.specs, " "), "--match-set", "fallback rule must not use ipset matching")
+	require.Contains(t, strings.Join(rule.specs, " "), "-s 10.20.0.42", "fallback rule must match the source IP")
+	require.NotContains(t, strings.Join(rule.specs, " "), "--match-set")
 
-	// The rule must actually be present in the ACL chain (not silently dropped).
+	// The rule must actually be present, not silently missing.
 	checkRuleSpecs(t, ipv4Client, rule.chain, true, rule.specs...)
+
+	require.False(t, manager.ipsetSupport.supported(), "failure must latch ipset off")
+
+	// A subsequent rule with a perfectly valid set name now skips ipset too.
+	next, err := manager.aclMgr.AddPeerFiltering(nil, netip.MustParseAddr("10.20.0.43").AsSlice(), "tcp", nil, port, fw.ActionAccept, "nb0000001")
+	require.NoError(t, err)
+	require.NotEmpty(t, next)
+	require.Empty(t, next[0].(*Rule).ipsetName, "later rules must skip ipset once latched")
+}
+
+// TestIptablesACLLeavesNoIPSetAfterFallback verifies the set created before the
+// failure is destroyed, so a later rule does not find a half-built set and assume
+// ipset works.
+func TestIptablesACLLeavesNoIPSetAfterFallback(t *testing.T) {
+	manager := newACLTestManager(t)
+
+	port := &fw.Port{Values: []uint16{22}}
+	ip := netip.MustParseAddr("10.20.0.42")
+
+	_, err := manager.aclMgr.AddPeerFiltering(nil, ip.AsSlice(), "tcp", nil, port, fw.ActionAccept, strings.Repeat("a", 40))
+	require.NoError(t, err)
+
+	_, exists := manager.aclMgr.ipsetStore.ipset(strings.Repeat("a", 40) + "-dport")
+	require.False(t, exists, "unusable set must not stay in the store")
 }

client/firewall/iptables/router_linux.go

@@ -3,6 +3,7 @@
 package iptables
 
 import (
+	"errors"
 	"fmt"
 	"maps"
 	"net/netip"
@@ -51,6 +52,10 @@ const (
 	markManglePost = "mark-mangle-post"
 	matchSet       = "--match-set"
 
+	// routeSourceSuffix names the extra rules a route ACL needs when ipset is
+	// unusable and each source prefix has to be matched by its own rule.
+	routeSourceSuffix = "_src"
+
 	dnatSuffix = "_dnat"
 	snatSuffix = "_snat"
 	fwdSuffix  = "_fwd"
@@ -68,7 +73,6 @@ type ruleInfo struct {
 }
 
 type routeFilteringRuleParams struct {
-	Source      firewall.Network
 	Destination firewall.Network
 	Proto       firewall.Protocol
 	SPort       *firewall.Port
@@ -90,12 +94,13 @@ type router struct {
 	legacyManagement bool
 	mtu              uint16
 	v6               bool
+	ipsetSupport     *ipsetSupport
 
 	stateManager *statemanager.Manager
 	ipFwdState   *ipfwdstate.IPForwardingState
 }
 
-func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint16) (*router, error) {
+func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint16, ipsetSupport *ipsetSupport) (*router, error) {
 	r := &router{
 		iptablesClient: iptablesClient,
 		rules:          make(map[string][]string),
@@ -103,6 +108,7 @@ func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint1
 		mtu:            mtu,
 		v6:             iptablesClient.Proto() == iptables.ProtocolIPv6,
 		ipFwdState:     ipfwdstate.NewIPForwardingState(wgIface.Name()),
+		ipsetSupport:   ipsetSupport,
 	}
 
 	r.ipsetCounter = refcounter.New(
@@ -151,15 +157,7 @@ func (r *router) AddRouteFiltering(
 		return ruleKey, nil
 	}
 
-	var source firewall.Network
-	if len(sources) > 1 {
-		source.Set = firewall.NewPrefixSet(sources)
-	} else if len(sources) > 0 {
-		source.Prefix = sources[0]
-	}
-
 	params := routeFilteringRuleParams{
-		Source:      source,
 		Destination: destination,
 		Proto:       proto,
 		SPort:       sPort,
@@ -167,30 +165,156 @@ func (r *router) AddRouteFiltering(
 		Action:      action,
 	}
 
-	rule, err := r.genRouteRuleSpec(params, sources)
-	if err != nil {
-		return nil, fmt.Errorf("generate route rule spec: %w", err)
-	}
+	err := r.installRouteRules(string(ruleKey), params, sources, r.ipsetSupport.supported())
 
-	// Insert DROP rules at the beginning, append ACCEPT rules at the end
-	if action == firewall.ActionDrop {
-		// after the established rule
-		err = r.iptablesClient.Insert(tableFilter, chainRTFWDIN, 2, rule...)
-	} else {
-		err = r.iptablesClient.Append(tableFilter, chainRTFWDIN, rule...)
+	var unusable *ipsetUnusableError
+	if errors.As(err, &unusable) {
+		// The set could not be created or matched. Retry matching each source
+		// prefix on its own; only if that works do we know ipset was to blame.
+		r.removeRouteRules(string(ruleKey))
+
+		if retryErr := r.installRouteRules(string(ruleKey), params, sources, false); retryErr != nil {
+			r.removeRouteRules(string(ruleKey))
+			return nil, fmt.Errorf("add route rule (ipset: %w): %w", unusable.cause, retryErr)
+		}
+
+		r.ipsetSupport.markUnsupported(unusable.cause)
+		err = nil
 	}
 
 	if err != nil {
-		return nil, fmt.Errorf("add route rule: %v", err)
-	}
+		// Leave nothing half-installed: a later call finding the rule key would
+		// report success while some sources were never installed, which for a
+		// drop rule would leave them unblocked.
+		r.removeRouteRules(string(ruleKey))
 
-	r.rules[string(ruleKey)] = rule
+		return nil, fmt.Errorf("add route rule: %w", err)
+	}
 
 	r.updateState()
 
 	return ruleKey, nil
 }
 
+// installRouteRules installs every rule needed for one route ACL and records them
+// under ruleKey. It is more than one rule only when useIPSet is false and the
+// sources have to be matched one prefix at a time.
+func (r *router) installRouteRules(ruleKey string, params routeFilteringRuleParams, sources []netip.Prefix, useIPSet bool) error {
+	specs, err := r.genRouteRuleSpecs(params, sources, useIPSet)
+	if err != nil {
+		return fmt.Errorf("generate route rule spec: %w", err)
+	}
+
+	for i, spec := range specs {
+		if err := r.insertRouteRule(params.Action, spec); err != nil {
+			if len(r.findSets(spec)) > 0 {
+				return ipsetUnusable(err)
+			}
+			return err
+		}
+		r.rules[routeRuleKey(ruleKey, i)] = spec
+	}
+
+	return nil
+}
+
+// genRouteRuleSpecs builds the rules for one route ACL. With ipset available that
+// is a single rule matching a set of sources; without it, one rule per source
+// prefix, which is the only form a stripped kernel can express.
+func (r *router) genRouteRuleSpecs(params routeFilteringRuleParams, sources []netip.Prefix, useIPSet bool) ([][]string, error) {
+	destExp, err := r.applyNetwork("-d", params.Destination, nil)
+	if err != nil {
+		return nil, fmt.Errorf("apply network -d: %w", err)
+	}
+
+	specs, err := r.genSourceRules(params, sources, useIPSet, destExp)
+	if err != nil {
+		// The destination match may have taken a set reference already.
+		if decErr := r.decrementSetCounter(destExp); decErr != nil {
+			log.Debugf("release destination set after failed rule generation: %v", decErr)
+		}
+
+		return nil, err
+	}
+
+	return specs, nil
+}
+
+func (r *router) genSourceRules(params routeFilteringRuleParams, sources []netip.Prefix, useIPSet bool, destExp []string) ([][]string, error) {
+	if useIPSet || len(sources) <= 1 {
+		sourceExp, err := r.applyNetwork("-s", sourceNetwork(sources), sources)
+		if err != nil {
+			return nil, fmt.Errorf("apply network -s: %w", err)
+		}
+
+		return [][]string{assembleRouteRule(sourceExp, destExp, params, r.v6)}, nil
+	}
+
+	specs := make([][]string, 0, len(sources))
+	for _, source := range sources {
+		sourceExp, err := r.applyNetwork("-s", firewall.Network{Prefix: source}, nil)
+		if err != nil {
+			return nil, fmt.Errorf("apply network -s: %w", err)
+		}
+
+		specs = append(specs, assembleRouteRule(sourceExp, destExp, params, r.v6))
+	}
+
+	return specs, nil
+}
+
+func sourceNetwork(sources []netip.Prefix) firewall.Network {
+	var source firewall.Network
+	if len(sources) > 1 {
+		source.Set = firewall.NewPrefixSet(sources)
+	} else if len(sources) > 0 {
+		source.Prefix = sources[0]
+	}
+
+	return source
+}
+
+func (r *router) insertRouteRule(action firewall.Action, spec []string) error {
+	// Insert DROP rules at the beginning, append ACCEPT rules at the end
+	if action == firewall.ActionDrop {
+		// after the established rule
+		return r.iptablesClient.Insert(tableFilter, chainRTFWDIN, 2, spec...)
+	}
+
+	return r.iptablesClient.Append(tableFilter, chainRTFWDIN, spec...)
+}
+
+// removeRouteRules deletes the rules recorded for ruleKey, used to undo a partial
+// install before retrying without ipset.
+func (r *router) removeRouteRules(ruleKey string) {
+	for i := 0; ; i++ {
+		key := routeRuleKey(ruleKey, i)
+		spec, exists := r.rules[key]
+		if !exists {
+			return
+		}
+
+		if err := r.iptablesClient.DeleteIfExists(tableFilter, chainRTFWDIN, spec...); err != nil {
+			log.Debugf("delete partial route rule %s: %v", key, err)
+		}
+		delete(r.rules, key)
+
+		if err := r.decrementSetCounter(spec); err != nil {
+			log.Debugf("decrement ipset counter for %s: %v", key, err)
+		}
+	}
+}
+
+// routeRuleKey names the i-th rule of a route ACL. The first keeps the plain rule
+// key so single-rule ACLs, which is every ACL when ipset works, are unaffected.
+func routeRuleKey(ruleKey string, i int) string {
+	if i == 0 {
+		return ruleKey
+	}
+
+	return fmt.Sprintf("%s%s%d", ruleKey, routeSourceSuffix, i)
+}
+
 func (r *router) hasRule(id string) bool {
 	_, ok := r.rules[id]
 	return ok
@@ -199,17 +323,29 @@ func (r *router) hasRule(id string) bool {
 func (r *router) DeleteRouteRule(rule firewall.Rule) error {
 	ruleKey := rule.ID()
 
-	if rule, exists := r.rules[ruleKey]; exists {
+	if _, exists := r.rules[ruleKey]; !exists {
+		log.Debugf("route rule %s not found", ruleKey)
+		r.updateState()
+
+		return nil
+	}
+
+	// In the ipset fallback one ACL is several rules, one per source prefix.
+	for i := 0; ; i++ {
+		key := routeRuleKey(ruleKey, i)
+		rule, exists := r.rules[key]
+		if !exists {
+			break
+		}
+
 		if err := r.iptablesClient.Delete(tableFilter, chainRTFWDIN, rule...); err != nil {
 			return fmt.Errorf("delete route rule: %v", err)
 		}
-		delete(r.rules, ruleKey)
+		delete(r.rules, key)
 
 		if err := r.decrementSetCounter(rule); err != nil {
 			return fmt.Errorf("decrement ipset counter: %w", err)
 		}
-	} else {
-		log.Debugf("route rule %s not found", ruleKey)
 	}
 
 	r.updateState()
@@ -927,31 +1063,23 @@ func (r *router) DeleteDNATRule(rule firewall.Rule) error {
 	return nberrors.FormatErrorOrNil(merr)
 }
 
-func (r *router) genRouteRuleSpec(params routeFilteringRuleParams, sources []netip.Prefix) ([]string, error) {
+// assembleRouteRule joins the pre-built source and destination matches with the
+// protocol, ports and target of a route ACL.
+func assembleRouteRule(sourceExp, destExp []string, params routeFilteringRuleParams, v6 bool) []string {
 	var rule []string
 
-	sourceExp, err := r.applyNetwork("-s", params.Source, sources)
-	if err != nil {
-		return nil, fmt.Errorf("apply network -s: %w", err)
-
-	}
-	destExp, err := r.applyNetwork("-d", params.Destination, nil)
-	if err != nil {
-		return nil, fmt.Errorf("apply network -d: %w", err)
-	}
-
 	rule = append(rule, sourceExp...)
 	rule = append(rule, destExp...)
 
 	if params.Proto != firewall.ProtocolALL {
-		rule = append(rule, "-p", strings.ToLower(protoForFamily(params.Proto, r.v6)))
+		rule = append(rule, "-p", strings.ToLower(protoForFamily(params.Proto, v6)))
 		rule = append(rule, applyPort("--sport", params.SPort)...)
 		rule = append(rule, applyPort("--dport", params.DPort)...)
 	}
 
 	rule = append(rule, "-j", actionToStr(params.Action))
 
-	return rule, nil
+	return rule
 }
 
 func (r *router) applyNetwork(flag string, network firewall.Network, prefixes []netip.Prefix) ([]string, error) {
@@ -961,9 +1089,17 @@ func (r *router) applyNetwork(flag string, network firewall.Network, prefixes []
 	}
 
 	if network.IsSet() {
+		// A destination set is populated later from DNS results, so unlike a
+		// source set it cannot be expanded into per-prefix rules here. Without
+		// ipset such a rule is not expressible; report it instead of installing
+		// something broader than the policy allows.
+		if flag == "-d" && !r.ipsetSupport.supported() {
+			return nil, fmt.Errorf("destination set %s requires ipset (ip_set_hash_net and xt_set)", network.Set.HashedName())
+		}
+
 		name := r.ipsetName(network.Set.HashedName())
 		if _, err := r.ipsetCounter.Increment(name, prefixes); err != nil {
-			return nil, fmt.Errorf("create or get ipset: %w", err)
+			return nil, ipsetUnusable(fmt.Errorf("create or get ipset: %w", err))
 		}
 
 		return []string{"-m", "set", matchSet, name, direction}, nil

client/firewall/iptables/router_linux_test.go

@@ -3,9 +3,11 @@
 package iptables
 
 import (
+	"errors"
 	"fmt"
 	"net/netip"
 	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/coreos/go-iptables/iptables"
@@ -15,7 +17,9 @@ import (
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 	"github.com/netbirdio/netbird/client/firewall/test"
 	"github.com/netbirdio/netbird/client/iface"
+	nbid "github.com/netbirdio/netbird/client/internal/acl/id"
 	nbnet "github.com/netbirdio/netbird/client/net"
+	"github.com/netbirdio/netbird/shared/management/domain"
 )
 
 func isIptablesSupported() bool {
@@ -31,7 +35,7 @@ func TestIptablesManager_RestoreOrCreateContainers(t *testing.T) {
 	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 	require.NoError(t, err, "failed to init iptables client")
 
-	manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
+	manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, newIPSetSupport())
 	require.NoError(t, err, "should return a valid iptables manager")
 	require.NoError(t, manager.init(nil))
 
@@ -84,7 +88,7 @@ func TestIptablesManager_AddNatRule(t *testing.T) {
 			iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 			require.NoError(t, err, "failed to init iptables client")
 
-			manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
+			manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, newIPSetSupport())
 			require.NoError(t, err, "shouldn't return error")
 			require.NoError(t, manager.init(nil))
 
@@ -157,7 +161,7 @@ func TestIptablesManager_RemoveNatRule(t *testing.T) {
 		t.Run(testCase.Name, func(t *testing.T) {
 			iptablesClient, _ := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 
-			manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
+			manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, newIPSetSupport())
 			require.NoError(t, err, "shouldn't return error")
 			require.NoError(t, manager.init(nil))
 			defer func() {
@@ -219,7 +223,7 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 	require.NoError(t, err, "Failed to create iptables client")
 
-	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
+	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, newIPSetSupport())
 	require.NoError(t, err, "Failed to create router manager")
 	require.NoError(t, r.init(nil))
 
@@ -337,27 +341,26 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 			ruleKey, err := r.AddRouteFiltering(nil, tt.sources, firewall.Network{Prefix: tt.destination}, tt.proto, tt.sPort, tt.dPort, tt.action)
 			require.NoError(t, err, "AddRouteFiltering failed")
 
-			// Check if the rule is in the internal map
-			rule, ok := r.rules[ruleKey.ID()]
-			assert.True(t, ok, "Rule not found in internal map")
+			// A kernel without usable ipset splits a multi-source ACL into one
+			// rule per source, so compare against whichever form is in effect.
+			useIPSet := r.ipsetSupport.supported()
 
-			// Log the internal rule
-			t.Logf("Internal rule: %v", rule)
+			// Check if the rules are in the internal map
+			rules := routeRuleSpecs(t, r, ruleKey.ID())
+			require.NotEmpty(t, rules, "Rule not found in internal map")
 
-			// Check if the rule exists in iptables
-			exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, rule...)
-			assert.NoError(t, err, "Failed to check rule existence")
-			assert.True(t, exists, "Rule not found in iptables")
+			// Log the internal rules
+			t.Logf("Internal rules: %v", rules)
 
-			var source firewall.Network
-			if len(tt.sources) > 1 {
-				source.Set = firewall.NewPrefixSet(tt.sources)
-			} else if len(tt.sources) > 0 {
-				source.Prefix = tt.sources[0]
+			// Check if the rules exist in iptables
+			for _, rule := range rules {
+				exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, rule...)
+				assert.NoError(t, err, "Failed to check rule existence")
+				assert.True(t, exists, "Rule not found in iptables")
 			}
+
 			// Verify rule content
 			params := routeFilteringRuleParams{
-				Source:      source,
 				Destination: firewall.Network{Prefix: tt.destination},
 				Proto:       tt.proto,
 				SPort:       tt.sPort,
@@ -365,20 +368,18 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 				Action:      tt.action,
 			}
 
-			expectedRule, err := r.genRouteRuleSpec(params, nil)
+			expectedRules, err := r.genRouteRuleSpecs(params, tt.sources, useIPSet)
 			require.NoError(t, err, "Failed to generate expected rule spec")
 
-			if tt.expectSet {
+			if tt.expectSet && useIPSet {
 				setName := firewall.NewPrefixSet(tt.sources).HashedName()
-				expectedRule, err = r.genRouteRuleSpec(params, nil)
-				require.NoError(t, err, "Failed to generate expected rule spec with set")
 
 				// Check if the set was created
 				_, exists := r.ipsetCounter.Get(setName)
 				assert.True(t, exists, "IPSet not created")
 			}
 
-			assert.Equal(t, expectedRule, rule, "Rule content mismatch")
+			assert.Equal(t, expectedRules, rules, "Rule content mismatch")
 
 			// Clean up
 			err = r.DeleteRouteRule(ruleKey)
@@ -445,3 +446,145 @@ func TestFindSetNameInRule(t *testing.T) {
 		})
 	}
 }
+
+// TestRouter_AddRouteFilteringIPSetFallback covers a kernel that cannot use ipset:
+// a multi-source route ACL must become one rule per source prefix, all present in
+// the chain, and deleting the ACL must remove every one of them. Without the
+// fallback the rule was never installed and the interface-wide DROP in FORWARD
+// silently dropped routed traffic.
+func TestRouter_AddRouteFilteringIPSetFallback(t *testing.T) {
+	if !isIptablesSupported() {
+		t.Skip("iptables not supported on this system")
+	}
+
+	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	support := newIPSetSupport()
+	support.markUnsupported(errors.New("test: pretend the kernel has no ipset"))
+
+	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, support)
+	require.NoError(t, err)
+	require.NoError(t, r.init(nil))
+	t.Cleanup(func() {
+		require.NoError(t, r.Reset())
+	})
+
+	sources := []netip.Prefix{
+		netip.MustParsePrefix("172.16.0.0/16"),
+		netip.MustParsePrefix("192.168.0.0/16"),
+	}
+	destination := firewall.Network{Prefix: netip.MustParsePrefix("10.0.0.0/8")}
+
+	rule, err := r.AddRouteFiltering(nil, sources, destination, firewall.ProtocolTCP, nil,
+		&firewall.Port{Values: []uint16{443}}, firewall.ActionAccept)
+	require.NoError(t, err, "route ACL must install without ipset")
+
+	specs := routeRuleSpecs(t, r, rule.ID())
+	require.Len(t, specs, len(sources), "each source prefix needs its own rule")
+
+	for i, spec := range specs {
+		joined := strings.Join(spec, " ")
+		require.Contains(t, joined, "-s "+sources[i].String(), "rule must match the source prefix directly")
+		require.NotContains(t, joined, matchSet, "fallback rule must not reference a set")
+
+		exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, spec...)
+		require.NoError(t, err)
+		require.True(t, exists, "rule %d must be present in %s", i, chainRTFWDIN)
+	}
+
+	require.NoError(t, r.DeleteRouteRule(rule))
+
+	for i, spec := range specs {
+		exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, spec...)
+		require.NoError(t, err)
+		require.False(t, exists, "rule %d must be removed", i)
+	}
+	require.Empty(t, routeRuleSpecs(t, r, rule.ID()), "no rule may be left recorded")
+}
+
+// TestRouter_DestinationSetRequiresIPSet documents that a dynamic (domain)
+// destination cannot be expressed without ipset: its prefixes are only known
+// after DNS resolution, so there is nothing to expand into per-prefix rules. The
+// call must report that rather than install a broader rule than the policy allows.
+func TestRouter_DestinationSetRequiresIPSet(t *testing.T) {
+	if !isIptablesSupported() {
+		t.Skip("iptables not supported on this system")
+	}
+
+	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	support := newIPSetSupport()
+	support.markUnsupported(errors.New("test: pretend the kernel has no ipset"))
+
+	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, support)
+	require.NoError(t, err)
+	require.NoError(t, r.init(nil))
+	t.Cleanup(func() {
+		require.NoError(t, r.Reset())
+	})
+
+	destination := firewall.Network{Set: firewall.NewDomainSet(domain.List{"example.com"})}
+
+	_, err = r.AddRouteFiltering(nil, []netip.Prefix{netip.MustParsePrefix("172.16.0.0/16")},
+		destination, firewall.ProtocolALL, nil, nil, firewall.ActionAccept)
+	require.Error(t, err, "a domain destination is not expressible without ipset")
+	require.ErrorContains(t, err, "requires ipset")
+}
+
+// TestRouter_RouteFilteringRollsBackPartialInstall covers a fallback ACL whose
+// second rule cannot be installed. Nothing may be left behind: if the rule key
+// survived, a later call would short-circuit on it and report success while some
+// sources were never installed, leaving them unblocked for a drop rule.
+func TestRouter_RouteFilteringRollsBackPartialInstall(t *testing.T) {
+	if !isIptablesSupported() {
+		t.Skip("iptables not supported on this system")
+	}
+
+	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	support := newIPSetSupport()
+	support.markUnsupported(errors.New("test: pretend the kernel has no ipset"))
+
+	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, support)
+	require.NoError(t, err)
+	require.NoError(t, r.init(nil))
+	t.Cleanup(func() {
+		require.NoError(t, r.Reset())
+	})
+
+	// The v6 prefix is rejected by the v4 iptables binary, so the second rule of
+	// the expansion fails after the first has been installed.
+	good := netip.MustParsePrefix("172.16.0.0/16")
+	sources := []netip.Prefix{good, netip.MustParsePrefix("2001:db8::/32")}
+	destination := firewall.Network{Prefix: netip.MustParsePrefix("10.0.0.0/8")}
+
+	_, err = r.AddRouteFiltering(nil, sources, destination, firewall.ProtocolALL, nil, nil, firewall.ActionDrop)
+	require.Error(t, err, "a source that iptables rejects must fail the whole ACL")
+
+	ruleKey := nbid.GenerateRouteRuleKey(sources, destination, firewall.ProtocolALL, nil, nil, firewall.ActionDrop)
+	require.Empty(t, routeRuleSpecs(t, r, string(ruleKey)), "no rule may stay recorded")
+
+	// The rule that did get installed must be gone from the chain.
+	installed := []string{"-s", good.String(), "-d", "10.0.0.0/8", "-j", "DROP"}
+	exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, installed...)
+	require.NoError(t, err)
+	require.False(t, exists, "the already-installed rule must be rolled back")
+}
+
+// routeRuleSpecs collects the rules recorded for one route ACL, which is more than
+// one when the ipset fallback splits it per source prefix.
+func routeRuleSpecs(t *testing.T, r *router, ruleKey string) [][]string {
+	t.Helper()
+
+	var specs [][]string
+	for i := 0; ; i++ {
+		spec, exists := r.rules[routeRuleKey(ruleKey, i)]
+		if !exists {
+			return specs
+		}
+		specs = append(specs, spec)
+	}
+}

client/installer.nsis

@@ -22,6 +22,8 @@
 !define UI_REG_APP_PATH "Software\Microsoft\Windows\CurrentVersion\App Paths\${UI_APP_EXE}"
 !define UI_UNINSTALL_PATH "Software\Microsoft\Windows\CurrentVersion\Uninstall\${UI_APP_NAME}"
 
+!define AUTOSTART_REG_KEY "Software\Microsoft\Windows\CurrentVersion\Run"
+
 !define NETBIRD_DATA_DIR "$COMMONPROGRAMDATA\Netbird"
 
 Unicode True
@@ -226,6 +228,13 @@ WriteRegStr ${REG_ROOT} "${UNINSTALL_PATH}"  "Publisher" "${COMP_NAME}"
 
 WriteRegStr ${REG_ROOT} "${UI_REG_APP_PATH}" "" "$INSTDIR\${UI_APP_EXE}"
 
+; Autostart is owned by the UI's per-user setting (HKCU\...\Run via Wails),
+; not the installer. Drop the machine-wide entry older installers wrote so the
+; toggle is the single source of truth. HKCU is left untouched -- it may hold
+; the user's own toggle state, which must survive upgrades.
+DetailPrint "Removing installer-managed autostart registry entry if present..."
+DeleteRegValue HKLM "${AUTOSTART_REG_KEY}" "${APP_NAME}"
+
 EnVar::SetHKLM
 EnVar::AddValueEx "path" "$INSTDIR"
 
@@ -290,6 +299,15 @@ ExecWait '"$INSTDIR\${MAIN_APP_EXE}" service uninstall'
 DetailPrint "Terminating Netbird UI process..."
 ExecWait `taskkill /im ${UI_APP_EXE}.exe /f`
 
+; Remove autostart registry entries
+DetailPrint "Removing autostart registry entries if they exist..."
+; Legacy machine-wide entry written by older installers.
+DeleteRegValue HKLM "${AUTOSTART_REG_KEY}" "${APP_NAME}"
+; Per-user entry the UI toggle writes via Wails (value name is the lowercase
+; app-name slug). Uninstall removes the app, so drop it too.
+DeleteRegValue HKCU "${AUTOSTART_REG_KEY}" "${APP_NAME}"
+DeleteRegValue HKCU "${AUTOSTART_REG_KEY}" "netbird"
+
 ; Handle data deletion based on checkbox
 DetailPrint "Checking if user requested data deletion..."
 ${If} $DeleteDataEnabled == "1"

client/internal/auth/auth.go

@@ -83,15 +83,6 @@ func NewAuth(ctx context.Context, privateKey string, mgmURL *url.URL, config *pr
 	}, nil
 }
 
-// grpcClient returns the current management connection. Callers must go through it rather than
-// reading a.client: reconnect replaces that field while other goroutines are using it.
-func (a *Auth) grpcClient() *mgm.GrpcClient {
-	a.mutex.RLock()
-	defer a.mutex.RUnlock()
-
-	return a.client
-}
-
 // Close closes the management client connection
 func (a *Auth) Close() error {
 	a.mutex.Lock()
@@ -149,20 +140,25 @@ func (a *Auth) IsSSOSupported(ctx context.Context) (bool, error) {
 // This avoids creating a new connection to the management server
 func (a *Auth) GetOAuthFlow(ctx context.Context, forceDeviceAuth bool) (OAuthFlow, error) {
 	var flow OAuthFlow
+	var err error
 
-	// the connection is owned by a and outlives this call, so a later fallback reuses it
-	newAuth := func(context.Context) (*Auth, func(), error) {
-		return a, func() {}, nil
-	}
-
-	err := a.withRetry(ctx, func(client *mgm.GrpcClient) error {
-		var err error
-		flow, err = oauthFlowWithFallback(a, client, flowOrder(forceDeviceAuth, true), "", newAuth)
-
-		if IsSSOUnavailable(err) {
-			return backoff.Permanent(err)
+	err = a.withRetry(ctx, func(client *mgm.GrpcClient) error {
+		if forceDeviceAuth {
+			flow, err = a.getDeviceFlow(client)
+			return err
 		}
-		return err
+
+		// Try PKCE flow first
+		flow, err = a.getPKCEFlow(client)
+		if err != nil {
+			// If PKCE not supported, try Device flow
+			if s, ok := status.FromError(err); ok && (s.Code() == codes.NotFound || s.Code() == codes.Unimplemented) {
+				flow, err = a.getDeviceFlow(client)
+				return err
+			}
+			return err
+		}
+		return nil
 	})
 
 	return flow, err

client/internal/auth/device_flow.go

@@ -48,17 +48,8 @@ type DeviceAuthProviderConfig struct {
 	LoginHint string
 }
 
-// validateDeviceAuthConfig validates device authorization provider configuration. A missing
-// value means management does not have this flow configured, so the error wraps
-// errFlowNotConfigured and the caller can fall back to the other flow.
+// validateDeviceAuthConfig validates device authorization provider configuration
 func validateDeviceAuthConfig(config *DeviceAuthProviderConfig) error {
-	if err := checkDeviceAuthConfig(config); err != nil {
-		return fmt.Errorf("%w: %w", errFlowNotConfigured, err)
-	}
-	return nil
-}
-
-func checkDeviceAuthConfig(config *DeviceAuthProviderConfig) error {
 	errorMsgFormat := "invalid provider configuration received from management: %s value is empty. Contact your NetBird administrator"
 
 	if config.Audience == "" {
@@ -170,12 +161,8 @@ func (d *DeviceAuthorizationFlow) RequestAuthInfo(ctx context.Context) (AuthFlow
 		return AuthFlowInfo{}, fmt.Errorf("reading body failed with error: %v", err)
 	}
 
-	if res.StatusCode != http.StatusOK {
-		reqErr := fmt.Errorf("request device code returned status %d error: %s", res.StatusCode, string(body))
-		if deviceGrantUnsupported(res.StatusCode, body) {
-			return AuthFlowInfo{}, fmt.Errorf("%w: %w", errFlowNotConfigured, reqErr)
-		}
-		return AuthFlowInfo{}, reqErr
+	if res.StatusCode != 200 {
+		return AuthFlowInfo{}, fmt.Errorf("request device code returned status %d error: %s", res.StatusCode, string(body))
 	}
 
 	deviceCode := AuthFlowInfo{}
@@ -199,34 +186,6 @@ func (d *DeviceAuthorizationFlow) RequestAuthInfo(ctx context.Context) (AuthFlow
 	return deviceCode, err
 }
 
-// deviceGrantUnsupported reports whether the IdP's answer to a device code request means it does
-// not serve the device authorization grant at all, rather than a transient or request-specific
-// failure. An IdP that does not route the endpoint answers 404/405/501; one that knows the
-// endpoint but has the grant disabled for this client answers with an OAuth 2.0 error code.
-func deviceGrantUnsupported(statusCode int, body []byte) bool {
-	switch statusCode {
-	case http.StatusNotFound, http.StatusMethodNotAllowed, http.StatusNotImplemented:
-		return true
-	case http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden:
-	default:
-		return false
-	}
-
-	var oauthErr struct {
-		Error string `json:"error"`
-	}
-	if err := json.Unmarshal(body, &oauthErr); err != nil {
-		return false
-	}
-
-	switch oauthErr.Error {
-	case "unsupported_grant_type", "unauthorized_client":
-		return true
-	default:
-		return false
-	}
-}
-
 func appendLoginHint(uri, loginHint string) string {
 	if uri == "" || loginHint == "" {
 		return uri

client/internal/auth/oauth.go

@@ -2,19 +2,15 @@ package auth
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/http"
-	"net/url"
 	"runtime"
-	"sync"
 
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
 	gstatus "google.golang.org/grpc/status"
 
 	"github.com/netbirdio/netbird/client/internal/profilemanager"
-	mgm "github.com/netbirdio/netbird/shared/management/client"
 )
 
 // OAuthFlow represents an interface for authorization using different OAuth 2.0 flows
@@ -63,327 +59,77 @@ func (t TokenInfo) GetTokenToUse() string {
 	return t.AccessToken
 }
 
-// errFlowNotConfigured marks a flow this deployment does not offer: management returned no
-// configuration for it, the configuration it returned is incomplete, or the IdP refuses to serve
-// the grant. It is the only condition that makes the client try the other flow, so that a
-// transient failure keeps failing on the flow the user actually wants.
-var errFlowNotConfigured = errors.New("authorization flow is not configured")
-
-// ssoUnavailableError reports that the management server offers no usable SSO flow at all.
-// Retrying cannot help, so callers should surface it to the user instead of backing off.
-type ssoUnavailableError struct {
-	msg string
+func shouldUseDeviceFlow(force bool, isUnixDesktopClient bool) bool {
+	return force || (runtime.GOOS == "linux" || runtime.GOOS == "freebsd") && !isUnixDesktopClient
 }
 
-func (e *ssoUnavailableError) Error() string {
-	return e.msg
-}
-
-// oauthFlowInit names one of the OAuth flows and builds it from the management configuration.
-type oauthFlowInit struct {
-	name string
-	init func(a *Auth, client *mgm.GrpcClient, hint string) (OAuthFlow, error)
-}
-
-// authFactory hands out a management connection to build a flow with, plus the cleanup that
-// releases it. Callers that own a long-lived connection return it with a no-op cleanup.
-type authFactory func(ctx context.Context) (*Auth, func(), error)
-
-// loginHintSetter is implemented by both concrete flows but is deliberately not part of
-// OAuthFlow, so callers reach it through a type assertion.
-type loginHintSetter interface {
-	SetLoginHint(hint string)
-}
-
-// fallbackFlow wraps the flow that was picked at initialization time with the flows that were
-// not tried. Whether the IdP actually serves a flow only shows up when the flow is run: an IdP
-// with the device grant disabled answers the device code request with 404 even though
-// management handed out a device flow configuration. When that happens the wrapper swaps in the
-// next flow instead of failing the login.
-type fallbackFlow struct {
-	mu        sync.Mutex
-	active    OAuthFlow
-	remaining []oauthFlowInit
-	hint      string
-	newAuth   authFactory
-}
-
-func (f *fallbackFlow) RequestAuthInfo(ctx context.Context) (AuthFlowInfo, error) {
-	info, err := f.current().RequestAuthInfo(ctx)
-	if err == nil || !isFlowUnavailable(err) {
-		return info, err
-	}
-
-	next, nextErr := f.initNext(ctx)
-	if nextErr != nil {
-		log.Debugf("failed to fall back to another authorization flow: %v", nextErr)
-		return AuthFlowInfo{}, err
-	}
-
-	return next.RequestAuthInfo(ctx)
-}
-
-func (f *fallbackFlow) WaitToken(ctx context.Context, info AuthFlowInfo) (TokenInfo, error) {
-	return f.current().WaitToken(ctx, info)
-}
-
-func (f *fallbackFlow) GetClientID(ctx context.Context) string {
-	return f.current().GetClientID(ctx)
-}
-
-// SetLoginHint forwards the hint to the active flow and keeps it for a flow a later fallback
-// initializes. Callers that set the hint after building the flow reach the concrete flow through
-// a type assertion, which the OAuthFlow interface does not carry, so the wrapper has to offer it
-// too or the hint is silently dropped.
-func (f *fallbackFlow) SetLoginHint(hint string) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	f.hint = hint
-	if setter, ok := f.active.(loginHintSetter); ok {
-		setter.SetLoginHint(hint)
-	}
-}
-
-func (f *fallbackFlow) current() OAuthFlow {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	return f.active
-}
-
-// initNext initializes the next flow this deployment offers and makes it the active one.
-func (f *fallbackFlow) initNext(ctx context.Context) (OAuthFlow, error) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	if len(f.remaining) == 0 {
-		return nil, errors.New("no authorization flow left to try")
-	}
-
-	a, cleanup, err := f.newAuth(ctx)
-	if err != nil {
-		return nil, err
-	}
-	defer cleanup()
-
-	flow, remaining, err := initFirstAvailableFlow(a, a.grpcClient(), f.remaining, f.hint)
-	if err != nil {
-		return nil, err
-	}
-
-	log.Infof("the identity provider does not serve the selected authorization flow, continuing with the next one")
-	f.active = flow
-	f.remaining = remaining
-
-	return flow, nil
-}
-
-// preferDeviceFlow reports whether the device code flow should be tried before PKCE. PKCE needs
-// a browser on this host and a loopback listener to receive the redirect, neither of which
-// exists on a Unix host without a graphical session. The GOOS guard keeps a caller that reports
-// no graphical session on a platform that always has one from changing the preference.
-func preferDeviceFlow(hasGraphicalSession bool) bool {
-	return (runtime.GOOS == "linux" || runtime.GOOS == "freebsd") && !hasGraphicalSession
-}
-
-// flowOrder returns the flows to attempt, in order.
+// NewOAuthFlow initializes and returns the appropriate OAuth flow based on the management configuration
 //
-// force leaves the device code flow on its own rather than first: it marks a device with no
-// browser at all, such as Android TV or tvOS. PKCE cannot work there even from another device,
-// because the redirect has to arrive on the loopback listener of the device being enrolled, so
-// offering it as a fallback would only replace a clear error with a login that cannot complete.
-func flowOrder(force bool, hasGraphicalSession bool) []oauthFlowInit {
-	pkce := oauthFlowInit{name: "pkce authorization flow", init: initPKCEFlow}
-	device := oauthFlowInit{name: "device code flow", init: initDeviceFlow}
-
-	switch {
-	case force:
-		return []oauthFlowInit{device}
-	case preferDeviceFlow(hasGraphicalSession):
-		return []oauthFlowInit{device, pkce}
-	default:
-		return []oauthFlowInit{pkce, device}
-	}
-}
-
-func initPKCEFlow(a *Auth, client *mgm.GrpcClient, hint string) (OAuthFlow, error) {
-	flow, err := a.getPKCEFlow(client)
-	if err != nil {
-		return nil, err
-	}
-
-	if hint != "" {
-		flow.SetLoginHint(hint)
-	}
-
-	return flow, nil
-}
-
-func initDeviceFlow(a *Auth, client *mgm.GrpcClient, hint string) (OAuthFlow, error) {
-	flow, err := a.getDeviceFlow(client)
-	if err != nil {
-		return nil, err
-	}
-
-	if hint != "" {
-		flow.SetLoginHint(hint)
-	}
-
-	return flow, nil
-}
-
-// NewOAuthFlow initializes and returns an OAuth flow based on the management configuration.
+// It starts by initializing the PKCE.If this process fails, it resorts to the Device Code Flow,
+// and if that also fails, the authentication process is deemed unsuccessful
 //
-// Both flows are optional server side: management answers NotFound for a flow it has no
-// configuration for. The preferred flow is tried first and the other one is used as a fallback,
-// so a server that only offers one of them still works. forceDeviceCodeFlow restricts the client
-// to the device code flow with no fallback, for a device that has no browser at all.
-func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, hasGraphicalSession bool, forceDeviceCodeFlow bool, hint string) (OAuthFlow, error) {
+// On Linux distros without desktop environment support, it only tries to initialize the Device Code Flow
+// forceDeviceCodeFlow can be used to skip PKCE and go directly to Device Code Flow (e.g., for Android TV)
+func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesktopClient bool, forceDeviceCodeFlow bool, hint string) (OAuthFlow, error) {
+	if shouldUseDeviceFlow(forceDeviceCodeFlow, isUnixDesktopClient) {
+		return authenticateWithDeviceCodeFlow(ctx, config, hint)
+	}
+
+	pkceFlow, err := authenticateWithPKCEFlow(ctx, config, hint)
+	if err != nil {
+		log.Debugf("failed to initialize pkce authentication with error: %v\n", err)
+		log.Debug("falling back to device code flow")
+		return authenticateWithDeviceCodeFlow(ctx, config, hint)
+	}
+	return pkceFlow, nil
+}
+
+// authenticateWithPKCEFlow initializes the Proof Key for Code Exchange flow auth flow
+func authenticateWithPKCEFlow(ctx context.Context, config *profilemanager.Config, hint string) (OAuthFlow, error) {
 	authClient, err := NewAuth(ctx, config.PrivateKey, config.ManagementURL, config)
 	if err != nil {
-		return nil, fmt.Errorf("create auth client: %w", err)
+		return nil, fmt.Errorf("failed to create auth client: %v", err)
 	}
-	defer func() {
-		if err := authClient.Close(); err != nil {
-			log.Debugf("failed to close auth client: %v", err)
-		}
-	}()
+	defer authClient.Close()
 
-	// the connection above is closed on return, so a later fallback opens its own
-	newAuth := func(ctx context.Context) (*Auth, func(), error) {
-		a, err := NewAuth(ctx, config.PrivateKey, config.ManagementURL, config)
-		if err != nil {
-			return nil, nil, fmt.Errorf("create auth client: %w", err)
-		}
-		return a, func() {
-			if err := a.Close(); err != nil {
-				log.Debugf("failed to close auth client: %v", err)
-			}
-		}, nil
-	}
-
-	flows := flowOrder(forceDeviceCodeFlow, hasGraphicalSession)
-	return oauthFlowWithFallback(authClient, authClient.grpcClient(), flows, hint, newAuth)
-}
-
-// oauthFlowWithFallback initializes the first flow this deployment offers, moving on to the next
-// one when a flow is not configured here. It only fails once every flow has been tried, and any
-// flow left untried is handed to the returned flow so it can still fall back if the IdP rejects
-// the flow that was picked.
-func oauthFlowWithFallback(a *Auth, client *mgm.GrpcClient, flows []oauthFlowInit, hint string, newAuth authFactory) (OAuthFlow, error) {
-	flow, remaining, err := initFirstAvailableFlow(a, client, flows, hint)
+	pkceFlowInfo, err := authClient.getPKCEFlow(authClient.client)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("getting pkce authorization flow info failed with error: %v", err)
 	}
 
-	if len(remaining) == 0 {
-		return flow, nil
+	if hint != "" {
+		pkceFlowInfo.SetLoginHint(hint)
 	}
 
-	return &fallbackFlow{
-		active:    flow,
-		remaining: remaining,
-		hint:      hint,
-		newAuth:   newAuth,
-	}, nil
+	return pkceFlowInfo, nil
 }
 
-// initFirstAvailableFlow returns the first flow that could be initialized along with the flows
-// after it, which are still untried.
-func initFirstAvailableFlow(a *Auth, client *mgm.GrpcClient, flows []oauthFlowInit, hint string) (OAuthFlow, []oauthFlowInit, error) {
-	var errs []error
-	for i, f := range flows {
-		flow, err := f.init(a, client, hint)
-		if err == nil {
-			return flow, flows[i+1:], nil
-		}
+// authenticateWithDeviceCodeFlow initializes the Device Code auth Flow
+func authenticateWithDeviceCodeFlow(ctx context.Context, config *profilemanager.Config, hint string) (OAuthFlow, error) {
+	authClient, err := NewAuth(ctx, config.PrivateKey, config.ManagementURL, config)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create auth client: %v", err)
+	}
+	defer authClient.Close()
 
-		errs = append(errs, fmt.Errorf("%s: %w", f.name, err))
-
-		// only a flow this deployment does not offer is worth replacing with another one
-		if !isFlowUnavailable(err) {
-			break
-		}
-		if i < len(flows)-1 {
-			log.Infof("%s is not configured (%v), falling back to %s", f.name, err, flows[i+1].name)
+	deviceFlowInfo, err := authClient.getDeviceFlow(authClient.client)
+	if err != nil {
+		switch s, ok := gstatus.FromError(err); {
+		case ok && s.Code() == codes.NotFound:
+			return nil, fmt.Errorf("no SSO provider returned from management. " +
+				"Please proceed with setting up this device using setup keys " +
+				"https://docs.netbird.io/how-to/register-machines-using-setup-keys")
+		case ok && s.Code() == codes.Unimplemented:
+			return nil, fmt.Errorf("the management server, %s, does not support SSO providers, "+
+				"please update your server or use Setup Keys to login", config.ManagementURL)
+		default:
+			return nil, fmt.Errorf("getting device authorization flow info failed with error: %v", err)
 		}
 	}
 
-	return nil, nil, flowInitError(a.mgmURL, errs)
-}
-
-// flowInitError turns the per-flow initialization errors into a single actionable error. The
-// message stays neutral about what to do instead: SSO is also how a peer extends its session and
-// authenticates SSH, where a setup key is no alternative. Callers that are enrolling a device add
-// that advice themselves, see IsSSOUnavailable.
-func flowInitError(mgmURL *url.URL, errs []error) error {
-	if allMatch(errs, isFlowUnimplemented) {
-		return &ssoUnavailableError{msg: fmt.Sprintf("the management server, %s, does not support SSO providers, "+
-			"please update your server", mgmURL)}
+	if hint != "" {
+		deviceFlowInfo.SetLoginHint(hint)
 	}
 
-	if allMatch(errs, isFlowUnavailable) {
-		return &ssoUnavailableError{msg: "the management server has no SSO provider configured: " +
-			"neither the pkce authorization flow nor the device code flow is available"}
-	}
-
-	return fmt.Errorf("initialize authorization flow: %w", errors.Join(errs...))
-}
-
-// IsSSOUnavailable reports whether err means the management server offers no usable SSO flow, so
-// no retry and no other flow can help. Enrollment paths use it to point the user at setup keys.
-func IsSSOUnavailable(err error) bool {
-	var ssoUnavailable *ssoUnavailableError
-	return errors.As(err, &ssoUnavailable)
-}
-
-// WithSetupKeyAdvice appends enrollment guidance to an SSO-unavailable error and returns any
-// other error unchanged. Only enrollment can fall back to a setup key: extending a session and
-// authenticating SSH cannot, so those paths must not call this.
-//
-// The login paths that do call it cannot tell an unregistered peer from an SSO-enrolled one
-// whose session expired, since both answer PermissionDenied, so the advice names the case it
-// applies to rather than telling an enrolled peer to do something that cannot work.
-func WithSetupKeyAdvice(err error) error {
-	if !IsSSOUnavailable(err) {
-		return err
-	}
-
-	return fmt.Errorf("%w. If this device is not enrolled yet, enroll it with a setup key instead: "+
-		"https://docs.netbird.io/how-to/register-machines-using-setup-keys", err)
-}
-
-func allMatch(errs []error, match func(error) bool) bool {
-	if len(errs) == 0 {
-		return false
-	}
-
-	for _, err := range errs {
-		if !match(err) {
-			return false
-		}
-	}
-	return true
-}
-
-// isFlowUnavailable reports whether the flow is not on offer here: management has no
-// configuration for it (NotFound), predates the RPC entirely (Unimplemented), returned an
-// incomplete configuration, or the IdP does not serve the grant.
-func isFlowUnavailable(err error) bool {
-	return errors.Is(err, errFlowNotConfigured) ||
-		hasStatusCode(err, codes.NotFound) ||
-		hasStatusCode(err, codes.Unimplemented)
-}
-
-func isFlowUnimplemented(err error) bool {
-	return hasStatusCode(err, codes.Unimplemented)
-}
-
-func hasStatusCode(err error, code codes.Code) bool {
-	s, ok := gstatus.FromError(err)
-	if !ok {
-		return false
-	}
-	return s.Code() == code
+	return deviceFlowInfo, nil
 }

client/internal/auth/oauth_test.go

@@ -1,330 +0,0 @@
-package auth
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"net/url"
-	"runtime"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
-
-	mgm "github.com/netbirdio/netbird/shared/management/client"
-)
-
-// stubFlow is a minimal OAuthFlow returned by the fake initializers below. requestErr, when set,
-// is what its RequestAuthInfo returns, standing in for an IdP that rejects the flow.
-type stubFlow struct {
-	name       string
-	hint       string
-	requestErr error
-}
-
-func (s *stubFlow) RequestAuthInfo(context.Context) (AuthFlowInfo, error) {
-	if s.requestErr != nil {
-		return AuthFlowInfo{}, s.requestErr
-	}
-	return AuthFlowInfo{UserCode: s.name}, nil
-}
-
-func (s *stubFlow) WaitToken(context.Context, AuthFlowInfo) (TokenInfo, error) {
-	return TokenInfo{}, nil
-}
-
-func (s *stubFlow) GetClientID(context.Context) string {
-	return ""
-}
-
-func (s *stubFlow) SetLoginHint(hint string) {
-	s.hint = hint
-}
-
-// stubInit returns a flow initializer that yields a named stub flow, or err when err is non-nil.
-func stubInit(name string, err error) oauthFlowInit {
-	return stubInitFlow(name, err, nil)
-}
-
-// stubInitFlow is stubInit with control over what the resulting flow's RequestAuthInfo returns.
-func stubInitFlow(name string, initErr, requestErr error) oauthFlowInit {
-	return oauthFlowInit{
-		name: name,
-		init: func(_ *Auth, _ *mgm.GrpcClient, hint string) (OAuthFlow, error) {
-			if initErr != nil {
-				return nil, initErr
-			}
-			return &stubFlow{name: name, hint: hint, requestErr: requestErr}, nil
-		},
-	}
-}
-
-// stubAuthFactory hands out an Auth without a management connection, which the stub
-// initializers above never touch.
-func stubAuthFactory(a *Auth) authFactory {
-	return func(context.Context) (*Auth, func(), error) {
-		return a, func() {}, nil
-	}
-}
-
-func TestOAuthFlowWithFallback(t *testing.T) {
-	notFound := status.Error(codes.NotFound, "no device authorization flow information available")
-	unimplemented := status.Error(codes.Unimplemented, "unknown method")
-	incompleteConfig := fmt.Errorf("%w: Client ID value is empty", errFlowNotConfigured)
-	unreachable := status.Error(codes.Unavailable, "connection refused")
-
-	tests := []struct {
-		name          string
-		flows         []oauthFlowInit
-		expectedFlow  string
-		expectedErr   string
-		expectedNoSSO bool
-	}{
-		{
-			name:         "preferred flow is used",
-			flows:        []oauthFlowInit{stubInit("device", nil), stubInit("pkce", nil)},
-			expectedFlow: "device",
-		},
-		{
-			// the RedHat case: device code flow disabled on management, PKCE configured
-			name:         "falls back when preferred flow is not configured",
-			flows:        []oauthFlowInit{stubInit("device", notFound), stubInit("pkce", nil)},
-			expectedFlow: "pkce",
-		},
-		{
-			name:         "falls back on an incomplete flow configuration",
-			flows:        []oauthFlowInit{stubInit("pkce", incompleteConfig), stubInit("device", nil)},
-			expectedFlow: "device",
-		},
-		{
-			name:        "does not fall back when the preferred flow fails for another reason",
-			flows:       []oauthFlowInit{stubInit("pkce", unreachable), stubInit("device", nil)},
-			expectedErr: "connection refused",
-		},
-		{
-			// stays neutral about the remedy: --extend and SSH auth cannot use a setup key
-			name:          "neither flow configured reports no SSO provider",
-			flows:         []oauthFlowInit{stubInit("device", notFound), stubInit("pkce", notFound)},
-			expectedErr:   "no SSO provider configured",
-			expectedNoSSO: true,
-		},
-		{
-			name:          "old server without the flow RPCs asks for an update",
-			flows:         []oauthFlowInit{stubInit("device", unimplemented), stubInit("pkce", unimplemented)},
-			expectedErr:   "does not support SSO providers",
-			expectedNoSSO: true,
-		},
-	}
-
-	mgmURL, err := url.Parse("https://api.netbird.io:443")
-	require.NoError(t, err)
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			a := &Auth{mgmURL: mgmURL}
-			flow, err := oauthFlowWithFallback(a, nil, tt.flows, "user@example.com", stubAuthFactory(a))
-
-			if tt.expectedErr != "" {
-				require.Error(t, err)
-				assert.Contains(t, err.Error(), tt.expectedErr)
-
-				var ssoUnavailable *ssoUnavailableError
-				assert.Equal(t, tt.expectedNoSSO, errors.As(err, &ssoUnavailable),
-					"terminal SSO-unavailable classification mismatch for %v", err)
-				return
-			}
-
-			require.NoError(t, err)
-			stub := activeStub(t, flow)
-			assert.Equal(t, tt.expectedFlow, stub.name)
-			assert.Equal(t, "user@example.com", stub.hint, "login hint must be passed to the flow")
-		})
-	}
-}
-
-// activeStub unwraps the flow currently in use, which is behind a fallbackFlow whenever an
-// untried flow is left.
-func activeStub(t *testing.T, flow OAuthFlow) *stubFlow {
-	t.Helper()
-
-	if fallback, ok := flow.(*fallbackFlow); ok {
-		flow = fallback.current()
-	}
-
-	stub, ok := flow.(*stubFlow)
-	require.True(t, ok, "unexpected flow type %T", flow)
-	return stub
-}
-
-// TestFallbackFlowRequestAuthInfo covers the failure the RedHat report hit: management hands out
-// a device flow configuration, but the IdP does not serve the grant and only says so when the
-// device code is requested.
-func TestFallbackFlowRequestAuthInfo(t *testing.T) {
-	mgmURL, err := url.Parse("https://api.netbird.io:443")
-	require.NoError(t, err)
-	a := &Auth{mgmURL: mgmURL}
-
-	idpRejects := fmt.Errorf("%w: request device code returned status 404", errFlowNotConfigured)
-
-	t.Run("swaps in the untried flow", func(t *testing.T) {
-		flows := []oauthFlowInit{stubInitFlow("device", nil, idpRejects), stubInit("pkce", nil)}
-
-		flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a))
-		require.NoError(t, err)
-		require.Equal(t, "device", activeStub(t, flow).name)
-
-		info, err := flow.RequestAuthInfo(context.Background())
-		require.NoError(t, err)
-		assert.Equal(t, "pkce", info.UserCode, "the request must be served by the fallback flow")
-		assert.Equal(t, "pkce", activeStub(t, flow).name, "the fallback flow must stay active for WaitToken")
-	})
-
-	t.Run("keeps the original error when nothing else is configured", func(t *testing.T) {
-		flows := []oauthFlowInit{
-			stubInitFlow("device", nil, idpRejects),
-			stubInit("pkce", status.Error(codes.NotFound, "no pkce authorization flow information available")),
-		}
-
-		flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a))
-		require.NoError(t, err)
-
-		_, err = flow.RequestAuthInfo(context.Background())
-		require.Error(t, err)
-		assert.Contains(t, err.Error(), "status 404")
-	})
-
-	t.Run("keeps the original error when the fallback cannot reach management", func(t *testing.T) {
-		flows := []oauthFlowInit{stubInitFlow("device", nil, idpRejects), stubInit("pkce", nil)}
-
-		unreachable := func(context.Context) (*Auth, func(), error) {
-			return nil, nil, errors.New("connect to management: connection refused")
-		}
-
-		flow, err := oauthFlowWithFallback(a, nil, flows, "", unreachable)
-		require.NoError(t, err)
-
-		_, err = flow.RequestAuthInfo(context.Background())
-		require.Error(t, err)
-		assert.Contains(t, err.Error(), "status 404", "the IdP error must survive a failed fallback")
-		assert.Equal(t, "device", activeStub(t, flow).name, "a failed fallback must not swap the flow")
-	})
-
-	t.Run("does not swap flows on an unrelated failure", func(t *testing.T) {
-		flows := []oauthFlowInit{
-			stubInitFlow("device", nil, errors.New("timeout talking to the IdP")),
-			stubInit("pkce", nil),
-		}
-
-		flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a))
-		require.NoError(t, err)
-
-		_, err = flow.RequestAuthInfo(context.Background())
-		require.Error(t, err)
-		assert.Equal(t, "device", activeStub(t, flow).name, "the preferred flow must stay active")
-	})
-}
-
-// TestForcedDeviceFlowHasNoFallback covers Android TV and tvOS: a browserless device must get the
-// device code error rather than a PKCE flow it can never complete.
-func TestForcedDeviceFlowHasNoFallback(t *testing.T) {
-	mgmURL, err := url.Parse("https://api.netbird.io:443")
-	require.NoError(t, err)
-	a := &Auth{mgmURL: mgmURL}
-
-	notFound := status.Error(codes.NotFound, "no device authorization flow information available")
-
-	t.Run("no wrapper when the device flow works", func(t *testing.T) {
-		// flowOrder(force) yields this single-entry list, see TestFlowOrder
-		forced := []oauthFlowInit{stubInit("device", nil)}
-
-		flow, err := oauthFlowWithFallback(a, nil, forced, "", stubAuthFactory(a))
-		require.NoError(t, err)
-
-		_, wrapped := flow.(*fallbackFlow)
-		assert.False(t, wrapped, "nothing may swap the flow later on a browserless device")
-	})
-
-	t.Run("reports the device flow error instead of falling back", func(t *testing.T) {
-		forced := []oauthFlowInit{stubInit("device", notFound)}
-
-		_, err := oauthFlowWithFallback(a, nil, forced, "", stubAuthFactory(a))
-		require.Error(t, err)
-		assert.True(t, IsSSOUnavailable(err), "the caller must see that SSO is unavailable here")
-	})
-}
-
-// TestFallbackFlowSetLoginHint covers the Android SDK's pattern: it sets the login hint after the
-// flow is built, through a type assertion that the wrapper must satisfy.
-func TestFallbackFlowSetLoginHint(t *testing.T) {
-	mgmURL, err := url.Parse("https://api.netbird.io:443")
-	require.NoError(t, err)
-	a := &Auth{mgmURL: mgmURL}
-
-	idpRejects := fmt.Errorf("%w: request device code returned status 404", errFlowNotConfigured)
-	flows := []oauthFlowInit{stubInitFlow("device", nil, idpRejects), stubInit("pkce", nil)}
-
-	flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a))
-	require.NoError(t, err)
-
-	setter, ok := flow.(loginHintSetter)
-	require.True(t, ok, "the wrapper must accept a login hint like the concrete flows do")
-	setter.SetLoginHint("user@example.com")
-	assert.Equal(t, "user@example.com", activeStub(t, flow).hint, "the active flow must get the hint")
-
-	// the device flow is rejected by the IdP here, so the hint has to survive into the fallback
-	_, err = flow.RequestAuthInfo(context.Background())
-	require.NoError(t, err)
-	assert.Equal(t, "pkce", activeStub(t, flow).name)
-	assert.Equal(t, "user@example.com", activeStub(t, flow).hint, "the fallback flow must get the hint too")
-}
-
-func TestWithSetupKeyAdvice(t *testing.T) {
-	other := errors.New("connection refused")
-	assert.Equal(t, other, WithSetupKeyAdvice(other), "only an SSO-unavailable error gets advice")
-
-	advised := WithSetupKeyAdvice(&ssoUnavailableError{msg: "no SSO provider configured"})
-	assert.Contains(t, advised.Error(), "no SSO provider configured", "the original message must survive")
-	assert.Contains(t, advised.Error(), "setup key")
-	// a setup key cannot re-enrol a peer whose SSO session expired, and the login paths cannot
-	// tell that peer apart from an unregistered one, so the advice must state its condition
-	assert.Contains(t, advised.Error(), "not enrolled yet")
-	assert.True(t, IsSSOUnavailable(advised), "advice must keep the error classifiable")
-}
-
-func flowNames(flows []oauthFlowInit) []string {
-	names := make([]string, 0, len(flows))
-	for _, f := range flows {
-		names = append(names, f.name)
-	}
-	return names
-}
-
-func TestFlowOrder(t *testing.T) {
-	const pkce, device = "pkce authorization flow", "device code flow"
-
-	assert.Equal(t, []string{pkce, device}, flowNames(flowOrder(false, true)),
-		"a device with a browser tries PKCE first and keeps the device code flow as a fallback")
-
-	// only a unix host without a graphical session lacks a browser; the other platforms have one
-	headless := []string{pkce, device}
-	if runtime.GOOS == "linux" || runtime.GOOS == "freebsd" {
-		headless = []string{device, pkce}
-	}
-	assert.Equal(t, headless, flowNames(flowOrder(false, false)), "on %s", runtime.GOOS)
-
-	// Android TV and tvOS have no browser, so PKCE cannot complete there even from another
-	// device: the redirect must reach the loopback listener of the device being enrolled.
-	assert.Equal(t, []string{device}, flowNames(flowOrder(true, false)),
-		"a forced device code flow must not fall back to PKCE")
-	assert.Equal(t, []string{device}, flowNames(flowOrder(true, true)),
-		"force wins over a reported graphical session")
-}
-
-func TestPreferDeviceFlow(t *testing.T) {
-	isUnix := runtime.GOOS == "linux" || runtime.GOOS == "freebsd"
-
-	assert.Equal(t, isUnix, preferDeviceFlow(false), "headless unix hosts prefer the device flow")
-	assert.False(t, preferDeviceFlow(true), "clients with a graphical session prefer PKCE")
-}

client/internal/auth/pkce_flow.go

@@ -62,17 +62,8 @@ type PKCEAuthProviderConfig struct {
 	LoginHint string
 }
 
-// validatePKCEConfig validates PKCE provider configuration. A missing value means management
-// does not have this flow configured, so the error wraps errFlowNotConfigured and the caller can
-// fall back to the other flow.
+// validatePKCEConfig validates PKCE provider configuration
 func validatePKCEConfig(config *PKCEAuthProviderConfig) error {
-	if err := checkPKCEConfig(config); err != nil {
-		return fmt.Errorf("%w: %w", errFlowNotConfigured, err)
-	}
-	return nil
-}
-
-func checkPKCEConfig(config *PKCEAuthProviderConfig) error {
 	errorMsgFormat := "invalid provider configuration received from management: %s value is empty. Contact your NetBird administrator"
 
 	if config.ClientID == "" {

client/proto/daemon.pb.go

@@ -5628,13 +5628,9 @@ func (x *GetPeerSSHHostKeyResponse) GetFound() bool {
 type RequestJWTAuthRequest struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
 	// hint for OIDC login_hint parameter (typically email address)
-	Hint *string `protobuf:"bytes,1,opt,name=hint,proto3,oneof" json:"hint,omitempty"`
-	// hasGraphicalSession tells the daemon that the caller has a graphical session,
-	// which decides whether PKCE or the device code flow is preferred. The daemon
-	// cannot detect this itself: it does not inherit the session environment.
-	HasGraphicalSession bool `protobuf:"varint,2,opt,name=hasGraphicalSession,proto3" json:"hasGraphicalSession,omitempty"`
-	unknownFields       protoimpl.UnknownFields
-	sizeCache           protoimpl.SizeCache
+	Hint          *string `protobuf:"bytes,1,opt,name=hint,proto3,oneof" json:"hint,omitempty"`
+	unknownFields protoimpl.UnknownFields
+	sizeCache     protoimpl.SizeCache
 }
 
 func (x *RequestJWTAuthRequest) Reset() {
@@ -5674,13 +5670,6 @@ func (x *RequestJWTAuthRequest) GetHint() string {
 	return ""
 }
 
-func (x *RequestJWTAuthRequest) GetHasGraphicalSession() bool {
-	if x != nil {
-		return x.HasGraphicalSession
-	}
-	return false
-}
-
 // RequestJWTAuthResponse contains authentication flow information
 type RequestJWTAuthResponse struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
@@ -5905,13 +5894,9 @@ type RequestExtendAuthSessionRequest struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
 	// Optional OIDC login_hint (typically the user's email) to pre-fill the
 	// IdP login form.
-	Hint *string `protobuf:"bytes,1,opt,name=hint,proto3,oneof" json:"hint,omitempty"`
-	// hasGraphicalSession tells the daemon that the caller has a graphical session,
-	// which decides whether PKCE or the device code flow is preferred. The daemon
-	// cannot detect this itself: it does not inherit the session environment.
-	HasGraphicalSession bool `protobuf:"varint,2,opt,name=hasGraphicalSession,proto3" json:"hasGraphicalSession,omitempty"`
-	unknownFields       protoimpl.UnknownFields
-	sizeCache           protoimpl.SizeCache
+	Hint          *string `protobuf:"bytes,1,opt,name=hint,proto3,oneof" json:"hint,omitempty"`
+	unknownFields protoimpl.UnknownFields
+	sizeCache     protoimpl.SizeCache
 }
 
 func (x *RequestExtendAuthSessionRequest) Reset() {
@@ -5951,13 +5936,6 @@ func (x *RequestExtendAuthSessionRequest) GetHint() string {
 	return ""
 }
 
-func (x *RequestExtendAuthSessionRequest) GetHasGraphicalSession() bool {
-	if x != nil {
-		return x.HasGraphicalSession
-	}
-	return false
-}
-
 // RequestExtendAuthSessionResponse carries the verification URI the UI
 // should open in a browser. The daemon retains the flow state and resolves
 // it via WaitExtendAuthSession.
@@ -7525,10 +7503,9 @@ const file_daemon_proto_rawDesc = "" +
 	"sshHostKey\x12\x16\n" +
 	"\x06peerIP\x18\x02 \x01(\tR\x06peerIP\x12\x1a\n" +
 	"\bpeerFQDN\x18\x03 \x01(\tR\bpeerFQDN\x12\x14\n" +
-	"\x05found\x18\x04 \x01(\bR\x05found\"k\n" +
+	"\x05found\x18\x04 \x01(\bR\x05found\"9\n" +
 	"\x15RequestJWTAuthRequest\x12\x17\n" +
-	"\x04hint\x18\x01 \x01(\tH\x00R\x04hint\x88\x01\x01\x120\n" +
-	"\x13hasGraphicalSession\x18\x02 \x01(\bR\x13hasGraphicalSessionB\a\n" +
+	"\x04hint\x18\x01 \x01(\tH\x00R\x04hint\x88\x01\x01B\a\n" +
 	"\x05_hint\"\x9a\x02\n" +
 	"\x16RequestJWTAuthResponse\x12(\n" +
 	"\x0fverificationURI\x18\x01 \x01(\tR\x0fverificationURI\x128\n" +
@@ -7548,10 +7525,9 @@ const file_daemon_proto_rawDesc = "" +
 	"\x14WaitJWTTokenResponse\x12\x14\n" +
 	"\x05token\x18\x01 \x01(\tR\x05token\x12\x1c\n" +
 	"\ttokenType\x18\x02 \x01(\tR\ttokenType\x12\x1c\n" +
-	"\texpiresIn\x18\x03 \x01(\x03R\texpiresIn\"u\n" +
+	"\texpiresIn\x18\x03 \x01(\x03R\texpiresIn\"C\n" +
 	"\x1fRequestExtendAuthSessionRequest\x12\x17\n" +
-	"\x04hint\x18\x01 \x01(\tH\x00R\x04hint\x88\x01\x01\x120\n" +
-	"\x13hasGraphicalSession\x18\x02 \x01(\bR\x13hasGraphicalSessionB\a\n" +
+	"\x04hint\x18\x01 \x01(\tH\x00R\x04hint\x88\x01\x01B\a\n" +
 	"\x05_hint\"\xe0\x01\n" +
 	" RequestExtendAuthSessionResponse\x12(\n" +
 	"\x0fverificationURI\x18\x01 \x01(\tR\x0fverificationURI\x128\n" +

client/proto/daemon.proto

@@ -894,10 +894,6 @@ message GetPeerSSHHostKeyResponse {
 message RequestJWTAuthRequest {
   // hint for OIDC login_hint parameter (typically email address)
   optional string hint = 1;
-  // hasGraphicalSession tells the daemon that the caller has a graphical session,
-  // which decides whether PKCE or the device code flow is preferred. The daemon
-  // cannot detect this itself: it does not inherit the session environment.
-  bool hasGraphicalSession = 2;
 }
 
 // RequestJWTAuthResponse contains authentication flow information
@@ -941,10 +937,6 @@ message RequestExtendAuthSessionRequest {
   // Optional OIDC login_hint (typically the user's email) to pre-fill the
   // IdP login form.
   optional string hint = 1;
-  // hasGraphicalSession tells the daemon that the caller has a graphical session,
-  // which decides whether PKCE or the device code flow is preferred. The daemon
-  // cannot detect this itself: it does not inherit the session environment.
-  bool hasGraphicalSession = 2;
 }
 
 // RequestExtendAuthSessionResponse carries the verification URI the UI

client/server/server.go

@@ -682,11 +682,6 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
 		oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint)
 		if err != nil {
 			state.Set(internal.StatusLoginFailed)
-			// enrolling a device is the one flow a setup key can replace. NotFound so the CLI
-			// stops its backoff loop and shows this instead of retrying a permanent condition.
-			if auth.IsSSOUnavailable(err) {
-				return nil, gstatus.Error(codes.NotFound, auth.WithSetupKeyAdvice(err).Error())
-			}
 			return nil, err
 		}
 
@@ -1728,8 +1723,8 @@ func (s *Server) RequestJWTAuth(
 		hint = profilemanager.GetLoginHint()
 	}
 
-	// the daemon has no graphical session of its own, only the caller can answer this
-	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint)
+	isDesktop := isUnixRunningDesktop()
+	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, isDesktop, false, hint)
 	if err != nil {
 		return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
 	}
@@ -1832,8 +1827,8 @@ func (s *Server) RequestExtendAuthSession(
 		hint = profilemanager.GetLoginHint()
 	}
 
-	// the daemon has no graphical session of its own, only the caller can answer this
-	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint)
+	isDesktop := isUnixRunningDesktop()
+	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, isDesktop, false, hint)
 	if err != nil {
 		return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
 	}
@@ -2005,6 +2000,13 @@ func (s *Server) ExposeService(req *proto.ExposeServiceRequest, srv proto.Daemon
 	return nil
 }
 
+func isUnixRunningDesktop() bool {
+	if runtime.GOOS != "linux" && runtime.GOOS != "freebsd" {
+		return false
+	}
+	return os.Getenv("DESKTOP_SESSION") != "" || os.Getenv("XDG_CURRENT_DESKTOP") != ""
+}
+
 func (s *Server) runProbes(ctx context.Context, waitForProbeResult bool) {
 	if s.connectClient == nil {
 		return

client/ssh/common.go

@@ -13,7 +13,6 @@ import (
 	"golang.org/x/crypto/ssh"
 
 	"github.com/netbirdio/netbird/client/proto"
-	"github.com/netbirdio/netbird/util"
 )
 
 const (
@@ -93,8 +92,7 @@ func printAuthInstructions(stderr io.Writer, authResponse *proto.RequestJWTAuthR
 
 // RequestJWTToken requests or retrieves a JWT token for SSH authentication
 func RequestJWTToken(ctx context.Context, client proto.DaemonServiceClient, stdout, stderr io.Writer, useCache bool, hint string, openBrowser func(string) error) (string, error) {
-	// the ssh client runs in the user's session, the daemon does not: tell it what we can see
-	req := &proto.RequestJWTAuthRequest{HasGraphicalSession: util.HasGraphicalSession()}
+	req := &proto.RequestJWTAuthRequest{}
 	if hint != "" {
 		req.Hint = &hint
 	}
@@ -195,3 +193,4 @@ func buildAddressList(hostname string, remote net.Addr) []string {
 	}
 	return addresses
 }
+

client/ui/authsession/service.go

@@ -58,8 +58,7 @@ func (s *Session) RequestExtend(ctx context.Context, p ExtendStartParams) (Exten
 		return ExtendStartResult{}, err
 	}
 
-	// a request from the UI implies a graphical session, which the daemon cannot detect itself
-	req := &proto.RequestExtendAuthSessionRequest{HasGraphicalSession: true}
+	req := &proto.RequestExtendAuthSessionRequest{}
 	if p.Hint != "" {
 		h := p.Hint
 		req.Hint = &h

client/ui/services/connection.go

@@ -108,11 +108,10 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
 	}
 
 	req := &proto.LoginRequest{
-		ManagementUrl: p.ManagementURL,
-		SetupKey:      p.SetupKey,
-		Hostname:      p.Hostname,
-		// a login driven by the UI always has a graphical session available
-		IsUnixDesktopClient: true,
+		ManagementUrl:       p.ManagementURL,
+		SetupKey:            p.SetupKey,
+		Hostname:            p.Hostname,
+		IsUnixDesktopClient: runtime.GOOS == "linux",
 	}
 	if profileName != "" {
 		req.ProfileName = ptrStr(profileName)

shared/management/http/api/openapi.yml

@@ -1433,14 +1433,13 @@ components:
           enum: [ "all", "tcp", "udp", "icmp", "netbird-ssh" ]
           example: "tcp"
         ports:
-          description: Policy rule affected ports. Mutually exclusive with `port_ranges`. A rule accepts either individual ports or port ranges, not both.
-          x-omit-from-example: true
+          description: Policy rule affected ports
           type: array
           items:
             type: string
             example: "80"
         port_ranges:
-          description: Policy rule affected ports ranges list. Mutually exclusive with `ports`. To mix individual ports with ranges in one rule, express each single port as a range with identical start and end values (for example, start 443, end 443).
+          description: Policy rule affected ports ranges list
           type: array
           items:
             $ref: '#/components/schemas/RulePortRange'
@@ -1460,7 +1459,7 @@ components:
         - action
 
     RulePortRange:
-      description: Policy rule affected ports range. A range with identical start and end values represents a single port.
+      description: Policy rule affected ports range
       type: object
       properties:
         start:

shared/management/http/api/types.gen.go

@@ -4468,10 +4468,10 @@ type PolicyRule struct {
 	// Name Policy rule name identifier
 	Name string `json:"name"`
 
-	// PortRanges Policy rule affected ports ranges list. Mutually exclusive with `ports`. To mix individual ports with ranges in one rule, express each single port as a range with identical start and end values (for example, start 443, end 443).
+	// PortRanges Policy rule affected ports ranges list
 	PortRanges *[]RulePortRange `json:"port_ranges,omitempty"`
 
-	// Ports Policy rule affected ports. Mutually exclusive with `port_ranges`. A rule accepts either individual ports or port ranges, not both.
+	// Ports Policy rule affected ports
 	Ports *[]string `json:"ports,omitempty"`
 
 	// Protocol Policy rule type of the traffic
@@ -4508,10 +4508,10 @@ type PolicyRuleMinimum struct {
 	// Name Policy rule name identifier
 	Name string `json:"name"`
 
-	// PortRanges Policy rule affected ports ranges list. Mutually exclusive with `ports`. To mix individual ports with ranges in one rule, express each single port as a range with identical start and end values (for example, start 443, end 443).
+	// PortRanges Policy rule affected ports ranges list
 	PortRanges *[]RulePortRange `json:"port_ranges,omitempty"`
 
-	// Ports Policy rule affected ports. Mutually exclusive with `port_ranges`. A rule accepts either individual ports or port ranges, not both.
+	// Ports Policy rule affected ports
 	Ports *[]string `json:"ports,omitempty"`
 
 	// Protocol Policy rule type of the traffic
@@ -4551,10 +4551,10 @@ type PolicyRuleUpdate struct {
 	// Name Policy rule name identifier
 	Name string `json:"name"`
 
-	// PortRanges Policy rule affected ports ranges list. Mutually exclusive with `ports`. To mix individual ports with ranges in one rule, express each single port as a range with identical start and end values (for example, start 443, end 443).
+	// PortRanges Policy rule affected ports ranges list
 	PortRanges *[]RulePortRange `json:"port_ranges,omitempty"`
 
-	// Ports Policy rule affected ports. Mutually exclusive with `port_ranges`. A rule accepts either individual ports or port ranges, not both.
+	// Ports Policy rule affected ports
 	Ports *[]string `json:"ports,omitempty"`
 
 	// Protocol Policy rule type of the traffic
@@ -4962,7 +4962,7 @@ type RouteRequest struct {
 	SkipAutoApply *bool `json:"skip_auto_apply,omitempty"`
 }
 
-// RulePortRange Policy rule affected ports range. A range with identical start and end values represents a single port.
+// RulePortRange Policy rule affected ports range
 type RulePortRange struct {
 	// End The ending port of the range
 	End int `json:"end"`

util/common.go

@@ -3,69 +3,18 @@ package util
 import (
 	"os"
 	"os/exec"
-	"runtime"
-	"slices"
 
 	"github.com/skratchdot/open-golang/open"
 )
 
-const (
-	// envBrowser overrides the browser OpenBrowser launches
-	envBrowser = "BROWSER"
-	// envDesktopSession and envXDGCurrentDesktop are what xdg-open uses to pick a handler
-	envDesktopSession    = "DESKTOP_SESSION"
-	envXDGCurrentDesktop = "XDG_CURRENT_DESKTOP"
-	// envDisplay and envWaylandDisplay are what a graphical browser needs to reach a display
-	envDisplay        = "DISPLAY"
-	envWaylandDisplay = "WAYLAND_DISPLAY"
-	// envXDGSessionType names the session kind, e.g. tty, x11 or wayland
-	envXDGSessionType = "XDG_SESSION_TYPE"
-)
-
 // OpenBrowser opens the URL in a browser, respecting the BROWSER environment variable.
 func OpenBrowser(url string) error {
-	if browser := os.Getenv(envBrowser); browser != "" {
+	if browser := os.Getenv("BROWSER"); browser != "" {
 		return exec.Command(browser, url).Start()
 	}
 	return open.Run(url)
 }
 
-// browserSessionEnvVars returns the variables that decide whether OpenBrowser can open a URL.
-// DISPLAY and WAYLAND_DISPLAY are exactly what xdg-open's own has_display() checks, and without
-// them it degrades to terminal browsers. BROWSER is the explicit override both xdg-open and
-// OpenBrowser honor first. DESKTOP_SESSION and XDG_CURRENT_DESKTOP only tell xdg-open which
-// desktop-specific opener to prefer, so they are weaker evidence, kept because the previous
-// detection relied on them alone and dropping them would demote sessions that work today.
-func browserSessionEnvVars() []string {
-	return []string{envDisplay, envWaylandDisplay, envBrowser, envDesktopSession, envXDGCurrentDesktop}
-}
-
-// graphicalXDGSessionTypes are the systemd-logind session types that come with a display. The
-// other documented values are "tty" and "unspecified"; anything unrecognized is treated as no
-// display, so an unknown value picks the device code flow, which works without a browser.
-func graphicalXDGSessionTypes() []string {
-	return []string{"x11", "wayland", "mir"}
-}
-
-// HasGraphicalSession reports whether this process can open a browser and serve a loopback
-// redirect back to it. Windows and macOS always can. On Linux and FreeBSD the answer is env
-// based, so it only holds for a process started from the graphical session itself: a service
-// does not inherit those variables and always reports false, which is why callers running in
-// the user's session pass their own answer to the daemon.
-func HasGraphicalSession() bool {
-	if runtime.GOOS != "linux" && runtime.GOOS != "freebsd" {
-		return true
-	}
-
-	for _, env := range browserSessionEnvVars() {
-		if os.Getenv(env) != "" {
-			return true
-		}
-	}
-
-	return slices.Contains(graphicalXDGSessionTypes(), os.Getenv(envXDGSessionType))
-}
-
 // SliceDiff returns the elements in slice `x` that are not in slice `y`
 func SliceDiff(x, y []string) []string {
 	mapY := make(map[string]struct{}, len(y))

util/session_test.go

@@ -1,50 +0,0 @@
-package util
-
-import (
-	"os"
-	"runtime"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestHasGraphicalSession(t *testing.T) {
-	if runtime.GOOS != "linux" && runtime.GOOS != "freebsd" {
-		assert.True(t, HasGraphicalSession(), "%s always has a graphical session", runtime.GOOS)
-		return
-	}
-
-	// clear anything inherited from the session running the test, restored on cleanup
-	for _, env := range append(browserSessionEnvVars(), envXDGSessionType) {
-		t.Setenv(env, "")
-		os.Unsetenv(env)
-	}
-
-	assert.False(t, HasGraphicalSession(), "no session variables means no graphical session")
-
-	tests := []struct {
-		env      string
-		value    string
-		expected bool
-	}{
-		{env: envDisplay, value: ":0", expected: true},
-		{env: envWaylandDisplay, value: "wayland-0", expected: true},
-		{env: envDesktopSession, value: "gnome", expected: true},
-		{env: envXDGCurrentDesktop, value: "KDE", expected: true},
-		{env: envBrowser, value: "firefox", expected: true},
-		{env: envXDGSessionType, value: "wayland", expected: true},
-		{env: envXDGSessionType, value: "x11", expected: true},
-		{env: envXDGSessionType, value: "mir", expected: true},
-		{env: envXDGSessionType, value: "tty", expected: false},
-		{env: envXDGSessionType, value: "unspecified", expected: false},
-		// an unrecognized type must not be read as a display: the device code flow works anyway
-		{env: envXDGSessionType, value: "something-new", expected: false},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.env+"="+tt.value, func(t *testing.T) {
-			t.Setenv(tt.env, tt.value)
-			assert.Equal(t, tt.expected, HasGraphicalSession(), "%s=%s", tt.env, tt.value)
-		})
-	}
-}