From 98efb9da068e9d627469047cdd6301641a5ddf2d Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Fri, 23 Mar 2018 18:31:27 +0000 Subject: [PATCH 01/11] generate oob at will and without passing in connection handle --- features/FEATURE_BLE/ble/SecurityManager.h | 16 ++++++- .../ble/generic/GenericSecurityManager.h | 6 ++- .../FEATURE_BLE/ble/pal/PalSecurityManager.h | 9 +--- .../source/generic/GenericSecurityManager.cpp | 46 ++++++++++++++----- .../TARGET_CORDIO/CordioPalSecurityManager.h | 4 +- .../source/CordioPalSecurityManager.cpp | 4 +- 6 files changed, 58 insertions(+), 27 deletions(-) diff --git a/features/FEATURE_BLE/ble/SecurityManager.h b/features/FEATURE_BLE/ble/SecurityManager.h index 43a83e7fe7e..9bf6ea26bc9 100644 --- a/features/FEATURE_BLE/ble/SecurityManager.h +++ b/features/FEATURE_BLE/ble/SecurityManager.h @@ -735,9 +735,23 @@ class SecurityManager { // MITM // + /** + * Generate OOB data with the given address. If Secure Connections is supported this will + * also generate Secure Connections OOB data on top of legacy pairing OOB data. This can be used + * to generate such data before any connections take place. + * + * @param[in] address The local address you will use in the connection using this OOB data. + * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason. + */ + virtual ble_error_t generateOOB(const ble::address_t *address) { + /* Avoid compiler warnings about unused variables */ + (void) address; + return BLE_ERROR_NOT_IMPLEMENTED; /* Requesting action from porters: override this API if security is supported. */ + } + /** * Enable OOB data usage during paring. If Secure Connections is supported enabling useOOB will - * generate Secure Connections OOB data through oobGenerated(). + * generate Secure Connections OOB data through oobGenerated() on top of legacy pairing OOB data. * * @param[in] connectionHandle Handle to identify the connection. * @param[in] useOOB If set to true, authenticate using OOB data. diff --git a/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h b/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h index 583097fa762..b01020b19ec 100644 --- a/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h +++ b/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h @@ -192,6 +192,10 @@ class GenericSecurityManager : public SecurityManager, // MITM // + virtual ble_error_t generateOOB( + const address_t *address + ); + virtual ble_error_t setOOBDataUsage( connection_handle_t connection, bool useOOB, @@ -441,6 +445,7 @@ class GenericSecurityManager : public SecurityManager, pal::ConnectionEventMonitor &_connection_monitor; /* OOB data */ + address_t _oob_local_address; address_t _oob_peer_address; oob_lesc_value_t _oob_peer_random; oob_confirm_t _oob_peer_confirm; @@ -572,7 +577,6 @@ class GenericSecurityManager : public SecurityManager, /** @copydoc ble::pal::SecurityManager::on_secure_connections_oob_generated */ virtual void on_secure_connections_oob_generated( - connection_handle_t connection, const oob_lesc_value_t &random, const oob_confirm_t &confirm ); diff --git a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h index e452f6fe4c3..d8b0dcf9862 100644 --- a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h +++ b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h @@ -399,7 +399,6 @@ class SecurityManager : private mbed::NonCopyable { * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason. */ virtual void on_secure_connections_oob_generated( - connection_handle_t connection, const oob_lesc_value_t &random, const oob_confirm_t &confirm ) = 0; @@ -975,13 +974,9 @@ class SecurityManager : private mbed::NonCopyable { ) = 0; /** - * Generate local OOB data to be sent to the application which sends it to the peer. - * - * @param[in] connectionHandle Handle to identify the connection. + * Generate local OOB data to be sent to the application which sends it to the peer.p */ - virtual ble_error_t generate_secure_connections_oob( - connection_handle_t connection - ) = 0; + virtual ble_error_t generate_secure_connections_oob() = 0; /* Entry points for the underlying stack to report events back to the user. */ public: diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index 7c210cc17ae..dcb83331f7c 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -500,6 +500,25 @@ ble_error_t GenericSecurityManager::requestAuthentication(connection_handle_t co // MITM // +ble_error_t GenericSecurityManager::generateOOB( + const address_t *address +) { + /* legacy pairing */ + _oob_temporary_key_creator_address = *address; + get_random_data(_oob_temporary_key.buffer(), 16); + + eventHandler->legacyPairingOobGenerated( + &_oob_temporary_key_creator_address, + &_oob_temporary_key + ); + + /* secure connections */ + _oob_local_address = *address; + _pal.generate_secure_connections_oob(); + + return BLE_ERROR_NONE; +} + ble_error_t GenericSecurityManager::setOOBDataUsage( connection_handle_t connection, bool useOOB, @@ -513,6 +532,7 @@ ble_error_t GenericSecurityManager::setOOBDataUsage( cb->attempt_oob = useOOB; cb->oob_mitm_protection = OOBProvidesMITM; + /* legacy pairing */ _oob_temporary_key_creator_address = cb->local_address; get_random_data(_oob_temporary_key.buffer(), 16); @@ -521,7 +541,9 @@ ble_error_t GenericSecurityManager::setOOBDataUsage( &_oob_temporary_key ); - _pal.generate_secure_connections_oob(connection); + /* secure connections */ + _oob_local_address = cb->local_address; + _pal.generate_secure_connections_oob(); return BLE_ERROR_NONE; } @@ -714,13 +736,18 @@ void GenericSecurityManager::update_oob_presence(connection_handle_t connection) return; } - /* only update the oob state if we support secure connections, - * otherwise follow the user set preference for providing legacy - * pairing oob data */ - cb->oob_present = cb->attempt_oob; - + /* if we support secure connection we only care about secure connections oob data */ if (_default_authentication.get_secure_connections()) { cb->oob_present = (cb->peer_address == _oob_peer_address); + } else { + /* otherwise for legacy pairing we first set the oob based on set preference */ + cb->oob_present = cb->attempt_oob; + + /* and also turn it on if we have oob data for legacy pairing */ + if (cb->peer_address == _oob_temporary_key_creator_address + || cb->local_address == _oob_temporary_key_creator_address) { + cb->oob_present = true; + } } } @@ -1016,15 +1043,10 @@ void GenericSecurityManager::on_legacy_pairing_oob_request(connection_handle_t c } void GenericSecurityManager::on_secure_connections_oob_generated( - connection_handle_t connection, const oob_lesc_value_t &random, const oob_confirm_t &confirm ) { - ControlBlock_t *cb = get_control_block(connection); - if (!cb) { - return; - } - eventHandler->oobGenerated(&cb->local_address, &random, &confirm); + eventHandler->oobGenerated(&_oob_local_address, &random, &confirm); _oob_local_random = random; } diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h index 1181abb273d..0461beaec63 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h @@ -297,9 +297,7 @@ class CordioSecurityManager : public ::ble::pal::SecurityManager { /** * @see ::ble::pal::SecurityManager::generate_secure_connections_oob */ - virtual ble_error_t generate_secure_connections_oob( - connection_handle_t connection - ); + virtual ble_error_t generate_secure_connections_oob(); /** * @see ::ble::pal::SecurityManager::secure_connections_oob_request_reply diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp index 7764bbd09c0..1948c0560ef 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp @@ -394,9 +394,7 @@ ble_error_t CordioSecurityManager::send_keypress_notification( return BLE_ERROR_NONE; } -ble_error_t CordioSecurityManager::generate_secure_connections_oob( - connection_handle_t connection -) { +ble_error_t CordioSecurityManager::generate_secure_connections_oob() { return BLE_ERROR_NOT_IMPLEMENTED; } From 02ba2848a8b0b3f8bbdebd01a8f6e6a6ed530f3a Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Mon, 26 Mar 2018 17:48:32 +0100 Subject: [PATCH 02/11] avoid recalculating oob fi already calculating --- .../FEATURE_BLE/ble/pal/PalSecurityManager.h | 3 +- .../source/generic/GenericSecurityManager.cpp | 36 ++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h index f5ac38497eb..a8bccc81421 100644 --- a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h +++ b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h @@ -974,7 +974,8 @@ class SecurityManager : private mbed::NonCopyable { ) = 0; /** - * Generate local OOB data to be sent to the application which sends it to the peer.p + * Generate local OOB data to be sent to the application which sends it to the peer. + * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t generate_secure_connections_oob() = 0; diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index a599868770b..a6606e9d64f 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -510,17 +510,35 @@ ble_error_t GenericSecurityManager::generateOOB( const address_t *address ) { /* legacy pairing */ - _oob_temporary_key_creator_address = *address; - get_random_data(_oob_temporary_key.buffer(), 16); + ble_error_t status = get_random_data(_oob_temporary_key.buffer(), 16); - eventHandler->legacyPairingOobGenerated( - &_oob_temporary_key_creator_address, - &_oob_temporary_key - ); + if (status == BLE_ERROR_NONE) { + _oob_temporary_key_creator_address = *address; - /* secure connections */ - _oob_local_address = *address; - _pal.generate_secure_connections_oob(); + eventHandler->legacyPairingOobGenerated( + &_oob_temporary_key_creator_address, + &_oob_temporary_key + ); + } else { + return status; + } + + /* Secure connections. Avoid generating if we're already waiting for it. + * If a local random is set to 0 it means we're already calculating + * unless the address is invalid which means we've never done so yet */ + if (_oob_local_random != 0 || _oob_local_address == address_t()) { + status = _pal.generate_secure_connections_oob(); + + if (status == BLE_ERROR_NONE) { + _oob_local_address = *address; + /* this will be updated when calculation completes */ + _oob_local_random = 0; + } else if (status != BLE_ERROR_NOT_IMPLEMENTED) { + return status; + } + } else { + return BLE_STACK_BUSY; + } return BLE_ERROR_NONE; } From d1b4713ae62a3eeecc947654192f6e0425b0e6df Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Mon, 26 Mar 2018 17:53:40 +0100 Subject: [PATCH 03/11] removed redundancy --- .../source/generic/GenericSecurityManager.cpp | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index a6606e9d64f..edf758d0dc4 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -556,20 +556,7 @@ ble_error_t GenericSecurityManager::setOOBDataUsage( cb->attempt_oob = useOOB; cb->oob_mitm_protection = OOBProvidesMITM; - /* legacy pairing */ - _oob_temporary_key_creator_address = cb->local_address; - get_random_data(_oob_temporary_key.buffer(), 16); - - eventHandler->legacyPairingOobGenerated( - &_oob_temporary_key_creator_address, - &_oob_temporary_key - ); - - /* secure connections */ - _oob_local_address = cb->local_address; - _pal.generate_secure_connections_oob(); - - return BLE_ERROR_NONE; + return generateOOB(&cb->local_address);; } ble_error_t GenericSecurityManager::confirmationEntered( From e1885486fa46841a914963039ad24bd7ced04030 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Mon, 26 Mar 2018 18:00:05 +0100 Subject: [PATCH 04/11] only generate oob if using oob --- .../FEATURE_BLE/source/generic/GenericSecurityManager.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index edf758d0dc4..29cbc066cc7 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -556,7 +556,11 @@ ble_error_t GenericSecurityManager::setOOBDataUsage( cb->attempt_oob = useOOB; cb->oob_mitm_protection = OOBProvidesMITM; - return generateOOB(&cb->local_address);; + if (useOOB) { + return generateOOB(&cb->local_address); + } else { + return BLE_ERROR_NONE; + } } ble_error_t GenericSecurityManager::confirmationEntered( From c54265073436cf2f6defa85eb09d5e066c2ec6ea Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Mon, 26 Mar 2018 18:07:29 +0100 Subject: [PATCH 05/11] removed unused param from call --- .../targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp index 92996ff06f4..bff6eef6114 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp @@ -636,7 +636,6 @@ bool CordioSecurityManager::sm_handler(const wsfMsgHdr_t* msg) { case DM_SEC_CALC_OOB_IND: { dmSecOobCalcIndEvt_t* evt = (dmSecOobCalcIndEvt_t*) msg; handler->on_secure_connections_oob_generated( - evt->hdr.param, evt->random, evt->confirm ); From aa90f0df652b8686820a62767cd98345b8cbe457 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Tue, 27 Mar 2018 12:25:50 +0100 Subject: [PATCH 06/11] rely solely on random vlalue to know if already calculating simplify by setting a fake random value at the start so that first run is the same as subsequent runs --- features/FEATURE_BLE/ble/BLETypes.h | 19 +++++++++++++++++++ .../ble/generic/GenericSecurityManager.h | 5 +++++ .../source/generic/GenericSecurityManager.cpp | 10 +++++----- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/features/FEATURE_BLE/ble/BLETypes.h b/features/FEATURE_BLE/ble/BLETypes.h index 856f3ca84d5..1fbde5df4de 100644 --- a/features/FEATURE_BLE/ble/BLETypes.h +++ b/features/FEATURE_BLE/ble/BLETypes.h @@ -324,6 +324,25 @@ struct byte_array_t { return _value; } + /** + * Returns true if every byte is equal to zero + */ + bool is_all_zeros() { + for (size_t i = 0; i < array_size; i++) { + if (_value[i] != 0) { + return false; + } + } + return true; + } + + /** + * Zero out all bytes + */ + void set_all_zeros() { + memset(_value, 0x00, array_size); + } + /** * Size in byte of a data. */ diff --git a/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h b/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h index b01020b19ec..1b16a548fa8 100644 --- a/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h +++ b/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h @@ -244,6 +244,11 @@ class GenericSecurityManager : public SecurityManager, _legacy_pairing_allowed(true), _master_sends_keys(false) { _pal.set_event_handler(this); + + /* We create a fake value for oob to allow creation of the next oob which needs + * the last process to finish first before restarting (this is to simplify checking). + * This fake value will not be used as the oob address is currently invalid */ + _oob_local_random[0] = 1; } //////////////////////////////////////////////////////////////////////////// diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index 29cbc066cc7..f93b9efa6e5 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -524,15 +524,15 @@ ble_error_t GenericSecurityManager::generateOOB( } /* Secure connections. Avoid generating if we're already waiting for it. - * If a local random is set to 0 it means we're already calculating - * unless the address is invalid which means we've never done so yet */ - if (_oob_local_random != 0 || _oob_local_address == address_t()) { + * If a local random is set to 0 it means we're already calculating. */ + if (!_oob_local_random.is_all_zeros()) { status = _pal.generate_secure_connections_oob(); if (status == BLE_ERROR_NONE) { _oob_local_address = *address; - /* this will be updated when calculation completes */ - _oob_local_random = 0; + /* this will be updated when calculation completes, + * a value of all zeros is an invalid random value */ + _oob_local_random.set_all_zeros(); } else if (status != BLE_ERROR_NOT_IMPLEMENTED) { return status; } From 3c1a5a4a56f4806594d52974f610e521c5162cf5 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Tue, 3 Apr 2018 12:16:49 +0100 Subject: [PATCH 07/11] incorrect retval usage fixed --- .../FEATURE_BLE/ble/pal/PalSecurityManager.h | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h index a8bccc81421..1833c8bda56 100644 --- a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h +++ b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h @@ -544,21 +544,21 @@ class SecurityManager : private mbed::NonCopyable { /** * Initialise stack. Called before first use. * - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t initialize() = 0; /** * Finalise all actions. Called before shutdown. * - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t terminate() = 0; /** * Reset to same state as after initialize. * - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t reset() = 0; @@ -573,7 +573,7 @@ class SecurityManager : private mbed::NonCopyable { * @warning: The number of entries is considered fixed. * * @see BLUETOOTH SPECIFICATION Version 5.0 | Vol 2, Part E: 7.8.41 - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual uint8_t read_resolving_list_capacity() = 0; @@ -584,7 +584,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] peer_identity_address address of the device whose entry is to be added * @param[in] peer_irk peer identity resolving key * @see BLUETOOTH SPECIFICATION Version 5.0 | Vol 2, Part E: 7.8.38 - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t add_device_to_resolving_list( advertising_peer_address_type_t peer_identity_address_type, @@ -598,7 +598,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] peer_identity_address_type public/private indicator * @param[in] peer_identity_address address of the device whose entry is to be removed * @see BLUETOOTH SPECIFICATION Version 5.0 | Vol 2, Part E: 7.8.39 - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t remove_device_from_resolving_list( advertising_peer_address_type_t peer_identity_address_type, @@ -609,7 +609,7 @@ class SecurityManager : private mbed::NonCopyable { * Remove all devices from the resolving list. * * @see BLUETOOTH SPECIFICATION Version 5.0 | Vol 2, Part E: 7.8.40 - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t clear_resolving_list() = 0; @@ -626,7 +626,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] initiator_dist key distribution * @param[in] responder_dist key distribution * @see BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part H - 3.5.1 - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t send_pairing_request( connection_handle_t connection, @@ -645,7 +645,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] authentication_requirements authentication requirements * @param[in] initiator_dist key distribution * @param[in] responder_dist key distribution - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t send_pairing_response( connection_handle_t connection, @@ -661,7 +661,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] connection connection handle * @param[in] reason pairing failure error * @see BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part H - 3.5.5 - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t cancel_pairing( connection_handle_t connection, @@ -676,7 +676,7 @@ class SecurityManager : private mbed::NonCopyable { * Check if the Secure Connections feature is supported by the stack and controller. * * @param[out] enabled true if SC are supported - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t get_secure_connections_support( bool &enabled @@ -686,7 +686,7 @@ class SecurityManager : private mbed::NonCopyable { * Set the IO capability that will be used during pairing feature exchange. * * @param[in] io_capability type of IO capabilities available on the local device - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t set_io_capability( io_capability_t io_capability @@ -702,7 +702,7 @@ class SecurityManager : private mbed::NonCopyable { * * @param[in] connection connection handle * @param[in] timeout_in_10ms time measured in units of 10 milliseconds - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t set_authentication_timeout( connection_handle_t connection, @@ -715,7 +715,7 @@ class SecurityManager : private mbed::NonCopyable { * * @param[in] connection connection handle * @param[out] timeout_in_10ms time measured in units of 10 milliseconds - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t get_authentication_timeout( connection_handle_t connection, @@ -733,7 +733,7 @@ class SecurityManager : private mbed::NonCopyable { * required for pairing. This value shall be in the range * [min_encryption_key_size : 16]. * - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t set_encryption_key_requirements( uint8_t min_encryption_key_size, @@ -748,7 +748,7 @@ class SecurityManager : private mbed::NonCopyable { * * @param[in] connection connection handle * @param[in] authentication authentication requirements - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t slave_security_request( connection_handle_t connection, @@ -769,7 +769,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] ediv encryption diversifier from the peer * @param[in] rand random value from the peer * @param[in] mitm does the LTK have man in the middle protection - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t enable_encryption( connection_handle_t connection, @@ -786,7 +786,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] connection connection handle * @param[in] ltk long term key from the peer * @param[in] mitm does the LTK have man in the middle protection - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t enable_encryption( connection_handle_t connection, @@ -800,7 +800,7 @@ class SecurityManager : private mbed::NonCopyable { * * @param[in] key encryption key * @param[in,out] data data to be encrypted, if successful contains the result - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t encrypt_data( const byte_array_t<16> &key, @@ -826,7 +826,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] ltk long term key * @param[in] mitm does the LTK have man in the middle protection * @param[in] secure_connections is this a secure_connections pairing - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t set_ltk( connection_handle_t connection, @@ -839,7 +839,7 @@ class SecurityManager : private mbed::NonCopyable { * Inform the stack we don't have the LTK. * * @param[in] connection connection handle - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t set_ltk_not_found( connection_handle_t connection @@ -849,7 +849,7 @@ class SecurityManager : private mbed::NonCopyable { * Set the local IRK. * * @param[in] irk identity resolution key - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t set_irk( const irk_t &irk @@ -859,7 +859,7 @@ class SecurityManager : private mbed::NonCopyable { * Set the local CSRK. * * @param[in] csrk signing key - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t set_csrk( const csrk_t &csrk @@ -874,7 +874,7 @@ class SecurityManager : private mbed::NonCopyable { * * @param[out] random_data returns 8 octets of random data * @see BLUETOOTH SPECIFICATION Version 5.0 | Vol 2, Part H 2 - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t get_random_data( byte_array_t<8> &random_data @@ -902,7 +902,7 @@ class SecurityManager : private mbed::NonCopyable { * passkey is set to 0 then the security manager generates a random * passkey every time it calls SecurityManagerEvent::on_passkey_display. * - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t set_display_passkey( passkey_num_t passkey @@ -911,7 +911,7 @@ class SecurityManager : private mbed::NonCopyable { /** * Reply to a passkey request received from the SecurityManagerEventHandler. * - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t passkey_request_reply( connection_handle_t connection, @@ -926,7 +926,7 @@ class SecurityManager : private mbed::NonCopyable { * @param[in] peer_random random number used to generate the confirmation on peer * @param[in] peer_confirm confirmation value to be use for authentication * in secure connections pairing - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t secure_connections_oob_request_reply( connection_handle_t connection, @@ -940,7 +940,7 @@ class SecurityManager : private mbed::NonCopyable { * * @param[in] connection connection handle * @param[in] oob_data pointer to out of band data - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t legacy_pairing_oob_request_reply( connection_handle_t connection, @@ -953,7 +953,7 @@ class SecurityManager : private mbed::NonCopyable { * * @param[in] connection connection handle * @param[in] confirmation true if the user indicated the numbers match - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t confirmation_entered( connection_handle_t connection, @@ -966,7 +966,7 @@ class SecurityManager : private mbed::NonCopyable { * * @param[in] connection connection handle * @param[in] keypress type of keypress event - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t send_keypress_notification( connection_handle_t connection, @@ -975,7 +975,7 @@ class SecurityManager : private mbed::NonCopyable { /** * Generate local OOB data to be sent to the application which sends it to the peer. - * @retval BLE_ERROR_NONE On success, else an error code indicating reason for failure + * @return BLE_ERROR_NONE On success, else an error code indicating reason for failure */ virtual ble_error_t generate_secure_connections_oob() = 0; From ba5b0f30d2945f64869ed7de771c11d3b599ed4a Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Tue, 3 Apr 2018 13:54:29 +0100 Subject: [PATCH 08/11] added comments about address for oob generation --- features/FEATURE_BLE/ble/SecurityManager.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/features/FEATURE_BLE/ble/SecurityManager.h b/features/FEATURE_BLE/ble/SecurityManager.h index 9bf6ea26bc9..139e61b68db 100644 --- a/features/FEATURE_BLE/ble/SecurityManager.h +++ b/features/FEATURE_BLE/ble/SecurityManager.h @@ -740,7 +740,9 @@ class SecurityManager { * also generate Secure Connections OOB data on top of legacy pairing OOB data. This can be used * to generate such data before any connections take place. * - * @param[in] address The local address you will use in the connection using this OOB data. + * @param[in] address The local address you will use in the connection using this OOB data. This + * address will be returned along with the rest of the OOB data when generation + * is complete. Using an invalid address is illegal. * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason. */ virtual ble_error_t generateOOB(const ble::address_t *address) { From 0a494a0bbca689f2eaa34eaba1f92cbbe5416f8c Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Tue, 3 Apr 2018 14:47:27 +0100 Subject: [PATCH 09/11] all_zeros now free functions --- features/FEATURE_BLE/ble/BLETypes.h | 40 ++++++++++--------- .../source/generic/GenericSecurityManager.cpp | 4 +- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/features/FEATURE_BLE/ble/BLETypes.h b/features/FEATURE_BLE/ble/BLETypes.h index 1fbde5df4de..80953456576 100644 --- a/features/FEATURE_BLE/ble/BLETypes.h +++ b/features/FEATURE_BLE/ble/BLETypes.h @@ -261,6 +261,27 @@ class PasskeyAscii { uint8_t ascii[PASSKEY_LEN]; }; +/** + * Returns true if every byte is equal to zero + */ +template +bool is_all_zeros(byte_array_class &byte_array) { + for (size_t i = 0; i < byte_array.size(); i++) { + if (byte_array[i] != 0) { + return false; + } + } + return true; +} + +/** + * Zero out all bytes + */ +template +void set_all_zeros(byte_array_class &byte_array) { + memset(&byte_array[0], 0x00, byte_array.size()); +} + template struct byte_array_t { /** @@ -324,25 +345,6 @@ struct byte_array_t { return _value; } - /** - * Returns true if every byte is equal to zero - */ - bool is_all_zeros() { - for (size_t i = 0; i < array_size; i++) { - if (_value[i] != 0) { - return false; - } - } - return true; - } - - /** - * Zero out all bytes - */ - void set_all_zeros() { - memset(_value, 0x00, array_size); - } - /** * Size in byte of a data. */ diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index f93b9efa6e5..2d9da8a0a93 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -525,14 +525,14 @@ ble_error_t GenericSecurityManager::generateOOB( /* Secure connections. Avoid generating if we're already waiting for it. * If a local random is set to 0 it means we're already calculating. */ - if (!_oob_local_random.is_all_zeros()) { + if (!is_all_zeros(_oob_local_random)) { status = _pal.generate_secure_connections_oob(); if (status == BLE_ERROR_NONE) { _oob_local_address = *address; /* this will be updated when calculation completes, * a value of all zeros is an invalid random value */ - _oob_local_random.set_all_zeros(); + set_all_zeros(_oob_local_random); } else if (status != BLE_ERROR_NOT_IMPLEMENTED) { return status; } From 26b047549c87a50fa10934c3c63b915b47c6dfbb Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Tue, 3 Apr 2018 15:17:15 +0100 Subject: [PATCH 10/11] extra comments for api --- features/FEATURE_BLE/ble/SecurityManager.h | 23 +++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/features/FEATURE_BLE/ble/SecurityManager.h b/features/FEATURE_BLE/ble/SecurityManager.h index 139e61b68db..11294a74614 100644 --- a/features/FEATURE_BLE/ble/SecurityManager.h +++ b/features/FEATURE_BLE/ble/SecurityManager.h @@ -738,7 +738,12 @@ class SecurityManager { /** * Generate OOB data with the given address. If Secure Connections is supported this will * also generate Secure Connections OOB data on top of legacy pairing OOB data. This can be used - * to generate such data before any connections take place. + * to generate such data before the connection takes place. + * + * In this model the OOB exchange takes place before the devices connect. Devices should establish + * communication over another channel and exchange the OOB data. The address provided will be used + * by the peer to associate the received data with the address of the device it will then connect + * to over BLE. * * @param[in] address The local address you will use in the connection using this OOB data. This * address will be returned along with the rest of the OOB data when generation @@ -755,11 +760,23 @@ class SecurityManager { * Enable OOB data usage during paring. If Secure Connections is supported enabling useOOB will * generate Secure Connections OOB data through oobGenerated() on top of legacy pairing OOB data. * + * You do not have to call this function to return received OOB data. Use legacyPairingOobReceived + * or oobReceived to hand it in. This will allow the stack to use it if possible. You only need to + * call this function to attempt legacy OOB data exchange after pairing start and to inform + * the stack OOB data does not provide MITM protection (by default it is set to provide this). + * + * In this model the OOB exchange takes places after the devices have connected but possibly + * prior to pairing. For secure connections pairing must not be started until after the OOB + * data has been sent and/or received. The address in the OOB data generated will match + * the original address used to establish the connection and will be used by the peer to + * identify which connection the OOB data belongs to. + * * @param[in] connectionHandle Handle to identify the connection. * @param[in] useOOB If set to true, authenticate using OOB data. * @param[in] OOBProvidesMITM If set to true keys exchanged during pairing using OOB data - * will provide MITM protection. This indicates that the form - * of exchange used by the OOB data itself provides MITM protection. + * will provide Man-in-the-Middle protection. This indicates that + * the form of exchange used by the OOB data itself provides MITM + * protection. * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason. */ virtual ble_error_t setOOBDataUsage(ble::connection_handle_t connectionHandle, bool useOOB, bool OOBProvidesMITM = true) { From 576796b28979f23d22a498316e330b28f5388ba6 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Tue, 3 Apr 2018 15:29:26 +0100 Subject: [PATCH 11/11] reset OOB on use --- .../FEATURE_BLE/source/generic/GenericSecurityManager.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index 2d9da8a0a93..1178e5afe9b 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -1032,6 +1032,8 @@ void GenericSecurityManager::on_secure_connections_oob_request(connection_handle if (cb->peer_address == _oob_peer_address) { _pal.secure_connections_oob_request_reply(connection, _oob_local_random, _oob_peer_random, _oob_peer_confirm); + /* do not re-use peer OOB */ + set_all_zeros(_oob_peer_address); } else { _pal.cancel_pairing(connection, pairing_failure_t::OOB_NOT_AVAILABLE); } @@ -1049,6 +1051,11 @@ void GenericSecurityManager::on_legacy_pairing_oob_request(connection_handle_t c set_mitm_performed(connection); _pal.legacy_pairing_oob_request_reply(connection, _oob_temporary_key); + /* do not re-use peer OOB */ + if (cb->peer_address == _oob_temporary_key_creator_address) { + set_all_zeros(_oob_temporary_key_creator_address); + } + } else if (!cb->legacy_pairing_oob_request_pending) { cb->legacy_pairing_oob_request_pending = true;