Repository navigation
Conversation
The server never returned Read Not Permitted or Write Not Permitted: an attribute could be written without any write permission, and read without any read permission. Only the encryption, authentication and authorization requirements were enforced. Requests from a peer are now checked before the attribute is accessed: - Read, Read Blob, Read By Type, Read Multiple and Read Multiple Variable return Read Not Permitted. - Find By Type Value skips attributes that cannot be read. - Write and Prepare Write return Write Not Permitted. - Write Command is ignored. A security requirement implies the access that it protects, so that permissions like READ_REQUIRES_ENCRYPTION alone keep working. Notifications and indications are not affected, since the value is then read locally. Read Multiple and Read Multiple Variable now also return an error response when reading an attribute fails, instead of not responding. The heart rate test was reading the measurement characteristic, which can only be notified. It now checks a notification.
Holding off on approval to discuss backward compatibility for quirk testing.
zxzxwu
left a comment
zxzxwu
left a comment
There was a problem hiding this comment.
One concern here: since Bumble is widely used as a testing tool to simulate device quirks and non-standard behaviors, some users/tests may intentionally (or inadvertently, like heart_rate_service_test.py) read/write attributes without matching permission bits set, or rely on CharacteristicValue(read=..., write=...) callbacks to return custom ATT_Error codes.
Enforcing this unconditionally by default might break existing quirk tests. Perhaps we could either make strict permission checking configurable/optional on the GATT server, or discuss how to preserve flexibility for testing?
Bumble is also used to simulate devices that do not follow the specification, and existing tests may read or write attributes that do not have the matching permission, or rely on a value callback to return its own error code. Server.strict_permissions, True by default, controls whether READABLE and WRITEABLE are required for reads and writes from a peer. When it is False, those checks are skipped, as before, and the value callbacks are always reached.
|
Makes sense, |
This branch has not been deployed
2 participants
Summary
The GATT server never returned Read Not Permitted or Write Not Permitted. A peer
could write an attribute that has no write permission and read one that has no
read permission. Only the encryption, authentication and authorization
requirements were enforced. This resolves the three
# TODO: check permissionscomments in
gatt_server.py.Behavior
Notes for review
Attribute.read_value, because notifications and indications read the valuethrough
Attribute.read_value, and characteristics such as the Heart RateMeasurement are notify-only with no permissions.
*_REQUIRES_*permission implies the access it protects. Several profiles(MCP, AICS, CSIP, VOCS, HAP, VCS) declare for example
READ_REQUIRES_ENCRYPTIONwithoutREADABLE, and they keep working.reading an attribute fails. Before, the error was raised inside the handler
task and no response was sent, for example for an attribute that requires
encryption on an unencrypted link.
tests/profiles/heart_rate_service_test.pywas reading the Heart RateMeasurement, which is notify-only. It now subscribes and checks a
notification.
Compatibility
Servers that declare a property without the matching permission (for example
WRITEwith onlyREADABLE) will now have those requests rejected. There is nosuch declaration in
bumble,appsorexamples.Testing
invoke project.pre-commitpasses. Three tests added intests/gatt_test.py.