Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201111011745.2016-1-sonnysasaka@chromium.org>
 <20201111011745.2016-7-sonnysasaka@chromium.org> <aa1c080e8a7813299e6a093608211684e074e427.camel@hadess.net>
In-Reply-To: <aa1c080e8a7813299e6a093608211684e074e427.camel@hadess.net>
From:   Sonny Sasaka <sonnysasaka@chromium.org>
Date:   Tue, 17 Nov 2020 14:16:08 -0800
Message-ID: <CAO271m=O3hyS6Pp4fQ1pnsir7wYbLFwDm7f-a5yd0o4NTUUewA@mail.gmail.com>
Subject: Re: [PATCH BlueZ v2 7/7] battery: Implement Battery Provider API
To:     Bastien Nocera <hadess@hadess.net>
Cc:     BlueZ <linux-bluetooth@vger.kernel.org>,
        Miao-chen Chou <mcchou@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Bastien,

Thank you for the feedback. Please find my answers below.

On Tue, Nov 17, 2020 at 2:51 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> Hey Sonny,
>
> On Tue, 2020-11-10 at 17:17 -0800, Sonny Sasaka wrote:
> > This patch implements the BatteryProvider1 and
> > BatteryProviderManager1
> > API. This is a means for external clients to feed battery information
> > to
> > BlueZ if they handle some profile and can decode battery reporting.
> >
> > The battery information is then exposed externally via the existing
> > Battery1 interface. UI components can consume this API to display
> > Bluetooth peripherals' battery via a unified BlueZ API.
>
> Was this patch reviewed for potential security problems? From the top
> of my head, the possible problems would be:
> - I don't see any filters on which user could register battery
> providers, so on a multi user system, you could have a user logged in
> via SSH squatting all the battery providers, while the user "at the
> console" can't have their own providers. Also, what happens if the user
> at the console changes (fast user switching)?
> - It looks like battery providers don't check for paired, trusted or
> even connected devices, so I would be able to create nearly unbound
> number of battery providers depending on how big the cache for "seen"
> devices is.
For security, the API can be access-limited at D-Bus level using D-Bus
configuration files. For example, we can let only trusted UNIX users
as the callers for this API. This D-Bus config file would be
distribution-specific. In Chrome OS, for example, only the "audio" and
"power" users are allowed to call this API. This way we can make sure
that the callers do not abuse the API for denial-of-service kind of
attack.

>
> Given that the interface between upower and bluez is supposedly
> trusted, it might be good to ensure that there are no fuzzing problems
> on the bluez API side that could translate to causing problems in
> upower itself.
Could you give an example of what potential problems of upower can be
caused by communicating with BlueZ through this API?

>
> I didn't review the code in depth, but, having written this mechanism
> for Bluetooth battery reporting, I think that this is the right way to
> go to allow daemons like pulseaudio to report battery status.
>
> Cheers
>