[PATCH 0/3] Handle link key type and security requirements

[PATCH 0/3] Handle link key type and security requirements

[Waldemar Rymarkiewicz]

Hi,

Please put some comments on these patches. I will test them in next days.

There will be further patches to support 16 digit pin notification to userspace 
and rfcomm re-authentication.

Waldek


Waldemar Rymarkiewicz (3):
  Bluetooth: Add definitions for link key types
  Bluetooth: Map sec_level to link key requirements
  Bluetooth: Ignore key unauthenticated for high security

 include/net/bluetooth/hci.h      |    9 +++++
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_conn.c         |   61 +++++++++++++++++++++++++++++++------
 net/bluetooth/hci_core.c         |    2 +-
 net/bluetooth/hci_event.c        |   28 ++++++++++++++---
 5 files changed, 85 insertions(+), 16 deletions(-)

[PATCH 1/3] Bluetooth: Add definitions for link key types

[Waldemar Rymarkiewicz]

Introduce the link key types defs and use them instead of magic numbers.

Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
---
 include/net/bluetooth/hci.h |    9 +++++++++
 net/bluetooth/hci_core.c    |    2 +-
 net/bluetooth/hci_event.c   |    7 ++++---
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 6138e31..e0a3cf1 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
 #define HCI_AT_GENERAL_BONDING		0x04
 #define HCI_AT_GENERAL_BONDING_MITM	0x05
 
+/* Link Key types */
+#define HCI_LK_COMBINATION		0x00
+#define HCI_LK_LOCAL_UNIT		0x01
+#define HCI_LK_REMOTE_UNIT		0x02
+#define HCI_LK_DEBUG_COMBINATION	0x03
+#define HCI_LK_UNAUTH_COMBINATION	0x04
+#define HCI_LK_AUTH_COMBINATION		0x05
+#define HCI_LK_CHANGED_COMBINATION	0x06
+
 /* -----  HCI Commands ---- */
 #define HCI_OP_NOP			0x0000
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a80bc1c..cfa5621 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1049,7 +1049,7 @@ int hci_add_link_key(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
 	if (new_key)
 		mgmt_new_key(hdev->id, key, old_key_type);
 
-	if (type == 0x06)
+	if (type == HCI_LK_CHANGED_COMBINATION)
 		key->type = old_key_type;
 
 	return 0;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c7eb073..36eb062 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2037,15 +2037,16 @@ static inline void hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff
 	BT_DBG("%s found key type %u for %s", hdev->name, key->type,
 							batostr(&ev->bdaddr));
 
-	if (!test_bit(HCI_DEBUG_KEYS, &hdev->flags) && key->type == 0x03) {
+	if (!test_bit(HCI_DEBUG_KEYS, &hdev->flags) &&
+				key->type == HCI_LK_DEBUG_COMBINATION) {
 		BT_DBG("%s ignoring debug key", hdev->name);
 		goto not_found;
 	}
 
 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
 
-	if (key->type == 0x04 && conn && conn->auth_type != 0xff &&
-						(conn->auth_type & 0x01)) {
+	if (key->type == HCI_LK_UNAUTH_COMBINATION && conn &&
+			conn->auth_type != 0xff && (conn->auth_type & 0x01)) {
 		BT_DBG("%s ignoring unauthenticated key", hdev->name);
 		goto not_found;
 	}
-- 
1.7.1

[PATCH 2/3] Bluetooth: Map sec_level to link key requirements

[Waldemar Rymarkiewicz]

Keep the link key type together with connection and use it to
map security level to link key requirements. Authenticate and/or
encrypt connection if the link is insufficiently secure.

Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_conn.c         |   61 +++++++++++++++++++++++++++++++------
 net/bluetooth/hci_event.c        |    4 ++
 3 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4093133..02e7256 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -226,6 +226,7 @@ struct hci_conn {
 	__u16		pkt_type;
 	__u16		link_policy;
 	__u32		link_mode;
+	__u8		key_type;
 	__u8		auth_type;
 	__u8		sec_level;
 	__u8		pending_sec_level;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 7a6f56b..a339845 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -287,6 +287,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
 	conn->auth_type = HCI_AT_GENERAL_BONDING;
 	conn->io_capability = hdev->io_capability;
 	conn->remote_auth = 0xff;
+	conn->key_type = 0xff;
 
 	conn->power_save = 1;
 	conn->disc_timeout = HCI_DISCONN_TIMEOUT;
@@ -535,32 +536,72 @@ static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
 	return 0;
 }
 
+/* Encrypt the the link */
+static void hci_conn_encrypt(struct hci_conn *conn)
+{
+	BT_DBG("conn %p", conn);
+
+	if (!test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend)) {
+		struct hci_cp_set_conn_encrypt cp;
+		cp.handle  = cpu_to_le16(conn->handle);
+		cp.encrypt = 0x01;
+		hci_send_cmd(conn->hdev, HCI_OP_SET_CONN_ENCRYPT, sizeof(cp),
+									&cp);
+	}
+}
+
 /* Enable security */
 int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
 {
 	BT_DBG("conn %p", conn);
 
+	/* For sdp we don't need the link key. */
 	if (sec_level == BT_SECURITY_SDP)
 		return 1;
 
+	/* For non 2.1 devices and low security level we don't need the link
+	   key. */
 	if (sec_level == BT_SECURITY_LOW &&
 				(!conn->ssp_mode || !conn->hdev->ssp_mode))
 		return 1;
 
-	if (conn->link_mode & HCI_LM_ENCRYPT)
-		return hci_conn_auth(conn, sec_level, auth_type);
-
+	/* For other security levels we need the link key. */
+	if (!(conn->link_mode & HCI_LM_AUTH))
+		goto auth;
+
+	/* An authenticated combination key has sufficient security for any
+	   security level. */
+	if (conn->key_type == HCI_LK_AUTH_COMBINATION)
+		goto encrypt;
+
+	/* An unauthenticated combination key has sufficient security for
+	   security level 1 and 2. */
+	if (conn->key_type == HCI_LK_UNAUTH_COMBINATION &&
+			(sec_level == BT_SECURITY_MEDIUM ||
+			sec_level == BT_SECURITY_LOW))
+		goto encrypt;
+
+	/* A combination key has always sufficient security for the security
+	   levels 1 or 2. High security level requires the combination key
+	   is generated using maximum PIN code length (16).
+	   For pre 2.1 units. */
+	if (conn->key_type == HCI_LK_COMBINATION &&
+			(sec_level != BT_SECURITY_HIGH ||
+			conn->pin_length >= 16))
+		goto encrypt;
+
+auth:
 	if (test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend))
 		return 0;
 
-	if (hci_conn_auth(conn, sec_level, auth_type)) {
-		struct hci_cp_set_conn_encrypt cp;
-		cp.handle  = cpu_to_le16(conn->handle);
-		cp.encrypt = 1;
-		hci_send_cmd(conn->hdev, HCI_OP_SET_CONN_ENCRYPT,
-							sizeof(cp), &cp);
-	}
+	hci_conn_auth(conn, sec_level, auth_type);
+	return 0;
+
+encrypt:
+	if (conn->link_mode & HCI_LM_ENCRYPT)
+		return 1;
 
+	hci_conn_encrypt(conn);
 	return 0;
 }
 EXPORT_SYMBOL(hci_conn_security);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 36eb062..5c5e614 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2080,6 +2080,10 @@ static inline void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff
 		hci_conn_hold(conn);
 		conn->disc_timeout = HCI_DISCONN_TIMEOUT;
 		pin_len = conn->pin_length;
+
+		if (conn->key_type != HCI_LK_CHANGED_COMBINATION)
+			conn->key_type = ev->key_type;
+
 		hci_conn_put(conn);
 	}
 
-- 
1.7.1

[PATCH 3/3] Bluetooth: Ignore key unauthenticated for high security

[Waldemar Rymarkiewicz]

High security level for pre v2.1 devices requires combination link key
authenticated by at least 16 digit PIN code.

It's also necessary to update key_type and pin_length when the key
exists and is sufficently secured for the connection as there will be
no link key notify event in that case.

Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
---
 net/bluetooth/hci_event.c |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 5c5e614..337da2b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2044,11 +2044,24 @@ static inline void hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff
 	}
 
 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
+	if (conn) {
+		if (key->type == HCI_LK_UNAUTH_COMBINATION &&
+				conn->auth_type != 0xff &&
+				(conn->auth_type & 0x01)) {
+			BT_DBG("%s ignoring unauthenticated key", hdev->name);
+			goto not_found;
+		}
 
-	if (key->type == HCI_LK_UNAUTH_COMBINATION && conn &&
-			conn->auth_type != 0xff && (conn->auth_type & 0x01)) {
-		BT_DBG("%s ignoring unauthenticated key", hdev->name);
-		goto not_found;
+		if (key->type == HCI_LK_COMBINATION &&
+					conn->sec_level == BT_SECURITY_HIGH &&
+					conn->pin_length < 16) {
+			BT_DBG("%s ignoring key unauthenticated for high \
+							security", hdev->name);
+			goto not_found;
+		}
+
+		conn->key_type = key->type;
+		conn->pin_length = key->pin_len;
 	}
 
 	bacpy(&cp.bdaddr, &ev->bdaddr);
-- 
1.7.1

RE: [PATCH 3/3] Bluetooth: Ignore key unauthenticated for high security

[Waldemar.Rymarkiewicz]

Hi, 

>Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
>---
> net/bluetooth/hci_event.c |   21 +++++++++++++++++----
> 1 files changed, 17 insertions(+), 4 deletions(-)
>
>diff --git a/net/bluetooth/hci_event.c 
>b/net/bluetooth/hci_event.c index 5c5e614..337da2b 100644
>--- a/net/bluetooth/hci_event.c
>+++ b/net/bluetooth/hci_event.c
>@@ -2044,11 +2044,24 @@ static inline void 
>hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff
> 	}
> 
> 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
>+	if (conn) {
>+		if (key->type == HCI_LK_UNAUTH_COMBINATION &&
>+				conn->auth_type != 0xff &&
>+				(conn->auth_type & 0x01)) {
>+			BT_DBG("%s ignoring unauthenticated 
>key", hdev->name);
>+			goto not_found;
>+		}
> 
>-	if (key->type == HCI_LK_UNAUTH_COMBINATION && conn &&
>-			conn->auth_type != 0xff && 
>(conn->auth_type & 0x01)) {
>-		BT_DBG("%s ignoring unauthenticated key", hdev->name);
>-		goto not_found;
>+		if (key->type == HCI_LK_COMBINATION &&
>+					conn->sec_level == 
>BT_SECURITY_HIGH &&
>+					conn->pin_length < 16) {


That's wrong. I should check it against stored key->pin_len  and  conn->pending_sec_level. 
We are in the middle of authentication so we don't have conn->sec_level set properly yet. The same apply for conn->pin_length.

if (key->type == HCI_LK_COMBINATION && key->pin_len < 16 &&
	conn->pending_sec_level == BT_SECURITY_HIGH) {
			goto not_found;
		}

/Waldek