From ffa0636719391c464c565dbf93e2cb3427f653b1 Mon Sep 17 00:00:00 2001 From: Andrzej Kaczmarek Date: Tue, 22 Nov 2016 21:09:22 +0100 Subject: [PATCH] nimble/sm: Improve pairing req/rsp validation The "Invalid Parameters" error code is only valid if encryption key size it larger than allowed maximum - if it is lower than supported minimum key size, the error code shall be "Encryption Key Size". Also we should accept pairing req/rsp in case either IO Capabilities or OOB Flags are set to reserved values by forcing either Just Works or discarding OOB Flags information respectively. This should allow to handle pairing with future specs (if reserved values are used) and is inline with other stacks (e.g. BlueZ, Zephyr). X-Original-Commit: 3d129607c9078bde96753208d35829e2c762e8ae --- nimble/host/src/ble_sm.c | 12 +++++++++--- nimble/host/src/ble_sm_cmd.c | 21 --------------------- nimble/host/src/ble_sm_lgcy.c | 6 +++++- nimble/host/src/ble_sm_sc.c | 6 +++++- 4 files changed, 19 insertions(+), 26 deletions(-) diff --git a/nimble/host/src/ble_sm.c b/nimble/host/src/ble_sm.c index 67e8728de..d6527d4f0 100644 --- a/nimble/host/src/ble_sm.c +++ b/nimble/host/src/ble_sm.c @@ -1487,9 +1487,12 @@ ble_sm_pair_req_rx(uint16_t conn_handle, uint8_t op, struct os_mbuf **om, if (conn->bhc_flags & BLE_HS_CONN_F_MASTER) { res->sm_err = BLE_SM_ERR_CMD_NOT_SUPP; res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_CMD_NOT_SUPP); - } else if (!ble_sm_pair_cmd_is_valid(&req)) { + } else if (req.max_enc_key_size < BLE_SM_PAIR_KEY_SZ_MIN) { + res->sm_err = BLE_SM_ERR_ENC_KEY_SZ; + res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_ENC_KEY_SZ); + } else if (req.max_enc_key_size > BLE_SM_PAIR_KEY_SZ_MAX) { res->sm_err = BLE_SM_ERR_INVAL; - res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_INVAL); + res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_INVAL); } else { res->execute = 1; } @@ -1520,7 +1523,10 @@ ble_sm_pair_rsp_rx(uint16_t conn_handle, uint8_t op, struct os_mbuf **om, proc = ble_sm_proc_find(conn_handle, BLE_SM_PROC_STATE_PAIR, 1, &prev); if (proc != NULL) { proc->pair_rsp = rsp; - if (!ble_sm_pair_cmd_is_valid(&rsp)) { + if (rsp.max_enc_key_size < BLE_SM_PAIR_KEY_SZ_MIN) { + res->sm_err = BLE_SM_ERR_ENC_KEY_SZ; + res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_ENC_KEY_SZ); + } else if (rsp.max_enc_key_size > BLE_SM_PAIR_KEY_SZ_MAX) { res->sm_err = BLE_SM_ERR_INVAL; res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_INVAL); } else { diff --git a/nimble/host/src/ble_sm_cmd.c b/nimble/host/src/ble_sm_cmd.c index e438c023b..67fa1b098 100644 --- a/nimble/host/src/ble_sm_cmd.c +++ b/nimble/host/src/ble_sm_cmd.c @@ -90,26 +90,6 @@ ble_sm_pair_cmd_parse(void *payload, int len, struct ble_sm_pair_cmd *cmd) cmd->resp_key_dist = u8ptr[5]; } -int -ble_sm_pair_cmd_is_valid(struct ble_sm_pair_cmd *cmd) -{ - if (cmd->io_cap >= BLE_SM_IO_CAP_RESERVED) { - return 0; - } - - if (cmd->oob_data_flag >= BLE_SM_PAIR_OOB_RESERVED) { - return 0; - } - - if (cmd->max_enc_key_size < BLE_SM_PAIR_KEY_SZ_MIN || - cmd->max_enc_key_size > BLE_SM_PAIR_KEY_SZ_MAX) { - - return 0; - } - - return 1; -} - void ble_sm_pair_cmd_write(void *payload, int len, int is_req, struct ble_sm_pair_cmd *cmd) @@ -143,7 +123,6 @@ ble_sm_pair_cmd_tx(uint16_t conn_handle, int is_req, ble_sm_pair_cmd_write(txom->om_data, txom->om_len, is_req, cmd); BLE_SM_LOG_CMD(1, is_req ? "pair req" : "pair rsp", conn_handle, ble_sm_pair_cmd_log, cmd); - BLE_HS_DBG_ASSERT(ble_sm_pair_cmd_is_valid(cmd)); rc = ble_sm_tx(conn_handle, txom); if (rc != 0) { diff --git a/nimble/host/src/ble_sm_lgcy.c b/nimble/host/src/ble_sm_lgcy.c index ddc666de9..a77671e32 100644 --- a/nimble/host/src/ble_sm_lgcy.c +++ b/nimble/host/src/ble_sm_lgcy.c @@ -65,12 +65,16 @@ ble_sm_lgcy_io_action(struct ble_sm_proc *proc) { int action; - if (proc->pair_req.oob_data_flag && proc->pair_rsp.oob_data_flag) { + if (proc->pair_req.oob_data_flag == BLE_SM_PAIR_OOB_YES && + proc->pair_rsp.oob_data_flag == BLE_SM_PAIR_OOB_YES) { action = BLE_SM_IOACT_OOB; } else if (!(proc->pair_req.authreq & BLE_SM_PAIR_AUTHREQ_MITM) && !(proc->pair_rsp.authreq & BLE_SM_PAIR_AUTHREQ_MITM)) { action = BLE_SM_IOACT_NONE; + } else if (proc->pair_req.io_cap >= BLE_SM_IO_CAP_RESERVED || + proc->pair_rsp.io_cap >= BLE_SM_IO_CAP_RESERVED) { + action = BLE_SM_IOACT_NONE; } else if (proc->flags & BLE_SM_PROC_F_INITIATOR) { action = ble_sm_lgcy_init_ioa[proc->pair_rsp.io_cap] [proc->pair_req.io_cap]; diff --git a/nimble/host/src/ble_sm_sc.c b/nimble/host/src/ble_sm_sc.c index 11b29a1c8..f8a698305 100644 --- a/nimble/host/src/ble_sm_sc.c +++ b/nimble/host/src/ble_sm_sc.c @@ -112,12 +112,16 @@ ble_sm_sc_io_action(struct ble_sm_proc *proc) { int action; - if (proc->pair_req.oob_data_flag || proc->pair_rsp.oob_data_flag) { + if (proc->pair_req.oob_data_flag == BLE_SM_PAIR_OOB_YES || + proc->pair_rsp.oob_data_flag == BLE_SM_PAIR_OOB_YES) { action = BLE_SM_IOACT_OOB; } else if (!(proc->pair_req.authreq & BLE_SM_PAIR_AUTHREQ_MITM) && !(proc->pair_rsp.authreq & BLE_SM_PAIR_AUTHREQ_MITM)) { action = BLE_SM_IOACT_NONE; + } else if (proc->pair_req.io_cap >= BLE_SM_IO_CAP_RESERVED || + proc->pair_rsp.io_cap >= BLE_SM_IO_CAP_RESERVED) { + action = BLE_SM_IOACT_NONE; } else if (proc->flags & BLE_SM_PROC_F_INITIATOR) { action = ble_sm_sc_init_ioa[proc->pair_rsp.io_cap] [proc->pair_req.io_cap];