Support icmp rules with warden net_out

[#72801782]

Signed-off-by: Joseph Palermo <[email protected]>

warden-protocol/lib/warden/protocol/pb.rb

@@ -492,13 +492,16 @@ class NetOutRequest
       module Protocol
         TCP = 0
         UDP = 1
+        ICMP = 2
       end
 
       required :handle, :string, 1
       optional :network, :string, 2
       optional :port, :uint32, 3
       optional :port_range, :string, 4
       optional :protocol, NetOutRequest::Protocol, 5
+      optional :icmp_type, :int32, 6
+      optional :icmp_code, :int32, 7
 
     end
 

warden-protocol/lib/warden/protocol/pb/net_out.proto

@@ -12,7 +12,9 @@
 // * `port`: Port to whitelist.
 // * `port_range`: Colon separated port range (in the form `8080:9080`).
 //    Note: port_range takes precedence over port
-// * `protocol`: Network protocol (`TCP` or `UDP`).
+// * `protocol`: Network protocol (`TCP`, `UDP` or `ICMP`). Defaults to `TCP` if not set.
+// * `icmp_type`: ICMP type (use -1 to specify all ICMP types).
+// * `icmp_code`: ICMP code (use -1 to specify all ICMP codes for the given type).
 //
 // ### Response
 //
@@ -31,13 +33,16 @@ message NetOutRequest {
   enum Protocol {
     TCP = 0;
     UDP = 1;
+    ICMP = 2;
   }
   required string handle     = 1;
 
   optional string network    = 2;
   optional uint32 port       = 3;
   optional string port_range = 4;
   optional Protocol protocol = 5;
+  optional int32 icmp_type   = 6;
+  optional int32 icmp_code   = 7;
 }
 
 message NetOutResponse {

warden/lib/warden/container/features/net.rb

@@ -44,8 +44,8 @@ def restore
           end
 
           if @resources.has_key?("net_out")
-            @resources["net_out"].each do |network, port_range, protocol|
-              _net_out(network, port_range, protocol)
+            @resources["net_out"].each do |*args|
+              _net_out(*args)
             end
           end
         end
@@ -121,11 +121,13 @@ def do_net_in(request, response)
           raise
         end
 
-        def _net_out(network, port_range, protocol)
+        def _net_out(network, port_range, protocol, icmp_type, icmp_code)
           sh File.join(container_path, "net.sh"), "out", :env => {
             "NETWORK" => network,
             "PORTS"    => port_range,
             "PROTOCOL" => protocol,
+            "ICMP_TYPE" => icmp_type,
+            "ICMP_CODE" => icmp_code,
           }
         end
 
@@ -135,20 +137,27 @@ def do_net_out(request, response)
           end
 
           port_range = request.port_range || "#{request.port}"
+          validate_port_range(port_range)
+          icmp_type = nil
+          icmp_code = nil
 
-          protocol = case request.protocol
+          case request.protocol
             when Warden::Protocol::NetOutRequest::Protocol::TCP
-              "tcp"
+              protocol = "tcp"
             when Warden::Protocol::NetOutRequest::Protocol::UDP
-              "udp"
+              protocol = "udp"
+            when Warden::Protocol::NetOutRequest::Protocol::ICMP
+              icmp_type = request.icmp_type unless request.icmp_type == -1
+              icmp_code = request.icmp_code unless request.icmp_code == -1
+              protocol = "icmp"
             else
-              "tcp"
+              protocol = "tcp"
           end
 
-          _net_out(request.network, port_range, protocol)
+          _net_out(request.network, port_range, protocol, icmp_type, icmp_code)
 
           @resources["net_out"] ||= []
-          @resources["net_out"] << [request.network, port_range, protocol]
+          @resources["net_out"] << [request.network, port_range, protocol, icmp_type, icmp_code]
         end
 
         def acquire(opts = {})
@@ -187,6 +196,15 @@ def setup(config)
             self.allow_networks = config.network["allow_networks"]
           end
         end
+
+        private
+
+        def validate_port_range(port_range)
+          return if port_range.nil?
+          return unless port_range.include? ":"
+          min_port, max_port = port_range.split(":")
+          raise WardenError.new("Port range maximum must be greater than minimum") unless min_port.to_i < max_port.to_i
+        end
       end
     end
   end

warden/root/linux/skeleton/net.sh

@@ -107,27 +107,30 @@ case "${1}" in
     ;;
 
   "out")
-    if [ -z "${NETWORK:-}" ] && [ -z "${PORTS:-}" ]; then
+    if [ "${PROTOCOL:-}" != "icmp" ] && [ -z "${NETWORK:-}" ] && [ -z "${PORTS:-}" ]; then
       echo "Please specify NETWORK and/or PORTS..." 1>&2
       exit 1
     fi
 
-    opts=""
+    opts="--protocol ${PROTOCOL:-tcp}"
 
     if [ -n "${NETWORK:-}" ]; then
       opts="${opts} --destination ${NETWORK}"
     fi
 
-    if [ -n "${PROTOCOL}" ]; then
-      opts="${opts} --protocol ${PROTOCOL}"
-    else
-      opts="${opts} --protocol tcp"
-    fi
-
     if [ -n "${PORTS:-}" ]; then
       opts="${opts} --destination-port ${PORTS}"
     fi
 
+    if [ "${PROTOCOL}" == "icmp" ]; then
+      if [ -n "${ICMP_TYPE}" ]; then
+        opts="${opts} --icmp-type ${ICMP_TYPE}"
+        if [ -n "${ICMP_CODE}" ]; then
+          opts="${opts}/${ICMP_CODE}"
+        fi
+      fi
+    fi
+
     iptables -I ${filter_instance_chain} 1 ${opts} --jump RETURN
 
     ;;

warden/spec/container/linux_spec.rb

@@ -483,7 +483,7 @@ def verify_tcp_connectivity(server_container, client_container, port)
 
       unless response.stdout.strip == "ok"
         # Clean up
-        client.stop(:handle => server_container[:handle])
+        client.run(:handle => server_container[:handle], :script => "pkill -9 nc")
         return false
       end
 
@@ -506,6 +506,14 @@ def verify_udp_connectivity(server_container, client_container, port)
       response.stdout.strip == "ok"
     end
 
+    def verify_ping_connectivity(server_container, client_container)
+      # Try to ping the server container
+      client_script = "ping -c1 -w1 #{server_container[:ip]}"
+      response = run(client_container[:handle], client_script)
+
+      response.exit_status == 0
+    end
+
     context "reachability" do
       # Allow traffic to the first two subnets
       let(:allow_networks) do
@@ -546,24 +554,68 @@ def verify_udp_connectivity(server_container, client_container, port)
       describe "net_out control" do
         it "disallows traffic to networks before net_out" do
           expect(verify_tcp_connectivity(@containers[1], @containers[0], 2000)).to eq false
+          expect(verify_udp_connectivity(@containers[1], @containers[0], 2000)).to eq false
+          expect(verify_ping_connectivity(@containers[2], @containers[1])).to eq false
         end
 
         context "tcp" do
-          it "allows traffic to networks after net_out" do
+          it "allows outbound tcp traffic to networks after net_out" do
             net_out(:handle => @containers[0][:handle], :network => @containers[1][:ip], :port => 2000, :protocol => Warden::Protocol::NetOutRequest::Protocol::TCP)
             expect(verify_tcp_connectivity(@containers[1], @containers[0], 2000)).to eq true
             expect(verify_tcp_connectivity(@containers[1], @containers[0], 2001)).to eq false
           end
         end
 
         context "udp" do
-          it "allows traffic to networks after net_out" do
+          it "allows outbound udp traffic to networks after net_out" do
             net_out(:handle => @containers[0][:handle], :network => @containers[1][:ip], :port => 2000, :protocol => Warden::Protocol::NetOutRequest::Protocol::UDP)
             expect(verify_udp_connectivity(@containers[1], @containers[0], 2000)).to eq true
             expect(verify_udp_connectivity(@containers[1], @containers[0], 2001)).to eq false
           end
         end
 
+        context "icmp" do
+          # Assertions testing that containers do NOT have connectivity should only be done
+          # between containers that have NEVER had connectivity in any tests. This is because
+          # the ESTABLISHED state is cached for 30 seconds and can pollute other tests.
+          it "allows outbound icmp traffic after net out" do
+            net_out(:handle => @containers[0][:handle],
+                    :network => @containers[1][:ip],
+                    :protocol => Warden::Protocol::NetOutRequest::Protocol::ICMP,
+                    :icmp_type => 8, :icmp_code => 0) # ICMP Echo Request
+
+            expect(verify_ping_connectivity(@containers[1], @containers[0])).to eq true
+            expect(verify_ping_connectivity(@containers[2], @containers[1])).to eq false
+          end
+
+          it "allows outbound icmp traffic after net out when type and code are -1" do
+            net_out(:handle => @containers[0][:handle],
+                    :network => @containers[1][:ip],
+                    :protocol => Warden::Protocol::NetOutRequest::Protocol::ICMP,
+                    :icmp_type => -1, :icmp_code => -1) # Everything
+
+            expect(verify_ping_connectivity(@containers[1], @containers[0])).to eq true
+          end
+
+          it "does not allow outbound when type does not match" do
+            net_out(:handle => @containers[1][:handle],
+                    :network => @containers[2][:ip],
+                    :protocol => Warden::Protocol::NetOutRequest::Protocol::ICMP,
+                    :icmp_type => 0, :icmp_code => 0) # ICMP Echo Reply
+
+            expect(verify_ping_connectivity(@containers[2], @containers[1])).to eq false
+          end
+
+          it "does not allow outbound when code does not match" do
+            net_out(:handle => @containers[1][:handle],
+                    :network => @containers[2][:ip],
+                    :protocol => Warden::Protocol::NetOutRequest::Protocol::ICMP,
+                    :icmp_type => 8, :icmp_code => 8) # Bogus code
+
+            expect(verify_ping_connectivity(@containers[2], @containers[1])).to eq false
+          end
+        end
+
         context "when port ranges are specified" do
           it "should allow access to all ports in the range" do
             net_out(:handle => @containers[0][:handle], :network => @containers[1][:ip], :port_range => "2000:2002", :protocol => Warden::Protocol::NetOutRequest::Protocol::TCP)
@@ -572,12 +624,6 @@ def verify_udp_connectivity(server_container, client_container, port)
             expect(verify_tcp_connectivity(@containers[1], @containers[0], 2002)).to eq true
             expect(verify_tcp_connectivity(@containers[1], @containers[0], 1999)).to eq false
           end
-
-          it "will raise an error if min > max" do
-            expect {
-              net_out(:handle => @containers[0][:handle], :network => @containers[1][:ip], :port_range => "2002:2000", :protocol => Warden::Protocol::NetOutRequest::Protocol::TCP)
-            }.to raise_error
-          end
         end
 
         context "network using cidr" do
@@ -600,7 +646,7 @@ def verify_udp_connectivity(server_container, client_container, port)
       end
     end
 
-    describe "check network and port fields" do
+    describe "check argument handling" do
       let(:handle) { client.create.handle }
 
       it "should raise error when both fields are absent" do
@@ -620,6 +666,18 @@ def verify_udp_connectivity(server_container, client_container, port)
       it "should not raise error when both network and port fields are present" do
         net_out(:handle => handle, :network => "4.2.2.2", :port => 1234).should be_ok
       end
+
+      it "should raise an error when the port range specifies min > max" do
+        expect do
+          net_out(:handle => handle, :port_range => "2002:2000", :protocol => Warden::Protocol::NetOutRequest::Protocol::TCP)
+        end.to raise_error(Warden::Client::ServerError, %r"port range maximum must be greater than minimum"i)
+      end
+
+      it "should raise an error when an unknown protocol is specified" do
+        expect do
+          net_out(:handle => handle, :protocol => 10)
+        end.to raise_error(Warden::Protocol::ProtocolError)
+      end
     end
   end