Bletooth/GATT bt_gatt_characteristic properties write with and without response are not mutually exclusive

[akshhr]

I have an issue if I want certain attribute to accept only write without response. Even if property is set to write without response only , write with response is getting through. It looks like neither stack checks whether an attribute has BT_GATT_CHRC_WRITE or BT_GATT_CHRC_WRITE_WITHOUT_RESP nor the actual opcodes for these operations are exposed to application so that it can control the write operation. Could these opcodes be exposed to application so that application can control through write handlers ?

[jhedberg]

CC @carlescufi

[Vudentz]

Usually there is no problem to respond to a regular when BT_GATT_CHRC_WRITE_WITHOUT_RESP is set, in fact there would be hard to map an error to say only write without response is supported, and even if there would be an error like that the result would probably be a retry with write without response so I wonder what is the benefit to disallow something like that.

In case of write without response without BT_GATT_CHRC_WRITE_WITHOUT_RESP the stack could perhaps block that, but as the name imply there is no response so it would just be dropped silently in which case I would prefer to just add a warning and perhaps pass a flag or something so the application can decide to ignore it.

[wdhpawa]

The BT spec (core v5.0, vol 3, part G, section 3.3.1.1 "Characteristic Properties") states:

The Characteristic Properties bit field determines how the Characteristic Value can be used, [...]. If the bits defined in Table 3 5 are set, the action described is permitted.

In my view this clearly implies that if the bits are NOT set, the corresponding actions are NOT permitted.

For the properties in question table 3.5 says:

Properties	Description
Write Without Response	If set, permit writes of the Characteristic Value without response using procedures defined in Section 4.9.1.
Write	If set, permits writes of the Characteristic Value with response using procedures defined in Section 4.9.3 or Section 4.9.4.

In my view the above clearly implies that there is one property bit for each type of write, and if that bit is NOT set the corresponding type of write is NOT permitted.

The spec is less clear about what should happen if the client performs a type of write that is not permitted by the properties.

Section 4.9.1 "Write Without Response" says only:

If the Characteristic Value write request is the wrong size, or has an invalid value as defined by the profile, then the write shall not succeed and no error shall be generated by the server.

This does not mention the case where the write is not permitted. But I think it is safe to assume that the write should not succeed (what else would "not permitted" mean?). Furthermore, table 4.2 in section 4.13 "GATT Procedure Mapping to ATT Protocol Opcodes" says:

Sub-Procedure	ATT Protocol Opcodes
Write Without Response	Write Command
Write Characteristic Value	Write Request Write Response Error Response

This clearly implies that an Error Response should never be sent as a result of a Write Without Response, so I think we can conclude that the behavior described in section 4.9.1 ("the write shall not succeed and no error shall be generated by the server") also applies to writes that are not permitted.

Regarding the write with response, section 4.9.3 "Write Characteristic Value" says:

An Error Response shall be sent by the server in response to the Write Request if insufficient authentication, insufficient authorization, insufficient encryption key size is used by the client, or if a write operation is not permitted on the Characteristic Value. The Error Code parameter is set as specified in the Attribute Protocol. If the Characteristic Value that is written is the wrong size, or has an invalid value as defined by the profile, then the value shall not be written and an Error Response shall be sent with the Error Code set to Application Error by the server.

This says that an error response should be sent if the client performs a write with response which is not permitted, and again I think it is safe to assume that the write should not succeed (since it is not permitted). The specific error code to use is not mentioned for this case, but I see a few possible candidates (note that "Application Error" doesn't make sense here as this is a GATT issue, not an application issue):

• 0x02: Write Not Permitted
• 0x06: Request Not Supported
• 0xFC: Write Request Rejected

The first two codes are documented in table 3.3 in section 3.4.1.1 "Error Response", and the last one is documented in the Core Specification Supplement v7 part B section 2.4 "WRITE REQUEST REJECTED (0xFC)":

The Write Request Rejected error code is used when a requested write operation cannot be fulfilled for reasons other than permissions. Note: This differs from the “Write Not Permitted” error response in Vol 3, Part F, Section 3.4.1.1 (ATT), which is intended when the write operation cannot be fulfilled due to permissions.

In my opinion this "Write Request Rejected" appears to be the most appropriate error code to use for violations due to characteristic properties. (But this code did not exist in early versions of the spec, and I have seen a BT 4.0 compliant stack reporting "Request Not Supported" in this case).

Regarding the matter of whether the error handling should be done by the stack or the application layer, I would argue that the behavior related to characteristic properties is a GATT feature, so the stack should take care of it. However, we can live with a solution that gives the application layer full control, so that it is at least possible to comply with the BT standard.

[carlescufi]

@wdhpawa

Thanks for the extensive analysis. A couple of comments:

Regarding the write with response, section 4.9.3 "Write Characteristic Value" says: [...] This says that an error response should be sent if the client performs a write with response which is not permitted, and again I think it is safe to assume that the write should not succeed (since it is not permitted)

I believe that sentence, "if a write operation is not permitted" refers to permissions, not properties. That's why it says "write operation" and not "write request operation".

In my opinion this "Write Request Rejected" appears to be the most appropriate error code

~That is an application error code, so I am unsure whether the stack itself should reply with it.~

@Vudentz That said, I am not against actually implementing this. I would probably add a configuration option CONFIG_BT_GATT_ENFORCE_CHAR_PROPS that can be enabled to get GATT to enforce those. That way the user can choose what behavior fits best the application.

[carlescufi]

Also since there seems to be a bit of contention here regarding the implementation of the spec, I would like to page @holtmann to get his point of view.

[wdhpawa]

@carlescufi The "Write Request Rejected" (0xFC) error code is actually not an application error, it is a "Common Profile and Service Error Code". Application errors are in the range 0x80-0x9F. So I think it is appropriate to use this code.

Yes, it is unclear what the spec means by "permitted" in the various sections, but my point is that the operation in question is definitely not allowed, so it makes no sense for the stack to allow it.

[carlescufi]

@wdhpawa

The "Write Request Rejected" (0xFC) error code is actually not an application error, it is a "Common Profile and Service Error Code". Application errors are in the range 0x80-0x9F. So I think it is appropriate to use this code.

You are right, my mistake, apologies.

Yes, it is unclear what the spec means by "permitted" in the various sections, but my point is that the operation in question is definitely not allowed, so it makes no sense for the stack to allow it.

While I do agree with you that the operation is not allowed as per various sections of the GATT spec, I would argue there is enough uncertainty about the role the stack needs to play in enforcing this to make it configurable as I described above.

[Vudentz]

@carlescufi

In my opinion this "Write Request Rejected" appears to be the most appropriate error code

That is an application error code, so I am unsure whether the stack itself should reply with it.

If that is an application error code that normally comes from the application, since that was introduced later I suspect that it is exactly to handle the specific case where the stack support certain operation but the application doesn't.

So Im thinking on adding another flag BT_GATT_WRITE_FLAG_NO_RSP so the application will be able to distinguish between write operations and in case it does want to strictly forbid a characteristic to be written with write request it can just return Write Request Rejected.

One important detail we are missing here is that GATT client are not required to discovery the properties and in case of descriptors like CCC there is no properties for it, furthermore some specs don't seem to clearly document what properties should be set:

Attribute Handle	Attribute Type	Attribute Value	Attribute Permissions
0xMMMM	0x2A00 – UUID for «Device Name»	Device Name	Readable without authentication or authorization when discoverable. Optionally writable, authentication and authorization may be defined by a higher layer specification or be implementation specific.

It doesn't say what to set in the properties, I assume either Write Request or Write Without Response should be valid here, though we only set Write Request:

https://github.com/zephyrproject-rtos/zephyr/blob/master/subsys/bluetooth/host/gatt.c#L138

[carlescufi]

So Im thinking on adding another flag BT_GATT_WRITE_FLAG_NO_RSP so the application will be able to distinguish between write operations and in case it does want to strictly forbid a characteristic to be written with write request it can just return Write Request Rejected.

That is exactly how we solved it at Nordic (at least a while back), so I am in favor of this solution. The only reason I suggested CONFIG_BT_GATT_ENFORCE_CHAR_PROPS is that there seems to be a push towards having the stack handling this automatically. Personally I still think this is better handled at the application level, as I wrote originally.

[wdhpawa]

Well, making it configurable is always the safe route, and has the advantage of allowing application specific hacks if needed, so I'm fine with that.

[wdhpawa]

@Vudentz The Generic Access Service you refer to is actually an area where we are probably going to need more configurability, because right now the optional features appear to be quite hardcoded in the stack. But that's a separate issue.

[wdhpawa]

@Vudentz The Generic Access Service code you linked to actually looks much more configurable than what I saw previously, so maybe we're fine there after all. :-)