Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Firewall: If Builtin rule does not exist than a wide open rule gets created #422

Open
invisibleninja06 opened this issue Oct 16, 2019 · 5 comments
Labels
bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community.

Comments

@invisibleninja06
Copy link

Details of the scenario you tried and the problem that is occurring

If you try to enable a predefined rule that does not already exist than instead you end up with a wide open rule permitting all traffic inbound.

This could potentially cause security holes to someone that does not notice.

Firewall export of the rule as CSV
RemoteFwAdmin-In-TCP,,All,Yes,Allow,No,Any,Any,Any,Any,Any,Any,Any,Any,Any,Any,Any,Any,

Verbose logs showing the problem

Suggested solution to the issue

Perhaps send an error to the user indicating that the builtin rule they are trying to use does not exist and do not create a wide open rule.

The DSC configuration that is used to reproduce the issue (as detailed as possible)

Configuration Firewall_EnableBuiltInFirewallRule_Config
{
    Import-DSCResource -ModuleName NetworkingDsc

    Node localhost
    {
        Firewall EnableBuiltInFirewallRule
        {
            Name                  = 'RemoteFwAdmin-In-TCP'
            Ensure                = 'Present'
            Enabled               = 'True'
        }
    }
}

# insert configuration here

instance of MSFT_Firewall as $MSFT_Firewall51ref
{
ResourceID = "[Firewall]RemoteFwAdmin-In-TCP";
Enabled = "True";
Ensure = "Present";
Name = "RemoteFwAdmin-In-TCP";
ModuleName = "NetworkingDSC";
ModuleVersion = "7.3.0.1";

};

The operating system the target node is running

OsName : Microsoft Windows Server 2016 Standard
OsOperatingSystemSKU : StandardServerEdition
OsArchitecture : 64-bit
WindowsBuildLabEx : 14393.3204.amd64fre.rs1_release.190830-1500
OsLanguage : en-US
OsMuiLanguages : {en-US}

Version and build of PowerShell the target node is running

Name Value


PSVersion 5.1.14393.3053
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.14393.3053
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

Version of the DSC module that was used ('dev' if using current dev branch)

ModuleName = "NetworkingDSC";
ModuleVersion = "7.3.0.1"

@n3snah
Copy link

n3snah commented Oct 17, 2019

I have got the same issue but i can't even get the prebuilt rules to match.
i find that it is just creating a completely open rule if I follow example 4.

After looking into this further, it appears the built in rules use a different name to the display name which if you didn't know then comes back to opening an allow all rule just like the rule never existed.
You can find the correct rule name by searching for the rule in powershell using the Get-NetFirewallRule command.

@invisibleninja06
Copy link
Author

Thanks @laywah2016 for confirming my suspicions.
In my example i am using the name 'RemoteFwAdmin-In-TCP' and not display name 'Windows Firewall Remote Management (RPC)' and even so it ends up creating a completely open rule.

If i go a bit further and start to specify the protocol, service, program, etc. at some point it seems to realize its a builtin because it will complain that the display name and description do not match when pushed to a 2019 server where the same named rule goes by the display name Windows Defender Firewall Remote Management (RPC).

It really seems a bit flaky to me and the builtin detection is not working as described in example 4.
The fact that it makes a wide open rule disturbs me a bit.
I havent found the time yet to look over the common code but i suspect that maybe having a Builtin = True might be an idea to force dsc to try to find and a matching predefined rule and if not error and not create the rule.

@PlagueHO PlagueHO added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. labels Oct 18, 2019
@PlagueHO
Copy link
Member

@laywah2016 - you are quite correct. For the built in rules, Windows often represents these with Guids and then uses the DisplayName to set a friendly name.

@invisibleninja06 - you're right - this is definitely a security hole and should be fixed. The rule detection code uses:

Get-NetFirewallRule -Name RemoteFwAdmin-In-TCP

Are you able to share the Verbose DSC logs so I can see what flow the resource is taking to get to that point?

@invisibleninja06
Copy link
Author

I manually imported the get-firewallrule code as well as the convert function it requires and it does return null. My thought on builtin rules was that it would apply to predefined rules even if they are not present however that is not the case since the function is using
Get-NetFirewallRule -Name (ConvertTo-FirewallRuleNameEscapedString -Name $Name).
Even if i do a Get-NetFirewallRule -Name 'RemoteFwAdmin-In-TCP' I get null.

Looking deeper at predefined rules they are actually defined in, [HKLM\SYSTEM\CurrentControlSet\services\SharedAccess\Defaults\FirewallPolicy\FirewallRules]
Would be neat if the code could support predefined rules and grab the parameters from registry.

As for the security issue i believe the problem is this bit of code as it never validates if it has parameters and thus can create a wide open rule when the user actually wanted to enable a builtin rule that was missing or removed.

else
        {
            Write-Verbose -Message ( @( "$($MyInvocation.MyCommand): "
                    $($script:localizedData.FirewallRuleShouldExistAndDoesNotMessage) -f $Name
                ) -join '')

            # Set any default parameter values
            if (-not $DisplayName)
            {
                if (-not $PSBoundParameters.ContainsKey('DisplayName'))
                {
                    $null = $PSBoundParameters.Add('DisplayName', $Name)
                }
                else
                {
                    $PSBoundParameters.DisplayName = $Name
                }
            }

            # Add the new Firewall rule based on specified parameters
            New-NetFirewallRule @PSBoundParameters
        }

My first thought would be an if statement in the else that checks if ports, services and address's are not defined since one should be defined in any proper firewall rule.

Thoughts?

@PlagueHO
Copy link
Member

@invisibleninja06 - thank you for helping figure this one out! It is a strange one.

When I run:

Get-NetFirewallRule -Name 'RemoteFwAdmin-In-TCP'

I do get a result:
image

This is the same on Windows Server 2019.

But it might be possible to get the function to look up pre-defined rules.

I think you're also right about the code above: It should probably not create a new rule if it doesn't at least have a minimum set of parameters. I suspect there might be a bit more to such a change though and it needs a bit more investigation to check the impact of such a change won't have any undesired effects. Still, it is worth investigating because as you say, the current behavior is a risk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community.
Projects
None yet
Development

No branches or pull requests

3 participants