From: Mikel Astiz <[email protected]>
The way the kernel handles MITM Protection during pairing is inconsistent: General Bonding and Dedicated Bonding are not treated equally.
>From the user's perspective, using the MITM Protection usually means he will have to confirm the pairing in the UI (some pop-up showing the passkey). Making a difference between General and Dedicated Bonding is undesired because, in practice, the user normally doesn't care about which of them is used. Currently, if an iPhone is paired (initiated on the phone), no pop-up will be shown (because it's using General Bonding). This differs from pairing an Android device (using Dedicated Bonding), which an average user would not understand why.
The GAP Specification describes when MITM Protection should be used (Bluetooth Core Specification v4.0 Volume 3, part C, section 6.5.3). It makes no distinction between General vs Dedicated Bonding: the recommendation is *not* to use it "unless the security policy of an available local service requires MITM Protection".
However, the kernel doesn't necessarily have this information in a reliable way. Therefore, the safest choice is to always request MITM Protection, also for General Bonding [1]. The proposal here is to do this for both incoming (patch 6/8) and outgoing (patch 7/8) procedures, as it was previously done for Dedicated Bonding. This "conservative" approach is smart enough to fall back to not using MITM Protection if the IO capabilities don't allow it (this policy already existed before for Dedicated Bonding, see patch 5/8).
Some systems might however know that MITM Protection is not required at all, because no supported profile requires high security. This can be expressed by userland using the newly introduced management command (patch 8/8). In this case, the recommendation in the spec will be followed (affecting both General and Dedicated Bonding).
Note that the first 5 patches are refactoring patches which shouldn't change the behavior of the code. Within this group, patch 5/8 is the most tricky one since side effects could exist (we weren't able to observed them though).
[1] We make an exception here for No-Bonding, which remains unmodified. In this case, no MITM Protection is required by default since an additional pop-up would be undesireable for most use-cases.
Mikel Astiz (6):
Bluetooth: Add HCI authentication capabilities macros
Bluetooth: Use defines in in hci_get_auth_req()
Bluetooth: Use defines instead of integer literals
Bluetooth: Refactor hci_get_auth_req()
Bluetooth: Refactor code for outgoing dedicated bonding
Bluetooth: Request MITM Protection when initiator
Timo Mueller (2):
Bluetooth: Use MITM Protection when IO caps allow it
Bluetooth: Add management command to relax MITM Protection
include/net/bluetooth/hci.h | 9 ++++++-
include/net/bluetooth/mgmt.h | 3 +++
net/bluetooth/hci_event.c | 63 ++++++++++++++++++++++++++++----------------
net/bluetooth/mgmt.c | 53 +++++++++++++++++++++++++++++++++----
4 files changed, 100 insertions(+), 28 deletions(-)
--
1.8.1.4
Hi,
On Fri, Jun 28, 2013 at 10:56 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> The way the kernel handles MITM Protection during pairing is inconsistent: General Bonding and Dedicated Bonding are not treated equally.
>
> From the user's perspective, using the MITM Protection usually means he will have to confirm the pairing in the UI (some pop-up showing the passkey). Making a difference between General and Dedicated Bonding is undesired because, in practice, the user normally doesn't care about which of them is used. Currently, if an iPhone is paired (initiated on the phone), no pop-up will be shown (because it's using General Bonding). This differs from pairing an Android device (using Dedicated Bonding), which an average user would not understand why.
>
> The GAP Specification describes when MITM Protection should be used (Bluetooth Core Specification v4.0 Volume 3, part C, section 6.5.3). It makes no distinction between General vs Dedicated Bonding: the recommendation is *not* to use it "unless the security policy of an available local service requires MITM Protection".
>
> However, the kernel doesn't necessarily have this information in a reliable way. Therefore, the safest choice is to always request MITM Protection, also for General Bonding [1]. The proposal here is to do this for both incoming (patch 6/8) and outgoing (patch 7/8) procedures, as it was previously done for Dedicated Bonding. This "conservative" approach is smart enough to fall back to not using MITM Protection if the IO capabilities don't allow it (this policy already existed before for Dedicated Bonding, see patch 5/8).
>
> Some systems might however know that MITM Protection is not required at all, because no supported profile requires high security. This can be expressed by userland using the newly introduced management command (patch 8/8). In this case, the recommendation in the spec will be followed (affecting both General and Dedicated Bonding).
>
> Note that the first 5 patches are refactoring patches which shouldn't change the behavior of the code. Within this group, patch 5/8 is the most tricky one since side effects could exist (we weren't able to observed them though).
>
> [1] We make an exception here for No-Bonding, which remains unmodified. In this case, no MITM Protection is required by default since an additional pop-up would be undesireable for most use-cases.
>
> Mikel Astiz (6):
> Bluetooth: Add HCI authentication capabilities macros
> Bluetooth: Use defines in in hci_get_auth_req()
> Bluetooth: Use defines instead of integer literals
> Bluetooth: Refactor hci_get_auth_req()
> Bluetooth: Refactor code for outgoing dedicated bonding
> Bluetooth: Request MITM Protection when initiator
>
> Timo Mueller (2):
> Bluetooth: Use MITM Protection when IO caps allow it
> Bluetooth: Add management command to relax MITM Protection
>
> include/net/bluetooth/hci.h | 9 ++++++-
> include/net/bluetooth/mgmt.h | 3 +++
> net/bluetooth/hci_event.c | 63 ++++++++++++++++++++++++++++----------------
> net/bluetooth/mgmt.c | 53 +++++++++++++++++++++++++++++++++----
> 4 files changed, 100 insertions(+), 28 deletions(-)
For the record, I have no particular interest in the last two patches.
They're submitted here for completion purposes as a base for the
discussion.
Cheers,
Mikel
From: Mikel Astiz <[email protected]>
The GAP Specification gives the flexibility to decide whether MITM
Protection is requested or not (Bluetooth Core Specification v4.0
Volume 3, part C, section 6.5.3) when replying to an
HCI_EV_IO_CAPA_REQUEST event.
The recommendation is *not* to set this flag "unless the security
policy of an available local service requires MITM Protection"
(regardless of the bonding type). However, the kernel doesn't
necessarily have this information and therefore the safest choice is
to always use MITM Protection, also for General Bonding.
This patch changes the behavior for the General Bonding initiator
role, always requesting MITM Protection even if no high security level
is used. Depending on the remote capabilities, the protection might
not be actually used, and we will accept this locally unless of course
a high security level was originally required.
Note that this was already done for Dedicated Bonding. No-Bonding is
left unmodified because MITM Protection is normally not desired in
these cases.
Signed-off-by: Mikel Astiz <[email protected]>
Signed-off-by: Timo Mueller <[email protected]>
---
net/bluetooth/hci_event.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 283cb3f..461150d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3071,9 +3071,11 @@ static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
if (conn->remote_auth == 0xff) {
cp.authentication = conn->auth_type;
- /* Use MITM protection for outgoing dedicated bonding */
+ /* Request MITM protection if our IO caps allow it
+ * except for the no-bonding case
+ */
if (conn->io_capability != HCI_IO_NO_INPUT_OUTPUT &&
- cp.authentication == HCI_AT_DEDICATED_BONDING)
+ cp.authentication != HCI_AT_NO_BONDING)
cp.authentication |= 0x01;
} else {
conn->auth_type = hci_get_auth_req(conn);
--
1.8.1.4
From: Mikel Astiz <[email protected]>
Refactor the code without changing its behavior by handling the
no-bonding cases first followed by General Bonding.
Signed-off-by: Mikel Astiz <[email protected]>
Signed-off-by: Timo Mueller <[email protected]>
---
net/bluetooth/hci_event.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 50e39f4..fe78b9c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3023,24 +3023,25 @@ unlock:
static u8 hci_get_auth_req(struct hci_conn *conn)
{
- /* If remote requests dedicated bonding follow that lead */
- if (conn->remote_auth == HCI_AT_DEDICATED_BONDING ||
- conn->remote_auth == HCI_AT_DEDICATED_BONDING_MITM) {
- /* If both remote and local IO capabilities allow MITM
- * protection then require it, otherwise don't */
- if (conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT ||
- conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)
- return HCI_AT_DEDICATED_BONDING;
- else
- return HCI_AT_DEDICATED_BONDING_MITM;
- }
-
/* If remote requests no-bonding follow that lead */
if (conn->remote_auth == HCI_AT_NO_BONDING ||
conn->remote_auth == HCI_AT_NO_BONDING_MITM)
return conn->remote_auth | (conn->auth_type & 0x01);
- return conn->auth_type;
+ /* For general bonding, use the given auth_type */
+ if (conn->remote_auth == HCI_AT_GENERAL_BONDING ||
+ conn->remote_auth == HCI_AT_GENERAL_BONDING_MITM)
+ return conn->auth_type;
+
+ /* If both remote and local have enough IO capabilities, require
+ * MITM protection
+ */
+ if (conn->remote_cap != HCI_IO_NO_INPUT_OUTPUT &&
+ conn->io_capability != HCI_IO_NO_INPUT_OUTPUT)
+ return conn->remote_auth | 0x01;
+
+ /* No MITM protection possible so remove requirement */
+ return conn->remote_auth & ~0x01;
}
static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
@@ -3070,8 +3071,14 @@ static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
* to DisplayYesNo as it is not supported by BT spec. */
cp.capability = (conn->io_capability == 0x04) ?
HCI_IO_DISPLAY_YESNO : conn->io_capability;
- conn->auth_type = hci_get_auth_req(conn);
- cp.authentication = conn->auth_type;
+
+ /* If we are initiators, there is no remote information yet */
+ if (conn->remote_auth == 0xff) {
+ cp.authentication = conn->auth_type;
+ } else {
+ conn->auth_type = hci_get_auth_req(conn);
+ cp.authentication = conn->auth_type;
+ }
if (hci_find_remote_oob_data(hdev, &conn->dst) &&
(conn->out || test_bit(HCI_CONN_REMOTE_OOB, &conn->flags)))
--
1.8.1.4
From: Timo Mueller <[email protected]>
As a general rule, the Bluetooth Specification (v4.0 Volume 3, part C,
section 6.5.3) recommends *NOT* to require MITM Protection, unless the
available local services require it. The Kernel doesn't however adhere
to this recommendation because the locally available services are not
known reliably.
This lack of information is exactly what this patch addresses: a
dedicated flag is proposed in the management interface. If set to 1, the
recommentation described in the specification will be followed: it will
be assumed that none of the locally available services require MITM
Protection, unless the Kernel has any evidence of the contrary (i.e. a
socket exists with a high security level, which requires MITM
Protection).
If set to 0, MITM Protection will always be required, provided that it
is possible according to the I/O capabilities. This was the behavior
prior to this patch and therefore the flag is set to 0 by default.
Note that this affects General Bonding and Dedicated Bonding equally as
well as locally or remotely initiated pairing procedures.
Signed-off-by: Timo Mueller <[email protected]>
Signed-off-by: Mikel Astiz <[email protected]>
---
include/net/bluetooth/hci.h | 3 ++-
include/net/bluetooth/mgmt.h | 3 +++
net/bluetooth/hci_event.c | 15 +++++++++++---
net/bluetooth/mgmt.c | 48 +++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a01fbb4..9973fcc 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -112,6 +112,7 @@ enum {
HCI_LE_SCAN,
HCI_SSP_ENABLED,
+ HCI_RELAX_MITM,
HCI_HS_ENABLED,
HCI_LE_ENABLED,
HCI_LE_PERIPHERAL,
@@ -126,7 +127,7 @@ enum {
* or the HCI device is closed.
*/
#define HCI_PERSISTENT_MASK (BIT(HCI_LE_SCAN) | BIT(HCI_PERIODIC_INQ) | \
- BIT(HCI_FAST_CONNECTABLE))
+ BIT(HCI_FAST_CONNECTABLE) | BIT(HCI_RELAX_MITM))
/* HCI ioctl defines */
#define HCIDEVUP _IOW('H', 201, int)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 9944c3e..9a3218f 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -93,6 +93,7 @@ struct mgmt_rp_read_index_list {
#define MGMT_SETTING_BREDR 0x00000080
#define MGMT_SETTING_HS 0x00000100
#define MGMT_SETTING_LE 0x00000200
+#define MGMT_SETTING_RELAX_MITM 0x00000400
#define MGMT_OP_READ_INFO 0x0004
#define MGMT_READ_INFO_SIZE 0
@@ -351,6 +352,8 @@ struct mgmt_cp_set_device_id {
} __packed;
#define MGMT_SET_DEVICE_ID_SIZE 8
+#define MGMT_OP_SET_RELAX_MITM 0x0029
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 461150d..4c978d6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3028,6 +3028,12 @@ static u8 hci_get_auth_req(struct hci_conn *conn)
conn->remote_auth == HCI_AT_NO_BONDING_MITM)
return conn->remote_auth | (conn->auth_type & 0x01);
+ /* MITM Protection should be used only if strictly required, so follow
+ * the recommendation in the Spec and do not require it otherwise
+ */
+ if (test_bit(HCI_RELAX_MITM, &conn->hdev->dev_flags))
+ return conn->remote_auth | (conn->auth_type & 0x01);
+
/* If both remote and local have enough IO capabilities, require
* MITM protection
*/
@@ -3071,11 +3077,14 @@ static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
if (conn->remote_auth == 0xff) {
cp.authentication = conn->auth_type;
- /* Request MITM protection if our IO caps allow it
- * except for the no-bonding case
+ /* MITM Protection should be used only if strictly
+ * required, so follow the recommendation in the Spec
+ * and do not require it otherwise (no-bonding is left
+ * unmodified in any case)
*/
if (conn->io_capability != HCI_IO_NO_INPUT_OUTPUT &&
- cp.authentication != HCI_AT_NO_BONDING)
+ cp.authentication != HCI_AT_NO_BONDING &&
+ !test_bit(HCI_RELAX_MITM, &conn->hdev->dev_flags))
cp.authentication |= 0x01;
} else {
conn->auth_type = hci_get_auth_req(conn);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ca0ad32..66fdd8e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -76,6 +76,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_BLOCK_DEVICE,
MGMT_OP_UNBLOCK_DEVICE,
MGMT_OP_SET_DEVICE_ID,
+ MGMT_OP_SET_RELAX_MITM,
};
static const u16 mgmt_events[] = {
@@ -366,8 +367,10 @@ static u32 get_supported_settings(struct hci_dev *hdev)
settings |= MGMT_SETTING_POWERED;
settings |= MGMT_SETTING_PAIRABLE;
- if (lmp_ssp_capable(hdev))
+ if (lmp_ssp_capable(hdev)) {
settings |= MGMT_SETTING_SSP;
+ settings |= MGMT_SETTING_RELAX_MITM;
+ }
if (lmp_bredr_capable(hdev)) {
settings |= MGMT_SETTING_CONNECTABLE;
@@ -421,6 +424,9 @@ static u32 get_current_settings(struct hci_dev *hdev)
if (test_bit(HCI_HS_ENABLED, &hdev->dev_flags))
settings |= MGMT_SETTING_HS;
+ if (test_bit(HCI_RELAX_MITM, &hdev->dev_flags))
+ settings |= MGMT_SETTING_RELAX_MITM;
+
return settings;
}
@@ -1299,6 +1305,45 @@ failed:
return err;
}
+static int set_relax_mitm(struct sock *sk, struct hci_dev *hdev, void *data,
+ u16 len)
+{
+ struct mgmt_mode *cp = data;
+ u8 val;
+ int err;
+
+ BT_DBG("request for %s", hdev->name);
+
+ if (!lmp_ssp_capable(hdev))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_RELAX_MITM,
+ MGMT_STATUS_NOT_SUPPORTED);
+
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_RELAX_MITM,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ hci_dev_lock(hdev);
+
+ val = !!cp->val;
+
+ if (val == test_bit(HCI_RELAX_MITM, &hdev->dev_flags)) {
+ err = send_settings_rsp(sk, MGMT_OP_SET_RELAX_MITM, hdev);
+ goto failed;
+ }
+
+ change_bit(HCI_RELAX_MITM, &hdev->dev_flags);
+
+ err = send_settings_rsp(sk, MGMT_OP_SET_RELAX_MITM, hdev);
+ if (err < 0)
+ goto failed;
+
+ err = new_settings(hdev, sk);
+
+failed:
+ hci_dev_unlock(hdev);
+ return err;
+}
+
static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
{
struct mgmt_mode *cp = data;
@@ -3273,6 +3318,7 @@ static const struct mgmt_handler {
{ block_device, false, MGMT_BLOCK_DEVICE_SIZE },
{ unblock_device, false, MGMT_UNBLOCK_DEVICE_SIZE },
{ set_device_id, false, MGMT_SET_DEVICE_ID_SIZE },
+ { set_relax_mitm, false, MGMT_SETTING_SIZE },
};
--
1.8.1.4
From: Timo Mueller <[email protected]>
When responding to a remotely-initiated pairing procedure, a MITM
protected SSP associaton model can be used for pairing if both local
and remote IO capabilities are set to something other than
NoInputNoOutput, regardless of the bonding type (Dedicated or
General).
This was already done for Dedicated Bonding but this patch proposes to
use the same policy for General Bonding as well.
The GAP Specification gives the flexibility to decide whether MITM
Protection is used ot not (Bluetooth Core Specification v4.0 Volume 3,
part C, section 6.5.3).
Note however that the recommendation is *not* to set this flag "unless
the security policy of an available local service requires MITM
Protection" (for both Dedicated and General Bonding). However, the
kernel doesn't necessarily have this information and therefore the
safest choice is to always use MITM Protection, also for General
Bonding.
Signed-off-by: Timo Mueller <[email protected]>
Signed-off-by: Mikel Astiz <[email protected]>
---
net/bluetooth/hci_event.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4e5dc5b..283cb3f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3028,11 +3028,6 @@ static u8 hci_get_auth_req(struct hci_conn *conn)
conn->remote_auth == HCI_AT_NO_BONDING_MITM)
return conn->remote_auth | (conn->auth_type & 0x01);
- /* For general bonding, use the given auth_type */
- if (conn->remote_auth == HCI_AT_GENERAL_BONDING ||
- conn->remote_auth == HCI_AT_GENERAL_BONDING_MITM)
- return conn->auth_type;
-
/* If both remote and local have enough IO capabilities, require
* MITM protection
*/
@@ -3040,8 +3035,8 @@ static u8 hci_get_auth_req(struct hci_conn *conn)
conn->io_capability != HCI_IO_NO_INPUT_OUTPUT)
return conn->remote_auth | 0x01;
- /* No MITM protection possible so remove requirement */
- return conn->remote_auth & ~0x01;
+ /* No MITM protection possible so ignore remote requirement */
+ return (conn->remote_auth & ~0x01) | (conn->auth_type & 0x01);
}
static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
--
1.8.1.4
From: Mikel Astiz <[email protected]>
Make the code in hci_get_auth_req() more readable by using the
defined macros instead of inlining magic numbers.
Signed-off-by: Mikel Astiz <[email protected]>
Signed-off-by: Timo Mueller <[email protected]>
---
net/bluetooth/hci_event.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0437200..ce8be0b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3024,17 +3024,20 @@ unlock:
static u8 hci_get_auth_req(struct hci_conn *conn)
{
/* If remote requests dedicated bonding follow that lead */
- if (conn->remote_auth == 0x02 || conn->remote_auth == 0x03) {
+ if (conn->remote_auth == HCI_AT_DEDICATED_BONDING ||
+ conn->remote_auth == HCI_AT_DEDICATED_BONDING_MITM) {
/* If both remote and local IO capabilities allow MITM
* protection then require it, otherwise don't */
- if (conn->remote_cap == 0x03 || conn->io_capability == 0x03)
- return 0x02;
+ if (conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT ||
+ conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)
+ return HCI_AT_DEDICATED_BONDING;
else
- return 0x03;
+ return HCI_AT_DEDICATED_BONDING_MITM;
}
/* If remote requests no-bonding follow that lead */
- if (conn->remote_auth == 0x00 || conn->remote_auth == 0x01)
+ if (conn->remote_auth == HCI_AT_NO_BONDING ||
+ conn->remote_auth == HCI_AT_NO_BONDING_MITM)
return conn->remote_auth | (conn->auth_type & 0x01);
return conn->auth_type;
--
1.8.1.4
From: Mikel Astiz <[email protected]>
Replace the occurrences of integer literals in hci_event.c with the
newly introduced macros in hci.h.
Signed-off-by: Mikel Astiz <[email protected]>
---
net/bluetooth/hci_event.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ce8be0b..50e39f4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3069,7 +3069,7 @@ static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
/* Change the IO capability from KeyboardDisplay
* to DisplayYesNo as it is not supported by BT spec. */
cp.capability = (conn->io_capability == 0x04) ?
- 0x01 : conn->io_capability;
+ HCI_IO_DISPLAY_YESNO : conn->io_capability;
conn->auth_type = hci_get_auth_req(conn);
cp.authentication = conn->auth_type;
@@ -3143,7 +3143,8 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
* request. The only exception is when we're dedicated bonding
* initiators (connect_cfm_cb set) since then we always have the MITM
* bit set. */
- if (!conn->connect_cfm_cb && loc_mitm && conn->remote_cap == 0x03) {
+ if (!conn->connect_cfm_cb && loc_mitm &&
+ conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) {
BT_DBG("Rejecting request: remote device can't provide MITM");
hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
sizeof(ev->bdaddr), &ev->bdaddr);
@@ -3151,8 +3152,8 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
}
/* If no side requires MITM protection; auto-accept */
- if ((!loc_mitm || conn->remote_cap == 0x03) &&
- (!rem_mitm || conn->io_capability == 0x03)) {
+ if ((!loc_mitm || conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) &&
+ (!rem_mitm || conn->io_capability == HCI_IO_NO_INPUT_OUTPUT)) {
/* If we're not the initiators request authorization to
* proceed from user space (mgmt_user_confirm with
--
1.8.1.4
From: Mikel Astiz <[email protected]>
Do not always set the MITM protection requirement by default in the
field conn->auth_type, since this will be added later in
hci_io_capa_request_evt(), as part of the requirements specified in
HCI_OP_IO_CAPABILITY_REPLY.
This avoids a hackish exception for the auto-reject case, but doesn't
change the behavior of the code at all.
Signed-off-by: Mikel Astiz <[email protected]>
---
net/bluetooth/hci_event.c | 14 ++++++++------
net/bluetooth/mgmt.c | 5 +----
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index fe78b9c..4e5dc5b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3075,6 +3075,11 @@ static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
/* If we are initiators, there is no remote information yet */
if (conn->remote_auth == 0xff) {
cp.authentication = conn->auth_type;
+
+ /* Use MITM protection for outgoing dedicated bonding */
+ if (conn->io_capability != HCI_IO_NO_INPUT_OUTPUT &&
+ cp.authentication == HCI_AT_DEDICATED_BONDING)
+ cp.authentication |= 0x01;
} else {
conn->auth_type = hci_get_auth_req(conn);
cp.authentication = conn->auth_type;
@@ -3146,12 +3151,9 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
rem_mitm = (conn->remote_auth & 0x01);
/* If we require MITM but the remote device can't provide that
- * (it has NoInputNoOutput) then reject the confirmation
- * request. The only exception is when we're dedicated bonding
- * initiators (connect_cfm_cb set) since then we always have the MITM
- * bit set. */
- if (!conn->connect_cfm_cb && loc_mitm &&
- conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) {
+ * (it has NoInputNoOutput) then reject the confirmation request
+ */
+ if (loc_mitm && conn->remote_cap == HCI_IO_NO_INPUT_OUTPUT) {
BT_DBG("Rejecting request: remote device can't provide MITM");
hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
sizeof(ev->bdaddr), &ev->bdaddr);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fedc539..ca0ad32 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2185,10 +2185,7 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
}
sec_level = BT_SECURITY_MEDIUM;
- if (cp->io_cap == 0x03)
- auth_type = HCI_AT_DEDICATED_BONDING;
- else
- auth_type = HCI_AT_DEDICATED_BONDING_MITM;
+ auth_type = HCI_AT_DEDICATED_BONDING;
if (cp->addr.type == BDADDR_BREDR)
conn = hci_connect(hdev, ACL_LINK, &cp->addr.bdaddr,
--
1.8.1.4
From: Mikel Astiz <[email protected]>
Add macros for the HCI capabilities as described in the Bluetooth Core
Specification v4.0, Volume 2, part E, section 7.1.29.
Signed-off-by: Mikel Astiz <[email protected]>
---
include/net/bluetooth/hci.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 3c592cf..a01fbb4 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -296,6 +296,12 @@ enum {
#define HCI_AT_GENERAL_BONDING 0x04
#define HCI_AT_GENERAL_BONDING_MITM 0x05
+/* I/O capabilities */
+#define HCI_IO_DISPLAY_ONLY 0x00
+#define HCI_IO_DISPLAY_YESNO 0x01
+#define HCI_IO_KEYBOARD_ONLY 0x02
+#define HCI_IO_NO_INPUT_OUTPUT 0x03
+
/* Link Key types */
#define HCI_LK_COMBINATION 0x00
#define HCI_LK_LOCAL_UNIT 0x01
--
1.8.1.4
Hi Mikel,
* Mikel Astiz <[email protected]> [2013-06-28 10:56:29 +0200]:
> From: Mikel Astiz <[email protected]>
>
> Replace the occurrences of integer literals in hci_event.c with the
> newly introduced macros in hci.h.
>
> Signed-off-by: Mikel Astiz <[email protected]>
> ---
> net/bluetooth/hci_event.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
Patches 1 to 3 have been applied to bluetooth-next since they aren't harmful.
The other will have to wait, as Johan said. Thanks.
Gustavo
Hi Mikel,
On Mon, Jul 08, 2013, Mikel Astiz wrote:
> On Fri, Jun 28, 2013 at 1:40 PM, Mikel Astiz <[email protected]> wrote:
> > Hi,
> >
> > On Fri, Jun 28, 2013 at 10:56 AM, Mikel Astiz <[email protected]> wrote:
> >> From: Mikel Astiz <[email protected]>
> >>
> >> The way the kernel handles MITM Protection during pairing is inconsistent: General Bonding and Dedicated Bonding are not treated equally.
> >>
> >> From the user's perspective, using the MITM Protection usually means he will have to confirm the pairing in the UI (some pop-up showing the passkey). Making a difference between General and Dedicated Bonding is undesired because, in practice, the user normally doesn't care about which of them is used. Currently, if an iPhone is paired (initiated on the phone), no pop-up will be shown (because it's using General Bonding). This differs from pairing an Android device (using Dedicated Bonding), which an average user would not understand why.
> >>
> >> The GAP Specification describes when MITM Protection should be used (Bluetooth Core Specification v4.0 Volume 3, part C, section 6.5.3). It makes no distinction between General vs Dedicated Bonding: the recommendation is *not* to use it "unless the security policy of an available local service requires MITM Protection".
> >>
> >> However, the kernel doesn't necessarily have this information in a reliable way. Therefore, the safest choice is to always request MITM Protection, also for General Bonding [1]. The proposal here is to do this for both incoming (patch 6/8) and outgoing (patch 7/8) procedures, as it was previously done for Dedicated Bonding. This "conservative" approach is smart enough to fall back to not using MITM Protection if the IO capabilities don't allow it (this policy already existed before for Dedicated Bonding, see patch 5/8).
> >>
> >> Some systems might however know that MITM Protection is not required at all, because no supported profile requires high security. This can be expressed by userland using the newly introduced management command (patch 8/8). In this case, the recommendation in the spec will be followed (affecting both General and Dedicated Bonding).
> >>
> >> Note that the first 5 patches are refactoring patches which shouldn't change the behavior of the code. Within this group, patch 5/8 is the most tricky one since side effects could exist (we weren't able to observed them though).
> >>
> >> [1] We make an exception here for No-Bonding, which remains unmodified. In this case, no MITM Protection is required by default since an additional pop-up would be undesireable for most use-cases.
> >>
> >> Mikel Astiz (6):
> >> Bluetooth: Add HCI authentication capabilities macros
> >> Bluetooth: Use defines in in hci_get_auth_req()
> >> Bluetooth: Use defines instead of integer literals
> >> Bluetooth: Refactor hci_get_auth_req()
> >> Bluetooth: Refactor code for outgoing dedicated bonding
> >> Bluetooth: Request MITM Protection when initiator
> >>
> >> Timo Mueller (2):
> >> Bluetooth: Use MITM Protection when IO caps allow it
> >> Bluetooth: Add management command to relax MITM Protection
> >>
> >> include/net/bluetooth/hci.h | 9 ++++++-
> >> include/net/bluetooth/mgmt.h | 3 +++
> >> net/bluetooth/hci_event.c | 63 ++++++++++++++++++++++++++++----------------
> >> net/bluetooth/mgmt.c | 53 +++++++++++++++++++++++++++++++++----
> >> 4 files changed, 100 insertions(+), 28 deletions(-)
> >
> > For the record, I have no particular interest in the last two patches.
> > They're submitted here for completion purposes as a base for the
> > discussion.
> >
> > Cheers,
> > Mikel
>
> Ping.
I don't see anything obviously wrong with this set and probably the
first three or four patches could be safely applied. However for the
actual changes to the pairing logic I wouldn't be comfortable in giving
acks for them before at least some testing with the BITE as well as at
an UnplugFest.
Johan
On Fri, Jun 28, 2013 at 1:40 PM, Mikel Astiz <[email protected]> wrote:
> Hi,
>
> On Fri, Jun 28, 2013 at 10:56 AM, Mikel Astiz <[email protected]> wrote:
>> From: Mikel Astiz <[email protected]>
>>
>> The way the kernel handles MITM Protection during pairing is inconsistent: General Bonding and Dedicated Bonding are not treated equally.
>>
>> From the user's perspective, using the MITM Protection usually means he will have to confirm the pairing in the UI (some pop-up showing the passkey). Making a difference between General and Dedicated Bonding is undesired because, in practice, the user normally doesn't care about which of them is used. Currently, if an iPhone is paired (initiated on the phone), no pop-up will be shown (because it's using General Bonding). This differs from pairing an Android device (using Dedicated Bonding), which an average user would not understand why.
>>
>> The GAP Specification describes when MITM Protection should be used (Bluetooth Core Specification v4.0 Volume 3, part C, section 6.5.3). It makes no distinction between General vs Dedicated Bonding: the recommendation is *not* to use it "unless the security policy of an available local service requires MITM Protection".
>>
>> However, the kernel doesn't necessarily have this information in a reliable way. Therefore, the safest choice is to always request MITM Protection, also for General Bonding [1]. The proposal here is to do this for both incoming (patch 6/8) and outgoing (patch 7/8) procedures, as it was previously done for Dedicated Bonding. This "conservative" approach is smart enough to fall back to not using MITM Protection if the IO capabilities don't allow it (this policy already existed before for Dedicated Bonding, see patch 5/8).
>>
>> Some systems might however know that MITM Protection is not required at all, because no supported profile requires high security. This can be expressed by userland using the newly introduced management command (patch 8/8). In this case, the recommendation in the spec will be followed (affecting both General and Dedicated Bonding).
>>
>> Note that the first 5 patches are refactoring patches which shouldn't change the behavior of the code. Within this group, patch 5/8 is the most tricky one since side effects could exist (we weren't able to observed them though).
>>
>> [1] We make an exception here for No-Bonding, which remains unmodified. In this case, no MITM Protection is required by default since an additional pop-up would be undesireable for most use-cases.
>>
>> Mikel Astiz (6):
>> Bluetooth: Add HCI authentication capabilities macros
>> Bluetooth: Use defines in in hci_get_auth_req()
>> Bluetooth: Use defines instead of integer literals
>> Bluetooth: Refactor hci_get_auth_req()
>> Bluetooth: Refactor code for outgoing dedicated bonding
>> Bluetooth: Request MITM Protection when initiator
>>
>> Timo Mueller (2):
>> Bluetooth: Use MITM Protection when IO caps allow it
>> Bluetooth: Add management command to relax MITM Protection
>>
>> include/net/bluetooth/hci.h | 9 ++++++-
>> include/net/bluetooth/mgmt.h | 3 +++
>> net/bluetooth/hci_event.c | 63 ++++++++++++++++++++++++++++----------------
>> net/bluetooth/mgmt.c | 53 +++++++++++++++++++++++++++++++++----
>> 4 files changed, 100 insertions(+), 28 deletions(-)
>
> For the record, I have no particular interest in the last two patches.
> They're submitted here for completion purposes as a base for the
> discussion.
>
> Cheers,
> Mikel
Ping.
Cheers,
Mikel