From 1521dee77322392357402aa1a0c6d2be81a5a335 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Fri, 9 Mar 2018 18:06:56 +0000 Subject: [PATCH 01/12] add doxygen comment about OOB deneration cause bu setOOBDataUsage --- features/FEATURE_BLE/ble/SecurityManager.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/features/FEATURE_BLE/ble/SecurityManager.h b/features/FEATURE_BLE/ble/SecurityManager.h index a01882959a..43a83e7fe7 100644 --- a/features/FEATURE_BLE/ble/SecurityManager.h +++ b/features/FEATURE_BLE/ble/SecurityManager.h @@ -736,7 +736,8 @@ public: // /** - * Enable OOB data usage during paring. + * Enable OOB data usage during paring. If Secure Connections is supported enabling useOOB will + * generate Secure Connections OOB data through oobGenerated(). * * @param[in] connectionHandle Handle to identify the connection. * @param[in] useOOB If set to true, authenticate using OOB data. From 66867d4dd32e0b123c5acf480a8b8984f410c5db Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Fri, 16 Mar 2018 14:48:04 +0000 Subject: [PATCH 02/12] oob stored in generic and handed over to pal when requested --- .../ble/generic/GenericSecurityManager.h | 14 ++++- .../FEATURE_BLE/ble/pal/PalSecurityManager.h | 60 ++++++++++--------- .../source/generic/GenericSecurityManager.cpp | 31 ++++++++-- .../TARGET_CORDIO/CordioPalSecurityManager.h | 18 ++---- .../source/CordioPalSecurityManager.cpp | 15 ++--- 5 files changed, 82 insertions(+), 56 deletions(-) diff --git a/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h b/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h index 322cef4b5d..8b33c08dae 100644 --- a/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h +++ b/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h @@ -439,6 +439,12 @@ private: pal::SecurityDb &_db; pal::ConnectionEventMonitor &_connection_monitor; + /* OOB data */ + address_t _oob_peer_address; + oob_lesc_value_t _oob_peer_random; + oob_confirm_t _oob_peer_confirm; + oob_lesc_value_t _oob_local_random; + pal::AuthenticationMask _default_authentication; pal::KeyDistribution _default_key_distribution; @@ -548,6 +554,12 @@ public: connection_handle_t connection ); + /** @copydoc ble::pal::SecurityManager::on_secure_connections_oob_request + */ + virtual void on_secure_connections_oob_request( + connection_handle_t connection + ); + /** @copydoc ble::pal::SecurityManager::on_legacy_pairing_oob_request */ virtual void on_legacy_pairing_oob_request( @@ -557,7 +569,7 @@ public: /** @copydoc ble::pal::SecurityManager::on_secure_connections_oob_generated */ virtual void on_secure_connections_oob_generated( - const address_t &local_address, + 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 ac984ab0d8..e452f6fe4c 100644 --- a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h +++ b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h @@ -367,6 +367,17 @@ public: Keypress_t keypress ) = 0; + /** + * Request OOB data from the user application. + * + * @param[in] connection connection handle + * @note shall be followed by: pal::SecurityManager::secure_connections_oob_request_reply + * or a cancellation of the procedure. + */ + virtual void on_secure_connections_oob_request( + connection_handle_t connection + ) = 0; + /** * Request OOB data from the user application. * @@ -381,14 +392,14 @@ public: /** * Send OOB data to the application for transport to the peer. * - * @param[in] address address of the local device + * @param[in] connection connection handle * @param[in] random random number used to generate the confirmation * @param[in] confirm confirmation value to be use for authentication * in secure connections pairing * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason. */ virtual void on_secure_connections_oob_generated( - const address_t &local_address, + connection_handle_t connection, const oob_lesc_value_t &random, const oob_confirm_t &confirm ) = 0; @@ -909,7 +920,24 @@ public: ) = 0; /** - * Reply to an oob data request received from the SecurityManagerEventHandler. + * Reply to a Secure Connections oob data request received from the SecurityManagerEventHandler. + * + * @param[in] connection connection handle + * @param[in] local_random local random number used for the last oob exchange + * @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 + */ + virtual ble_error_t secure_connections_oob_request_reply( + connection_handle_t connection, + const oob_lesc_value_t &local_random, + const oob_lesc_value_t &peer_random, + const oob_confirm_t &peer_confirm + ) = 0; + + /** + * Reply to a legacy pairing oob data request received from the SecurityManagerEventHandler. * * @param[in] connection connection handle * @param[in] oob_data pointer to out of band data @@ -955,32 +983,6 @@ public: connection_handle_t connection ) = 0; - /** - * Supply the stack with the OOB data for secure connections. - * - * @param[in] address address of the peer device this data comes from - * @param[in] random random number used to generate the confirmation - * @param[in] confirm confirmation value to be use for authentication - * in secure connections pairing - * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason. - */ - virtual ble_error_t secure_connections_oob_received( - const address_t &address, - const oob_lesc_value_t &random, - const oob_confirm_t &confirm - ) = 0; - - /** - * Supply the stack with the OOB data for secure connections. - * - * @param[in] address address of the peer device oob data is needed for - * @return True if oob data present, false if not or if the functionality - * is not implemented. - */ - virtual bool is_secure_connections_oob_present( - const address_t &address - ) = 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 0e6cb57e5e..95b15269b1 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -563,7 +563,10 @@ ble_error_t GenericSecurityManager::oobReceived( const oob_confirm_t *confirm ) { if (address && random && confirm) { - return _pal.secure_connections_oob_received(*address, *random, *confirm); + _oob_peer_address = *address; + _oob_peer_random = *random; + _oob_peer_confirm = *confirm; + return BLE_ERROR_NONE; } return BLE_ERROR_INVALID_PARAM; @@ -696,7 +699,7 @@ void GenericSecurityManager::update_oob_presence(connection_handle_t connection) cb->oob_present = cb->attempt_oob; if (_default_authentication.get_secure_connections()) { - cb->oob_present = _pal.is_secure_connections_oob_present(cb->peer_address); + cb->oob_present = (cb->peer_address == _oob_peer_address); } } @@ -944,17 +947,37 @@ void GenericSecurityManager::on_confirmation_request(connection_handle_t connect eventHandler->confirmationRequest(connection); } +void GenericSecurityManager::on_secure_connections_oob_request(connection_handle_t connection) { + set_mitm_performed(connection); + + ControlBlock_t *cb = get_control_block(connection); + if (!cb) { + return; + } + + if (cb->peer_address == _oob_peer_address) { + _pal.secure_connections_oob_request_reply(connection, _oob_local_random, _oob_peer_random, _oob_peer_confirm); + } else { + _pal.cancel_pairing(connection, pairing_failure_t::OOB_NOT_AVAILABLE); + } +} + void GenericSecurityManager::on_legacy_pairing_oob_request(connection_handle_t connection) { set_mitm_performed(connection); eventHandler->legacyPairingOobRequest(connection); } void GenericSecurityManager::on_secure_connections_oob_generated( - const address_t &local_address, + connection_handle_t connection, const oob_lesc_value_t &random, const oob_confirm_t &confirm ) { - eventHandler->oobGenerated(&local_address, &random, &confirm); + ControlBlock_t *cb = get_control_block(connection); + if (!cb) { + return; + } + eventHandler->oobGenerated(&cb->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 d79c1c788c..1181abb273 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h @@ -302,19 +302,13 @@ public: ); /** - * @see ::ble::pal::SecurityManager::secure_connections_oob_received + * @see ::ble::pal::SecurityManager::secure_connections_oob_request_reply */ - virtual ble_error_t secure_connections_oob_received( - const address_t &address, - const oob_lesc_value_t &random, - const oob_confirm_t &confirm - ); - - /** - * @see ::ble::pal::SecurityManager::is_secure_connections_oob_present - */ - virtual bool is_secure_connections_oob_present( - const address_t &address + virtual ble_error_t secure_connections_oob_request_reply( + connection_handle_t connection, + const oob_lesc_value_t &local_random, + const oob_lesc_value_t &peer_random, + const oob_confirm_t &peer_confirm ); // singleton of the ARM Cordio Security Manager diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp index 24e405ef65..7764bbd09c 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp @@ -400,20 +400,15 @@ ble_error_t CordioSecurityManager::generate_secure_connections_oob( return BLE_ERROR_NOT_IMPLEMENTED; } -ble_error_t CordioSecurityManager::secure_connections_oob_received( - const address_t &address, - const oob_lesc_value_t &random, - const oob_confirm_t &confirm +ble_error_t CordioSecurityManager::secure_connections_oob_request_reply( + connection_handle_t connection, + const oob_lesc_value_t &local_random, + const oob_lesc_value_t &peer_random, + const oob_confirm_t &peer_confirm ) { return BLE_ERROR_NOT_IMPLEMENTED; } -bool CordioSecurityManager::is_secure_connections_oob_present( - const address_t &address -) { - return false; -} - CordioSecurityManager& CordioSecurityManager::get_security_manager() { static CordioSecurityManager _security_manager; From 909f9513cfd594dfceabdf1eadf4962533900425 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 22 Mar 2018 12:01:34 +0000 Subject: [PATCH 03/12] allow preloading legacy oob, generate tk --- .../ble/generic/GenericSecurityManager.h | 3 ++ .../source/generic/GenericSecurityManager.cpp | 33 +++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h b/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h index 8b33c08dae..583097fa76 100644 --- a/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h +++ b/features/FEATURE_BLE/ble/generic/GenericSecurityManager.h @@ -433,6 +433,7 @@ private: uint8_t attempt_oob:1; uint8_t oob_mitm_protection:1; uint8_t oob_present:1; + uint8_t legacy_pairing_oob_request_pending:1; }; pal::SecurityManager &_pal; @@ -444,6 +445,8 @@ private: oob_lesc_value_t _oob_peer_random; oob_confirm_t _oob_peer_confirm; oob_lesc_value_t _oob_local_random; + address_t _oob_temporary_key_creator_address; /**< device which generated and sent the TK */ + oob_tk_t _oob_temporary_key; /**< used for legacy pairing */ pal::AuthenticationMask _default_authentication; pal::KeyDistribution _default_key_distribution; diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index 95b15269b1..3994319709 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -513,6 +513,14 @@ ble_error_t GenericSecurityManager::setOOBDataUsage( cb->attempt_oob = useOOB; cb->oob_mitm_protection = OOBProvidesMITM; + _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 + ); + _pal.generate_secure_connections_oob(connection); return BLE_ERROR_NONE; @@ -552,7 +560,13 @@ ble_error_t GenericSecurityManager::legacyPairingOobReceived( return BLE_ERROR_INVALID_PARAM; } - return _pal.legacy_pairing_oob_request_reply(cb->connection, *tk); + _oob_temporary_key = *tk; + _oob_peer_address = *address; + + if (cb->legacy_pairing_oob_request_pending) { + cb->legacy_pairing_oob_request_pending = false; + return _pal.legacy_pairing_oob_request_reply(cb->connection, *tk); + } } return BLE_ERROR_NONE; } @@ -964,7 +978,19 @@ void GenericSecurityManager::on_secure_connections_oob_request(connection_handle void GenericSecurityManager::on_legacy_pairing_oob_request(connection_handle_t connection) { set_mitm_performed(connection); - eventHandler->legacyPairingOobRequest(connection); + + ControlBlock_t *cb = get_control_block(connection); + if (!cb) { + return; + } + + if (cb->peer_address == _oob_temporary_key_creator_address + || cb->local_address == _oob_temporary_key_creator_address) { + _pal.legacy_pairing_oob_request_reply(connection, _oob_temporary_key); + } else { + cb->legacy_pairing_oob_request_pending = true; + eventHandler->legacyPairingOobRequest(connection); + } } void GenericSecurityManager::on_secure_connections_oob_generated( @@ -1135,7 +1161,8 @@ GenericSecurityManager::ControlBlock_t::ControlBlock_t() : mitm_performed(false), attempt_oob(false), oob_mitm_protection(false), - oob_present(false) { } + oob_present(false), + legacy_pairing_oob_request_pending(false) { } void GenericSecurityManager::on_ltk_request(connection_handle_t connection) { From c848c79a5bc856fb0387e7e2a4f730da28ffd311 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 22 Mar 2018 12:10:21 +0000 Subject: [PATCH 04/12] avoid code redundancy --- .../source/generic/GenericSecurityManager.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index 3994319709..ede91d5db1 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -561,11 +561,10 @@ ble_error_t GenericSecurityManager::legacyPairingOobReceived( } _oob_temporary_key = *tk; - _oob_peer_address = *address; + _oob_temporary_key_creator_address = *address; if (cb->legacy_pairing_oob_request_pending) { - cb->legacy_pairing_oob_request_pending = false; - return _pal.legacy_pairing_oob_request_reply(cb->connection, *tk); + on_legacy_pairing_oob_request(cb->connection); } } return BLE_ERROR_NONE; @@ -977,8 +976,6 @@ void GenericSecurityManager::on_secure_connections_oob_request(connection_handle } void GenericSecurityManager::on_legacy_pairing_oob_request(connection_handle_t connection) { - set_mitm_performed(connection); - ControlBlock_t *cb = get_control_block(connection); if (!cb) { return; @@ -986,10 +983,15 @@ void GenericSecurityManager::on_legacy_pairing_oob_request(connection_handle_t c if (cb->peer_address == _oob_temporary_key_creator_address || cb->local_address == _oob_temporary_key_creator_address) { + + set_mitm_performed(connection); _pal.legacy_pairing_oob_request_reply(connection, _oob_temporary_key); - } else { + + } else if (!cb->legacy_pairing_oob_request_pending) { + cb->legacy_pairing_oob_request_pending = true; eventHandler->legacyPairingOobRequest(connection); + } } From 2b2d9a24534f6f03d8502dcf7ba01d2ce1960e56 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 22 Mar 2018 12:23:37 +0000 Subject: [PATCH 05/12] reset pending state when attempt ends, added comments --- .../source/generic/GenericSecurityManager.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index ede91d5db1..afb7fc7469 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -565,6 +565,10 @@ ble_error_t GenericSecurityManager::legacyPairingOobReceived( if (cb->legacy_pairing_oob_request_pending) { on_legacy_pairing_oob_request(cb->connection); + /* legacy_pairing_oob_request_pending stops us from + * going into a loop of asking the user for oob + * so this reset needs to happen after the call above */ + cb->legacy_pairing_oob_request_pending = false; } } return BLE_ERROR_NONE; @@ -720,6 +724,11 @@ void GenericSecurityManager::set_mitm_performed(connection_handle_t connection, ControlBlock_t *cb = get_control_block(connection); if (cb) { cb->mitm_performed = enable; + /* whenever we reset mitm performed we also reset pending requests + * as this happens whenever a new pairing attempt happens */ + if (!enable) { + cb->legacy_pairing_oob_request_pending = false; + } } } From 350924129f0146353b94f20ce2af46604e6acd26 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 22 Mar 2018 12:32:01 +0000 Subject: [PATCH 06/12] fix the attempt oob flag if we receive oob --- .../FEATURE_BLE/source/generic/GenericSecurityManager.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index afb7fc7469..cef46986a8 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -563,6 +563,10 @@ ble_error_t GenericSecurityManager::legacyPairingOobReceived( _oob_temporary_key = *tk; _oob_temporary_key_creator_address = *address; + if (cb->peer_address == _oob_temporary_key_creator_address) { + cb->attempt_oob = true; + } + if (cb->legacy_pairing_oob_request_pending) { on_legacy_pairing_oob_request(cb->connection); /* legacy_pairing_oob_request_pending stops us from From 81cb1f9c83c9eafdd3ff927542652cdba56adc82 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 22 Mar 2018 17:11:15 +0000 Subject: [PATCH 07/12] enable encryption for slave request added --- .../source/generic/GenericSecurityManager.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index 95b15269b1..e773190e4c 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -853,16 +853,23 @@ void GenericSecurityManager::on_slave_security_request( return; } - if (authentication.get_secure_connections() - && _default_authentication.get_secure_connections() - && !cb->secure_connections_paired) { - requestPairing(connection); + bool pairing_required = false; + + if (authentication.get_secure_connections() && !cb->secure_connections_paired + && _default_authentication.get_secure_connections()) { + pairing_required = true; } - if (authentication.get_mitm() - && !cb->ltk_mitm_protected) { + if (authentication.get_mitm() && !cb->ltk_mitm_protected) { + pairing_required = true; cb->mitm_requested = true; + } + + if (pairing_required) { requestPairing(connection); + } else { + /* this will refresh keys if encryption is already present */ + enable_encryption(connection); } } From 6833c79fb301ab3bdab22b555c1fb56d54ff5a69 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 22 Mar 2018 17:54:13 +0000 Subject: [PATCH 08/12] don't request encrypt when pending --- features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index e773190e4c..69f0306e5e 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -867,7 +867,7 @@ void GenericSecurityManager::on_slave_security_request( if (pairing_required) { requestPairing(connection); - } else { + } else if (!cb->encryption_requested) { /* this will refresh keys if encryption is already present */ enable_encryption(connection); } From e25d5c9aa301e9538e4cf7dc4fd7c4bf2ac1b103 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Fri, 9 Mar 2018 17:56:59 +0000 Subject: [PATCH 09/12] BLE: qualification of SecurityManager types --- features/FEATURE_BLE/ble/pal/PalSecurityManager.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h index e452f6fe4c..5dca19404b 100644 --- a/features/FEATURE_BLE/ble/pal/PalSecurityManager.h +++ b/features/FEATURE_BLE/ble/pal/PalSecurityManager.h @@ -27,10 +27,10 @@ namespace ble { namespace pal { -typedef SecurityManager::SecurityCompletionStatus_t SecurityCompletionStatus_t; -typedef SecurityManager::SecurityMode_t SecurityMode_t; -typedef SecurityManager::LinkSecurityStatus_t LinkSecurityStatus_t; -typedef SecurityManager::Keypress_t Keypress_t; +typedef ::SecurityManager::SecurityCompletionStatus_t SecurityCompletionStatus_t; +typedef ::SecurityManager::SecurityMode_t SecurityMode_t; +typedef ::SecurityManager::LinkSecurityStatus_t LinkSecurityStatus_t; +typedef ::SecurityManager::Keypress_t Keypress_t; /** * Key distribution as required by the SMP with convenient setters and getters, From 0a710e533192f4dd9301f4d6c4babe149e3faf7a Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Fri, 9 Mar 2018 18:04:47 +0000 Subject: [PATCH 10/12] BLE: Initialize the pal in GenericSecurityManager --- .gitignore | 2 ++ .../FEATURE_BLE/source/generic/GenericSecurityManager.cpp | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/.gitignore b/.gitignore index 88ff051ecc..7b85cc3417 100644 --- a/.gitignore +++ b/.gitignore @@ -86,3 +86,5 @@ tags # Visual Studio Code .vscode/ + +features/FEATURE_BLE/targets/TARGET_CORDIO/stack_backup/ diff --git a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp index 7c210cc17a..2acca2755f 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -39,6 +39,11 @@ ble_error_t GenericSecurityManager::init( const Passkey_t passkey, bool signing ) { + ble_error_t err = _pal.initialize(); + if (err) { + return err; + } + _db.restore(); _pal.set_io_capability((io_capability_t::type) iocaps); @@ -73,6 +78,7 @@ ble_error_t GenericSecurityManager::init( ble_error_t GenericSecurityManager::reset(void) { _db.sync(); + _pal.reset(); SecurityManager::reset(); return BLE_ERROR_NONE; From b859907481f0b7592badc46950bb8b8e48f52996 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Fri, 9 Mar 2018 18:05:31 +0000 Subject: [PATCH 11/12] Cordio: remove generate_public_key from the security manager. --- .../targets/TARGET_CORDIO/CordioPalSecurityManager.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h index 1181abb273..d8fb104990 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h @@ -195,11 +195,6 @@ public: */ virtual ble_error_t set_csrk(const csrk_t &csrk); - /** - * @see ::ble::pal::SecurityManager::generate_public_key - */ - virtual ble_error_t generate_public_key(); - //////////////////////////////////////////////////////////////////////////// // Global parameters // From 3579653533e8a3d89255323b6edf45fd2712e77c Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Fri, 9 Mar 2018 18:06:22 +0000 Subject: [PATCH 12/12] Cordio: Implement missing functions for secure connection --- .../TARGET_CORDIO/CordioPalSecurityManager.h | 130 +++++++++--------- .../source/CordioPalSecurityManager.cpp | 86 +++++++++--- 2 files changed, 135 insertions(+), 81 deletions(-) diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h index d8fb104990..458f20311c 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalSecurityManager.h @@ -20,6 +20,8 @@ #include "ble/pal/PalSecurityManager.h" #include "wsf_types.h" #include "wsf_os.h" +#include "sec_api.h" +#include "smp_defs.h" namespace ble { namespace pal { @@ -83,16 +85,42 @@ public: virtual ble_error_t clear_resolving_list(); //////////////////////////////////////////////////////////////////////////// - // Feature support + // Pairing // /** - * @see ::ble::pal::SecurityManager::set_secure_connections_support + * @see ::ble::pal::SecurityManager::send_pairing_request */ - virtual ble_error_t set_secure_connections_support( - bool enabled, bool secure_connections_only = false + virtual ble_error_t send_pairing_request( + connection_handle_t connection, + bool oob_data_flag, + AuthenticationMask authentication_requirements, + KeyDistribution initiator_dist, + KeyDistribution responder_dist ); + /** + * @see ::ble::pal::SecurityManager::send_pairing_response + */ + virtual ble_error_t send_pairing_response( + connection_handle_t connection, + bool oob_data_flag, + AuthenticationMask authentication_requirements, + KeyDistribution initiator_dist, + KeyDistribution responder_dist + ); + + /** + * @see ::ble::pal::SecurityManager::cancel_pairing + */ + virtual ble_error_t cancel_pairing( + connection_handle_t connection, pairing_failure_t reason + ); + + //////////////////////////////////////////////////////////////////////////// + // Feature support + // + /** * @see ::ble::pal::SecurityManager::get_secure_connections_support */ @@ -100,6 +128,11 @@ public: bool &enabled ); + /** + * @see ::ble::pal::SecurityManager::set_io_capability + */ + virtual ble_error_t set_io_capability(io_capability_t io_capability); + //////////////////////////////////////////////////////////////////////////// // Security settings // @@ -118,6 +151,17 @@ public: connection_handle_t, uint16_t &timeout_in_10ms ); + /** + * @see ::ble::pal::SecurityManager::set_encryption_key_requirements + */ + virtual ble_error_t set_encryption_key_requirements( + uint8_t min_encryption_key_size, + uint8_t max_encryption_key_size + ); + + /** + * @see ::ble::pal::SecurityManager::slave_security_request + */ virtual ble_error_t slave_security_request( connection_handle_t connection, AuthenticationMask authentication @@ -195,61 +239,10 @@ public: */ virtual ble_error_t set_csrk(const csrk_t &csrk); - //////////////////////////////////////////////////////////////////////////// - // Global parameters - // - - /** - * @see ::ble::pal::SecurityManager::set_display_passkey - */ - virtual ble_error_t set_display_passkey(passkey_num_t passkey); - - /** - * @see ::ble::pal::SecurityManager::set_io_capability - */ - virtual ble_error_t set_io_capability(io_capability_t io_capability); - - /** - * @see ::ble::pal::SecurityManager::set_encryption_key_requirements - */ - virtual ble_error_t set_encryption_key_requirements( - uint8_t min_encryption_key_size, - uint8_t max_encryption_key_size - ); - //////////////////////////////////////////////////////////////////////////// // Authentication // - /** - * @see ::ble::pal::SecurityManager::send_pairing_request - */ - virtual ble_error_t send_pairing_request( - connection_handle_t connection, - bool oob_data_flag, - AuthenticationMask authentication_requirements, - KeyDistribution initiator_dist, - KeyDistribution responder_dist - ); - - /** - * @see ::ble::pal::SecurityManager::send_pairing_response - */ - virtual ble_error_t send_pairing_response( - connection_handle_t connection, - bool oob_data_flag, - AuthenticationMask authentication_requirements, - KeyDistribution initiator_dist, - KeyDistribution responder_dist - ); - - /** - * @see ::ble::pal::SecurityManager::cancel_pairing - */ - virtual ble_error_t cancel_pairing( - connection_handle_t connection, pairing_failure_t reason - ); - /** * @see ::ble::pal::SecurityManager::get_random_data */ @@ -259,6 +252,11 @@ public: // MITM // + /** + * @see ::ble::pal::SecurityManager::set_display_passkey + */ + virtual ble_error_t set_display_passkey(passkey_num_t passkey); + /** * @see ::ble::pal::SecurityManager::passkey_request_reply */ @@ -267,6 +265,16 @@ public: passkey_num_t passkey ); + /** + * @see ::ble::pal::SecurityManager::secure_connections_oob_request_reply + */ + virtual ble_error_t secure_connections_oob_request_reply( + connection_handle_t connection, + const oob_lesc_value_t &local_random, + const oob_lesc_value_t &peer_random, + const oob_confirm_t &peer_confirm + ); + /** * @see ::ble::pal::SecurityManager::legacy_pairing_oob_request_reply */ @@ -296,16 +304,6 @@ public: connection_handle_t connection ); - /** - * @see ::ble::pal::SecurityManager::secure_connections_oob_request_reply - */ - virtual ble_error_t secure_connections_oob_request_reply( - connection_handle_t connection, - const oob_lesc_value_t &local_random, - const oob_lesc_value_t &peer_random, - const oob_confirm_t &peer_confirm - ); - // singleton of the ARM Cordio Security Manager static CordioSecurityManager &get_security_manager(); @@ -315,6 +313,8 @@ public: private: bool _use_default_passkey; passkey_num_t _default_passkey; + bool _lesc_keys_generated; + uint8_t _public_key_x[SEC_ECC_KEY_LEN]; }; } // cordio diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp index 7764bbd09c..2935fdcc41 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include + #include "CordioPalSecurityManager.h" #include "dm_api.h" #include "smp_api.h" @@ -27,7 +29,9 @@ namespace cordio { CordioSecurityManager::CordioSecurityManager() : ::ble::pal::SecurityManager(), _use_default_passkey(false), - _default_passkey(0) + _default_passkey(0), + _lesc_keys_generated(false), + _public_key_x() { } @@ -43,6 +47,17 @@ CordioSecurityManager::~CordioSecurityManager() ble_error_t CordioSecurityManager::initialize() { + // reset local state + _use_default_passkey = false; + _default_passkey = 0; + _lesc_keys_generated = false; + +#if 0 + // FIXME: need help from the stack or local calculation + // generate a new set of keys + DmSecGenerateEccKeyReq(); +#endif + return BLE_ERROR_NONE; } @@ -53,6 +68,7 @@ ble_error_t CordioSecurityManager::terminate() ble_error_t CordioSecurityManager::reset() { + initialize(); return BLE_ERROR_NONE; } @@ -93,6 +109,8 @@ ble_error_t CordioSecurityManager::clear_resolving_list() // Feature support // +// FIXME: Enable when new function available in the pal. +#if 0 ble_error_t CordioSecurityManager::set_secure_connections_support( bool enabled, bool secure_connections_only ) { @@ -104,6 +122,7 @@ ble_error_t CordioSecurityManager::set_secure_connections_support( } return BLE_ERROR_NONE; } +#endif ble_error_t CordioSecurityManager::get_secure_connections_support( bool &enabled @@ -253,12 +272,6 @@ ble_error_t CordioSecurityManager::set_csrk(const csrk_t& csrk) return BLE_ERROR_NONE; } -ble_error_t CordioSecurityManager::generate_public_key() -{ - // FIXME - return BLE_ERROR_NOT_IMPLEMENTED; -} - //////////////////////////////////////////////////////////////////////////// // Global parameters // @@ -380,8 +393,8 @@ ble_error_t CordioSecurityManager::legacy_pairing_oob_request_reply( ble_error_t CordioSecurityManager::confirmation_entered( connection_handle_t connection, bool confirmation ) { - // FIXME: - return BLE_ERROR_NOT_IMPLEMENTED; + DmSecCompareRsp(connection, confirmation); + return BLE_ERROR_NONE; } // FIXME: remove when declaration from the stack is available @@ -397,6 +410,13 @@ ble_error_t CordioSecurityManager::send_keypress_notification( ble_error_t CordioSecurityManager::generate_secure_connections_oob( connection_handle_t connection ) { + // Note: this is not tie to a connection; only one oob value is present in + // the pal. + + uint8_t oobLocalRandom[SMP_RAND_LEN]; + SecRand(oobLocalRandom, SMP_RAND_LEN); + DmSecCalcOobReq(oobLocalRandom, _public_key_x); + return BLE_ERROR_NOT_IMPLEMENTED; } @@ -406,7 +426,18 @@ ble_error_t CordioSecurityManager::secure_connections_oob_request_reply( const oob_lesc_value_t &peer_random, const oob_confirm_t &peer_confirm ) { - return BLE_ERROR_NOT_IMPLEMENTED; + dmSecLescOobCfg_t oob_config = { 0 }; + + memcpy(oob_config.localRandom, local_random.data(), local_random.size()); + // FIXME: + // memcpy(oob_config.localConfirm, ?, ?); + memcpy(oob_config.peerRandom, peer_random.data(), peer_random.size()); + memcpy(oob_config.peerConfirm, peer_confirm.data(), peer_confirm.size()); + + DmSecSetOob(connection, &oob_config); + DmSecAuthRsp(connection, 0, NULL); + + return BLE_ERROR_NONE; } CordioSecurityManager& CordioSecurityManager::get_security_manager() @@ -416,8 +447,8 @@ CordioSecurityManager& CordioSecurityManager::get_security_manager() } bool CordioSecurityManager::sm_handler(const wsfMsgHdr_t* msg) { - SecurityManager::EventHandler* handler = - get_security_manager().get_event_handler(); + CordioSecurityManager& self = get_security_manager(); + SecurityManager::EventHandler* handler = self.get_event_handler(); if ((msg == NULL) || (handler == NULL)) { return false; @@ -479,6 +510,11 @@ bool CordioSecurityManager::sm_handler(const wsfMsgHdr_t* msg) { connection_handle_t connection = evt->hdr.param; if (evt->oob) { + // FIXME: Nothing in the API indicates if smp or sc OOB are + // requested. + // To set secure connection OOB: + // - DmSecSetOob(connection, oob_data) + // - DmSecAuthRsp(connection, 0, NULL) handler->on_legacy_pairing_oob_request(connection); } else if (evt->display) { if (get_security_manager()._use_default_passkey) { @@ -602,18 +638,36 @@ bool CordioSecurityManager::sm_handler(const wsfMsgHdr_t* msg) { return true; } - case DM_SEC_CALC_OOB_IND: + 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 + ); return true; + } - case DM_SEC_ECC_KEY_IND: + case DM_SEC_ECC_KEY_IND: { + secEccMsg_t* evt = (secEccMsg_t*) msg; + DmSecSetEccKey(&evt->data.key); + memcpy(self._public_key_x, evt->data.key.pubKey_x, sizeof(_public_key_x)); + self._lesc_keys_generated = true; return true; + } - case DM_SEC_COMPARE_IND: + case DM_SEC_COMPARE_IND: { + dmSecCnfIndEvt_t* evt = (dmSecCnfIndEvt_t*) msg; + handler->on_passkey_display( + /* connection */ evt->hdr.param, + DmSecGetCompareValue(evt->confirm) + ); + handler->on_confirmation_request(/* connection */ evt->hdr.param); return true; + } case DM_SEC_KEYPRESS_IND: { dmSecKeypressIndEvt_t* evt = (dmSecKeypressIndEvt_t*) msg; - handler->on_keypress_notification( /* connection */ evt->hdr.param, (Keypress_t) evt->notificationType