All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yun-hao Chung <howardchung@google.com>
To: Matias Karhumaa <matias.karhumaa@gmail.com>
Cc: chromeos-bluetooth-upstreaming@chromium.org,
	"David S. Miller" <davem@davemloft.net>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	Marcel Holtmann <marcel@holtmann.org>,
	netdev@vger.kernel.org
Subject: Re: [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack
Date: Tue, 7 Jan 2020 11:30:24 +0800	[thread overview]
Message-ID: <CAPHZWUfMZAqkj++icECVPs48=S4SEHsLz-A=kUcoOVp8A-Mu7Q@mail.gmail.com> (raw)
In-Reply-To: <CAMCGoNyzN17pK_4t6bWt4OLsWnFUEdn06dJwz=mhzxQLv0BbOA@mail.gmail.com>

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

  reply	other threads:[~2020-01-07  3:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-06 10:14 [Bluez PATCH v1] bluetooth: secure bluetooth stack from bluedump attack howardchung
     [not found] ` <CAMCGoNzCvOgcg0hAhDO-wcFLgX75JL-G6q1KuGFEjrEz+oPTxA@mail.gmail.com>
2020-01-06 11:44   ` Fwd: " Matias Karhumaa
2020-01-07  3:30     ` Yun-hao Chung [this message]
2020-01-08 21:02 ` Marcel Holtmann
2020-01-22 17:52   ` Alain Michaud
2020-01-23 17:46     ` Marcel Holtmann
2020-01-31  0:25     ` Luiz Augusto von Dentz
2020-01-29  9:00 ` Marcel Holtmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPHZWUfMZAqkj++icECVPs48=S4SEHsLz-A=kUcoOVp8A-Mu7Q@mail.gmail.com' \
    --to=howardchung@google.com \
    --cc=chromeos-bluetooth-upstreaming@chromium.org \
    --cc=davem@davemloft.net \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=matias.karhumaa@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.