Hi Yu Liu,
On Thu, Sep 24, 2020 at 2:08 PM Yu Liu <yudiliu@xxxxxxxxxx> wrote:
>
> When connecting a LE keyboard, if the user input the wrong passkey, the
> stack would keep auto connect and thus allow the user to retry the
> passkey indefinitely which is a security concern. This fix would
> disallow the auto connect if the authentication failed.
>
> ---
>
> Changes in v1:
> - Initial change
>
> src/device.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index a4b5968d4..764cca60e 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -6033,11 +6033,17 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
> device_cancel_authentication(device, TRUE);
>
> /* Put the device back to the temporary state so that it will be
> - * treated as a newly discovered device.
> + * treated as a newly discovered device; also disable auto
> + * connect.
> */
> if (!device_is_paired(device, bdaddr_type) &&
> - !device_is_trusted(device))
> + !device_is_trusted(device)) {
> btd_device_set_temporary(device, true);
> + if (device->auto_connect) {
> + device->disable_auto_connect = TRUE;
> + device_set_auto_connect(device, FALSE);
> + }
> + }
While this looks correct we could perhaps design it in such a way that
only trusted device are allowed to set auto connect, though that may
need a lot more changes than this one so I would be fine applying this
so we can think about the implications, also perhaps this should be
incorporated to btd_device_set_temporary since a temporary device
should probably not be left in to auto-connect and only a user action
to attempt to use it again to restore its auto connect status.
To summarize we should answer 2 questions:
1. Should an untrusted device be allowed to be marked as auto-connect?
Maybe, though we didn't consider trusted property for auto-connect,
but I think using it might make auto-connect more predictable to the
upper layer.
2. Should a temporary device be allowed to be marked as auto-connect?
Most likely not, it should be removed if the user doesn't take any
action during the temporary grace period.
> device_bonding_failed(device, status);
> return;
> --
> 2.28.0.681.g6f77f65b4e-goog
>
--
Luiz Augusto von Dentz
