Validate resource access when creating loadbalancer or member

* Make sure the user has access to the subnet in the request for
  creating or updating pool member.
* Make sure the user has access to port or subnet or network for
  creating load balancer

Story: 2007531
Task: 39339

Change-Id: I479019a911b5a1acfc1951d1cbbc2a351089cb4d
This commit is contained in:
Lingxian Kong 2020-04-09 13:58:30 +12:00
parent 9d50c7918f
commit a33d42fa59
11 changed files with 176 additions and 33 deletions

View File

@ -156,21 +156,31 @@
[networking] [networking]
# The maximum attempts to retry an action with the networking service. # The maximum attempts to retry an action with the networking service.
# max_retries = 15 # max_retries = 15
# Seconds to wait before retrying an action with the networking service. # Seconds to wait before retrying an action with the networking service.
# retry_interval = 1 # retry_interval = 1
# The maximum time to wait, in seconds, for a port to detach from an amphora # The maximum time to wait, in seconds, for a port to detach from an amphora
# port_detach_timeout = 300 # port_detach_timeout = 300
# Allow/disallow specific network object types when creating VIPs. # Allow/disallow specific network object types when creating VIPs.
# allow_vip_network_id = True # allow_vip_network_id = True
# allow_vip_subnet_id = True # allow_vip_subnet_id = True
# allow_vip_port_id = True # allow_vip_port_id = True
# List of network_ids that are valid for VIP creation. # List of network_ids that are valid for VIP creation.
# If this field empty, no validation is performed. # If this field empty, no validation is performed.
# valid_vip_networks = # valid_vip_networks =
# List of reserved IP addresses that cannot be used for member addresses # List of reserved IP addresses that cannot be used for member addresses
# The default is the nova metadata service address # The default is the nova metadata service address
# reserved_ips = ['169.254.169.254'] # reserved_ips = ['169.254.169.254']
# When True, users can use network resources they cannot normally see as VIP
# or member subnets. Making this True may allow users to access resources on
# subnets they do not normally have access to via neutron RBAC policies.
# allow_invisible_resource_usage = False
[haproxy_amphora] [haproxy_amphora]
# base_path = /var/lib/octavia # base_path = /var/lib/octavia
# base_cert_dir = /var/lib/octavia/certs # base_cert_dir = /var/lib/octavia/certs

View File

@ -118,10 +118,12 @@ class LoadBalancersController(base.BaseController):
state=prov_status, id=id) state=prov_status, id=id)
@staticmethod @staticmethod
def _validate_network_and_fill_or_validate_subnet(load_balancer): def _validate_network_and_fill_or_validate_subnet(load_balancer,
context=None):
network = validate.network_exists_optionally_contains_subnet( network = validate.network_exists_optionally_contains_subnet(
network_id=load_balancer.vip_network_id, network_id=load_balancer.vip_network_id,
subnet_id=load_balancer.vip_subnet_id) subnet_id=load_balancer.vip_subnet_id,
context=context)
if not load_balancer.vip_subnet_id: if not load_balancer.vip_subnet_id:
network_driver = utils.get_network_driver() network_driver = utils.get_network_driver()
if load_balancer.vip_address: if load_balancer.vip_address:
@ -169,8 +171,10 @@ class LoadBalancersController(base.BaseController):
break break
@staticmethod @staticmethod
def _validate_port_and_fill_or_validate_subnet(load_balancer): def _validate_port_and_fill_or_validate_subnet(load_balancer,
port = validate.port_exists(port_id=load_balancer.vip_port_id) context=None):
port = validate.port_exists(port_id=load_balancer.vip_port_id,
context=context)
validate.check_port_in_use(port) validate.check_port_in_use(port)
load_balancer.vip_network_id = port.network_id load_balancer.vip_network_id = port.network_id
@ -185,7 +189,8 @@ class LoadBalancersController(base.BaseController):
# Identify the subnet for this port # Identify the subnet for this port
if load_balancer.vip_subnet_id: if load_balancer.vip_subnet_id:
validate.subnet_exists(subnet_id=load_balancer.vip_subnet_id) validate.subnet_exists(subnet_id=load_balancer.vip_subnet_id,
context=context)
else: else:
if load_balancer.vip_address: if load_balancer.vip_address:
for port_fixed_ip in port.fixed_ips: for port_fixed_ip in port.fixed_ips:
@ -203,7 +208,7 @@ class LoadBalancersController(base.BaseController):
"VIP port's subnet could not be determined. Please " "VIP port's subnet could not be determined. Please "
"specify either a VIP subnet or address.")) "specify either a VIP subnet or address."))
def _validate_vip_request_object(self, load_balancer): def _validate_vip_request_object(self, load_balancer, context=None):
allowed_network_objects = [] allowed_network_objects = []
if CONF.networking.allow_vip_port_id: if CONF.networking.allow_vip_port_id:
allowed_network_objects.append('vip_port_id') allowed_network_objects.append('vip_port_id')
@ -235,10 +240,12 @@ class LoadBalancersController(base.BaseController):
# Validate the port id # Validate the port id
if load_balancer.vip_port_id: if load_balancer.vip_port_id:
self._validate_port_and_fill_or_validate_subnet(load_balancer) self._validate_port_and_fill_or_validate_subnet(load_balancer,
context=context)
# If no port id, validate the network id (and subnet if provided) # If no port id, validate the network id (and subnet if provided)
elif load_balancer.vip_network_id: elif load_balancer.vip_network_id:
self._validate_network_and_fill_or_validate_subnet(load_balancer) self._validate_network_and_fill_or_validate_subnet(load_balancer,
context=context)
# Validate just the subnet id # Validate just the subnet id
elif load_balancer.vip_subnet_id: elif load_balancer.vip_subnet_id:
subnet = validate.subnet_exists( subnet = validate.subnet_exists(
@ -387,7 +394,7 @@ class LoadBalancersController(base.BaseController):
self._auth_validate_action(context, load_balancer.project_id, self._auth_validate_action(context, load_balancer.project_id,
constants.RBAC_POST) constants.RBAC_POST)
self._validate_vip_request_object(load_balancer) self._validate_vip_request_object(load_balancer, context=context)
self._validate_flavor(context.session, load_balancer) self._validate_flavor(context.session, load_balancer)

View File

@ -149,9 +149,9 @@ class MemberController(base.BaseController):
validate.ip_not_reserved(member.address) validate.ip_not_reserved(member.address)
# Validate member subnet # Validate member subnet
if member.subnet_id and not validate.subnet_exists(member.subnet_id): if (member.subnet_id and
raise exceptions.NotFound(resource='Subnet', not validate.subnet_exists(member.subnet_id, context=context)):
id=member.subnet_id) raise exceptions.NotFound(resource='Subnet', id=member.subnet_id)
pool = self.repositories.pool.get(context.session, id=self.pool_id) pool = self.repositories.pool.get(context.session, id=self.pool_id)
member.project_id, provider = self._get_lb_project_id_provider( member.project_id, provider = self._get_lb_project_id_provider(
context.session, pool.load_balancer_id) context.session, pool.load_balancer_id)
@ -345,7 +345,7 @@ class MembersController(MemberController):
# Validate member subnets # Validate member subnets
for member in members: for member in members:
if member.subnet_id and not validate.subnet_exists( if member.subnet_id and not validate.subnet_exists(
member.subnet_id): member.subnet_id, context=context):
raise exceptions.NotFound(resource='Subnet', raise exceptions.NotFound(resource='Subnet',
id=member.subnet_id) id=member.subnet_id)

View File

@ -12,6 +12,8 @@
from cinderclient import client as cinder_client from cinderclient import client as cinder_client
from glanceclient import client as glance_client from glanceclient import client as glance_client
from keystoneauth1.identity.generic import token
from keystoneauth1 import session
from neutronclient.neutron import client as neutron_client from neutronclient.neutron import client as neutron_client
from novaclient import api_versions from novaclient import api_versions
from novaclient import client as nova_client from novaclient import client as nova_client
@ -107,6 +109,32 @@ class NeutronAuth(object):
LOG.exception("Error creating Neutron client.") LOG.exception("Error creating Neutron client.")
return cls.neutron_client return cls.neutron_client
@classmethod
def get_user_neutron_client(cls, context):
# get a normal session
ksession = keystone.KeystoneSession()
service_auth = ksession.get_auth()
# make user auth and swap it in session
user_auth = token.Token(auth_url=service_auth.auth_url,
token=context.auth_token,
project_id=context.project_id)
user_session = session.Session(auth=user_auth)
kwargs = {
'session': user_session,
'region_name': CONF.neutron.region_name,
'endpoint_type': CONF.neutron.endpoint_type,
'service_name': CONF.neutron.service_name,
'insecure': CONF.neutron.insecure,
'ca_cert': CONF.neutron.ca_certificates_file
}
if CONF.neutron.endpoint:
kwargs['endpoint_override'] = CONF.neutron.endpoint
# create neutron client using user's session
return neutron_client.Client(NEUTRON_VERSION, **kwargs)
class GlanceAuth(object): class GlanceAuth(object):
glance_client = None glance_client = None

View File

@ -212,6 +212,12 @@ networking_opts = [
help=_('List of IP addresses reserved from being used for ' help=_('List of IP addresses reserved from being used for '
'member addresses. IPv6 addresses should be in ' 'member addresses. IPv6 addresses should be in '
'expanded, uppercase form.')), 'expanded, uppercase form.')),
cfg.BoolOpt('allow_invisible_resource_usage', default=False,
help=_("When True, users can use network resources they "
"cannot normally see as VIP or member subnets. Making "
"this True may allow users to access resources on "
"subnets they do not normally have access to via "
"neutron RBAC policies.")),
] ]
healthmanager_opts = [ healthmanager_opts = [

View File

@ -312,11 +312,11 @@ def sanitize_l7policy_api_args(l7policy, create=False):
return l7policy return l7policy
def port_exists(port_id): def port_exists(port_id, context=None):
"""Raises an exception when a port does not exist.""" """Raises an exception when a port does not exist."""
network_driver = utils.get_network_driver() network_driver = utils.get_network_driver()
try: try:
port = network_driver.get_port(port_id) port = network_driver.get_port(port_id, context=context)
except Exception: except Exception:
raise exceptions.InvalidSubresource(resource='Port', id=port_id) raise exceptions.InvalidSubresource(resource='Port', id=port_id)
return port return port
@ -331,11 +331,11 @@ def check_port_in_use(port):
return False return False
def subnet_exists(subnet_id): def subnet_exists(subnet_id, context=None):
"""Raises an exception when a subnet does not exist.""" """Raises an exception when a subnet does not exist."""
network_driver = utils.get_network_driver() network_driver = utils.get_network_driver()
try: try:
subnet = network_driver.get_subnet(subnet_id) subnet = network_driver.get_subnet(subnet_id, context=context)
except Exception: except Exception:
raise exceptions.InvalidSubresource(resource='Subnet', id=subnet_id) raise exceptions.InvalidSubresource(resource='Subnet', id=subnet_id)
return subnet return subnet
@ -358,14 +358,15 @@ def qos_extension_enabled(network_driver):
"VIP QoS policy is not allowed in this deployment.")) "VIP QoS policy is not allowed in this deployment."))
def network_exists_optionally_contains_subnet(network_id, subnet_id=None): def network_exists_optionally_contains_subnet(network_id, subnet_id=None,
context=None):
"""Raises an exception when a network does not exist. """Raises an exception when a network does not exist.
If a subnet is provided, also validate the network contains that subnet. If a subnet is provided, also validate the network contains that subnet.
""" """
network_driver = utils.get_network_driver() network_driver = utils.get_network_driver()
try: try:
network = network_driver.get_network(network_id) network = network_driver.get_network(network_id, context=context)
except Exception: except Exception:
raise exceptions.InvalidSubresource(resource='Network', id=network_id) raise exceptions.InvalidSubresource(resource='Network', id=network_id)
if subnet_id: if subnet_id:

View File

@ -183,28 +183,31 @@ class AbstractNetworkDriver(object, metaclass=abc.ABCMeta):
""" """
@abc.abstractmethod @abc.abstractmethod
def get_network(self, network_id): def get_network(self, network_id, context=None):
"""Retrieves network from network id. """Retrieves network from network id.
:param network_id: id of an network to retrieve :param network_id: id of an network to retrieve
:param context: A request context
:return: octavia.network.data_models.Network :return: octavia.network.data_models.Network
:raises: NetworkException, NetworkNotFound :raises: NetworkException, NetworkNotFound
""" """
@abc.abstractmethod @abc.abstractmethod
def get_subnet(self, subnet_id): def get_subnet(self, subnet_id, context=None):
"""Retrieves subnet from subnet id. """Retrieves subnet from subnet id.
:param subnet_id: id of a subnet to retrieve :param subnet_id: id of a subnet to retrieve
:param context: A request context
:return: octavia.network.data_models.Subnet :return: octavia.network.data_models.Subnet
:raises: NetworkException, SubnetNotFound :raises: NetworkException, SubnetNotFound
""" """
@abc.abstractmethod @abc.abstractmethod
def get_port(self, port_id): def get_port(self, port_id, context=None):
"""Retrieves port from port id. """Retrieves port from port id.
:param port_id: id of a port to retrieve :param port_id: id of a port to retrieve
:param context: A request context
:return: octavia.network.data_models.Port :return: octavia.network.data_models.Port
:raises: NetworkException, PortNotFound :raises: NetworkException, PortNotFound
""" """

View File

@ -173,9 +173,14 @@ class BaseNeutronDriver(base.AbstractNetworkDriver):
return [self._port_to_octavia_interface( return [self._port_to_octavia_interface(
compute_id, port) for port in ports['ports']] compute_id, port) for port in ports['ports']]
def _get_resource(self, resource_type, resource_id): def _get_resource(self, resource_type, resource_id, context=None):
neutron_client = self.neutron_client
if context and not CONF.networking.allow_invisible_resource_usage:
neutron_client = clients.NeutronAuth.get_user_neutron_client(
context)
try: try:
resource = getattr(self.neutron_client, 'show_%s' % resource = getattr(neutron_client, 'show_%s' %
resource_type)(resource_id) resource_type)(resource_id)
return getattr(utils, 'convert_%s_dict_to_model' % return getattr(utils, 'convert_%s_dict_to_model' %
resource_type)(resource) resource_type)(resource)
@ -225,14 +230,14 @@ class BaseNeutronDriver(base.AbstractNetworkDriver):
LOG.exception(message) LOG.exception(message)
raise base.NetworkException(message) raise base.NetworkException(message)
def get_network(self, network_id): def get_network(self, network_id, context=None):
return self._get_resource('network', network_id) return self._get_resource('network', network_id, context=context)
def get_subnet(self, subnet_id): def get_subnet(self, subnet_id, context=None):
return self._get_resource('subnet', subnet_id) return self._get_resource('subnet', subnet_id, context=context)
def get_port(self, port_id): def get_port(self, port_id, context=None):
return self._get_resource('port', port_id) return self._get_resource('port', port_id, context=context)
def get_network_by_name(self, network_name): def get_network_by_name(self, network_name):
return self._get_resources_by_filters( return self._get_resources_by_filters(

View File

@ -313,13 +313,13 @@ class NoopNetworkDriver(driver_base.AbstractNetworkDriver):
def update_vip(self, loadbalancer, for_delete=False): def update_vip(self, loadbalancer, for_delete=False):
self.driver.update_vip(loadbalancer, for_delete) self.driver.update_vip(loadbalancer, for_delete)
def get_network(self, network_id): def get_network(self, network_id, context=None):
return self.driver.get_network(network_id) return self.driver.get_network(network_id)
def get_subnet(self, subnet_id): def get_subnet(self, subnet_id, context=None):
return self.driver.get_subnet(subnet_id) return self.driver.get_subnet(subnet_id)
def get_port(self, port_id): def get_port(self, port_id, context=None):
return self.driver.get_port(port_id) return self.driver.get_port(port_id)
def get_qos_policy(self, qos_policy_id): def get_qos_policy(self, qos_policy_id):

View File

@ -14,6 +14,8 @@
from unittest import mock from unittest import mock
from neutronclient.common import exceptions as neutron_client_exceptions from neutronclient.common import exceptions as neutron_client_exceptions
from oslo_config import cfg
from oslo_config import fixture as oslo_fixture
from octavia.common import clients from octavia.common import clients
from octavia.common import data_models from octavia.common import data_models
@ -200,6 +202,9 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
port2['fixed_ips'][0]['ip_address']]) port2['fixed_ips'][0]['ip_address']])
def test_get_network(self): def test_get_network(self):
config = self.useFixture(oslo_fixture.Config(cfg.CONF))
config.config(group="networking", allow_invisible_resource_usage=True)
show_network = self.driver.neutron_client.show_network show_network = self.driver.neutron_client.show_network
show_network.return_value = {'network': { show_network.return_value = {'network': {
'id': t_constants.MOCK_NETWORK_ID, 'id': t_constants.MOCK_NETWORK_ID,
@ -210,7 +215,25 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
self.assertEqual(1, len(network.subnets)) self.assertEqual(1, len(network.subnets))
self.assertEqual(t_constants.MOCK_SUBNET_ID, network.subnets[0]) self.assertEqual(t_constants.MOCK_SUBNET_ID, network.subnets[0])
@mock.patch("octavia.common.clients.NeutronAuth.get_user_neutron_client")
def test_get_user_network(self, neutron_client_mock):
show_network = neutron_client_mock.return_value.show_network
show_network.return_value = {'network': {
'id': t_constants.MOCK_NETWORK_ID,
'subnets': [t_constants.MOCK_SUBNET_ID]}}
network = self.driver.get_network(t_constants.MOCK_NETWORK_ID,
context=mock.ANY)
self.assertIsInstance(network, network_models.Network)
self.assertEqual(t_constants.MOCK_NETWORK_ID, network.id)
self.assertEqual(1, len(network.subnets))
self.assertEqual(t_constants.MOCK_SUBNET_ID, network.subnets[0])
def test_get_subnet(self): def test_get_subnet(self):
config = self.useFixture(oslo_fixture.Config(cfg.CONF))
config.config(group="networking", allow_invisible_resource_usage=True)
show_subnet = self.driver.neutron_client.show_subnet show_subnet = self.driver.neutron_client.show_subnet
show_subnet.return_value = {'subnet': { show_subnet.return_value = {'subnet': {
'id': t_constants.MOCK_SUBNET_ID, 'id': t_constants.MOCK_SUBNET_ID,
@ -222,7 +245,26 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
self.assertEqual(t_constants.MOCK_IP_ADDRESS, subnet.gateway_ip) self.assertEqual(t_constants.MOCK_IP_ADDRESS, subnet.gateway_ip)
self.assertEqual(t_constants.MOCK_CIDR, subnet.cidr) self.assertEqual(t_constants.MOCK_CIDR, subnet.cidr)
@mock.patch("octavia.common.clients.NeutronAuth.get_user_neutron_client")
def test_get_user_subnet(self, neutron_client_mock):
show_subnet = neutron_client_mock.return_value.show_subnet
show_subnet.return_value = {'subnet': {
'id': t_constants.MOCK_SUBNET_ID,
'gateway_ip': t_constants.MOCK_IP_ADDRESS,
'cidr': t_constants.MOCK_CIDR}}
subnet = self.driver.get_subnet(t_constants.MOCK_SUBNET_ID,
context=mock.ANY)
self.assertIsInstance(subnet, network_models.Subnet)
self.assertEqual(t_constants.MOCK_SUBNET_ID, subnet.id)
self.assertEqual(t_constants.MOCK_IP_ADDRESS, subnet.gateway_ip)
self.assertEqual(t_constants.MOCK_CIDR, subnet.cidr)
def test_get_port(self): def test_get_port(self):
config = self.useFixture(oslo_fixture.Config(cfg.CONF))
config.config(group="networking", allow_invisible_resource_usage=True)
show_port = self.driver.neutron_client.show_port show_port = self.driver.neutron_client.show_port
show_port.return_value = {'port': { show_port.return_value = {'port': {
'id': t_constants.MOCK_PORT_ID, 'id': t_constants.MOCK_PORT_ID,
@ -244,6 +286,31 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
self.assertEqual(t_constants.MOCK_IP_ADDRESS, self.assertEqual(t_constants.MOCK_IP_ADDRESS,
port.fixed_ips[0].ip_address) port.fixed_ips[0].ip_address)
@mock.patch("octavia.common.clients.NeutronAuth.get_user_neutron_client")
def test_get_user_port(self, neutron_client_mock):
show_port = neutron_client_mock.return_value.show_port
show_port.return_value = {'port': {
'id': t_constants.MOCK_PORT_ID,
'mac_address': t_constants.MOCK_MAC_ADDR,
'network_id': t_constants.MOCK_NETWORK_ID,
'fixed_ips': [{
'subnet_id': t_constants.MOCK_SUBNET_ID,
'ip_address': t_constants.MOCK_IP_ADDRESS
}]}}
port = self.driver.get_port(t_constants.MOCK_PORT_ID, context=mock.ANY)
self.assertIsInstance(port, network_models.Port)
self.assertEqual(t_constants.MOCK_PORT_ID, port.id)
self.assertEqual(t_constants.MOCK_MAC_ADDR, port.mac_address)
self.assertEqual(t_constants.MOCK_NETWORK_ID, port.network_id)
self.assertEqual(1, len(port.fixed_ips))
self.assertIsInstance(port.fixed_ips[0], network_models.FixedIP)
self.assertEqual(t_constants.MOCK_SUBNET_ID,
port.fixed_ips[0].subnet_id)
self.assertEqual(t_constants.MOCK_IP_ADDRESS,
port.fixed_ips[0].ip_address)
def test_get_network_by_name(self): def test_get_network_by_name(self):
list_network = self.driver.neutron_client.list_networks list_network = self.driver.neutron_client.list_networks
list_network.return_value = {'networks': [{'network': { list_network.return_value = {'networks': [{'network': {

View File

@ -0,0 +1,16 @@
---
upgrade:
- |
After this upgrade, users will no longer be able use network resources they
cannot see or "show" on load balancers. Operators can revert this behavior
by setting the "allow_invisible_reourece_usage" configuration file setting
to ``True``.
security:
- |
Previously, if a user knew or could guess the UUID for a network resource,
they could use that UUID to create load balancer resources using that UUID.
Now the user must have permission to see or "show" the resource before it
can be used with a load balancer. This will be the new default, but
operators can disable this behavior via the setting the configuration file
setting "allow_invisible_resource_usage" to ``True``. This issue falls
under the "Class C1" security issue as the user would require a valid UUID.