From: Timo Mueller <[email protected]>
Hi,
this is a rebased version of the rfc v3. I've successfully tested
these changes in the last couple of months at the UPF #46 in Vienna,
with the CE4A golden device and the bluetooth PTS (where applicable).
At the UPF I've tested remotely initiated pairing with different io
capabilities, as well locally initiated pairing. Regardless of the
bonding mode, the protocol chosen in ssp has been consistent when
being responder and also when being initiator. Pairing tests have been
successful in all 22 test sessions.
The configuration I used for testing was as follows:
bluez: 5.9-154-gf7773c7
kernel: v3.12-rc3-65-gf927318
with the remaining patches from [RFC BlueZ v3 0/8] SSP MITM protection
I used the same configuration to test the patches with the CE4A golden
device. Pairing here has been working as expected with all
combinations of io capabilities, bonding mode and intiator role.
Lastly I've successfully ran the applicable GAP tests with the
bluetooth PTS on this rebased version and the current head of
bluez. Unfortunately the interesting bonding test cases are not yet
implemented with the test suite. So I could only make sure general
functionality is preserved.
from the original cover letter:
The way the kernel handles MITM Protection during pairing is
inconsistent: General Bonding and Dedicated Bonding are not treated
equally.
<snip>
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).
<snip>
Best regards
Timo
Mikel Astiz (3):
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 | 3 ++-
include/net/bluetooth/mgmt.h | 3 +++
net/bluetooth/hci_event.c | 57 ++++++++++++++++++++++++++++----------------
net/bluetooth/mgmt.c | 50 ++++++++++++++++++++++++++++++++++----
4 files changed, 87 insertions(+), 26 deletions(-)
--
1.8.3.1
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 6f9c425..1cbec8f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3089,6 +3089,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;
@@ -3160,12 +3165,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 a03ca3c..8e302f4 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2692,10 +2692,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.3.1
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 5f81245..6f9c425 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3037,24 +3037,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)
@@ -3084,8 +3085,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.3.1
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 882e569..4516215 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3085,9 +3085,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.3.1
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]>
---
include/net/bluetooth/hci.h | 3 ++-
include/net/bluetooth/mgmt.h | 3 +++
net/bluetooth/hci_event.c | 15 ++++++++++++---
net/bluetooth/mgmt.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5dc3d90..147fac6 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -121,6 +121,7 @@ enum {
HCI_LE_SCAN,
HCI_SSP_ENABLED,
+ HCI_RELAX_MITM,
HCI_HS_ENABLED,
HCI_LE_ENABLED,
HCI_ADVERTISING,
@@ -138,7 +139,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 518c5c8..2da018b 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -94,6 +94,7 @@ struct mgmt_rp_read_index_list {
#define MGMT_SETTING_HS 0x00000100
#define MGMT_SETTING_LE 0x00000200
#define MGMT_SETTING_ADVERTISING 0x00000400
+#define MGMT_SETTING_RELAX_MITM 0x00000800
#define MGMT_OP_READ_INFO 0x0004
#define MGMT_READ_INFO_SIZE 0
@@ -369,6 +370,8 @@ struct mgmt_cp_set_scan_params {
} __packed;
#define MGMT_SET_SCAN_PARAMS_SIZE 4
+#define MGMT_OP_SET_RELAX_MITM 0x002D
+
#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 4516215..d23370b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3042,6 +3042,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
*/
@@ -3085,11 +3091,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 8e302f4..2aca565 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -79,6 +79,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_SET_BREDR,
MGMT_OP_SET_STATIC_ADDRESS,
MGMT_OP_SET_SCAN_PARAMS,
+ MGMT_OP_SET_RELAX_MITM,
};
static const u16 mgmt_events[] = {
@@ -375,6 +376,7 @@ static u32 get_supported_settings(struct hci_dev *hdev)
if (lmp_ssp_capable(hdev)) {
settings |= MGMT_SETTING_SSP;
settings |= MGMT_SETTING_HS;
+ settings |= MGMT_SETTING_RELAX_MITM;
}
}
@@ -423,6 +425,9 @@ static u32 get_current_settings(struct hci_dev *hdev)
if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
settings |= MGMT_SETTING_ADVERTISING;
+ if (test_bit(HCI_RELAX_MITM, &hdev->dev_flags))
+ settings |= MGMT_SETTING_RELAX_MITM;
+
return settings;
}
@@ -1719,6 +1724,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;
@@ -4124,6 +4168,7 @@ static const struct mgmt_handler {
{ set_bredr, false, MGMT_SETTING_SIZE },
{ set_static_address, false, MGMT_SET_STATIC_ADDRESS_SIZE },
{ set_scan_params, false, MGMT_SET_SCAN_PARAMS_SIZE },
+ { set_relax_mitm, false, MGMT_SETTING_SIZE },
};
--
1.8.3.1
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 1cbec8f..882e569 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3042,11 +3042,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
*/
@@ -3054,8 +3049,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.3.1
Timo Mueller wrote, On 19.12.2013 14:08:
> From: Timo Mueller <[email protected]>
>
> Hi,
>
> this is a rebased version of the rfc v3. I've successfully tested
> these changes in the last couple of months at the UPF #46 in Vienna,
> with the CE4A golden device and the bluetooth PTS (where applicable).
>
> At the UPF I've tested remotely initiated pairing with different io
> capabilities, as well locally initiated pairing. Regardless of the
> bonding mode, the protocol chosen in ssp has been consistent when
> being responder and also when being initiator. Pairing tests have been
> successful in all 22 test sessions.
>
> The configuration I used for testing was as follows:
> bluez: 5.9-154-gf7773c7
> kernel: v3.12-rc3-65-gf927318
> with the remaining patches from [RFC BlueZ v3 0/8] SSP MITM protection
>
> I used the same configuration to test the patches with the CE4A golden
> device. Pairing here has been working as expected with all
> combinations of io capabilities, bonding mode and intiator role.
>
> Lastly I've successfully ran the applicable GAP tests with the
> bluetooth PTS on this rebased version and the current head of
> bluez. Unfortunately the interesting bonding test cases are not yet
> implemented with the test suite. So I could only make sure general
> functionality is preserved.
>
> from the original cover letter:
> The way the kernel handles MITM Protection during pairing is
> inconsistent: General Bonding and Dedicated Bonding are not treated
> equally.
> <snip>
> 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).
> <snip>
>
> Best regards
> Timo
>
> Mikel Astiz (3):
> 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 | 3 ++-
> include/net/bluetooth/mgmt.h | 3 +++
> net/bluetooth/hci_event.c | 57 ++++++++++++++++++++++++++++----------------
> net/bluetooth/mgmt.c | 50 ++++++++++++++++++++++++++++++++++----
> 4 files changed, 87 insertions(+), 26 deletions(-)
>
Ping