Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200320133748.154926-1-alainm@chromium.org>
In-Reply-To: <20200320133748.154926-1-alainm@chromium.org>
From:   Alain Michaud <alainmichaud@google.com>
Date:   Fri, 20 Mar 2020 09:41:53 -0400
Message-ID: <CALWDO_WSHiNw+uwcZzufJ7rjHr=zpMs6f3ry=rdLMGLz2gJZ=w@mail.gmail.com>
Subject: Re: [PATCH v2] bluetooth: Enforce classic key size verification.
To:     Alain Michaud <alainm@chromium.org>,
        "Holtmann, Marcel" <marcel.holtmann@intel.com>
Cc:     BlueZ <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Marcel,


On Fri, Mar 20, 2020 at 9:37 AM Alain Michaud <alainm@chromium.org> wrote:
>
> This change introduces a new configuration to strictly enforce key size
> checks.  This ensures that systems are in a secured configuration by
> default while allowing for a compatible posture via a Kconfig option to
> support controllers who may not support the read encryption key size
> command.
>
> Signed-off-by: Alain Michaud <alainm@chromium.org>
> ---
>
>  net/bluetooth/Kconfig     | 20 ++++++++++++++++++++
>  net/bluetooth/hci_core.c  | 10 ++++++++++
>  net/bluetooth/hci_event.c |  4 ++++
>  3 files changed, 34 insertions(+)
>
> diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
> index 165148c7c4ce..8e177d4f3f02 100644
> --- a/net/bluetooth/Kconfig
> +++ b/net/bluetooth/Kconfig
> @@ -128,4 +128,24 @@ config BT_DEBUGFS
>           Provide extensive information about internal Bluetooth states
>           in debugfs.
>
> +config BT_EXPERT
> +       bool "Expert Bluetooth options"
> +       depends on BT
> +       default n
> +       help
> +         Provides a set of expert options and configurations that should
> +         only be used deliberately by BT experts.  This is considered a
> +         global switch to ensure these advanced features or options that
> +         depends on BT_EXPERT are only used in expert mode.
> +
> +config BT_ENFORCE_CLASSIC_KEY_SIZES
> +       bool "Enforces security requirements for Bluetooth classic"
> +       depends on BT && BT_EXPERT
> +       default y
> +       help
> +         Enforces Bluetooth classic security requirements by disallowing
> +         use of insecure Bluetooth controllers, i.e. that doesn't support
> +         Read Encryption Key Size command to prevent BT classic connection
> +         with very short encryption key.
> +
>  source "drivers/bluetooth/Kconfig"
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 4e6d61a95b20..142130d4b66b 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1540,6 +1540,16 @@ static int hci_dev_do_open(struct hci_dev *hdev)
>
>         clear_bit(HCI_INIT, &hdev->flags);
>
> +#ifdef BT_ENFORCE_CLASSIC_KEY_SIZES
> +       /* Don't allow usage of Bluetooth if the chip doesn't support */
> +       /* Read Encryption Key Size command */
> +       if (!ret && !(hdev->commands[20] & 0x10)) {
> +               bt_dev_err(hdev,
> +                          "Disabling BT, Read Encryption Key Size !supported");
> +               ret = -EIO;
> +       }
> +#endif
Just FYI, I haven't changed this bit yet.  I'll wait for your guidance
on where best to put this to leave the controller in the right state.
> +
>         if (!ret) {
>                 hci_dev_hold(hdev);
>                 hci_dev_set_flag(hdev, HCI_RPA_EXPIRED);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index a40ed31f6eb8..54f90799a088 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2902,7 +2902,11 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status,
>         if (rp->status) {
>                 bt_dev_err(hdev, "failed to read key size for handle %u",
>                            handle);
> +#ifdef BT_ENFORCE_CLASSIC_KEY_SIZES
> +               conn->enc_key_size = 0;
> +#else
>                 conn->enc_key_size = HCI_LINK_KEY_SIZE;
> +#endif
>         } else {
>                 conn->enc_key_size = rp->key_size;
>         }
> --
> 2.25.1.696.g5e7596f4ac-goog
>