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
(cherry picked from commit a33d42fa59)
This commit is contained in:
Lingxian Kong 2020-04-09 13:58:30 +12:00 committed by Michael Johnson
parent a0f75b10ed
commit 1a77745993
11 changed files with 176 additions and 33 deletions

View File

@ -160,21 +160,31 @@
[networking]
# The maximum attempts to retry an action with the networking service.
# max_retries = 15
# Seconds to wait before retrying an action with the networking service.
# retry_interval = 1
# The maximum time to wait, in seconds, for a port to detach from an amphora
# port_detach_timeout = 300
# Allow/disallow specific network object types when creating VIPs.
# allow_vip_network_id = True
# allow_vip_subnet_id = True
# allow_vip_port_id = True
# List of network_ids that are valid for VIP creation.
# If this field empty, no validation is performed.
# valid_vip_networks =
# List of reserved IP addresses that cannot be used for member addresses
# The default is the nova metadata service address
# 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]
# base_path = /var/lib/octavia
# base_cert_dir = /var/lib/octavia/certs

View File

@ -116,10 +116,12 @@ class LoadBalancersController(base.BaseController):
state=prov_status, id=id)
@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_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:
network_driver = utils.get_network_driver()
if load_balancer.vip_address:
@ -167,8 +169,10 @@ class LoadBalancersController(base.BaseController):
break
@staticmethod
def _validate_port_and_fill_or_validate_subnet(load_balancer):
port = validate.port_exists(port_id=load_balancer.vip_port_id)
def _validate_port_and_fill_or_validate_subnet(load_balancer,
context=None):
port = validate.port_exists(port_id=load_balancer.vip_port_id,
context=context)
validate.check_port_in_use(port)
load_balancer.vip_network_id = port.network_id
@ -183,7 +187,8 @@ class LoadBalancersController(base.BaseController):
# Identify the subnet for this port
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:
if load_balancer.vip_address:
for port_fixed_ip in port.fixed_ips:
@ -201,7 +206,7 @@ class LoadBalancersController(base.BaseController):
"VIP port's subnet could not be determined. Please "
"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 = []
if CONF.networking.allow_vip_port_id:
allowed_network_objects.append('vip_port_id')
@ -233,10 +238,12 @@ class LoadBalancersController(base.BaseController):
# Validate the 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)
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
elif load_balancer.vip_subnet_id:
subnet = validate.subnet_exists(
@ -281,7 +288,7 @@ class LoadBalancersController(base.BaseController):
self._auth_validate_action(context, load_balancer.project_id,
constants.RBAC_POST)
self._validate_vip_request_object(load_balancer)
self._validate_vip_request_object(load_balancer, context=context)
# Load the driver early as it also provides validation
driver = driver_factory.get_driver(load_balancer.provider)

View File

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

View File

@ -11,6 +11,8 @@
# under the License.
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 novaclient import api_versions
from novaclient import client as nova_client
@ -105,6 +107,32 @@ class NeutronAuth(object):
LOG.exception("Error creating 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):
glance_client = None

View File

@ -172,6 +172,12 @@ networking_opts = [
help=_('List of IP addresses reserved from being used for '
'member addresses. IPv6 addresses should be in '
'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 = [

View File

@ -231,11 +231,11 @@ def sanitize_l7policy_api_args(l7policy, create=False):
return l7policy
def port_exists(port_id):
def port_exists(port_id, context=None):
"""Raises an exception when a port does not exist."""
network_driver = utils.get_network_driver()
try:
port = network_driver.get_port(port_id)
port = network_driver.get_port(port_id, context=context)
except Exception:
raise exceptions.InvalidSubresource(resource='Port', id=port_id)
return port
@ -250,11 +250,11 @@ def check_port_in_use(port):
return False
def subnet_exists(subnet_id):
def subnet_exists(subnet_id, context=None):
"""Raises an exception when a subnet does not exist."""
network_driver = utils.get_network_driver()
try:
subnet = network_driver.get_subnet(subnet_id)
subnet = network_driver.get_subnet(subnet_id, context=context)
except Exception:
raise exceptions.InvalidSubresource(resource='Subnet', id=subnet_id)
return subnet
@ -277,14 +277,15 @@ def qos_extension_enabled(network_driver):
"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.
If a subnet is provided, also validate the network contains that subnet.
"""
network_driver = utils.get_network_driver()
try:
network = network_driver.get_network(network_id)
network = network_driver.get_network(network_id, context=context)
except Exception:
raise exceptions.InvalidSubresource(resource='Network', id=network_id)
if subnet_id:

View File

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

View File

@ -170,9 +170,14 @@ class BaseNeutronDriver(base.AbstractNetworkDriver):
return [self._port_to_octavia_interface(
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:
resource = getattr(self.neutron_client, 'show_%s' %
resource = getattr(neutron_client, 'show_%s' %
resource_type)(resource_id)
return getattr(utils, 'convert_%s_dict_to_model' %
resource_type)(resource)
@ -222,14 +227,14 @@ class BaseNeutronDriver(base.AbstractNetworkDriver):
LOG.exception(message)
raise base.NetworkException(message)
def get_network(self, network_id):
return self._get_resource('network', network_id)
def get_network(self, network_id, context=None):
return self._get_resource('network', network_id, context=context)
def get_subnet(self, subnet_id):
return self._get_resource('subnet', subnet_id)
def get_subnet(self, subnet_id, context=None):
return self._get_resource('subnet', subnet_id, context=context)
def get_port(self, port_id):
return self._get_resource('port', port_id)
def get_port(self, port_id, context=None):
return self._get_resource('port', port_id, context=context)
def get_network_by_name(self, network_name):
return self._get_resources_by_filters(

View File

@ -270,13 +270,13 @@ class NoopNetworkDriver(driver_base.AbstractNetworkDriver):
def update_vip(self, loadbalancer, for_delete=False):
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)
def get_subnet(self, subnet_id):
def get_subnet(self, subnet_id, context=None):
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)
def get_qos_policy(self, qos_policy_id):

View File

@ -14,6 +14,8 @@
import mock
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 data_models
@ -198,6 +200,9 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
port2['fixed_ips'][0]['ip_address']])
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.return_value = {'network': {
'id': t_constants.MOCK_NETWORK_ID,
@ -208,7 +213,25 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
self.assertEqual(1, len(network.subnets))
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):
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.return_value = {'subnet': {
'id': t_constants.MOCK_SUBNET_ID,
@ -220,7 +243,26 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
self.assertEqual(t_constants.MOCK_IP_ADDRESS, subnet.gateway_ip)
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):
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.return_value = {'port': {
'id': t_constants.MOCK_PORT_ID,
@ -242,6 +284,31 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
self.assertEqual(t_constants.MOCK_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):
list_network = self.driver.neutron_client.list_networks
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.