Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 15 Aug 2019 10:44:19 +0200
From:   Andreas Kemnade <andreas@kemnade.info>
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     csonsino <csonsino@gmail.com>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        linux-bluetooth@vger.kernel.org, Sasha Levin <sashal@kernel.org>
Subject: Re: [PATCH 1/1] bluetooth: validate BLE connection interval updates
Message-ID: <20190815104419.161177fa@kemnade.info>
In-Reply-To: <138296C5-49FA-475D-9618-FC8D241B8823@holtmann.org>
References: <f9176553-d4d5-c2e0-ecd7-937cac0d8098@gmail.com>
        <138296C5-49FA-475D-9618-FC8D241B8823@holtmann.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
 boundary="Sig_/0/wm7a4AECkZFPlvUVAk=jG"; protocol="application/pgp-signature"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

--Sig_/0/wm7a4AECkZFPlvUVAk=jG
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable

On Sat, 6 Jul 2019 15:34:34 +0200
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Carey,
>=20
> > Problem: The Linux bluetooth stack yields complete control over the BLE
> > connection interval to the remote device.
> >=20
> > The Linux bluetooth stack provides access to the BLE connection interval
> > min and max values through /sys/kernel/debug/bluetooth/hci0/
> > conn_min_interval and /sys/kernel/debug/bluetooth/hci0/conn_max_interva=
l.
> > These values are used for initial BLE connections, but the remote device
> > has the ability to request a connection parameter update. In the event
> > that the remote side requests to change the connection interval, the Li=
nux
> > kernel currently only validates that the desired value is within the
> > acceptable range in the bluetooth specification (6 - 3200, correspondin=
g to
> > 7.5ms - 4000ms). There is currently no validation that the desired value
> > requested by the remote device is within the min/max limits specified in
> > the conn_min_interval/conn_max_interval configurations. This essentially
> > leads to Linux yielding complete control over the connection interval to
> > the remote device.
> >=20
> > The proposed patch adds a verification step to the connection parameter
> > update mechanism, ensuring that the desired value is within the min/max
> > bounds of the current connection. If the desired value is outside of the
> > current connection min/max values, then the connection parameter update
> > request is rejected and the negative response is returned to the remote
> > device. Recall that the initial connection is established using the loc=
al
> > conn_min_interval/conn_max_interval values, so this allows the Linux
> > administrator to retain control over the BLE connection interval.
> >=20
> > The one downside that I see is that the current default Linux values for
> > conn_min_interval and conn_max_interval typically correspond to 30ms and
> > 50ms respectively. If this change were accepted, then it is feasible th=
at
> > some devices would no longer be able to negotiate to their desired
> > connection interval values. This might be remedied by setting the defau=
lt
> > Linux conn_min_interval and conn_max_interval values to the widest
> > supported range (6 - 3200 / 7.5ms - 4000ms). This could lead to the same
> > behavior as the current implementation, where the remote device could
> > request to change the connection interval value to any value that is
> > permitted by the bluetooth specification, and Linux would accept the
> > desired value.
> >=20
> > Signed-off-by: Carey Sonsino <csonsino@gmail.com> =20
>=20
> patch has been applied to bluetooth-next tree.
>=20
There are devices which require low connection intervals for usable operati=
on,
e.g. BLE smartcard readers. having 30ms instead of 7.5ms means speed four t=
imes
lower.

Other devices might want to set the connection interval  to high values to =
save
power.

So if the device is not allowed to set the connection interval to such valu=
es,
how should the driver sitting on top of the gatt dbus interface of bluetoot=
hd
set such things?

The debugfs nodes are not necessarily available in distro kernels. Anything
sitting on top of gatt dbus interface does not have access to the connectio=
n handle
and cannot call hci_le_conn_update() to set things to nice values.

Using l2cap sockets instead of the dbus gatt interface is also not a viable=
 altenative
because it interferes with bluetoothd.

So IMHO this patch causes regressions and should be reverted.

Sorry for stepping out this late.

Regards,
Andreas

--Sig_/0/wm7a4AECkZFPlvUVAk=jG
Content-Type: application/pgp-signature
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEPIWxmAFyOaBcwCpFl4jFM1s/ye8FAl1VG2MACgkQl4jFM1s/
ye8NHxAAlBmgxN5V1adiofEggaQYZGboR+crJgLpHmlfZo7JjSdz4A9T04rUG/5i
i4t/kyFyKmzSXjc+YWr4JEA9EuA+goUMxuIhE4v+QhHDOZ+JSRd7GQnWBbVCOUw9
3VXc47389ZwhiDTUQAl3hHJY12q+nOfK9YJyicO8kI1vjlGonR96gid5PQtdlWzi
24SMkjfQRIPZ7C4kUuWshw+KaY5UPW/MIzYP6/Ot7re92UQpzVqDwZbwkVVqdgrE
EAB9PUjOjXuKijb5awezoM+hnnQOD94JjCzO1cqTbwjfMOr2o3N3vrfKOecPGVf8
xMH+VCZYd2R1f5toLVTjU4HI7lMA/j6qwNF+/6P2oC+YCInzl+4pbHAog/0rGKj1
sskhv4KK+T9soMKJXxCSSDayQjX6d8Pg9kasnmq7g2je6A+188DCp0plDGy9eWlM
ilrehTca+TWuhbWQ9ky4/o/iQ/pCT6QjN8iJ4hs0yj++Yl2WozKdp9mczJx+MJKD
X1chg2w9sLKpirvTXk8W7qW9D3CTMr3AG2DKwGWXVJF+YH58qU450FwdmrWxRDD9
EglXZdFYOBvLKwXeq66877NpWTXvPCrCLWOQ0S8LFDz/eHkCp0Z1H6KOXpwjZo6c
Jj/InjAhmAY2lOq6+vtvU0J0DY1Qqki/adJPoVwOpkFFa3kges0=
=SrE9
-----END PGP SIGNATURE-----

--Sig_/0/wm7a4AECkZFPlvUVAk=jG--