I can't add interface to router, if there is another port in non-shared network of other tenant

[Yaroslav]

Hello, I have a problem. I'm unable to add interface to router, if previously another interface was added, from network in other subnet. There is some confusing error message:
yar@os-controller:/opt/stack/quantum/quantum/db$ sudo quantum --os_username=demo --os_tenant_name=demo --os_password=bf1af2bd6a80158f7827 --os_auth_url=http://localhost:35357/v2.0 router-interface-add 08d76d1d-3aba-4b8e-a4e0-99ef420ac520 3e159530-1397-4a3f-b380-1f7c93eb7df0
Subnet afb62abc-9fe1-46af-b90d-be9696c270e9 could not be found
It's confusing, because I've trying to add interface with subnet 3e159530-1397-4a3f-b380-1f7c93eb7df0, but got error about subnet afb62abc-9fe1-46af-b90d-be9696c270e9.

Stacktrace:
2012-09-27 14:17:48 ERROR [quantum.api.v2.resource] add_router_interface failed
Traceback (most recent call last):
  File "/opt/stack/quantum/quantum/api/v2/resource.py", line 95, in resource
    result = method(request=request, **args)
  File "/opt/stack/quantum/quantum/api/v2/base.py", line 154, in _handle_action
    return getattr(self._plugin, name)(request.context, id, body)
  File "/opt/stack/quantum/quantum/db/l3_db.py", line 318, in add_router_interface
    subnet['network_id'], subnet_id)
  File "/opt/stack/quantum/quantum/db/l3_db.py", line 264, in _check_for_dup_router_subnet
    cidr = self._get_subnet(context, ip['subnet_id'])['cidr']
  File "/opt/stack/quantum/quantum/db/db_base_plugin_v2.py", line 158, in _get_subnet
    raise q_exc.SubnetNotFound(subnet_id=id)
SubnetNotFound: Subnet afb62abc-9fe1-46af-b90d-be9696c270e9 could not be found

So, obviously, the problem was caused by subnet overlapping check, which iterates from all interface ports in this router, and get subnets from it. the main problem is that it try to do this with user's context, and subnets from other tenants was not found.

BTW, this bug leads to information disclosure about other tenant's network ids.

[dan wendlandt (danwent)]

thanks for the report. so is it that the router is owned by demo, but earlier an admin user added a subnet from a different tenant to the network, which cause a later attempt by demo to add an additional interface to fail?

The fix here should be fairly straight forward, that the duplicate check should get a get on subnets with an elevated context.

I agree that the error message is confusion.

Exposing the UUID of a different subnet alone does not strike me as a significant security concern, as I can think of any way that the UUID exposes any useful information, since calls to access that UUID would be prevented by policy. Does anyone see any other issues?

Please update with confirmed steps to repro, at which point I think the fix should be pretty straightforward.

[Yaroslav (yaroslav-isakov)]

No, I created router with admin user and admin tenant.

[Yaroslav (yaroslav-isakov)]

Oops, sorry, it was devstack who creates router, and it creates it in tenant demo. Steps are simple - use devstack, add q-l3 to ENABLED_SERVICES, create two networks (one network per tenant) and first add interface of admin net, then try to add interface with demo.

[dan wendlandt (danwent)]

are you on IRC? might be good to chat? i'm danwent on openstack-dev . would be good if we could chat about this quickly as something doesn't add up here.

otherwise, can you post:

keystone tenant-list
quantum router-show <router-uuid> for each router
quantum subnet-show <subnet-uuid> for each subnet
quantum network-show <network-uuid> for each network

[dan wendlandt (danwent)]

when i create a router that is owned by admin, then create a net + subnet owned by demo, and then as demo try to add that subnet to the router, I get an error that the router does not exist (which is correct, since demo can't see the router created by admin). So I must not be correctly understanding what you did here. Thanks.

[Yaroslav (yaroslav-isakov)]

Here is output http://paste.openstack.org/show/21153/

[Yaroslav (yaroslav-isakov)]

BTW, which is proposed way to add routers? Does tenant admin must add at least one, and use it, or he can share router with other tenants (so, for example, we can route between networks and don't use floating IPs)

[dan wendlandt (danwent)]

Ok, so sound like its the original scenario that I mentioned above. This patch shoudl fix it, please confirm by applying the patch and restarting quantum-server, then trying to add the interface again:

diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py
index 1c6707c..da9a9c4 100644
--- a/quantum/db/l3_db.py
+++ b/quantum/db/l3_db.py
@@ -261,7 +261,8 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                         msg = ("Router already has a port on subnet %s"
                                % subnet_id)
                         raise q_exc.BadRequest(resource='router', msg=msg)
- cidr = self._get_subnet(context, ip['subnet_id'])['cidr']
+ cidr = self._get_subnet(context.elevated(),
+ ip['subnet_id'])['cidr']
                     ipnet = netaddr.IPNetwork(cidr)
                     match1 = netaddr.all_matching_cidrs(new_ipnet, [cidr])
                     match2 = netaddr.all_matching_cidrs(ipnet, [new_cidr])

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/14962
Committed: http://github.com/openstack/quantum/commit/121a79317cbeec83025c1fafdf472982c8de035d
Submitter: Jenkins
Branch: master

commit 121a79317cbeec83025c1fafdf472982c8de035d
Author: Akihiro MOTOKI <email address hidden>
Date: Mon Oct 29 22:37:37 2012 +0900

    Get subnets of router interfaces with an elevated context.

    Fixes bug 1057558

    A router may have interfaces owned by other tenants (by admin operations).
    An elevated context is required to get subnet information for such interfaces.

    Change-Id: Iaf24c842c2c1e3c52573e7f9831d5f6d8fc01885

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/15102

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/15102
Committed: http://github.com/openstack/quantum/commit/cb5793241739dfb2c3b2b74cc1c5f6972f37cccc
Submitter: Jenkins
Branch: stable/folsom

commit cb5793241739dfb2c3b2b74cc1c5f6972f37cccc
Author: Akihiro MOTOKI <email address hidden>
Date: Mon Oct 29 22:37:37 2012 +0900

    Get subnets of router interfaces with an elevated context.

    Fixes bug 1057558

    A router may have interfaces owned by other tenants (by admin operations).
    An elevated context is required to get subnet information for such interfaces.

    Change-Id: Iaf24c842c2c1e3c52573e7f9831d5f6d8fc01885

[Clint Byrum (clint-fewbar)]

Hello Yaroslav, or anyone else affected,

Accepted quantum into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/quantum/2012.2.1-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Launchpad Janitor (janitor)]

This bug was fixed in the package quantum - 2012.2.1-0ubuntu1

---------------
quantum (2012.2.1-0ubuntu1) quantal-proposed; urgency=low

  * Resynchronize with stable/folsom (1e774867) (LP: #1085255):
    - [aeabb42] There are routing problems when the dnsmasq port does not come
      first in the routing table (LP: #1083238)
    - [04aab72] Quantum linux bridge not optimized with libvirt (LP: #1078210)
    - [ca7fc10] getting quotas from database has severe performance implications
      (LP: #1075369)
    - [66605e8] failed to update an external network into non external network
      (LP: #1083387)
    - [c60051a] Quantum test suite leaks memory like a sieve (LP: #1065276)
    - [3179dfc] clear_db() does incomplete db teardown (LP: #1080988)
    - [c1e19d7] Unauthorized command: cat /proc/None/cmdline (LP: #1077651)
    - [af9e076] At times a instance will not receive an IP address from the DHCP
      agent (LP: #1081664)
    - [e0d1a7d] allow multiple floating-ip on single port if they use different
      fixed ips and/or external nets (LP: #1057844)
    - [8471d79] Delete port fails to gateway ip (LP: #1079980)
    - [aca8b4a] fixed_ip allocation which is not included within
      allocation_pools makes error when delete port or re-create port
      (LP: #1077292)
    - [eacc9d3] Mapping same bridge to different phyiscal networks succeed
      (LP: #1067669)
    - [51b4c82] python-quantum: not region aware (LP: #1080793)
    - [6f0a486] delete floatingip should be in one transaction to delete port
      (LP: #1080516)
    - [db6cda7] Remove qpid configuration variables no longer supported
    - [a112840] Allow NVP plugin to use per-tenant quota extension
    - [82b1a55] Quantum service does not restart after reboot (LP: #1073999)
    - [c01a839] There are some cases that L3 API with an invalid parameter
      returns 500. (LP: #1064765)
    - [26b383f] external network can be plugged also as internal network for one
      router (LP: #1053633)
    - [49f649c] There is a lot of cases that API with an invalid parameter
      returns 500. (LP: #1062046)
    - [4546a18] When create subnet, you con set up the value as cidr (the value
      isn't cidr form). (LP: #1067959)
    - [9ba453a] killfilter should handle updated/deleted executables
      (LP: #1073768)
    - [7c8a55c] a port which is not able to delete is made when floatingip
      create fails. (LP: #1064748)
    - [c9b84cf] Linux bridge port update causes exception (LP: #1072713)
    - [cb57932] I can't add interface to router, if there is another port in
      non-shared network of other tenant (LP: #1057558)
    - [574e278] Ryu plugin does not support Security Groups (LP: #1059393)
    - [607f486] tap device added to integration bridge without tag
      (LP: #1064070)
    - [21a0fdf] L3 agent external network flag (LP: #1056720)
    - [5cbaff4] router create with external_gateway_info fails with 500 always.
      (LP: #1064235)
    - [63b81f6] l3 db operations failed in multiple transactions (LP: #1070335)
    - [bff17fb] Ensure that the SqlSoup import is still supported.
    - [e091a29] l3_nat_agent was renamed to l3_agent
    - [9030969] remove default value of 'local_ip' of 10...