Open jcoconnor opened 5 years ago
@jcoconnor and @randomnoun7 thanks for the report there's some interesting stuff here. I think localising for French is a non-starter, as you said its non-trivial and it would need re-doing to support other languages if needed. By way of background I started off using the netsh
command because it directly matches the hardening guides that this module attempts to implement and from what I can tell netsh
is the de-facto method of enabling/disabling remote access.
PowerShell would be the sensible way to read the firewall status on non-english Windows:
get-NetFirewallProfile -Profile Private
Name : Private
Enabled : True
DefaultInboundAction : NotConfigured
DefaultOutboundAction : NotConfigured
AllowInboundRules : NotConfigured
AllowLocalFirewallRules : NotConfigured
AllowLocalIPsecRules : NotConfigured
AllowUserApps : NotConfigured
AllowUserPorts : NotConfigured
AllowUnicastResponseToMulticast : NotConfigured
NotifyOnListen : True
EnableStealthModeForIPsec : NotConfigured
LogFileName : %systemroot%\system32\LogFiles\Firewall\pfirewall.log
LogMaxSizeKilobytes : 4096
LogAllowed : NotConfigured
LogBlocked : NotConfigured
LogIgnored : NotConfigured
DisabledInterfaceAliases : {NotConfigured}
vs netsh
:
Private Profile Settings:
----------------------------------------------------------------------
State ON
Firewall Policy BlockInbound,AllowOutbound
LocalFirewallRules N/A (GPO-store only)
LocalConSecRules N/A (GPO-store only)
InboundUserNotification Enable
RemoteManagement Disable
UnicastResponseToMulticast Enable
Logging:
LogAllowedConnections Disable
LogDroppedConnections Disable
FileName %systemroot%\system32\LogFiles\Firewall\pfirewall.log
MaxFileSize 4096
Ok.
As you mentioned, this is completely different output to what the provider currently publishes. Most of the fields are read-only so could easily be changed (or removed) but there are some that are being used that do not map directly or that give conflicting information (above output was captured from same machine running latest Windows 10 build...):
These would need to be worked around in PowerShell or removed if they are no longer supported by Windows.
I'm not working on Puppet at the moment but happy to review a PR. I'd probably approach this by adding JSON serialisation to the ps-bridge.ps1 script and then have puppet type and provider be a thin wrapper around it. That way the PowerShell can be hacked and tested without waiting for Puppet runs to finish.
Thanks for the quick response @GeoffWilliams
I'm going to use a workaround for our French OS build for Windows so not immediately looking for this to be fixed.
But think its definitely worth noting in the Limitations and would be glad to help test any PR work.
@GeoffWilliams For reference, this is the workaround code I'm using for or French Builds:
['public', 'private', 'domain'].each | $profile | {
exec { "Disable Firewall Profile ${profile}":
command => "Set-NetFirewallProfile -Profile ${profile} -Enabled False",
unless => "if([System.Convert]::ToBoolean((get-NetFirewallProfile -Profile ${profile}).enabled)) {Exit 1}",
provider => powershell,
logoutput => true,
}
}
@GeoffWilliams Firstly - I really like the simplicity of being able to disable the firewall using
Unfortunately when I attempted to use it on a Windows 2019/2016/2012r2 French edition (installed using the French ISO), the module failed to detect that the firewall had been disabled and kept trying to disable it.
I ran
--debug
and discovered that the module is using the following to determine the state of the profile:This returns:
So I'm assuming the detection is not handling the
État Inactif
and looking forOff
instead.@RandomNoun7 did some investigation in the module and these are his findings:
I looked into the puppet/windows_firewall module, and getting it to support french is definitely non-trivial. Because they chose to use the netsh tool instead of powershell, they get localized string output, instead of non-localized properties with data types like booleans.
So the trouble begins with lines like this one https://github.com/GeoffWilliams/puppet-windows_firewall/blob/master/lib/puppet_x/windows_firewall.rb#L256 that attempt to figure out the state of a given firewall profile. It assumes that the first word of the first line will contain the name of the profile it's getting the state for because in English that's what you get. But in french the string is localized so that the name of the profile is the last word on the line, not the first, because Latin languages put nouns before adjectives. And the issues spiral from there. If the module had chosen to use PowerShell instead, the return objects do not use localized property names, which you could argue is an oversight, but it is what it is. And the values you get back are much more likely to be either a real data type that can be compared, or at least a non localized string.
Even if the maintainers were willing to go through the trouble of figuring out how to detect the output is french and get the right values out of the returned strings, the property names in the type files would now be wrong. The property names in the type file are assumed to be either exactly the names of the properties in the strings you get from netsh, or some easily derived version thereof. In this case though there is an arbitrary difference between the firewall state property in the type file called state, and the string you get back from french netsh which is called État. You would need either new type files designed for french, or some pretty odd work in the provider to overcome that mismatch.