2011-04-15 11:06:54

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: [PATCH 0/3] Handle link key type and security requirements

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



2011-04-18 13:19:58

by Rymarkiewicz Waldemar

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

Hi,

>Signed-off-by: Waldemar Rymarkiewicz <[email protected]>
>---
> 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

2011-04-15 11:06:57

by Rymarkiewicz Waldemar

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

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 <[email protected]>
---
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


2011-04-15 11:06:56

by Rymarkiewicz Waldemar

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

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 <[email protected]>
---
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


2011-04-15 11:06:55

by Rymarkiewicz Waldemar

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

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

Signed-off-by: Waldemar Rymarkiewicz <[email protected]>
---
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