chef-boneyard / windows_firewall

Chef cookbook to configure Windows Firewall
Apache License 2.0
6 stars 9 forks source link

windows_firewall_rule no longer handles comma lists for local port flag correctly #35

Closed Ryuzavi closed 5 years ago

Ryuzavi commented 5 years ago

Cookbook version

5.0.0

Chef-client version

14.0.202

Platform Details

Windows Server 2016

Scenario:

Define a Windows Firewall rule for mulitple ports, e.g. 80 and 443.

Steps to Reproduce:

Before the 5.0.0 update we were able to define multiple remote ports in a comma delimited list, e.g. '80,443'. This now fails with the below error.

After some digging, it appears the speciifc cause here is that the comma lists are passed as quoted strings to the new powershell command, e.g. -LocalPort '80,443'. If you pass the lists without the quotes the command works as expected, e.g. -LocalPort 80,443.

Expected Result:

Able to define Windows Firewall rules for port lists instead of duplicating them for each port individually.

Actual Result:

New-NetFirewallRule : The port is invalid.  When Protocol is TCP or UDP, individual ports or ranges are allowed.  Also, the following port keywords are allowed on Firewall Rules: RPC,
RPCEPMap, Teredo, IPHTTPSIn, IPHTTPSOut, PlayToDiscovery.
welcomebot commented 5 years ago

Hey There It looks like this is the first issue you've filed against the chef-cookbooks project. I'm here to offer you a bit of extra help to make sure we can quickly get back to you. Make sure you've filled out all the fields in our issue template. Make sure you've provided us with the version of chef-client you're running, your operating system and the version of the cookbook. If you're not using the most up to date version of the cookbook then please make sure to update first. Lots of things change between versions even if you're issue isn't listed in the changelog. Finally please give us a detailed description of the issue you're having. The more we know about what you're trying to do, what actually happens, and how you can reproduce the problem, the better.

If you're looking for more immediate troubleshooting help make sure to check out #general on the Chef Community Slack. There's plenty of folks there willing to lend a helping hand. Thanks for the first issue. We hope we can get back to you soon with a solution.

tas50 commented 5 years ago

@Ryuzavi thanks for this issue. This one took a while because it actually requires some pretty extensive refactoring to those properties and how we get data from the system on the current firewall state. The fix is in 5.0.1 and I updated the resource we moved into chef-client so 14.7 will ship with this fix as well. Thanks for pointing it out.

Ryuzavi commented 5 years ago

Ah brilliant, thanks for this.

Apologies though I should've made it more clear that another property is affected in the same way - remote_address:

STDERR: New-NetFirewallRule : The address is invalid.  Addresses may be specified as IP addresses, ranges, or subnets.  Also, the following address keywords are allowed in certain places: LocalSubnet, DNS, DHCP, WINS, DefaultGateway, Internet, Intranet, IntranetRemoteAccess, PlayToDevice.  Keywords can be restricted to IPv4 or IPv6 by appending a 4 or 6.

Again this is due to the comma list of remote addresses being place in quotes. When the list is passed without quotes it works fine. Should I raise a separate issue @tas50?

Ryuzavi commented 5 years ago

I've raised a separate issue for the Remote Address flag.

Cheers, R.