From 76318f3f0618e4e08d0cef4a6766240e9b4166ff Mon Sep 17 00:00:00 2001
From: Maycon Santos <mlsmaycon@gmail.com>
Date: Fri, 27 Oct 2023 10:54:26 +0200
Subject: [PATCH] Fix Windows firewall message check (#1254)

The no rules matched message is operating system language specific, and can cause errors

Now we check if firewall is reachable by the app and then if the rule is returned or not in two different calls:

isWindowsFirewallReachable

isFirewallRuleActive
---
 .../uspfilter/allow_netbird_windows.go        | 87 ++++++++++---------
 client/internal/acl/manager_create.go         |  2 +-
 2 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/client/firewall/uspfilter/allow_netbird_windows.go b/client/firewall/uspfilter/allow_netbird_windows.go
index 05a6d22ae..140bfc87a 100644
--- a/client/firewall/uspfilter/allow_netbird_windows.go
+++ b/client/firewall/uspfilter/allow_netbird_windows.go
@@ -1,21 +1,19 @@
 package uspfilter
 
 import (
-	"errors"
 	"fmt"
 	"os/exec"
-	"strings"
 	"syscall"
+
+	log "github.com/sirupsen/logrus"
 )
 
 type action string
 
 const (
-	addRule    action = "add"
-	deleteRule action = "delete"
-
-	firewallRuleName     = "Netbird"
-	noRulesMatchCriteria = "No rules match the specified criteria"
+	addRule          action = "add"
+	deleteRule       action = "delete"
+	firewallRuleName        = "Netbird"
 )
 
 // Reset firewall to the default state
@@ -26,6 +24,14 @@ func (m *Manager) Reset() error {
 	m.outgoingRules = make(map[string]RuleSet)
 	m.incomingRules = make(map[string]RuleSet)
 
+	if !isWindowsFirewallReachable() {
+		return nil
+	}
+
+	if !isFirewallRuleActive(firewallRuleName) {
+		return nil
+	}
+
 	if err := manageFirewallRule(firewallRuleName, deleteRule); err != nil {
 		return fmt.Errorf("couldn't remove windows firewall: %w", err)
 	}
@@ -35,6 +41,13 @@ func (m *Manager) Reset() error {
 
 // AllowNetbird allows netbird interface traffic
 func (m *Manager) AllowNetbird() error {
+	if !isWindowsFirewallReachable() {
+		return nil
+	}
+
+	if isFirewallRuleActive(firewallRuleName) {
+		return nil
+	}
 	return manageFirewallRule(firewallRuleName,
 		addRule,
 		"dir=in",
@@ -45,47 +58,37 @@ func (m *Manager) AllowNetbird() error {
 	)
 }
 
-func manageFirewallRule(ruleName string, action action, args ...string) error {
-	active, err := isFirewallRuleActive(ruleName)
-	if err != nil {
-		return err
+func manageFirewallRule(ruleName string, action action, extraArgs ...string) error {
+
+	args := []string{"advfirewall", "firewall", string(action), "rule", "name=" + ruleName}
+	if action == addRule {
+		args = append(args, extraArgs...)
 	}
 
-	if (action == addRule && !active) || (action == deleteRule && active) {
-		baseArgs := []string{"advfirewall", "firewall", string(action), "rule", "name=" + ruleName}
-		args := append(baseArgs, args...)
-
-		cmd := exec.Command("netsh", args...)
-		cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
-		return cmd.Run()
-	}
-
-	return nil
+	cmd := exec.Command("netsh", args...)
+	cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
+	return cmd.Run()
 }
 
-func isFirewallRuleActive(ruleName string) (bool, error) {
+func isWindowsFirewallReachable() bool {
+	args := []string{"advfirewall", "show", "allprofiles", "state"}
+	cmd := exec.Command("netsh", args...)
+	cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
+
+	_, err := cmd.Output()
+	if err != nil {
+		log.Infof("Windows firewall is not reachable, skipping default rule management. Using only user space rules. Error: %s", err)
+		return false
+	}
+
+	return true
+}
+
+func isFirewallRuleActive(ruleName string) bool {
 	args := []string{"advfirewall", "firewall", "show", "rule", "name=" + ruleName}
 
 	cmd := exec.Command("netsh", args...)
 	cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
-	output, err := cmd.Output()
-	if err != nil {
-		var exitError *exec.ExitError
-		if errors.As(err, &exitError) {
-			// if the firewall rule is not active, we expect last exit code to be 1
-			exitStatus := exitError.Sys().(syscall.WaitStatus).ExitStatus()
-			if exitStatus == 1 {
-				if strings.Contains(string(output), noRulesMatchCriteria) {
-					return false, nil
-				}
-			}
-		}
-		return false, err
-	}
-
-	if strings.Contains(string(output), noRulesMatchCriteria) {
-		return false, nil
-	}
-
-	return true, nil
+	_, err := cmd.Output()
+	return err == nil
 }
diff --git a/client/internal/acl/manager_create.go b/client/internal/acl/manager_create.go
index 2fdca02ae..66185749b 100644
--- a/client/internal/acl/manager_create.go
+++ b/client/internal/acl/manager_create.go
@@ -20,7 +20,7 @@ func Create(iface IFaceMapper) (manager *DefaultManager, err error) {
 			return nil, err
 		}
 		if err := fm.AllowNetbird(); err != nil {
-			log.Errorf("failed to allow netbird interface traffic: %v", err)
+			log.Warnf("failed to allow netbird interface traffic: %v", err)
 		}
 		return newDefaultManager(fm), nil
 	}