2020-04-04 00:05:23

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH] Bluetooth: Always request for user confirmation for Just Works (LE SC)

To improve security, always give the user-space daemon a chance to
accept or reject a Just Works pairing (LE). The daemon may decide to
auto-accept based on the user's intent.

This patch is similar to the previous patch but applies for LE Secure
Connections (SC).

Signed-off-by: Sonny Sasaka <[email protected]>
---
net/bluetooth/smp.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index d0b695ee49f6..daf03339dedd 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -2202,7 +2202,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
if (err)
return SMP_UNSPECIFIED;

- if (smp->method == JUST_WORKS || smp->method == REQ_OOB) {
+ if (smp->method == REQ_OOB) {
if (hcon->out) {
sc_dhkey_check(smp);
SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK);
@@ -2210,6 +2210,18 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
return 0;
}

+ /* If Just Works, ask user-space for confirmation. */
+ if (smp->method == JUST_WORKS) {
+ err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst,
+ hcon->type, hcon->dst_type, passkey, 1);
+ if (err)
+ return SMP_UNSPECIFIED;
+
+ set_bit(SMP_FLAG_WAIT_USER, &smp->flags);
+
+ return 0;
+ }
+
err = smp_g2(smp->tfm_cmac, pkax, pkbx, na, nb, &passkey);
if (err)
return SMP_UNSPECIFIED;
--
2.17.1


2020-04-06 12:05:04

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Always request for user confirmation for Just Works (LE SC)

Hi Sonny,

> To improve security, always give the user-space daemon a chance to
> accept or reject a Just Works pairing (LE). The daemon may decide to
> auto-accept based on the user's intent.
>
> This patch is similar to the previous patch but applies for LE Secure
> Connections (SC).
>
> Signed-off-by: Sonny Sasaka <[email protected]>
> ---
> net/bluetooth/smp.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index d0b695ee49f6..daf03339dedd 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -2202,7 +2202,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
> if (err)
> return SMP_UNSPECIFIED;
>
> - if (smp->method == JUST_WORKS || smp->method == REQ_OOB) {
> + if (smp->method == REQ_OOB) {
> if (hcon->out) {
> sc_dhkey_check(smp);
> SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK);
> @@ -2210,6 +2210,18 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
> return 0;
> }
>
> + /* If Just Works, ask user-space for confirmation. */
> + if (smp->method == JUST_WORKS) {
> + err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst,
> + hcon->type, hcon->dst_type, passkey, 1);
> + if (err)
> + return SMP_UNSPECIFIED;
> +
> + set_bit(SMP_FLAG_WAIT_USER, &smp->flags);
> +
> + return 0;
> + }
> +
> err = smp_g2(smp->tfm_cmac, pkax, pkbx, na, nb, &passkey);
> if (err)
> return SMP_UNSPECIFIED;

@@ -2202,7 +2204,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
if (err)
return SMP_UNSPECIFIED;

- if (smp->method == JUST_WORKS || smp->method == REQ_OOB) {
+ if (smp->method == REQ_OOB) {
if (hcon->out) {
sc_dhkey_check(smp);
SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK);
@@ -2214,7 +2216,10 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
if (err)
return SMP_UNSPECIFIED;

- confirm_hint = 0;
+ if (smp->method == JUST_WORKS)
+ confirm_hint = 0;
+ else
+ confirm_hint = 1;

confirm:
err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, hcon->type,

Isn’t this what you are actually doing (minus the required comment of course)?

Regards

Marcel

2020-04-06 18:05:10

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH v2] Bluetooth: Always request for user confirmation for Just Works (LE SC)

To improve security, always give the user-space daemon a chance to
accept or reject a Just Works pairing (LE). The daemon may decide to
auto-accept based on the user's intent.

This patch is similar to the previous patch but applies for LE Secure
Connections (SC).

Signed-off-by: Sonny Sasaka <[email protected]>
---
net/bluetooth/smp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index d0b695ee49f6..2f48518d120b 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -2202,7 +2202,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
if (err)
return SMP_UNSPECIFIED;

- if (smp->method == JUST_WORKS || smp->method == REQ_OOB) {
+ if (smp->method == REQ_OOB) {
if (hcon->out) {
sc_dhkey_check(smp);
SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK);
@@ -2217,6 +2217,9 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
confirm_hint = 0;

confirm:
+ if (smp->method == JUST_WORKS)
+ confirm_hint = 1;
+
err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, hcon->type,
hcon->dst_type, passkey, confirm_hint);
if (err)
--
2.17.1

2020-04-06 18:08:51

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: Always request for user confirmation for Just Works (LE SC)

Hi Marcel,

Thanks for the suggestion. I have sent an updated patch based on your
suggestion with a little modification. Let me know if this looks good.
Thanks!

On Mon, Apr 6, 2020 at 11:04 AM Sonny Sasaka <[email protected]> wrote:
>
> To improve security, always give the user-space daemon a chance to
> accept or reject a Just Works pairing (LE). The daemon may decide to
> auto-accept based on the user's intent.
>
> This patch is similar to the previous patch but applies for LE Secure
> Connections (SC).
>
> Signed-off-by: Sonny Sasaka <[email protected]>
> ---
> net/bluetooth/smp.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index d0b695ee49f6..2f48518d120b 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -2202,7 +2202,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
> if (err)
> return SMP_UNSPECIFIED;
>
> - if (smp->method == JUST_WORKS || smp->method == REQ_OOB) {
> + if (smp->method == REQ_OOB) {
> if (hcon->out) {
> sc_dhkey_check(smp);
> SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK);
> @@ -2217,6 +2217,9 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
> confirm_hint = 0;
>
> confirm:
> + if (smp->method == JUST_WORKS)
> + confirm_hint = 1;
> +
> err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, hcon->type,
> hcon->dst_type, passkey, confirm_hint);
> if (err)
> --
> 2.17.1
>

2020-04-08 19:52:13

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: Always request for user confirmation for Just Works (LE SC)

Hi Marcel,

Could you please take another look at this v2 patch based on your
suggestions? Thanks.

On Mon, Apr 6, 2020 at 11:04 AM Sonny Sasaka <[email protected]> wrote:
>
> To improve security, always give the user-space daemon a chance to
> accept or reject a Just Works pairing (LE). The daemon may decide to
> auto-accept based on the user's intent.
>
> This patch is similar to the previous patch but applies for LE Secure
> Connections (SC).
>
> Signed-off-by: Sonny Sasaka <[email protected]>
> ---
> net/bluetooth/smp.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index d0b695ee49f6..2f48518d120b 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -2202,7 +2202,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
> if (err)
> return SMP_UNSPECIFIED;
>
> - if (smp->method == JUST_WORKS || smp->method == REQ_OOB) {
> + if (smp->method == REQ_OOB) {
> if (hcon->out) {
> sc_dhkey_check(smp);
> SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK);
> @@ -2217,6 +2217,9 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
> confirm_hint = 0;
>
> confirm:
> + if (smp->method == JUST_WORKS)
> + confirm_hint = 1;
> +
> err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, hcon->type,
> hcon->dst_type, passkey, confirm_hint);
> if (err)
> --
> 2.17.1
>

2020-04-08 20:48:19

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: Always request for user confirmation for Just Works (LE SC)

Hi Sonny,

> To improve security, always give the user-space daemon a chance to
> accept or reject a Just Works pairing (LE). The daemon may decide to
> auto-accept based on the user's intent.
>
> This patch is similar to the previous patch but applies for LE Secure
> Connections (SC).
>
> Signed-off-by: Sonny Sasaka <[email protected]>
> ---
> net/bluetooth/smp.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel