Message ID | 20200214191609.Bluez.v5.1.Ia71869d2f3e19a76a6a352c61088a085a1d41ba6@changeid (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Marcel Holtmann |
Headers | show |
Series | [Bluez,v5] 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 v5: > - Rephrase the comment > > Changes in v4: > - optimise the check in smp.c. > > 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 | 19 +++++++++++++++++++ > 2 files changed, 29 insertions(+) patch has been applied to bluetooth-next tree. Regards Marcel
Hi Howard, On Fri, Feb 14, 2020 at 07:16:41PM +0800, Howard Chung wrote: > 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 v5: > - Rephrase the comment > > Changes in v4: > - optimise the check in smp.c. > > 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 | 19 +++++++++++++++++++ > 2 files changed, 29 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..25dbf77d216b 100644 > --- a/net/bluetooth/smp.c > +++ b/net/bluetooth/smp.c > @@ -2192,6 +2192,25 @@ 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); > + > + /* Only Just-Works pairing requires extra checks */ > + if (smp->method != JUST_WORKS) > + goto mackey_and_ltk; > + > + /* 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_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, > + hcon->role)) { > + err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, > + hcon->type, > + hcon->dst_type, passkey, We received a report from the 0day bot when building with clang that passkey is uninitialized when used here: https://groups.google.com/forum/#!topic/clang-built-linux/kyRKCjRsGoU It appears to be legitimate as if we get to this point, we have not touched the value of passkey but I do not know if there is any contextual code flow going on where this can never happen but I do not see how. Would you mind looking into it? Cheers, Nathan
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..25dbf77d216b 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -2192,6 +2192,25 @@ 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); + + /* Only Just-Works pairing requires extra checks */ + if (smp->method != JUST_WORKS) + goto mackey_and_ltk; + + /* 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_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, + hcon->role)) { + 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 v5: - Rephrase the comment Changes in v4: - optimise the check in smp.c. 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 | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+)