From 2654789af7c56bd114312e76a212018de586d1b4 Mon Sep 17 00:00:00 2001 From: Dreamcat4 Date: Mon, 5 Oct 2015 21:10:59 +0100 Subject: [PATCH 1/5] fix: fw rule names must always be quoted, to permit spaces ' ' and brackets '()' Without this fix, the 'netsh' command gets name=Firewall Rule Name instead of name="Firewall Rule Name". Thus causing all sorts of havoc. Basic shell quoting rules seems to apply to Windows Powershell too. This is very much needed as many of windows 10's default firewall rules contain spaces and brackets () characters. --- windows/win_firewall_rule.ps1 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/windows/win_firewall_rule.ps1 b/windows/win_firewall_rule.ps1 index 223d8b17b69..9c73507509b 100644 --- a/windows/win_firewall_rule.ps1 +++ b/windows/win_firewall_rule.ps1 @@ -24,7 +24,7 @@ function getFirewallRule ($fwsettings) { try { #$output = Get-NetFirewallRule -name $($fwsettings.name); - $rawoutput=@(netsh advfirewall firewall show rule name=$($fwsettings.Name)) + $rawoutput=@(netsh advfirewall firewall show rule name="$($fwsettings.Name)") if (!($rawoutput -eq 'No rules match the specified criteria.')){ $rawoutput | Where {$_ -match '^([^:]+):\s*(\S.*)$'} | Foreach -Begin { $FirstRun = $true; @@ -123,8 +123,9 @@ function createFireWallRule ($fwsettings) { $execString+=" "; $execString+=$key; $execString+="="; + $execString+='"'; $execString+=$fwsetting.value; - #$execString+="'"; + $execString+='"'; }; try { #$msg+=@($execString); @@ -152,7 +153,7 @@ function createFireWallRule ($fwsettings) { function removeFireWallRule ($fwsettings) { $msg=@() try { - $rawoutput=@(netsh advfirewall firewall delete rule name=$($fwsettings.name)) + $rawoutput=@(netsh advfirewall firewall delete rule name="$($fwsettings.name)") $rawoutput | Where {$_ -match '^([^:]+):\s*(\S.*)$'} | Foreach -Begin { $FirstRun = $true; $HashProps = @{}; From 6c5a4a14ef415e4635f7e0dc7fcb345f0c617a98 Mon Sep 17 00:00:00 2001 From: Dreamcat4 Date: Mon, 5 Oct 2015 21:36:24 +0100 Subject: [PATCH 2/5] fix: win10 - Add exception handling for 'Profiles:' textual output key name mismatch. In win10 (and pribably win8x also): The output of 'show rule' key includes the line "Profiles:Public,Private". Yet your script expects the key name printed out to be "Profile:value". This commit added the necessary exception handling to avoid flagging 'different=true' under the false circumstance. The key name to SET a firewall rule is still "profile=" and not "profiles=". There is coming up another commit to fix the value handling for win10/win8. Which is another (different) error with the profile: key. --- windows/win_firewall_rule.ps1 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/windows/win_firewall_rule.ps1 b/windows/win_firewall_rule.ps1 index 9c73507509b..0b0a2cd54f9 100644 --- a/windows/win_firewall_rule.ps1 +++ b/windows/win_firewall_rule.ps1 @@ -75,6 +75,8 @@ function getFirewallRule ($fwsettings) { $donothing=$false } elseif ((($fwsetting.Key -eq 'Name') -or ($fwsetting.Key -eq 'DisplayName')) -and ($output."Rule Name" -eq $fwsettings.$($fwsetting.Key))) { $donothing=$false + } elseif (($fwsetting.Key -eq 'Profile') -and ($output."Profiles" -eq $fwsettings.$($fwsetting.Key))) { + $donothing=$false } else { $diff=$true; $difference+=@($fwsettings.$($fwsetting.Key)); From 469d22df973508b163a4aae028b8dff6faafbb3f Mon Sep 17 00:00:00 2001 From: Dreamcat4 Date: Mon, 5 Oct 2015 21:53:11 +0100 Subject: [PATCH 3/5] fix: The names of firewall profiles are different on win10 & win2008r2 Hi again. This commit removes a small portion of your script's own internal error checking. In specific: for the value of the profile: key. This is essential to avoid errors on other verisons of the windows operating system which are not win2008r2 (your version). For example: on win10 (and most likely win8x too), the names of the profiles don't include the values 'current' and 'all'. But instead the values are 'Public' 'Private' 'Domain' and 'Any. But in addition, there are also certain combinatorial values, such as profile=Public,Private etc. Which is too many to error check yourself. Yet removing the error checking here should not cause any ill effects however: since the netsh advfirewall ... cmds themselves to add / remove / modify actually to their own error checking of the profile=value. So when the cmd is run, it will error out itself with an appropriate / informative error msg. No harm done. Therefore please remove the highlighed portions from your own script. It is essential for interoperability with win10 and win8x. Many thanks. --- windows/win_firewall_rule.ps1 | 8 +------- windows/win_firewall_rule.py | 6 +++--- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/windows/win_firewall_rule.ps1 b/windows/win_firewall_rule.ps1 index 0b0a2cd54f9..8ef2d83aff6 100644 --- a/windows/win_firewall_rule.ps1 +++ b/windows/win_firewall_rule.ps1 @@ -246,13 +246,7 @@ foreach ($arg in $args){ }; $winprofile=Get-Attr $params "profile" "current"; -if (($winprofile -ne 'current') -or ($winprofile -ne 'domain') -or ($winprofile -ne 'standard') -or ($winprofile -ne 'all') ) { - $misArg+="Profile"; - $msg+=@("for the Profile parameter only the values 'current', 'domain', 'standard' or 'all' are allowed"); -} else { - - $fwsettings.Add("profile", $winprofile) -} +$fwsettings.Add("profile", $winprofile) if ($($($misArg|measure).count) -gt 0){ $result=New-Object psobject @{ diff --git a/windows/win_firewall_rule.py b/windows/win_firewall_rule.py index 295979b248f..ecdec5882cd 100644 --- a/windows/win_firewall_rule.py +++ b/windows/win_firewall_rule.py @@ -90,10 +90,10 @@ options: default: null required: false profile: - describtion: + description: - the profile this rule applies to - default: current - choices: ['current', 'domain', 'standard', 'all'] + default: null + required: false force: description: - Enforces the change if a rule with different values exists From dcaa79494995e50930a0b0b66995bc18cb6f5668 Mon Sep 17 00:00:00 2001 From: Dreamcat4 Date: Tue, 6 Oct 2015 10:47:27 +0100 Subject: [PATCH 4/5] fix: update documentation with new module name "win_firewall_rule" --- windows/win_firewall_rule.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/windows/win_firewall_rule.py b/windows/win_firewall_rule.py index ecdec5882cd..1463719356d 100644 --- a/windows/win_firewall_rule.py +++ b/windows/win_firewall_rule.py @@ -19,7 +19,7 @@ DOCUMENTATION = ''' --- -module: win_fw +module: win_firewall_rule version_added: "2.0" author: Timothy Vandenbrande short_description: Windows firewall automation @@ -99,13 +99,13 @@ options: - Enforces the change if a rule with different values exists default: false required: false - + ''' EXAMPLES = ''' -# create smtp firewall rule - action: win_fw +- name: Firewall rule to allow smtp on TCP port 25 + action: win_firewall_rule args: name: smtp state: present From ece9c2b43aba70228efdcf4efe36eb578b81abd3 Mon Sep 17 00:00:00 2001 From: Dreamcat4 Date: Tue, 6 Oct 2015 14:03:27 +0100 Subject: [PATCH 5/5] fix: Add 'enable:' flag for enabling existing rules which are disabled by default. This is a very much needed flag. To turn on/off existing firewall rules. And like the recent fix of the 'Profile' key, the netsh cmd prints 'Enabled' in the textual output. (at least on win10 it does). So again a similar small code added for the necessary exception handling when the difference check happens. Please merge / push upstream like the other fixes. Many thanks. This is the last fix I have put together for this patch set. So I will raise my PR now. But if you want to fix more bugs, it seems there may be others. In terms of the control code. Sometimes it will delete a rule under 'force' condition (when found difference) - but instead it is supposed to just modify the existing rule. Some weird behaviour regarding that. The other problem is that ansible does not return the error text printed by 'netsh' cmd verbatim... but it should as that makes debugging these errors a *lot* easier. --- windows/win_firewall_rule.ps1 | 18 ++++++++++++++++-- windows/win_firewall_rule.py | 9 ++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/windows/win_firewall_rule.ps1 b/windows/win_firewall_rule.ps1 index 8ef2d83aff6..63ada997456 100644 --- a/windows/win_firewall_rule.ps1 +++ b/windows/win_firewall_rule.ps1 @@ -22,7 +22,7 @@ function getFirewallRule ($fwsettings) { try { - + #$output = Get-NetFirewallRule -name $($fwsettings.name); $rawoutput=@(netsh advfirewall firewall show rule name="$($fwsettings.Name)") if (!($rawoutput -eq 'No rules match the specified criteria.')){ @@ -77,6 +77,8 @@ function getFirewallRule ($fwsettings) { $donothing=$false } elseif (($fwsetting.Key -eq 'Profile') -and ($output."Profiles" -eq $fwsettings.$($fwsetting.Key))) { $donothing=$false + } elseif (($fwsetting.Key -eq 'Enable') -and ($output."Enabled" -eq $fwsettings.$($fwsetting.Key))) { + $donothing=$false } else { $diff=$true; $difference+=@($fwsettings.$($fwsetting.Key)); @@ -196,6 +198,7 @@ $fwsettings=@{} # Variabelise the arguments $params=Parse-Args $args; +$enable=Get-Attr $params "enable" $null; $state=Get-Attr $params "state" "present"; $name=Get-Attr $params "name" ""; $direction=Get-Attr $params "direction" ""; @@ -203,6 +206,17 @@ $force=Get-Attr $params "force" $false; $action=Get-Attr $params "action" ""; # Check the arguments +if ($enable -ne $null) { + if ($enable -eq $true) { + $fwsettings.Add("Enable", "yes"); + } elseif ($enable -eq $false) { + $fwsettings.Add("Enable", "no"); + } else { + $misArg+="enable"; + $msg+=@("for the enable parameter only yes and no is allowed"); + }; +}; + if (($state -ne "present") -And ($state -ne "absent")){ $misArg+="state"; $msg+=@("for the state parameter only present and absent is allowed"); @@ -294,7 +308,7 @@ switch ($state.ToLower()){ }; Exit-Json $result; } - } elseif ($capture.identical -eq $false) { + } elseif ($capture.identical -eq $false) { if ($force -eq $true) { $capture=removeFirewallRule($fwsettings); $msg+=$capture.msg; diff --git a/windows/win_firewall_rule.py b/windows/win_firewall_rule.py index 1463719356d..64ec3050474 100644 --- a/windows/win_firewall_rule.py +++ b/windows/win_firewall_rule.py @@ -25,7 +25,13 @@ author: Timothy Vandenbrande short_description: Windows firewall automation description: - allows you to create/remove/update firewall rules -options: +options: + enable: + description: + - is this firewall rule enabled or disabled + default: null + required: false + choices: ['yes', 'no'] state: description: - create/remove/update or powermanage your VM @@ -108,6 +114,7 @@ EXAMPLES = ''' action: win_firewall_rule args: name: smtp + enabled: yes state: present localport: 25 action: allow