Re: bluez / CVE-2020-0556
Hi all,
My own conclusion is that a backport of the stretch version of bluez is
the best course. If no objections are raised, then I will proceed with
uploading the backport at the end of this week.
I have added my own comments and analysis below, inline.
On Tue, May 12, 2020 at 08:01:03AM +1000, Brian May wrote:
> Brian May <brian@linuxpenguins.xyz> writes:
>
> > Looking at commit
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=7d9718cfcc11eaa9d8059e721301cdc00ef8c82e,
> > it looks like maybe we should be patching the attio_connected_cb()
> > function instead. But this function doesn't appear to have any way to
> > return an error indicating it failed, which seems to be required by the
> > patch. It might be sufficient just to ignore the error and return
> > without immediately if device is not bonded. Not sure how much I can
> > trust this however.
> >
> > My gut feeling to fix this we should backport version 5.43-2+deb9u2 from
> > stretch to Jessie. Yes, this might break stuff, but I suspect just the
> > very basic idea of this security fix - rejecting unbonded connections -
> > could break stuff also.
>
> Thinking this through some more, I struggle to get bluetooth working
> correctly on the latest Debian, let alone testing an older release. I am
> not sure if this is due to hardware or software issues. Not to mention
> the fact I don't have a lot of bluetooth HID devices to test. I am sure
> I had a bluetooth keyboard somewhere...
>
I've generally had good success with bluetooth working on Debian, though
that is mostly for tethering to my phone for Internet access.
> Is anybody here in a better position then I am to test this? If not,
> this might be another reason to backport the Stretch version...
>
I've spent a little bit of time going over the upstream commits
associated with the CVE and related parts of the code in jessie and the
upstream history. My conclusion is also that a backport of the stretch
version is the best course. More detail on my thoughts below.
> Regardless, I suspect something like the following patch might be a good
> starting point. Although I am not entirely convinced you can reject a
> connection from the attio_connected_cb function like this...
>
> === cut ====
> diff --git a/profiles/input/hog.c b/profiles/input/hog.c
> index b9aba657a..971fda822 100644
> --- a/profiles/input/hog.c
> +++ b/profiles/input/hog.c
> @@ -654,6 +654,11 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>
> DBG("HoG connected");
>
> + /* HOGP 1.0 Section 6.1 requires bonding */
> + if (!device_is_bonded(hogdev, btd_device_get_bdaddr_type(hogdev)))
> + DBG("HoG not bonded");
> + return;
> +
> hogdev->attrib = g_attrib_ref(attrib);
>
> if (hogdev->reports == NULL) {
> === cut ====
I too concluded that something like this would be the closest adaptation
of the upstream fix to the older bluez in jessie. That said, I will
note that the absence of enclosing braces around the two statements in
your 'if' block make the 'return' fall outside the 'if'; the function
will always return early here.
That said, my version of this, upon which I applied the patches for the
further three upstream commits (containing the new configuration option
and the fixes for functional regressions in the first commit) still
failed to build. I also added patches incorporating the following
upstream commits:
640236664966eed0e6bc7126044742ef4a5e3237
c21c706a50c50ab97ad266e9e33044eba4f26e04
a891c1adb541a73aac7dfeb33cce3cae807e1155
Those provide some additional API functions used the by last two
upstream commits associated with the CVE. Even then, it became clear
that the internals are different. For example,
a891c1adb541a73aac7dfeb33cce3cae807e1155 relies on the 'db' and 'client'
members of the btd_device struct, which are also not present in jessie.
My position is that each additional upstream commit brought in to
support the required changes in the four commits associated with the CVE
increases the risk of a regression or some other breakage. It also
strengthens the argument for a backport of the stretch version of bluez
to jessie.
It is clear that the lower risk path is the stretch backport.
Regards,
-Roberto
--
Roberto C. Sánchez
Reply to: