Open Stack

On 3/10/21 06:42, Sean Mooney wrote:
> ok since there appear to be no stong objection form neutron
> we proably should deprecate the policy in nova then so we can either remove it next cycle
> or change its default value melanie are your plannign to propose a patch to do that?

Sorry I hadn't replied again yet but I wanted to check on the history of 
network:attach_external_network policy before going ahead.

I found the commit that introduced restrictions on the nova side [3], 
the commit message reads:

"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."

and that was converted into a policy check not long afterward [4].

The restriction was added to fix a bug [5] where users could get into a 
situation where they've been allowed to attach to an external network 
and then were unable to delete their instance later because the port 
create was done as admin and the port delete was done as the user.

I'm not quite sure if years later, we're in the clear now with regard to 
the idea of removing this policy check. I will be looking through the 
code to see if I can figure out whether [5] could become a problem again 
or if things have changed in a way that it cannot be a problem.

I'll go ahead and send this now while I look so that everyone can lend 
their thoughts on ^ in the meantime.

Cheers,
-melanie

[3] 
https://github.com/openstack/nova/commit/7d1b4117fda7709307a35e56625cfa7709a6b795
[4] 
https://github.com/openstack/nova/commit/674954f731bf4b66356fadaa5baaeb58279c5832
[5] https://bugs.launchpad.net/nova/+bug/1284718


> On Sat, 2021-03-06 at 17:00 +0100, Slawek Kaplonski wrote:
>> Hi,
>>
>> Dnia sobota, 6 marca 2021 13:53:02 CET Sean Mooney pisze:
>>> On Sat, 2021-03-06 at 08:37 +0100, Slawek Kaplonski wrote:
>>>> Hi,
>>>>
>>>> Dnia piątek, 5 marca 2021 17:26:19 CET melanie witt pisze:
>>>>> Hello all,
>>>>>
>>>>> I'm seeking input from the neutron and nova teams regarding policy
>>>>> enforcement for allowing attachment to external networks. Details below.
>>>>>
>>>>> Recently we've been looking at an issue that was reported quite a long
>>>>> time ago (2017) [1] where we have a policy check in nova-compute that
>>>>> controls whether to allow users to attach an external network to their
>>>>> instances.
>>>>>
>>>>> This has historically been a pain point for operators as (1) it goes
>>>>> against convention of having policy checks in nova-api only and (2)
>>>>> setting the policy to anything other than the default requires deploying
>>>>> a policy file change to all of the compute hosts in the deployment.
>>>>>
>>>>> The launchpad bug report mentions neutron refactoring work that was
>>>>> happening at the time, which was thought might make the
>>>>> 'network:attach_external_network' policy check on the nova side
>>>>> redundant.
>>>>>
>>>>> Years have passed since then and customers are still running into this
>>>>> problem, so we are thinking, can this policy check be removed on the
>>>>> nova-compute side now?
>>>>>
>>>>> I did a local test with devstack to verify what the behavior is if we
>>>>> were to remove the 'network:attach_external_network' policy check
>>>>> entirely [2] and found that neutron appears to properly enforce
>>>>> permission to attach to external networks itself. It appears that the
>>>>> enforcement on the neutron side makes the nova policy check redundant.
>>>>>
>>>>> When I tried to boot an instance to attach to an external network,
>>>>> neutron API returned the following:
>>>>>
>>>>> INFO neutron.pecan_wsgi.hooks.translation
>>>>> [req-58fdb103-cd20-48c9-b73b-c9074061998c
>>>>> req-4d68df7e-e0fd-4b1e-9b57-733731123d46 demo demo] POST failed (client
>>>>> error): Tenant 7c60976c662a414cb2661831ff41ee30 not allowed to create
>>>>> port on this network
>>>>> [...]
>>>>> INFO neutron.wsgi [req-58fdb103-cd20-48c9-b73b-c9074061998c
>>>>> req-4d68df7e-e0fd-4b1e-9b57-733731123d46 demo demo] 127.0.0.1 "POST
>>>>> /v2.0/ports HTTP/1.1" status: 403  len: 360 time: 0.1582518
>>>>
>>>> I just checked in Neutron code and we don't have any policy rule related
>>>> directly to the creation of ports on the external network.
>>>> Probably what You had there is the fact that Your router:external network
>>>> was owned by other tenant and due to that You wasn't able to create port
>>>> directly on it. If as an admin You would create external network which
>>>> would belong to Your tenant, You would be allowed to create port there.
>>>>
>>>>> Can anyone from the neutron team confirm whether it would be OK for us
>>>>> to remove our nova-compute policy check for external network attach
>>>>> permission and let neutron take care of the check?
>>>>
>>>> I don't know exactly the reasons why it is forbiden on Nova's side but TBH
>>>> I don't see any reason why we should forbid pluging instances directly to
>>>> the network marked as router:external=True.
>>>
>>> i have listed the majority of my consers in
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1933047#c6 which is one of the
>>> downstream bug related to this.
>>> there are a number of issue sthat i was concerd about but tl;dr
>>> - booting ip form external network consumes ip from the floating ip subnet
>>> withtout using quota - by default neutron upstream and downstream is
>>> configured to provide nova metadata api access via the neutron router not
>>> the dhcp server so by default the metadata api will not work with external
>>> network. that would require neueton to be configre to use the dhcp server
>>> for metadta or config driver or else insance wont get ssh keys ingject by
>>> cloud init.
>>> - there might be security considertaions. typeically external networks are
>>> vlan or flat networks and in some cases operators may not want tenats to be
>>> able to boot on such networks expsially with vnic-type=driect-physical
>>> since that might allow them to violate tenant isolation if the top of rack
>>> switch was not configured by a heracical port binding driver to provide
>>> adiquite isolation in that case. this is not so much because this is an
>>> external network and more a concern anytime you do PF passtough but there
>>> may be other implication to allowing this by default. that said if neutron
>>> has a way to express policy in this regard nova does not have too.
>>
>> Those are all valid points, true. But TBH, if administrator created such
>> network as pool of FIPs for the user, then users will not be able to plug vms
>> directly to that network as they aren't owners of the network so neutron will
>> forbid that.
>>
>>>
>>> router:external=True is really used to mark a network as providing
>>> connectivity such that it can be used for the gateway port of neutron
>>> routers. the workaroud that i have come up with currently is to mark the
>>> network as shared and then use neturon rbac to only share it with the teant
>>> that owns it.
>>>
>>> i assigning external network to speficic tenat being useful when you want to
>>> provde a specific ip allocation pool to them or just a set of ips. i
>>> understand that the current motivation for this request is commign form
>>> some edge deployments. in general i dont thinkthis would be widely used but
>>> for those that need its better ux then marking it as shared.
>>>
>>>>> And on the nova side, I assume we would need a deprecation cycle before
>>>>> removing the 'network:attach_external_network' policy. If we can get
>>>>> confirmation from the neutron team, is anyone opposed to the idea of
>>>>> deprecating the 'network:attach_external_network' policy in the Wallaby
>>>>> cycle, to be removed in the Xena release?
>>>>>
>>>>> I would appreciate your thoughts.
>>>>>
>>>>> Cheers,
>>>>> -melanie
>>>>>
>>>>> [1] https://bugs.launchpad.net/nova/+bug/1675486
>>>>> [2] https://bugs.launchpad.net/nova/+bug/1675486/comments/4
>>
>>
> 
> 
>