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 <[email protected]> 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
