Skip to content

Commit f7bc6d8

Browse files
jochenehretjochenehret
authored
Issue 4404: Add support for "icmpv6" protocol to ASGs (#4424)
* 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 * Fix Rubocop findings * Add test for icmpv6 with IPv6 address range * and fix version check * Spec for icmp with IPv4 address range * Add "icmpv6" protocol to SecurityGroup class * Update v3 security group docu with "icmpv6" protocol
1 parent 013748e commit f7bc6d8

File tree

7 files changed

+324
-25
lines changed

7 files changed

+324
-25
lines changed

‎app/messages/validators/security_group_rule_validator.rb

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,28 @@ def validate(record)
2626
end
2727

2828
validate_allowed_keys(rule, record, index)
29-
30-
add_rule_error("protocol must be 'tcp', 'udp', 'icmp', or 'all'", record, index) unless valid_protocol(rule[:protocol])
29+
add_rule_error("protocol must be 'tcp', 'udp', 'icmp', 'icmpv6' or 'all'", record, index) unless valid_protocol(rule[:protocol])
3130

3231
if valid_destination_type(rule[:destination], record, index)
3332
destinations = rule[:destination].split(',', -1)
3433
add_rule_error("maximum destinations per rule exceeded - must be under #{MAX_DESTINATIONS_PER_RULE}", record, index) unless destinations.length <= MAX_DESTINATIONS_PER_RULE
3534

3635
destinations.each do |d|
37-
validate_destination(d, record, index)
36+
validate_destination(d, rule[:protocol], get_allowed_ip_version(rule), record, index)
3837
end
3938
end
4039

4140
validate_description(rule, record, index)
4241
validate_log(rule, record, index)
42+
validate_protocol(rule, record, index)
43+
end
44+
end
4345

44-
case rule[:protocol]
45-
when 'tcp', 'udp'
46-
validate_tcp_udp_protocol(rule, record, index)
47-
when 'icmp'
48-
validate_icmp_protocol(rule, record, index)
49-
when 'all'
50-
add_rule_error('ports are not allowed for protocols of type all', record, index) if rule[:ports]
51-
end
46+
def get_allowed_ip_version(rule)
47+
if rule[:protocol] == 'icmp'
48+
4
49+
elsif rule[:protocol] == 'icmpv6'
50+
6
5251
end
5352
end
5453

@@ -57,7 +56,7 @@ def boolean?(value)
5756
end
5857

5958
def valid_protocol(protocol)
60-
protocol.is_a?(String) && %w[tcp udp icmp all].include?(protocol)
59+
protocol.is_a?(String) && %w[tcp udp icmp icmpv6 all].include?(protocol)
6160
end
6261

6362
def validate_allowed_keys(rule, record, index)
@@ -73,6 +72,20 @@ def validate_log(rule, record, index)
7372
add_rule_error('log must be a boolean', record, index) if rule[:log] && !boolean?(rule[:log])
7473
end
7574

75+
def validate_protocol(rule, record, index)
76+
case rule[:protocol]
77+
when 'tcp', 'udp'
78+
validate_tcp_udp_protocol(rule, record, index)
79+
when 'icmp'
80+
validate_icmp_protocol(rule, record, index)
81+
when 'icmpv6'
82+
add_rule_error('icmpv6 cannot be used if enable_ipv6 is false', record, index) unless CloudController::RuleValidator.ipv6_enabled?
83+
validate_icmp_protocol(rule, record, index)
84+
when 'all'
85+
add_rule_error('ports are not allowed for protocols of type all', record, index) if rule[:ports]
86+
end
87+
end
88+
7689
def validate_tcp_udp_protocol(rule, record, index)
7790
add_rule_error('ports are required for protocols of type TCP and UDP', record, index) unless rule[:ports]
7891

@@ -128,7 +141,7 @@ def valid_destination_type(destination, record, index)
128141
true
129142
end
130143

131-
def validate_destination(destination, record, index)
144+
def validate_destination(destination, protocol, allowed_ip_version, record, index)
132145
error_message = 'destination must be a valid CIDR, IP address, or IP address range'
133146
error_message = 'destination must contain valid CIDR(s), IP address(es), or IP address range(s)' if CloudController::RuleValidator.comma_delimited_destinations_enabled?
134147
add_rule_error('empty destination specified in comma-delimited list', record, index) if destination.empty?
@@ -137,12 +150,14 @@ def validate_destination(destination, record, index)
137150

138151
zeros_error_message = 'destination octets cannot contain leading zeros'
139152
add_rule_error(zeros_error_message, record, index) unless CloudController::RuleValidator.no_leading_zeros(address_list)
140-
141153
if address_list.length == 1
142-
add_rule_error(error_message, record, index) unless CloudController::RuleValidator.parse_ip(address_list.first)
143-
154+
parsed_ip = CloudController::RuleValidator.parse_ip(address_list.first)
155+
add_rule_error(error_message, record, index) unless parsed_ip
156+
add_rule_error("for protocol \"#{protocol}\" you cannot use IPv#{parsed_ip.version} addresses", record, index) \
157+
unless valid_ip_version?(allowed_ip_version, parsed_ip)
144158
elsif address_list.length == 2
145159
ips = CloudController::RuleValidator.parse_ip(address_list)
160+
146161
return add_rule_error('destination IP address range is invalid', record, index) unless ips
147162

148163
sorted_ips = if ips.first.is_a?(NetAddr::IPv4)
@@ -153,12 +168,17 @@ def validate_destination(destination, record, index)
153168

154169
reversed_range_error = 'beginning of IP address range is numerically greater than the end of its range (range endpoints are inverted)'
155170
add_rule_error(reversed_range_error, record, index) unless ips.first == sorted_ips.first
156-
171+
add_rule_error("for protocol \"#{protocol}\" you cannot use IPv#{ips.first.version} addresses", record, index) \
172+
unless valid_ip_version?(allowed_ip_version, sorted_ips.first)
157173
else
158174
add_rule_error(error_message, record, index)
159175
end
160176
end
161177

178+
def valid_ip_version?(allowed_ip_version, parsed_ip)
179+
parsed_ip.nil? || allowed_ip_version.nil? || parsed_ip.version == allowed_ip_version
180+
end
181+
162182
def add_rule_error(message, record, index)
163183
record.errors.add("Rules[#{index}]:", message)
164184
end

‎app/models/runtime/security_group.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def validate_rules
7171
validation_errors = case protocol
7272
when 'tcp', 'udp'
7373
CloudController::TransportRuleValidator.validate(stringified_rule)
74-
when 'icmp'
74+
when 'icmp', 'icmpv6'
7575
CloudController::ICMPRuleValidator.validate(stringified_rule)
7676
when 'all'
7777
CloudController::RuleValidator.validate(stringified_rule)

‎docs/v3/source/includes/api_resources/_security_groups.erb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@
2121
"code": 0,
2222
"description": "Allow ping requests to private services"
2323
},
24+
{
25+
"protocol": "icmpv6",
26+
"destination": "::/0",
27+
"type": -1,
28+
"code": -1,
29+
"description": "Allow all ICMPv6 traffic"
30+
},
2431
{
2532
"protocol": "tcp",
2633
"destination": "1.1.1.1,2.2.2.2/24,10.0.0.0-10.0.0.255",

‎docs/v3/source/includes/resources/security_groups/_object.md.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ Name | Type | Description
2525

2626
| Name | Type | Description | Required | Default
2727
| ---- | ---- | ----------- | -------- | -------
28-
| **protocol** | _string_ | Protocol type Valid values are `tcp`, `udp`, `icmp`, or `all` | yes | N/A |
29-
| **destination** | _string_ | The destination where the rule applies. Must be a singular Valid CIDR, IP address, or IP address range unless `cc.security_groups.enable_comma_delimited_destinations` is enabled. Then, the destination can be a comma-delimited string of CIDRs, IP addresses, or IP address ranges. Octets within destinations cannot contain leading zeros; eg. `10.0.0.0/24` is valid, but `010.00.000.0/24` is *not*. | yes | N/A |
28+
| **protocol** | _string_ | Protocol type Valid values are `tcp`, `udp`, `icmp`, `icmpv6` or `all` | yes | N/A |
29+
| **destination** | _string_ | The destination where the rule applies. Must be a singular valid CIDR, IP address, or IP address range unless `cc.security_groups.enable_comma_delimited_destinations` is enabled. Then, the destination can be a comma-delimited string of CIDRs, IP addresses, or IP address ranges. Octets within destinations cannot contain leading zeros; eg. `10.0.0.0/24` is valid, but `010.00.000.0/24` is *not*. For `icmp`, only IPv4 addresses are allowed and for `icmpv6` only IPv6 addresses. | yes | N/A |
3030
| **ports** | _string_ | Ports that the rule applies to; can be a single port (`9000`), a comma-separated list (`9000,9001`), or a range (`9000-9200`) | no | `null` |
3131
| **type** | _integer_ |[Type](https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml#icmp-parameters-types) required for ICMP protocol; valid values are between -1 and 255 (inclusive), where -1 allows all | no | `null` |
3232
| **code** | _integer_ |[Code](https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml#icmp-parameters-codes) required for ICMP protocol; valid values are between -1 and 255 (inclusive), where -1 allows all | no | `null` |

‎lib/cloud_controller/rule_validator.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ def self.parse_ip(val)
7777
ipv4 || ipv6
7878
end
7979

80+
def self.ipv6_enabled?
81+
config.get(:enable_ipv6)
82+
end
83+
8084
def self.comma_delimited_destinations_enabled?
8185
config.get(:security_groups, :enable_comma_delimited_destinations)
8286
end

0 commit comments

Comments
 (0)