Open Stack

Hi,

I'm probably too late, as it's already merged, but as explained in
initial review [0],
these checks are more about security than feature support:

ip netns uses mount namespaces since check-in
(and mount namespaces are supported since 2.4.19 [1])

The reason for this check was more to make sure that this command isn't used
outside a mount namespace to bind-mount arbitrary directory in the
root filesystem,
as neutron_netns_wrapper is allowed by CommandFilter. The only way i figured
out to check that at time of writing was to use namespace inodes, that
landed in 3.8 [2]

There may be anyway better solutions to ensure that this bind-mount is
done within a
namespace...

[0] https://review.openstack.org/#/c/33467/4/quantum/agent/linux/netns_wrapper.py
[1] http://lwn.net/Articles/531114/
[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=98f842e675f96ffac96e6c50315790912b2812be

francois.

2015-01-23 17:57 GMT+01:00 Paul Michali <pc at michali.net>:
> Maybe I'm misunderstanding the issue?
>
> I thought the reason there is no version check currently, is because a check
> is being made to see if the process is in the same namespace as root for the
> net namespace (as a proxy to checking that the mount namespace is being
> used).
>
> The comment indicates that one could check the mount namespace, but that
> would require a kernel 3.8+.
>
> My question was whether we should check the mount namespace and therefore
> require a kernel of 3.8+?
>
> Maybe it is just easier to stick with (C).
>
> Thoughts? Should this review move forward as-is?
>
> Regards,
>
> PCM
>
>
> PCM (Paul Michali)
>
> IRC............ pc_m (irc.freenode.com)
> Twitter....... @pmichali
>
>
> On Fri, Jan 23, 2015 at 10:37 AM, Kyle Mestery <mestery at mestery.com> wrote:
>>
>> According to the patch author, the check isn't necessary at all.
>>
>> On Fri, Jan 23, 2015 at 7:12 AM, Paul Michali <pc at michali.net> wrote:
>>>
>>> To summarize, should we...
>>>
>>> A) Assume all kernels will be 3.8+ and use mount namespace (risky?)
>>> B) Do a check to ensure kernel is 3.8+ and fall back to net namespace and
>>> mount --bind if not (more work).
>>> C) Just use net namespace as indication that namespace with mount --bind
>>> done (simple)
>>>
>>> Maybe it is best to just do the simple thing for now. I wanted to double
>>> check though, to see if the alternatives could/should be considered.
>>>
>>> Regards,
>>>
>>> PCM
>>>
>>>
>>> PCM (Paul Michali)
>>>
>>> IRC............ pc_m (irc.freenode.com)
>>> Twitter....... @pmichali
>>>
>>>
>>> On Fri, Jan 23, 2015 at 1:35 AM, Joshua Zhang
>>> <joshua.zhang at canonical.com> wrote:
>>>>
>>>> pls note that actually this patch doesn't have minumum kernel
>>>> requirement because it only uses 'mount --bind' and 'net namespace', not use
>>>> 'mount namespace'. ('mount --bind' is since linux 2.4, 'net namespace' is
>>>> since Linux 3.0, 'mount namespace' is since Linux 3.8).
>>>>
>>>> so I think sanity checks for 3.8 is not need, any thoughts ?
>>>>
>>>> thanks.
>>>>
>>>>
>>>> On Fri, Jan 23, 2015 at 2:12 PM, Kevin Benton <blak111 at gmail.com> wrote:
>>>>>
>>>>> >If we can consolidate that and use a single tool from the master
>>>>> > neutron repository, that would be my vote.
>>>>>
>>>>> +1 with a hook mechanism so the sanity checks stay in the *aas repos
>>>>> and they are only run if installed.
>>>>>
>>>>>
>>>>> On Thu, Jan 22, 2015 at 7:30 AM, Kyle Mestery <mestery at mestery.com>
>>>>> wrote:
>>>>>>
>>>>>> On Wed, Jan 21, 2015 at 10:27 AM, Ihar Hrachyshka
>>>>>> <ihrachys at redhat.com> wrote:
>>>>>>>
>>>>>>> On 01/20/2015 05:40 PM, Paul Michali wrote:
>>>>>>>
>>>>>>> Review https://review.openstack.org/#/c/146508/ is adding support for
>>>>>>> StrongSwan VPN, which needs mount bind to be able to specify different paths
>>>>>>> for config files.
>>>>>>>
>>>>>>> The code, which used some older patch, does a test for
>>>>>>> /proc/1/ns/net, instead of /proc/1/ns/mnt, because it stated that the latter
>>>>>>> is only supported in kernel 3.8+. That was a while ago, and I'm wondering if
>>>>>>> the condition is still true.  If we know that for Kilo and on, we'll be
>>>>>>> dealing with 3.8+ kernels, we could use the more accurate test.
>>>>>>>
>>>>>>> Can we require 3.8+ kernel for this?
>>>>>>>
>>>>>>>
>>>>>>> I think we can but it's better to check with distributions. Red Hat
>>>>>>> wise, we ship a kernel that is newer than 3.8.
>>>>>>>
>>>>>>> If so, how and where do we ensure that is true?
>>>>>>>
>>>>>>>
>>>>>>> Ideally, you would implement a sanity check for the feature you need
>>>>>>> from the kernel. Though it opens a question of whether we want to ship
>>>>>>> multiple sanity check tools for each of repos (neutron + 3 *aas repos).
>>>>>>>
>>>>>> If we can consolidate that and use a single tool from the master
>>>>>> neutron repository, that would be my vote.
>>>>>>>
>>>>>>>
>>>>>>> Also, if you can kindly review the code here:
>>>>>>> https://review.openstack.org/#/c/146508/5/neutron_vpnaas/services/vpn/common/netns_wrapper.py,
>>>>>>> I'd really appreciate it, as I'm not versed in the Linux proc files at all.
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>
>>>>>>> PCM (Paul Michali)
>>>>>>>
>>>>>>> IRC............ pc_m (irc.freenode.com)
>>>>>>> Twitter....... @pmichali
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> __________________________________________________________________________
>>>>>>> OpenStack Development Mailing List (not for usage questions)
>>>>>>> Unsubscribe:
>>>>>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> __________________________________________________________________________
>>>>>>> OpenStack Development Mailing List (not for usage questions)
>>>>>>> Unsubscribe:
>>>>>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> __________________________________________________________________________
>>>>>> OpenStack Development Mailing List (not for usage questions)
>>>>>> Unsubscribe:
>>>>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Kevin Benton
>>>>>
>>>>>
>>>>> __________________________________________________________________________
>>>>> OpenStack Development Mailing List (not for usage questions)
>>>>> Unsubscribe:
>>>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Best Regards
>>>> Zhang Hua(张华）
>>>> Software Engineer | Canonical
>>>> IRC:  zhhuabj
>>>>
>>>>
>>>> __________________________________________________________________________
>>>> OpenStack Development Mailing List (not for usage questions)
>>>> Unsubscribe:
>>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>
>>>
>>>
>>>
>>> __________________________________________________________________________
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe:
>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>
>>
>> __________________________________________________________________________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>