Bluetooth: controller: Fix central enc termination

Terminate connection with a MIC failure if an unexpected control PDU
is received during the Encryption Start procedure.

Add a greedy option to pdu_is_expected() to make sure the procedure
processes all unexpected control PDU in all cases.

Add unit test inspired by Bluetooth Qualification test
LL/SEC/CEN/BV-14-C,
Central Receiving unexpected PDU during encryption start

Signed-off-by: Thomas Ebert Hansen <thoh@oticon.com>

subsys/bluetooth/controller/ll_sw/ull_llcp.c

@@ -1000,7 +1000,7 @@ void ull_cp_cte_req_set_disable(struct ll_conn *conn)
 
 static bool pdu_is_expected(struct pdu_data *pdu, struct proc_ctx *ctx)
 {
-	return ctx->rx_opcode == pdu->llctrl.opcode;
+	return (ctx->rx_opcode == pdu->llctrl.opcode || ctx->rx_greedy);
 }
 
 static bool pdu_is_unknown(struct pdu_data *pdu, struct proc_ctx *ctx)

subsys/bluetooth/controller/ll_sw/ull_llcp_enc.c

@@ -404,6 +404,15 @@ static void lp_enc_st_wait_rx_enc_rsp(struct ll_conn *conn, struct proc_ctx *ctx
 		/* Pause Rx data */
 		ull_conn_pause_rx_data(conn);
 		lp_enc_store_s(conn, ctx, pdu);
+
+		/* After the Central has received the LL_ENC_RSP PDU,
+		 * only PDUs related to this procedure are valid, and invalids should
+		 * result in disconnect.
+		 * to achieve this enable the greedy RX behaviour, such that
+		 * all PDU's end up in this FSM.
+		 */
+		ctx->rx_greedy = 1U;
+
 		/* Wait for LL_START_ENC_REQ */
 		ctx->rx_opcode = PDU_DATA_LLCTRL_TYPE_START_ENC_REQ;
 		ctx->state = LP_ENC_STATE_WAIT_RX_START_ENC_REQ;
@@ -450,6 +459,9 @@ static void lp_enc_st_wait_rx_start_enc_req(struct ll_conn *conn, struct proc_ct
 		/* Resume possibly paused remote procedure */
 		llcp_rr_resume(conn);
 
+		/* Disable the greedy behaviour */
+		ctx->rx_greedy = 0U;
+
 		lp_enc_complete(conn, ctx, evt, param);
 		break;
 	default:
@@ -485,6 +497,9 @@ static void lp_enc_st_wait_rx_start_enc_rsp(struct ll_conn *conn, struct proc_ct
 		/* Resume possibly paused remote procedure */
 		llcp_rr_resume(conn);
 
+		/* Disable the greedy behaviour */
+		ctx->rx_greedy = 0U;
+
 		lp_enc_complete(conn, ctx, evt, param);
 		break;
 	default:
@@ -631,7 +646,23 @@ void llcp_lp_enc_rx(struct ll_conn *conn, struct proc_ctx *ctx, struct node_rx_p
 		break;
 	default:
 		/* Unknown opcode */
-		LL_ASSERT(0);
+
+		/*
+		 * BLUETOOTH CORE SPECIFICATION Version 5.3
+		 * Vol 6, Part B, 5.1.3.1 Encryption Start procedure
+		 *
+		 * [...]
+		 *
+		 * If, at any time during the encryption start procedure after the Peripheral has
+		 * received the LL_ENC_REQ PDU or the Central has received the
+		 * LL_ENC_RSP PDU, the Link Layer of the Central or the Peripheral receives an
+		 * unexpected Data Physical Channel PDU from the peer Link Layer, it shall
+		 * immediately exit the Connection state, and shall transition to the Standby state.
+		 * The Host shall be notified that the link has been disconnected with the error
+		 * code Connection Terminated Due to MIC Failure (0x3D).
+		 */
+
+		conn->llcp_terminate.reason_final = BT_HCI_ERR_TERM_DUE_TO_MIC_FAIL;
 	}
 }
 

subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h

@@ -134,6 +134,9 @@ struct proc_ctx {
 	/* Expected opcode to be received next */
 	enum pdu_data_llctrl_type rx_opcode;
 
+	/* Greedy RX (used for central encryption) */
+	uint8_t rx_greedy;
+
 	/* Last transmitted opcode used for unknown/reject */
 	enum pdu_data_llctrl_type tx_opcode;
 

tests/bluetooth/controller/ctrl_encrypt/src/main.c

@@ -935,6 +935,138 @@ void test_encryption_start_central_loc_no_ltk_2(void)
 				  "Free CTX buffers %d", ctx_buffers_free());
 }
 
+/* +-----+                     +-------+              +-----+
+ * | UT  |                     | LL_A  |              | LT  |
+ * +-----+                     +-------+              +-----+
+ *    |                            |                     |
+ *    | Initiate                   |                     |
+ *    | Encryption Start Proc.     |                     |
+ *    |--------------------------->|                     |
+ *    |         -----------------\ |                     |
+ *    |         | Empty Tx queue |-|                     |
+ *    |         |----------------| |                     |
+ *    |                            |                     |
+ *    |                            | LL_ENC_REQ          |
+ *    |                            |-------------------->|
+ *    |                            |                     |
+ *    |                            |          LL_ENC_RSP |
+ *    |                            |<--------------------|
+ *    |                            |                     |
+ *    |                            |      LL_VERSION_IND |
+ *    |                            |<--------------------|
+ *    |                            |                     |
+ *    |     Encryption Start Proc. |                     |
+ *    |                   Complete |                     |
+ *    |<---------------------------|                     |
+ *    |                            |                     |
+ */
+void test_encryption_start_central_loc_mic(void)
+{
+	uint8_t err;
+	struct node_tx *tx;
+
+	const uint8_t rand[] = { RAND };
+	const uint8_t ediv[] = { EDIV };
+	const uint8_t ltk[] = { LTK };
+
+	/* Prepare expected LL_ENC_REQ */
+	struct pdu_data_llctrl_enc_req exp_enc_req = {
+		.rand = { RAND },
+		.ediv = { EDIV },
+		.skdm = { SKDM },
+		.ivm = { IVM },
+	};
+
+	/* Prepare LL_ENC_RSP */
+	struct pdu_data_llctrl_enc_rsp enc_rsp = { .skds = { SKDS }, .ivs = { IVS } };
+
+	struct pdu_data_llctrl_version_ind remote_version_ind = {
+		.version_number = 0x55,
+		.company_id = 0xABCD,
+		.sub_version_number = 0x1234,
+	};
+
+	/* Prepare mocked call to lll_csrand_get */
+	ztest_returns_value(lll_csrand_get, sizeof(exp_enc_req.skdm) + sizeof(exp_enc_req.ivm));
+	ztest_return_data(lll_csrand_get, buf, exp_enc_req.skdm);
+	ztest_expect_value(lll_csrand_get, len, sizeof(exp_enc_req.skdm) + sizeof(exp_enc_req.ivm));
+
+	/* Role */
+	test_set_role(&conn, BT_HCI_ROLE_CENTRAL);
+
+	/* Connect */
+	ull_cp_state_set(&conn, ULL_CP_CONNECTED);
+
+	/* Check state */
+	CHECK_RX_PE_STATE(conn, RESUMED, UNENCRYPTED); /* Rx unenc. */
+	CHECK_TX_PE_STATE(conn, RESUMED, UNENCRYPTED); /* Tx unenc. */
+
+	/* Initiate an Encryption Start Procedure */
+	err = ull_cp_encryption_start(&conn, rand, ediv, ltk);
+	zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL);
+
+	/* Prepare */
+	event_prepare(&conn);
+
+	/* Tx Queue should have one LL Control PDU */
+	lt_rx(LL_ENC_REQ, &conn, &tx, &exp_enc_req);
+	lt_rx_q_is_empty(&conn);
+
+	/* Check state */
+	CHECK_RX_PE_STATE(conn, RESUMED, UNENCRYPTED); /* Rx unenc. */
+	CHECK_TX_PE_STATE(conn, PAUSED, UNENCRYPTED); /* Tx paused & unenc. */
+
+	/* Release Tx */
+	ull_cp_release_tx(&conn, tx);
+
+	/* Rx */
+	lt_tx(LL_ENC_RSP, &conn, &enc_rsp);
+
+	/* Rx */
+	lt_tx(LL_VERSION_IND, &conn, &remote_version_ind);
+
+	/* Done */
+	event_done(&conn);
+
+	/* Check state */
+	CHECK_RX_PE_STATE(conn, PAUSED, UNENCRYPTED); /* Rx paused & unenc. */
+	CHECK_TX_PE_STATE(conn, PAUSED, UNENCRYPTED); /* Tx paused & unenc. */
+
+	/* There should not be a host notification */
+	ut_rx_q_is_empty();
+
+	/**/
+	zassert_equal(conn.llcp_terminate.reason_final, BT_HCI_ERR_TERM_DUE_TO_MIC_FAIL,
+		      "Expected termination due to MIC failure");
+
+	/*
+	 * For a 40s procedure response timeout with a connection interval of
+	 * 7.5ms, a total of 5333.33 connection events are needed, verify that
+	 * the state doesn't change for that many invocations.
+	 */
+	for (int n = 5334; n > 0; n--) {
+		/* Prepare */
+		event_prepare(&conn);
+
+		/* Tx Queue should NOT have a LL Control PDU */
+		lt_rx_q_is_empty(&conn);
+
+		/* Done */
+		event_done(&conn);
+
+		/* Check state */
+		CHECK_RX_PE_STATE(conn, PAUSED, UNENCRYPTED); /* Rx paused & unenc. */
+		CHECK_TX_PE_STATE(conn, PAUSED, UNENCRYPTED); /* Tx paused & unenc. */
+
+		/* There should NOT be a host notification */
+		ut_rx_q_is_empty();
+	}
+
+	/* Note that for this test the context is not released */
+	zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt() - 1,
+				  "Free CTX buffers %d", ctx_buffers_free());
+}
+
 /* +-----+                +-------+              +-----+
  * | UT  |                | LL_A  |              | LT  |
  * +-----+                +-------+              +-----+
@@ -2071,6 +2203,8 @@ void test_main(void)
 					       unit_test_noop),
 		ztest_unit_test_setup_teardown(test_encryption_start_central_loc_no_ltk_2, setup,
 					       unit_test_noop),
+		ztest_unit_test_setup_teardown(test_encryption_start_central_loc_mic, setup,
+					       unit_test_noop),
 		ztest_unit_test_setup_teardown(test_encryption_start_periph_rem, setup,
 					       unit_test_noop),
 		ztest_unit_test_setup_teardown(test_encryption_start_periph_rem_limited_memory,