Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
xiaoxiang781216 merged PR #16364: URL: https://github.com/apache/nuttx/pull/16364 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
robertc2000 commented on code in PR #16364: URL: https://github.com/apache/nuttx/pull/16364#discussion_r2110927790 ## wireless/bluetooth/bt_smp.h: ## @@ -163,6 +163,29 @@ begin_packed_struct struct bt_smp_security_request_s uint8_t auth_req; } end_packed_struct; +begin_packed_struct struct bt_smp_auth_cb_s Review Comment: OK, you are right. I removed begin_packed_struct. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
xiaoxiang781216 commented on code in PR #16364: URL: https://github.com/apache/nuttx/pull/16364#discussion_r2110709177 ## wireless/bluetooth/bt_smp.h: ## @@ -163,6 +163,29 @@ begin_packed_struct struct bt_smp_security_request_s uint8_t auth_req; } end_packed_struct; +begin_packed_struct struct bt_smp_auth_cb_s Review Comment: normally, begin_packed_struct is attached to structure which define the wire or hardware register layout, but bt_smp_auth_cb_s isn't this case. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
robertc2000 commented on code in PR #16364: URL: https://github.com/apache/nuttx/pull/16364#discussion_r2109801930 ## wireless/bluetooth/bt_smp.h: ## @@ -163,6 +163,29 @@ begin_packed_struct struct bt_smp_security_request_s uint8_t auth_req; } end_packed_struct; +begin_packed_struct struct bt_smp_auth_cb_s Review Comment: It's a structure containing callbacks that can be implemented by application layer. I think they can be useful to notify the user of internal actions like pairing failure, cancelation, success or passkey generation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
xiaoxiang781216 commented on code in PR #16364:
URL: https://github.com/apache/nuttx/pull/16364#discussion_r2106273345
##
wireless/bluetooth/bt_smp.c:
##
@@ -71,6 +71,16 @@
/* SMP channel specific context */
+enum pairing_method
Review Comment:
```suggestion
enum smp_pairing_method_e
```
##
wireless/bluetooth/bt_smp.c:
##
@@ -576,17 +637,79 @@ static int smp_init(struct bt_smp_s *smp)
return 0;
}
+static enum pairing_method smp_get_pairing_method(uint8_t local_io,
+ uint8_t remote_io,
+ uint8_t local_auth,
+ uint8_t remote_auth)
+{
+ bool local_mitm = (local_auth & BT_SMP_AUTH_MITM);
+ bool remote_mitm = (remote_auth & BT_SMP_AUTH_MITM);
+ bool mitm_requested = local_mitm || remote_mitm;
+
+ wlinfo("Local IO: %d, Remote IO: %d, MITM Req: %d\n", local_io, remote_io,
+ mitm_requested);
+
+ /* Mapping based on Core Spec v4.2, Vol 3, Part H, Table 2.8
+ * and Figure 2.7 flow
+ */
+
+ switch (local_io)
+{
+ case BT_SMP_IO_DISPLAY_ONLY:
+switch (remote_io)
+ {
+case BT_SMP_IO_DISPLAY_ONLY:
+case BT_SMP_IO_NO_INPUT_OUTPUT:
+ return PAIRING_METHOD_JUST_WORKS;
+case BT_SMP_IO_KEYBOARD_ONLY:
+case BT_SMP_IO_KEYBOARD_DISPLAY:
+case BT_SMP_IO_DISPLAY_YESNO:
+ return mitm_requested ? PAIRING_METHOD_PASSKEY_DISPLAY
+: PAIRING_METHOD_JUST_WORKS;
+default:
+ return PAIRING_METHOD_NOT_SUPPORTED;
+ }
+break;
+ case BT_SMP_IO_NO_INPUT_OUTPUT:
+return PAIRING_METHOD_JUST_WORKS; /* Cannot support MITM */
+ default:
+wlwarn("Unhandled local IO Cap %d\n", local_io);
+return PAIRING_METHOD_JUST_WORKS;
+}
+}
+
+static bool method_provides_mitm(enum pairing_method method)
+{
+ return (method == PAIRING_METHOD_PASSKEY_DISPLAY ||
+method == PAIRING_METHOD_PASSKEY_INPUT);
+}
+
+static void smp_passkey_to_tk(uint32_t passkey, uint8_t *tk)
+{
+ memset(tk, 0, 16);
+ tk[0] = passkey & 0xff;
+ tk[1] = (passkey >> 8) & 0xff;
+ tk[2] = (passkey >> 16) & 0xff;
+ tk[3] = (passkey >> 24) & 0xff;
+}
+
static uint8_t smp_pairing_req(FAR struct bt_conn_s *conn,
FAR struct bt_buf_s *buf)
{
FAR struct bt_smp_pairing_s *req = (FAR void *)buf->data;
FAR struct bt_smp_pairing_s *rsp;
FAR struct bt_buf_s *rsp_buf;
FAR struct bt_smp_s *smp = conn->smp;
- uint8_t auth;
int ret;
Review Comment:
move after line 705
##
wireless/bluetooth/bt_smp.h:
##
@@ -163,6 +163,29 @@ begin_packed_struct struct bt_smp_security_request_s
uint8_t auth_req;
} end_packed_struct;
+begin_packed_struct struct bt_smp_auth_cb_s
Review Comment:
why need patch the internal structure
##
wireless/bluetooth/bt_smp.c:
##
@@ -1062,30 +1401,49 @@ static uint8_t smp_security_request(FAR struct
bt_conn_s *conn,
{
FAR struct bt_smp_security_request_s *req = (FAR void *)buf->data;
FAR struct bt_keys_s *keys;
- uint8_t auth;
+ uint8_t slave_auth_req = req->auth_req & BT_SMP_AUTH_MASK;
- wlinfo("\n");
+ wlinfo("Security Request received (req=0x%02x)\n", slave_auth_req);
keys = bt_keys_find(BT_KEYS_LTK, &conn->dst);
- if (!keys)
+ if (keys)
{
- goto pair;
-}
+ wlinfo("Found existing LTK (level=%d)\n", keys->ltk.level);
Review Comment:
move after line 1414
##
wireless/bluetooth/bt_smp.c:
##
@@ -576,17 +637,79 @@ static int smp_init(struct bt_smp_s *smp)
return 0;
}
+static enum pairing_method smp_get_pairing_method(uint8_t local_io,
+ uint8_t remote_io,
+ uint8_t local_auth,
+ uint8_t remote_auth)
+{
+ bool local_mitm = (local_auth & BT_SMP_AUTH_MITM);
+ bool remote_mitm = (remote_auth & BT_SMP_AUTH_MITM);
+ bool mitm_requested = local_mitm || remote_mitm;
+
+ wlinfo("Local IO: %d, Remote IO: %d, MITM Req: %d\n", local_io, remote_io,
+ mitm_requested);
+
+ /* Mapping based on Core Spec v4.2, Vol 3, Part H, Table 2.8
+ * and Figure 2.7 flow
+ */
+
+ switch (local_io)
+{
+ case BT_SMP_IO_DISPLAY_ONLY:
+switch (remote_io)
+ {
+case BT_SMP_IO_DISPLAY_ONLY:
+case BT_SMP_IO_NO_INPUT_OUTPUT:
+ return PAIRING_METHOD_JUST_WORKS;
+case BT_SMP_IO_KEYBOARD_ONLY:
+case BT_SMP_IO_KEYBOARD_DISPLAY:
+case BT_SMP_IO_DISPLAY_YESNO:
+ return mitm_requested ? PAIRING_METHOD_PASSKEY_DISPLAY
+: PAIRING_METHOD_JUST_WORKS;
+default:
+ return PAIRING_METHOD_NOT_SUPPORTE
Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
xiaoxiang781216 commented on PR #16364: URL: https://github.com/apache/nuttx/pull/16364#issuecomment-2907754651 @robertc2000 please the spelling error: ``` 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/wireless/bluetooth/bt_smp.c:166: pres ==> press /home/runner/work/nuttx/nuttx/nuttx/wireless/bluetooth/bt_smp.c:494: pres ==> press /home/runner/work/nuttx/nuttx/nuttx/wireless/bluetooth/bt_smp.c:504: pres ==> press /home/runner/work/nuttx/nuttx/nuttx/wireless/bluetooth/bt_smp.c:504: pres ==> press /home/runner/work/nuttx/nuttx/nuttx/wireless/bluetooth/bt_smp.c:506: pres ==> press /home/runner/work/nuttx/nuttx/nuttx/wireless/bluetooth/bt_smp.c:511: pres ==> press ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
robertc2000 commented on PR #16364: URL: https://github.com/apache/nuttx/pull/16364#issuecomment-2907689995 Fixed all coding style checks! (though there is only 1 test failing, not sure why...) Can I get another review, please? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
xiaoxiang781216 commented on code in PR #16364:
URL: https://github.com/apache/nuttx/pull/16364#discussion_r2086920818
##
wireless/bluetooth/bt_smp.c:
##
@@ -600,6 +710,45 @@ static uint8_t smp_pairing_req(FAR struct bt_conn_s *conn,
return ret;
}
+ // Perform pairing method selection before sending response
+ smp->selected_method = smp_get_pairing_method(local_io_cap,
req->io_capability,
+local_auth_req, req->auth_req);
Review Comment:
align
##
wireless/bluetooth/bt_smp.h:
##
@@ -172,5 +189,6 @@ bool bt_smp_irk_matches(FAR const uint8_t irk[16],
int bt_smp_send_pairing_req(FAR struct bt_conn_s *conn);
int bt_smp_send_security_req(FAR struct bt_conn_s *conn);
int bt_smp_initialize(void);
+void bt_smp_auth_cb_register(const struct bt_smp_auth_cb_s *cb);
Review Comment:
add FAR
##
wireless/bluetooth/bt_smp.c:
##
@@ -687,20 +828,53 @@ static uint8_t smp_pairing_rsp(FAR struct bt_conn_s *conn,
{
struct bt_smp_pairing_s *rsp = (FAR void *)buf->data;
struct bt_smp_s *smp = conn->smp;
+ uint8_t local_io_cap = CONFIG_BLUETOOTH_SMP_IO_CAPABILITY;
+ uint8_t local_auth_req = smp->preq[3];
- wlinfo("\n");
+ wlinfo("Pairing Response Received\n");
if ((rsp->max_key_size > BT_SMP_MAX_ENC_KEY_SIZE) ||
(rsp->max_key_size < BT_SMP_MIN_ENC_KEY_SIZE))
{
return BT_SMP_ERR_ENC_KEY_SIZE;
}
- smp->local_dist &= rsp->init_key_dist;
- smp->remote_dist &= rsp->resp_key_dist;
+ smp->selected_method = smp_get_pairing_method(local_io_cap,
rsp->io_capability,
+local_auth_req, rsp->auth_req);
- /* Store rsp for later use */
+ wlinfo("Selected pairing method: %d\n", smp->selected_method);
+ if (conn->sec_level >= BT_SECURITY_HIGH &&
!method_provides_mitm(smp->selected_method))
+{
+ wlerr("ERROR: Cannot achieve HIGH security (MITM) with selected method
%d\n", smp->selected_method);
+ return BT_SMP_ERR_AUTH_REQUIREMENTS;
+}
+ if (smp->selected_method == PAIRING_METHOD_NOT_SUPPORTED)
+{
+ wlerr("ERROR: Pairing method for IO Caps %d/%d not supported\n",
local_io_cap, rsp->io_capability);
+ return BT_SMP_ERR_PAIRING_NOTSUPP;
+}
+
+ if (smp->selected_method == PAIRING_METHOD_PASSKEY_DISPLAY) {
+uint32_t passkey;
+le_rand(&passkey, sizeof(passkey));
+passkey %= 100; // 6 digit passkey
+smp->passkey = passkey;
+wlinfo("Using Passkey Display method. Generated Passkey: %06u\n",
(unsigned int) passkey);
+smp_passkey_to_tk(passkey, smp->tk);
+if (g_smp_auth_cb && g_smp_auth_cb->passkey_display) {
+ g_smp_auth_cb->passkey_display(conn, passkey);
+}
+ } else if (smp->selected_method == PAIRING_METHOD_JUST_WORKS) {
+wlwarn("Using Just Works method.\n");
+memset(smp->tk, 0, sizeof(smp->tk));
+ } else {
+wlerr("ERROR: Invalid selected method %d\n", smp->selected_method);
+return BT_SMP_ERR_UNSPECIFIED;
+ }
+
+ smp->local_dist &= rsp->init_key_dist;
+ smp->remote_dist &= rsp->resp_key_dist;
Review Comment:
please run `tools/checkpatch.sh -g HEAD~...HEAD` and fix all warning before
updating the patch
##
wireless/bluetooth/bt_smp.c:
##
@@ -576,17 +631,72 @@ static int smp_init(struct bt_smp_s *smp)
return 0;
}
+static enum pairing_method smp_get_pairing_method(uint8_t local_io, uint8_t
remote_io,
+ uint8_t local_auth, uint8_t
remote_auth)
Review Comment:
align
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Bluetooth SMP: added support for Legacy pairing (MITM) with passkey [nuttx]
acassis commented on code in PR #16364:
URL: https://github.com/apache/nuttx/pull/16364#discussion_r2086397247
##
wireless/bluetooth/bt_smp.c:
##
@@ -71,6 +71,15 @@
/* SMP channel specific context */
+enum pairing_method {
+PAIRING_METHOD_JUST_WORKS,
+PAIRING_METHOD_PASSKEY_DISPLAY, // Local displays, remote inputs
+PAIRING_METHOD_PASSKEY_INPUT, // Local inputs, remote displays
Review Comment:
Please fix code style issues, there are many C++ "//" comments and even "{"
starting at end of the line
##
wireless/bluetooth/bt_smp.c:
##
@@ -687,20 +828,53 @@ static uint8_t smp_pairing_rsp(FAR struct bt_conn_s *conn,
{
struct bt_smp_pairing_s *rsp = (FAR void *)buf->data;
struct bt_smp_s *smp = conn->smp;
+ uint8_t local_io_cap = CONFIG_BLUETOOTH_SMP_IO_CAPABILITY;
+ uint8_t local_auth_req = smp->preq[3];
- wlinfo("\n");
+ wlinfo("Pairing Response Received\n");
if ((rsp->max_key_size > BT_SMP_MAX_ENC_KEY_SIZE) ||
(rsp->max_key_size < BT_SMP_MIN_ENC_KEY_SIZE))
{
return BT_SMP_ERR_ENC_KEY_SIZE;
}
- smp->local_dist &= rsp->init_key_dist;
- smp->remote_dist &= rsp->resp_key_dist;
+ smp->selected_method = smp_get_pairing_method(local_io_cap,
rsp->io_capability,
+local_auth_req, rsp->auth_req);
- /* Store rsp for later use */
+ wlinfo("Selected pairing method: %d\n", smp->selected_method);
+ if (conn->sec_level >= BT_SECURITY_HIGH &&
!method_provides_mitm(smp->selected_method))
+{
+ wlerr("ERROR: Cannot achieve HIGH security (MITM) with selected method
%d\n", smp->selected_method);
+ return BT_SMP_ERR_AUTH_REQUIREMENTS;
+}
+ if (smp->selected_method == PAIRING_METHOD_NOT_SUPPORTED)
+{
+ wlerr("ERROR: Pairing method for IO Caps %d/%d not supported\n",
local_io_cap, rsp->io_capability);
+ return BT_SMP_ERR_PAIRING_NOTSUPP;
+}
+
+ if (smp->selected_method == PAIRING_METHOD_PASSKEY_DISPLAY) {
+uint32_t passkey;
+le_rand(&passkey, sizeof(passkey));
+passkey %= 100; // 6 digit passkey
+smp->passkey = passkey;
+wlinfo("Using Passkey Display method. Generated Passkey: %06u\n",
(unsigned int) passkey);
+smp_passkey_to_tk(passkey, smp->tk);
+if (g_smp_auth_cb && g_smp_auth_cb->passkey_display) {
+ g_smp_auth_cb->passkey_display(conn, passkey);
+}
+ } else if (smp->selected_method == PAIRING_METHOD_JUST_WORKS) {
+wlwarn("Using Just Works method.\n");
+memset(smp->tk, 0, sizeof(smp->tk));
+ } else {
+wlerr("ERROR: Invalid selected method %d\n", smp->selected_method);
+return BT_SMP_ERR_UNSPECIFIED;
+ }
+
+ smp->local_dist &= rsp->init_key_dist;
+ smp->remote_dist &= rsp->resp_key_dist;
Review Comment:
@robertc2000 "{" should be in a new line, without line of code. Seems like
nxstyle failed to catch it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
