[PATCHv3 0/7] android/tester: Cases for HIDHost security checks

[PATCHv3 0/7] android/tester: Cases for HIDHost security checks

[Jakub Tyszkowski]

This adds cases checking if Hidhost rejects connection from unknown HID Devices
and if proper security level is used for devices which requires encryption.

v3 changes:
	* improved remote side event verification by defining emulator callbacks
	* removed sdp call counter and use transaction id from the request frame

V2 changes:
	* removed btdev hacks for which proper fix was already pushed
	* smal function flow improvement for emu_remote_connect_hci_action()
	* slightly reordered patch order


Jakub Tyszkowski (7):
  android/tester: Allow HIDHost to use custom sdp response
  android/tester: Rename and expose callback verification function
  android/tester: Improve HIDHost data sending verification
  android/tester: Add encryption change verification mechanism
  android/tester: Add case verifying encryption on HIDHost
  android/tester: Fix HIDHost cases sending fixed tid sdp responses
  android/tester: Add Hidhost rejecting connection case

 android/tester-hidhost.c | 256 ++++++++++++++++++++++++++++++++++++++++++-----
 android/tester-main.c    |  83 +++++++++------
 android/tester-main.h    |   7 ++
 3 files changed, 294 insertions(+), 52 deletions(-)

--
1.9.1

[PATCHv3 1/7] android/tester: Allow HIDHost to use custom sdp response

[Jakub Tyszkowski]

This is needed to test security levels for different devices. So far we
had Mouse device but, we need to test security level elevation for
keyboards.
---
 android/tester-hidhost.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
index c7e3a67..b454db4 100644
--- a/android/tester-hidhost.c
+++ b/android/tester-hidhost.c
@@ -55,10 +55,17 @@ struct emu_cid_data {
 	uint16_t ctrl_cid;
 	uint16_t intr_handle;
 	uint16_t intr_cid;
+
+	void *user_data;
 };
 
 static struct emu_cid_data cid_data;
 
+struct raw_dataset {
+	const void *pdu;
+	int len;
+};
+
 static const uint8_t did_req_pdu[] = { 0x06, /* PDU id */
 			0x00, 0x00, /* Transaction id */
 			0x00, 0x0f, /* Req length */
@@ -128,11 +135,17 @@ static const uint8_t hid_rsp_pdu[] = { 0x07, /* PDU id */
 			0x00, 0x09, 0x02, 0x0e, 0x28, 0x01,
 			0x00 }; /* no continuation */
 
+static struct raw_dataset hid_rsp_data = {
+	.pdu = hid_rsp_pdu,
+	.len = sizeof(hid_rsp_pdu),
+};
+
 static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
 {
 	struct test_data *t_data = tester_get_data();
 	struct bthost *bthost = hciemu_client_get_host(t_data->hciemu);
 	struct emu_cid_data *cid_data = user_data;
+	struct raw_dataset *sdp_data = cid_data->user_data;
 
 	if (!memcmp(did_req_pdu, data, len)) {
 		bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
@@ -141,7 +154,7 @@ static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
 	}
 
 	bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
-					hid_rsp_pdu, sizeof(hid_rsp_pdu));
+					sdp_data->pdu, sdp_data->len);
 }
 static void hid_sdp_search_cb(uint16_t handle, uint16_t cid, void *user_data)
 {
@@ -150,6 +163,7 @@ static void hid_sdp_search_cb(uint16_t handle, uint16_t cid, void *user_data)
 
 	cid_data.sdp_handle = handle;
 	cid_data.sdp_cid = cid;
+	cid_data.user_data = user_data;
 
 	bthost_add_cid_hook(bthost, handle, cid, hid_sdp_cid_hook_cb,
 								&cid_data);
@@ -267,7 +281,7 @@ static void hid_intr_connect_cb(uint16_t handle, uint16_t cid, void *user_data)
 static struct emu_set_l2cap_data l2cap_setup_sdp_data = {
 	.psm = 1,
 	.func = hid_sdp_search_cb,
-	.user_data = NULL,
+	.user_data = &hid_rsp_data,
 };
 
 /* Emulate Control Channel (PSM = 17) */
-- 
1.9.1

[PATCHv3 2/7] android/tester: Rename and expose callback verification function

[Jakub Tyszkowski]

This is needed to verify action result on remotes side by using hciemu
hooks for example. As hooks are test specific we need this to be exposed
to other testers.
---
 android/tester-main.c | 58 +++++++++++++++++++++++++--------------------------
 android/tester-main.h |  1 +
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/android/tester-main.c b/android/tester-main.c
index 297aadb..0505a59 100644
--- a/android/tester-main.c
+++ b/android/tester-main.c
@@ -695,7 +695,7 @@ static gboolean verify_callback(gpointer user_data)
 	return FALSE;
 }
 
-static void schedule_callback_call(struct step *step)
+void schedule_callback_verification(struct step *step)
 {
 	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(verify_callback, step);
@@ -713,7 +713,7 @@ static void adapter_state_changed_cb(bt_state_t state)
 	step->callback_result.state = state;
 	step->callback = CB_BT_ADAPTER_STATE_CHANGED;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static bt_property_t *copy_properties(int num_properties,
@@ -769,7 +769,7 @@ static void adapter_properties_cb(bt_status_t status, int num_properties,
 								properties);
 	step->callback = CB_BT_ADAPTER_PROPERTIES;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void discovery_state_changed_cb(bt_discovery_state_t state)
@@ -779,7 +779,7 @@ static void discovery_state_changed_cb(bt_discovery_state_t state)
 	step->callback = CB_BT_DISCOVERY_STATE_CHANGED;
 	step->callback_result.state = state;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void device_found_cb(int num_properties, bt_property_t *properties)
@@ -792,7 +792,7 @@ static void device_found_cb(int num_properties, bt_property_t *properties)
 
 	step->callback = CB_BT_DEVICE_FOUND;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void remote_device_properties_cb(bt_status_t status,
@@ -807,7 +807,7 @@ static void remote_device_properties_cb(bt_status_t status,
 
 	step->callback = CB_BT_REMOTE_DEVICE_PROPERTIES;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void bond_state_changed_cb(bt_status_t status,
@@ -827,7 +827,7 @@ static void bond_state_changed_cb(bt_status_t status,
 
 	step->callback = CB_BT_BOND_STATE_CHANGED;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void pin_request_cb(bt_bdaddr_t *remote_bd_addr,
@@ -856,7 +856,7 @@ static void pin_request_cb(bt_bdaddr_t *remote_bd_addr,
 	g_free(props[2]->val);
 	g_free(props[2]);
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void ssp_request_cb(bt_bdaddr_t *remote_bd_addr,
@@ -887,7 +887,7 @@ static void ssp_request_cb(bt_bdaddr_t *remote_bd_addr,
 	g_free(props[2]->val);
 	g_free(props[2]);
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void acl_state_changed_cb(bt_status_t status,
@@ -900,7 +900,7 @@ static void acl_state_changed_cb(bt_status_t status,
 	step->callback_result.status = status;
 	step->callback_result.state = state;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static bt_callbacks_t bt_callbacks = {
@@ -927,7 +927,7 @@ static void hidhost_connection_state_cb(bt_bdaddr_t *bd_addr,
 	step->callback = CB_HH_CONNECTION_STATE;
 	step->callback_result.state = state;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
@@ -937,7 +937,7 @@ static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
 	step->callback = CB_HH_VIRTUAL_UNPLUG;
 	step->callback_result.status = status;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
@@ -952,7 +952,7 @@ static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
 
 	/* TODO: add bdaddr to verify? */
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
@@ -961,7 +961,7 @@ static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
 
 	step->callback = CB_HH_HID_INFO;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
@@ -974,7 +974,7 @@ static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
 	step->callback_result.status = status;
 	step->callback_result.report_size = size;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static bthh_callbacks_t bthh_callbacks = {
@@ -996,7 +996,7 @@ static void gattc_register_client_cb(int status, int client_if,
 
 	step->callback_result.status = status;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void gattc_scan_result_cb(bt_bdaddr_t *bda, int rssi, uint8_t *adv_data)
@@ -1020,7 +1020,7 @@ static void gattc_scan_result_cb(bt_bdaddr_t *bda, int rssi, uint8_t *adv_data)
 	g_free(props[1]->val);
 	g_free(props[1]);
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void gattc_connect_cb(int conn_id, int status, int client_if,
@@ -1043,7 +1043,7 @@ static void gattc_connect_cb(int conn_id, int status, int client_if,
 	g_free(props[0]->val);
 	g_free(props[0]);
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void gattc_disconnect_cb(int conn_id, int status, int client_if,
@@ -1066,7 +1066,7 @@ static void gattc_disconnect_cb(int conn_id, int status, int client_if,
 	g_free(props[0]->val);
 	g_free(props[0]);
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void gattc_listen_cb(int status, int server_if)
@@ -1076,7 +1076,7 @@ static void gattc_listen_cb(int status, int server_if)
 	step->callback = CB_GATTC_LISTEN;
 	step->callback_result.status = status;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void gattc_search_result_cb(int conn_id, btgatt_srvc_id_t *srvc_id)
@@ -1087,7 +1087,7 @@ static void gattc_search_result_cb(int conn_id, btgatt_srvc_id_t *srvc_id)
 	step->callback_result.conn_id = conn_id;
 	step->callback_result.service = g_memdup(srvc_id, sizeof(*srvc_id));
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void gattc_search_complete_cb(int conn_id, int status)
@@ -1097,7 +1097,7 @@ static void gattc_search_complete_cb(int conn_id, int status)
 	step->callback = CB_GATTC_SEARCH_COMPLETE;
 	step->callback_result.conn_id = conn_id;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void gattc_get_characteristic_cb(int conn_id, int status,
@@ -1114,7 +1114,7 @@ static void gattc_get_characteristic_cb(int conn_id, int status,
 							sizeof(*char_id));
 	step->callback_result.char_prop = char_prop;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void pan_control_state_cb(btpan_control_state_t state,
@@ -1128,7 +1128,7 @@ static void pan_control_state_cb(btpan_control_state_t state,
 	step->callback_result.ctrl_state = error;
 	step->callback_result.local_role = state;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void pan_connection_state_cb(btpan_connection_state_t state,
@@ -1144,7 +1144,7 @@ static void pan_connection_state_cb(btpan_connection_state_t state,
 	step->callback_result.local_role = local_role;
 	step->callback_result.remote_role = remote_role;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static btpan_callbacks_t btpan_callbacks = {
@@ -1161,7 +1161,7 @@ static void hdp_app_reg_state_cb(int app_id, bthl_app_reg_state_t state)
 	step->callback_result.app_id = app_id;
 	step->callback_result.app_state = state;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void hdp_channel_state_cb(int app_id, bt_bdaddr_t *bd_addr,
@@ -1176,7 +1176,7 @@ static void hdp_channel_state_cb(int app_id, bt_bdaddr_t *bd_addr,
 	step->callback_result.mdep_cfg_index = mdep_cfg_index;
 	step->callback_result.channel_state = state;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static bthl_callbacks_t bthl_callbacks = {
@@ -1193,7 +1193,7 @@ static void a2dp_connection_state_cb(btav_connection_state_t state,
 	step->callback = CB_A2DP_CONN_STATE;
 	step->callback_result.state = state;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static void a2dp_audio_state_cb(btav_audio_state_t state, bt_bdaddr_t *bd_addr)
@@ -1203,7 +1203,7 @@ static void a2dp_audio_state_cb(btav_audio_state_t state, bt_bdaddr_t *bd_addr)
 	step->callback = CB_A2DP_AUDIO_STATE;
 	step->callback_result.state = state;
 
-	schedule_callback_call(step);
+	schedule_callback_verification(step);
 }
 
 static btav_callbacks_t bta2dp_callbacks = {
diff --git a/android/tester-main.h b/android/tester-main.h
index 6cad803..d40b995 100644
--- a/android/tester-main.h
+++ b/android/tester-main.h
@@ -452,6 +452,7 @@ void remove_gatt_tests(void);
 
 /* Generic tester API */
 void schedule_action_verification(struct step *step);
+void schedule_callback_verification(struct step *step);
 
 /* Emulator actions */
 void emu_setup_powered_remote_action(void);
-- 
1.9.1

[PATCHv3 3/7] android/tester: Improve HIDHost data sending verification

[Jakub Tyszkowski]

Hciemu hooks may be called in different test stage than we expect
and we shouldn't blindly verifying the step with success every time
when data is send, as we really dont know the step we are currently in.

This patch fixes this issue by splitting the action in two steps: action
step that triggers data sending and callback step that verifies on
remotes sid that data was send.
---
 android/tester-hidhost.c | 36 ++++++++++++++----------------------
 android/tester-main.h    |  3 +++
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
index b454db4..dd378cc 100644
--- a/android/tester-hidhost.c
+++ b/android/tester-hidhost.c
@@ -230,10 +230,11 @@ static void hid_ctrl_cid_hook_cb(const void *data, uint16_t len,
 		/* Successfully verify sending data step */
 		step = g_new0(struct step, 1);
 
-		step->action_status = BT_STATUS_SUCCESS;
+		step->callback = CB_EMU_CONFIRM_SEND_DATA;
 
-		schedule_action_verification(step);
+		schedule_callback_verification(step);
 		break;
+
 	}
 }
 static void hid_ctrl_connect_cb(uint16_t handle, uint16_t cid, void *user_data)
@@ -259,9 +260,9 @@ static void hid_intr_cid_hook_cb(const void *data, uint16_t len,
 		/* Successfully verify sending data step */
 		step = g_new0(struct step, 1);
 
-		step->action_status = BT_STATUS_SUCCESS;
+		step->callback = CB_EMU_CONFIRM_SEND_DATA;
 
-		schedule_action_verification(step);
+		schedule_callback_verification(step);
 		break;
 	}
 }
@@ -390,40 +391,29 @@ static void hidhost_set_report_action(void)
 {
 	struct test_data *data = tester_get_data();
 	const uint8_t *hid_addr = hciemu_get_client_bdaddr(data->hciemu);
+	struct step *step = g_new0(struct step, 1);
 	char *buf = "fe0201";
 	bt_bdaddr_t bdaddr;
-	int status;
 
 	bdaddr2android((const bdaddr_t *) hid_addr, &bdaddr);
 
-	/* Successfull result should be verified on the other end (hook) */
-	status = data->if_hid->send_data(&bdaddr, buf);
-	if (status) {
-		struct step *step = g_new0(struct step, 1);
-
-		step->action_status = status;
-		schedule_action_verification(step);
-	}
+	step->action_status = data->if_hid->send_data(&bdaddr, buf);
+	schedule_action_verification(step);
 }
 
 static void hidhost_send_data_action(void)
 {
 	struct test_data *data = tester_get_data();
 	const uint8_t *hid_addr = hciemu_get_client_bdaddr(data->hciemu);
+	struct step *step = g_new0(struct step, 1);
 	char *buf = "010101";
 	bt_bdaddr_t bdaddr;
-	int status;
 
 	bdaddr2android((const bdaddr_t *) hid_addr, &bdaddr);
 
-	/* Successfull result should be verified on the other end (hook) */
-	status = data->if_hid->set_report(&bdaddr, BTHH_INPUT_REPORT, buf);
-	if (status) {
-		struct step *step = g_new0(struct step, 1);
-
-		step->action_status = status;
-		schedule_action_verification(step);
-	}
+	step->action_status = data->if_hid->set_report(&bdaddr,
+							BTHH_INPUT_REPORT, buf);
+	schedule_action_verification(step);
 }
 
 static struct test_case test_cases[] = {
@@ -554,6 +544,7 @@ static struct test_case test_cases[] = {
 		CALLBACK_STATE(CB_HH_CONNECTION_STATE,
 						BTHH_CONN_STATE_CONNECTED),
 		ACTION_SUCCESS(hidhost_set_report_action, NULL),
+		CALLBACK(CB_EMU_CONFIRM_SEND_DATA),
 	),
 	TEST_CASE_BREDRLE("HidHost SendData Success",
 		ACTION_SUCCESS(bluetooth_enable_action, NULL),
@@ -570,6 +561,7 @@ static struct test_case test_cases[] = {
 		CALLBACK_STATE(CB_HH_CONNECTION_STATE,
 						BTHH_CONN_STATE_CONNECTED),
 		ACTION_SUCCESS(hidhost_send_data_action, NULL),
+		CALLBACK(CB_EMU_CONFIRM_SEND_DATA),
 	),
 };
 
diff --git a/android/tester-main.h b/android/tester-main.h
index d40b995..9e579fc 100644
--- a/android/tester-main.h
+++ b/android/tester-main.h
@@ -306,6 +306,9 @@ typedef enum {
 	CB_GATTS_REQUEST_WRITE,
 	CB_GATTS_REQUEST_EXEC_WRITE,
 	CB_GATTS_RESPONSE_CONFIRMATION,
+
+	/* Emulator callbacks */
+	CB_EMU_CONFIRM_SEND_DATA,
 } expected_bt_callback_t;
 
 struct test_data {
-- 
1.9.1

[PATCHv3 4/7] android/tester: Add encryption change verification mechanism

[Jakub Tyszkowski]

Use callback mechanism for encryption verification
---
 android/tester-main.c | 25 +++++++++++++++++++++++++
 android/tester-main.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/android/tester-main.c b/android/tester-main.c
index 0505a59..efe5bbb 100644
--- a/android/tester-main.c
+++ b/android/tester-main.c
@@ -28,6 +28,8 @@ static gint scheduled_cbacks_num;
 #define EMULATOR_SIGNAL_TIMEOUT 2 /* in seconds */
 #define EMULATOR_SIGNAL "emulator_started"
 
+static const uint8_t hci_encr_change_hdr[] = { 0x04, 0x08, 0x04, 0x00 };
+
 static gboolean check_callbacks_called(gpointer user_data)
 {
 	/*
@@ -213,6 +215,25 @@ static void mgmt_debug(const char *str, void *user_data)
 	tester_print("%s%s", prefix, str);
 }
 
+static bool hciemu_post_encr_hook(const void *data, uint16_t len,
+							void *user_data)
+{
+	struct step *step = g_new0(struct step, 1);
+
+	/*
+	 * Expected data: header (4 octets) + conn. handle (2 octets) +
+	 * encryption flag (1 octet)
+	 */
+	if (len < sizeof(hci_encr_change_hdr) + 3)
+		return true;
+
+	step->callback = ((uint8_t *)data)[6] ? CB_EMU_ENCRYPTION_ENABLED :
+						CB_EMU_ENCRYPTION_DISABLED;
+
+	schedule_callback_verification(step);
+	return true;
+}
+
 static void read_info_callback(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {
@@ -250,6 +271,10 @@ static void read_info_callback(uint8_t status, uint16_t length,
 		return;
 	}
 
+	/* set hook for encryption change */
+	hciemu_add_hook(data->hciemu, HCIEMU_HOOK_POST_EVT, 0x08,
+						hciemu_post_encr_hook, NULL);
+
 	tester_pre_setup_complete();
 }
 
diff --git a/android/tester-main.h b/android/tester-main.h
index 9e579fc..c6c76d9 100644
--- a/android/tester-main.h
+++ b/android/tester-main.h
@@ -309,6 +309,8 @@ typedef enum {
 
 	/* Emulator callbacks */
 	CB_EMU_CONFIRM_SEND_DATA,
+	CB_EMU_ENCRYPTION_ENABLED,
+	CB_EMU_ENCRYPTION_DISABLED,
 } expected_bt_callback_t;
 
 struct test_data {
-- 
1.9.1

[PATCHv3 5/7] android/tester: Add case verifying encryption on HIDHost

[Jakub Tyszkowski]

This case verifies if encryption is enabled when connecting to
HID Keyboard device.
---
 android/tester-hidhost.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
index dd378cc..abff837 100644
--- a/android/tester-hidhost.c
+++ b/android/tester-hidhost.c
@@ -140,6 +140,74 @@ static struct raw_dataset hid_rsp_data = {
 	.len = sizeof(hid_rsp_pdu),
 };
 
+static const uint8_t hid_keyboard_rsp_pdu[] = { 0x07,
+			0x00, 0x01, /* Transaction id */
+			0x02, 0x04, /* Response length */
+			0x02, 0x01, /* Attributes length */
+			0x36, 0x01, 0xfe, 0x36, 0x01, 0x93, 0x09, 0x00, 0x00,
+			0x0a, 0x00, 0x01, 0x00, 0x00, 0x09, 0x00, 0x01, 0x35,
+			0x03, 0x19, 0x11, 0x24, 0x09, 0x00, 0x04, 0x35, 0x0d,
+			0x35, 0x06, 0x19, 0x01, 0x00, 0x09, 0x00, 0x11, 0x35,
+			0x03, 0x19, 0x00, 0x11, 0x09, 0x00, 0x06, 0x35, 0x09,
+			0x09, 0x65, 0x6e, 0x09, 0x00, 0x6a, 0x09, 0x01, 0x00,
+			0x09, 0x00, 0x09, 0x35, 0x08, 0x35, 0x06, 0x19, 0x11,
+			0x24, 0x09, 0x01, 0x00, 0x09, 0x00, 0x0d, 0x35, 0x0f,
+			0x35, 0x0d, 0x35, 0x06, 0x19, 0x01, 0x00, 0x09, 0x00,
+			0x13, 0x35, 0x03, 0x19, 0x00, 0x11, 0x09, 0x01, 0x00,
+			0x25, 0x10, 0x53, 0x41, 0x4d, 0x53, 0x55, 0x4e, 0x47,
+			0x20, 0x4b, 0x65, 0x79, 0x62, 0x6f, 0x61, 0x72, 0x64,
+			0x09, 0x01, 0x01, 0x25, 0x08, 0x4b, 0x65, 0x79, 0x62,
+			0x6f, 0x61, 0x72, 0x64, 0x09, 0x01, 0x02, 0x25, 0x0d,
+			0x43, 0x53, 0x52, 0x20, 0x48, 0x49, 0x44, 0x45, 0x6e,
+			0x67, 0x69, 0x6e, 0x65, 0x09, 0x02, 0x00, 0x09, 0x01,
+			0x00, 0x09, 0x02, 0x01, 0x09, 0x01, 0x11, 0x09, 0x02,
+			0x02, 0x08, 0x40, 0x09, 0x02, 0x03, 0x08, 0x23, 0x09,
+			0x02, 0x04, 0x28, 0x01, 0x09, 0x02, 0x05, 0x28, 0x01,
+			0x09, 0x02, 0x06, 0x35, 0xb7, 0x35, 0xb5, 0x08, 0x22,
+			0x25, 0xb1, 0x05, 0x01, 0x09, 0x06, 0xa1, 0x01, 0x05,
+			0x07, 0x85, 0x01, 0x19, 0xe0, 0x29, 0xe7, 0x15, 0x00,
+			0x25, 0x01, 0x75, 0x01, 0x95, 0x08, 0x81, 0x02, 0x95,
+			0x01, 0x75, 0x08, 0x81, 0x01, 0x95, 0x05, 0x75, 0x01,
+			0x05, 0x08, 0x85, 0x01, 0x19, 0x01, 0x29, 0x05, 0x91,
+			0x02, 0x95, 0x01, 0x75, 0x03, 0x91, 0x03, 0x95, 0x06,
+			0x75, 0x08, 0x15, 0x00, 0x25, 0x65, 0x05, 0x07, 0x19,
+			0x00, 0x29, 0x6f, 0x81, 0x00, 0xc0, 0x05, 0x0c, 0x09,
+			0x01, 0xa1, 0x01, 0x85, 0x02, 0x05, 0x0c, 0x15, 0x00,
+			0x25, 0x01, 0x75, 0x01, 0x95, 0x18, 0x09, 0xe2, 0x09,
+			0xea, 0x09, 0xe9, 0x09, 0xb7, 0x09, 0xcd, 0x0a, 0x23,
+			0x02, 0x0a, 0x8a, 0x01, 0x0a, 0x21, 0x02, 0x75, 0x01,
+			0x95, 0x03, 0x81, 0x02, 0x75, 0x01, 0x95, 0x05, 0x81,
+			0x01, 0x05, 0x08, 0x85, 0xff, 0x95, 0x01, 0x75, 0x02,
+			0x09, 0x24, 0x09, 0x26, 0x81, 0x02, 0x75, 0x06, 0x81,
+			0x01, 0xc0, 0x06, 0x7f, 0xff, 0x09, 0x01, 0xa1, 0x01,
+			0x85, 0x03, 0x15, 0x00, 0x25, 0x01, 0x09, 0xb9, 0x09,
+			0xb5, 0x09, 0xba, 0x09, 0xbb, 0x09, 0xbc, 0x09, 0xbd,
+			0x09, 0xb6, 0x09, 0xb7, 0x75, 0x01, 0x95, 0x06, 0x81,
+			0x02, 0x75, 0x01, 0x95, 0x02, 0x81, 0x01, 0xc0, 0x09,
+			0x02, 0x07, 0x35, 0x08, 0x35, 0x06, 0x09, 0x04, 0x09,
+			0x09, 0x01, 0x00, 0x09, 0x02, 0x08, 0x28, 0x00, 0x09,
+			0x02, 0x09, 0x28, 0x01, 0x09, 0x02, 0x0a, 0x28, 0x01,
+			0x09, 0x02, 0x0b, 0x09, 0x01, 0x00, 0x09, 0x02, 0x0c,
+			0x09, 0x1f, 0x40, 0x09, 0x02, 0x0d, 0x28, 0x00, 0x09,
+			0x02, 0x0e, 0x28, 0x01, 0x36, 0x00, 0x65, 0x09, 0x00,
+			0x00, 0x0a, 0x00, 0x01, 0x00, 0x01, 0x09, 0x00, 0x01,
+			0x35, 0x03, 0x19, 0x12, 0x00, 0x09, 0x00, 0x04, 0x35,
+			0x0d, 0x35, 0x06, 0x19, 0x01, 0x00, 0x09, 0x00, 0x01,
+			0x35, 0x03, 0x19, 0x00, 0x01, 0x09, 0x00, 0x06, 0x35,
+			0x09, 0x09, 0x65, 0x6e, 0x09, 0x00, 0x6a, 0x09, 0x01,
+			0x00, 0x09, 0x00, 0x09, 0x35, 0x08, 0x35, 0x06, 0x19,
+			0x12, 0x00, 0x09, 0x01, 0x00, 0x09, 0x01, 0x01, 0x25,
+			0x00, 0x09, 0x02, 0x00, 0x09, 0x01, 0x00, 0x09, 0x02,
+			0x01, 0x09, 0x23, 0x3d, 0x09, 0x02, 0x02, 0x09, 0x01,
+			0x3d, 0x09, 0x02, 0x03, 0x09, 0x00, 0x00, 0x09, 0x02,
+			0x04, 0x28, 0x01, 0x09, 0x02, 0x05, 0x09, 0x00,	0x02,
+			0x00 }; /* no continuation */
+
+static struct raw_dataset hid_keyboard_rsp_data = {
+	.pdu = hid_keyboard_rsp_pdu,
+	.len = sizeof(hid_keyboard_rsp_pdu),
+};
+
 static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
 {
 	struct test_data *t_data = tester_get_data();
@@ -285,6 +353,12 @@ static struct emu_set_l2cap_data l2cap_setup_sdp_data = {
 	.user_data = &hid_rsp_data,
 };
 
+static struct emu_set_l2cap_data l2cap_setup_keyboard_sdp_data = {
+	.psm = 1,
+	.func = hid_sdp_search_cb,
+	.user_data = &hid_keyboard_rsp_data,
+};
+
 /* Emulate Control Channel (PSM = 17) */
 static struct emu_set_l2cap_data l2cap_setup_cc_data = {
 	.psm = 17,
@@ -563,6 +637,28 @@ static struct test_case test_cases[] = {
 		ACTION_SUCCESS(hidhost_send_data_action, NULL),
 		CALLBACK(CB_EMU_CONFIRM_SEND_DATA),
 	),
+	TEST_CASE_BREDRLE("HidHost Encrypted Out. Conn. Success",
+		ACTION_SUCCESS(bluetooth_enable_action, NULL),
+		CALLBACK_STATE(CB_BT_ADAPTER_STATE_CHANGED, BT_STATE_ON),
+		ACTION_SUCCESS(emu_setup_powered_remote_action, NULL),
+		ACTION_SUCCESS(emu_set_ssp_mode_action, NULL),
+		ACTION_SUCCESS(set_default_ssp_request_handler, NULL),
+		ACTION_SUCCESS(emu_add_l2cap_server_action,
+						&l2cap_setup_keyboard_sdp_data),
+		ACTION_SUCCESS(emu_add_l2cap_server_action,
+							&l2cap_setup_cc_data),
+		ACTION_SUCCESS(emu_add_l2cap_server_action,
+							&l2cap_setup_ic_data),
+		ACTION_SUCCESS(hidhost_connect_action, NULL),
+		CALLBACK(CB_EMU_ENCRYPTION_ENABLED),
+		CALLBACK_STATE(CB_HH_CONNECTION_STATE,
+						BTHH_CONN_STATE_CONNECTED),
+		ACTION_SUCCESS(hidhost_send_data_action, NULL),
+		ACTION_SUCCESS(bluetooth_disable_action, NULL),
+		CALLBACK_STATE(CB_HH_CONNECTION_STATE,
+						BTHH_CONN_STATE_DISCONNECTED),
+		CALLBACK_STATE(CB_BT_ADAPTER_STATE_CHANGED, BT_STATE_OFF),
+	),
 };
 
 struct queue *get_hidhost_tests(void)
-- 
1.9.1

[PATCHv3 6/7] android/tester: Fix HIDHost cases sending fixed tid sdp responses

[Jakub Tyszkowski]

Multiple cases were affected because of hardcoded transaction id for
emulated remote's SDP responses.

This resulted in the following error in the daemon:
	bluetoothd[13486]: sdp_process: Protocol error.
---
 android/tester-hidhost.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
index abff837..d5741f9 100644
--- a/android/tester-hidhost.c
+++ b/android/tester-hidhost.c
@@ -21,6 +21,7 @@
 #include "tester-main.h"
 
 #include "android/utils.h"
+#include "src/shared/util.h"
 
 #define HID_GET_REPORT_PROTOCOL		0x60
 #define HID_GET_BOOT_PROTOCOL		0x61
@@ -214,15 +215,24 @@ static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
 	struct bthost *bthost = hciemu_client_get_host(t_data->hciemu);
 	struct emu_cid_data *cid_data = user_data;
 	struct raw_dataset *sdp_data = cid_data->user_data;
+	const uint8_t *req_buf = data;
+	uint8_t *sdp_buf;
 
-	if (!memcmp(did_req_pdu, data, len)) {
+	if (!memcmp(did_req_pdu, req_buf, len)) {
 		bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
 					did_rsp_pdu, sizeof(did_rsp_pdu));
 		return;
 	}
 
+	/* Get transaction ID from the request */
+	sdp_buf = g_memdup(sdp_data->pdu, sdp_data->len);
+	sdp_buf[1] = req_buf[1];
+	sdp_buf[2] = req_buf[2];
+
 	bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
-					sdp_data->pdu, sdp_data->len);
+							sdp_buf, sdp_data->len);
+
+	g_free(sdp_buf);
 }
 static void hid_sdp_search_cb(uint16_t handle, uint16_t cid, void *user_data)
 {
-- 
1.9.1

[PATCHv3 7/7] android/tester: Add Hidhost rejecting connection case

[Jakub Tyszkowski]

This test case checks if hidhost rejects connections from
unknown devices.
---
 android/tester-hidhost.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
 android/tester-main.h    |  1 +
 2 files changed, 99 insertions(+)

diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
index d5741f9..64945e8 100644
--- a/android/tester-hidhost.c
+++ b/android/tester-hidhost.c
@@ -42,6 +42,9 @@
 #define HID_MODE_BREDR			0x01
 #define HID_MODE_LE			0x02
 
+#define HID_MSG_CONTROL			0x10
+#define HID_VIRTUAL_CABLE_UNPLUG	0x05
+
 #define HID_EXPECTED_REPORT_SIZE	0x02
 
 static struct queue *list; /* List of hidhost test cases */
@@ -209,6 +212,15 @@ static struct raw_dataset hid_keyboard_rsp_data = {
 	.len = sizeof(hid_keyboard_rsp_pdu),
 };
 
+static bt_scan_mode_t setprop_scan_mode_conn_val =
+					BT_SCAN_MODE_CONNECTABLE;
+
+static bt_property_t prop_test_scan_mode_conn = {
+	.type = BT_PROPERTY_ADAPTER_SCAN_MODE,
+	.val = &setprop_scan_mode_conn_val,
+	.len = sizeof(setprop_scan_mode_conn_val),
+};
+
 static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
 {
 	struct test_data *t_data = tester_get_data();
@@ -312,7 +324,13 @@ static void hid_ctrl_cid_hook_cb(const void *data, uint16_t len,
 
 		schedule_callback_verification(step);
 		break;
+	case HID_MSG_CONTROL | HID_VIRTUAL_CABLE_UNPLUG:
+		step = g_new0(struct step, 1);
+
+		step->callback = CB_EMU_CONNECTION_REJECTED;
 
+		schedule_callback_verification(step);
+		break;
 	}
 }
 static void hid_ctrl_connect_cb(uint16_t handle, uint16_t cid, void *user_data)
@@ -500,6 +518,68 @@ static void hidhost_send_data_action(void)
 	schedule_action_verification(step);
 }
 
+static void client_l2cap_rsp(uint8_t code, const void *data, uint16_t len,
+							void *user_data)
+{
+	static const uint8_t con_req[] = { 0x13, 0x00,		/* PSM */
+						0x41, 0x00 };	/* Source CID */
+	const uint16_t *psm = data;
+	struct test_data *t_data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(t_data->hciemu);
+
+	if (len < sizeof(*psm)) {
+		tester_warn("Invalid l2cap response.");
+		return;
+	}
+
+	switch (*psm) {
+	case 0x40:
+		tester_print("Connected ctrl channel");
+
+		bthost_add_cid_hook(bthost, cid_data.ctrl_handle, 0x40,
+					hid_ctrl_cid_hook_cb, &cid_data);
+
+		cid_data.intr_handle = cid_data.ctrl_handle;
+
+		bthost_l2cap_req(bthost, cid_data.intr_handle, 0x02,
+					con_req, sizeof(con_req),
+					client_l2cap_rsp, &cid_data);
+		break;
+	case 0x41:
+		tester_print("Connected intr channel");
+
+		bthost_add_cid_hook(bthost, cid_data.intr_handle, 0x41,
+					hid_intr_cid_hook_cb, &cid_data);
+
+		break;
+	default:
+		break;
+	}
+}
+
+static void hidhost_conn_cb(uint16_t handle, void *user_data)
+{
+	static const uint8_t con_req[] = { 0x11, 0x00,		/* PSM */
+						0x40, 0x00 };	/* Source CID */
+
+	struct test_data *data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+
+	if (data->hciemu_type == HCIEMU_TYPE_BREDR) {
+		tester_warn("Not handled device type.");
+		return;
+	}
+
+	cid_data.ctrl_cid = 0x40;
+	cid_data.ctrl_handle = handle;
+
+	tester_print("Sending L2CAP Request from remote");
+
+	bthost_l2cap_req(bthost, handle, 0x02,
+					con_req, sizeof(con_req),
+					client_l2cap_rsp, &cid_data);
+}
+
 static struct test_case test_cases[] = {
 	TEST_CASE_BREDRLE("HidHost Init",
 		ACTION_SUCCESS(dummy_action, NULL),
@@ -647,6 +727,24 @@ static struct test_case test_cases[] = {
 		ACTION_SUCCESS(hidhost_send_data_action, NULL),
 		CALLBACK(CB_EMU_CONFIRM_SEND_DATA),
 	),
+	TEST_CASE_BREDRLE("HidHost Reject Unknown Remote Connection",
+		ACTION_SUCCESS(bluetooth_enable_action, NULL),
+		CALLBACK_STATE(CB_BT_ADAPTER_STATE_CHANGED, BT_STATE_ON),
+		ACTION_SUCCESS(bt_set_property_action,
+						&prop_test_scan_mode_conn),
+		CALLBACK_ADAPTER_PROPS(&prop_test_scan_mode_conn, 1),
+		ACTION_SUCCESS(emu_setup_powered_remote_action, NULL),
+		ACTION_SUCCESS(emu_add_l2cap_server_action,
+						&l2cap_setup_keyboard_sdp_data),
+		ACTION_SUCCESS(emu_add_l2cap_server_action,
+							&l2cap_setup_cc_data),
+		ACTION_SUCCESS(emu_add_l2cap_server_action,
+							&l2cap_setup_ic_data),
+		/* Trigger incoming connection */
+		ACTION_SUCCESS(emu_set_connect_cb_action, hidhost_conn_cb),
+		ACTION_SUCCESS(emu_remote_connect_hci_action, NULL),
+		CALLBACK(CB_EMU_CONNECTION_REJECTED),
+	),
 	TEST_CASE_BREDRLE("HidHost Encrypted Out. Conn. Success",
 		ACTION_SUCCESS(bluetooth_enable_action, NULL),
 		CALLBACK_STATE(CB_BT_ADAPTER_STATE_CHANGED, BT_STATE_ON),
diff --git a/android/tester-main.h b/android/tester-main.h
index c6c76d9..6f6e57f 100644
--- a/android/tester-main.h
+++ b/android/tester-main.h
@@ -311,6 +311,7 @@ typedef enum {
 	CB_EMU_CONFIRM_SEND_DATA,
 	CB_EMU_ENCRYPTION_ENABLED,
 	CB_EMU_ENCRYPTION_DISABLED,
+	CB_EMU_CONNECTION_REJECTED,
 } expected_bt_callback_t;
 
 struct test_data {
-- 
1.9.1

Re: [PATCHv3 1/7] android/tester: Allow HIDHost to use custom sdp response

[Luiz Augusto von Dentz]

Hi Jakub,

On Tue, Sep 9, 2014 at 10:46 AM, Jakub Tyszkowski
<jakub.tyszkowski@tieto.com> wrote:
> This is needed to test security levels for different devices. So far we
> had Mouse device but, we need to test security level elevation for
> keyboards.
> ---
>  android/tester-hidhost.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
> index c7e3a67..b454db4 100644
> --- a/android/tester-hidhost.c
> +++ b/android/tester-hidhost.c
> @@ -55,10 +55,17 @@ struct emu_cid_data {
>         uint16_t ctrl_cid;
>         uint16_t intr_handle;
>         uint16_t intr_cid;
> +
> +       void *user_data;
>  };
>
>  static struct emu_cid_data cid_data;
>
> +struct raw_dataset {
> +       const void *pdu;
> +       int len;
> +};
> +
>  static const uint8_t did_req_pdu[] = { 0x06, /* PDU id */
>                         0x00, 0x00, /* Transaction id */
>                         0x00, 0x0f, /* Req length */
> @@ -128,11 +135,17 @@ static const uint8_t hid_rsp_pdu[] = { 0x07, /* PDU id */
>                         0x00, 0x09, 0x02, 0x0e, 0x28, 0x01,
>                         0x00 }; /* no continuation */
>
> +static struct raw_dataset hid_rsp_data = {
> +       .pdu = hid_rsp_pdu,
> +       .len = sizeof(hid_rsp_pdu),
> +};
> +

Maybe in the future we can create helper function for PDU matching,
perhaps in tester-main.c, so the profile just provide the PDU table
that way we don't have to reimplement this over and over again.

>  static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
>  {
>         struct test_data *t_data = tester_get_data();
>         struct bthost *bthost = hciemu_client_get_host(t_data->hciemu);
>         struct emu_cid_data *cid_data = user_data;
> +       struct raw_dataset *sdp_data = cid_data->user_data;
>
>         if (!memcmp(did_req_pdu, data, len)) {
>                 bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
> @@ -141,7 +154,7 @@ static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
>         }
>
>         bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
> -                                       hid_rsp_pdu, sizeof(hid_rsp_pdu));
> +                                       sdp_data->pdu, sdp_data->len);
>  }
>  static void hid_sdp_search_cb(uint16_t handle, uint16_t cid, void *user_data)
>  {
> @@ -150,6 +163,7 @@ static void hid_sdp_search_cb(uint16_t handle, uint16_t cid, void *user_data)
>
>         cid_data.sdp_handle = handle;
>         cid_data.sdp_cid = cid;
> +       cid_data.user_data = user_data;
>
>         bthost_add_cid_hook(bthost, handle, cid, hid_sdp_cid_hook_cb,
>                                                                 &cid_data);
> @@ -267,7 +281,7 @@ static void hid_intr_connect_cb(uint16_t handle, uint16_t cid, void *user_data)
>  static struct emu_set_l2cap_data l2cap_setup_sdp_data = {
>         .psm = 1,
>         .func = hid_sdp_search_cb,
> -       .user_data = NULL,
> +       .user_data = &hid_rsp_data,
>  };
>
>  /* Emulate Control Channel (PSM = 17) */
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

Re: [PATCHv3 6/7] android/tester: Fix HIDHost cases sending fixed tid sdp responses

[Luiz Augusto von Dentz]

Hi Jakub,

On Tue, Sep 9, 2014 at 10:47 AM, Jakub Tyszkowski
<jakub.tyszkowski@tieto.com> wrote:
> Multiple cases were affected because of hardcoded transaction id for
> emulated remote's SDP responses.
>
> This resulted in the following error in the daemon:
>         bluetoothd[13486]: sdp_process: Protocol error.
> ---
>  android/tester-hidhost.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
> index abff837..d5741f9 100644
> --- a/android/tester-hidhost.c
> +++ b/android/tester-hidhost.c
> @@ -21,6 +21,7 @@
>  #include "tester-main.h"
>
>  #include "android/utils.h"
> +#include "src/shared/util.h"
>
>  #define HID_GET_REPORT_PROTOCOL                0x60
>  #define HID_GET_BOOT_PROTOCOL          0x61
> @@ -214,15 +215,24 @@ static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
>         struct bthost *bthost = hciemu_client_get_host(t_data->hciemu);
>         struct emu_cid_data *cid_data = user_data;
>         struct raw_dataset *sdp_data = cid_data->user_data;
> +       const uint8_t *req_buf = data;
> +       uint8_t *sdp_buf;
>
> -       if (!memcmp(did_req_pdu, data, len)) {
> +       if (!memcmp(did_req_pdu, req_buf, len)) {
>                 bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
>                                         did_rsp_pdu, sizeof(did_rsp_pdu));
>                 return;
>         }
>
> +       /* Get transaction ID from the request */
> +       sdp_buf = g_memdup(sdp_data->pdu, sdp_data->len);
> +       sdp_buf[1] = req_buf[1];
> +       sdp_buf[2] = req_buf[2];
> +
>         bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
> -                                       sdp_data->pdu, sdp_data->len);
> +                                                       sdp_buf, sdp_data->len);
> +
> +       g_free(sdp_buf);
>  }

Do we really need this copy? Wouldn't it be enough not to mark it as
const and just change in place? Anyway this is also broke in AVRCP, I
have been planning to handle this but perhaps we should unify this,
btw in case of did we should also check tid.



-- 
Luiz Augusto von Dentz

Re: [PATCHv3 6/7] android/tester: Fix HIDHost cases sending fixed tid sdp responses

[Tyszkowski Jakub]

Hi Luiz

On 09/09/2014 11:10 AM, Luiz Augusto von Dentz wrote:
> Hi Jakub,
>
> On Tue, Sep 9, 2014 at 10:47 AM, Jakub Tyszkowski
> <jakub.tyszkowski@tieto.com> wrote:
>> Multiple cases were affected because of hardcoded transaction id for
>> emulated remote's SDP responses.
>>
>> This resulted in the following error in the daemon:
>>          bluetoothd[13486]: sdp_process: Protocol error.
>> ---
>>   android/tester-hidhost.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
>> index abff837..d5741f9 100644
>> --- a/android/tester-hidhost.c
>> +++ b/android/tester-hidhost.c
>> @@ -21,6 +21,7 @@
>>   #include "tester-main.h"
>>
>>   #include "android/utils.h"
>> +#include "src/shared/util.h"
>>
>>   #define HID_GET_REPORT_PROTOCOL                0x60
>>   #define HID_GET_BOOT_PROTOCOL          0x61
>> @@ -214,15 +215,24 @@ static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
>>          struct bthost *bthost = hciemu_client_get_host(t_data->hciemu);
>>          struct emu_cid_data *cid_data = user_data;
>>          struct raw_dataset *sdp_data = cid_data->user_data;
>> +       const uint8_t *req_buf = data;
>> +       uint8_t *sdp_buf;
>>
>> -       if (!memcmp(did_req_pdu, data, len)) {
>> +       if (!memcmp(did_req_pdu, req_buf, len)) {
>>                  bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
>>                                          did_rsp_pdu, sizeof(did_rsp_pdu));
>>                  return;
>>          }
>>
>> +       /* Get transaction ID from the request */
>> +       sdp_buf = g_memdup(sdp_data->pdu, sdp_data->len);
>> +       sdp_buf[1] = req_buf[1];
>> +       sdp_buf[2] = req_buf[2];
>> +
>>          bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
>> -                                       sdp_data->pdu, sdp_data->len);
>> +                                                       sdp_buf, sdp_data->len);
>> +
>> +       g_free(sdp_buf);
>>   }
>
> Do we really need this copy? Wouldn't it be enough not to mark it as
> const and just change in place? Anyway this is also broke in AVRCP,  I

Yes, It looks like we could use non-const pdu array to avoid copying.
First idea was not leave any trace of previous test case but we broke it 
and use non-const global variables as storage elsewhere anyway. Not sure 
which way we should go to be consistent.

> have been planning to handle this but perhaps we should unify this,
> btw in case of did we should also check tid.

Agree, memcmp checks for did request with tid == 0 and responses with 
tid == 0. Luckily for now its always 0 in request, but we should fix 
this anyway. ;)

It looks like we need a pair of helpers, one for matching pdu and one 
for sending responses to unify this 'tid' issue.

Is this something we can fix later in all testers ,by providing helpers 
in tester-main or should It be part of this patch set?

Regards,
Jakub

Re: [PATCHv3 1/7] android/tester: Allow HIDHost to use custom sdp response

[Szymon Janc]

Hi Luiz, Jakub,

On Tuesday 09 of September 2014 12:02:26 Luiz Augusto von Dentz wrote:
> Hi Jakub,
> 
> On Tue, Sep 9, 2014 at 10:46 AM, Jakub Tyszkowski
> <jakub.tyszkowski@tieto.com> wrote:
> > This is needed to test security levels for different devices. So far we
> > had Mouse device but, we need to test security level elevation for
> > keyboards.
> > ---
> >  android/tester-hidhost.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/android/tester-hidhost.c b/android/tester-hidhost.c
> > index c7e3a67..b454db4 100644
> > --- a/android/tester-hidhost.c
> > +++ b/android/tester-hidhost.c
> > @@ -55,10 +55,17 @@ struct emu_cid_data {
> >         uint16_t ctrl_cid;
> >         uint16_t intr_handle;
> >         uint16_t intr_cid;
> > +
> > +       void *user_data;
> >  };
> >
> >  static struct emu_cid_data cid_data;
> >
> > +struct raw_dataset {
> > +       const void *pdu;
> > +       int len;
> > +};
> > +
> >  static const uint8_t did_req_pdu[] = { 0x06, /* PDU id */
> >                         0x00, 0x00, /* Transaction id */
> >                         0x00, 0x0f, /* Req length */
> > @@ -128,11 +135,17 @@ static const uint8_t hid_rsp_pdu[] = { 0x07, /* PDU id */
> >                         0x00, 0x09, 0x02, 0x0e, 0x28, 0x01,
> >                         0x00 }; /* no continuation */
> >
> > +static struct raw_dataset hid_rsp_data = {
> > +       .pdu = hid_rsp_pdu,
> > +       .len = sizeof(hid_rsp_pdu),
> > +};
> > +
> 
> Maybe in the future we can create helper function for PDU matching,
> perhaps in tester-main.c, so the profile just provide the PDU table
> that way we don't have to reimplement this over and over again.

I agree that this would be nice to have. Jakub, please have a look at this.

> >  static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
> >  {
> >         struct test_data *t_data = tester_get_data();
> >         struct bthost *bthost = hciemu_client_get_host(t_data->hciemu);
> >         struct emu_cid_data *cid_data = user_data;
> > +       struct raw_dataset *sdp_data = cid_data->user_data;
> >
> >         if (!memcmp(did_req_pdu, data, len)) {
> >                 bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
> > @@ -141,7 +154,7 @@ static void hid_sdp_cid_hook_cb(const void *data, uint16_t len, void *user_data)
> >         }
> >
> >         bthost_send_cid(bthost, cid_data->sdp_handle, cid_data->sdp_cid,
> > -                                       hid_rsp_pdu, sizeof(hid_rsp_pdu));
> > +                                       sdp_data->pdu, sdp_data->len);
> >  }
> >  static void hid_sdp_search_cb(uint16_t handle, uint16_t cid, void *user_data)
> >  {
> > @@ -150,6 +163,7 @@ static void hid_sdp_search_cb(uint16_t handle, uint16_t cid, void *user_data)
> >
> >         cid_data.sdp_handle = handle;
> >         cid_data.sdp_cid = cid;
> > +       cid_data.user_data = user_data;
> >
> >         bthost_add_cid_hook(bthost, handle, cid, hid_sdp_cid_hook_cb,
> >                                                                 &cid_data);
> > @@ -267,7 +281,7 @@ static void hid_intr_connect_cb(uint16_t handle, uint16_t cid, void *user_data)
> >  static struct emu_set_l2cap_data l2cap_setup_sdp_data = {
> >         .psm = 1,
> >         .func = hid_sdp_search_cb,
> > -       .user_data = NULL,
> > +       .user_data = &hid_rsp_data,
> >  };
> >
> >  /* Emulate Control Channel (PSM = 17) */
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> 

-- 
Best regards, 
Szymon Janc