[RFC 0/2] Returning Encryption Key Size

[RFC 0/2] Returning Encryption Key Size

[Vinicius Costa Gomes]

Disclaimer: the code this is based on is still on review, so this is
is subject to change.

Having Encryption Key Size exported to userspace is needed because
some profiles might have some restrictions depending on the key size
used to encrypt the link.

This patch series implement exporting to userspace adding one more
field to the bt_security structure.

For now, this only is valid for LE links, for 3.0+HS controllers
the Read Encryption Key Size command is defined. The plan is to
use that command for reading the key size as soon as the link
is encrypted, and storing the value read in the field that was
added to hci_conn.

The related userspace patches should follow this series.

--
Cheers,

Vinicius Costa Gomes (2):
  Bluetooth: Add support for storing the key size
  Bluetooth: Add support for returning the encryption key size

 include/net/bluetooth/bluetooth.h |    1 +
 include/net/bluetooth/hci_core.h  |    3 ++-
 net/bluetooth/hci_core.c          |    3 ++-
 net/bluetooth/hci_event.c         |    1 +
 net/bluetooth/l2cap_sock.c        |    4 ++++
 net/bluetooth/mgmt.c              |    4 ++--
 net/bluetooth/smp.c               |   16 ++++++++++------
 7 files changed, 22 insertions(+), 10 deletions(-)

--
1.7.4.3

[RFC 1/2] Bluetooth: Add support for storing the key size

[Vinicius Costa Gomes]

In some cases it will be useful having the key size used for
encrypting the link. For example, some profiles may restrict
some operations depending on the key length.

The key size is stored in the key that is passed to userspace
using the pin_length field in the key structure.

For now this field is only valid for LE controllers. 3.0+HS
controllers define the Read Encryption Key Size command, this
field is intended for storing the value returned by that
command.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    3 ++-
 net/bluetooth/hci_core.c         |    3 ++-
 net/bluetooth/hci_event.c        |    1 +
 net/bluetooth/mgmt.c             |    4 ++--
 net/bluetooth/smp.c              |   16 ++++++++++------
 5 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9bb3c42..3345d96 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -263,6 +263,7 @@ struct hci_conn {
 	__u8		sec_level;
 	__u8		pending_sec_level;
 	__u8		pin_length;
+	__u8		enc_key_size;
 	__u8		io_capability;
 	__u8		power_save;
 	__u16		disc_timeout;
@@ -552,7 +553,7 @@ struct link_key *hci_find_ltk(struct hci_dev *hdev, __le16 ediv, u8 rand[8]);
 struct link_key *hci_find_link_key_type(struct hci_dev *hdev,
 					bdaddr_t *bdaddr, u8 type);
 int hci_add_ltk(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
-					__le16 ediv, u8 rand[8], u8 ltk[16]);
+			u8 key_size, __le16 ediv, u8 rand[8], u8 ltk[16]);
 int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr);
 
 int hci_remote_oob_data_clear(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index c380665..dc7ff39 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1103,7 +1103,7 @@ int hci_add_link_key(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
 }
 
 int hci_add_ltk(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
-					__le16 ediv, u8 rand[8], u8 ltk[16])
+			u8 key_size, __le16 ediv, u8 rand[8], u8 ltk[16])
 {
 	struct link_key *key, *old_key;
 	struct key_master_id *id;
@@ -1128,6 +1128,7 @@ int hci_add_ltk(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
 	bacpy(&key->bdaddr, bdaddr);
 	memcpy(key->val, ltk, sizeof(key->val));
 	key->type = KEY_TYPE_LTK;
+	key->pin_len = key_size;
 
 	id = (void *) &key->data;
 	id->ediv = ediv;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 882a2be..5c8a51f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2654,6 +2654,7 @@ static inline void hci_le_ltk_request_evt(struct hci_dev *hdev,
 
 	memcpy(cp.ltk, ltk->val, sizeof(ltk->val));
 	cp.handle = cpu_to_le16(conn->handle);
+	conn->pin_length = ltk->pin_len;
 
 	hci_send_cmd(hdev, HCI_OP_LE_LTK_REPLY, sizeof(cp), &cp);
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9c25513..ffbcd6e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -956,8 +956,8 @@ static int load_keys(struct sock *sk, u16 index, unsigned char *data, u16 len)
 			if (key->dlen != sizeof(struct key_master_id))
 				continue;
 
-			hci_add_ltk(hdev, 0, &key->bdaddr, id->ediv,
-							id->rand, key->val);
+			hci_add_ltk(hdev, 0, &key->bdaddr, key->pin_len,
+						id->ediv, id->rand, key->val);
 
 			continue;
 		}
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index a93ac78..2dd5748 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -399,6 +399,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 				SMP_MAX_ENC_KEY_SIZE - conn->smp_key_size);
 
 		hci_le_start_enc(hcon, ediv, rand, stk);
+		hcon->enc_key_size = conn->smp_key_size;
 
 		hex_dump_to_buffer(key, sizeof(key), 16, 1, buf,
 							sizeof(buf), 0);
@@ -419,7 +420,8 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 		memset(stk + conn->smp_key_size, 0,
 				SMP_MAX_ENC_KEY_SIZE - conn->smp_key_size);
 
-		hci_add_ltk(conn->hcon->hdev, 0, conn->dst, ediv, rand, stk);
+		hci_add_ltk(conn->hcon->hdev, 0, conn->dst, conn->smp_key_size,
+							ediv, rand, stk);
 
 		hex_dump_to_buffer(key, sizeof(key), 16, 1, buf,
 							sizeof(buf), 0);
@@ -489,6 +491,8 @@ int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
 
 			hci_le_start_enc(hcon, master->ediv, master->rand,
 								key->val);
+			hcon->enc_key_size = key->pin_len;
+
 			goto done;
 		}
 	}
@@ -529,7 +533,7 @@ static int smp_cmd_encrypt_info(struct l2cap_conn *conn, struct sk_buff *skb)
 
 	memset(rand, 0, sizeof(rand));
 
-	err = hci_add_ltk(conn->hcon->hdev, 0, conn->dst, 0, rand, rp->ltk);
+	err = hci_add_ltk(conn->hcon->hdev, 0, conn->dst, 0, 0, rand, rp->ltk);
 	if (err)
 		return SMP_UNSPECIFIED;
 
@@ -556,8 +560,8 @@ static int smp_cmd_master_ident(struct l2cap_conn *conn, struct sk_buff *skb)
 	id->ediv = rp->ediv;
 	memcpy(id->rand, rp->rand, sizeof(rp->rand));
 
-	hci_add_ltk(conn->hcon->hdev, 1, conn->src, rp->ediv,
-						rp->rand, key->val);
+	hci_add_ltk(conn->hcon->hdev, 1, conn->src, conn->smp_key_size,
+						rp->ediv, rp->rand, key->val);
 
 	smp_distribute_keys(conn, 1);
 
@@ -676,8 +680,8 @@ int smp_distribute_keys(struct l2cap_conn *conn, __u8 force)
 
 		smp_send_cmd(conn, SMP_CMD_ENCRYPT_INFO, sizeof(enc), &enc);
 
-		hci_add_ltk(conn->hcon->hdev, 1, conn->dst, ediv,
-							ident.rand, enc.ltk);
+		hci_add_ltk(conn->hcon->hdev, 1, conn->dst, conn->smp_key_size,
+						ediv, ident.rand, enc.ltk);
 
 		ident.ediv = cpu_to_le16(ediv);
 
-- 
1.7.4.3

[RFC 2/2] Bluetooth: Add support for returning the encryption key size

[Vinicius Costa Gomes]

This will be useful when userspace wants to restrict some kinds of
operations based on the length of the key size used to encrypt the
link.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 include/net/bluetooth/bluetooth.h |    1 +
 net/bluetooth/l2cap_sock.c        |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index acf186d..28ae91a 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -56,6 +56,7 @@
 #define BT_SECURITY	4
 struct bt_security {
 	__u8 level;
+	__u8 key_size;
 };
 #define BT_SECURITY_SDP		0
 #define BT_SECURITY_LOW		1
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index b5796fd..7d143f4 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -492,8 +492,12 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
 			break;
 		}
 
+		memset(&sec, 0, sizeof(sec));
 		sec.level = l2cap_pi(sk)->sec_level;
 
+		if (sk->sk_state == BT_CONNECTED)
+			sec.key_size = l2cap_pi(sk)->conn->hcon->enc_key_size;
+
 		len = min_t(unsigned int, len, sizeof(sec));
 		if (copy_to_user(optval, (char *) &sec, len))
 			err = -EFAULT;
-- 
1.7.4.3

Re: [RFC 2/2] Bluetooth: Add support for returning the encryption key size

[Marcel Holtmann]

Hi Vinicius,

> This will be useful when userspace wants to restrict some kinds of
> operations based on the length of the key size used to encrypt the
> link.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  include/net/bluetooth/bluetooth.h |    1 +
>  net/bluetooth/l2cap_sock.c        |    4 ++++
>  2 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index acf186d..28ae91a 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -56,6 +56,7 @@
>  #define BT_SECURITY	4
>  struct bt_security {
>  	__u8 level;
> +	__u8 key_size;
>  };

there is one thing we need to keep in mind. Who is enforcing the
encryption key size and triggers are re-pairing if needed? Do we wanna
do that inside kernel space or have userspace involved?

Essentially besides maybe exporting the current encryption key size, you
also wanna enforce a minium encryption key size.

We can do this with this socket option in one go. I am fine with that,
but we need to have a way to ensure minium encryption key size or 0 if
we do not care.

And of course the same now applies for PIN code length. Even if with
Simple Pairing this does not matter anymore. For Legacy Pairing this is
still relevant.

Regards

Marcel

Re: [RFC 2/2] Bluetooth: Add support for returning the encryption key size

[Marcel Holtmann]

Hi Vinicius,

> And of course the same now applies for PIN code length. Even if with
> Simple Pairing this does not matter anymore. For Legacy Pairing this is
> still relevant.

forget about this comment. It is actually bogus since the HIGH vs MEDIUM
security for Legacy Pairing implies the PIN length requirement already.

We just need to make sure to enforce it properly via the new in-kernel
handling of pairing.

Regards

Marcel