Skip to content

Conversation

piotrnarajowski
Copy link
Contributor

@piotrnarajowski piotrnarajowski commented Mar 20, 2025

This introduces handling authorization in nimble. Each operation on characteristic that has authorize flags will require authorization. Application can respond in GAP authorization event callback with accept, reject or pending response. The latter response should trigger user action to either accept or reject request and finish handling att request
This is done on top of PR #1668 by SumeetSingh19
Fixes GATT/SR/GAW/BI-11-C qualification test case

@github-actions github-actions bot added host size/M Medium PR labels Mar 20, 2025
@piotrnarajowski piotrnarajowski force-pushed the authorization branch 3 times, most recently from 5552678 to cacfff5 Compare March 24, 2025 15:34
@github-actions github-actions bot added the size/L Large PR label Apr 8, 2025
@piotrnarajowski piotrnarajowski marked this pull request as ready for review April 8, 2025 14:01
@piotrnarajowski piotrnarajowski changed the title [wip] nimble/host: add support for asynchronous authorization nimble/host: add support for asynchronous authorization Apr 8, 2025
@piotrnarajowski
Copy link
Contributor Author

#AutoPTS run mynewt GATT/SR/GAR/BI-03-C GATT/SR/GAR/BI-09-C GATT/SR/GAR/BI-15-C GATT/SR/GAR/BI-20-C GATT/SR/GAR/BI-40-C GATT/SR/GAW/BI-04-C GATT/SR/GAW/BI-11-C

@codecoup-tester
Copy link

Scheduled PR #2013 (comment), board: nrf52, estimated start time: 16:05:06, test case count: 7, estimated duration: 0:14:24

Test cases to be runGATT/SR/GAR/BI-03-C
GATT/SR/GAR/BI-09-C
GATT/SR/GAR/BI-15-C
GATT/SR/GAR/BI-20-C
GATT/SR/GAR/BI-40-C
GATT/SR/GAW/BI-04-C
GATT/SR/GAW/BI-11-C

@codecoup-tester
Copy link

AutoPTS Bot results:
No failed test found.

Successful tests (7)GATT GATT/SR/GAR/BI-03-C PASS
GATT GATT/SR/GAR/BI-09-C PASS
GATT GATT/SR/GAR/BI-15-C PASS
GATT GATT/SR/GAR/BI-20-C PASS
GATT GATT/SR/GAR/BI-40-C PASS
GATT GATT/SR/GAW/BI-04-C PASS
GATT GATT/SR/GAW/BI-11-C PASS

@@ -471,6 +560,190 @@ ble_att_svr_read_flat(uint16_t conn_handle,
return rc;
}

int
ble_att_svr_create_read_rsp(uint16_t conn_handle, uint16_t cid,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

helpers that are not used outside this file should be static and they don't need to be declared in ble_att_priv.h

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But they are used in ble_gatts_pending_req_auth(). Maybe it's confusing because they are in separate commits.
Or did you mean to remove all ble_att_svr_build_foo_rsp from ble_att_priv.h, because these are not used outside ble_att_svr.c file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, removed all unnecessary declarations from ble_att_priv.h and converted them back to static. PTAL

Comment on lines +1564 to +1566
if (authorized_attrs[i].conn_handle ==
event->authorize.conn_handle && authorized_attrs[i].attr_handle ==
event->authorize.attr_handle) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (authorized_attrs[i].conn_handle ==
event->authorize.conn_handle && authorized_attrs[i].attr_handle ==
event->authorize.attr_handle) {
if (authorized_attrs[i].conn_handle == event->authorize.conn_handle &&
authorized_attrs[i].attr_handle == event->authorize.attr_handle) {

SumeetSingh19 and others added 4 commits August 7, 2025 10:41
This commit introduces handling authorization in nimble. Each operation on
characteristic that has authorize flags will require authorization.
Application can respond in GAP authorization event callback with accept, reject
or pending response. The latter response should trigger user action to either
accept or reject request and finish handling att request.
ble_att_svr_create_foo_rsp helpers are introduced to hanldle preparing and
sending ATT responses.
This is done on top of PR apache#1668 by SumeetSingh19
For authorization testing purposes two characteristics with
authorize flags and authorize shell command are added.
BLE_GAP_AUTHORIZE_REJECT is always returned when authorized
attribute is being accessed. This is needed for GATT/SR tests
that requires rejecting ATT requests with insufficient
authorization error.
ble_gatts_pending_req_auth method can be used to prepare and send
response to pending ATT request e.g. when user has to authorize operation
on attribute that requires authorization
Comment on lines +100 to +108
/* BLE_GATT_READ_MAX_ATTRS * (1 ATT + 1 EATT chan) */
#define PENDING_ATTR_MAX MYNEWT_VAL(BLE_GATT_READ_MAX_ATTRS) * 2

struct auth_attr {
uint16_t conn_handle;
uint16_t attr_handle;
};

extern struct auth_attr authorized_attrs[PENDING_ATTR_MAX];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not convinced to this approach. It is because, I'm concerned it won't work as expected to the user. As the authorized_attrs size is limited to PENDING_ATTR_MAX, the authorization state can be lost IMO. I would expect the once authorized client won't be asked for authorization again on attribute access attempt.

The solution that comes to my mind would be a authorization state being stored by user. Meaning the user on authorization required characteristic registration provides authorization state block.
Such block would be flags where each flag would correspond to connection index, so the total number of bits would be BLE_MAX_CONNECTIONS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
host size/L Large PR size/M Medium PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants