2011-06-01 15:28:47

by Rymarkiewicz Waldemar

Subject: [PATCH 1/2] Bluetooth: Remove a magic number

Signed-off-by: Waldemar Rymarkiewicz <[email protected]>
---
net/bluetooth/mgmt.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fe835b8..78d1a5f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1143,7 +1143,7 @@ static int pin_code_reply(struct sock *sk, u16 index, unsigned char *data,

bacpy(&reply.bdaddr, &cp->bdaddr);
reply.pin_len = cp->pin_len;
- memcpy(reply.pin_code, cp->pin_code, 16);
+ memcpy(reply.pin_code, cp->pin_code, sizeof(reply.pin_code));

err = hci_send_cmd(hdev, HCI_OP_PIN_CODE_REPLY, sizeof(reply), &reply);
if (err < 0)
--
1.7.4.1

2011-06-02 08:27:09

by Rymarkiewicz Waldemar

Subject: RE: [PATCH 2/2] Bluetooth: Verify a pin code in pin_code_reply

Hi Gustavo,

>> struct hci_dev *hdev;
>> + struct hci_conn *conn;
>> struct mgmt_cp_pin_code_reply *cp;
>> + struct mgmt_cp_pin_code_neg_reply ncp;
>> struct hci_cp_pin_code_reply reply;
>> struct pending_cmd *cmd;
>
>please pay more attention on the compiler warnings. cmd is
>unused here. I fixed it up.

Right, but gcc 4.5 doesn't show me that. Will update to 4.6 then.

Thanks,
Waldek

2011-06-01 20:51:19

by Gustavo Padovan

Subject: Re: [PATCH 2/2] Bluetooth: Verify a pin code in pin_code_reply

Hi Waldemar,

* Waldemar Rymarkiewicz <[email protected]> [2011-06-01 17:28:48 +0200]:

> As we cannot relay on a userspace mgmt api implementation we should verify
> if pin_code_reply in fact contains the secure pin code.
>
> If userspace replied with unsecure pincode when secure was required we will
> send pin_code_neg_reply to the controller.
>
> Signed-off-by: Waldemar Rymarkiewicz <[email protected]>
> ---
> net/bluetooth/mgmt.c | 50 +++++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 39 insertions(+), 11 deletions(-)

Applied, but...

>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 78d1a5f..7cdf606 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1108,11 +1108,32 @@ unlock:
> return err;
> }
>
> +static int send_pin_code_neg_reply(struct sock *sk, u16 index,
> + struct hci_dev *hdev, struct mgmt_cp_pin_code_neg_reply *cp)
> +{
> + struct pending_cmd *cmd;
> + int err;
> +
> + cmd = mgmt_pending_add(sk, MGMT_OP_PIN_CODE_NEG_REPLY, index, cp,
> + sizeof(*cp));
> + if (!cmd)
> + return -ENOMEM;
> +
> + err = hci_send_cmd(hdev, HCI_OP_PIN_CODE_NEG_REPLY, sizeof(cp->bdaddr),
> + &cp->bdaddr);
> + if (err < 0)
> + mgmt_pending_remove(cmd);
> +
> + return err;
> +}
> +
> static int pin_code_reply(struct sock *sk, u16 index, unsigned char *data,
> u16 len)
> {
> struct hci_dev *hdev;
> + struct hci_conn *conn;
> struct mgmt_cp_pin_code_reply *cp;
> + struct mgmt_cp_pin_code_neg_reply ncp;
> struct hci_cp_pin_code_reply reply;
> struct pending_cmd *cmd;

please pay more attention on the compiler warnings. cmd is unused here. I
fixed it up.

> int err;
> @@ -1135,6 +1156,23 @@ static int pin_code_reply(struct sock *sk, u16 index, unsigned char *data,
> goto failed;
> }
>
> + conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->bdaddr);
> + if (!conn) {
> + err = cmd_status(sk, index, MGMT_OP_PIN_CODE_REPLY, ENOTCONN);
> + goto failed;
> + }
> +
> + if (conn->pending_sec_level == BT_SECURITY_HIGH && cp->pin_len != 16) {
> + bacpy(&ncp.bdaddr, &cp->bdaddr);
> +

And I added a error messages here (suggestion from Johan)

--
Gustavo F. Padovan
http://profusion.mobi

2011-06-01 19:56:19

by Gustavo Padovan

Subject: Re: [PATCH 1/2] Bluetooth: Remove a magic number

* Waldemar Rymarkiewicz <[email protected]> [2011-06-01 17:28:47 +0200]:

> Signed-off-by: Waldemar Rymarkiewicz <[email protected]>
> ---
> net/bluetooth/mgmt.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

-
Gustavo F. Padovan
http://profusion.mobi

2011-06-01 15:28:48

by Rymarkiewicz Waldemar

Subject: [PATCH 2/2] Bluetooth: Verify a pin code in pin_code_reply

As we cannot relay on a userspace mgmt api implementation we should verify
if pin_code_reply in fact contains the secure pin code.

If userspace replied with unsecure pincode when secure was required we will
send pin_code_neg_reply to the controller.

Signed-off-by: Waldemar Rymarkiewicz <[email protected]>
---
net/bluetooth/mgmt.c | 50 +++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 78d1a5f..7cdf606 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1108,11 +1108,32 @@ unlock:
return err;
}

+static int send_pin_code_neg_reply(struct sock *sk, u16 index,
+ struct hci_dev *hdev, struct mgmt_cp_pin_code_neg_reply *cp)
+{
+ struct pending_cmd *cmd;
+ int err;
+
+ cmd = mgmt_pending_add(sk, MGMT_OP_PIN_CODE_NEG_REPLY, index, cp,
+ sizeof(*cp));
+ if (!cmd)
+ return -ENOMEM;
+
+ err = hci_send_cmd(hdev, HCI_OP_PIN_CODE_NEG_REPLY, sizeof(cp->bdaddr),
+ &cp->bdaddr);
+ if (err < 0)
+ mgmt_pending_remove(cmd);
+
+ return err;
+}
+
static int pin_code_reply(struct sock *sk, u16 index, unsigned char *data,
u16 len)
{
struct hci_dev *hdev;
+ struct hci_conn *conn;
struct mgmt_cp_pin_code_reply *cp;
+ struct mgmt_cp_pin_code_neg_reply ncp;
struct hci_cp_pin_code_reply reply;
struct pending_cmd *cmd;
int err;
@@ -1135,6 +1156,23 @@ static int pin_code_reply(struct sock *sk, u16 index, unsigned char *data,
goto failed;
}

+ conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->bdaddr);
+ if (!conn) {
+ err = cmd_status(sk, index, MGMT_OP_PIN_CODE_REPLY, ENOTCONN);
+ goto failed;
+ }
+
+ if (conn->pending_sec_level == BT_SECURITY_HIGH && cp->pin_len != 16) {
+ bacpy(&ncp.bdaddr, &cp->bdaddr);
+
+ err = send_pin_code_neg_reply(sk, index, hdev, &ncp);
+ if (err >= 0)
+ err = cmd_status(sk, index, MGMT_OP_PIN_CODE_REPLY,
+ EINVAL);
+
+ goto failed;
+ }
+
cmd = mgmt_pending_add(sk, MGMT_OP_PIN_CODE_REPLY, index, data, len);
if (!cmd) {
err = -ENOMEM;
@@ -1185,17 +1223,7 @@ static int pin_code_neg_reply(struct sock *sk, u16 index, unsigned char *data,
goto failed;
}

- cmd = mgmt_pending_add(sk, MGMT_OP_PIN_CODE_NEG_REPLY, index,
- data, len);
- if (!cmd) {
- err = -ENOMEM;
- goto failed;
- }
-
- err = hci_send_cmd(hdev, HCI_OP_PIN_CODE_NEG_REPLY, sizeof(cp->bdaddr),
- &cp->bdaddr);
- if (err < 0)
- mgmt_pending_remove(cmd);
+ err = send_pin_code_neg_reply(sk, index, hdev, cp);

failed:
hci_dev_unlock(hdev);
--
1.7.4.1