Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   ebiederm@xmission.com (Eric W. Biederman)
To:     Matt Bennett <Matt.Bennett@alliedtelesis.co.nz>
Cc:     "netdev\@vger.kernel.org" <netdev@vger.kernel.org>,
        "zbr\@ioremap.net" <zbr@ioremap.net>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "containers\@lists.linux-foundation.org" 
        <containers@lists.linux-foundation.org>
References: <20200702002635.8169-1-matt.bennett@alliedtelesis.co.nz>
        <87k0zlspxs.fsf@x220.int.ebiederm.org>
        <94defa61204731d7dce37edeb98069c8647722c2.camel@alliedtelesis.co.nz>
Date:   Mon, 13 Jul 2020 13:39:48 -0500
In-Reply-To: <94defa61204731d7dce37edeb98069c8647722c2.camel@alliedtelesis.co.nz>
        (Matt Bennett's message of "Sun, 5 Jul 2020 22:31:25 +0000")
Message-ID: <87lfjn9s3v.fsf@x220.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH 0/5] RFC: connector: Add network namespace awareness
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Matt Bennett <Matt.Bennett@alliedtelesis.co.nz> writes:

> On Thu, 2020-07-02 at 13:59 -0500, Eric W. Biederman wrote:
>> Matt Bennett <matt.bennett@alliedtelesis.co.nz> writes:
>> 
>> > Previously the connector functionality could only be used by processes running in the
>> > default network namespace. This meant that any process that uses the connector functionality
>> > could not operate correctly when run inside a container. This is a draft patch series that
>> > attempts to now allow this functionality outside of the default network namespace.
>> > 
>> > I see this has been discussed previously [1], but am not sure how my changes relate to all
>> > of the topics discussed there and/or if there are any unintended side
>> > effects from my draft
>> 
>> In a quick skim this patchset does not look like it approaches a correct
>> conversion to having code that works in multiple namespaces.
>> 
>> I will take the changes to proc_id_connector for example.
>> You report the values in the callers current namespaces.
>> 
>> Which means an unprivileged user can create a user namespace and get
>> connector to report whichever ids they want to users in another
>> namespace.  AKA lie.
>> 
>> So this appears to make connector completely unreliable.
>> 
>> Eric
>> 
>
> Hi Eric,
>
> Thank you for taking the time to review. I wrote these patches in an
> attempt to show that I was willing to do the work myself rather than
> simply asking for someone else to do it for me. The changes worked for
> my use cases when I tested them, but I expected that some of the
> changes would be incorrect and that I would need some guidance. I can
> spend some time to really dig in and fully understand the changes I am
> trying to make (I have limited kernel development experience) but
> based on the rest of the discussion threads it seems that there is
> likely no appetite to ever support namespaces with the connector.

Good approach to this.

My sense is that there are few enough uses of connector that if don't
mind changing your code so that it works in a container (and the pidfd
support appears to already provide what you need) that is probably the
past of least resistance.

I don't think it maintaining connector support would be much more work
than it is now, if someone went through and did the work to carefully
convert the code.  So if someone really wants to use connector we can
namespace the code.

Otherwise it is probably makes sense to let the few users gradually stop
using connector so the code can eventually be removed.

Please checkout out the pidfd support and tell us how it meets your
needs.  If there is something that connector really does better it would
be good to know.

Thank you,
Eric