2021-07-28 13:17:48

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v3 01/13] core: add is_allowed property in btd_service

From: Yun-Hao Chung <[email protected]>

This adds is_allowed property in btd_service. When is_allowed is set to
false, calling btd_service_connect and service_accept will fail and the
existing service connection gets disconnected.

Reviewed-by: Miao-chen Chou <[email protected]>
---

Changes in v3:
- Rename plugins/admin_policy.c -> plugins/admin.c
- Use device_added callback in btd_adapter_driver instead of listen for
dbus
- Add authorization method in profiles/health/mcap.c and block incoming
connections in adapter authorization function.

Changes in v2:
- Move bt_uuid_hash and bt_uuid_equal functions to adapter.c.
- Modify the criteria to say a device is `Affected` from any-of-uuid
to any-of-auto-connect-profile.
- Remove the code to remove/reprobe disallowed/allowed profiles,
instead, check if the service is allowed in bt_io_accept connect_cb.
- Fix a typo in emit_property_change in
plugin/admin_policy.c:set_service_allowlist
- Instead of using device_state_cb, utilize D-BUS client to watch device
added/removed.
- Add a document in doc/

src/service.c | 39 +++++++++++++++++++++++++++++++++++++++
src/service.h | 2 ++
2 files changed, 41 insertions(+)

diff --git a/src/service.c b/src/service.c
index 21a52762e..929d6c136 100644
--- a/src/service.c
+++ b/src/service.c
@@ -41,6 +41,7 @@ struct btd_service {
void *user_data;
btd_service_state_t state;
int err;
+ bool is_allowed;
};

struct service_state_callback {
@@ -133,6 +134,7 @@ struct btd_service *service_create(struct btd_device *device,
service->device = device; /* Weak ref */
service->profile = profile;
service->state = BTD_SERVICE_STATE_UNAVAILABLE;
+ service->is_allowed = true;

return service;
}
@@ -186,6 +188,18 @@ int service_accept(struct btd_service *service)
if (!service->profile->accept)
return -ENOSYS;

+ if (!service->is_allowed) {
+ info("service %s is not allowed",
+ service->profile->remote_uuid);
+ return -ECONNABORTED;
+ }
+
+ if (!service->is_allowed) {
+ info("service %s is not allowed",
+ service->profile->remote_uuid);
+ return -ECONNABORTED;
+ }
+
err = service->profile->accept(service);
if (!err)
goto done;
@@ -245,6 +259,12 @@ int btd_service_connect(struct btd_service *service)
return -EBUSY;
}

+ if (!service->is_allowed) {
+ info("service %s is not allowed",
+ service->profile->remote_uuid);
+ return -ECONNABORTED;
+ }
+
err = profile->connect(service);
if (err == 0) {
change_state(service, BTD_SERVICE_STATE_CONNECTING, 0);
@@ -361,6 +381,25 @@ bool btd_service_remove_state_cb(unsigned int id)
return false;
}

+void btd_service_set_allowed(struct btd_service *service, bool allowed)
+{
+ if (allowed == service->is_allowed)
+ return;
+
+ service->is_allowed = allowed;
+
+ if (!allowed && (service->state == BTD_SERVICE_STATE_CONNECTING ||
+ service->state == BTD_SERVICE_STATE_CONNECTED)) {
+ btd_service_disconnect(service);
+ return;
+ }
+}
+
+bool btd_service_is_allowed(struct btd_service *service)
+{
+ return service->is_allowed;
+}
+
void btd_service_connecting_complete(struct btd_service *service, int err)
{
if (service->state != BTD_SERVICE_STATE_DISCONNECTED &&
diff --git a/src/service.h b/src/service.h
index 88530cc17..5a2a02447 100644
--- a/src/service.h
+++ b/src/service.h
@@ -51,6 +51,8 @@ int btd_service_get_error(const struct btd_service *service);
unsigned int btd_service_add_state_cb(btd_service_state_cb cb,
void *user_data);
bool btd_service_remove_state_cb(unsigned int id);
+void btd_service_set_allowed(struct btd_service *service, bool allowed);
+bool btd_service_is_allowed(struct btd_service *service);

/* Functions used by profile implementation */
void btd_service_connecting_complete(struct btd_service *service, int err);
--
2.32.0.432.gabb21c7263-goog



2021-07-28 13:41:05

by bluez.test.bot

[permalink] [raw]
Subject: RE: Admin policy series

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=522677

---Test result---

Test Summary:
CheckPatch FAIL 4.37 seconds
GitLint PASS 1.40 seconds
Prep - Setup ELL PASS 41.48 seconds
Build - Prep PASS 0.10 seconds
Build - Configure PASS 7.30 seconds
Build - Make FAIL 100.83 seconds
Make Check FAIL 0.37 seconds
Make Distcheck FAIL 105.14 seconds
Build w/ext ELL - Configure PASS 7.26 seconds
Build w/ext ELL - Make FAIL 89.75 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
mcap: add adapter authorization
WARNING:NEW_TYPEDEFS: do not add new typedefs
#139: FILE: profiles/health/mcap.h:253:
+typedef guint (* mcap_authorize_cb) (const bdaddr_t *src, const bdaddr_t *dst,

WARNING:LONG_LINE_COMMENT: line length of 93 exceeds 80 columns
#150: FILE: profiles/health/mcap.h:279:
+ mcap_authorize_cb authorize_cb; /* Method to request authorization */

- total: 0 errors, 2 warnings, 133 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] mcap: add adapter authorization" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

core: add device_added and device_removed to adapter driver
WARNING:SPACING: Unnecessary space before function pointer arguments
#130: FILE: src/adapter.h:114:
+ void (*device_added) (struct btd_adapter *adapter,

WARNING:SPACING: Unnecessary space before function pointer arguments
#132: FILE: src/adapter.h:116:
+ void (*device_removed) (struct btd_adapter *adapter,

- total: 0 errors, 2 warnings, 112 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] core: add device_added and device_removed to adapter driver" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

plugins/admin_policy: add ServiceAllowList property
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#63: FILE: plugins/admin.c:186:
+ const GDBusPropertyTable *property,
^

- total: 1 errors, 0 warnings, 164 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] plugins/admin_policy: add ServiceAllowList property" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

plugins/admin_policy: add AffectedByPolicy property
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#65: FILE: plugins/admin.c:254:
+ const GDBusPropertyTable *property,
^

- total: 1 errors, 0 warnings, 242 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] plugins/admin_policy: add AffectedByPolicy property" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

plugins/admin_policy: persist policy settings
WARNING:LINE_SPACING: Missing a blank line after declarations
#151: FILE: plugins/admin.c:336:
+ struct queue *uuid_list = NULL;
+ gchar **uuids = NULL;

- total: 0 errors, 1 warnings, 408 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] plugins/admin_policy: persist policy settings" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
android/health.c: In function ‘bt_health_register’:
android/health.c:2007:9: error: too few arguments to function ‘mcap_create_instance’
2007 | mcap = mcap_create_instance(&adapter_addr, BT_IO_SEC_MEDIUM, 0, 0,
| ^~~~~~~~~~~~~~~~~~~~
In file included from android/health.c:34:
./profiles/health/mcap.h:405:23: note: declared here
405 | struct mcap_instance *mcap_create_instance(const bdaddr_t *src,
| ^~~~~~~~~~~~~~~~~~~~
make[1]: *** [Makefile:6955: android/health.o] Error 1
make: *** [Makefile:4147: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
android/health.c: In function ‘bt_health_register’:
android/health.c:2007:9: error: too few arguments to function ‘mcap_create_instance’
2007 | mcap = mcap_create_instance(&adapter_addr, BT_IO_SEC_MEDIUM, 0, 0,
| ^~~~~~~~~~~~~~~~~~~~
In file included from android/health.c:34:
./profiles/health/mcap.h:405:23: note: declared here
405 | struct mcap_instance *mcap_create_instance(const bdaddr_t *src,
| ^~~~~~~~~~~~~~~~~~~~
make[1]: *** [Makefile:6955: android/health.o] Error 1
make: *** [Makefile:10436: check] Error 2


##############################
Test: Make Distcheck - FAIL
Desc: Run distcheck to check the distribution
Output:
../../android/health.c: In function ‘bt_health_register’:
../../android/health.c:2007:9: error: too few arguments to function ‘mcap_create_instance’
2007 | mcap = mcap_create_instance(&adapter_addr, BT_IO_SEC_MEDIUM, 0, 0,
| ^~~~~~~~~~~~~~~~~~~~
In file included from ../../android/health.c:34:
../../profiles/health/mcap.h:405:23: note: declared here
405 | struct mcap_instance *mcap_create_instance(const bdaddr_t *src,
| ^~~~~~~~~~~~~~~~~~~~
make[2]: *** [Makefile:6955: android/health.o] Error 1
make[1]: *** [Makefile:4147: all] Error 2
make: *** [Makefile:10357: distcheck] Error 1


##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
android/health.c: In function ‘bt_health_register’:
android/health.c:2007:9: error: too few arguments to function ‘mcap_create_instance’
2007 | mcap = mcap_create_instance(&adapter_addr, BT_IO_SEC_MEDIUM, 0, 0,
| ^~~~~~~~~~~~~~~~~~~~
In file included from android/health.c:34:
./profiles/health/mcap.h:405:23: note: declared here
405 | struct mcap_instance *mcap_create_instance(const bdaddr_t *src,
| ^~~~~~~~~~~~~~~~~~~~
make[1]: *** [Makefile:6955: android/health.o] Error 1
make: *** [Makefile:4147: all] Error 2




---
Regards,
Linux Bluetooth