Hi,
During tests it was found a problem that when using the mgmt to pair
devices that only support Legacy Pairing, it was not working. Here are
some logs[1].
The problem is that pairing was using the HIGH security level, which
for devices without SSP support means using a 16 digit pin code. Which
is unacceptable in most scenarios.
One question that might be asked is why this isn't a problem when not
using the mgmt interface. The problem doesn't happen when using the
hciops backend because the way that the pin code is requested in
userspace (the interesting bit is at plugins/hciops.c:1348, inside
pin_code_request()), userspace at that point has no information if the
pin code request should be marked as "secure" or not.
The solution for this problem is divided in two main parts, adding one
more condition for deciding whether to authenticate the link, if the
link requires MITM protection no matter the security level we should
authenticate the link. The other part is to decrease the requested
security level when pairing using the mgmt interface.
Cheers,
--
[1] http://littlechina.org/~vcgomes/hcidump-master.txt
http://littlechina.org/~vcgomes/hcidump-slave.txt
http://littlechina.org/~vcgomes/kernel-log.txt
Vinicius Costa Gomes (3):
Bluetooth: Require authentication if MITM protection is requested
Bluetooth: Fix not sending a Link Key Negative Reply
Bluetooth: Use the MEDIUM security level for pairings
net/bluetooth/hci_event.c | 13 ++++++++-----
net/bluetooth/mgmt.c | 8 +++-----
2 files changed, 11 insertions(+), 10 deletions(-)
--
1.7.6.1
Hi Vinicius,
* Vinicius Costa Gomes <[email protected]> [2011-09-02 14:51:22 -0300]:
> This lifts the requirement of 16 digits pin codes when pairing
> with devices that do not support SSP when using the mgmt interface.
>
> Signed-off-by: Vinicius Costa Gomes <[email protected]>
> ---
> net/bluetooth/mgmt.c | 8 +++-----
> 1 files changed, 3 insertions(+), 5 deletions(-)
Patches 1 and 3 area applied. Thanks.
Gustavo
Hi Johan,
On 11:18 Mon 05 Sep, Johan Hedberg wrote:
> Hi Vinicius,
>
> On Fri, Sep 02, 2011, Vinicius Costa Gomes wrote:
> > In the case that there are no keys loaded in the adapter, we should
> > also respond the Link Key Request event with the Link Key Negative
> > Reply command.
> >
> > Signed-off-by: Vinicius Costa Gomes <[email protected]>
> > ---
> > net/bluetooth/hci_event.c | 8 +++++---
> > 1 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 8404cd9..79f89d2 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -2208,11 +2208,13 @@ static inline void hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff
> >
> > BT_DBG("%s", hdev->name);
> >
> > - if (!test_bit(HCI_LINK_KEYS, &hdev->flags))
> > - return;
> > -
> > hci_dev_lock(hdev);
> >
> > + if (!test_bit(HCI_LINK_KEYS, &hdev->flags)) {
> > + BT_DBG("%s has no keys", hdev->name);
> > + goto not_found;
> > + }
> > +
> > key = hci_find_link_key(hdev, &ev->bdaddr);
> > if (!key) {
> > BT_DBG("%s link key not found for %s", hdev->name,
> > --
> > 1.7.6.1
>
> Nack. The purpose of this check in the beginning of the function is to
> prevent breakage on systems that do not use the management interface.
> With your patch you'd break all systems that use hciops. The
> HCI_LINK_KEYS bit will only be set when user space sends the
> MGMT_OP_LOAD_KEYS command.
Ok. This patch is to be ignored then. The rest of the series should should
still be considered.
>
> Johan
Cheers,
--
Vinicius
Hi Vinicius,
On Fri, Sep 02, 2011, Vinicius Costa Gomes wrote:
> In the case that there are no keys loaded in the adapter, we should
> also respond the Link Key Request event with the Link Key Negative
> Reply command.
>
> Signed-off-by: Vinicius Costa Gomes <[email protected]>
> ---
> net/bluetooth/hci_event.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 8404cd9..79f89d2 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2208,11 +2208,13 @@ static inline void hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff
>
> BT_DBG("%s", hdev->name);
>
> - if (!test_bit(HCI_LINK_KEYS, &hdev->flags))
> - return;
> -
> hci_dev_lock(hdev);
>
> + if (!test_bit(HCI_LINK_KEYS, &hdev->flags)) {
> + BT_DBG("%s has no keys", hdev->name);
> + goto not_found;
> + }
> +
> key = hci_find_link_key(hdev, &ev->bdaddr);
> if (!key) {
> BT_DBG("%s link key not found for %s", hdev->name,
> --
> 1.7.6.1
Nack. The purpose of this check in the beginning of the function is to
prevent breakage on systems that do not use the management interface.
With your patch you'd break all systems that use hciops. The
HCI_LINK_KEYS bit will only be set when user space sends the
MGMT_OP_LOAD_KEYS command.
Johan
This lifts the requirement of 16 digits pin codes when pairing
with devices that do not support SSP when using the mgmt interface.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
net/bluetooth/mgmt.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4f91a00..4796930 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1365,13 +1365,11 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len)
hci_dev_lock_bh(hdev);
- if (cp->io_cap == 0x03) {
- sec_level = BT_SECURITY_MEDIUM;
+ sec_level = BT_SECURITY_MEDIUM;
+ if (cp->io_cap == 0x03)
auth_type = HCI_AT_DEDICATED_BONDING;
- } else {
- sec_level = BT_SECURITY_HIGH;
+ else
auth_type = HCI_AT_DEDICATED_BONDING_MITM;
- }
entry = hci_find_adv_entry(hdev, &cp->bdaddr);
if (entry)
--
1.7.6.1
The HIGH security level requires a 16 digit pin code for non-SSP
bondings. Sometimes this requirement is not acceptable and we still
want protection againts MITM attacks (which is something that the
MEDIUM security level doesn't provide), for that we should allow
another way to request authentication without using the HIGH security
level.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
net/bluetooth/hci_event.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8483cab..8404cd9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1104,9 +1104,10 @@ static int hci_outgoing_auth_needed(struct hci_dev *hdev,
return 0;
/* Only request authentication for SSP connections or non-SSP
- * devices with sec_level HIGH */
+ * devices with sec_level HIGH or if MITM protection is requested */
if (!(hdev->ssp_mode > 0 && conn->ssp_mode > 0) &&
- conn->pending_sec_level != BT_SECURITY_HIGH)
+ conn->pending_sec_level != BT_SECURITY_HIGH &&
+ !(conn->auth_type & 0x01))
return 0;
return 1;
--
1.7.6.1
In the case that there are no keys loaded in the adapter, we should
also respond the Link Key Request event with the Link Key Negative
Reply command.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
net/bluetooth/hci_event.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8404cd9..79f89d2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2208,11 +2208,13 @@ static inline void hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff
BT_DBG("%s", hdev->name);
- if (!test_bit(HCI_LINK_KEYS, &hdev->flags))
- return;
-
hci_dev_lock(hdev);
+ if (!test_bit(HCI_LINK_KEYS, &hdev->flags)) {
+ BT_DBG("%s has no keys", hdev->name);
+ goto not_found;
+ }
+
key = hci_find_link_key(hdev, &ev->bdaddr);
if (!key) {
BT_DBG("%s link key not found for %s", hdev->name,
--
1.7.6.1