Merge pull request #9544 from pschaumburg/ps/windows_firewall_rule-rework

windows_firewall_rule: Fix idempotency and add icmp_type property

2 files changed, 173 insertions, 52 deletions

diff --git a/lib/chef/resource/windows_firewall_rule.rb b/lib/chef/resource/windows_firewall_rule.rb
index 2f6bcebc82..2b04bb2aa2 100644
--- a/lib/chef/resource/windows_firewall_rule.rb
+++ b/lib/chef/resource/windows_firewall_rule.rb
@@ -28,13 +28,51 @@ class Chef
 
       description "Use the windows_firewall_rule resource to create, change or remove windows firewall rules."
       introduced "14.7"
+      examples <<~DOC
+      Allowing port 80 access
+      ```ruby
+      windows_firewall_rule 'IIS' do
+        local_port '80'
+        protocol 'TCP'
+        firewall_action :allow
+      end
+      ```
+
+      Allow protocol ICMPv6 with ICMP Type
+      ```ruby
+      windows_firewall_rule 'CoreNet-Rule' do
+        rule_name 'CoreNet-ICMP6-LR2-In'
+        display_name 'Core Networking - Multicast Listener Report v2 (ICMPv6-In)'
+        local_port 'RPC'
+        protocol 'ICMPv6'
+        icmp_type '8'
+      end
+      ```
+
+      Blocking WinRM over HTTP on a particular IP
+      ```ruby
+      windows_firewall_rule 'Disable WinRM over HTTP' do
+        local_port '5985'
+        protocol 'TCP'
+        firewall_action :block
+        local_address '192.168.1.1'
+      end
+      ```
+
+      Deleting an existing rule
+      ```ruby
+      windows_firewall_rule 'Remove the SSH rule' do
+        rule_name 'ssh'
+        action :delete
+      end
+      ```
+      DOC
 
       property :rule_name, String,
         name_property: true,
         description: "An optional property to set the name of the firewall rule to assign if it differs from the resource block's name."
 
       property :description, String,
-        default: "Firewall rule",
         description: "The description to assign to the firewall rule."
 
       property :displayname, String,
@@ -71,6 +109,11 @@ class Chef
         default: "TCP",
         description: "The protocol the firewall rule applies to."
 
+      property :icmp_type, [String, Integer],
+        description: "Specifies the ICMP Type parameter for using a protocol starting with ICMP",
+        default: "Any",
+        introduced: "16.0"
+
       property :firewall_action, [Symbol, String],
         default: :allow, equal_to: %i{allow block notconfigured},
         description: "The action of the firewall rule.",
@@ -128,6 +171,7 @@ class Chef
         remote_port Array(state["remote_port"]).sort
         direction state["direction"]
         protocol state["protocol"]
+        icmp_type state["icmp_type"]
         firewall_action state["firewall_action"]
         profile current_profiles
         program state["program"]
@@ -138,11 +182,10 @@ class Chef
 
       action :create do
         description "Create a Windows firewall entry."
-
         if current_resource
           converge_if_changed :rule_name, :description, :displayname, :local_address, :local_port, :remote_address,
-            :remote_port, :direction, :protocol, :firewall_action, :profile, :program, :service, :interface_type,
-            :enabled do
+            :remote_port, :direction, :protocol, :icmp_type, :firewall_action, :profile, :program, :service,
+            :interface_type, :enabled do
               cmd = firewall_command("Set")
               powershell_out!(cmd)
             end
@@ -186,6 +229,7 @@ class Chef
           cmd << " -RemotePort '#{new_resource.remote_port.join("', '")}'" if new_resource.remote_port
           cmd << " -Direction '#{new_resource.direction}'" if new_resource.direction
           cmd << " -Protocol '#{new_resource.protocol}'" if new_resource.protocol
+          cmd << " -IcmpType '#{new_resource.icmp_type}'"
           cmd << " -Action '#{new_resource.firewall_action}'" if new_resource.firewall_action
           cmd << " -Profile '#{new_resource.profile.join("', '")}'" if new_resource.profile
           cmd << " -Program '#{new_resource.program}'" if new_resource.program
@@ -195,12 +239,53 @@ class Chef
 
           cmd
         end
+
+        def define_resource_requirements
+          requirements.assert(:create) do |a|
+            a.assertion do
+              if new_resource.icmp_type.is_a?(String)
+                !new_resource.icmp_type.empty?
+              elsif new_resource.icmp_type.is_a?(Integer)
+                !new_resource.icmp_type.nil?
+              end
+            end
+            a.failure_message("The :icmp_type property can not be empty in #{new_resource.rule_name}")
+          end
+
+          requirements.assert(:create) do |a|
+            a.assertion do
+              if new_resource.icmp_type.is_a?(Integer)
+                new_resource.protocol.start_with?("ICMP")
+              elsif new_resource.icmp_type.is_a?(String) && !new_resource.protocol.start_with?("ICMP")
+                new_resource.icmp_type == "Any"
+              else
+                true
+              end
+            end
+            a.failure_message("The :icmp_type property has a value of #{new_resource.icmp_type} set, but is not allowed for :protocol #{new_resource.protocol} in #{new_resource.rule_name}")
+          end
+
+          requirements.assert(:create) do |a|
+            a.assertion do
+              if new_resource.icmp_type.is_a?(Integer)
+                (0..255).include?(new_resource.icmp_type)
+              elsif new_resource.icmp_type.is_a?(String) && !new_resource.icmp_type.include?(":") && new_resource.protocol.start_with?("ICMP")
+                (0..255).include?(new_resource.icmp_type.to_i)
+              elsif new_resource.icmp_type.is_a?(String) && new_resource.icmp_type.include?(":") && new_resource.protocol.start_with?("ICMP")
+                new_resource.icmp_type.split(":").all? { |type| (0..255).include?(type.to_i) }
+              else
+                true
+              end
+            end
+            a.failure_message("Can not set :icmp_type to #{new_resource.icmp_type} as one value is out of range (0 to 255) in #{new_resource.rule_name}")
+          end
+        end
       end
 
       private
 
       # build the command to load the current resource
-      # # @return [String] current firewall state
+      # @return [String] current firewall state
       def load_firewall_state(rule_name)
         <<-EOH
           Remove-TypeData System.Array # workaround for PS bug here: https://bit.ly/2SRMQ8M
@@ -221,6 +306,7 @@ class Chef
             remote_port = $portFilter.RemotePort
             direction = $rule.Direction.ToString()
             protocol = $portFilter.Protocol
+            icmp_type = $portFilter.IcmpType
             firewall_action = $rule.Action.ToString()
             profile = $rule.Profile.ToString()
             program = $applicationFilter.Program
diff --git a/spec/unit/resource/windows_firewall_rule_spec.rb b/spec/unit/resource/windows_firewall_rule_spec.rb
index 16d48ec348..f4dfea1e0a 100644
--- a/spec/unit/resource/windows_firewall_rule_spec.rb
+++ b/spec/unit/resource/windows_firewall_rule_spec.rb
@@ -48,6 +48,11 @@ describe Chef::Resource::WindowsFirewallRule do
     expect(resource.description).to eql("firewall rule")
   end
 
+  it "the group property accepts strings" do
+    resource.group("New group")
+    expect(resource.group).to eql("New group")
+  end
+
   it "the local_address property accepts strings" do
     resource.local_address("192.168.1.1")
     expect(resource.local_address).to eql("192.168.1.1")
@@ -125,6 +130,16 @@ describe Chef::Resource::WindowsFirewallRule do
     expect(resource.protocol).to eql("TCP")
   end
 
+  it "the icmp_type property accepts strings" do
+    resource.icmp_type("Any")
+    expect(resource.icmp_type).to eql("Any")
+  end
+
+  it "the icmp_type property accepts integers" do
+    resource.icmp_type(8)
+    expect(resource.icmp_type).to eql(8)
+  end
+
   it "the firewall_action property accepts :allow, :block and :notconfigured" do
     resource.firewall_action(:allow)
     expect(resource.firewall_action).to eql(:allow)
@@ -234,217 +249,236 @@ describe Chef::Resource::WindowsFirewallRule do
 
     context "#new" do
       it "build a minimal command" do
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets a description" do
         resource.description("New description")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'New description' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'New description' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets a displayname" do
         resource.displayname("New displayname")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'New displayname' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'New displayname' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+      end
+
+      it "sets a group" do
+        resource.group("New groupname")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Group 'New groupname' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets LocalAddress" do
         resource.local_address("127.0.0.1")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -LocalAddress '127.0.0.1' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -LocalAddress '127.0.0.1' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets LocalPort" do
         resource.local_port("80")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -LocalPort '80' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -LocalPort '80' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets LocalPort with int" do
         resource.local_port(80)
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -LocalPort '80' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -LocalPort '80' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets multiple LocalPorts" do
         resource.local_port(%w{80 RPC})
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -LocalPort '80', 'RPC' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -LocalPort '80', 'RPC' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets RemoteAddress" do
         resource.remote_address("8.8.8.8")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -RemoteAddress '8.8.8.8' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -RemoteAddress '8.8.8.8' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets RemotePort" do
         resource.remote_port("443")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -RemotePort '443' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -RemotePort '443' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets RemotePort with int" do
         resource.remote_port(443)
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -RemotePort '443' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -RemotePort '443' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets multiple RemotePorts" do
         resource.remote_port(%w{443 445})
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -RemotePort '443', '445' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -RemotePort '443', '445' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Direction" do
         resource.direction(:outbound)
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'outbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'outbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Protocol" do
         resource.protocol("UDP")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'UDP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'UDP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+      end
+
+      it "sets ICMP Protocol with type 8" do
+        resource.protocol("ICMPv6")
+        resource.icmp_type(8)
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'ICMPv6' -IcmpType '8' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Action" do
         resource.firewall_action(:block)
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'block' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'block' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Profile" do
         resource.profile(:private)
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'private' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'private' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets multiple Profiles (must be comma-plus-space delimited for PowerShell to treat as an array)" do
         resource.profile(%i{private public})
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'private', 'public' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'private', 'public' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Program" do
         resource.program("C:/calc.exe")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -Program 'C:/calc.exe' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -Program 'C:/calc.exe' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Service" do
         resource.service("Spooler")
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -Service 'Spooler' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -Service 'Spooler' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets InterfaceType" do
         resource.interface_type(:wired)
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'wired' -Enabled 'true'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'wired' -Enabled 'true'")
       end
 
       it "sets Enabled" do
         resource.enabled(false)
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'false'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule' -DisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'false'")
       end
 
-      it "sets all properties" do
+      it "sets all properties UDP" do
         resource.rule_name("test_rule_the_second")
-        resource.description("some other rule")
         resource.displayname("some cool display name")
+        resource.description("some other rule")
+        resource.group("new group")
         resource.local_address("192.168.40.40")
         resource.local_port("80")
         resource.remote_address("8.8.4.4")
         resource.remote_port("8081")
         resource.direction(:outbound)
         resource.protocol("UDP")
+        resource.icmp_type("Any")
         resource.firewall_action(:notconfigured)
         resource.profile(:domain)
         resource.program('%WINDIR%\System32\lsass.exe')
         resource.service("SomeService")
         resource.interface_type(:remoteaccess)
         resource.enabled(false)
-        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule_the_second' -DisplayName 'some cool display name' -Description 'some other rule' -LocalAddress '192.168.40.40' -LocalPort '80' -RemoteAddress '8.8.4.4' -RemotePort '8081' -Direction 'outbound' -Protocol 'UDP' -Action 'notconfigured' -Profile 'domain' -Program '%WINDIR%\\System32\\lsass.exe' -Service 'SomeService' -InterfaceType 'remoteaccess' -Enabled 'false'")
+        expect(provider.firewall_command("New")).to eql("New-NetFirewallRule -Name 'test_rule_the_second' -DisplayName 'some cool display name' -Group 'new group' -Description 'some other rule' -LocalAddress '192.168.40.40' -LocalPort '80' -RemoteAddress '8.8.4.4' -RemotePort '8081' -Direction 'outbound' -Protocol 'UDP' -IcmpType 'Any' -Action 'notconfigured' -Profile 'domain' -Program '%WINDIR%\\System32\\lsass.exe' -Service 'SomeService' -InterfaceType 'remoteaccess' -Enabled 'false'")
       end
     end
 
     context "#set" do
       it "build a minimal command" do
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
-      end
-
-      it "sets a description" do
-        resource.description("New description")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'New description' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets a displayname" do
         resource.displayname("New displayname")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'New displayname' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'New displayname' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+      end
+
+      it "sets a description" do
+        resource.description("New description")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'New description' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets LocalAddress" do
         resource.local_address("127.0.0.1")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -LocalAddress '127.0.0.1' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -LocalAddress '127.0.0.1' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets LocalPort" do
         resource.local_port("80")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -LocalPort '80' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -LocalPort '80' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets LocalPort with int" do
         resource.local_port(80)
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -LocalPort '80' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -LocalPort '80' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets multiple LocalPorts (must be comma-plus-space delimited for PowerShell to treat as an array)" do
         resource.local_port(%w{80 8080})
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -LocalPort '80', '8080' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -LocalPort '80', '8080' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets RemoteAddress" do
         resource.remote_address("8.8.8.8")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -RemoteAddress '8.8.8.8' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -RemoteAddress '8.8.8.8' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets RemotePort" do
         resource.remote_port("443")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -RemotePort '443' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -RemotePort '443' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets RemotePort with int" do
         resource.remote_port(443)
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -RemotePort '443' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -RemotePort '443' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets multiple RemotePorts (must be comma-plus-space delimited for PowerShell to treat as an array)" do
         resource.remote_port(%w{443 445})
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -RemotePort '443', '445' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -RemotePort '443', '445' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Direction" do
         resource.direction(:outbound)
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'outbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'outbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Protocol" do
         resource.protocol("UDP")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'UDP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'UDP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+      end
+
+      it "sets ICMP Protocol with type 8" do
+        resource.protocol("ICMPv6")
+        resource.icmp_type(8)
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'ICMPv6' -IcmpType '8' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Action" do
         resource.firewall_action(:block)
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'block' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'block' -Profile 'any' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Profile" do
         resource.profile(:private)
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'private' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'private' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Program" do
         resource.program("C:/calc.exe")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -Program 'C:/calc.exe' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -Program 'C:/calc.exe' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets Service" do
         resource.service("Spooler")
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -Service 'Spooler' -InterfaceType 'any' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -Service 'Spooler' -InterfaceType 'any' -Enabled 'true'")
       end
 
       it "sets InterfaceType" do
         resource.interface_type(:wired)
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'wired' -Enabled 'true'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'wired' -Enabled 'true'")
       end
 
       it "sets Enabled" do
         resource.enabled(false)
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Description 'Firewall rule' -Direction 'inbound' -Protocol 'TCP' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'false'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule' -NewDisplayName 'test_rule' -Direction 'inbound' -Protocol 'TCP' -IcmpType 'Any' -Action 'allow' -Profile 'any' -InterfaceType 'any' -Enabled 'false'")
       end
 
       it "sets all properties" do
@@ -457,13 +491,14 @@ describe Chef::Resource::WindowsFirewallRule do
         resource.remote_port("8081")
         resource.direction(:outbound)
         resource.protocol("UDP")
+        resource.icmp_type("Any")
         resource.firewall_action(:notconfigured)
         resource.profile(:domain)
         resource.program('%WINDIR%\System32\lsass.exe')
         resource.service("SomeService")
         resource.interface_type(:remoteaccess)
         resource.enabled(false)
-        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule_the_second' -NewDisplayName 'some cool display name' -Description 'some other rule' -LocalAddress '192.168.40.40' -LocalPort '80' -RemoteAddress '8.8.4.4' -RemotePort '8081' -Direction 'outbound' -Protocol 'UDP' -Action 'notconfigured' -Profile 'domain' -Program '%WINDIR%\\System32\\lsass.exe' -Service 'SomeService' -InterfaceType 'remoteaccess' -Enabled 'false'")
+        expect(provider.firewall_command("Set")).to eql("Set-NetFirewallRule -Name 'test_rule_the_second' -NewDisplayName 'some cool display name' -Description 'some other rule' -LocalAddress '192.168.40.40' -LocalPort '80' -RemoteAddress '8.8.4.4' -RemotePort '8081' -Direction 'outbound' -Protocol 'UDP' -IcmpType 'Any' -Action 'notconfigured' -Profile 'domain' -Program '%WINDIR%\\System32\\lsass.exe' -Service 'SomeService' -InterfaceType 'remoteaccess' -Enabled 'false'")
       end
     end
   end