| Message ID | 20200212212316.Bluez.v3.1.Ia71869d2f3e19a76a6a352c61088a085a1d41ba6@changeid (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Marcel Holtmann |
| Headers | show |
| Series | [Bluez,v3] bluetooth: secure bluetooth stack from bluedump attack | expand |
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. > > Since we don't know whether the repairing is legitimate or not, > leave the decision to user space 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 > > Signed-off-by: Howard Chung <howardchung@google.com> > --- > > Changes in v3: > - Change confirm_hint from 2 to 1 > - Fix coding style (declaration order) > > Changes in v2: > - Remove the HCI_PERMIT_JUST_WORK_REPAIR debugfs option > - Fix the added code in classic > - Add a similar fix for LE > > net/bluetooth/hci_event.c | 10 ++++++++++ > net/bluetooth/smp.c | 18 ++++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 2c833dae9366..e6982f4f51ea 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -4571,6 +4571,16 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev, > goto confirm; > } > > + /* If there already exists link key in local host, leave the > + * decision to user space since the remote device could be > + * legitimate or malicious. > + */ > + if (hci_find_link_key(hdev, &ev->bdaddr)) { > + bt_dev_warn(hdev, "Local host already has link key"); > + confirm_hint = 1; > + goto confirm; > + } > + > BT_DBG("Auto-accept of user confirmation with %ums delay", > hdev->auto_accept_delay); > > diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c > index 2cba6e07c02b..1483ceea3bab 100644 > --- a/net/bluetooth/smp.c > +++ b/net/bluetooth/smp.c > @@ -2139,6 +2139,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb) > struct l2cap_chan *chan = conn->smp; > struct smp_chan *smp = chan->data; > struct hci_conn *hcon = conn->hcon; > + struct smp_ltk *key; > u8 *pkax, *pkbx, *na, *nb; > u32 passkey; > int err; > @@ -2192,6 +2193,23 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb) > smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(smp->prnd), > smp->prnd); > SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK); > + > + key = hci_find_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, > + hcon->role); > + > + /* If there already exists link key in local host, leave the > + * decision to user space since the remote device could be > + * legitimate or malicious. > + */ > + if (smp->method == JUST_WORKS && key) { > + err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, > + hcon->type, > + hcon->dst_type, passkey, > + 1); > + if (err) > + return SMP_UNSPECIFIED; > + set_bit(SMP_FLAG_WAIT_USER, &smp->flags); > + } > } while this looks good, I like to optimize this to only look up the LTK when needed. /* comment here */ if (smp->method != JUST_WORKS) goto mackey_and_ltk; /* and command here */ if (hci_find_ltk()) { mgmt_user_confirm_request() .. } And my preference that we also get an Ack from Johan or Luiz that double checked that this is fine. Regards Marcel
Hi Marcel, On 12. Feb 2020, at 17.19, Marcel Holtmann <marcel@holtmann.org> wrote: >> + key = hci_find_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, >> + hcon->role); >> + >> + /* If there already exists link key in local host, leave the >> + * decision to user space since the remote device could be >> + * legitimate or malicious. >> + */ >> + if (smp->method == JUST_WORKS && key) { >> + err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, >> + hcon->type, >> + hcon->dst_type, passkey, >> + 1); >> + if (err) >> + return SMP_UNSPECIFIED; >> + set_bit(SMP_FLAG_WAIT_USER, &smp->flags); >> + } >> } > > while this looks good, I like to optimize this to only look up the LTK when needed. > > /* comment here */ > if (smp->method != JUST_WORKS) > goto mackey_and_ltk; > > > /* and command here */ > if (hci_find_ltk()) { > mgmt_user_confirm_request() > .. > } > > And my preference that we also get an Ack from Johan or Luiz that double checked that this is fine. Acked-by: Johan Hedberg <johan.hedberg@intel.com> Johan
Hi Howard, On Wed, Feb 12, 2020 at 7:39 AM Johan Hedberg <johan.hedberg@gmail.com> wrote: > > Hi Marcel, > > On 12. Feb 2020, at 17.19, Marcel Holtmann <marcel@holtmann.org> wrote: > >> + key = hci_find_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, > >> + hcon->role); > >> + > >> + /* If there already exists link key in local host, leave the > >> + * decision to user space since the remote device could be > >> + * legitimate or malicious. > >> + */ > >> + if (smp->method == JUST_WORKS && key) { > >> + err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, > >> + hcon->type, > >> + hcon->dst_type, passkey, > >> + 1); > >> + if (err) > >> + return SMP_UNSPECIFIED; > >> + set_bit(SMP_FLAG_WAIT_USER, &smp->flags); > >> + } > >> } > > > > while this looks good, I like to optimize this to only look up the LTK when needed. I wonder why we don't just call user confirm everytime? That way the new policy preference applies to both a new pair or when already paired, and we don't have to really do the key lookup here since the userspace can do the check if really needed. > > > > /* comment here */ > > if (smp->method != JUST_WORKS) > > goto mackey_and_ltk; > > > > > > /* and command here */ > > if (hci_find_ltk()) { > > mgmt_user_confirm_request() > > .. > > } > > > > And my preference that we also get an Ack from Johan or Luiz that double checked that this is fine. > > Acked-by: Johan Hedberg <johan.hedberg@intel.com> > > Johan
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 2c833dae9366..e6982f4f51ea 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -4571,6 +4571,16 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev, goto confirm; } + /* If there already exists link key in local host, leave the + * decision to user space since the remote device could be + * legitimate or malicious. + */ + if (hci_find_link_key(hdev, &ev->bdaddr)) { + bt_dev_warn(hdev, "Local host already has link key"); + confirm_hint = 1; + goto confirm; + } + BT_DBG("Auto-accept of user confirmation with %ums delay", hdev->auto_accept_delay); diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 2cba6e07c02b..1483ceea3bab 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -2139,6 +2139,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb) struct l2cap_chan *chan = conn->smp; struct smp_chan *smp = chan->data; struct hci_conn *hcon = conn->hcon; + struct smp_ltk *key; u8 *pkax, *pkbx, *na, *nb; u32 passkey; int err; @@ -2192,6 +2193,23 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb) smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(smp->prnd), smp->prnd); SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK); + + key = hci_find_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, + hcon->role); + + /* If there already exists link key in local host, leave the + * decision to user space since the remote device could be + * legitimate or malicious. + */ + if (smp->method == JUST_WORKS && key) { + err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, + hcon->type, + hcon->dst_type, passkey, + 1); + if (err) + return SMP_UNSPECIFIED; + set_bit(SMP_FLAG_WAIT_USER, &smp->flags); + } } mackey_and_ltk:
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. Since we don't know whether the repairing is legitimate or not, leave the decision to user space 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 Signed-off-by: Howard Chung <howardchung@google.com> --- Changes in v3: - Change confirm_hint from 2 to 1 - Fix coding style (declaration order) Changes in v2: - Remove the HCI_PERMIT_JUST_WORK_REPAIR debugfs option - Fix the added code in classic - Add a similar fix for LE net/bluetooth/hci_event.c | 10 ++++++++++ net/bluetooth/smp.c | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+)