interface-attach to external network a) works and b) results in undeletable instances

This happens because "shared" networks access rights allow any tenant to create ports, thus overriding the settings for "external" networks where only admins can create ports, but any tenant can create floating IPs.

A potential use case for this would be a deployment where the same publicly connected network can be used to deploy internet facing appliances, such as load balancers, as well as floating IPs allowing access to instances running on private networks.
Whether this scenario makes sense or not, it is debatable. A new constraint might be added to prevent external networks to be made shared as well.

I think the condition in which a tenant can create ports on external networks can be avoided by simply removing the shared attribute for the network. If that's confirmed this is not a security issue.

Even in the case when a network is explicitly made external and shared, I am still not sure I see a security issue of which people choosing this strategy should be aware.

However, the reported issue where a port created with interface-attach can be removed only in admin context looks like a bug and needs to be triaged.

Details of external network definition:

neutron net-show 2bfdbd29-ce26-42f0-a0ef-ac895903a1ed
+-----------------+--------------------------------------+
| Field | Value |
+-----------------+--------------------------------------+
| admin_state_up | True |
| id | 2bfdbd29-ce26-42f0-a0ef-ac895903a1ed |
| name | ext_net |
| router:external | True |
| shared | False |
| status | ACTIVE |
| subnets | 8910f065-6f71-43fb-849b-b4e7aff14384 |
| tenant_id | 79699f6f71e245b186720f1e2bc03cf0 |
+-----------------+--------------------------------------+

Salvatore

The external network definition is defined as shared=False

I have then rectified the bug title to remove the "shared" keyword.
The situation is then different, ports should not be created on external networks with tenant credentials.

Assigning to myself for triage.

So if I understand correctly there are two separate issues with potential security implications:

Issue (a) is that anyone can attach an interface to an "external" network -- what are the security consequences of that ? Unexpected snooping ? What are the natural security expectations of an "external" network ?

Issue (b) is a bug affecting removal of instances making use of such interfaces -- I don't really see an attack vector in that one. You can make it so that you can't delete your own instances. That sounds like a regular bug ?

I think in the context of (a) the driver for a security issue would be if by direct attaching to the floating ip network, you could consume more ports than you would normally be able to consume floating ip's (as dictated by quota). This could cause some sort of potential DoS attach by consuming floating ips via a backdoor route. I've not confirmed whether this is the case or not.

Re b) - yes that does just sound like a bug.

@neutron-coresec, could you confirm the (a) attack secnario above ?

Sorry folks,

I was supposed to be on top of this one, but limited to same meaningless testing.

This is a bug that we can have a fix ready for Icehouse and a backport for Havana. I do not think we should have a security embargo because tenant isolation of traffic is not compromised. Additionally, a denial of service attack should trigger quota resource restrictions on VM instance and ports.

I agree with Mark this is not a security issue. Even if a user attaches a port to an external network it won't have connectivity as we only use external networks to keep track of ip addresses for NAT rules. It looks like in his trace he is hitting this bug which was fixed and backported to havana already: https://review.openstack.org/#/c/54521/ .

I believe gary had a patch that validated the input on attach_interface but I'm not able to find it at the moment on gerrit X.x

The root cause is the following.

If port binding extension is enabled, nova will select an admin client to create the port.
The admin client is allowed to plug ports into any network, including external networks.

As a result a tenant can not only add interfaces on an external network, but even boot vms on external networks.

A fast and (in my opinion) correct fix is to restrict the search for non-shared networks to those which are actually owned by the tenant.
In this way:
- admins will still be able to boot VMs on external networks, as they would own it.
- regular tenants won't be able to boot or attach interfaces on external networks
- if an admin want to give another tenant the chance to have an interface on an external network, this can be achieve with a two step workflow:
 1) create neutron port for tenant on external network
 2) attach interface by port id

Since the latter use case is in my understanding really marginal, I think the two-step replacement workflow will be acceptable.
I don't think this solution will break any concrete use case; if existing deployment are leveraging this bug to boot tenant VMs on external networks, I don't think we should find a solution to keep this as a valid use case.

In order to remove the security flag from this bug, I have also validated that this bug cannot be exploited to allow a tenant to boot a VM on another tenant's network or port.

Please do not consider the attached patch as final as it still raises a 500 on interface-attach, and I want a 400 instead.

Hi Salvatore,

I don't think that is the right fix. The reason this is occurring is because we are not calling validate_networks before calling to the nova-compute node. The fix was included in this patch set though was dropped for some reason:

https://review.openstack.org/#/c/57229/8..9/nova/api/openstack/compute/contrib/attach_interfaces.py

In addition we might need to adiff --git a/nova/exception.py b/nova/exception.py
index cce9125..c5caa6d 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -624,6 +624,9 @@ class NetworkRequiresSubnet(Invalid):
     msg_fmt = _("Network %(network_uuid)s requires a subnet in order to boot"
                 " instances on.")

+class NetworkIsExternal(Invalid):
+ msg_fmt = _("Creating ports on router:external network %(network_uuid)s "
+ "is not allowed.")

 class DatastoreNotFound(NotFound):
     msg_fmt = _("Could not find the datastore reference(s) which the VM uses.")
diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py
index 17e3bed..03db13c 100644
--- a/nova/network/neutronv2/api.py
+++ b/nova/network/neutronv2/api.py
@@ -588,6 +588,7 @@ class API(base.Base):

         neutron = neutronv2.get_client(context)
         ports_needed_per_instance = 0

         if not requested_networks:
             nets = self._get_available_networks(context, context.project_id,
@@ -634,10 +635,14 @@ class API(base.Base):
             nets = self._get_available_networks(context,
                                     context.project_id, net_ids,
                                     neutron=neutron)

             for net in nets:
                 if not net.get('subnets'):
                     raise exception.NetworkRequiresSubnet(
                         network_uuid=net['id'])
+ if net.get('router:external'):
+ raise exception.NetworkIsExternal(network_uuid=net['id'])

             if len(nets) != len(net_ids):
                 requsted_netid_set = set(net_ids)
dd these lines as well:

I went for this approach as well, but then I thought it was probably too restrictive to forbid boot/attach for all cases on external networks.

This is why I went for this alternate approach which, just like the one you highlighted, operates on the 'validate' phase.

Anyway, I think we'll let a third party pick the right approach!

Seems there's consensus that this is not an exploitable vulnerability. Also, the bug was originally, even if only very briefly, public when it was first opened (thus broader exposure has already compromised any effective embargo).

This bug is in progress: https://review.openstack.org/#/c/85189/

launchpad seem not be willing to update status

The attachment "bug_ext_net.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

Reviewed: https://review.openstack.org/85189
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=7d1b4117fda7709307a35e56625cfa7709a6b795
Submitter: Jenkins
Branch: master

commit 7d1b4117fda7709307a35e56625cfa7709a6b795
Author: Salvatore Orlando <email address hidden>
Date: Thu Apr 3 14:54:11 2014 -0700

    Require admin context for interfaces on ext network

    Currently any user can attach an interface to a neutron
    external network, if the neutron plugin supports the port
    binding extension.
    In this case, nova will create neutron ports using the admin
    client, thus bypassing neutron authZ checks for creating ports
    on external networks.

    This patch adds a check in nova to verify the API request has an
    admin context when a request for an interface is made on a
    neutron external network.

    Change-Id: I5fb0bdcbf19eb82746ea3b192c1f65899bfb3c0b
    Closes-Bug: 1284718

Fix proposed to branch: milestone-proposed
Review: https://review.openstack.org/85823

Reviewed: https://review.openstack.org/85823
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=da66d50010d5b1ba1d7fc9c3d59d81b6c01bb0b0
Submitter: Jenkins
Branch: milestone-proposed

commit da66d50010d5b1ba1d7fc9c3d59d81b6c01bb0b0
Author: Salvatore Orlando <email address hidden>
Date: Thu Apr 3 14:54:11 2014 -0700

    Require admin context for interfaces on ext network

    Currently any user can attach an interface to a neutron
    external network, if the neutron plugin supports the port
    binding extension.
    In this case, nova will create neutron ports using the admin
    client, thus bypassing neutron authZ checks for creating ports
    on external networks.

    This patch adds a check in nova to verify the API request has an
    admin context when a request for an interface is made on a
    neutron external network.

    Change-Id: I5fb0bdcbf19eb82746ea3b192c1f65899bfb3c0b
    Closes-Bug: 1284718
    (cherry picked from commit 7d1b4117fda7709307a35e56625cfa7709a6b795)

This bug was fixed in the package nova - 1:2014.1~rc2-0ubuntu1

---------------
nova (1:2014.1~rc2-0ubuntu1) trusty; urgency=medium

  * New upstream release candidate (LP: #1299055) including fixes for:
    - Require admin context for interfaces on external networks to prevent
      non-admin users directly creating ports on external networks
      (LP: #1284718).
 -- James Page <email address hidden> Thu, 10 Apr 2014 10:59:37 +0100

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/110476

Reviewed: https://review.openstack.org/110476
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=1b69111f07de241b2cf80ea37e6fa09fcb959655
Submitter: Jenkins
Branch: stable/havana

commit 1b69111f07de241b2cf80ea37e6fa09fcb959655
Author: Salvatore Orlando <email address hidden>
Date: Thu Apr 3 14:54:11 2014 -0700

    Require admin context for interfaces on ext network

    Currently any user can attach an interface to a neutron
    external network, if the neutron plugin supports the port
    binding extension.
    In this case, nova will create neutron ports using the admin
    client, thus bypassing neutron authZ checks for creating ports
    on external networks.

    This patch adds a check in nova to verify the API request has an
    admin context when a request for an interface is made on a
    neutron external network.

    Conflicts:
     nova/exception.py

    Change-Id: I5fb0bdcbf19eb82746ea3b192c1f65899bfb3c0b
    Closes-Bug: 1284718
    (cherry picked from commit 7d1b4117fda7709307a35e56625cfa7709a6b795)

this breaks provider networks, now my users are unable to boot vm using provider networks.

i've filled a bug for the regression, https://bugs.launchpad.net/nova/+bug/1352102

Yes, this "fix" now breaks our setup. Our users have access to an external network but to give them admin would be a huge policy violation. This really should have been changed to use the policy.json to limit access or open it up.