Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Aug 2023 08:38:08 -0300
From:   Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
        Jiri Slaby <jirislaby@kernel.org>, ofono@ofono.org
Subject: Re: [PATCH] tty: n_gsm: require CAP_NET_ADMIN to attach N_GSM0710
 ldisc
Message-ID: <ZMuRoDbMcQrsCs3m@quatroqueijos.cascardo.eti.br>
References: <20230731185942.279611-1-cascardo@canonical.com>
 <2023080111-lucid-stiffness-ccfa@gregkh>
 <ZMkCWL4r9Z35j3hC@quatroqueijos.cascardo.eti.br>
 <2023080344-happiness-duffel-c6ee@gregkh>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2023080344-happiness-duffel-c6ee@gregkh>
Precedence: bulk

On Thu, Aug 03, 2023 at 09:48:24AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Aug 01, 2023 at 10:02:16AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > On Tue, Aug 01, 2023 at 06:53:30AM +0200, Greg Kroah-Hartman wrote:
> > > On Mon, Jul 31, 2023 at 03:59:42PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > > > Any unprivileged user can attach N_GSM0710 ldisc, but it requires
> > > > CAP_NET_ADMIN to create a GSM network anyway.
> > > > 
> > > > Require initial namespace CAP_NET_ADMIN to do that.
> > > > 
> > > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> > > 
> > > What commit id does this fix?  Or has this always been a problem?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > This has always been like this. It is not really fixing a specific commit, but
> > introducing further restriction on access.
> 
> So by restricting access, will this now break existing userspace tools
> that do not have this permission?  I'm all for tightening up
> permissions, but we can't break existing workflows without a good
> reason.
> 
> thanks,
> 
> greg k-h

Yes, this will break any userspace trying to attach this without those
permissions.

I was under the impression that some operations on the line discipline also
required those same permissions, but they are actually operations on the
virtual demux ttys. So, at least we should change that on the commit
message.

The good reason to do it is reducing attack surface, given known bugs
in this code (see
https://lore.kernel.org/all/CA+UBctCZok5FSQ=LPRA+A-jocW=L8FuMVZ_7MNqhh483P5yN8A@mail.gmail.com/T/#u).

This has been done for N_HCI too
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c05731d0c6bd9a625e27ea5c5157ebf1303229e0).

The only significant user I found looking at codesearch.debian.net was
ofono, but I am having trouble finding out if the project is still active.
I am copying their list anyway here, in case it finds anyone who could tell
us that they are fine requiring such privileges.

Cascardo.