_validate_network_tenant_ownership must be less strict

[Avishay Balderman]

Neutron, currently does a strict validation code in https://github.com/openstack/neutron/blob/master/neutron/api/v2/base.py#L618
so that for non-shared network the subnets and ports must belong to the same tenant as the network. In the case of a “service VM” created by an admin user, this function should return thus allowing admin users to create ports and networks in a tenant network.

Original code: https://github.com/openstack/neutron/blob/master/neutron/api/v2/base.py#L604

Proposed Fix:

    def _validate_network_tenant_ownership(self, request, resource_item):
        # TODO(salvatore-orlando): consider whether this check can be folded
        # in the policy engine
        if self._resource not in ('port', 'subnet') or request.context.is_admin:
            return
        network = self._plugin.get_network(
            request.context,
            resource_item['network_id'])
        # do not perform the check on shared networks
        if network.get('shared'):
            return

        network_owner = network['tenant_id']

        if network_owner != resource_item['tenant_id']:
            msg = _("Tenant %(tenant_id)s not allowed to "
                    "create %(resource)s on this network")
            raise webob.exc.HTTPForbidden(msg % {
                "tenant_id": resource_item['tenant_id'],
                "resource": self._resource,
            })

[Avishay Balderman (avishayb)]

This bug is a "show stopper" for Radware LBaaS driver

[Eugene Nikanorov (enikanorov)]

Hi Avishay, can you clarify what you're trying to accomplish? Bug description is not very clear to me.
Could you give an example of the workflow? Which users/tenants create which resources and how they are used.

[Avishay Balderman (avishayb)]

Taken from ML: "Connecting a VM from one tenant to a non-shared network in another tenant"

Below see a discussion between Sam and Salvatore. I hope it makes the bug clear now.

From: Salvatore Orlando [mailto:<email address hidden>]
Sent: Wednesday, July 31, 2013 5:42 PM
To: OpenStack Development Mailing List
Subject: Re: [openstack-dev] [Neutron]Connecting a VM from one tenant to a non-shared network in another tenant

Hi Sam,

is what you're trying to do tantamount to creating a port on a network whose tenant_id is different from the network's tenant_id?
We have at the moment a fairly strict ownership check - which does not allow even admin users to do this operation.

I do not have a strong opinion against relaxing the check, and allowing admin users to create ports on any network - I don't think this would constitute a potential vulnerability, as in neutron is someone's manages to impersonate an admin user, he/she can make much more damage.

Salvatore

On 31 July 2013 16:11, Samuel Bercovici <email address hidden> wrote:
Hi All,

We are providing load balancing services via virtual machines running under an admin tenant that needs to be connected to VMs attached to a non-shared/private tenant network.
The virtual machine fails to be provisioned connected to the private tenant network event if it is provisioned using the admin user which has admin role on both tenants.
Please advise?

Best Regards,
                -Sam.

[Salvatore Orlando (salvatore-orlando)]

I am leaning towards being in favour of this change - there is a related discussion in the dev mailing list with Avishay and Sam.

The full side of the story is that we should get rid of this function, and let the policy engine deal with the authorization.
The reason for which this has not happened in havana is that the ownership checks needs a db access; translating this into the logic of the policy engine, this would have likely to become a plugin access, causing potentially serious performance issues at scale.

In general the policy currently applied is that admin users can see anything but not operate on other tenants' resources.
I must also concede that some community members might be scared by a change in API layer in RC-phase; it is therefore worth considering whether creating tenant-less ports, as we do for floating IPs and ext gw ports might be useful for your use case.

[Samuel Bercovici (samuelb)]

Hi Salvatore,

Is there a way to provision a VM from Nova with a tenant-less port?
I was not able to do so.

[Salvatore Orlando (salvatore-orlando)]

No, I am afraid you can't; I thought these ports where needed internally by neutron - I did not realize nova used to manage the life cycle of the service vm.

As I said, I am happy to relax this constraint for Havana.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/46071

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/46071
Committed: http://github.com/openstack/neutron/commit/44fd0f74e5b19593e9d37eaec1f87134003b10f4
Submitter: Jenkins
Branch: master

commit 44fd0f74e5b19593e9d37eaec1f87134003b10f4
Author: Avishay Balderman <email address hidden>
Date: Wed Sep 11 13:46:56 2013 +0200

    _validate_network_tenant_ownership must be less strict

    Neutron, currently does a strict validation code
    so that for non-shared network the subnets and
    ports must belong to the same tenant as the network. In
    the case of a "service VM" created by
    admin user, this function should return thus allowing
    admin users to create ports and networks in a tenant
    network.

    Change-Id: Ied831402d56b98a1323d30eb6a769fd2df5278ee
    Closes-Bug: #1221315