Skip to content

Commit

Permalink
Fix Windows firewall message check
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mlsmaycon committed Oct 26, 2023
1 parent a8d03d8 commit 20df241
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 37 deletions.
79 changes: 43 additions & 36 deletions client/firewall/uspfilter/allow_netbird_windows.go
Original file line number Diff line number Diff line change
@@ -1,28 +1,34 @@
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
func (m *Manager) Reset() error {
m.mutex.Lock()
defer m.mutex.Unlock()

if !isWindowsFirewallReachable() {
return nil
}

if !isFirewallRuleActive(firewallRuleName) {
return nil
}

m.outgoingRules = make(map[string]RuleSet)
m.incomingRules = make(map[string]RuleSet)

Expand All @@ -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",
Expand All @@ -45,47 +58,41 @@ 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()
}

func isWindowsFirewallReachable() bool {
args := []string{"advfirewall", "firewall", "show", "allprofiles", "state"}
cmd := exec.Command("netsh", args...)
cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}

cmd := exec.Command("netsh", args...)
cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
return cmd.Run()
_, err := cmd.Output()
if err != nil {
log.Info("Windows firewall is not reachable, skipping default rule creation. Error: ", err)
return false
}

return nil
return true
}

func isFirewallRuleActive(ruleName string) (bool, error) {
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()
_, err := cmd.Output()
if err != nil {

Check failure on line 93 in client/firewall/uspfilter/allow_netbird_windows.go

View workflow job for this annotation

GitHub Actions / lint (windows-latest)

S1008: should use 'return err == nil' instead of 'if err != nil { return false }; return true' (gosimple)
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 false
}

return true, nil
return true
}
2 changes: 1 addition & 1 deletion client/internal/acl/manager_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 20df241

Please sign in to comment.