allowed_cidr validation for additional_vips
The validation for the allowed_cidr parameter did not take into account
the IP version of additional vIPs. The parameter was rejected if the IP
version did not match the IP version of the primary vIP. As additional
vIPs can have a different IP version from the primary vIP all vIPs must
be checked during validation.
Change-Id: I3706fd5e12e17be37edce974563c6806d4f09709
(cherry picked from commit 60f579b64a)
This commit is contained in:
Gaudenz Steinlin
parent
332d7deeba
commit
68d464fa82
@ -156,14 +156,18 @@ class ListenersController(base.BaseController):
|
||||
value=headers,
|
||||
option=('%s protocol listener.' % listener_protocol))
|
||||
|
||||
def _validate_cidr_compatible_with_vip(self, vip, allowed_cidrs):
|
||||
def _validate_cidr_compatible_with_vip(self, vips, allowed_cidrs):
|
||||
for cidr in allowed_cidrs:
|
||||
# Check if CIDR IP version matches VIP IP version
|
||||
if common_utils.is_cidr_ipv6(cidr) != common_utils.is_ipv6(vip):
|
||||
msg = _("CIDR %(cidr)s IP version incompatible with VIP "
|
||||
"%(vip)s IP version.")
|
||||
for vip in vips:
|
||||
# Check if CIDR IP version matches VIP IP version
|
||||
if (common_utils.is_cidr_ipv6(cidr) ==
|
||||
common_utils.is_ipv6(vip)):
|
||||
break
|
||||
else:
|
||||
msg = _("CIDR %(cidr)s IP version incompatible with all VIPs "
|
||||
"%(vips)s IP version.")
|
||||
raise exceptions.ValidationException(
|
||||
detail=msg % {'cidr': cidr, 'vip': vip})
|
||||
detail=msg % {'cidr': cidr, 'vips': vips})
|
||||
|
||||
def _validate_create_listener(self, lock_session, listener_dict):
|
||||
"""Validate listener for wrong protocol or duplicate listeners
|
||||
@ -306,10 +310,11 @@ class ListenersController(base.BaseController):
|
||||
# Validate allowed CIDRs
|
||||
allowed_cidrs = listener_dict.get('allowed_cidrs', []) or []
|
||||
lb_id = listener_dict.get('load_balancer_id')
|
||||
vip_db = self.repositories.vip.get(
|
||||
lock_session, load_balancer_id=lb_id)
|
||||
vip_address = vip_db.ip_address
|
||||
self._validate_cidr_compatible_with_vip(vip_address, allowed_cidrs)
|
||||
lb_db = self.repositories.load_balancer.get(
|
||||
lock_session, id=lb_id)
|
||||
vip_addresses = [lb_db.vip.ip_address]
|
||||
vip_addresses.extend([vip.ip_address for vip in lb_db.additional_vips])
|
||||
self._validate_cidr_compatible_with_vip(vip_addresses, allowed_cidrs)
|
||||
|
||||
if _can_tls_offload:
|
||||
# Validate TLS version list
|
||||
@ -525,9 +530,13 @@ class ListenersController(base.BaseController):
|
||||
|
||||
# Validate allowed CIDRs
|
||||
if (listener.allowed_cidrs and listener.allowed_cidrs != wtypes.Unset):
|
||||
vip_address = db_listener.load_balancer.vip.ip_address
|
||||
vip_addresses = [db_listener.load_balancer.vip.ip_address]
|
||||
vip_addresses.extend(
|
||||
[vip.ip_address
|
||||
for vip in db_listener.load_balancer.additional_vips]
|
||||
)
|
||||
self._validate_cidr_compatible_with_vip(
|
||||
vip_address, listener.allowed_cidrs)
|
||||
vip_addresses, listener.allowed_cidrs)
|
||||
|
||||
# Check TLS cipher prohibit list
|
||||
if listener.tls_ciphers:
|
||||
|
||||
@ -147,6 +147,10 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver):
|
||||
address = ipaddress.ip_address(ip)
|
||||
return 'IPv6' if address.version == 6 else 'IPv4'
|
||||
|
||||
def _get_ethertype_for_cidr(self, cidr):
|
||||
net = ipaddress.ip_network(cidr)
|
||||
return 'IPv6' if net.version == 6 else 'IPv4'
|
||||
|
||||
def _update_security_group_rules(self, load_balancer, sec_grp_id):
|
||||
rules = self.neutron_client.list_security_group_rules(
|
||||
security_group_id=sec_grp_id)
|
||||
@ -224,11 +228,15 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver):
|
||||
ethertypes.add(self._get_ethertype_for_ip(add_vip.ip_address))
|
||||
for port_protocol in add_ports:
|
||||
for ethertype in ethertypes:
|
||||
self._create_security_group_rule(sec_grp_id, port_protocol[1],
|
||||
port_min=port_protocol[0],
|
||||
port_max=port_protocol[0],
|
||||
ethertype=ethertype,
|
||||
cidr=port_protocol[2])
|
||||
cidr = port_protocol[2]
|
||||
if not cidr or self._get_ethertype_for_cidr(cidr) == ethertype:
|
||||
self._create_security_group_rule(
|
||||
sec_grp_id, port_protocol[1],
|
||||
port_min=port_protocol[0],
|
||||
port_max=port_protocol[0],
|
||||
ethertype=ethertype,
|
||||
cidr=cidr,
|
||||
)
|
||||
|
||||
# Currently we are using the VIP network for VRRP
|
||||
# so we need to open up the protocols for it
|
||||
|
||||
@ -1199,9 +1199,9 @@ class TestListener(base.BaseAPITest):
|
||||
"It must be a valid x509 PEM format certificate.",
|
||||
response['faultstring'])
|
||||
|
||||
def _test_create_with_allowed_cidrs(self, allowed_cidrs):
|
||||
def _test_create_with_allowed_cidrs(self, allowed_cidrs, lb_id):
|
||||
listener = self.create_listener(constants.PROTOCOL_TCP,
|
||||
80, self.lb_id,
|
||||
80, lb_id,
|
||||
allowed_cidrs=allowed_cidrs)
|
||||
listener_path = self.LISTENER_PATH.format(
|
||||
listener_id=listener['listener']['id'])
|
||||
@ -1210,14 +1210,17 @@ class TestListener(base.BaseAPITest):
|
||||
|
||||
def test_create_with_allowed_cidrs_ipv4(self):
|
||||
allowed_cidrs = ['10.0.1.0/24', '172.16.55.0/25']
|
||||
self._test_create_with_allowed_cidrs(allowed_cidrs)
|
||||
self._test_create_with_allowed_cidrs(allowed_cidrs, self.lb_id)
|
||||
|
||||
def test_create_with_allowed_cidrs_ipv6(self):
|
||||
lb_ipv6 = self.create_load_balancer(
|
||||
uuidutils.generate_uuid(),
|
||||
vip_address='2001:db9:a1b:13f0::1',
|
||||
)
|
||||
lb_id = lb_ipv6.get('loadbalancer').get('id')
|
||||
self.set_lb_status(lb_id)
|
||||
allowed_cidrs = ['2001:db8:a0b:12f0::/64', '2a02:8071:69e::/64']
|
||||
with mock.patch('octavia.db.repositories.VipRepository.'
|
||||
'get') as repo_mock:
|
||||
repo_mock.return_value.ip_address = "2001:db9:a1b:13f0::1"
|
||||
self._test_create_with_allowed_cidrs(allowed_cidrs)
|
||||
self._test_create_with_allowed_cidrs(allowed_cidrs, lb_id)
|
||||
|
||||
def test_create_with_bad_allowed_cidrs(self):
|
||||
allowed_cidrs = [u'10.0.1.0/33', u'172.16.55.1.0/25']
|
||||
@ -1243,24 +1246,42 @@ class TestListener(base.BaseAPITest):
|
||||
body = self._build_body(lb_listener)
|
||||
response = self.post(self.LISTENERS_PATH, body, status=400).json
|
||||
self.assertIn("Validation failure: CIDR 2001:db8:a0b:12f0::/64 IP "
|
||||
"version incompatible with VIP 198.0.2.5 IP version.",
|
||||
"version incompatible with all VIPs ['198.0.2.5'] IP "
|
||||
"version.",
|
||||
response['faultstring'])
|
||||
|
||||
def test_create_with_incompatible_allowed_cidrs_ipv4(self):
|
||||
lb_ipv6 = self.create_load_balancer(
|
||||
uuidutils.generate_uuid(),
|
||||
vip_address='2001:db9:a1b:13f0::1',
|
||||
)
|
||||
lb_id = lb_ipv6.get('loadbalancer').get('id')
|
||||
self.set_lb_status(lb_id)
|
||||
lb_listener = {
|
||||
'protocol': constants.PROTOCOL_TCP,
|
||||
'protocol_port': 80,
|
||||
'project_id': self.project_id,
|
||||
'loadbalancer_id': self.lb_id,
|
||||
'loadbalancer_id': lb_id,
|
||||
'allowed_cidrs': ['10.0.1.0/24']}
|
||||
with mock.patch('octavia.db.repositories.VipRepository.'
|
||||
'get') as repo_mock:
|
||||
repo_mock.return_value.ip_address = "2001:db9:a1b:13f0::1"
|
||||
body = self._build_body(lb_listener)
|
||||
response = self.post(self.LISTENERS_PATH, body, status=400).json
|
||||
self.assertIn("Validation failure: CIDR 10.0.1.0/24 IP version "
|
||||
"incompatible with VIP 2001:db9:a1b:13f0::1 IP "
|
||||
"version.", response['faultstring'])
|
||||
body = self._build_body(lb_listener)
|
||||
response = self.post(self.LISTENERS_PATH, body, status=400).json
|
||||
self.assertIn("Validation failure: CIDR 10.0.1.0/24 IP version "
|
||||
"incompatible with all VIPs "
|
||||
"['2001:db9:a1b:13f0::1'] IP version.",
|
||||
response['faultstring'])
|
||||
|
||||
def test_create_with_mixed_version_allowed_cidrs(self):
|
||||
lb_dualstack = self.create_load_balancer(
|
||||
uuidutils.generate_uuid(),
|
||||
additional_vips=[{'subnet_id': uuidutils.generate_uuid(),
|
||||
'ip_address': '2001:db9:a1b:13f0::1',
|
||||
}],
|
||||
)
|
||||
lb_id = lb_dualstack.get('loadbalancer').get('id')
|
||||
self.set_lb_status(lb_id)
|
||||
self._test_create_with_allowed_cidrs(['10.0.1.0/24',
|
||||
'2001:db9:a1b:13f0::/64'],
|
||||
lb_id)
|
||||
|
||||
def test_create_with_duplicated_allowed_cidrs(self):
|
||||
allowed_cidrs = ['10.0.1.0/24', '10.0.2.0/24', '10.0.2.0/24']
|
||||
|
||||
@ -1099,12 +1099,13 @@ class TestAllowedAddressPairsDriver(base.TestCase):
|
||||
lc_1 = data_models.ListenerCidr('l1', '10.0.101.0/24')
|
||||
lc_2 = data_models.ListenerCidr('l2', '10.0.102.0/24')
|
||||
lc_3 = data_models.ListenerCidr('l2', '10.0.103.0/24')
|
||||
lc_4 = data_models.ListenerCidr('l2', '2001:0DB8::/32')
|
||||
listeners = [data_models.Listener(protocol_port=80, peer_port=1024,
|
||||
protocol=constants.PROTOCOL_TCP,
|
||||
allowed_cidrs=[lc_1]),
|
||||
data_models.Listener(protocol_port=443, peer_port=1025,
|
||||
protocol=constants.PROTOCOL_TCP,
|
||||
allowed_cidrs=[lc_2, lc_3]),
|
||||
allowed_cidrs=[lc_2, lc_3, lc_4]),
|
||||
data_models.Listener(protocol_port=50, peer_port=1026,
|
||||
protocol=constants.PROTOCOL_UDP)]
|
||||
vip = data_models.Vip(ip_address='10.0.0.2')
|
||||
@ -1183,7 +1184,18 @@ class TestAllowedAddressPairsDriver(base.TestCase):
|
||||
'remote_ip_prefix': '10.0.103.0/24'
|
||||
}
|
||||
}
|
||||
expected_create_rule_udp = {
|
||||
expected_create_rule_5 = {
|
||||
'security_group_rule': {
|
||||
'security_group_id': 'secgrp-1',
|
||||
'direction': 'ingress',
|
||||
'protocol': 'tcp',
|
||||
'port_range_min': 443,
|
||||
'port_range_max': 443,
|
||||
'ethertype': 'IPv6',
|
||||
'remote_ip_prefix': '2001:0DB8::/32'
|
||||
}
|
||||
}
|
||||
expected_create_rule_udp_1 = {
|
||||
'security_group_rule': {
|
||||
'security_group_id': 'secgrp-1',
|
||||
'direction': 'ingress',
|
||||
@ -1194,13 +1206,26 @@ class TestAllowedAddressPairsDriver(base.TestCase):
|
||||
'remote_ip_prefix': None
|
||||
}
|
||||
}
|
||||
expected_create_rule_udp_2 = {
|
||||
'security_group_rule': {
|
||||
'security_group_id': 'secgrp-1',
|
||||
'direction': 'ingress',
|
||||
'protocol': 'udp',
|
||||
'port_range_min': 50,
|
||||
'port_range_max': 50,
|
||||
'ethertype': 'IPv6',
|
||||
'remote_ip_prefix': None
|
||||
}
|
||||
}
|
||||
|
||||
create_rule.assert_has_calls([mock.call(expected_create_rule_1),
|
||||
mock.call(expected_create_rule_udp_peer),
|
||||
mock.call(expected_create_rule_2),
|
||||
mock.call(expected_create_rule_3),
|
||||
mock.call(expected_create_rule_4),
|
||||
mock.call(expected_create_rule_udp)],
|
||||
mock.call(expected_create_rule_5),
|
||||
mock.call(expected_create_rule_udp_1),
|
||||
mock.call(expected_create_rule_udp_2)],
|
||||
any_order=True)
|
||||
|
||||
def test_update_vip_when_protocol_and_peer_ports_overlap(self):
|
||||
|
||||
@ -0,0 +1,7 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
The validation for the allowed_cidr parameter only took into account the
|
||||
IP version of the primary VIP. CIDRs which only matched the version of an
|
||||
additonal VIP were rejected. This if fixed and CIDRs are now matched
|
||||
against the IP version of all VIPs.
|
||||
Loading…
Reference in New Issue
Block a user