feat: update default descriptor to default char permissions

[klow68]

When no descriptor is provided, a default one is created.
It feel strange that this one doesn't have the default permissions of his characteristic.

This could be a nice improvement to avoid overriding the descriptor in the case the char need encryption or authentication for example

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[zxzxwu]

Many characteristics support only READ | NOTIFY but not WRITE, so if they have the permissions, CCCD could be unwritable?

Though I agree with the concern, I didn't come up with a good solution here. Maybe we can let users to determine this?

[klow68]

Indeed, maybe it could be better to do:
characteristic.permissions | att.Attribute.READABLE | att.Attribute.WRITEABLE

That will ensure cccd to be writable even if the char is only read|notify

[zxzxwu]

I am perservative on this change because:

1. CCCD values are not shared between clients so they are not sensitive. The access control might be introduced for notify and indicate, instead of CCCD themselves.
2. Core Spec doesn't regulate the behavior of descriptors and reserve it for higher profiles, but usually they are not regulated in profile specs as well.

If necessary, I tend to add a default CCCD constructor function and let callers control permissions there.

[klow68]

Thx for your quick response!
Maybe I should also have explained, why I needed that for my project.
In my case, I use this permission on the cccd to trigger the pairing on the mobile app side.
At the moment I just override the permissions to get the same permissions like the ones I proposed and everything works fine. I just thought this would have been the default behavior. But I guess there is a lot of use cases I don't know.

I'm not to much into Bluetooth, so I don't understand what you mean by: "The access control might be introduced for notify and indicate, instead of CCCD themselves."

For the constructor, how the users could handle the permissions ? By passing them when adding the service ? monkey patching ? or something else ?

[zxzxwu]

The current GATT server only adds CCCD if notification is enabled and CCCD doesn't exist. You can check the conditions above around line #250.

So if users want to pass their own CCCD, just add them when constructing characteristics, like

Characteristic(
    'D901B45B-4916-412E-ACCA-376ECB603B2C',
    Characteristic.Properties.READ | Characteristic.Properties.NOTIFY,
    Characteristic.READABLE | Characteristic.WRITEABLE,
    descriptors=[
        Descriptor(
            GATT_CLIENT_CHARACTERISTIC_CONFIGURATION_DESCRIPTOR, ...
        )
    ],
),

I'm not to much into Bluetooth, so I don't understand what you mean by: "The access control might be introduced for notify and indicate, instead of CCCD themselves."

You can check notify_subscriber() to see how notification is implemented.

bumble/bumble/gatt_server.py

Line 333 in d112901

async def notify_subscriber(

In short, CCCD is a flag to enable and disable subscriptions, and servers can decide whether to send notifications based on it, and other factors like permissions. The current implementation won't send notifications if value is not present by callers, and we can add a checker to block notifications if they don't have enough permissions to Characteristics themselves, but CCCD permission is another thing.

[klow68]

Actually we can't add the descriptor with the lambda function to update the characteristic value when we define the characteristic, because the gatt server is created on runtime.
But yeah that's what I do (adding the descriptor, with permissions), and just before adding the service, I override the characteristicto have the callbacks on the characteristic value for the CCCD.

Adding the permissions by default from the characteristic sounded easier but if it doesn't work for all cases, you are right it's fine the way it is.

Thx a lot for the explanation and you're time!
I think we can close then this topic :)

[zxzxwu]

Since it's really not easy to compose a CCCD value callback, later we may add a helper to construct it.