Tenant able to create networks using N1kv network profiles not explicitly assigned to it

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

Reviewed: https://review.openstack.org/119236
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=f0ea09dffc5bbb69febe399d6b077e5b4657cbc7
Submitter: Jenkins
Branch: master

commit f0ea09dffc5bbb69febe399d6b077e5b4657cbc7
Author: Saksham Varma <email address hidden>
Date: Thu Sep 4 14:53:50 2014 -0700

    CSCO:Tenants not to access unshared n/w profiles

    Ensure that a n1kv tenant who has no access to a network profile
    belonging to some other tenant, is not allowed to modify that profile.
    Currently, a tenant can create networks on any network profile if he
    has the network profile id.

    Change-Id: I53d767acceaa5e2c08e75e6f18847f659cda8d8b
    Closes-Bug: 1365727

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/158018

I wonder whether the bug should have been handled by vulnerability team. It looks like a privilege escalation problem.

I also wonder why the patch in question allows incorrect behaviour (allowing access to all profiles) to be explicitly configured via a new configuration option.

Does this also affect kilo?

@neutron-coresec can you weigh in on this? I'm unsure if net profile id is a uuid or something else.

I think the wording of the commit comment is actually slightly misleading. Network profiles are N1KV specific concept. It isn't so much the case that we are restricting them to close a security vulnerability as we are restricting them to better support multi-tenant cases from the plugin side. We added the logic to limit the scope of network profiles to be per-tenant from the plugin perspective by default. Additionally we included the config option to give the user the ability to relax this constraint and share profiles among all tenants for a particular deployment (avoiding the need to manually assign all tenants to all profiles).

If that's not a security issue, it means we probably should not change behaviour in stable branches. So let's merge it, but with restrict = false for backport.

This clearly seems like a security issue to me and likely should have been handled with a CVE.

@Kyle- The primary purpose of this patch was to enforce network profiles (effectively segmentation ranges) being scoped per-tenant. Without this patch (or with the config value set to false) they are effectively global. Unlike what the commit message says, there is no modification of other tenant's network profiles possible, as that is locked to admin-only at the CLI level. The only action possible with a profile that isn't assigned to a tenant is network creation, in which case the behavior is no different then default Openstack behavior- all of the configured segmenation range is available to all tenants.

I have updated the bug title/description. Since this is a backport I can't modify the original commit message.

Kyle: does Steven's clarification make sense? Based on that it doesn't seem like we need a CVE/advisory here.

Steven's wording makes sense to me. No CVE/advisory needed here.

Thanks Kyle, in that case I've switched our security advisory task to "won't fix" reflecting that.

@Kyle/Jeremy- Thanks to you both for the quick follow up. Definitely can appreciate your concerns based on the original wording.

Reviewed: https://review.openstack.org/158018
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=48017f27ba747dc11a8334a3b8a98a6db1fdb9a8
Submitter: Jenkins
Branch: stable/juno

commit 48017f27ba747dc11a8334a3b8a98a6db1fdb9a8
Author: Saksham Varma <email address hidden>
Date: Thu Sep 4 14:53:50 2014 -0700

    CSCO:Tenants not to access unshared n/w profiles

    Ensure that a n1kv tenant who has no access to a network profile
    belonging to some other tenant, is not allowed to modify that profile.
    Currently, a tenant can create networks on any network profile if he
    has the network profile id. For this backport the default value for
    the config option is set to False for compatibility reasons.

    Conflicts:
            neutron/plugins/cisco/db/n1kv_db_v2.py
            neutron/plugins/cisco/n1kv/n1kv_neutron_plugin.py

    Change-Id: I53d767acceaa5e2c08e75e6f18847f659cda8d8b
    Closes-Bug: 1365727
    (cherry picked from commit f0ea09dffc5bbb69febe399d6b077e5b4657cbc7)