[RFCv5 0/4] SSP MITM protection

[RFCv5 0/4] SSP MITM protection

[Timo Mueller]

From: Timo Mueller <timo.mueller@bmw-carit.de>

Hi,

this is an updated version of v4 with the following changes:

1) The management option to relax the MITM Protection was dropped
2) Some minor modifications to the commit messages

Test information from the v4 cover letter:
<snip>
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).
<snip>

>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>

For completeness, a quick recap on the motivation behind this patch
set:
In our cars we always want to use SSP methods other than Just
Works. We even go as far as rejecting JW connection attempts. In order
to still be able to pair all devices that are capable of using any of
the remaining SSP methods, we need to make sure that we're not falling
back to JW, regardless of the bonding type (Dedicated or General). For
example, currently a connection with the iPhone acting as the
initiating LM would lead a JW connection which would get rejected.

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 (1):
  Bluetooth: Use MITM Protection when IO caps allow it

 net/bluetooth/hci_event.c | 48 ++++++++++++++++++++++++++---------------------
 net/bluetooth/mgmt.c      |  5 +----
 2 files changed, 28 insertions(+), 25 deletions(-)

-- 
1.9.0

[RFCv5 1/4] Bluetooth: Refactor hci_get_auth_req()

[Timo Mueller]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Refactor the code without changing its behavior by handling the
no-bonding cases first followed by General Bonding.

Signed-off-by: Mikel Astiz <mikel.astiz@bmw-carit.de>
Signed-off-by: Timo Mueller <timo.mueller@bmw-carit.de>
---
 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 84acc4a..0801668 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3419,24 +3419,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)
@@ -3466,8 +3467,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.9.0

[RFCv5 2/4] Bluetooth: Refactor code for outgoing dedicated bonding

[Timo Mueller]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

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 <mikel.astiz@bmw-carit.de>
---
 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 0801668..2c09732 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3471,6 +3471,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;
@@ -3542,12 +3547,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 11cb00a..54abbce 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2850,10 +2850,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_acl(hdev, &cp->addr.bdaddr, sec_level,
-- 
1.9.0

[RFCv5 3/4] Bluetooth: Use MITM Protection when IO caps allow it

[Timo Mueller]

From: Timo Mueller <timo.mueller@bmw-carit.de>

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, as we are
already requiring MITM for Dedicated Bonding, we will follow this
behaviour also for General Bonding.

Signed-off-by: Timo Mueller <timo.mueller@bmw-carit.de>
Signed-off-by: Mikel Astiz <mikel.astiz@bmw-carit.de>
---
 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 2c09732..8f76e35 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3424,11 +3424,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
 	 */
@@ -3436,8 +3431,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.9.0

[RFCv5 4/4] Bluetooth: Request MITM Protection when initiator

[Timo Mueller]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

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 <mikel.astiz@bmw-carit.de>
Signed-off-by: Timo Mueller <timo.mueller@bmw-carit.de>
---
 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 8f76e35..07c37d0 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3467,9 +3467,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.9.0

Re: [RFCv5 0/4] SSP MITM protection

[Johan Hedberg]

Hi Timo,

On Tue, Apr 08, 2014, Timo Mueller wrote:
> this is an updated version of v4 with the following changes:
> 
> 1) The management option to relax the MITM Protection was dropped
> 2) Some minor modifications to the commit messages
> 
> Test information from the v4 cover letter:
> <snip>
> 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).
> <snip>
> 
> 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>
> 
> For completeness, a quick recap on the motivation behind this patch
> set:
> In our cars we always want to use SSP methods other than Just
> Works. We even go as far as rejecting JW connection attempts. In order
> to still be able to pair all devices that are capable of using any of
> the remaining SSP methods, we need to make sure that we're not falling
> back to JW, regardless of the bonding type (Dedicated or General). For
> example, currently a connection with the iPhone acting as the
> initiating LM would lead a JW connection which would get rejected.
> 
> 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 (1):
>   Bluetooth: Use MITM Protection when IO caps allow it
> 
>  net/bluetooth/hci_event.c | 48 ++++++++++++++++++++++++++---------------------
>  net/bluetooth/mgmt.c      |  5 +----
>  2 files changed, 28 insertions(+), 25 deletions(-)

I ran the BITE SSP tests on these (test vector 1.0.51) and they seem to
be fine. The patches have now been applied to bluetooth-next.

Johan