RE: [RFC v2 4/9] Bluetooth: simple SMP pairing negotiation

["Brian Gix" <bgix@codeaurora.org>]

Hi Vinicius,

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Vinicius Costa Gomes
> Sent: 06 December, 2010 1:44 PM
> To: linux-bluetooth@vger.kernel.org
> Cc: Anderson Briglia; Vinicius Costa Gomes
> Subject: [RFC v2 4/9] Bluetooth: simple SMP pairing negotiation
> 
> From: Anderson Briglia <anderson.briglia@openbossa.org>
> 
> This implementation only exchanges SMP messages between the Host and
> the
> Remote. No keys are being generated. TK and STK generation will be
> provided in further patches.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  net/bluetooth/l2cap_core.c |    3 +-
>  net/bluetooth/smp.c        |  114
> ++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 112 insertions(+), 5 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 674799c..da4f13d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4630,7 +4630,8 @@ static void l2cap_recv_frame(struct l2cap_conn
> *conn, struct sk_buff *skb)
>  		break;
> 
>  	case L2CAP_CID_SMP:
> -		smp_sig_channel(conn, skb);
> +		if (smp_sig_channel(conn, skb))
> +			l2cap_conn_del(conn->hcon, 0x05);
>  		break;
> 
>  	default:
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index e9dde5f..b25010f 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -64,6 +64,102 @@ static void smp_send_cmd(struct l2cap_conn *conn,
> u8 code, u16 len, void *data)
>  	hci_send_acl(conn->hcon, skb, 0);
>  }
> 
> +static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct
> sk_buff *skb)
> +{
> +	struct smp_cmd_pairing *rp = (void *) skb->data;
> +
> +	BT_DBG("");
> +
> +	skb_pull(skb, sizeof(struct smp_cmd_pairing));
> +
> +	rp->io_capability = 0x00;
> +	rp->oob_flag = 0x00;
> +	rp->max_key_size = 16;
> +	rp->init_key_dist = 0x00;
> +	rp->resp_key_dist = 0x00;
> +	rp->auth_req &= 0x05;
> +
> +	smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
> +}

As a "placeholder" I understand that there is a fair amount of fleshing
out that these changes need.  However, as you have an conn->hcon->out
flag that indicates direction (which hopefully is based on Link Master),
I would like to see checking in this function and next, that the
correct role has received these SMP packets, with a rejection if they
were received by the incorrect role. Also, although the placeholder is
requesting no key distribution, in the fleshed out version, the responder
should be returning the subset (logical AND) of the requesters and the
responders key_dist masks, which in this case is still of course Zero.

I'm sorry if this is to many comments for this starting point.

> +
> +static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct
> sk_buff *skb)
> +{
> +	struct smp_cmd_pairing_confirm cp;
> +
> +	BT_DBG("");
> +
> +	memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
> +
> +	smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
> +}
> +
> +static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct
> sk_buff *skb)
> +{
> +	BT_DBG("");
> +
> +	if (conn->hcon->out) {
> +		struct smp_cmd_pairing_random random;
> +
> +		BT_DBG("master");
> +
> +		memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
> +
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
> +								&random);
> +	} else {
> +		struct smp_cmd_pairing_confirm confirm;
> +
> +		BT_DBG("slave");
> +
> +		memset(&confirm, 0, sizeof(struct
> smp_cmd_pairing_confirm));
> +
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM,
> sizeof(confirm),
> +								&confirm);
> +	}
> +}
> +
> +static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct
> sk_buff *skb)
> +{
> +	struct smp_cmd_pairing_random cp;
> +
> +	BT_DBG("");
> +
> +	skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
> +
> +	/* FIXME: check if random matches */

The random numbers will not match. The correct check will be that
when the encryption with p1, p2, k, and the remote's random number,
is performed, that it matches the confirm previously received
via smp_cmd_pairing_confirm.

> +
> +	if (conn->hcon->out) {
> +		BT_DBG("master");
> +		/* FIXME: start encryption */
> +	} else {
> +		BT_DBG("slave");
> +
> +		memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
> +
> +		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp),
> &cp);
> +	}
> +}
> +
> +static void smp_cmd_security_req(struct l2cap_conn *conn, struct
> sk_buff *skb)
> +{
> +	struct smp_cmd_security_req *rp = (void *) skb->data;
> +	struct smp_cmd_pairing cp;
> +
> +	BT_DBG("");
> +
> +	skb_pull(skb, sizeof(struct smp_cmd_security_req));
> +	memset(&cp, 0, sizeof(struct smp_cmd_pairing));
> +
> +	cp.io_capability = 0x00;
> +	cp.oob_flag = 0x00;
> +	cp.max_key_size = 16;
> +	cp.init_key_dist = 0x00;
> +	cp.resp_key_dist = 0x00;
> +	cp.auth_req = rp->auth_req & 0x05;
> +
> +	smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
> +}
> +

This function may need to be overloaded, such that if an existing
set of keys already exist (from an earlier pairing) that they are
used by simply encrypting the link, or signing the WRITE_CMD pkt
as needed.  Should the link encryption fail due to remote rejection,
we might then request security, subject to the same limitations
used by BR/EDR's SSP. 

But I do not know where the division lies between the key storage dB,
the kernel mode code and the user mode code.


>  int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
>  {
>  	__u8 authreq;
> @@ -114,23 +210,33 @@ int smp_sig_channel(struct l2cap_conn *conn,
> struct sk_buff *skb)
> 
>  	switch (code) {
>  	case SMP_CMD_PAIRING_REQ:
> -		reason = SMP_PAIRING_NOTSUPP;
> -		smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
> -		err = -1;
> +		smp_cmd_pairing_req(conn, skb);
>  		break;
> 
>  	case SMP_CMD_PAIRING_FAIL:
>  		break;
> 
>  	case SMP_CMD_PAIRING_RSP:
> +		smp_cmd_pairing_rsp(conn, skb);
> +		break;
> +
> +	case SMP_CMD_SECURITY_REQ:
> +		smp_cmd_security_req(conn, skb);
> +		break;
> +
>  	case SMP_CMD_PAIRING_CONFIRM:
> +		smp_cmd_pairing_confirm(conn, skb);
> +		break;
> +
>  	case SMP_CMD_PAIRING_RANDOM:
> +		smp_cmd_pairing_random(conn, skb);
> +		break;
> +
>  	case SMP_CMD_ENCRYPT_INFO:
>  	case SMP_CMD_MASTER_IDENT:
>  	case SMP_CMD_IDENT_INFO:
>  	case SMP_CMD_IDENT_ADDR_INFO:
>  	case SMP_CMD_SIGN_INFO:
> -	case SMP_CMD_SECURITY_REQ:
>  	default:
>  		BT_DBG("Unknown command code 0x%2.2x", code);
> 
> --
> 1.7.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html