RE: [PATCH] Bluetooth: Link Keys should be stored if MITM is not required

[Vishal AGARWAL <vishal.agarwal@xxxxxxxxxxxxxx>]

Hi Johan,

I am testing with PTS. I have attached the HCI dump also for this case.

Also pls refer to function "link_key_request" in file hciops.c. It also has the same kind of implementation.

	/* Don't use unauthenticated combination keys if MITM is
	 * required */
	if (key_info->type == 0x04 && conn->loc_auth != 0xff &&
						(conn->loc_auth & 0x01))
		hci_send_cmd(dev->sk, OGF_LINK_CTL, OCF_LINK_KEY_NEG_REPLY,
								6, dba);
	else if (key_info->type == 0x00 &&
				sec_level == BT_SECURITY_HIGH &&
				key_info->pin_len <16) {
		hci_send_cmd(dev->sk, OGF_LINK_CTL, OCF_LINK_KEY_NEG_REPLY,
								6, dba);
	} else {
		link_key_reply_cp lr;

		memcpy(lr.link_key, key_info->key, 16);
		bacpy(&lr.bdaddr, dba);

		hci_send_cmd(dev->sk, OGF_LINK_CTL, OCF_LINK_KEY_REPLY,
						LINK_KEY_REPLY_CP_SIZE, &lr);
	}

Same PTS setup is passing if we use hci_ops instead of mgmt_ops because of the first check in which it checks if for MITM (conn->loc_auth & 0x01).
And if MITM is not required then key of type 04 (UNAUTHENTICATED_COMBINATION_KEY) will also work.

In case you are not able to open logs in this format, pls let me know. I will provide you raw HCI dump.

Thanks
Vishal

-----Original Message-----
From: Johan Hedberg [mailto:johan.hedberg@xxxxxxxxx] 
Sent: Tuesday, April 03, 2012 3:08 PM
To: Vishal AGARWAL
Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Naresh-kumar GUPTA
Subject: Re: [PATCH] Bluetooth: Link Keys should be stored if MITM is not required

Hi,

On Tue, Apr 03, 2012, Vishal Agarwal wrote:
> If MITM protection is not required then except for Debug Keys, all
> link keys should be persistent. And they should be stored for future
> use.
> 
> Change-Id: Id438d424b999e9a30f29193d02ac266bee5f672b
> Signed-off-by: Vishal Agarwal <vishal.agarwal@xxxxxxxxxxxxxx>
> ---
>  net/bluetooth/hci_core.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index c5ee97c..bcb68dd 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1246,6 +1246,10 @@ static int hci_persistent_key(struct hci_dev *hdev, struct hci_conn *conn,
>  	if (conn->remote_auth == 0x02 || conn->remote_auth == 0x03)
>  		return 1;
>  
> +	/* If MITM is not required then store the Link Key */
> +	if (!(conn->auth_type & 0x01))
> +		return 1;
> +
>  	/* If none of the above criteria match, then don't store the key
>  	 * persistently */
>  	return 0;

Nack.

This doesn't make much sense to me. Why should the MITM flag have
anything to do with the persistency of the key?

This looks more like a workaround for some device that is incorrectly
having a no-bonding requirement (which means that we should *not* store
the key). Please describe what kind of setup you've seen this with and
include a hcidump for it showing the local and remote authentication
requirement and IO capabilities.

Johan

Attachment: cfa.cfa
Description: cfa.cfa