endless loop in client.c when read request is rejected

Hi,

 While running an IOP between blueZ and a LE device, I found that if
the LE device rejects the read_char_req with error_response
(insufficient Encryption), the following code will make the read_req
an endless loop,

	if (status == 0)
		characteristic_set_value(chr, pdu + 1, len - 1);
	else if (status == ATT_ECODE_INSUFF_ENC) {
		GIOChannel *io = g_attrib_get_channel(gatt->attrib);

		if (bt_io_set(io, BT_IO_L2CAP, NULL,
				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_HIGH,
				BT_IO_OPT_INVALID)) {

                        //End Less loop here
			gatt_read_char(gatt->attrib, chr->handle, 0,
					update_char_value, current);
			return;
		}
	}

   I am not sure what effect bt_io_set should have to improve the
security (bonding?), but the code simply has no effect in my
environment (with 2.6.39.1 kernel and latest bluez), therefore the
read_req, error_response will continue until link is disconnected.

Regards,

Mike

Hi Mike,

On Fri, Jul 1, 2011 at 6:06 PM, mike tsai [off-list ref] wrote:

  I am not sure what effect bt_io_set should have to improve the
security (bonding?), but the code simply has no effect in my
environment (with 2.6.39.1 kernel and latest bluez), therefore the
read_req, error_response will continue until link is disconnected.

bt_io_set(..., BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_HIGH, ...) will trigger
SMP pairing (and block further writes until pairing is complete). I
believe you see the loop because your kernel does not have SMP
support, which has been added upstream only a few weeks ago (so should
come only on 3.0, I believe). Therefore SMP does never happen in your
case.

Try compiling a recent development kernel. I recommend Gustavo
Padovan's tree (master branch):
git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-next-2.6.git

HTH,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Hi again,

On Fri, Jul 1, 2011 at 7:38 PM, Anderson Lizardo
[off-list ref] wrote:

Hi Mike,

On Fri, Jul 1, 2011 at 6:06 PM, mike tsai [off-list ref] wrote:

quoted

  I am not sure what effect bt_io_set should have to improve the
security (bonding?), but the code simply has no effect in my
environment (with 2.6.39.1 kernel and latest bluez), therefore the
read_req, error_response will continue until link is disconnected.

bt_io_set(..., BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_HIGH, ...) will trigger
SMP pairing (and block further writes until pairing is complete). I
believe you see the loop because your kernel does not have SMP
support, which has been added upstream only a few weeks ago (so should
come only on 3.0, I believe). Therefore SMP does never happen in your
case.

Forgot to mention: I still think we need a fix on BlueZ though, which
cannot assume SMP pairing will succeed. Therefore we should have a way
to abort the write attempt.

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Hi Anderson,

On Fri, Jul 1, 2011 at 4:40 PM, Anderson Lizardo
[off-list ref] wrote:

Hi again,

On Fri, Jul 1, 2011 at 7:38 PM, Anderson Lizardo
[off-list ref] wrote:

quoted

Hi Mike,

On Fri, Jul 1, 2011 at 6:06 PM, mike tsai [off-list ref] wrote:

quoted

  I am not sure what effect bt_io_set should have to improve the
security (bonding?), but the code simply has no effect in my
environment (with 2.6.39.1 kernel and latest bluez), therefore the
read_req, error_response will continue until link is disconnected.

bt_io_set(..., BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_HIGH, ...) will trigger
SMP pairing (and block further writes until pairing is complete). I
believe you see the loop because your kernel does not have SMP
support, which has been added upstream only a few weeks ago (so should
come only on 3.0, I believe). Therefore SMP does never happen in your
case.

Forgot to mention: I still think we need a fix on BlueZ though, which
cannot assume SMP pairing will succeed. Therefore we should have a way
to abort the write attempt.

After upgrade the kernel to 2.6-next branch, it does initiate the
just-work, no-bonding operation and it does work.(no more endless
loop). However, I found another issue is that during discovery, the
client will try to read every characteristic it found, regardless the
property setting. This causes "Read Not Permitted" error response for
some characteristic that does not allowed to be read (like the
alert-level). I am not sure why the discovery process wants to read
the characteristic value though, that should be invoked by profile
later when needed,

Regards,

Mike

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Hi Mike,

After upgrade the kernel to 2.6-next branch, it does initiate the
just-work, no-bonding operation and it does work.(no more endless
loop). However, I found another issue is that during discovery, the
client will try to read every characteristic it found, regardless the
property setting. This causes "Read Not Permitted" error response for
some characteristic that does not allowed to be read (like the
alert-level). I am not sure why the discovery process wants to read
the characteristic value though, that should be invoked by profile
later when needed,

Regards,

Mike

The Generic Attribute is still experimental. DiscoverCharacteristics
method returns after discovering all characteristic declarations,
characteristic values and descriptor are read in "background".

"Read Not Permitted" needs to be fixed. Feel free to send a patch for it.

The characteristic value is required due the GetProperties method of
the characteristic "object". At the moment, the characteristics
storage is not being used properly. For example, Proximity could read
the handle from the storage if available.

Regards,
Claudio.