stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
@ 2020-10-15 21:12 Hans-Christian Noren Egtvedt
  2020-10-15 21:12 ` [v4.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm` Hans-Christian Noren Egtvedt
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Hans-Christian Noren Egtvedt @ 2020-10-15 21:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Luiz Augusto von Dentz, Marcel Holtmann, stable

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes hci_encrypt_cfm calls hci_connect_cfm in case the connection
state is BT_CONFIG so callers don't have to check the state.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
(cherry picked from commit 3ca44c16b0dcc764b641ee4ac226909f5c421aa3)
Cc: stable@vger.kernel.org # 4.4
---
 include/net/bluetooth/hci_core.h | 20 ++++++++++++++++++--
 net/bluetooth/hci_event.c        | 28 +++-------------------------
 2 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 7c0c83dfe86e..0269a772bfe1 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1235,10 +1235,26 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
 		conn->security_cfm_cb(conn, status);
 }
 
-static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status,
-								__u8 encrypt)
+static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
 {
 	struct hci_cb *cb;
+	__u8 encrypt;
+
+	if (conn->state == BT_CONFIG) {
+		if (status)
+			conn->state = BT_CONNECTED;
+
+		hci_connect_cfm(conn, status);
+		hci_conn_drop(conn);
+		return;
+	}
+
+	if (!test_bit(HCI_CONN_ENCRYPT, &conn->flags))
+		encrypt = 0x00;
+	else if (test_bit(HCI_CONN_AES_CCM, &conn->flags))
+		encrypt = 0x02;
+	else
+		encrypt = 0x01;
 
 	if (conn->sec_level == BT_SECURITY_SDP)
 		conn->sec_level = BT_SECURITY_LOW;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 03319ab8a7c6..bb9c13506bca 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2479,7 +2479,7 @@ static void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 				     &cp);
 		} else {
 			clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
-			hci_encrypt_cfm(conn, ev->status, 0x00);
+			hci_encrypt_cfm(conn, ev->status);
 		}
 	}
 
@@ -2565,22 +2565,7 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status,
 		conn->enc_key_size = rp->key_size;
 	}
 
-	if (conn->state == BT_CONFIG) {
-		conn->state = BT_CONNECTED;
-		hci_connect_cfm(conn, 0);
-		hci_conn_drop(conn);
-	} else {
-		u8 encrypt;
-
-		if (!test_bit(HCI_CONN_ENCRYPT, &conn->flags))
-			encrypt = 0x00;
-		else if (test_bit(HCI_CONN_AES_CCM, &conn->flags))
-			encrypt = 0x02;
-		else
-			encrypt = 0x01;
-
-		hci_encrypt_cfm(conn, 0, encrypt);
-	}
+	hci_encrypt_cfm(conn, 0);
 
 unlock:
 	hci_dev_unlock(hdev);
@@ -2674,14 +2659,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	}
 
 notify:
-	if (conn->state == BT_CONFIG) {
-		if (!ev->status)
-			conn->state = BT_CONNECTED;
-
-		hci_connect_cfm(conn, ev->status);
-		hci_conn_drop(conn);
-	} else
-		hci_encrypt_cfm(conn, ev->status, ev->encrypt);
+	hci_encrypt_cfm(conn, ev->status);
 
 unlock:
 	hci_dev_unlock(hdev);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [v4.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm`
  2020-10-15 21:12 [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Hans-Christian Noren Egtvedt
@ 2020-10-15 21:12 ` Hans-Christian Noren Egtvedt
  2020-10-15 21:12 ` [v4.4/bluetooth PATCH 3/3] Bluetooth: Disconnect if E0 is used for Level 4 Hans-Christian Noren Egtvedt
  2020-10-16  7:32 ` [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Greg KH
  2 siblings, 0 replies; 6+ messages in thread
From: Hans-Christian Noren Egtvedt @ 2020-10-15 21:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, Patrick Steinhardt, Luiz Augusto von Dentz,
	Marcel Holtmann, stable

From: Patrick Steinhardt <ps@pks.im>

Starting with the upgrade to v5.8-rc3, I've noticed I wasn't able to
connect to my Bluetooth headset properly anymore. While connecting to
the device would eventually succeed, bluetoothd seemed to be confused
about the current connection state where the state was flapping hence
and forth. Bisecting this issue led to commit 3ca44c16b0dc (Bluetooth:
Consolidate encryption handling in hci_encrypt_cfm, 2020-05-19), which
refactored `hci_encrypt_cfm` to also handle updating the connection
state.

The commit in question changed the code to call `hci_connect_cfm` inside
`hci_encrypt_cfm` and to change the connection state. But with the
conversion, we now only update the connection state if a status was set
already. In fact, the reverse should be true: the status should be
updated if no status is yet set. So let's fix the isuse by reversing the
condition.

Fixes: 3ca44c16b0dc ("Bluetooth: Consolidate encryption handling in hci_encrypt_cfm")
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Acked-by:  Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
(cherry picked from commit 339ddaa626995bc6218972ca241471f3717cc5f4)
Cc: stable@vger.kernel.org # 4.4
---
 include/net/bluetooth/hci_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0269a772bfe1..dfa672b6f89d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1241,7 +1241,7 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
 	__u8 encrypt;
 
 	if (conn->state == BT_CONFIG) {
-		if (status)
+		if (!status)
 			conn->state = BT_CONNECTED;
 
 		hci_connect_cfm(conn, status);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [v4.4/bluetooth PATCH 3/3] Bluetooth: Disconnect if E0 is used for Level 4
  2020-10-15 21:12 [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Hans-Christian Noren Egtvedt
  2020-10-15 21:12 ` [v4.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm` Hans-Christian Noren Egtvedt
@ 2020-10-15 21:12 ` Hans-Christian Noren Egtvedt
  2020-10-16  7:32 ` [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Greg KH
  2 siblings, 0 replies; 6+ messages in thread
From: Hans-Christian Noren Egtvedt @ 2020-10-15 21:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Luiz Augusto von Dentz, Marcel Holtmann, stable

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

E0 is not allowed with Level 4:

BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319:

  '128-bit equivalent strength for link and encryption keys
   required using FIPS approved algorithms (E0 not allowed,
   SAFER+ not allowed, and P-192 not allowed; encryption key
   not shortened'

SC enabled:

> HCI Event: Read Remote Extended Features (0x23) plen 13
        Status: Success (0x00)
        Handle: 256
        Page: 1/2
        Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00
          Secure Simple Pairing (Host Support)
          LE Supported (Host)
          Secure Connections (Host Support)
> HCI Event: Encryption Change (0x08) plen 4
        Status: Success (0x00)
        Handle: 256
        Encryption: Enabled with AES-CCM (0x02)

SC disabled:

> HCI Event: Read Remote Extended Features (0x23) plen 13
        Status: Success (0x00)
        Handle: 256
        Page: 1/2
        Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00
          Secure Simple Pairing (Host Support)
          LE Supported (Host)
> HCI Event: Encryption Change (0x08) plen 4
        Status: Success (0x00)
        Handle: 256
        Encryption: Enabled with E0 (0x01)
[May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used
< HCI Command: Disconnect (0x01|0x0006) plen 3
        Handle: 256
        Reason: Authentication Failure (0x05)

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
(cherry picked from commit 8746f135bb01872ff412d408ea1aa9ebd328c1f5,
adjusted to match linux-4.4.y sources.)
Cc: stable@vger.kernel.org # 4.4
---
 include/net/bluetooth/hci_core.h | 10 ++++++----
 net/bluetooth/hci_conn.c         | 17 +++++++++++++++++
 net/bluetooth/hci_event.c        | 20 ++++++++------------
 3 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index dfa672b6f89d..5aaf6cdb121a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1256,11 +1256,13 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
 	else
 		encrypt = 0x01;
 
-	if (conn->sec_level == BT_SECURITY_SDP)
-		conn->sec_level = BT_SECURITY_LOW;
+	if (!status) {
+		if (conn->sec_level == BT_SECURITY_SDP)
+			conn->sec_level = BT_SECURITY_LOW;
 
-	if (conn->pending_sec_level > conn->sec_level)
-		conn->sec_level = conn->pending_sec_level;
+		if (conn->pending_sec_level > conn->sec_level)
+			conn->sec_level = conn->pending_sec_level;
+	}
 
 	mutex_lock(&hci_cb_list_lock);
 	list_for_each_entry(cb, &hci_cb_list, list) {
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 114bcf6ea916..2c94e3cd3545 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1173,6 +1173,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn)
 			return 0;
 	}
 
+	 /* AES encryption is required for Level 4:
+	  *
+	  * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C
+	  * page 1319:
+	  *
+	  * 128-bit equivalent strength for link and encryption keys
+	  * required using FIPS approved algorithms (E0 not allowed,
+	  * SAFER+ not allowed, and P-192 not allowed; encryption key
+	  * not shortened)
+	  */
+	if (conn->sec_level == BT_SECURITY_FIPS &&
+	    !test_bit(HCI_CONN_AES_CCM, &conn->flags)) {
+		bt_dev_err(conn->hdev,
+			   "Invalid security: Missing AES-CCM usage");
+		return 0;
+	}
+
 	if (hci_conn_ssp_enabled(conn) &&
 	    !test_bit(HCI_CONN_ENCRYPT, &conn->flags))
 		return 0;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index bb9c13506bca..f0e6cce921d8 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2612,24 +2612,20 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 	clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
 
+	/* Check link security requirements are met */
+	if (!hci_conn_check_link_mode(conn))
+		ev->status = HCI_ERROR_AUTH_FAILURE;
+
 	if (ev->status && conn->state == BT_CONNECTED) {
+		/* Notify upper layers so they can cleanup before
+		 * disconnecting.
+		 */
+		hci_encrypt_cfm(conn, ev->status);
 		hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
 		hci_conn_drop(conn);
 		goto unlock;
 	}
 
-	/* In Secure Connections Only mode, do not allow any connections
-	 * that are not encrypted with AES-CCM using a P-256 authenticated
-	 * combination key.
-	 */
-	if (hci_dev_test_flag(hdev, HCI_SC_ONLY) &&
-	    (!test_bit(HCI_CONN_AES_CCM, &conn->flags) ||
-	     conn->key_type != HCI_LK_AUTH_COMBINATION_P256)) {
-		hci_connect_cfm(conn, HCI_ERROR_AUTH_FAILURE);
-		hci_conn_drop(conn);
-		goto unlock;
-	}
-
 	/* Try reading the encryption key size for encrypted ACL links */
 	if (!ev->status && ev->encrypt && conn->type == ACL_LINK) {
 		struct hci_cp_read_enc_key_size cp;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
  2020-10-15 21:12 [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Hans-Christian Noren Egtvedt
  2020-10-15 21:12 ` [v4.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm` Hans-Christian Noren Egtvedt
  2020-10-15 21:12 ` [v4.4/bluetooth PATCH 3/3] Bluetooth: Disconnect if E0 is used for Level 4 Hans-Christian Noren Egtvedt
@ 2020-10-16  7:32 ` Greg KH
  2020-10-16  7:46   ` Hans-Christian Egtvedt (hegtvedt)
  2 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2020-10-16  7:32 UTC (permalink / raw)
  To: Hans-Christian Noren Egtvedt
  Cc: linux-kernel, Luiz Augusto von Dentz, Marcel Holtmann, stable

On Thu, Oct 15, 2020 at 11:12:23PM +0200, Hans-Christian Noren Egtvedt wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This makes hci_encrypt_cfm calls hci_connect_cfm in case the connection
> state is BT_CONFIG so callers don't have to check the state.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> (cherry picked from commit 3ca44c16b0dcc764b641ee4ac226909f5c421aa3)
> Cc: stable@vger.kernel.org # 4.4
> ---
>  include/net/bluetooth/hci_core.h | 20 ++++++++++++++++++--
>  net/bluetooth/hci_event.c        | 28 +++-------------------------
>  2 files changed, 21 insertions(+), 27 deletions(-)

What differs here from the other patch series you sent?  Looks the same
to me...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
  2020-10-16  7:32 ` [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Greg KH
@ 2020-10-16  7:46   ` Hans-Christian Egtvedt (hegtvedt)
  2020-10-16  7:54     ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Hans-Christian Egtvedt (hegtvedt) @ 2020-10-16  7:46 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, Luiz Augusto von Dentz, Marcel Holtmann, stable

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

On 16/10/2020 09:32, Greg KH wrote:
> On Thu, Oct 15, 2020 at 11:12:23PM +0200, Hans-Christian Noren Egtvedt wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This makes hci_encrypt_cfm calls hci_connect_cfm in case the connection
>> state is BT_CONFIG so callers don't have to check the state.
>>
>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>> (cherry picked from commit 3ca44c16b0dcc764b641ee4ac226909f5c421aa3)
>> Cc: stable@vger.kernel.org # 4.4
>> ---
>>  include/net/bluetooth/hci_core.h | 20 ++++++++++++++++++--
>>  net/bluetooth/hci_event.c        | 28 +++-------------------------
>>  2 files changed, 21 insertions(+), 27 deletions(-)
> 
> What differs here from the other patch series you sent?  Looks the same
> to me...

Patch 1 and 2 in this series is identical, patch 3/3 is adjusted to
resolve a conflict. Sorry I did not make that clearer.

-- 
Best regards, Hans-Christian Noren Egtvedt

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1813 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
  2020-10-16  7:46   ` Hans-Christian Egtvedt (hegtvedt)
@ 2020-10-16  7:54     ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2020-10-16  7:54 UTC (permalink / raw)
  To: Hans-Christian Egtvedt (hegtvedt)
  Cc: linux-kernel, Luiz Augusto von Dentz, Marcel Holtmann, stable

On Fri, Oct 16, 2020 at 07:46:39AM +0000, Hans-Christian Egtvedt (hegtvedt) wrote:
> On 16/10/2020 09:32, Greg KH wrote:
> > On Thu, Oct 15, 2020 at 11:12:23PM +0200, Hans-Christian Noren Egtvedt wrote:
> >> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>
> >> This makes hci_encrypt_cfm calls hci_connect_cfm in case the connection
> >> state is BT_CONFIG so callers don't have to check the state.
> >>
> >> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> >> (cherry picked from commit 3ca44c16b0dcc764b641ee4ac226909f5c421aa3)
> >> Cc: stable@vger.kernel.org # 4.4
> >> ---
> >>  include/net/bluetooth/hci_core.h | 20 ++++++++++++++++++--
> >>  net/bluetooth/hci_event.c        | 28 +++-------------------------
> >>  2 files changed, 21 insertions(+), 27 deletions(-)
> > 
> > What differs here from the other patch series you sent?  Looks the same
> > to me...
> 
> Patch 1 and 2 in this series is identical, patch 3/3 is adjusted to
> resolve a conflict. Sorry I did not make that clearer.

No problem, thanks for confirming this, that makes me feel better that I
didn't miss anything.

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-16  7:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15 21:12 [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Hans-Christian Noren Egtvedt
2020-10-15 21:12 ` [v4.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm` Hans-Christian Noren Egtvedt
2020-10-15 21:12 ` [v4.4/bluetooth PATCH 3/3] Bluetooth: Disconnect if E0 is used for Level 4 Hans-Christian Noren Egtvedt
2020-10-16  7:32 ` [v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm Greg KH
2020-10-16  7:46   ` Hans-Christian Egtvedt (hegtvedt)
2020-10-16  7:54     ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).