New issue
Advanced search Search tips

Issue 653718 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature

Blocking:
issue 510287



Sign in to add a comment

bluetooth: Allow name(Prefix) filters longer than 23 characters.

Project Member Reported by ortuno@chromium.org, Oct 6 2016

Issue description

We are mistakenly rejecting promises if the name filter is longer than 29 bytes. Devices can't advertise a name longer than 29 bytes but it's possible the device name that was acquired through gap.device_name and is longer than 29 bytes.
 

Comment 1 by scheib@chromium.org, Oct 17 2016

Labels: Type-Feature
Owner: jeffcarp@chromium.org
Status: Assigned (was: Available)
Does this involve removing the max length check or increasing the size of kMaxFilterNameLength?

Comment 4 by scheib@chromium.org, Dec 31 2016

Probably not completely unbounded, we should still have some kind of length constraint to avoid malicious use of the API sending unbounded data from the renderer.

To determine the length we should look at the bluetooth specification for what name lengths can be. https://www.bluetooth.com/specifications/assigned-numbers/generic-access-profile cites where in the spec this is (vol 3, C, 8.1.2) I tried to take a quick look at Core spec 5.0 https://www.bluetooth.com/specifications/adopted-specifications and it seems the length is limited by the EIR packet size (240 bytes).

It would be good to double check me there. I think we can then increase the kMaxFilterNameLength to that value and explain why it is that number and cite the specification source.

Thanks, in section 7.7.7 Remote Name Request Complete Event it states that the Remote Name parameter can be 248 bytes:

"A UTF-8 encoded user-friendly descriptive name for the remote device. If the name contained in the parameter is shorter than 248 octets, the end of the name is indicated by a NULL octet (0x00), and the following octets (to fill up 248 octets, which is the length of the parameter) do not have valid values."

Which is a bit confusing if the EIR packet size is 240 bytes. If the name is >240 bytes, are the last 8 bytes simply omitted? Looking at (vol 3, C, 8) it seems that way:

"If applications on a Host provide more than 240 bytes of extended inquiry response data, it is up to the Host to limit it to 240 octets."

I'm new to Bluetooth but it seems like 240 bytes is correct.
Thanks for spec reading as well. I also think it's OK to round up and allow e.g. 512 bytes. The main intent is to avoid renderers terribly misbehaving - so that's an "order of magnitude" sort of thing.
Status: Started (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97239d883153487960a88a907740755e5459b2a6

commit 97239d883153487960a88a907740755e5459b2a6
Author: jeffcarp <jeffcarp@chromium.org>
Date: Wed Jan 18 23:54:14 2017

Bump Bluetooth max filter name length from 29 bytes to 240 bytes

A device can't advertise a name longer than 29 bytes, but it can acquire a name via gap.device_name that is longer than 29 bytes, limited by the maximum EIR packet size of 240 bytes.

BUG= 653718 

Review-Url: https://codereview.chromium.org/2615273002
Cr-Commit-Position: refs/heads/master@{#444551}

[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/content/browser/bluetooth/bluetooth_device_chooser_controller.cc
[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc
[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h
[add] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/device-name-longer-than-29-bytes.html
[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-name.html
[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-namePrefix.html
[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/unicode-max-length-for-device-name-name.html
[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/unicode-max-length-for-device-name-namePrefix.html
[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp
[modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py

BT5.0 3.C.3.2.2.3 says the device name can be up to 248 bytes (encoded in UTF-8). The Extended Inquiry Response is roughly the Classic version of an Advertising packet, and could hold up to 238 bytes of the name (+ 1 byte for length + 1 byte for EIR Data Type makes 240 bytes). 3.C.8 does say to send a shortened version of the name if it's longer than the EIR packet. The EIR isn't all that relevant for Web Bluetooth for now since we only support BLE.

From BT5.0 6.B.2.3.4.9, it looks like the extended advertising payload can contain up to 254 bytes when not fragmented or 1650 bytes when fragmented, which is more than enough to hold the whole 248-byte name. So we should probably limit to 248 bytes, not 240.
That sounds good, thanks for catching that detail, I'll submit a follow up CL to update the limit to 248 bytes and change the description.

Is 'BT5.0 6.B.2.3.4.9' the correct way to refer to that part of the spec? (i.e. is that how it should be referred to in the comment?)
Yeah, "BT 5.0" or "Bluetooth 5.0" or "Bluetooth Core 5.0" are fine. Thanks!
As of https://crrev.com/2642123003 the maximum size is 248 bytes. Is this fixed now?
Status: Fixed (was: Started)
Thanks!
Project Member

Comment 14 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf

commit dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf
Author: scheib <scheib@chromium.org>
Date: Sat May 13 01:59:13 2017

bluetooth: web: Remove now-duplicate max name length tests.

Code used to be stricter about filters and device names and
return different error types. We realized that the longest
name should always be viable to filter on. Only TypeErrors
are produced now, making these tests redundant.

4 test are kept, originally named:
  "*max-length-for-device-name*"
and remove 4
  "*max-length-for-name-in-adv-name*"

These duplicate tests should have been removed in
https://codereview.chromium.org/2642123003

The names are simplified to:
  "max-length-name*.html"

BUG= 653718 

Review-Url: https://codereview.chromium.org/2866783002
Cr-Commit-Position: refs/heads/master@{#471540}

[delete] https://crrev.com/16d5c759172948ac4c1eeca0d5bcdbc7fb7535c6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-name.html
[delete] https://crrev.com/16d5c759172948ac4c1eeca0d5bcdbc7fb7535c6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-namePrefix.html
[rename] https://crrev.com/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-name-unicode.html
[rename] https://crrev.com/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-name.html
[rename] https://crrev.com/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-namePrefix-unicode.html
[rename] https://crrev.com/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-namePrefix.html
[delete] https://crrev.com/16d5c759172948ac4c1eeca0d5bcdbc7fb7535c6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/unicode-max-length-for-name-in-adv-name.html
[delete] https://crrev.com/16d5c759172948ac4c1eeca0d5bcdbc7fb7535c6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/unicode-max-length-for-name-in-adv-namePrefix.html

Sign in to add a comment