[PATCH 0/3] Bluetooth: SMP fixes

[PATCH 0/3] Bluetooth: SMP fixes

[johan.hedberg]

Hi,

Here's a set of patches I ended up creating when testing HoG
reconnections. Now that we react to any direct advertising report when
connectable it's particularly important that we have the user
interaction semantics for incoming connections right.

Strictly speaking the Core spec simply tells us to use "just-works" when
no side has MITM required, so these changes should be checked against
current PTS and Android CTS tests. That said, imo this is still doing a
just-works pairing but simply with an extra user consent for proceeding
with the pairing for incoming requests.

Johan

----------------------------------------------------------------
Johan Hedberg (3):
      Bluetooth: Always confirm incoming SMP just-works requests
      Bluetooth: Fix forcing SMP just-works with no-bonding
      Bluetooth: Fix incorrect clearing of SMP_FLAG_INITIATOR

 net/bluetooth/smp.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

[PATCH 1/3] Bluetooth: Always confirm incoming SMP just-works requests

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

For incoming requests we want to let the user know that pairing is
happening since otherwise there could be access to MEDIUM security
services without any user interaction at all. Therefore, set the
selected method to JUST_CFM instead of JUST_WORKS and let it be
converted back to JUST_WORKS later if we are the initators.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/smp.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 55c41de2f5a0..a17761c83820 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -391,10 +391,12 @@ static const u8 gen_method[5][5] = {
 
 static u8 get_auth_method(struct smp_chan *smp, u8 local_io, u8 remote_io)
 {
-	/* If either side has unknown io_caps, use JUST WORKS */
+	/* If either side has unknown io_caps, use JUST_CFM (which gets
+	 * converted later to JUST_WORKS if we're initiators.
+	 */
 	if (local_io > SMP_IO_KEYBOARD_DISPLAY ||
 	    remote_io > SMP_IO_KEYBOARD_DISPLAY)
-		return JUST_WORKS;
+		return JUST_CFM;
 
 	return gen_method[remote_io][local_io];
 }
@@ -414,10 +416,14 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
 
 	BT_DBG("tk_request: auth:%d lcl:%d rem:%d", auth, local_io, remote_io);
 
-	/* If neither side wants MITM, use JUST WORKS */
-	/* Otherwise, look up method from the table */
+	/* If neither side wants MITM, either "just" confirm an incoming
+	 * request or use just-works for outgoing ones. The JUST_CFM
+	 * will be converted to JUST_WORKS if necessary later in this
+	 * function. If either side has MITM look up the method from the
+	 * table.
+	 */
 	if (!(auth & SMP_AUTH_MITM))
-		method = JUST_WORKS;
+		method = JUST_CFM;
 	else
 		method = get_auth_method(smp, local_io, remote_io);
 
-- 
1.9.3

[PATCH 2/3] Bluetooth: Fix forcing SMP just-works with no-bonding

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

Whether we bond or not should not have any impact on the user
interaction model. This patch removes an incorrect fall-back from
JUST_CFM to JUST_WORKS in case we're not bonding.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/smp.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index a17761c83820..a5e51c686469 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -427,10 +427,6 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
 	else
 		method = get_auth_method(smp, local_io, remote_io);
 
-	/* If not bonding, don't ask user to confirm a Zero TK */
-	if (!(auth & SMP_AUTH_BONDING) && method == JUST_CFM)
-		method = JUST_WORKS;
-
 	/* Don't confirm locally initiated pairing attempts */
 	if (method == JUST_CFM && test_bit(SMP_FLAG_INITIATOR, &smp->flags))
 		method = JUST_WORKS;
-- 
1.9.3

[PATCH 3/3] Bluetooth: Fix incorrect clearing of SMP_FLAG_INITIATOR

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

When the SMP context is created all flags default to zero. To determine
that we are the initiators it's therefore best to simply change the flag
value when we know we're sending the first SMP PDU. Clearing the flag
when receiving a Pairing Request is not correct since the request may be
a response to a previous Security Request from us (for which we would
already have correctly set the flag). Same goes for receiving a Security
Request which may be coming after us already starting pairing by sending
a Pairing Request.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/smp.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index a5e51c686469..627d683203cf 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -735,8 +735,6 @@ static u8 smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
 	if (ret)
 		return SMP_UNSPECIFIED;
 
-	clear_bit(SMP_FLAG_INITIATOR, &smp->flags);
-
 	return 0;
 }
 
@@ -927,8 +925,6 @@ static u8 smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
 
 	smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
 
-	clear_bit(SMP_FLAG_INITIATOR, &smp->flags);
-
 	return 0;
 }
 
-- 
1.9.3

Re: [PATCH 0/3] Bluetooth: SMP fixes

[Marcel Holtmann]

Hi Johan,

> Here's a set of patches I ended up creating when testing HoG
> reconnections. Now that we react to any direct advertising report when
> connectable it's particularly important that we have the user
> interaction semantics for incoming connections right.
> 
> Strictly speaking the Core spec simply tells us to use "just-works" when
> no side has MITM required, so these changes should be checked against
> current PTS and Android CTS tests. That said, imo this is still doing a
> just-works pairing but simply with an extra user consent for proceeding
> with the pairing for incoming requests.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (3):
>      Bluetooth: Always confirm incoming SMP just-works requests
>      Bluetooth: Fix forcing SMP just-works with no-bonding
>      Bluetooth: Fix incorrect clearing of SMP_FLAG_INITIATOR
> 
> net/bluetooth/smp.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)

all 3 patches have been applied to bluetooth-next tree.

Regards

Marcel