* [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
@ 2021-02-22 10:30 Magdalena Kasenberg
2021-02-22 11:06 ` bluez.test.bot
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Magdalena Kasenberg @ 2021-02-22 10:30 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Magdalena Kasenberg, Szymon Janc
This is required for the qualification test L2CAP/LE/CFC/BV-15-C
Implementation does not allow to set different key size for SMP and
L2CAP, which is needed for a current specification of the test. This fix
workarounds it with the debugfs variable le_l2cap_min_key_size.
Logs from the test when the IUT uses a min and max l2cap encryption key size 16.
$ echo 16 > /sys/kernel/debug/bluetooth/hci0/le_l2cap_min_key_size
The lower tester uses a key size 7.
> ACL Data RX: Handle 99 flags 0x02 dlen 11 #34 [hci0] 25.007392
SMP: Pairing Request (0x01) len 6
IO capability: DisplayYesNo (0x01)
OOB data: Authentication data not present (0x00)
Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
Max encryption key size: 7
Initiator key distribution: <none> (0x00)
Responder key distribution: <none> (0x00)
< ACL Data TX: Handle 99 flags 0x00 dlen 11 #35 [hci0] 25.007591
SMP: Pairing Response (0x02) len 6
IO capability: KeyboardDisplay (0x04)
OOB data: Authentication data not present (0x00)
Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
Max encryption key size: 16
Initiator key distribution: <none> (0x00)
Responder key distribution: <none> (0x00)
@ MGMT Event: New Long Term Key (0x000a) plen 37 {0x0001} [hci0] 28.788872
Store hint: Yes (0x01)
LE Address: C0:DE:C0:FF:FF:01 (OUI C0-DE-C0)
Key type: Unauthenticated key from P-256 (0x02)
Master: 0x00
Encryption size: 7
Diversifier: 0000
Randomizer: 0000000000000000
Key: 529e11e8c7b9f5000000000000000000
<snip>
After pairing with key size 7, L2CAP connection is requested which
requires key size 16.
> ACL Data RX: Handle 99 flags 0x02 dlen 18 #56 [hci0] 34.998084
LE L2CAP: LE Connection Request (0x14) ident 3 len 10
PSM: 244 (0x00f4)
Source CID: 64
MTU: 256
MPS: 284
Credits: 1
< ACL Data TX: Handle 99 flags 0x00 dlen 18 #57 [hci0] 34.998325
LE L2CAP: LE Connection Response (0x15) ident 3 len 10
Destination CID: 0
MTU: 0
MPS: 0
Credits: 0
Result: Connection refused - insufficient encryption key size (0x0007)
Signed-off-by: Magdalena Kasenberg <magdalena.kasenberg@codecoup.pl>
Reviewed-by: Szymon Janc <szymon.janc@codecoup.pl>
Cc: Szymon Janc <szymon.janc@codecoup.pl>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 1 +
net/bluetooth/hci_debugfs.c | 30 ++++++++++++++++++++++++++++++
net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++++++
4 files changed, 57 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ebdd4afe30d2..0bf0543efec5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -379,6 +379,7 @@ struct hci_dev {
__u16 auth_payload_timeout;
__u8 min_enc_key_size;
__u8 max_enc_key_size;
+ __u8 le_l2cap_min_key_size;
__u8 pairing_opts;
__u8 ssp_debug_mode;
__u8 hw_error_code;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b0d9c36acc03..9ef4b39b380c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3788,6 +3788,7 @@ struct hci_dev *hci_alloc_dev(void)
hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;
+ hdev->le_l2cap_min_key_size = HCI_MIN_ENC_KEY_SIZE;
/* default 1.28 sec page scan */
hdev->def_page_scan_type = PAGE_SCAN_TYPE_STANDARD;
diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
index 1a0ab58bfad0..dec8b96b8427 100644
--- a/net/bluetooth/hci_debugfs.c
+++ b/net/bluetooth/hci_debugfs.c
@@ -1096,6 +1096,34 @@ static int max_key_size_get(void *data, u64 *val)
DEFINE_DEBUGFS_ATTRIBUTE(max_key_size_fops, max_key_size_get,
max_key_size_set, "%llu\n");
+static int le_l2cap_min_key_size_set(void *data, u64 val)
+{
+ struct hci_dev *hdev = data;
+
+ if (val > SMP_MAX_ENC_KEY_SIZE || val < SMP_MIN_ENC_KEY_SIZE)
+ return -EINVAL;
+
+ hci_dev_lock(hdev);
+ hdev->le_l2cap_min_key_size = val;
+ hci_dev_unlock(hdev);
+
+ return 0;
+}
+
+static int le_l2cap_min_key_size_get(void *data, u64 *val)
+{
+ struct hci_dev *hdev = data;
+
+ hci_dev_lock(hdev);
+ *val = hdev->le_l2cap_min_key_size;
+ hci_dev_unlock(hdev);
+
+ return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(le_l2cap_min_key_size_fops, le_l2cap_min_key_size_get,
+ le_l2cap_min_key_size_set, "%llu\n");
+
static int auth_payload_timeout_set(void *data, u64 val)
{
struct hci_dev *hdev = data;
@@ -1226,6 +1254,8 @@ void hci_debugfs_create_le(struct hci_dev *hdev)
&min_key_size_fops);
debugfs_create_file("max_key_size", 0644, hdev->debugfs, hdev,
&max_key_size_fops);
+ debugfs_create_file("le_l2cap_min_key_size", 0644, hdev->debugfs, hdev,
+ &le_l2cap_min_key_size_fops);
debugfs_create_file("auth_payload_timeout", 0644, hdev->debugfs, hdev,
&auth_payload_timeout_fops);
debugfs_create_file("force_no_mitm", 0644, hdev->debugfs, hdev,
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 72c2f5226d67..d9a3a1c1f366 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5742,6 +5742,20 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
return err;
}
+static bool le_l2cap_key_size_sufficient(struct hci_conn *hcon, u8 sec_level)
+{
+ struct smp_ltk *ltk;
+
+ if (sec_level == BT_SECURITY_LOW)
+ return true;
+
+ ltk = hci_find_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, hcon->role);
+ if (ltk && ltk->enc_size >= hcon->hdev->le_l2cap_min_key_size)
+ return true;
+
+ return false;
+}
+
static int l2cap_le_connect_req(struct l2cap_conn *conn,
struct l2cap_cmd_hdr *cmd, u16 cmd_len,
u8 *data)
@@ -5788,6 +5802,12 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
goto response_unlock;
}
+ if (!le_l2cap_key_size_sufficient(conn->hcon, pchan->sec_level)) {
+ result = L2CAP_CR_LE_BAD_KEY_SIZE;
+ chan = NULL;
+ goto response_unlock;
+ }
+
/* Check for valid dynamic CID range */
if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_LE_DYN_END) {
result = L2CAP_CR_LE_INVALID_SCID;
@@ -5969,6 +5989,11 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
goto unlock;
}
+ if (!le_l2cap_key_size_sufficient(conn->hcon, pchan->sec_level)) {
+ result = L2CAP_CR_LE_BAD_KEY_SIZE;
+ goto unlock;
+ }
+
result = L2CAP_CR_LE_SUCCESS;
cmd_len -= sizeof(*req);
num_scid = cmd_len / sizeof(u16);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* RE: Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
2021-02-22 10:30 [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C Magdalena Kasenberg
@ 2021-02-22 11:06 ` bluez.test.bot
2021-02-22 17:46 ` [PATCH] " Luiz Augusto von Dentz
2021-02-26 20:17 ` Marcel Holtmann
2 siblings, 0 replies; 9+ messages in thread
From: bluez.test.bot @ 2021-02-22 11:06 UTC (permalink / raw)
To: linux-bluetooth, magdalena.kasenberg
[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=436497
---Test result---
##############################
Test: CheckPatch - FAIL
Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#12:
Logs from the test when the IUT uses a min and max l2cap encryption key size 16.
total: 0 errors, 1 warnings, 0 checks, 99 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuildK - PASS
##############################
Test: CheckTestRunner: Setup - PASS
##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6
##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: mgmt-tester - PASS
Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14
##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 43342 bytes --]
[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3531 bytes --]
[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 546680 bytes --]
[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11653 bytes --]
[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9888 bytes --]
[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11799 bytes --]
[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5429 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
2021-02-22 10:30 [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C Magdalena Kasenberg
2021-02-22 11:06 ` bluez.test.bot
@ 2021-02-22 17:46 ` Luiz Augusto von Dentz
2021-02-26 20:17 ` Marcel Holtmann
2 siblings, 0 replies; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2021-02-22 17:46 UTC (permalink / raw)
To: Magdalena Kasenberg; +Cc: linux-bluetooth, Szymon Janc
Hi Magdalena,
On Mon, Feb 22, 2021 at 2:34 AM Magdalena Kasenberg
<magdalena.kasenberg@codecoup.pl> wrote:
>
> This is required for the qualification test L2CAP/LE/CFC/BV-15-C
>
> Implementation does not allow to set different key size for SMP and
> L2CAP, which is needed for a current specification of the test. This fix
> workarounds it with the debugfs variable le_l2cap_min_key_size.
>
> Logs from the test when the IUT uses a min and max l2cap encryption key size 16.
> $ echo 16 > /sys/kernel/debug/bluetooth/hci0/le_l2cap_min_key_size
> The lower tester uses a key size 7.
>
> > ACL Data RX: Handle 99 flags 0x02 dlen 11 #34 [hci0] 25.007392
> SMP: Pairing Request (0x01) len 6
> IO capability: DisplayYesNo (0x01)
> OOB data: Authentication data not present (0x00)
> Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
> Max encryption key size: 7
> Initiator key distribution: <none> (0x00)
> Responder key distribution: <none> (0x00)
> < ACL Data TX: Handle 99 flags 0x00 dlen 11 #35 [hci0] 25.007591
> SMP: Pairing Response (0x02) len 6
> IO capability: KeyboardDisplay (0x04)
> OOB data: Authentication data not present (0x00)
> Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
> Max encryption key size: 16
> Initiator key distribution: <none> (0x00)
> Responder key distribution: <none> (0x00)
> @ MGMT Event: New Long Term Key (0x000a) plen 37 {0x0001} [hci0] 28.788872
> Store hint: Yes (0x01)
> LE Address: C0:DE:C0:FF:FF:01 (OUI C0-DE-C0)
> Key type: Unauthenticated key from P-256 (0x02)
> Master: 0x00
> Encryption size: 7
> Diversifier: 0000
> Randomizer: 0000000000000000
> Key: 529e11e8c7b9f5000000000000000000
>
> <snip>
>
> After pairing with key size 7, L2CAP connection is requested which
> requires key size 16.
>
> > ACL Data RX: Handle 99 flags 0x02 dlen 18 #56 [hci0] 34.998084
> LE L2CAP: LE Connection Request (0x14) ident 3 len 10
> PSM: 244 (0x00f4)
> Source CID: 64
> MTU: 256
> MPS: 284
> Credits: 1
> < ACL Data TX: Handle 99 flags 0x00 dlen 18 #57 [hci0] 34.998325
> LE L2CAP: LE Connection Response (0x15) ident 3 len 10
> Destination CID: 0
> MTU: 0
> MPS: 0
> Credits: 0
> Result: Connection refused - insufficient encryption key size (0x0007)
>
> Signed-off-by: Magdalena Kasenberg <magdalena.kasenberg@codecoup.pl>
> Reviewed-by: Szymon Janc <szymon.janc@codecoup.pl>
Reviewed-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> Cc: Szymon Janc <szymon.janc@codecoup.pl>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 1 +
> net/bluetooth/hci_debugfs.c | 30 ++++++++++++++++++++++++++++++
> net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++++++
> 4 files changed, 57 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ebdd4afe30d2..0bf0543efec5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -379,6 +379,7 @@ struct hci_dev {
> __u16 auth_payload_timeout;
> __u8 min_enc_key_size;
> __u8 max_enc_key_size;
> + __u8 le_l2cap_min_key_size;
> __u8 pairing_opts;
> __u8 ssp_debug_mode;
> __u8 hw_error_code;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index b0d9c36acc03..9ef4b39b380c 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3788,6 +3788,7 @@ struct hci_dev *hci_alloc_dev(void)
> hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
> hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
> hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;
> + hdev->le_l2cap_min_key_size = HCI_MIN_ENC_KEY_SIZE;
>
> /* default 1.28 sec page scan */
> hdev->def_page_scan_type = PAGE_SCAN_TYPE_STANDARD;
> diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
> index 1a0ab58bfad0..dec8b96b8427 100644
> --- a/net/bluetooth/hci_debugfs.c
> +++ b/net/bluetooth/hci_debugfs.c
> @@ -1096,6 +1096,34 @@ static int max_key_size_get(void *data, u64 *val)
> DEFINE_DEBUGFS_ATTRIBUTE(max_key_size_fops, max_key_size_get,
> max_key_size_set, "%llu\n");
>
> +static int le_l2cap_min_key_size_set(void *data, u64 val)
> +{
> + struct hci_dev *hdev = data;
> +
> + if (val > SMP_MAX_ENC_KEY_SIZE || val < SMP_MIN_ENC_KEY_SIZE)
> + return -EINVAL;
> +
> + hci_dev_lock(hdev);
> + hdev->le_l2cap_min_key_size = val;
> + hci_dev_unlock(hdev);
> +
> + return 0;
> +}
> +
> +static int le_l2cap_min_key_size_get(void *data, u64 *val)
> +{
> + struct hci_dev *hdev = data;
> +
> + hci_dev_lock(hdev);
> + *val = hdev->le_l2cap_min_key_size;
> + hci_dev_unlock(hdev);
> +
> + return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(le_l2cap_min_key_size_fops, le_l2cap_min_key_size_get,
> + le_l2cap_min_key_size_set, "%llu\n");
> +
> static int auth_payload_timeout_set(void *data, u64 val)
> {
> struct hci_dev *hdev = data;
> @@ -1226,6 +1254,8 @@ void hci_debugfs_create_le(struct hci_dev *hdev)
> &min_key_size_fops);
> debugfs_create_file("max_key_size", 0644, hdev->debugfs, hdev,
> &max_key_size_fops);
> + debugfs_create_file("le_l2cap_min_key_size", 0644, hdev->debugfs, hdev,
> + &le_l2cap_min_key_size_fops);
> debugfs_create_file("auth_payload_timeout", 0644, hdev->debugfs, hdev,
> &auth_payload_timeout_fops);
> debugfs_create_file("force_no_mitm", 0644, hdev->debugfs, hdev,
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 72c2f5226d67..d9a3a1c1f366 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -5742,6 +5742,20 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
> return err;
> }
>
> +static bool le_l2cap_key_size_sufficient(struct hci_conn *hcon, u8 sec_level)
> +{
> + struct smp_ltk *ltk;
> +
> + if (sec_level == BT_SECURITY_LOW)
> + return true;
> +
> + ltk = hci_find_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, hcon->role);
> + if (ltk && ltk->enc_size >= hcon->hdev->le_l2cap_min_key_size)
> + return true;
> +
> + return false;
> +}
> +
> static int l2cap_le_connect_req(struct l2cap_conn *conn,
> struct l2cap_cmd_hdr *cmd, u16 cmd_len,
> u8 *data)
> @@ -5788,6 +5802,12 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
> goto response_unlock;
> }
>
> + if (!le_l2cap_key_size_sufficient(conn->hcon, pchan->sec_level)) {
> + result = L2CAP_CR_LE_BAD_KEY_SIZE;
> + chan = NULL;
> + goto response_unlock;
> + }
> +
> /* Check for valid dynamic CID range */
> if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_LE_DYN_END) {
> result = L2CAP_CR_LE_INVALID_SCID;
> @@ -5969,6 +5989,11 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> goto unlock;
> }
>
> + if (!le_l2cap_key_size_sufficient(conn->hcon, pchan->sec_level)) {
> + result = L2CAP_CR_LE_BAD_KEY_SIZE;
> + goto unlock;
> + }
> +
> result = L2CAP_CR_LE_SUCCESS;
> cmd_len -= sizeof(*req);
> num_scid = cmd_len / sizeof(u16);
> --
> 2.25.1
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
2021-02-22 10:30 [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C Magdalena Kasenberg
2021-02-22 11:06 ` bluez.test.bot
2021-02-22 17:46 ` [PATCH] " Luiz Augusto von Dentz
@ 2021-02-26 20:17 ` Marcel Holtmann
2021-02-26 21:42 ` Luiz Augusto von Dentz
2 siblings, 1 reply; 9+ messages in thread
From: Marcel Holtmann @ 2021-02-26 20:17 UTC (permalink / raw)
To: Magdalena Kasenberg; +Cc: Bluetooth Kernel Mailing List, Szymon Janc
Hi Magdalena,
> This is required for the qualification test L2CAP/LE/CFC/BV-15-C
>
> Implementation does not allow to set different key size for SMP and
> L2CAP, which is needed for a current specification of the test. This fix
> workarounds it with the debugfs variable le_l2cap_min_key_size.
>
> Logs from the test when the IUT uses a min and max l2cap encryption key size 16.
> $ echo 16 > /sys/kernel/debug/bluetooth/hci0/le_l2cap_min_key_size
> The lower tester uses a key size 7.
>
>> ACL Data RX: Handle 99 flags 0x02 dlen 11 #34 [hci0] 25.007392
> SMP: Pairing Request (0x01) len 6
> IO capability: DisplayYesNo (0x01)
> OOB data: Authentication data not present (0x00)
> Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
> Max encryption key size: 7
> Initiator key distribution: <none> (0x00)
> Responder key distribution: <none> (0x00)
> < ACL Data TX: Handle 99 flags 0x00 dlen 11 #35 [hci0] 25.007591
> SMP: Pairing Response (0x02) len 6
> IO capability: KeyboardDisplay (0x04)
> OOB data: Authentication data not present (0x00)
> Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
> Max encryption key size: 16
> Initiator key distribution: <none> (0x00)
> Responder key distribution: <none> (0x00)
> @ MGMT Event: New Long Term Key (0x000a) plen 37 {0x0001} [hci0] 28.788872
> Store hint: Yes (0x01)
> LE Address: C0:DE:C0:FF:FF:01 (OUI C0-DE-C0)
> Key type: Unauthenticated key from P-256 (0x02)
> Master: 0x00
> Encryption size: 7
> Diversifier: 0000
> Randomizer: 0000000000000000
> Key: 529e11e8c7b9f5000000000000000000
>
> <snip>
>
> After pairing with key size 7, L2CAP connection is requested which
> requires key size 16.
>
>> ACL Data RX: Handle 99 flags 0x02 dlen 18 #56 [hci0] 34.998084
> LE L2CAP: LE Connection Request (0x14) ident 3 len 10
> PSM: 244 (0x00f4)
> Source CID: 64
> MTU: 256
> MPS: 284
> Credits: 1
> < ACL Data TX: Handle 99 flags 0x00 dlen 18 #57 [hci0] 34.998325
> LE L2CAP: LE Connection Response (0x15) ident 3 len 10
> Destination CID: 0
> MTU: 0
> MPS: 0
> Credits: 0
> Result: Connection refused - insufficient encryption key size (0x0007)
>
> Signed-off-by: Magdalena Kasenberg <magdalena.kasenberg@codecoup.pl>
> Reviewed-by: Szymon Janc <szymon.janc@codecoup.pl>
> Cc: Szymon Janc <szymon.janc@codecoup.pl>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 1 +
> net/bluetooth/hci_debugfs.c | 30 ++++++++++++++++++++++++++++++
> net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++++++
> 4 files changed, 57 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ebdd4afe30d2..0bf0543efec5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -379,6 +379,7 @@ struct hci_dev {
> __u16 auth_payload_timeout;
> __u8 min_enc_key_size;
> __u8 max_enc_key_size;
> + __u8 le_l2cap_min_key_size;
> __u8 pairing_opts;
> __u8 ssp_debug_mode;
> __u8 hw_error_code;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index b0d9c36acc03..9ef4b39b380c 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3788,6 +3788,7 @@ struct hci_dev *hci_alloc_dev(void)
> hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
> hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
> hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;
> + hdev->le_l2cap_min_key_size = HCI_MIN_ENC_KEY_SIZE;
so I am not a fan of doing this with another variable and managing through debugfs. Can we pass the qualification test case by using BT_SECURITY_FIPS (which will enforce 128-bit key size)?
If not then we might want to add a socket option to set min/max encryption key size requirement on a per socket basis.
Regards
Marcel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
2021-02-26 20:17 ` Marcel Holtmann
@ 2021-02-26 21:42 ` Luiz Augusto von Dentz
2021-02-27 20:14 ` Marcel Holtmann
0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2021-02-26 21:42 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Magdalena Kasenberg, Bluetooth Kernel Mailing List, Szymon Janc
Hi Marcel,
On Fri, Feb 26, 2021 at 12:23 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Magdalena,
>
> > This is required for the qualification test L2CAP/LE/CFC/BV-15-C
> >
> > Implementation does not allow to set different key size for SMP and
> > L2CAP, which is needed for a current specification of the test. This fix
> > workarounds it with the debugfs variable le_l2cap_min_key_size.
> >
> > Logs from the test when the IUT uses a min and max l2cap encryption key size 16.
> > $ echo 16 > /sys/kernel/debug/bluetooth/hci0/le_l2cap_min_key_size
> > The lower tester uses a key size 7.
> >
> >> ACL Data RX: Handle 99 flags 0x02 dlen 11 #34 [hci0] 25.007392
> > SMP: Pairing Request (0x01) len 6
> > IO capability: DisplayYesNo (0x01)
> > OOB data: Authentication data not present (0x00)
> > Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
> > Max encryption key size: 7
> > Initiator key distribution: <none> (0x00)
> > Responder key distribution: <none> (0x00)
> > < ACL Data TX: Handle 99 flags 0x00 dlen 11 #35 [hci0] 25.007591
> > SMP: Pairing Response (0x02) len 6
> > IO capability: KeyboardDisplay (0x04)
> > OOB data: Authentication data not present (0x00)
> > Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
> > Max encryption key size: 16
> > Initiator key distribution: <none> (0x00)
> > Responder key distribution: <none> (0x00)
> > @ MGMT Event: New Long Term Key (0x000a) plen 37 {0x0001} [hci0] 28.788872
> > Store hint: Yes (0x01)
> > LE Address: C0:DE:C0:FF:FF:01 (OUI C0-DE-C0)
> > Key type: Unauthenticated key from P-256 (0x02)
> > Master: 0x00
> > Encryption size: 7
> > Diversifier: 0000
> > Randomizer: 0000000000000000
> > Key: 529e11e8c7b9f5000000000000000000
> >
> > <snip>
> >
> > After pairing with key size 7, L2CAP connection is requested which
> > requires key size 16.
> >
> >> ACL Data RX: Handle 99 flags 0x02 dlen 18 #56 [hci0] 34.998084
> > LE L2CAP: LE Connection Request (0x14) ident 3 len 10
> > PSM: 244 (0x00f4)
> > Source CID: 64
> > MTU: 256
> > MPS: 284
> > Credits: 1
> > < ACL Data TX: Handle 99 flags 0x00 dlen 18 #57 [hci0] 34.998325
> > LE L2CAP: LE Connection Response (0x15) ident 3 len 10
> > Destination CID: 0
> > MTU: 0
> > MPS: 0
> > Credits: 0
> > Result: Connection refused - insufficient encryption key size (0x0007)
> >
> > Signed-off-by: Magdalena Kasenberg <magdalena.kasenberg@codecoup.pl>
> > Reviewed-by: Szymon Janc <szymon.janc@codecoup.pl>
> > Cc: Szymon Janc <szymon.janc@codecoup.pl>
> > ---
> > include/net/bluetooth/hci_core.h | 1 +
> > net/bluetooth/hci_core.c | 1 +
> > net/bluetooth/hci_debugfs.c | 30 ++++++++++++++++++++++++++++++
> > net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++++++
> > 4 files changed, 57 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index ebdd4afe30d2..0bf0543efec5 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -379,6 +379,7 @@ struct hci_dev {
> > __u16 auth_payload_timeout;
> > __u8 min_enc_key_size;
> > __u8 max_enc_key_size;
> > + __u8 le_l2cap_min_key_size;
> > __u8 pairing_opts;
> > __u8 ssp_debug_mode;
> > __u8 hw_error_code;
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index b0d9c36acc03..9ef4b39b380c 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -3788,6 +3788,7 @@ struct hci_dev *hci_alloc_dev(void)
> > hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
> > hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
> > hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;
> > + hdev->le_l2cap_min_key_size = HCI_MIN_ENC_KEY_SIZE;
>
> so I am not a fan of doing this with another variable and managing through debugfs. Can we pass the qualification test case by using BT_SECURITY_FIPS (which will enforce 128-bit key size)?
I guess that will depend if PTS supports MITM which afaik it is
required with BT_SECURITY_FIPS, from the logs it looks like it doesn't
support it so we end up with an unauthenticated key so the error would
probably be different.
> If not then we might want to add a socket option to set min/max encryption key size requirement on a per socket basis.
Yep, it seems to be a common trend to have such tests on upper layers
(ATT/GATT also have such encryption key size), even though it is more
of a GAP test and perhaps could have been done at SMP level.
>
> Regards
>
> Marcel
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
2021-02-26 21:42 ` Luiz Augusto von Dentz
@ 2021-02-27 20:14 ` Marcel Holtmann
2021-03-09 10:27 ` Szymon Janc
0 siblings, 1 reply; 9+ messages in thread
From: Marcel Holtmann @ 2021-02-27 20:14 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Magdalena Kasenberg, Bluetooth Kernel Mailing List, Szymon Janc
Hi Luiz,
>>> This is required for the qualification test L2CAP/LE/CFC/BV-15-C
>>>
>>> Implementation does not allow to set different key size for SMP and
>>> L2CAP, which is needed for a current specification of the test. This fix
>>> workarounds it with the debugfs variable le_l2cap_min_key_size.
>>>
>>> Logs from the test when the IUT uses a min and max l2cap encryption key size 16.
>>> $ echo 16 > /sys/kernel/debug/bluetooth/hci0/le_l2cap_min_key_size
>>> The lower tester uses a key size 7.
>>>
>>>> ACL Data RX: Handle 99 flags 0x02 dlen 11 #34 [hci0] 25.007392
>>> SMP: Pairing Request (0x01) len 6
>>> IO capability: DisplayYesNo (0x01)
>>> OOB data: Authentication data not present (0x00)
>>> Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
>>> Max encryption key size: 7
>>> Initiator key distribution: <none> (0x00)
>>> Responder key distribution: <none> (0x00)
>>> < ACL Data TX: Handle 99 flags 0x00 dlen 11 #35 [hci0] 25.007591
>>> SMP: Pairing Response (0x02) len 6
>>> IO capability: KeyboardDisplay (0x04)
>>> OOB data: Authentication data not present (0x00)
>>> Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
>>> Max encryption key size: 16
>>> Initiator key distribution: <none> (0x00)
>>> Responder key distribution: <none> (0x00)
>>> @ MGMT Event: New Long Term Key (0x000a) plen 37 {0x0001} [hci0] 28.788872
>>> Store hint: Yes (0x01)
>>> LE Address: C0:DE:C0:FF:FF:01 (OUI C0-DE-C0)
>>> Key type: Unauthenticated key from P-256 (0x02)
>>> Master: 0x00
>>> Encryption size: 7
>>> Diversifier: 0000
>>> Randomizer: 0000000000000000
>>> Key: 529e11e8c7b9f5000000000000000000
>>>
>>> <snip>
>>>
>>> After pairing with key size 7, L2CAP connection is requested which
>>> requires key size 16.
>>>
>>>> ACL Data RX: Handle 99 flags 0x02 dlen 18 #56 [hci0] 34.998084
>>> LE L2CAP: LE Connection Request (0x14) ident 3 len 10
>>> PSM: 244 (0x00f4)
>>> Source CID: 64
>>> MTU: 256
>>> MPS: 284
>>> Credits: 1
>>> < ACL Data TX: Handle 99 flags 0x00 dlen 18 #57 [hci0] 34.998325
>>> LE L2CAP: LE Connection Response (0x15) ident 3 len 10
>>> Destination CID: 0
>>> MTU: 0
>>> MPS: 0
>>> Credits: 0
>>> Result: Connection refused - insufficient encryption key size (0x0007)
>>>
>>> Signed-off-by: Magdalena Kasenberg <magdalena.kasenberg@codecoup.pl>
>>> Reviewed-by: Szymon Janc <szymon.janc@codecoup.pl>
>>> Cc: Szymon Janc <szymon.janc@codecoup.pl>
>>> ---
>>> include/net/bluetooth/hci_core.h | 1 +
>>> net/bluetooth/hci_core.c | 1 +
>>> net/bluetooth/hci_debugfs.c | 30 ++++++++++++++++++++++++++++++
>>> net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++++++
>>> 4 files changed, 57 insertions(+)
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>> index ebdd4afe30d2..0bf0543efec5 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -379,6 +379,7 @@ struct hci_dev {
>>> __u16 auth_payload_timeout;
>>> __u8 min_enc_key_size;
>>> __u8 max_enc_key_size;
>>> + __u8 le_l2cap_min_key_size;
>>> __u8 pairing_opts;
>>> __u8 ssp_debug_mode;
>>> __u8 hw_error_code;
>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>> index b0d9c36acc03..9ef4b39b380c 100644
>>> --- a/net/bluetooth/hci_core.c
>>> +++ b/net/bluetooth/hci_core.c
>>> @@ -3788,6 +3788,7 @@ struct hci_dev *hci_alloc_dev(void)
>>> hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
>>> hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
>>> hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;
>>> + hdev->le_l2cap_min_key_size = HCI_MIN_ENC_KEY_SIZE;
>>
>> so I am not a fan of doing this with another variable and managing through debugfs. Can we pass the qualification test case by using BT_SECURITY_FIPS (which will enforce 128-bit key size)?
>
> I guess that will depend if PTS supports MITM which afaik it is
> required with BT_SECURITY_FIPS, from the logs it looks like it doesn't
> support it so we end up with an unauthenticated key so the error would
> probably be different.
we should give this a try ..
>
>> If not then we might want to add a socket option to set min/max encryption key size requirement on a per socket basis.
>
> Yep, it seems to be a common trend to have such tests on upper layers
> (ATT/GATT also have such encryption key size), even though it is more
> of a GAP test and perhaps could have been done at SMP level.
.. however maybe we just deprecate BT_SECURITY or migrate it into something that allows specifying the details of a security level with extra parameters. I made the BT_SECURITY implementation in the kernel extendable. So we could also just add extra possible settings.
Regards
Marcel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
2021-02-27 20:14 ` Marcel Holtmann
@ 2021-03-09 10:27 ` Szymon Janc
2021-03-09 18:35 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 9+ messages in thread
From: Szymon Janc @ 2021-03-09 10:27 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Marcel Holtmann
Cc: Magdalena Kasenberg, Bluetooth Kernel Mailing List
Hi Marcel, Luiz,
On Saturday, 27 February 2021 21:14:57 CET Marcel Holtmann wrote:
> Hi Luiz,
>
> >>> This is required for the qualification test L2CAP/LE/CFC/BV-15-C
> >>>
> >>> Implementation does not allow to set different key size for SMP and
> >>> L2CAP, which is needed for a current specification of the test. This fix
> >>> workarounds it with the debugfs variable le_l2cap_min_key_size.
> >>>
> >>> Logs from the test when the IUT uses a min and max l2cap encryption key
> >>> size 16. $ echo 16 >
> >>> /sys/kernel/debug/bluetooth/hci0/le_l2cap_min_key_size The lower tester
> >>> uses a key size 7.
> >>>
> >>>> ACL Data RX: Handle 99 flags 0x02 dlen 11 #34 [hci0]
> >>>> 25.007392>>>>
> >>> SMP: Pairing Request (0x01) len 6
> >>>
> >>> IO capability: DisplayYesNo (0x01)
> >>> OOB data: Authentication data not present (0x00)
> >>> Authentication requirement: Bonding, No MITM, SC, No Keypresses
> >>> (0x09)
> >>> Max encryption key size: 7
> >>> Initiator key distribution: <none> (0x00)
> >>> Responder key distribution: <none> (0x00)
> >>>
> >>> < ACL Data TX: Handle 99 flags 0x00 dlen 11 #35 [hci0]
> >>> 25.007591>>>
> >>> SMP: Pairing Response (0x02) len 6
> >>>
> >>> IO capability: KeyboardDisplay (0x04)
> >>> OOB data: Authentication data not present (0x00)
> >>> Authentication requirement: Bonding, No MITM, SC, No Keypresses
> >>> (0x09)
> >>> Max encryption key size: 16
> >>> Initiator key distribution: <none> (0x00)
> >>> Responder key distribution: <none> (0x00)
> >>>
> >>> @ MGMT Event: New Long Term Key (0x000a) plen 37 {0x0001} [hci0]
> >>> 28.788872>>>
> >>> Store hint: Yes (0x01)
> >>> LE Address: C0:DE:C0:FF:FF:01 (OUI C0-DE-C0)
> >>> Key type: Unauthenticated key from P-256 (0x02)
> >>> Master: 0x00
> >>> Encryption size: 7
> >>> Diversifier: 0000
> >>> Randomizer: 0000000000000000
> >>> Key: 529e11e8c7b9f5000000000000000000
> >>>
> >>> <snip>
> >>>
> >>> After pairing with key size 7, L2CAP connection is requested which
> >>> requires key size 16.
> >>>
> >>>> ACL Data RX: Handle 99 flags 0x02 dlen 18 #56 [hci0]
> >>>> 34.998084>>>>
> >>> LE L2CAP: LE Connection Request (0x14) ident 3 len 10
> >>>
> >>> PSM: 244 (0x00f4)
> >>> Source CID: 64
> >>> MTU: 256
> >>> MPS: 284
> >>> Credits: 1
> >>>
> >>> < ACL Data TX: Handle 99 flags 0x00 dlen 18 #57 [hci0]
> >>> 34.998325>>>
> >>> LE L2CAP: LE Connection Response (0x15) ident 3 len 10
> >>>
> >>> Destination CID: 0
> >>> MTU: 0
> >>> MPS: 0
> >>> Credits: 0
> >>> Result: Connection refused - insufficient encryption key size
> >>> (0x0007)
> >>>
> >>> Signed-off-by: Magdalena Kasenberg <magdalena.kasenberg@codecoup.pl>
> >>> Reviewed-by: Szymon Janc <szymon.janc@codecoup.pl>
> >>> Cc: Szymon Janc <szymon.janc@codecoup.pl>
> >>> ---
> >>> include/net/bluetooth/hci_core.h | 1 +
> >>> net/bluetooth/hci_core.c | 1 +
> >>> net/bluetooth/hci_debugfs.c | 30 ++++++++++++++++++++++++++++++
> >>> net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++++++
> >>> 4 files changed, 57 insertions(+)
> >>>
> >>> diff --git a/include/net/bluetooth/hci_core.h
> >>> b/include/net/bluetooth/hci_core.h index ebdd4afe30d2..0bf0543efec5
> >>> 100644
> >>> --- a/include/net/bluetooth/hci_core.h
> >>> +++ b/include/net/bluetooth/hci_core.h
> >>> @@ -379,6 +379,7 @@ struct hci_dev {
> >>>
> >>> __u16 auth_payload_timeout;
> >>> __u8 min_enc_key_size;
> >>> __u8 max_enc_key_size;
> >>>
> >>> + __u8 le_l2cap_min_key_size;
> >>>
> >>> __u8 pairing_opts;
> >>> __u8 ssp_debug_mode;
> >>> __u8 hw_error_code;
> >>>
> >>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >>> index b0d9c36acc03..9ef4b39b380c 100644
> >>> --- a/net/bluetooth/hci_core.c
> >>> +++ b/net/bluetooth/hci_core.c
> >>> @@ -3788,6 +3788,7 @@ struct hci_dev *hci_alloc_dev(void)
> >>>
> >>> hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
> >>> hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
> >>> hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;
> >>>
> >>> + hdev->le_l2cap_min_key_size = HCI_MIN_ENC_KEY_SIZE;
> >>
> >> so I am not a fan of doing this with another variable and managing
> >> through debugfs. Can we pass the qualification test case by using
> >> BT_SECURITY_FIPS (which will enforce 128-bit key size)?>
> > I guess that will depend if PTS supports MITM which afaik it is
> > required with BT_SECURITY_FIPS, from the logs it looks like it doesn't
> > support it so we end up with an unauthenticated key so the error would
> > probably be different.
>
> we should give this a try ..
PTS is not supporting GAP in this test at all, but since it is cat D test we
can run it with our own LT (nimBLE).
Using BT_SECURITY_FIPS will not do since it requires 128bit key to get to
FIPS level so with lower key size it fails on SMP.
BTW We are going to propose TSE which would allow to have alternative pass
case in this test with early fail on SMP so that it can be handled as GAP
configuration, not L2CAP. But for now, we have to handle it as defined in test
spec.
>
> >> If not then we might want to add a socket option to set min/max
> >> encryption key size requirement on a per socket basis.>
> > Yep, it seems to be a common trend to have such tests on upper layers
> > (ATT/GATT also have such encryption key size), even though it is more
> > of a GAP test and perhaps could have been done at SMP level.
>
> .. however maybe we just deprecate BT_SECURITY or migrate it into something
> that allows specifying the details of a security level with extra
> parameters. I made the BT_SECURITY implementation in the kernel extendable.
> So we could also just add extra possible settings.
I'd not do it on per socket basis, pretty much the same as bluetoothd is not
handling keysize on per characteristic but as a global setting. If one needs
to use only full key size he will rather be set it globally. And apparently
no-one is using that anyway..
--
pozdrawiam
Szymon Janc
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
2021-03-09 10:27 ` Szymon Janc
@ 2021-03-09 18:35 ` Luiz Augusto von Dentz
2021-03-10 6:55 ` Marcel Holtmann
0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2021-03-09 18:35 UTC (permalink / raw)
To: Szymon Janc
Cc: Marcel Holtmann, Magdalena Kasenberg, Bluetooth Kernel Mailing List
Hi Szymon,
On Tue, Mar 9, 2021 at 2:27 AM Szymon Janc <szymon.janc@codecoup.pl> wrote:
>
> Hi Marcel, Luiz,
>
> On Saturday, 27 February 2021 21:14:57 CET Marcel Holtmann wrote:
> > Hi Luiz,
> >
> > >>> This is required for the qualification test L2CAP/LE/CFC/BV-15-C
> > >>>
> > >>> Implementation does not allow to set different key size for SMP and
> > >>> L2CAP, which is needed for a current specification of the test. This fix
> > >>> workarounds it with the debugfs variable le_l2cap_min_key_size.
> > >>>
> > >>> Logs from the test when the IUT uses a min and max l2cap encryption key
> > >>> size 16. $ echo 16 >
> > >>> /sys/kernel/debug/bluetooth/hci0/le_l2cap_min_key_size The lower tester
> > >>> uses a key size 7.
> > >>>
> > >>>> ACL Data RX: Handle 99 flags 0x02 dlen 11 #34 [hci0]
> > >>>> 25.007392>>>>
> > >>> SMP: Pairing Request (0x01) len 6
> > >>>
> > >>> IO capability: DisplayYesNo (0x01)
> > >>> OOB data: Authentication data not present (0x00)
> > >>> Authentication requirement: Bonding, No MITM, SC, No Keypresses
> > >>> (0x09)
> > >>> Max encryption key size: 7
> > >>> Initiator key distribution: <none> (0x00)
> > >>> Responder key distribution: <none> (0x00)
> > >>>
> > >>> < ACL Data TX: Handle 99 flags 0x00 dlen 11 #35 [hci0]
> > >>> 25.007591>>>
> > >>> SMP: Pairing Response (0x02) len 6
> > >>>
> > >>> IO capability: KeyboardDisplay (0x04)
> > >>> OOB data: Authentication data not present (0x00)
> > >>> Authentication requirement: Bonding, No MITM, SC, No Keypresses
> > >>> (0x09)
> > >>> Max encryption key size: 16
> > >>> Initiator key distribution: <none> (0x00)
> > >>> Responder key distribution: <none> (0x00)
> > >>>
> > >>> @ MGMT Event: New Long Term Key (0x000a) plen 37 {0x0001} [hci0]
> > >>> 28.788872>>>
> > >>> Store hint: Yes (0x01)
> > >>> LE Address: C0:DE:C0:FF:FF:01 (OUI C0-DE-C0)
> > >>> Key type: Unauthenticated key from P-256 (0x02)
> > >>> Master: 0x00
> > >>> Encryption size: 7
> > >>> Diversifier: 0000
> > >>> Randomizer: 0000000000000000
> > >>> Key: 529e11e8c7b9f5000000000000000000
> > >>>
> > >>> <snip>
> > >>>
> > >>> After pairing with key size 7, L2CAP connection is requested which
> > >>> requires key size 16.
> > >>>
> > >>>> ACL Data RX: Handle 99 flags 0x02 dlen 18 #56 [hci0]
> > >>>> 34.998084>>>>
> > >>> LE L2CAP: LE Connection Request (0x14) ident 3 len 10
> > >>>
> > >>> PSM: 244 (0x00f4)
> > >>> Source CID: 64
> > >>> MTU: 256
> > >>> MPS: 284
> > >>> Credits: 1
> > >>>
> > >>> < ACL Data TX: Handle 99 flags 0x00 dlen 18 #57 [hci0]
> > >>> 34.998325>>>
> > >>> LE L2CAP: LE Connection Response (0x15) ident 3 len 10
> > >>>
> > >>> Destination CID: 0
> > >>> MTU: 0
> > >>> MPS: 0
> > >>> Credits: 0
> > >>> Result: Connection refused - insufficient encryption key size
> > >>> (0x0007)
> > >>>
> > >>> Signed-off-by: Magdalena Kasenberg <magdalena.kasenberg@codecoup.pl>
> > >>> Reviewed-by: Szymon Janc <szymon.janc@codecoup.pl>
> > >>> Cc: Szymon Janc <szymon.janc@codecoup.pl>
> > >>> ---
> > >>> include/net/bluetooth/hci_core.h | 1 +
> > >>> net/bluetooth/hci_core.c | 1 +
> > >>> net/bluetooth/hci_debugfs.c | 30 ++++++++++++++++++++++++++++++
> > >>> net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++++++
> > >>> 4 files changed, 57 insertions(+)
> > >>>
> > >>> diff --git a/include/net/bluetooth/hci_core.h
> > >>> b/include/net/bluetooth/hci_core.h index ebdd4afe30d2..0bf0543efec5
> > >>> 100644
> > >>> --- a/include/net/bluetooth/hci_core.h
> > >>> +++ b/include/net/bluetooth/hci_core.h
> > >>> @@ -379,6 +379,7 @@ struct hci_dev {
> > >>>
> > >>> __u16 auth_payload_timeout;
> > >>> __u8 min_enc_key_size;
> > >>> __u8 max_enc_key_size;
> > >>>
> > >>> + __u8 le_l2cap_min_key_size;
> > >>>
> > >>> __u8 pairing_opts;
> > >>> __u8 ssp_debug_mode;
> > >>> __u8 hw_error_code;
> > >>>
> > >>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > >>> index b0d9c36acc03..9ef4b39b380c 100644
> > >>> --- a/net/bluetooth/hci_core.c
> > >>> +++ b/net/bluetooth/hci_core.c
> > >>> @@ -3788,6 +3788,7 @@ struct hci_dev *hci_alloc_dev(void)
> > >>>
> > >>> hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
> > >>> hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
> > >>> hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;
> > >>>
> > >>> + hdev->le_l2cap_min_key_size = HCI_MIN_ENC_KEY_SIZE;
> > >>
> > >> so I am not a fan of doing this with another variable and managing
> > >> through debugfs. Can we pass the qualification test case by using
> > >> BT_SECURITY_FIPS (which will enforce 128-bit key size)?>
> > > I guess that will depend if PTS supports MITM which afaik it is
> > > required with BT_SECURITY_FIPS, from the logs it looks like it doesn't
> > > support it so we end up with an unauthenticated key so the error would
> > > probably be different.
> >
> > we should give this a try ..
>
> PTS is not supporting GAP in this test at all, but since it is cat D test we
> can run it with our own LT (nimBLE).
>
> Using BT_SECURITY_FIPS will not do since it requires 128bit key to get to
> FIPS level so with lower key size it fails on SMP.
>
> BTW We are going to propose TSE which would allow to have alternative pass
> case in this test with early fail on SMP so that it can be handled as GAP
> configuration, not L2CAP. But for now, we have to handle it as defined in test
> spec.
Do we have a ticket for that already? It sounds rather strange that
test like this are being added when the security level already have
requirements for key size, it is as if the spec is testing that the
stack support the use of custom security level e.g. use level 3 but
requires 128-bit keys, which imo breaks the security level as one can
no longer expect the levels work the same regardless the
implementation.
> >
> > >> If not then we might want to add a socket option to set min/max
> > >> encryption key size requirement on a per socket basis.>
> > > Yep, it seems to be a common trend to have such tests on upper layers
> > > (ATT/GATT also have such encryption key size), even though it is more
> > > of a GAP test and perhaps could have been done at SMP level.
> >
> > .. however maybe we just deprecate BT_SECURITY or migrate it into something
> > that allows specifying the details of a security level with extra
> > parameters. I made the BT_SECURITY implementation in the kernel extendable.
> > So we could also just add extra possible settings.
>
> I'd not do it on per socket basis, pretty much the same as bluetoothd is not
> handling keysize on per characteristic but as a global setting. If one needs
> to use only full key size he will rather be set it globally. And apparently
> no-one is using that anyway..
IMO it shouldn't, the spec is very clear on the requirements for
security mode/level, adding different requirements on top of the level
is the real problem here and adding it may possible create IOP
problems as we are experiencing here since when we bump the security
level it is no longer able to pair so the whole idea of having a
dedicated error for key size seems flawed since it breaks the security
level model.
>
> --
> pozdrawiam
> Szymon Janc
>
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C
2021-03-09 18:35 ` Luiz Augusto von Dentz
@ 2021-03-10 6:55 ` Marcel Holtmann
0 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2021-03-10 6:55 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Szymon Janc, Magdalena Kasenberg, Bluetooth Kernel Mailing List
Hi Luiz,
>>>>>> This is required for the qualification test L2CAP/LE/CFC/BV-15-C
>>>>>>
>>>>>> Implementation does not allow to set different key size for SMP and
>>>>>> L2CAP, which is needed for a current specification of the test. This fix
>>>>>> workarounds it with the debugfs variable le_l2cap_min_key_size.
>>>>>>
>>>>>> Logs from the test when the IUT uses a min and max l2cap encryption key
>>>>>> size 16. $ echo 16 >
>>>>>> /sys/kernel/debug/bluetooth/hci0/le_l2cap_min_key_size The lower tester
>>>>>> uses a key size 7.
>>>>>>
>>>>>>> ACL Data RX: Handle 99 flags 0x02 dlen 11 #34 [hci0]
>>>>>>> 25.007392>>>>
>>>>>> SMP: Pairing Request (0x01) len 6
>>>>>>
>>>>>> IO capability: DisplayYesNo (0x01)
>>>>>> OOB data: Authentication data not present (0x00)
>>>>>> Authentication requirement: Bonding, No MITM, SC, No Keypresses
>>>>>> (0x09)
>>>>>> Max encryption key size: 7
>>>>>> Initiator key distribution: <none> (0x00)
>>>>>> Responder key distribution: <none> (0x00)
>>>>>>
>>>>>> < ACL Data TX: Handle 99 flags 0x00 dlen 11 #35 [hci0]
>>>>>> 25.007591>>>
>>>>>> SMP: Pairing Response (0x02) len 6
>>>>>>
>>>>>> IO capability: KeyboardDisplay (0x04)
>>>>>> OOB data: Authentication data not present (0x00)
>>>>>> Authentication requirement: Bonding, No MITM, SC, No Keypresses
>>>>>> (0x09)
>>>>>> Max encryption key size: 16
>>>>>> Initiator key distribution: <none> (0x00)
>>>>>> Responder key distribution: <none> (0x00)
>>>>>>
>>>>>> @ MGMT Event: New Long Term Key (0x000a) plen 37 {0x0001} [hci0]
>>>>>> 28.788872>>>
>>>>>> Store hint: Yes (0x01)
>>>>>> LE Address: C0:DE:C0:FF:FF:01 (OUI C0-DE-C0)
>>>>>> Key type: Unauthenticated key from P-256 (0x02)
>>>>>> Master: 0x00
>>>>>> Encryption size: 7
>>>>>> Diversifier: 0000
>>>>>> Randomizer: 0000000000000000
>>>>>> Key: 529e11e8c7b9f5000000000000000000
>>>>>>
>>>>>> <snip>
>>>>>>
>>>>>> After pairing with key size 7, L2CAP connection is requested which
>>>>>> requires key size 16.
>>>>>>
>>>>>>> ACL Data RX: Handle 99 flags 0x02 dlen 18 #56 [hci0]
>>>>>>> 34.998084>>>>
>>>>>> LE L2CAP: LE Connection Request (0x14) ident 3 len 10
>>>>>>
>>>>>> PSM: 244 (0x00f4)
>>>>>> Source CID: 64
>>>>>> MTU: 256
>>>>>> MPS: 284
>>>>>> Credits: 1
>>>>>>
>>>>>> < ACL Data TX: Handle 99 flags 0x00 dlen 18 #57 [hci0]
>>>>>> 34.998325>>>
>>>>>> LE L2CAP: LE Connection Response (0x15) ident 3 len 10
>>>>>>
>>>>>> Destination CID: 0
>>>>>> MTU: 0
>>>>>> MPS: 0
>>>>>> Credits: 0
>>>>>> Result: Connection refused - insufficient encryption key size
>>>>>> (0x0007)
>>>>>>
>>>>>> Signed-off-by: Magdalena Kasenberg <magdalena.kasenberg@codecoup.pl>
>>>>>> Reviewed-by: Szymon Janc <szymon.janc@codecoup.pl>
>>>>>> Cc: Szymon Janc <szymon.janc@codecoup.pl>
>>>>>> ---
>>>>>> include/net/bluetooth/hci_core.h | 1 +
>>>>>> net/bluetooth/hci_core.c | 1 +
>>>>>> net/bluetooth/hci_debugfs.c | 30 ++++++++++++++++++++++++++++++
>>>>>> net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++++++
>>>>>> 4 files changed, 57 insertions(+)
>>>>>>
>>>>>> diff --git a/include/net/bluetooth/hci_core.h
>>>>>> b/include/net/bluetooth/hci_core.h index ebdd4afe30d2..0bf0543efec5
>>>>>> 100644
>>>>>> --- a/include/net/bluetooth/hci_core.h
>>>>>> +++ b/include/net/bluetooth/hci_core.h
>>>>>> @@ -379,6 +379,7 @@ struct hci_dev {
>>>>>>
>>>>>> __u16 auth_payload_timeout;
>>>>>> __u8 min_enc_key_size;
>>>>>> __u8 max_enc_key_size;
>>>>>>
>>>>>> + __u8 le_l2cap_min_key_size;
>>>>>>
>>>>>> __u8 pairing_opts;
>>>>>> __u8 ssp_debug_mode;
>>>>>> __u8 hw_error_code;
>>>>>>
>>>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>>>>> index b0d9c36acc03..9ef4b39b380c 100644
>>>>>> --- a/net/bluetooth/hci_core.c
>>>>>> +++ b/net/bluetooth/hci_core.c
>>>>>> @@ -3788,6 +3788,7 @@ struct hci_dev *hci_alloc_dev(void)
>>>>>>
>>>>>> hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
>>>>>> hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
>>>>>> hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;
>>>>>>
>>>>>> + hdev->le_l2cap_min_key_size = HCI_MIN_ENC_KEY_SIZE;
>>>>>
>>>>> so I am not a fan of doing this with another variable and managing
>>>>> through debugfs. Can we pass the qualification test case by using
>>>>> BT_SECURITY_FIPS (which will enforce 128-bit key size)?>
>>>> I guess that will depend if PTS supports MITM which afaik it is
>>>> required with BT_SECURITY_FIPS, from the logs it looks like it doesn't
>>>> support it so we end up with an unauthenticated key so the error would
>>>> probably be different.
>>>
>>> we should give this a try ..
>>
>> PTS is not supporting GAP in this test at all, but since it is cat D test we
>> can run it with our own LT (nimBLE).
>>
>> Using BT_SECURITY_FIPS will not do since it requires 128bit key to get to
>> FIPS level so with lower key size it fails on SMP.
>>
>> BTW We are going to propose TSE which would allow to have alternative pass
>> case in this test with early fail on SMP so that it can be handled as GAP
>> configuration, not L2CAP. But for now, we have to handle it as defined in test
>> spec.
>
> Do we have a ticket for that already? It sounds rather strange that
> test like this are being added when the security level already have
> requirements for key size, it is as if the spec is testing that the
> stack support the use of custom security level e.g. use level 3 but
> requires 128-bit keys, which imo breaks the security level as one can
> no longer expect the levels work the same regardless the
> implementation.
>
>>>
>>>>> If not then we might want to add a socket option to set min/max
>>>>> encryption key size requirement on a per socket basis.>
>>>> Yep, it seems to be a common trend to have such tests on upper layers
>>>> (ATT/GATT also have such encryption key size), even though it is more
>>>> of a GAP test and perhaps could have been done at SMP level.
>>>
>>> .. however maybe we just deprecate BT_SECURITY or migrate it into something
>>> that allows specifying the details of a security level with extra
>>> parameters. I made the BT_SECURITY implementation in the kernel extendable.
>>> So we could also just add extra possible settings.
>>
>> I'd not do it on per socket basis, pretty much the same as bluetoothd is not
>> handling keysize on per characteristic but as a global setting. If one needs
>> to use only full key size he will rather be set it globally. And apparently
>> no-one is using that anyway..
>
> IMO it shouldn't, the spec is very clear on the requirements for
> security mode/level, adding different requirements on top of the level
> is the real problem here and adding it may possible create IOP
> problems as we are experiencing here since when we bump the security
> level it is no longer able to pair so the whole idea of having a
> dedicated error for key size seems flawed since it breaks the security
> level model.
I have the feeling that it might be better to add an option to also define the security level via socket options. So you can either use the pre-defined ones or just build your own. That way we can add new levels via userspace changes. I am almost certain that with future specs, we have to add new security levels since everybody is pondering on legacy pairing etc. So the FIPS level might be just too much, but you want Secure Connections and 128-bit key size, but are fine with just works.
And I wouldn’t be surprised if an upgrade from AES-128 has to happen at some point as well.
Regards
Marcel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-10 6:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-22 10:30 [PATCH] Bluetooth: Fix for L2CAP/LE/CFC/BV-15-C Magdalena Kasenberg
2021-02-22 11:06 ` bluez.test.bot
2021-02-22 17:46 ` [PATCH] " Luiz Augusto von Dentz
2021-02-26 20:17 ` Marcel Holtmann
2021-02-26 21:42 ` Luiz Augusto von Dentz
2021-02-27 20:14 ` Marcel Holtmann
2021-03-09 10:27 ` Szymon Janc
2021-03-09 18:35 ` Luiz Augusto von Dentz
2021-03-10 6:55 ` Marcel Holtmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.