This is an automated email from the ASF dual-hosted git repository.
andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git
The following commit(s) were added to refs/heads/master by this push:
new 554e0634 nimble/ll: Fix encryption pause/restart procedure
554e0634 is described below
commit 554e063421f07fe2a9028ea2b389a503e1d24ee0
Author: Andrzej Kaczmarek <[email protected]>
AuthorDate: Thu Jun 13 18:32:24 2024 +0200
nimble/ll: Fix encryption pause/restart procedure
This fixes 2 issues in encrpytion pause/restart procedure (i.e. key
refresh) initiated in central role:
1. We transition to "paused" encryption state when LL_PAUSE_ENC_RSP is
txd, but that is done after we try to enqueue LL_ENC_REQ. This means
LL_ENC_REQ is put at the end of tx queue. By conicidence this makes
order of PDUs correct when there's only LL_PAUSE_ENC_RSP on tx queue,
but it fails if there was an ACL packet already queued. In such case
LL_ENC_REQ is queued after that ACL packet which means neither will
be sent - ACL packet cannot be sent because we have no encryption,
LL_ENC_REQ cannot be sent because there's ACL packet in front.
2. We do not check if LL_ENC_REQ was properly queued (i.e. if mbuf was
allocated for PDU) so it may happen that LL_ENC_REQ will never be
queued.
In both scenarios the connection will timeout eventually since
encryption restart procedure cannot be completed.
This fix ensures that LL_ENC_REQ is queued properly by using a "pending"
flag that is evaluated when checking for pending procedures.
---
nimble/controller/include/controller/ble_ll_conn.h | 1 +
nimble/controller/src/ble_ll_ctrl.c | 38 ++++++++++++----------
2 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/nimble/controller/include/controller/ble_ll_conn.h
b/nimble/controller/include/controller/ble_ll_conn.h
index 88e7934b..7d6af874 100644
--- a/nimble/controller/include/controller/ble_ll_conn.h
+++ b/nimble/controller/include/controller/ble_ll_conn.h
@@ -108,6 +108,7 @@ struct ble_ll_conn_sm_flags {
uint32_t encrypted : 1;
uint32_t encrypt_ltk_req : 1;
uint32_t encrypt_event_sent : 1;
+ uint32_t pending_encrypt_restart : 1;
uint32_t version_ind_txd : 1;
uint32_t version_ind_rxd : 1;
uint32_t features_rxd : 1;
diff --git a/nimble/controller/src/ble_ll_ctrl.c
b/nimble/controller/src/ble_ll_ctrl.c
index d2b8a31b..da12a5f3 100644
--- a/nimble/controller/src/ble_ll_ctrl.c
+++ b/nimble/controller/src/ble_ll_ctrl.c
@@ -2684,6 +2684,9 @@ ble_ll_ctrl_proc_start(struct ble_ll_conn_sm *connsm, int
ctrl_proc,
void
ble_ll_ctrl_chk_proc_start(struct ble_ll_conn_sm *connsm)
{
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION)
+ struct os_mbuf *om;
+#endif
int i;
/* XXX: TODO new rules! Cannot start certain control procedures if other
@@ -2706,6 +2709,22 @@ ble_ll_ctrl_chk_proc_start(struct ble_ll_conn_sm *connsm)
return;
}
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION)
+ if (connsm->flags.pending_encrypt_restart) {
+ /* This flag should only be set after LL_PAUSE_ENC_RSP was txd from
+ * central and there should be already active encryption procedure
+ * ongoing. We need to restart encryption to complete key refresh.
+ */
+ BLE_LL_ASSERT(connsm->cur_ctrl_proc == BLE_LL_CTRL_PROC_ENCRYPT);
+ BLE_LL_ASSERT(IS_PENDING_CTRL_PROC(connsm, BLE_LL_CTRL_PROC_ENCRYPT));
+
+ om = ble_ll_ctrl_proc_init(connsm, BLE_LL_CTRL_PROC_ENCRYPT, NULL);
+ if (om) {
+ connsm->flags.pending_encrypt_restart = 0;
+ }
+ }
+#endif
+
/* If there is a running procedure or no pending, do nothing */
if ((connsm->cur_ctrl_proc == BLE_LL_CTRL_PROC_IDLE) &&
(connsm->pending_ctrl_procs != 0)) {
@@ -2755,9 +2774,6 @@ ble_ll_ctrl_rx_pdu(struct ble_ll_conn_sm *connsm, struct
os_mbuf *om)
uint8_t *dptr;
uint8_t *rspbuf;
uint8_t *rspdata;
-#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION)
- int restart_encryption;
-#endif
int rc = 0;
uint8_t rsp_opcode = 0;
@@ -2802,10 +2818,6 @@ ble_ll_ctrl_rx_pdu(struct ble_ll_conn_sm *connsm, struct
os_mbuf *om)
ble_ll_trace_u32x2(BLE_LL_TRACE_ID_CTRL_RX, opcode, len);
-#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION)
- restart_encryption = 0;
-#endif
-
/* If opcode comes from reserved value or CtrlData fields is invalid
* we shall respond with LL_UNKNOWN_RSP
*/
@@ -2956,9 +2968,6 @@ ble_ll_ctrl_rx_pdu(struct ble_ll_conn_sm *connsm, struct
os_mbuf *om)
break;
case BLE_LL_CTRL_PAUSE_ENC_RSP:
rsp_opcode = ble_ll_ctrl_rx_pause_enc_rsp(connsm);
- if (rsp_opcode == BLE_LL_CTRL_PAUSE_ENC_RSP) {
- restart_encryption = 1;
- }
break;
#endif
case BLE_LL_CTRL_PING_REQ:
@@ -3032,13 +3041,6 @@ ll_ctrl_send_rsp:
}
len = g_ble_ll_ctrl_pkt_lengths[rsp_opcode] + 1;
ble_ll_conn_enqueue_pkt(connsm, om, BLE_LL_LLID_CTRL, len);
-#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION)
- if (restart_encryption) {
- /* XXX: what happens if this fails? Meaning we cant allocate
- mbuf? */
- ble_ll_ctrl_proc_init(connsm, BLE_LL_CTRL_PROC_ENCRYPT, NULL);
- }
-#endif
}
#if MYNEWT_VAL(BLE_LL_CONN_INIT_AUTO_DLE)
@@ -3206,6 +3208,8 @@ ble_ll_ctrl_tx_done(struct os_mbuf *txpdu, struct
ble_ll_conn_sm *connsm)
case BLE_LL_CTRL_PAUSE_ENC_RSP:
if (connsm->conn_role == BLE_LL_CONN_ROLE_PERIPHERAL) {
connsm->enc_data.enc_state = CONN_ENC_S_PAUSE_ENC_RSP_WAIT;
+ } else {
+ connsm->flags.pending_encrypt_restart = 1;
}
break;
#endif
