[Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[howardchung]

From: howardchung <howardchung@google.com>

Attack scenario:
1. A Chromebook (let's call this device A) is paired to a legitimate
   Bluetooth classic device (e.g. a speaker) (let's call this device
   B).
2. A malicious device (let's call this device C) pretends to be the
   Bluetooth speaker by using the same BT address.
3. If device A is not currently connected to device B, device A will
   be ready to accept connection from device B in the background
   (technically, doing Page Scan).
4. Therefore, device C can initiate connection to device A
   (because device A is doing Page Scan) and device A will accept the
   connection because device A trusts device C's address which is the
   same as device B's address.
5. Device C won't be able to communicate at any high level Bluetooth
   profile with device A because device A enforces that device C is
   encrypted with their common Link Key, which device C doesn't have.
   But device C can initiate pairing with device A with just-works
   model without requiring user interaction (there is only pairing
   notification). After pairing, device A now trusts device C with a
   new different link key, common between device A and C.
6. From now on, device A trusts device C, so device C can at anytime
   connect to device A to do any kind of high-level hijacking, e.g.
   speaker hijack or mouse/keyboard hijack.

To fix this, reject the pairing if all the conditions below are met.
- the pairing is initialized by peer
- the authorization method is just-work
- host already had the link key to the peer

Also create a debugfs option to permit the pairing even the
conditions above are met.

Signed-off-by: howardchung <howardchung@google.com>
---

 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_core.c    | 47 +++++++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c   | 12 ++++++++++
 3 files changed, 60 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 07b6ecedc6ce..4918b79baa41 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -283,6 +283,7 @@ enum {
 	HCI_FORCE_STATIC_ADDR,
 	HCI_LL_RPA_RESOLUTION,
 	HCI_CMD_PENDING,
+	HCI_PERMIT_JUST_WORK_REPAIR,
 
 	__HCI_NUM_FLAGS,
 };
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9e19d5a3aac8..9014aa567e7b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -172,10 +172,57 @@ static const struct file_operations vendor_diag_fops = {
 	.llseek		= default_llseek,
 };
 
+static ssize_t permit_just_work_repair_read(struct file *file,
+					    char __user *user_buf,
+					    size_t count, loff_t *ppos)
+{
+	struct hci_dev *hdev = file->private_data;
+	char buf[3];
+
+	buf[0] = hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) ? 'Y'
+								      : 'N';
+	buf[1] = '\n';
+	buf[2] = '\0';
+	return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
+}
+
+static ssize_t permit_just_work_repair_write(struct file *file,
+					     const char __user *user_buf,
+					     size_t count, loff_t *ppos)
+{
+	struct hci_dev *hdev = file->private_data;
+	char buf[32];
+	size_t buf_size = min(count, (sizeof(buf) - 1));
+	bool enable;
+
+	if (copy_from_user(buf, user_buf, buf_size))
+		return -EFAULT;
+
+	buf[buf_size] = '\0';
+	if (strtobool(buf, &enable))
+		return -EINVAL;
+
+	if (enable)
+		hci_dev_set_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
+	else
+		hci_dev_clear_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
+
+	return count;
+}
+
+static const struct file_operations permit_just_work_repair_fops = {
+	.open		= simple_open,
+	.read		= permit_just_work_repair_read,
+	.write		= permit_just_work_repair_write,
+	.llseek		= default_llseek,
+};
+
 static void hci_debugfs_create_basic(struct hci_dev *hdev)
 {
 	debugfs_create_file("dut_mode", 0644, hdev->debugfs, hdev,
 			    &dut_mode_fops);
+	debugfs_create_file("permit_just_work_repair", 0644, hdev->debugfs,
+			    hdev, &permit_just_work_repair_fops);
 
 	if (hdev->set_diag)
 		debugfs_create_file("vendor_diag", 0644, hdev->debugfs, hdev,
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6ddc4a74a5e4..898e347e19e0 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4539,6 +4539,18 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
 		goto unlock;
 	}
 
+	/* If there already exists link key in local host, terminate the
+	 * connection by default since the remote device could be malicious.
+	 * Permit the connection if permit_just_work_repair is enabled.
+	 */
+	if (!hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) &&
+	    hci_find_link_key(hdev, &ev->bdaddr)) {
+		BT_DBG("Rejecting request: local host already have link key");
+		hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
+			     sizeof(ev->bdaddr), &ev->bdaddr);
+		goto unlock;
+	}
+
 	/* If no side requires MITM protection; auto-accept */
 	if ((!loc_mitm || conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) &&
 	    (!rem_mitm || conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)) {
-- 
2.24.1.735.g03f4e72817-goog

Fwd: [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[Matias Karhumaa]

Hi Howard,

Re-sending as plain text.

This same attack scenario works also against Ubuntu 18.04 at least.

ma 6. tammik. 2020 klo 12.17 howardchung@google.com
(howardchung@google.com) kirjoitti:
>
> From: howardchung <howardchung@google.com>
>
> Attack scenario:
> 1. A Chromebook (let's call this device A) is paired to a legitimate
>    Bluetooth classic device (e.g. a speaker) (let's call this device
>    B).
> 2. A malicious device (let's call this device C) pretends to be the
>    Bluetooth speaker by using the same BT address.
> 3. If device A is not currently connected to device B, device A will
>    be ready to accept connection from device B in the background
>    (technically, doing Page Scan).
> 4. Therefore, device C can initiate connection to device A
>    (because device A is doing Page Scan) and device A will accept the
>    connection because device A trusts device C's address which is the
>    same as device B's address.
> 5. Device C won't be able to communicate at any high level Bluetooth
>    profile with device A because device A enforces that device C is
>    encrypted with their common Link Key, which device C doesn't have.
>    But device C can initiate pairing with device A with just-works
>    model without requiring user interaction (there is only pairing
>    notification). After pairing, device A now trusts device C with a
>    new different link key, common between device A and C.
> 6. From now on, device A trusts device C, so device C can at anytime
>    connect to device A to do any kind of high-level hijacking, e.g.
>    speaker hijack or mouse/keyboard hijack.
>
> To fix this, reject the pairing if all the conditions below are met.
> - the pairing is initialized by peer
> - the authorization method is just-work
> - host already had the link key to the peer
>
> Also create a debugfs option to permit the pairing even the
> conditions above are met.
>
> Signed-off-by: howardchung <howardchung@google.com>
> ---
>
>  include/net/bluetooth/hci.h |  1 +
>  net/bluetooth/hci_core.c    | 47 +++++++++++++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c   | 12 ++++++++++
>  3 files changed, 60 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 07b6ecedc6ce..4918b79baa41 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -283,6 +283,7 @@ enum {
>         HCI_FORCE_STATIC_ADDR,
>         HCI_LL_RPA_RESOLUTION,
>         HCI_CMD_PENDING,
> +       HCI_PERMIT_JUST_WORK_REPAIR,
>
>         __HCI_NUM_FLAGS,
>  };
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9e19d5a3aac8..9014aa567e7b 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -172,10 +172,57 @@ static const struct file_operations vendor_diag_fops = {
>         .llseek         = default_llseek,
>  };
>
> +static ssize_t permit_just_work_repair_read(struct file *file,
> +                                           char __user *user_buf,
> +                                           size_t count, loff_t *ppos)
> +{
> +       struct hci_dev *hdev = file->private_data;
> +       char buf[3];
> +
> +       buf[0] = hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) ? 'Y'
> +                                                                     : 'N';
> +       buf[1] = '\n';
> +       buf[2] = '\0';
> +       return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> +}
> +
> +static ssize_t permit_just_work_repair_write(struct file *file,
> +                                            const char __user *user_buf,
> +                                            size_t count, loff_t *ppos)
> +{
> +       struct hci_dev *hdev = file->private_data;
> +       char buf[32];
> +       size_t buf_size = min(count, (sizeof(buf) - 1));
> +       bool enable;
> +
> +       if (copy_from_user(buf, user_buf, buf_size))
> +               return -EFAULT;
> +
> +       buf[buf_size] = '\0';
> +       if (strtobool(buf, &enable))
> +               return -EINVAL;
> +
> +       if (enable)
> +               hci_dev_set_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> +       else
> +               hci_dev_clear_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> +
> +       return count;
> +}
> +
> +static const struct file_operations permit_just_work_repair_fops = {
> +       .open           = simple_open,
> +       .read           = permit_just_work_repair_read,
> +       .write          = permit_just_work_repair_write,
> +       .llseek         = default_llseek,
> +};
> +
>  static void hci_debugfs_create_basic(struct hci_dev *hdev)
>  {
>         debugfs_create_file("dut_mode", 0644, hdev->debugfs, hdev,
>                             &dut_mode_fops);
> +       debugfs_create_file("permit_just_work_repair", 0644, hdev->debugfs,
> +                           hdev, &permit_just_work_repair_fops);
>
>         if (hdev->set_diag)
>                 debugfs_create_file("vendor_diag", 0644, hdev->debugfs, hdev,
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 6ddc4a74a5e4..898e347e19e0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4539,6 +4539,18 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
>                 goto unlock;
>         }
>
> +       /* If there already exists link key in local host, terminate the
> +        * connection by default since the remote device could be malicious.
> +        * Permit the connection if permit_just_work_repair is enabled.
> +        */
> +       if (!hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) &&
> +           hci_find_link_key(hdev, &ev->bdaddr)) {
> +               BT_DBG("Rejecting request: local host already have link key");
> +               hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,

Why wouldn't we just request authorization from userspace in case we
already have link key? I think that is how it works on other
platforms.
>
> +                            sizeof(ev->bdaddr), &ev->bdaddr);
> +               goto unlock;
> +       }
> +
>         /* If no side requires MITM protection; auto-accept */
>         if ((!loc_mitm || conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) &&
>             (!rem_mitm || conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)) {
> --
> 2.24.1.735.g03f4e72817-goog


Best regard,
Matias Karhumaa

Re: [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[Yun-hao Chung]

Hi Matias,

Thanks for the comment.
I think in the just-work case, if we ask user for confirmation, they
will likely just accept it.
Rejecting the pairing immediately is more secure.

Thanks,
Howard

On Mon, Jan 6, 2020 at 7:44 PM Matias Karhumaa
<matias.karhumaa@gmail.com> wrote:
>
> Hi Howard,
>
> Re-sending as plain text.
>
> This same attack scenario works also against Ubuntu 18.04 at least.
>
> ma 6. tammik. 2020 klo 12.17 howardchung@google.com
> (howardchung@google.com) kirjoitti:
> >
> > From: howardchung <howardchung@google.com>
> >
> > Attack scenario:
> > 1. A Chromebook (let's call this device A) is paired to a legitimate
> >    Bluetooth classic device (e.g. a speaker) (let's call this device
> >    B).
> > 2. A malicious device (let's call this device C) pretends to be the
> >    Bluetooth speaker by using the same BT address.
> > 3. If device A is not currently connected to device B, device A will
> >    be ready to accept connection from device B in the background
> >    (technically, doing Page Scan).
> > 4. Therefore, device C can initiate connection to device A
> >    (because device A is doing Page Scan) and device A will accept the
> >    connection because device A trusts device C's address which is the
> >    same as device B's address.
> > 5. Device C won't be able to communicate at any high level Bluetooth
> >    profile with device A because device A enforces that device C is
> >    encrypted with their common Link Key, which device C doesn't have.
> >    But device C can initiate pairing with device A with just-works
> >    model without requiring user interaction (there is only pairing
> >    notification). After pairing, device A now trusts device C with a
> >    new different link key, common between device A and C.
> > 6. From now on, device A trusts device C, so device C can at anytime
> >    connect to device A to do any kind of high-level hijacking, e.g.
> >    speaker hijack or mouse/keyboard hijack.
> >
> > To fix this, reject the pairing if all the conditions below are met.
> > - the pairing is initialized by peer
> > - the authorization method is just-work
> > - host already had the link key to the peer
> >
> > Also create a debugfs option to permit the pairing even the
> > conditions above are met.
> >
> > Signed-off-by: howardchung <howardchung@google.com>
> > ---
> >
> >  include/net/bluetooth/hci.h |  1 +
> >  net/bluetooth/hci_core.c    | 47 +++++++++++++++++++++++++++++++++++++
> >  net/bluetooth/hci_event.c   | 12 ++++++++++
> >  3 files changed, 60 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 07b6ecedc6ce..4918b79baa41 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -283,6 +283,7 @@ enum {
> >         HCI_FORCE_STATIC_ADDR,
> >         HCI_LL_RPA_RESOLUTION,
> >         HCI_CMD_PENDING,
> > +       HCI_PERMIT_JUST_WORK_REPAIR,
> >
> >         __HCI_NUM_FLAGS,
> >  };
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 9e19d5a3aac8..9014aa567e7b 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -172,10 +172,57 @@ static const struct file_operations vendor_diag_fops = {
> >         .llseek         = default_llseek,
> >  };
> >
> > +static ssize_t permit_just_work_repair_read(struct file *file,
> > +                                           char __user *user_buf,
> > +                                           size_t count, loff_t *ppos)
> > +{
> > +       struct hci_dev *hdev = file->private_data;
> > +       char buf[3];
> > +
> > +       buf[0] = hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) ? 'Y'
> > +                                                                     : 'N';
> > +       buf[1] = '\n';
> > +       buf[2] = '\0';
> > +       return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> > +}
> > +
> > +static ssize_t permit_just_work_repair_write(struct file *file,
> > +                                            const char __user *user_buf,
> > +                                            size_t count, loff_t *ppos)
> > +{
> > +       struct hci_dev *hdev = file->private_data;
> > +       char buf[32];
> > +       size_t buf_size = min(count, (sizeof(buf) - 1));
> > +       bool enable;
> > +
> > +       if (copy_from_user(buf, user_buf, buf_size))
> > +               return -EFAULT;
> > +
> > +       buf[buf_size] = '\0';
> > +       if (strtobool(buf, &enable))
> > +               return -EINVAL;
> > +
> > +       if (enable)
> > +               hci_dev_set_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> > +       else
> > +               hci_dev_clear_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> > +
> > +       return count;
> > +}
> > +
> > +static const struct file_operations permit_just_work_repair_fops = {
> > +       .open           = simple_open,
> > +       .read           = permit_just_work_repair_read,
> > +       .write          = permit_just_work_repair_write,
> > +       .llseek         = default_llseek,
> > +};
> > +
> >  static void hci_debugfs_create_basic(struct hci_dev *hdev)
> >  {
> >         debugfs_create_file("dut_mode", 0644, hdev->debugfs, hdev,
> >                             &dut_mode_fops);
> > +       debugfs_create_file("permit_just_work_repair", 0644, hdev->debugfs,
> > +                           hdev, &permit_just_work_repair_fops);
> >
> >         if (hdev->set_diag)
> >                 debugfs_create_file("vendor_diag", 0644, hdev->debugfs, hdev,
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 6ddc4a74a5e4..898e347e19e0 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -4539,6 +4539,18 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
> >                 goto unlock;
> >         }
> >
> > +       /* If there already exists link key in local host, terminate the
> > +        * connection by default since the remote device could be malicious.
> > +        * Permit the connection if permit_just_work_repair is enabled.
> > +        */
> > +       if (!hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) &&
> > +           hci_find_link_key(hdev, &ev->bdaddr)) {
> > +               BT_DBG("Rejecting request: local host already have link key");
> > +               hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
>
> Why wouldn't we just request authorization from userspace in case we
> already have link key? I think that is how it works on other
> platforms.
> >
> > +                            sizeof(ev->bdaddr), &ev->bdaddr);
> > +               goto unlock;
> > +       }
> > +
> >         /* If no side requires MITM protection; auto-accept */
> >         if ((!loc_mitm || conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) &&
> >             (!rem_mitm || conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)) {
> > --
> > 2.24.1.735.g03f4e72817-goog
>
>
> Best regard,
> Matias Karhumaa

Re: [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[Marcel Holtmann]

Hi Howard,

> Attack scenario:
> 1. A Chromebook (let's call this device A) is paired to a legitimate
>   Bluetooth classic device (e.g. a speaker) (let's call this device
>   B).
> 2. A malicious device (let's call this device C) pretends to be the
>   Bluetooth speaker by using the same BT address.
> 3. If device A is not currently connected to device B, device A will
>   be ready to accept connection from device B in the background
>   (technically, doing Page Scan).
> 4. Therefore, device C can initiate connection to device A
>   (because device A is doing Page Scan) and device A will accept the
>   connection because device A trusts device C's address which is the
>   same as device B's address.
> 5. Device C won't be able to communicate at any high level Bluetooth
>   profile with device A because device A enforces that device C is
>   encrypted with their common Link Key, which device C doesn't have.
>   But device C can initiate pairing with device A with just-works
>   model without requiring user interaction (there is only pairing
>   notification). After pairing, device A now trusts device C with a
>   new different link key, common between device A and C.
> 6. From now on, device A trusts device C, so device C can at anytime
>   connect to device A to do any kind of high-level hijacking, e.g.
>   speaker hijack or mouse/keyboard hijack.
> 
> To fix this, reject the pairing if all the conditions below are met.
> - the pairing is initialized by peer
> - the authorization method is just-work
> - host already had the link key to the peer
> 
> Also create a debugfs option to permit the pairing even the
> conditions above are met.
> 
> Signed-off-by: howardchung <howardchung@google.com>

we prefer full name signed-off-by signatures.

> ---
> 
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_core.c    | 47 +++++++++++++++++++++++++++++++++++++
> net/bluetooth/hci_event.c   | 12 ++++++++++
> 3 files changed, 60 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 07b6ecedc6ce..4918b79baa41 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -283,6 +283,7 @@ enum {
> 	HCI_FORCE_STATIC_ADDR,
> 	HCI_LL_RPA_RESOLUTION,
> 	HCI_CMD_PENDING,
> +	HCI_PERMIT_JUST_WORK_REPAIR,
> 
> 	__HCI_NUM_FLAGS,
> };
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9e19d5a3aac8..9014aa567e7b 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -172,10 +172,57 @@ static const struct file_operations vendor_diag_fops = {
> 	.llseek		= default_llseek,
> };
> 
> +static ssize_t permit_just_work_repair_read(struct file *file,
> +					    char __user *user_buf,
> +					    size_t count, loff_t *ppos)
> +{
> +	struct hci_dev *hdev = file->private_data;
> +	char buf[3];
> +
> +	buf[0] = hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) ? 'Y'
> +								      : 'N';
> +	buf[1] = '\n';
> +	buf[2] = '\0';
> +	return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> +}
> +
> +static ssize_t permit_just_work_repair_write(struct file *file,
> +					     const char __user *user_buf,
> +					     size_t count, loff_t *ppos)
> +{
> +	struct hci_dev *hdev = file->private_data;
> +	char buf[32];
> +	size_t buf_size = min(count, (sizeof(buf) - 1));
> +	bool enable;
> +
> +	if (copy_from_user(buf, user_buf, buf_size))
> +		return -EFAULT;
> +
> +	buf[buf_size] = '\0';
> +	if (strtobool(buf, &enable))
> +		return -EINVAL;
> +
> +	if (enable)
> +		hci_dev_set_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> +	else
> +		hci_dev_clear_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> +
> +	return count;
> +}
> +
> +static const struct file_operations permit_just_work_repair_fops = {
> +	.open		= simple_open,
> +	.read		= permit_just_work_repair_read,
> +	.write		= permit_just_work_repair_write,
> +	.llseek		= default_llseek,
> +};
> +
> static void hci_debugfs_create_basic(struct hci_dev *hdev)
> {
> 	debugfs_create_file("dut_mode", 0644, hdev->debugfs, hdev,
> 			    &dut_mode_fops);
> +	debugfs_create_file("permit_just_work_repair", 0644, hdev->debugfs,
> +			    hdev, &permit_just_work_repair_fops);
> 
> 	if (hdev->set_diag)
> 		debugfs_create_file("vendor_diag", 0644, hdev->debugfs, hdev,
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 6ddc4a74a5e4..898e347e19e0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4539,6 +4539,18 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
> 		goto unlock;
> 	}
> 
> +	/* If there already exists link key in local host, terminate the
> +	 * connection by default since the remote device could be malicious.
> +	 * Permit the connection if permit_just_work_repair is enabled.
> +	 */
> +	if (!hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) &&
> +	    hci_find_link_key(hdev, &ev->bdaddr)) {
> +		BT_DBG("Rejecting request: local host already have link key");

Can we use bt_dev_warn() here.

> +		hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
> +			     sizeof(ev->bdaddr), &ev->bdaddr);
> +		goto unlock;
> +	}
> +
> 	/* If no side requires MITM protection; auto-accept */
> 	if ((!loc_mitm || conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) &&
> 	    (!rem_mitm || conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)) {

What about the LE cases?

In addition, I like to get a pair of second eyes from Johan and Luiz on this one.

Regards

Marcel

Re: [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[Alain Michaud]

Hi Johan, Luiz,

Did you have additional feedback on this before we can send a new
version to address Marcel's comments?

Marcel, you are right, LE likely will need a similar fix.  Given we
currently have SC disabled on chromium, we can probably submit this as
a separate patch unless someone else would like to contribute it
sooner.

Thanks,
Alain

On Wed, Jan 8, 2020 at 4:02 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Howard,
>
> > Attack scenario:
> > 1. A Chromebook (let's call this device A) is paired to a legitimate
> >   Bluetooth classic device (e.g. a speaker) (let's call this device
> >   B).
> > 2. A malicious device (let's call this device C) pretends to be the
> >   Bluetooth speaker by using the same BT address.
> > 3. If device A is not currently connected to device B, device A will
> >   be ready to accept connection from device B in the background
> >   (technically, doing Page Scan).
> > 4. Therefore, device C can initiate connection to device A
> >   (because device A is doing Page Scan) and device A will accept the
> >   connection because device A trusts device C's address which is the
> >   same as device B's address.
> > 5. Device C won't be able to communicate at any high level Bluetooth
> >   profile with device A because device A enforces that device C is
> >   encrypted with their common Link Key, which device C doesn't have.
> >   But device C can initiate pairing with device A with just-works
> >   model without requiring user interaction (there is only pairing
> >   notification). After pairing, device A now trusts device C with a
> >   new different link key, common between device A and C.
> > 6. From now on, device A trusts device C, so device C can at anytime
> >   connect to device A to do any kind of high-level hijacking, e.g.
> >   speaker hijack or mouse/keyboard hijack.
> >
> > To fix this, reject the pairing if all the conditions below are met.
> > - the pairing is initialized by peer
> > - the authorization method is just-work
> > - host already had the link key to the peer
> >
> > Also create a debugfs option to permit the pairing even the
> > conditions above are met.
> >
> > Signed-off-by: howardchung <howardchung@google.com>
>
> we prefer full name signed-off-by signatures.
>
> > ---
> >
> > include/net/bluetooth/hci.h |  1 +
> > net/bluetooth/hci_core.c    | 47 +++++++++++++++++++++++++++++++++++++
> > net/bluetooth/hci_event.c   | 12 ++++++++++
> > 3 files changed, 60 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 07b6ecedc6ce..4918b79baa41 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -283,6 +283,7 @@ enum {
> >       HCI_FORCE_STATIC_ADDR,
> >       HCI_LL_RPA_RESOLUTION,
> >       HCI_CMD_PENDING,
> > +     HCI_PERMIT_JUST_WORK_REPAIR,
> >
> >       __HCI_NUM_FLAGS,
> > };
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 9e19d5a3aac8..9014aa567e7b 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -172,10 +172,57 @@ static const struct file_operations vendor_diag_fops = {
> >       .llseek         = default_llseek,
> > };
> >
> > +static ssize_t permit_just_work_repair_read(struct file *file,
> > +                                         char __user *user_buf,
> > +                                         size_t count, loff_t *ppos)
> > +{
> > +     struct hci_dev *hdev = file->private_data;
> > +     char buf[3];
> > +
> > +     buf[0] = hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) ? 'Y'
> > +                                                                   : 'N';
> > +     buf[1] = '\n';
> > +     buf[2] = '\0';
> > +     return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> > +}
> > +
> > +static ssize_t permit_just_work_repair_write(struct file *file,
> > +                                          const char __user *user_buf,
> > +                                          size_t count, loff_t *ppos)
> > +{
> > +     struct hci_dev *hdev = file->private_data;
> > +     char buf[32];
> > +     size_t buf_size = min(count, (sizeof(buf) - 1));
> > +     bool enable;
> > +
> > +     if (copy_from_user(buf, user_buf, buf_size))
> > +             return -EFAULT;
> > +
> > +     buf[buf_size] = '\0';
> > +     if (strtobool(buf, &enable))
> > +             return -EINVAL;
> > +
> > +     if (enable)
> > +             hci_dev_set_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> > +     else
> > +             hci_dev_clear_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> > +
> > +     return count;
> > +}
> > +
> > +static const struct file_operations permit_just_work_repair_fops = {
> > +     .open           = simple_open,
> > +     .read           = permit_just_work_repair_read,
> > +     .write          = permit_just_work_repair_write,
> > +     .llseek         = default_llseek,
> > +};
> > +
> > static void hci_debugfs_create_basic(struct hci_dev *hdev)
> > {
> >       debugfs_create_file("dut_mode", 0644, hdev->debugfs, hdev,
> >                           &dut_mode_fops);
> > +     debugfs_create_file("permit_just_work_repair", 0644, hdev->debugfs,
> > +                         hdev, &permit_just_work_repair_fops);
> >
> >       if (hdev->set_diag)
> >               debugfs_create_file("vendor_diag", 0644, hdev->debugfs, hdev,
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 6ddc4a74a5e4..898e347e19e0 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -4539,6 +4539,18 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
> >               goto unlock;
> >       }
> >
> > +     /* If there already exists link key in local host, terminate the
> > +      * connection by default since the remote device could be malicious.
> > +      * Permit the connection if permit_just_work_repair is enabled.
> > +      */
> > +     if (!hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) &&
> > +         hci_find_link_key(hdev, &ev->bdaddr)) {
> > +             BT_DBG("Rejecting request: local host already have link key");
>
> Can we use bt_dev_warn() here.
>
> > +             hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
> > +                          sizeof(ev->bdaddr), &ev->bdaddr);
> > +             goto unlock;
> > +     }
> > +
> >       /* If no side requires MITM protection; auto-accept */
> >       if ((!loc_mitm || conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) &&
> >           (!rem_mitm || conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)) {
>
> What about the LE cases?
>
> In addition, I like to get a pair of second eyes from Johan and Luiz on this one.
>
> Regards
>
> Marcel
>
> --
> You received this message because you are subscribed to the Google Groups "ChromeOS Bluetooth Upstreaming" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to chromeos-bluetooth-upstreaming+unsubscribe@chromium.org.
> To post to this group, send email to chromeos-bluetooth-upstreaming@chromium.org.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromeos-bluetooth-upstreaming/CD07E771-6F40-4158-A0F9-03FC128CDCD3%40holtmann.org.

Re: [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[Marcel Holtmann]

Hi Alain,

> Did you have additional feedback on this before we can send a new
> version to address Marcel's comments?
> 
> Marcel, you are right, LE likely will need a similar fix.  Given we
> currently have SC disabled on chromium, we can probably submit this as
> a separate patch unless someone else would like to contribute it
> sooner.

I would prefer if we get BR/EDR and LE fixed in the same kernel release.

Regards

Marcel

Re: [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[Luiz Augusto von Dentz]

Hi Alain,

On Wed, Jan 22, 2020 at 9:53 AM Alain Michaud <alainmichaud@google.com> wrote:
>
> Hi Johan, Luiz,
>
> Did you have additional feedback on this before we can send a new
> version to address Marcel's comments?
>
> Marcel, you are right, LE likely will need a similar fix.  Given we
> currently have SC disabled on chromium, we can probably submit this as
> a separate patch unless someone else would like to contribute it
> sooner.

At first glance this looks like it would block repairing from the
legitimate device as well, i.e. user has unpaired, so I wonder if
perhaps we should attempt to notify the user via pairing agent if he
wants to drop the old key or not, the problem is that in headless
system this might have the same effect of always dropping the old keys
which would enable the attack to happen anyway. Perhaps we should have
an entry on main.conf to enable the system/package maintainer to tell
what policy shall be used under this circumstance, the daemon could
then take the action instead leaving this hardcoded in the kernel,
that way it is a lot simpler to enable or disable this without having
the whole trouble of getting a new kernel.

Btw, I recall some discussion on BT SIG group regarding cases of half
bonded, one side has key but the other doesn't, iirc the discussion I
had with Johan was that we should allow the pairing procedure to start
but only remove the old key in case the key has been accepted and the
bonding has been completed, though perhaps for just works that is not
really desirable since there is no authentication happening it may be
authorized without user consent.

> Thanks,
> Alain
>
> On Wed, Jan 8, 2020 at 4:02 PM Marcel Holtmann <marcel@holtmann.org> wrote:
> >
> > Hi Howard,
> >
> > > Attack scenario:
> > > 1. A Chromebook (let's call this device A) is paired to a legitimate
> > >   Bluetooth classic device (e.g. a speaker) (let's call this device
> > >   B).
> > > 2. A malicious device (let's call this device C) pretends to be the
> > >   Bluetooth speaker by using the same BT address.
> > > 3. If device A is not currently connected to device B, device A will
> > >   be ready to accept connection from device B in the background
> > >   (technically, doing Page Scan).
> > > 4. Therefore, device C can initiate connection to device A
> > >   (because device A is doing Page Scan) and device A will accept the
> > >   connection because device A trusts device C's address which is the
> > >   same as device B's address.
> > > 5. Device C won't be able to communicate at any high level Bluetooth
> > >   profile with device A because device A enforces that device C is
> > >   encrypted with their common Link Key, which device C doesn't have.
> > >   But device C can initiate pairing with device A with just-works
> > >   model without requiring user interaction (there is only pairing
> > >   notification). After pairing, device A now trusts device C with a
> > >   new different link key, common between device A and C.
> > > 6. From now on, device A trusts device C, so device C can at anytime
> > >   connect to device A to do any kind of high-level hijacking, e.g.
> > >   speaker hijack or mouse/keyboard hijack.
> > >
> > > To fix this, reject the pairing if all the conditions below are met.
> > > - the pairing is initialized by peer
> > > - the authorization method is just-work
> > > - host already had the link key to the peer
> > >
> > > Also create a debugfs option to permit the pairing even the
> > > conditions above are met.
> > >
> > > Signed-off-by: howardchung <howardchung@google.com>
> >
> > we prefer full name signed-off-by signatures.
> >
> > > ---
> > >
> > > include/net/bluetooth/hci.h |  1 +
> > > net/bluetooth/hci_core.c    | 47 +++++++++++++++++++++++++++++++++++++
> > > net/bluetooth/hci_event.c   | 12 ++++++++++
> > > 3 files changed, 60 insertions(+)
> > >
> > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > > index 07b6ecedc6ce..4918b79baa41 100644
> > > --- a/include/net/bluetooth/hci.h
> > > +++ b/include/net/bluetooth/hci.h
> > > @@ -283,6 +283,7 @@ enum {
> > >       HCI_FORCE_STATIC_ADDR,
> > >       HCI_LL_RPA_RESOLUTION,
> > >       HCI_CMD_PENDING,
> > > +     HCI_PERMIT_JUST_WORK_REPAIR,
> > >
> > >       __HCI_NUM_FLAGS,
> > > };
> > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > index 9e19d5a3aac8..9014aa567e7b 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > > @@ -172,10 +172,57 @@ static const struct file_operations vendor_diag_fops = {
> > >       .llseek         = default_llseek,
> > > };
> > >
> > > +static ssize_t permit_just_work_repair_read(struct file *file,
> > > +                                         char __user *user_buf,
> > > +                                         size_t count, loff_t *ppos)
> > > +{
> > > +     struct hci_dev *hdev = file->private_data;
> > > +     char buf[3];
> > > +
> > > +     buf[0] = hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) ? 'Y'
> > > +                                                                   : 'N';
> > > +     buf[1] = '\n';
> > > +     buf[2] = '\0';
> > > +     return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> > > +}
> > > +
> > > +static ssize_t permit_just_work_repair_write(struct file *file,
> > > +                                          const char __user *user_buf,
> > > +                                          size_t count, loff_t *ppos)
> > > +{
> > > +     struct hci_dev *hdev = file->private_data;
> > > +     char buf[32];
> > > +     size_t buf_size = min(count, (sizeof(buf) - 1));
> > > +     bool enable;
> > > +
> > > +     if (copy_from_user(buf, user_buf, buf_size))
> > > +             return -EFAULT;
> > > +
> > > +     buf[buf_size] = '\0';
> > > +     if (strtobool(buf, &enable))
> > > +             return -EINVAL;
> > > +
> > > +     if (enable)
> > > +             hci_dev_set_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> > > +     else
> > > +             hci_dev_clear_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> > > +
> > > +     return count;
> > > +}
> > > +
> > > +static const struct file_operations permit_just_work_repair_fops = {
> > > +     .open           = simple_open,
> > > +     .read           = permit_just_work_repair_read,
> > > +     .write          = permit_just_work_repair_write,
> > > +     .llseek         = default_llseek,
> > > +};
> > > +
> > > static void hci_debugfs_create_basic(struct hci_dev *hdev)
> > > {
> > >       debugfs_create_file("dut_mode", 0644, hdev->debugfs, hdev,
> > >                           &dut_mode_fops);
> > > +     debugfs_create_file("permit_just_work_repair", 0644, hdev->debugfs,
> > > +                         hdev, &permit_just_work_repair_fops);
> > >
> > >       if (hdev->set_diag)
> > >               debugfs_create_file("vendor_diag", 0644, hdev->debugfs, hdev,
> > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > index 6ddc4a74a5e4..898e347e19e0 100644
> > > --- a/net/bluetooth/hci_event.c
> > > +++ b/net/bluetooth/hci_event.c
> > > @@ -4539,6 +4539,18 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
> > >               goto unlock;
> > >       }
> > >
> > > +     /* If there already exists link key in local host, terminate the
> > > +      * connection by default since the remote device could be malicious.
> > > +      * Permit the connection if permit_just_work_repair is enabled.
> > > +      */
> > > +     if (!hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) &&
> > > +         hci_find_link_key(hdev, &ev->bdaddr)) {
> > > +             BT_DBG("Rejecting request: local host already have link key");
> >
> > Can we use bt_dev_warn() here.
> >
> > > +             hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
> > > +                          sizeof(ev->bdaddr), &ev->bdaddr);
> > > +             goto unlock;
> > > +     }
> > > +
> > >       /* If no side requires MITM protection; auto-accept */
> > >       if ((!loc_mitm || conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) &&
> > >           (!rem_mitm || conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)) {
> >
> > What about the LE cases?
> >
> > In addition, I like to get a pair of second eyes from Johan and Luiz on this one.
> >
> > Regards
> >
> > Marcel
> >
> > --
> > You received this message because you are subscribed to the Google Groups "ChromeOS Bluetooth Upstreaming" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to chromeos-bluetooth-upstreaming+unsubscribe@chromium.org.
> > To post to this group, send email to chromeos-bluetooth-upstreaming@chromium.org.
> > To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromeos-bluetooth-upstreaming/CD07E771-6F40-4158-A0F9-03FC128CDCD3%40holtmann.org.



-- 
Luiz Augusto von Dentz

Re: [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack

[Marcel Holtmann]

Hi Howard,

> Attack scenario:
> 1. A Chromebook (let's call this device A) is paired to a legitimate
>   Bluetooth classic device (e.g. a speaker) (let's call this device
>   B).
> 2. A malicious device (let's call this device C) pretends to be the
>   Bluetooth speaker by using the same BT address.
> 3. If device A is not currently connected to device B, device A will
>   be ready to accept connection from device B in the background
>   (technically, doing Page Scan).
> 4. Therefore, device C can initiate connection to device A
>   (because device A is doing Page Scan) and device A will accept the
>   connection because device A trusts device C's address which is the
>   same as device B's address.
> 5. Device C won't be able to communicate at any high level Bluetooth
>   profile with device A because device A enforces that device C is
>   encrypted with their common Link Key, which device C doesn't have.
>   But device C can initiate pairing with device A with just-works
>   model without requiring user interaction (there is only pairing
>   notification). After pairing, device A now trusts device C with a
>   new different link key, common between device A and C.
> 6. From now on, device A trusts device C, so device C can at anytime
>   connect to device A to do any kind of high-level hijacking, e.g.
>   speaker hijack or mouse/keyboard hijack.
> 
> To fix this, reject the pairing if all the conditions below are met.
> - the pairing is initialized by peer
> - the authorization method is just-work
> - host already had the link key to the peer
> 
> Also create a debugfs option to permit the pairing even the
> conditions above are met.
> 
> Signed-off-by: howardchung <howardchung@google.com>
> ---
> 
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_core.c    | 47 +++++++++++++++++++++++++++++++++++++
> net/bluetooth/hci_event.c   | 12 ++++++++++
> 3 files changed, 60 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 07b6ecedc6ce..4918b79baa41 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -283,6 +283,7 @@ enum {
> 	HCI_FORCE_STATIC_ADDR,
> 	HCI_LL_RPA_RESOLUTION,
> 	HCI_CMD_PENDING,
> +	HCI_PERMIT_JUST_WORK_REPAIR,

Call this simply JUST_WORKS_REPAIRING.

> 
> 	__HCI_NUM_FLAGS,
> };
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9e19d5a3aac8..9014aa567e7b 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -172,10 +172,57 @@ static const struct file_operations vendor_diag_fops = {
> 	.llseek		= default_llseek,
> };
> 
> +static ssize_t permit_just_work_repair_read(struct file *file,
> +					    char __user *user_buf,
> +					    size_t count, loff_t *ppos)
> +{
> +	struct hci_dev *hdev = file->private_data;
> +	char buf[3];
> +
> +	buf[0] = hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) ? 'Y'
> +								      : 'N';
> +	buf[1] = '\n';
> +	buf[2] = '\0';
> +	return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> +}
> +
> +static ssize_t permit_just_work_repair_write(struct file *file,
> +					     const char __user *user_buf,
> +					     size_t count, loff_t *ppos)
> +{
> +	struct hci_dev *hdev = file->private_data;
> +	char buf[32];
> +	size_t buf_size = min(count, (sizeof(buf) - 1));
> +	bool enable;
> +
> +	if (copy_from_user(buf, user_buf, buf_size))
> +		return -EFAULT;
> +
> +	buf[buf_size] = '\0';
> +	if (strtobool(buf, &enable))
> +		return -EINVAL;
> +
> +	if (enable)
> +		hci_dev_set_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> +	else
> +		hci_dev_clear_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR);
> +
> +	return count;
> +}
> +
> +static const struct file_operations permit_just_work_repair_fops = {
> +	.open		= simple_open,
> +	.read		= permit_just_work_repair_read,
> +	.write		= permit_just_work_repair_write,
> +	.llseek		= default_llseek,
> +};
> +
> static void hci_debugfs_create_basic(struct hci_dev *hdev)
> {
> 	debugfs_create_file("dut_mode", 0644, hdev->debugfs, hdev,
> 			    &dut_mode_fops);
> +	debugfs_create_file("permit_just_work_repair", 0644, hdev->debugfs,
> +			    hdev, &permit_just_work_repair_fops);

Call this just_works_repairing.

I have a bad association with “repair” since that means to me that you are trying to repair something that is broken.

> 
> 	if (hdev->set_diag)
> 		debugfs_create_file("vendor_diag", 0644, hdev->debugfs, hdev,
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 6ddc4a74a5e4..898e347e19e0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4539,6 +4539,18 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
> 		goto unlock;
> 	}
> 
> +	/* If there already exists link key in local host, terminate the
> +	 * connection by default since the remote device could be malicious.
> +	 * Permit the connection if permit_just_work_repair is enabled.
> +	 */
> +	if (!hci_dev_test_flag(hdev, HCI_PERMIT_JUST_WORK_REPAIR) &&
> +	    hci_find_link_key(hdev, &ev->bdaddr)) {
> +		BT_DBG("Rejecting request: local host already have link key");
> +		hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
> +			     sizeof(ev->bdaddr), &ev->bdaddr);
> +		goto unlock;
> +	}
> +
> 	/* If no side requires MITM protection; auto-accept */
> 	if ((!loc_mitm || conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) &&
> 	    (!rem_mitm || conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)) {

Looking at this patch as my own second pair of eyes, I am not sure this is the right location to just outright reject the confirmation request.

We need to support upgrading an unauthenticated key into an authenticated key (there is a qualification test case for this). Only when we decided that we are doing just-works auto-accept, then we should reject the pairing if there is an existing link key.

I know that PTS has a test case, but I wonder we actually have a test case in our own test suite. Maybe we don’t and we should really add one to ensure we behave correctly.

Regards

Marcel