xFirewall: Fails on port keywords

[dsccommunity]

xFirewall: Fails on port keywords

[PlagueHO]

I'll take a look and see if I can figure out what the issue is.

[PlagueHO]

Hi @nebffa ,

I did a bit of digging around and this isn't problem with the resource.

The issue is that the LocalPort parameter can only take on the parameter of PlayToDiscovery if the protocol is UDP. PlayToDiscovery is not supported by the TCP protocol in WFAS.

You can confirm this by executing:

New-NetFirewallRule `
    -Name "IIS Remote Administration" `
    -DisplayName "IIS Remote Administration" `
    -Action "Allow" `
    -Profile "Domain" `
    -Direction "InBound" `
    -RemotePort "Any" `
    -LocalPort "PlayToDiscovery" `
    -Protocol "TCP"

It will fail with the Error:
New-NetFirewallRule : One of the port keywords is invalid.
At line:1 char:1

• New-NetFirewallRule -Name "IIS Remote Administration" -DisplayName "I ...

• - CategoryInfo          : InvalidArgument: (MSFT_NetFirewallRule:root/standardcimv2/MSFT_NetFirewallRule) [New-NetFirewallRule], CimException
  - FullyQualifiedErrorId : HRESULT 0x80070057,New-NetFirewallRule
  
  

But:

New-NetFirewallRule `
    -Name "IIS Remote Administration" `
    -DisplayName "IIS Remote Administration" `
    -Action "Allow" `
    -Profile "Domain" `
    -Direction "InBound" `
    -RemotePort "Any" `
    -LocalPort "PlayToDiscovery" `
    -Protocol "UDP"

will succeed.

You can also confirm this in the Windows Firewall with Advanced Security management interface.

So either don't use the PlayToDiscovery for LocalPort or change the protocol to UDP.

HTH.

[tysonjhayes]

@PlagueHO Is there anything we can do to clear up the error logs to make that clearer for the next user?

[nebffa]

Hey @tysonjhayes , thanks that was really quick. The Microsoft documentation on that page I linked (not in this DSC resource) is clearly erroneous in that it says the keyword would work with TCP.

I noticed in other error messages that xFirewall returns, it displays the possible parameters that you could supply instead. I think that would also be useful in this case.

Interestingly, what I was trying to do from the start was set a firewall rule with LocalPort being RPC Dynamic Ports, I will play around with it to see what I can find.

[tysonjhayes]

Hey @nebffa thank @PlagueHO he did most of the work. 👍 Let us know what you can find we're trying to improve this as much as we can.

[nebffa]

Sorry, you're right @tysonjhayes . Thanks @PlagueHO as well for your help.

I played around with it a bit more. All I needed to do was use LocalPort = "RPC" to get what I need.

The Microsoft docs say If the Protocol parameter is not specified, then the acceptable values for this parameter are: RPC, RPCEPMap, Teredo, IPHTTPSIn, IPHTTPSOut, or Any. which makes it sound like you can't use these parameters if the Protocol parameter is being used.

Perhaps an option for clearer error logs would be something like:

The supplied value for "LocalPort" is invalid - it can be one of the following: 
 * A number between "0-65536"
 * "RPC"
 * "RPCEPMap"
etc. etc.

[PlagueHO]

Hi @tysonjhayes and @nebffa ,

When I was investigating this I found the New-NetFirewallRule cmdlet docs are definitely not too clear on this - which is what had be scratching my head as well. I've added a community addition to the documentation on this page.

@tysonjhayes, it would definitely be possible to sanitize the parameters in the Test-TargetResource (perhaps call out to a Test-Parameters function like we do in the other resources) and throw a slightly more descriptive error message should this be encountered. The only problem with this is that if an existing rule is being modified then might still slip through.

E.g. if a rule exists:
LocalPort: PlayToDiscovery, Protocol: UDP

And a resource is created that changes the Protocol to TCP (without modifying the LocalPort) then this test wouldn't flag the issue. It could be made to do this (compare both existing values and new values) but it would increase complexity.

A better way might be to just try and catch that specific exception when creating/updating the firewall rule.

But perhaps the best solution is just to document this in the Readme.md in a list of known bad combinations (because I'm sure there are a few of them that aren't documented). :)

[tysonjhayes]

We could start with a list of known bad combinations, and expand from there, I don't really want to write a bunch of code to handle everything as one off combinations just to bubble up the excepton properly. I was more thinking could we get it to say "One of the port keywords is invalid." which was the actual exception. I'll look at the code later.

Side note, I was noticing that this came out of the verbose logs:

VERBOSE: [PERFTEST01]:  [[xFirewall]IIS Remote Administration] Test-RuleProperties: LocalPort
 property value 'System.String[]' does not match desired state 'System.String[]'.

Which suggests we may not be comparing that array correctly or at least not displaying it in the verbose stream. I'd expect it to say 'LocalPort property value 123456 does not match desired state 80' or something to that effect.

[PlagueHO]

Regarding the "One of the port keywords is invalid" exception, when I actually tested this problem and ran the config through my test systems, that was actually the error that was reported to me in the Verbose log. This did strike me as odd given that the error the logs @nebffa posted shows a different error. I might do some additional investigation on this as I don't like mysteries...

And good catch with the bad messages - I ran into and fixed the same thing in the last PR I raised (pushing an array into a format string must be first joined). I can pop a fix through for this as well and see if I can identify any others if you like?

[PlagueHO]

After further experimentation, I found that the message "One of the port keywords is invalid" message is reported in the Verbose log if the firewall rule does not yet exist:

VERBOSE: [SA_FS1]: LCM:  [ Start  Resource ]  [[xFirewall]IIS Remote Administration]
VERBOSE: [SA_FS1]: LCM:  [ Start  Test     ]  [[xFirewall]IIS Remote Administration]
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Test-TargetResource: Checking settings for firewall rule with Name 'IIS Remote
Administration'.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Test-TargetResource: Find firewall rule with Name 'IIS Remote Administration'.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Get-FirewallRule: No Firewall Rule found with Name 'IIS Remote Administration'.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Test-TargetResource: Firewall rule with Name 'IIS Remote Administration' does not
exist.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Test-TargetResource: Check Firewall rule with Name 'IIS Remote Administration'
returning False.
VERBOSE: [SA_FS1]: LCM:  [ End    Test     ]  [[xFirewall]IIS Remote Administration]  in 0.1250 seconds.
VERBOSE: [SA_FS1]: LCM:  [ Start  Set      ]  [[xFirewall]IIS Remote Administration]
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Set-TargetResource: Applying settings for firewall rule with Name 'IIS Remote
Administration'.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Set-TargetResource: Find firewall rule with Name 'IIS Remote Administration'.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Get-FirewallRule: No Firewall Rule found with Name 'IIS Remote Administration'.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Set-TargetResource: We want the firewall rule with Name 'IIS Remote
Administration' to exist since Ensure is set to Present.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] Set-TargetResource: We want the firewall rule with Name 'IIS Remote
Administration' to exist, but it does not.
VERBOSE: [SA_FS1]:                            [[xFirewall]IIS Remote Administration] New-NetFirewallRule DisplayName: IIS Remote Administration
One of the port keywords is invalid.
    + CategoryInfo          : InvalidArgument: (MSFT_NetFirewallRule:) [], CimException
    + FullyQualifiedErrorId : HRESULT 0x80070057,New-NetFirewallRule
    + PSComputerName        : SA_FS1

VERBOSE: [SA_FS1]: LCM:  [ End    Set      ]  [[xFirewall]IIS Remote Administration]  in 0.2650 seconds.
The PowerShell DSC resource MSFT_xFirewall threw one or more non-terminating errors while running the Set-TargetResource functionality. These errors are logged to the
ETW channel called Microsoft-Windows-DSC/Operational. Refer to this channel for more details.
    + CategoryInfo          : InvalidOperation: (:) [], CimException
    + FullyQualifiedErrorId : NonTerminatingErrorFromProvider
    + PSComputerName        : SA_FS1

VERBOSE: [SA_FS1]: LCM:  [ End    Set      ]
The SendConfigurationApply function did not succeed.
    + CategoryInfo          : NotSpecified: (root/Microsoft/...gurationManager:String) [], CimException
    + FullyQualifiedErrorId : MI RESULT 1
    + PSComputerName        : SA_FS1

VERBOSE: Operation 'Invoke CimMethod' complete.
VERBOSE: Time taken for configuration job to complete is 20.693 seconds

If a version of the firewall rule already exists (regardless of how that rule is configured), then that is when the error that @nebffa's log is showing. So in summary, the odd error message only occurs when updating an existing rule with a bad combination. I'll see if I can figure out a way to resolve it.

[PlagueHO]

I've found and fixed the issue where the error is being misreported. It is actually a slightly more significant bug in that any time the DisplayName is passed when the rule already exists an error would be thrown. I'm going to add some pester tests to check for this issue in future, but I'm away for the rest of this week so I'll do it first thing next week. Thanks all.

[tysonjhayes]

No rush, I think you've documented it fairly well up to this point. I'm sure anyone who runs into this until we've gotten a fix in will be able to find this issue and sort it out. 😁 Thanks for looking into this.

[tysonjhayes]

This was fixed in #37 which is now merged. Please reopen this if you are still blocked on the latest code.