Issue 4404: Add support for "icmpv6" protocol to ASGs

* "icmpv6" can be used if "enable_ipv6" is configured
* "icmp" destinations may only consist of IPv4 addresses
* "icmpv6" destinations may only consist of IPv6 addresses

Author: jochenehret

app/messages/validators/security_group_rule_validator.rb

@@ -27,14 +27,22 @@ def validate(record)
 
       validate_allowed_keys(rule, record, index)
 
-      add_rule_error("protocol must be 'tcp', 'udp', 'icmp', or 'all'", record, index) unless valid_protocol(rule[:protocol])
+      add_rule_error("protocol must be 'tcp', 'udp', 'icmp', 'icmpv6' or 'all'", record, index) unless valid_protocol(rule[:protocol])
+
+      if rule[:protocol] == 'icmp'
+        allowed_ip_version = NetAddr::IPv4Net
+      elsif rule[:protocol] == 'icmpv6'
+        allowed_ip_version = NetAddr::IPv6Net
+      else
+        allowed_ip_version = nil
+      end
 
       if valid_destination_type(rule[:destination], record, index)
         destinations = rule[:destination].split(',', -1)
         add_rule_error("maximum destinations per rule exceeded - must be under #{MAX_DESTINATIONS_PER_RULE}", record, index) unless destinations.length <= MAX_DESTINATIONS_PER_RULE
 
         destinations.each do |d|
-          validate_destination(d, record, index)
+          validate_destination(d, rule[:protocol], allowed_ip_version, record, index)
         end
       end
 
@@ -46,6 +54,9 @@ def validate(record)
         validate_tcp_udp_protocol(rule, record, index)
       when 'icmp'
         validate_icmp_protocol(rule, record, index)
+      when 'icmpv6'
+        add_rule_error("icmpv6 cannot be used if enable_ipv6 is false", record, index) unless CloudController::RuleValidator.ipv6_enabled?
+        validate_icmp_protocol(rule, record, index)
       when 'all'
         add_rule_error('ports are not allowed for protocols of type all', record, index) if rule[:ports]
       end
@@ -57,7 +68,7 @@ def boolean?(value)
   end
 
   def valid_protocol(protocol)
-    protocol.is_a?(String) && %w[tcp udp icmp all].include?(protocol)
+    protocol.is_a?(String) && %w[tcp udp icmp icmpv6 all].include?(protocol)
   end
 
   def validate_allowed_keys(rule, record, index)
@@ -128,7 +139,7 @@ def valid_destination_type(destination, record, index)
     true
   end
 
-  def validate_destination(destination, record, index)
+  def validate_destination(destination, protocol, allowed_ip_version, record, index)
     error_message = 'destination must be a valid CIDR, IP address, or IP address range'
     error_message = 'destination must contain valid CIDR(s), IP address(es), or IP address range(s)' if CloudController::RuleValidator.comma_delimited_destinations_enabled?
     add_rule_error('empty destination specified in comma-delimited list', record, index) if destination.empty?
@@ -139,8 +150,9 @@ def validate_destination(destination, record, index)
     add_rule_error(zeros_error_message, record, index) unless CloudController::RuleValidator.no_leading_zeros(address_list)
 
     if address_list.length == 1
-      add_rule_error(error_message, record, index) unless CloudController::RuleValidator.parse_ip(address_list.first)
-
+      parsed_ip = CloudController::RuleValidator.parse_ip(address_list.first)
+      add_rule_error(error_message, record, index) unless parsed_ip
+      add_rule_error("for protocol \"#{protocol}\" you cannot use IPv#{parsed_ip.version} addresses", record, index) unless parsed_ip.nil? || allowed_ip_version.nil? || parsed_ip.is_a?(allowed_ip_version)
     elsif address_list.length == 2
       ips = CloudController::RuleValidator.parse_ip(address_list)
       return add_rule_error('destination IP address range is invalid', record, index) unless ips
@@ -153,6 +165,7 @@ def validate_destination(destination, record, index)
 
       reversed_range_error = 'beginning of IP address range is numerically greater than the end of its range (range endpoints are inverted)'
       add_rule_error(reversed_range_error, record, index) unless ips.first == sorted_ips.first
+      add_rule_error("for protocol \"#{protocol}\" you cannot use IPv#{ips.first.version} addresses", record, index) unless ips.first.nil? || allowed_ip_version.nil? || ips.first.is_a?(allowed_ip_version)
 
     else
       add_rule_error(error_message, record, index)

lib/cloud_controller/rule_validator.rb

@@ -77,6 +77,10 @@ def self.parse_ip(val)
       ipv4 || ipv6
     end
 
+    def self.ipv6_enabled?
+      config.get(:enable_ipv6)
+    end
+
     def self.comma_delimited_destinations_enabled?
       config.get(:security_groups, :enable_comma_delimited_destinations)
     end

spec/unit/messages/validators/security_group_rule_validator_spec.rb

@@ -48,9 +48,9 @@ def self.name
 
       it 'returns indexed errors corresponding to each invalid rule' do
         expect(subject).not_to be_valid
-        expect(subject.errors.full_messages).to include "Rules[0]: protocol must be 'tcp', 'udp', 'icmp', or 'all'"
+        expect(subject.errors.full_messages).to include "Rules[0]: protocol must be 'tcp', 'udp', 'icmp', 'icmpv6' or 'all'"
         expect(subject.errors.full_messages).to include 'Rules[0]: destination must be a valid CIDR, IP address, or IP address range'
-        expect(subject.errors.full_messages).to include "Rules[1]: protocol must be 'tcp', 'udp', 'icmp', or 'all'"
+        expect(subject.errors.full_messages).to include "Rules[1]: protocol must be 'tcp', 'udp', 'icmp', 'icmpv6' or 'all'"
         expect(subject.errors.full_messages).to include 'Rules[1]: destination must be a valid CIDR, IP address, or IP address range'
       end
     end
@@ -1064,7 +1064,7 @@ def self.name
 
         it 'adds an error' do
           expect(subject).not_to be_valid
-          expect(subject.errors.full_messages).to include "Rules[0]: protocol must be 'tcp', 'udp', 'icmp', or 'all'"
+          expect(subject.errors.full_messages).to include "Rules[0]: protocol must be 'tcp', 'udp', 'icmp', 'icmpv6' or 'all'"
         end
       end
 
@@ -1073,7 +1073,7 @@ def self.name
 
         it 'is not valid' do
           expect(subject).not_to be_valid
-          expect(subject.errors.full_messages).to include "Rules[0]: protocol must be 'tcp', 'udp', 'icmp', or 'all'"
+          expect(subject.errors.full_messages).to include "Rules[0]: protocol must be 'tcp', 'udp', 'icmp', 'icmpv6' or 'all'"
         end
       end
 
@@ -1082,7 +1082,7 @@ def self.name
 
         it 'adds an error' do
           expect(subject).not_to be_valid
-          expect(subject.errors.full_messages).to include "Rules[0]: protocol must be 'tcp', 'udp', 'icmp', or 'all'"
+          expect(subject.errors.full_messages).to include "Rules[0]: protocol must be 'tcp', 'udp', 'icmp', 'icmpv6' or 'all'"
         end
       end
 
@@ -1303,6 +1303,208 @@ def self.name
           expect(subject.errors.full_messages).to include 'Rules[0]: code must be an integer between -1 and 255 (inclusive)'
         end
       end
+
+      context 'ipv6 is disabled' do
+        before do
+          TestConfig.config[:enable_ipv6] = false
+        end
+
+        context 'icmpv6 protocol in a rule' do
+          let(:rules) do
+            [
+              {
+                protocol: 'icmpv6',
+                destination: '2001:db8::/32',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is not valid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors.full_messages).to include 'Rules[0]: icmpv6 cannot be used if enable_ipv6 is false'
+          end
+        end
+      end
+
+      context 'ipv6 is enabled' do
+        before do
+          TestConfig.config[:enable_ipv6] = true
+        end
+
+        context 'icmp protocol contains an IPv6 destination' do
+          let(:rules) do
+            [
+              {
+                protocol: 'icmp',
+                destination: '2001:db8::/32',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is invalid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors.full_messages).to include 'Rules[0]: for protocol "icmp" you cannot use IPv6 addresses'
+          end
+        end
+
+        context 'icmp protocol contains an IPv6 destination range' do
+          let(:rules) do
+            [
+              {
+                protocol: 'icmp',
+                destination: '2001:0db8::1-2001:0db8::ff',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is invalid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors.full_messages).to include 'Rules[0]: for protocol "icmp" you cannot use IPv6 addresses'
+          end
+        end
+
+        context 'icmp protocol contains a comma-delimited list of IPv6 destinations' do
+          before do
+            TestConfig.config[:security_groups][:enable_comma_delimited_destinations] = true
+          end
+
+          let(:rules) do
+            [
+              {
+                protocol: 'icmp',
+                destination: '2001:db8::/32,2001:db8:85a3::/64',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is invalid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors.full_messages).to include 'Rules[0]: for protocol "icmp" you cannot use IPv6 addresses'
+          end
+        end
+
+        context 'icmpv6 protocol contains an IPv6 destination' do
+          let(:rules) do
+            [
+              {
+                protocol: 'icmpv6',
+                destination: '2001:db8::/32',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is valid' do
+            expect(subject).to be_valid
+          end
+        end
+
+        context 'icmpv6 protocol contains a comma-delimited list of IPv6 destinations' do
+          before do
+            TestConfig.config[:security_groups][:enable_comma_delimited_destinations] = true
+          end
+
+          let(:rules) do
+            [
+              {
+                protocol: 'icmpv6',
+                destination: '2001:db8::/32,2001:db8:85a3::/64',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is valid' do
+            expect(subject).to be_valid
+          end
+        end
+
+        context 'icmpv6 protocol contains an IPv4 destination' do
+          let(:rules) do
+            [
+              {
+                protocol: 'icmpv6',
+                destination: '10.0.0.0/8',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is invalid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors.full_messages).to include 'Rules[0]: for protocol "icmpv6" you cannot use IPv4 addresses'
+          end
+        end
+
+        context 'icmpv6 protocol contains an IPv4 destination range' do
+          let(:rules) do
+            [
+              {
+                protocol: 'icmpv6',
+                destination: '1.0.0.000-1.0.0.200',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is invalid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors.full_messages).to include 'Rules[0]: for protocol "icmpv6" you cannot use IPv4 addresses'
+          end
+        end
+
+        context 'icmpv6 protocol contains a comma-delimited list of IPv4/IPv6 destinations' do
+          before do
+            TestConfig.config[:security_groups][:enable_comma_delimited_destinations] = true
+          end
+
+          let(:rules) do
+            [
+              {
+                protocol: 'icmpv6',
+                destination: '10.0.0.0/8,2001:db8::/32',
+                type: -1,
+                code: 255
+              }
+            ]
+          end
+
+          it 'is invalid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors.full_messages).to include 'Rules[0]: for protocol "icmpv6" you cannot use IPv4 addresses'
+          end
+        end
+
+        context 'the icmp rules are not provided when the protocol is icmpv6' do
+          let(:rules) do
+            [
+              {
+                protocol: 'icmpv6',
+                destination: '2001:db8::/32'
+              }
+            ]
+          end
+
+          it 'is invalid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors.full_messages).to include 'Rules[0]: type is required for protocols of type ICMP'
+            expect(subject.errors.full_messages).to include 'Rules[0]: code is required for protocols of type ICMP'
+          end
+        end
+
+      end
     end
   end
 end