Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH 1/4] Bluetooth: Disconnect if E0 is used for Level 4
Date: Wed, 20 May 2020 09:00:13 -0700
Message-ID: <CABBYNZLJhFqVTtmHip+oqUrBd-rYa19aZZroOHdVSNRM=-D_BQ@mail.gmail.com> (raw)
In-Reply-To: <180CD116-CEC8-42B3-B3F0-C8EAF76FB1A7@holtmann.org>

Hi Marcel,

On Wed, May 20, 2020 at 7:23 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > E0 is not allowed with Level 4:
> >
> > BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319:
> >
> >  '128-bit equivalent strength for link and encryption keys
> >   required using FIPS approved algorithms (E0 not allowed,
> >   SAFER+ not allowed, and P-192 not allowed; encryption key
> >   not shortened'
> >
> > SC enabled:
> >
> >> HCI Event: Read Remote Extended Features (0x23) plen 13
> >        Status: Success (0x00)
> >        Handle: 256
> >        Page: 1/2
> >        Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> >          Secure Simple Pairing (Host Support)
> >          LE Supported (Host)
> >          Secure Connections (Host Support)
> >> HCI Event: Encryption Change (0x08) plen 4
> >        Status: Success (0x00)
> >        Handle: 256
> >        Encryption: Enabled with AES-CCM (0x02)
> >
> > SC disabled:
> >
> >> HCI Event: Read Remote Extended Features (0x23) plen 13
> >        Status: Success (0x00)
> >        Handle: 256
> >        Page: 1/2
> >        Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> >          Secure Simple Pairing (Host Support)
> >          LE Supported (Host)
> >> HCI Event: Encryption Change (0x08) plen 4
> >        Status: Success (0x00)
> >        Handle: 256
> >        Encryption: Enabled with E0 (0x01)
> > [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used
> > < HCI Command: Disconnect (0x01|0x0006) plen 3
> >        Handle: 256
> >        Reason: Authentication Failure (0x05)
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > net/bluetooth/hci_conn.c  | 17 +++++++++++++++++
> > net/bluetooth/hci_event.c |  6 ++----
> > 2 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 07c34c55fc50..0c1cae83c8dc 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -1325,6 +1325,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn)
> >                       return 0;
> >       }
> >
> > +      /* AES encryption is required for Level 4:
> > +       *
> > +       * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C
> > +       * page 1319:
> > +       *
> > +       * 128-bit equivalent strength for link and encryption keys
> > +       * required using FIPS approved algorithms (E0 not allowed,
> > +       * SAFER+ not allowed, and P-192 not allowed; encryption key
> > +       * not shortened)
> > +       */
> > +     if (conn->sec_level == BT_SECURITY_FIPS &&
> > +         !test_bit(HCI_CONN_AES_CCM, &conn->flags)) {
> > +             bt_dev_err(conn->hdev, "Invalid security: expect AES but E0 "
> > +                        "was used");
>
> make this bt_dev_warn since it is something that can happen. We fail appropriately now. Also try to fit the error in a single line. “Invalid security: Missing AES-CCM usage”.

Sure will update this.

> > +             return 0;
> > +     }
> > +
> >       if (hci_conn_ssp_enabled(conn) &&
> >           !test_bit(HCI_CONN_ENCRYPT, &conn->flags))
> >               return 0;
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 006c24e04b44..dc1cc3c4348c 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -3078,10 +3078,8 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >        * that are not encrypted with AES-CCM using a P-256 authenticated
> >        * combination key.
> >        */
> > -     if (hci_dev_test_flag(hdev, HCI_SC_ONLY) &&
> > -         (!test_bit(HCI_CONN_AES_CCM, &conn->flags) ||
> > -          conn->key_type != HCI_LK_AUTH_COMBINATION_P256)) {
> > -             hci_connect_cfm(conn, HCI_ERROR_AUTH_FAILURE);
> > +     if (!hci_conn_check_link_mode(conn)) {
> > +             hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
>
> Why is this a disconnect now? If this is better, then this needs to be explained above, but I have the feeling we really want to respond to the connect request in a clean fashion.

While testing this I discover that calling connect_cfm doesn't
actually cause a disconnect, at least not immediately, and by
disconnecting it actually provides the right error to the remote. Note
that there is a similar usage in the if statement right above this
code, but I believe that even in case we want to cleanup the earlier
what we should be calling is hci_encrypt_cfm.

> >               hci_conn_drop(conn);
> >               goto unlock;
>
> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz

      reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-19 20:25 Luiz Augusto von Dentz
2020-05-19 20:25 ` [PATCH 2/4] Bluetooth: Fix assuming EIR flags can result in SSP authentication Luiz Augusto von Dentz
2020-05-20 14:34   ` Marcel Holtmann
2020-05-26 13:53     ` Alain Michaud
2020-05-26 14:17       ` Marcel Holtmann
2020-05-26 14:30         ` Alain Michaud
2020-05-28  8:22           ` Marcel Holtmann
2020-05-28 13:17             ` Alain Michaud
2020-05-28 16:53               ` Luiz Augusto von Dentz
2020-05-28 17:16                 ` Alain Michaud
2020-06-03 18:02                   ` Marcel Holtmann
2020-05-19 20:25 ` [PATCH 3/4] Bluetooth: Fix bogus check for re-auth no supported with non-ssp Luiz Augusto von Dentz
2020-05-20 14:25   ` Marcel Holtmann
2020-05-20 16:12     ` Luiz Augusto von Dentz
2020-05-19 20:25 ` [PATCH 4/4] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Luiz Augusto von Dentz
2020-05-20 14:31   ` Marcel Holtmann
2020-05-20 14:23 ` [PATCH 1/4] Bluetooth: Disconnect if E0 is used for Level 4 Marcel Holtmann
2020-05-20 16:00   ` Luiz Augusto von Dentz [this message]

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='CABBYNZLJhFqVTtmHip+oqUrBd-rYa19aZZroOHdVSNRM=-D_BQ@mail.gmail.com' \
    --to=luiz.dentz@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=marcel@holtmann.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

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org
	public-inbox-index linux-bluetooth

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git