From 95cd37b49113b7d9e49dd700a11a332863d4531a Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Thu, 29 Mar 2018 17:22:48 +0100 Subject: [PATCH 01/14] BLE: Add const overload for byte_array_t subscribt operator. --- features/FEATURE_BLE/ble/BLETypes.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/features/FEATURE_BLE/ble/BLETypes.h b/features/FEATURE_BLE/ble/BLETypes.h index 856f3ca84d..dbeadfe316 100644 --- a/features/FEATURE_BLE/ble/BLETypes.h +++ b/features/FEATURE_BLE/ble/BLETypes.h @@ -310,6 +310,13 @@ struct byte_array_t { return _value[i]; } + /** + * Subscript operator to access data content + */ + uint8_t operator[](uint8_t i) const { + return _value[i]; + } + /** * Return the pointer to the buffer holding data. */ From 3218e7980533cc17c1bb71ea6772d3e5114def37 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Thu, 29 Mar 2018 17:23:24 +0100 Subject: [PATCH 02/14] BLE: Fix reference to security manager in Nordic event handler. --- .../targets/TARGET_NORDIC/TARGET_NRF5/source/btle/btle.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/btle/btle.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/btle/btle.cpp index d3b63c211b..32e1e3df0c 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/btle/btle.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/btle/btle.cpp @@ -185,7 +185,7 @@ static void btle_handler(ble_evt_t *p_ble_evt) nRF5xn &ble = nRF5xn::Instance(BLE::DEFAULT_INSTANCE); nRF5xGap &gap = (nRF5xGap &) ble.getGap(); nRF5xGattServer &gattServer = (nRF5xGattServer &) ble.getGattServer(); - nRF5xSecurityManager &securityManager = (nRF5xSecurityManager &) ble.getSecurityManager(); + nRF5xSecurityManager &securityManager = nRF5xSecurityManager::get_security_manager(); /* Custom event handler */ switch (p_ble_evt->header.evt_id) { From f0e18fa007335a5ace2cd202cbd48e2e392c9315 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Thu, 29 Mar 2018 17:23:54 +0100 Subject: [PATCH 03/14] BLE: Add LESC crypto toolbox for Nordic. --- .../TARGET_NRF5/source/nRF5XCrypto.cpp | 140 ++++++++++++++++++ .../TARGET_NRF5/source/nRF5xCrypto.h | 71 +++++++++ 2 files changed, 211 insertions(+) create mode 100644 features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp create mode 100644 features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp new file mode 100644 index 0000000000..9c6cb00232 --- /dev/null +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp @@ -0,0 +1,140 @@ +#include + +#if !defined(MBEDTLS_CONFIG_FILE) +#include "mbedtls/config.h" +#else +#include MBEDTLS_CONFIG_FILE +#endif +#include +#include + + +#include "mbedtls/platform.h" +#include "mbedtls/ecdh.h" +#include "mbedtls/memory_buffer_alloc.h" +#include "mbedtls/entropy.h" +#include "mbedtls/ecp.h" + +#include "platform/NonCopyable.h" +#include "platform/CriticalSectionLock.h" +#include "ble/BLETypes.h" +#include "cmsis.h" +#include "nRF5xCrypto.h" + +namespace ble { +namespace pal { +namespace vendor { +namespace nordic { + +LescCrypto::LescCrypto() : _initialized(false) { + mbedtls_entropy_init(&_entropy_context); + mbedtls_ecp_group_init(&_group); + int err = mbedtls_ecp_group_load( + &_group, + MBEDTLS_ECP_DP_SECP256R1 + ); + _initialized = err ? false : true; +} + +LescCrypto::~LescCrypto() { + mbedtls_ecp_group_free(&_group); + mbedtls_entropy_free(&_entropy_context); +} + +bool LescCrypto::generate_keys( + ble::public_key_coord_t& X, + ble::public_key_coord_t& Y, + ble::public_key_coord_t& secret +) { + mbedtls_mpi secret_key; + mbedtls_ecp_point public_keys; + + mbedtls_mpi_init(&secret_key); + mbedtls_ecp_point_init(&public_keys); + + int err = mbedtls_ecp_gen_keypair( + &_group, + &secret_key, + &public_keys, + mbedtls_entropy_func, + &_entropy_context + ); + + if (!err) { + store_mpi(secret, secret_key); + store_mpi(X, public_keys.X); + store_mpi(Y, public_keys.Y); + } + + mbedtls_ecp_point_free(&public_keys); + mbedtls_mpi_free(&secret_key); + + return err ? false : true; +} + +bool LescCrypto::generate_shared_secret( + const ble::public_key_coord_t& peer_X, + const ble::public_key_coord_t& peer_Y, + const ble::public_key_coord_t& own_secret, + ble::public_key_coord_t& shared_secret +) { + mbedtls_mpi result; + mbedtls_mpi secret_key; + mbedtls_ecp_point public_keys; + + mbedtls_mpi_init(&result); + mbedtls_mpi_init(&secret_key); + mbedtls_ecp_point_init(&public_keys); + + load_mpi(secret_key, own_secret); + load_mpi(public_keys.X, peer_X); + load_mpi(public_keys.Y, peer_Y); + mbedtls_mpi_lset( &public_keys.Z, 1 ); + + int err = mbedtls_ecdh_compute_shared( + &_group, + &result, + &public_keys, + &secret_key, + /* rng function; optional */ NULL, + /* rng param */ NULL + ); + + if (!err) { + store_mpi(shared_secret, result); + } + + mbedtls_ecp_point_free(&public_keys); + mbedtls_mpi_free(&secret_key); + mbedtls_mpi_free(&result); + + return err ? false : true; +} + + +void LescCrypto::load_mpi(mbedtls_mpi& dest, const ble::public_key_coord_t& src) { + ble::public_key_coord_t src_be = src; + swap_endian(src_be); + mbedtls_mpi_read_binary(&dest, src_be.buffer(), src_be.size()); +} + +void LescCrypto::store_mpi(ble::public_key_coord_t& dest, const mbedtls_mpi& src) { + mbedtls_mpi_write_binary(&src, dest.buffer(), dest.size()); + swap_endian(dest); +} + +void LescCrypto::swap_endian(ble::public_key_coord_t& to_swap) { + swap_endian(to_swap.buffer(), to_swap.size()); +} + +void LescCrypto::swap_endian(uint8_t* buf, size_t len) { + for(size_t low = 0, high = (len - 1); high > low; --high, ++low) { + std::swap(buf[low], buf[high]); + } +} + +} // nordic +} // vendor +} // pal +} // ble + diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h new file mode 100644 index 0000000000..1478d4213f --- /dev/null +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h @@ -0,0 +1,71 @@ +#ifndef NRF5X_CRYPTO_ +#define NRF5X_CRYPTO_ + +#include + +#if !defined(MBEDTLS_CONFIG_FILE) +#include "mbedtls/config.h" +#else +#include MBEDTLS_CONFIG_FILE +#endif +#include +#include + + +#include "mbedtls/platform.h" +#include "mbedtls/ecdh.h" +#include "mbedtls/memory_buffer_alloc.h" +#include "mbedtls/entropy.h" +#include "mbedtls/ecp.h" + +#include "platform/NonCopyable.h" +#include "platform/CriticalSectionLock.h" +#include "ble/BLETypes.h" +#include "cmsis.h" + +namespace ble { +namespace pal { +namespace vendor { +namespace nordic { + +class LescCrypto : mbed::NonCopyable { + +public: + LescCrypto(); + + ~LescCrypto(); + + bool generate_keys( + ble::public_key_coord_t& X, + ble::public_key_coord_t& Y, + ble::public_key_coord_t& secret + ); + + bool generate_shared_secret( + const ble::public_key_coord_t& peer_X, + const ble::public_key_coord_t& peer_Y, + const ble::public_key_coord_t& own_secret, + ble::public_key_coord_t& shared_secret + ); + +private: + + void load_mpi(mbedtls_mpi& dest, const ble::public_key_coord_t& src); + + void store_mpi(ble::public_key_coord_t& dest, const mbedtls_mpi& src); + + void swap_endian(ble::public_key_coord_t& to_swap); + + void swap_endian(uint8_t* buf, size_t len); + + bool _initialized; + mbedtls_entropy_context _entropy_context; + mbedtls_ecp_group _group; +}; + +} // nordic +} // vendor +} // pal +} // ble + +#endif // NRF5X_CRYPTO_ From 3272863f8374f92866468da0bf88ad9321278b96 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Thu, 29 Mar 2018 17:24:46 +0100 Subject: [PATCH 04/14] BLE Nordic: Support LESC. --- .../source/nRF5xPalSecurityManager.cpp | 183 ++++++++++++++---- .../source/nRF5xPalSecurityManager.h | 32 +-- 2 files changed, 157 insertions(+), 58 deletions(-) diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp index adf5572f58..cce5cf1d83 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp @@ -76,6 +76,11 @@ struct nRF5xSecurityManager::pairing_control_block_t { ble_gap_id_key_t peer_id_key; ble_gap_sign_info_t peer_sign_key; ble_gap_lesc_p256_pk_t peer_pk; + + // flag required to help DHKey computation/process; should be removed with + // later versions of the softdevice + uint8_t own_oob:1; + uint8_t peer_oob:1; }; nRF5xSecurityManager::nRF5xSecurityManager() @@ -90,7 +95,7 @@ nRF5xSecurityManager::nRF5xSecurityManager() nRF5xSecurityManager::~nRF5xSecurityManager() { - + terminate(); } //////////////////////////////////////////////////////////////////////////// @@ -99,19 +104,27 @@ nRF5xSecurityManager::~nRF5xSecurityManager() ble_error_t nRF5xSecurityManager::initialize() { - // FIXME: generate and set public and private keys - return BLE_ERROR_NONE; + if (_crypto.generate_keys(X, Y, secret)) { + return BLE_ERROR_NONE; + } + + return BLE_ERROR_INTERNAL_STACK_FAILURE; } ble_error_t nRF5xSecurityManager::terminate() { + release_all_pairing_cb(); return BLE_ERROR_NONE; } ble_error_t nRF5xSecurityManager::reset() { - // FIXME: reset public and private keys - return BLE_ERROR_NONE; + ble_error_t err = terminate(); + if (err) { + return err; + } + + return initialize(); } //////////////////////////////////////////////////////////////////////////// @@ -162,6 +175,7 @@ ble_error_t nRF5xSecurityManager::send_pairing_request( KeyDistribution initiator_dist, KeyDistribution responder_dist ) { + printf("nRF5xSecurityManager::send_pairing_request\r\n"); // allocate the control block required for the procedure completion pairing_control_block_t* pairing_cb = allocate_pairing_cb(connection); if (!pairing_cb) { @@ -173,6 +187,10 @@ ble_error_t nRF5xSecurityManager::send_pairing_request( initiator_dist.set_signing(false); responder_dist.set_signing(false); + // override link parameter + initiator_dist.set_link(false); + responder_dist.set_link(false); + ble_gap_sec_params_t security_params = make_security_params( oob_data_flag, authentication_requirements, @@ -209,6 +227,10 @@ ble_error_t nRF5xSecurityManager::send_pairing_response( initiator_dist.set_signing(false); responder_dist.set_signing(false); + // override link parameter + initiator_dist.set_link(false); + responder_dist.set_link(false); + ble_gap_sec_params_t security_params = make_security_params( oob_data_flag, authentication_requirements, @@ -239,7 +261,7 @@ ble_error_t nRF5xSecurityManager::send_pairing_response( ble_error_t nRF5xSecurityManager::cancel_pairing( connection_handle_t connection, pairing_failure_t reason ) { - // this is the default path except when a key is expected to be enterred by + // this is the default path except when a key is expected to be entered by // the user. uint32_t err = sd_ble_gap_sec_params_reply( connection, @@ -272,9 +294,7 @@ ble_error_t nRF5xSecurityManager::cancel_pairing( ble_error_t nRF5xSecurityManager::get_secure_connections_support( bool &enabled ) { - // NRF5x platforms support secure connections - // FIXME: set to true once the ECC library is included - enabled = false; + enabled = true; return BLE_ERROR_NONE; } @@ -399,7 +419,7 @@ ble_error_t nRF5xSecurityManager::encrypt_data( const byte_array_t<16> &key, encryption_block_t &data ) { - // FIXME: With signing ? + // FIXME: Implement in LescCrypto ? return BLE_ERROR_NOT_IMPLEMENTED; } @@ -446,8 +466,8 @@ ble_error_t nRF5xSecurityManager::set_ltk( uint32_t err = sd_ble_gap_sec_info_reply( connection, &enc_info, - NULL, - NULL // Not supported + /* id info */ NULL, + /* sign info */ NULL // Not supported ); return convert_sd_error(err); @@ -520,6 +540,11 @@ ble_error_t nRF5xSecurityManager::set_display_passkey(passkey_num_t passkey) ble_error_t nRF5xSecurityManager::passkey_request_reply( connection_handle_t connection, const passkey_num_t passkey ) { + pairing_control_block_t* pairing_cb = get_pairing_cb(connection); + if (!pairing_cb) { + return BLE_ERROR_INVALID_STATE; + } + PasskeyAscii pkasc(passkey); uint32_t err = sd_ble_gap_auth_key_reply( connection, @@ -530,6 +555,37 @@ ble_error_t nRF5xSecurityManager::passkey_request_reply( return convert_sd_error(err); } +ble_error_t nRF5xSecurityManager::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 +) { + pairing_control_block_t* pairing_cb = get_pairing_cb(connection); + if (!pairing_cb) { + return BLE_ERROR_INVALID_STATE; + } + + ble_gap_lesc_oob_data_t oob_own; + ble_gap_lesc_oob_data_t oob_peer; + + // is own address important ? + memcpy(oob_own.r, local_random.data(), local_random.size()); + // FIXME: What to do with local confirm ??? + + // is peer address important ? + memcpy(oob_peer.r, peer_random.data(), peer_random.size()); + memcpy(oob_peer.c, peer_confirm.data(), peer_confirm.size()); + + uint32_t err = sd_ble_gap_lesc_oob_data_set( + connection, + pairing_cb->own_oob ? &oob_own : NULL, + pairing_cb->peer_oob ? &oob_peer : NULL + ); + + return convert_sd_error(err); +} + ble_error_t nRF5xSecurityManager::legacy_pairing_oob_request_reply( connection_handle_t connection, const oob_tk_t& oob_data @@ -546,11 +602,17 @@ ble_error_t nRF5xSecurityManager::legacy_pairing_oob_request_reply( ble_error_t nRF5xSecurityManager::confirmation_entered( connection_handle_t connection, bool confirmation ) { + pairing_control_block_t* pairing_cb = get_pairing_cb(connection); + if (!pairing_cb) { + return BLE_ERROR_INVALID_STATE; + } + uint32_t err = sd_ble_gap_auth_key_reply( connection, confirmation ? BLE_GAP_AUTH_KEY_TYPE_PASSKEY : BLE_GAP_AUTH_KEY_TYPE_NONE, NULL ); + return convert_sd_error(err); } @@ -568,25 +630,26 @@ ble_error_t nRF5xSecurityManager::send_keypress_notification( ble_error_t nRF5xSecurityManager::generate_secure_connections_oob( connection_handle_t connection ) { - // FIXME: made with external library; requires SDK v13 - return BLE_ERROR_NOT_IMPLEMENTED; -} + ble_gap_lesc_p256_pk_t own_secret; + ble_gap_lesc_oob_data_t oob_data; + memcpy(own_secret.pk, secret.buffer(), secret.size()); -ble_error_t nRF5xSecurityManager::secure_connections_oob_received( - const address_t &address, - const oob_lesc_value_t &random, - const oob_confirm_t &confirm -) { - // FIXME: store locally - return BLE_ERROR_NOT_IMPLEMENTED; -} + uint32_t err = sd_ble_gap_lesc_oob_data_get( + connection, + &own_secret, + &oob_data + ); -bool nRF5xSecurityManager::is_secure_connections_oob_present( - const address_t &address -) { - // FIXME: lookup local store - return false; + if (!err) { + get_event_handler()->on_secure_connections_oob_generated( + connection, + oob_data.r, + oob_data.c + ); + } + + return convert_sd_error(err); } nRF5xSecurityManager& nRF5xSecurityManager::get_security_manager() @@ -653,18 +716,15 @@ bool nRF5xSecurityManager::sm_handler(const ble_evt_t *evt) ); } } else { - AuthenticationMask authentication_requirements( - params.bond, - params.mitm, - params.lesc, - params.keypress - ); - - handler->on_pairing_request( connection, params.oob, - authentication_requirements, + AuthenticationMask( + params.bond, + params.mitm, + params.lesc, + params.keypress + ), initiator_dist, responder_dist ); @@ -695,7 +755,11 @@ bool nRF5xSecurityManager::sm_handler(const ble_evt_t *evt) PasskeyAscii::to_num(req.passkey) ); } else { - // FIXME handle this case for secure pairing + handler->on_passkey_display( + connection, + PasskeyAscii::to_num(req.passkey) + ); + handler->on_confirmation_request(connection); } return true; @@ -708,6 +772,7 @@ bool nRF5xSecurityManager::sm_handler(const ble_evt_t *evt) ); return true; } + case BLE_GAP_EVT_AUTH_KEY_REQUEST: { uint8_t key_type = gap_evt.params.auth_key_request.key_type; @@ -727,9 +792,32 @@ bool nRF5xSecurityManager::sm_handler(const ble_evt_t *evt) return true; } - case BLE_GAP_EVT_LESC_DHKEY_REQUEST: - // FIXME: Add with LESC support + case BLE_GAP_EVT_LESC_DHKEY_REQUEST: { + const ble_gap_evt_lesc_dhkey_request_t& dhkey_request = + gap_evt.params.lesc_dhkey_request; + + size_t key_size = public_key_coord_t::size(); + public_key_coord_t peer_X(dhkey_request.p_pk_peer->pk, key_size); + public_key_coord_t peer_Y(dhkey_request.p_pk_peer->pk + key_size, key_size); + public_key_coord_t sh_secret; + ble_gap_lesc_dhkey_t shared_secret; + + _crypto.generate_shared_secret( + peer_X, + peer_Y, + self.secret, + sh_secret + ); + + memcpy(shared_secret.key, sh_secret.data(), sh_secret.size()); + sd_ble_gap_lesc_dhkey_reply(connection, &shared_secret); + + if (dhkey_request.oobd_req) { + handler->on_secure_connections_oob_request(connection); + } + return true; + } case BLE_GAP_EVT_AUTH_STATUS: { const ble_gap_evt_auth_status_t& status = gap_evt.params.auth_status; @@ -954,13 +1042,13 @@ ble_gap_sec_keyset_t nRF5xSecurityManager::make_keyset( own_dist->get_encryption() ? &pairing_cb.own_enc_key : NULL, own_dist->get_identity() ? &pairing_cb.own_id_key : NULL, own_dist->get_signing() ? &pairing_cb.own_sign_key : NULL, - own_dist->get_link() ? &pairing_cb.own_pk : NULL + &pairing_cb.own_pk }, /* keys_peer */ { peer_dist->get_encryption() ? &pairing_cb.peer_enc_key : NULL, peer_dist->get_identity() ? &pairing_cb.peer_id_key : NULL, peer_dist->get_signing() ? &pairing_cb.peer_sign_key : NULL, - peer_dist->get_link() ? &pairing_cb.peer_pk : NULL + &pairing_cb.peer_pk } }; @@ -969,6 +1057,10 @@ ble_gap_sec_keyset_t nRF5xSecurityManager::make_keyset( memcpy(keyset.keys_own.p_sign_key->csrk, _csrk.data(), _csrk.size()); } + // copy public keys used + memcpy(pairing_cb.own_pk.pk, X.data(), X.size()); + memcpy(pairing_cb.own_pk.pk + X.size(), Y.data(), Y.size()); + return keyset; } @@ -1016,6 +1108,13 @@ nRF5xSecurityManager::get_pairing_cb(connection_handle_t connection) return NULL; } +void nRF5xSecurityManager::release_all_pairing_cb() +{ + while(_control_blocks) { + release_pairing_cb(_control_blocks); + } +} + } // nordic } // vendor } // pal diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.h b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.h index add7d4e6ce..f9263998ad 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.h +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.h @@ -20,6 +20,7 @@ #include "ble/BLETypes.h" #include "ble/pal/PalSecurityManager.h" #include "nrf_ble.h" +#include "nRF5xCrypto.h" namespace ble { namespace pal { @@ -264,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 */ @@ -293,22 +304,6 @@ public: connection_handle_t connection ); - /** - * @see ::ble::pal::SecurityManager::secure_connections_oob_received - */ - 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 - ); - // singleton of nordic Security Manager static nRF5xSecurityManager& get_security_manager(); @@ -339,8 +334,13 @@ private: pairing_control_block_t* allocate_pairing_cb(connection_handle_t connection); void release_pairing_cb(pairing_control_block_t* pairing_cb); pairing_control_block_t* get_pairing_cb(connection_handle_t connection); + void release_all_pairing_cb(); pairing_control_block_t* _control_blocks; + LescCrypto _crypto; + ble::public_key_coord_t X; + ble::public_key_coord_t Y; + ble::public_key_coord_t secret; }; } // nordic From 7bf0eb009a4384556a72d636c161b7e6420692e5 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 11:04:13 +0100 Subject: [PATCH 05/14] BLE: use ArrayView to pass and get parameters in Crypto API This change allow vendor pal code to use its own array format. --- .../TARGET_NRF5/source/nRF5XCrypto.cpp | 42 +++++++++++-------- .../TARGET_NRF5/source/nRF5xCrypto.h | 21 +++++----- .../source/nRF5xPalSecurityManager.cpp | 18 ++++---- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp index 9c6cb00232..7e4c0de9d0 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp @@ -20,6 +20,7 @@ #include "ble/BLETypes.h" #include "cmsis.h" #include "nRF5xCrypto.h" +#include "platform/mbed_assert.h" namespace ble { namespace pal { @@ -42,10 +43,14 @@ LescCrypto::~LescCrypto() { } bool LescCrypto::generate_keys( - ble::public_key_coord_t& X, - ble::public_key_coord_t& Y, - ble::public_key_coord_t& secret + ArrayView X, + ArrayView Y, + ArrayView secret ) { + MBED_ASSERT(X.size() == public_key_coord_t::size()); + MBED_ASSERT(Y.size() == public_key_coord_t::size()); + MBED_ASSERT(secret.size() == public_key_coord_t::size()); + mbedtls_mpi secret_key; mbedtls_ecp_point public_keys; @@ -73,11 +78,16 @@ bool LescCrypto::generate_keys( } bool LescCrypto::generate_shared_secret( - const ble::public_key_coord_t& peer_X, - const ble::public_key_coord_t& peer_Y, - const ble::public_key_coord_t& own_secret, - ble::public_key_coord_t& shared_secret + const ArrayView& peer_X, + const ArrayView& peer_Y, + const ArrayView& own_secret, + ArrayView shared_secret ) { + MBED_ASSERT(peer_X.size() == public_key_coord_t::size()); + MBED_ASSERT(peer_Y.size() == public_key_coord_t::size()); + MBED_ASSERT(own_secret.size() == public_key_coord_t::size()); + MBED_ASSERT(shared_secret.size() == dhkey_t::size()); + mbedtls_mpi result; mbedtls_mpi secret_key; mbedtls_ecp_point public_keys; @@ -112,19 +122,15 @@ bool LescCrypto::generate_shared_secret( } -void LescCrypto::load_mpi(mbedtls_mpi& dest, const ble::public_key_coord_t& src) { - ble::public_key_coord_t src_be = src; - swap_endian(src_be); - mbedtls_mpi_read_binary(&dest, src_be.buffer(), src_be.size()); +void LescCrypto::load_mpi(mbedtls_mpi& dest, const ArrayView& src) { + ble::public_key_coord_t src_be = src.data(); + swap_endian(src_be.buffer(), src_be.size()); + mbedtls_mpi_read_binary(&dest, src_be.data(), src_be.size()); } -void LescCrypto::store_mpi(ble::public_key_coord_t& dest, const mbedtls_mpi& src) { - mbedtls_mpi_write_binary(&src, dest.buffer(), dest.size()); - swap_endian(dest); -} - -void LescCrypto::swap_endian(ble::public_key_coord_t& to_swap) { - swap_endian(to_swap.buffer(), to_swap.size()); +void LescCrypto::store_mpi(ArrayView& dest, const mbedtls_mpi& src) { + mbedtls_mpi_write_binary(&src, dest.data(), dest.size()); + swap_endian(dest.data(), dest.size()); } void LescCrypto::swap_endian(uint8_t* buf, size_t len) { diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h index 1478d4213f..afbb37b898 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h @@ -22,6 +22,7 @@ #include "platform/CriticalSectionLock.h" #include "ble/BLETypes.h" #include "cmsis.h" +#include "ble/ArrayView.h" namespace ble { namespace pal { @@ -36,25 +37,25 @@ public: ~LescCrypto(); bool generate_keys( - ble::public_key_coord_t& X, - ble::public_key_coord_t& Y, - ble::public_key_coord_t& secret + ArrayView X, + ArrayView Y, + ArrayView secret ); bool generate_shared_secret( - const ble::public_key_coord_t& peer_X, - const ble::public_key_coord_t& peer_Y, - const ble::public_key_coord_t& own_secret, - ble::public_key_coord_t& shared_secret + const ArrayView& peer_X, + const ArrayView& peer_Y, + const ArrayView& own_secret, + ArrayView shared_secret ); private: - void load_mpi(mbedtls_mpi& dest, const ble::public_key_coord_t& src); + void load_mpi(mbedtls_mpi& dest, const ArrayView& src); - void store_mpi(ble::public_key_coord_t& dest, const mbedtls_mpi& src); + void store_mpi(ArrayView& dest, const mbedtls_mpi& src); - void swap_endian(ble::public_key_coord_t& to_swap); + void swap_endian(ArrayView& to_swap); void swap_endian(uint8_t* buf, size_t len); diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp index cce5cf1d83..e6f00eccc5 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp @@ -104,7 +104,11 @@ nRF5xSecurityManager::~nRF5xSecurityManager() ble_error_t nRF5xSecurityManager::initialize() { - if (_crypto.generate_keys(X, Y, secret)) { + if (_crypto.generate_keys( + make_ArrayView(X.buffer(), X.size()), + make_ArrayView(Y.buffer(), Y.size()), + make_ArrayView(secret.buffer(), secret.size()) + )) { return BLE_ERROR_NONE; } @@ -797,19 +801,15 @@ bool nRF5xSecurityManager::sm_handler(const ble_evt_t *evt) gap_evt.params.lesc_dhkey_request; size_t key_size = public_key_coord_t::size(); - public_key_coord_t peer_X(dhkey_request.p_pk_peer->pk, key_size); - public_key_coord_t peer_Y(dhkey_request.p_pk_peer->pk + key_size, key_size); - public_key_coord_t sh_secret; ble_gap_lesc_dhkey_t shared_secret; _crypto.generate_shared_secret( - peer_X, - peer_Y, - self.secret, - sh_secret + make_const_ArrayView(dhkey_request.p_pk_peer->pk, key_size), + make_const_ArrayView(dhkey_request.p_pk_peer->pk + key_size, key_size), + make_const_ArrayView(secret.data(), secret.size()), + shared_secret.key ); - memcpy(shared_secret.key, sh_secret.data(), sh_secret.size()); sd_ble_gap_lesc_dhkey_reply(connection, &shared_secret); if (dhkey_request.oobd_req) { From bf41bb3164084fed4988ef043ddcc3c30a97ce5a Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 15:21:05 +0100 Subject: [PATCH 06/14] BLE: Export byte_array_t size exploitable at compile time. --- features/FEATURE_BLE/ble/BLETypes.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/features/FEATURE_BLE/ble/BLETypes.h b/features/FEATURE_BLE/ble/BLETypes.h index dbeadfe316..5dba99796c 100644 --- a/features/FEATURE_BLE/ble/BLETypes.h +++ b/features/FEATURE_BLE/ble/BLETypes.h @@ -263,6 +263,11 @@ private: template struct byte_array_t { + /** + * Size of the array; accessible at compile time. + */ + static const size_t size_ = array_size; + /** * Default to all zeroes */ From d8f3d9c5eff76358229a2d11028a4ebdf1a90af5 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 15:21:54 +0100 Subject: [PATCH 07/14] BLE: provide non const overload of byte_array_t::data member function. --- features/FEATURE_BLE/ble/BLETypes.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/features/FEATURE_BLE/ble/BLETypes.h b/features/FEATURE_BLE/ble/BLETypes.h index 5dba99796c..f5f13a84df 100644 --- a/features/FEATURE_BLE/ble/BLETypes.h +++ b/features/FEATURE_BLE/ble/BLETypes.h @@ -329,6 +329,14 @@ struct byte_array_t { return _value; } + /** + * Return the pointer to the buffer holding data. + */ + uint8_t* data() { + return _value; + } + + /** * Return the pointer to the buffer holding data. */ From 9e1f0b34f8e69994582fa97140bac73029217f1f Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 15:22:15 +0100 Subject: [PATCH 08/14] BLE: Fix byte_array_t subscript operator --- features/FEATURE_BLE/ble/BLETypes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/FEATURE_BLE/ble/BLETypes.h b/features/FEATURE_BLE/ble/BLETypes.h index f5f13a84df..5f20c232a4 100644 --- a/features/FEATURE_BLE/ble/BLETypes.h +++ b/features/FEATURE_BLE/ble/BLETypes.h @@ -311,14 +311,14 @@ struct byte_array_t { /** * Subscript operator to access data content */ - uint8_t& operator[](uint8_t i) { + uint8_t& operator[](size_t i) { return _value[i]; } /** * Subscript operator to access data content */ - uint8_t operator[](uint8_t i) const { + uint8_t operator[](size_t i) const { return _value[i]; } From 5761caff00f8791068bdc9a932d51547850c8cf0 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 15:28:17 +0100 Subject: [PATCH 09/14] BLE: Extend ArrayView to encode size in type. With this change, it is possible to encode the size of the array viewed by an ArrayView into the type itself: ArrayView. Such objects are lighter than ArrayView of arbitrary size and allows verification of the size at compile time. This change also fix operator== and bring new make_ArrayView overloads. --- features/FEATURE_BLE/ble/ArrayView.h | 241 +++++++++++++++++++++++---- 1 file changed, 206 insertions(+), 35 deletions(-) diff --git a/features/FEATURE_BLE/ble/ArrayView.h b/features/FEATURE_BLE/ble/ArrayView.h index d2a2b6d135..fad829eba1 100644 --- a/features/FEATURE_BLE/ble/ArrayView.h +++ b/features/FEATURE_BLE/ble/ArrayView.h @@ -17,8 +17,11 @@ #ifndef BLE_ARRAY_VIEW_H_ #define BLE_ARRAY_VIEW_H_ +#include + #include #include +#include "platform/mbed_assert.h" /** * @addtogroup ble @@ -33,11 +36,18 @@ namespace ble { +/** + * Special value for the Size parameter of ArrayView. + * If the type use this value then the size of the array is stored in the object + * at runtime. + */ +#define ARRAY_VIEW_DYNAMIC_SIZE -1 + /** * Immutable view to an array. * * Array views encapsulate the pointer to an array and its size into a single - * object; however, it does not manage the lifetime of the array viewed. + * object or type; however, it does not manage the lifetime of the array viewed. * You can use instances of ArrayView to replace the traditional pair of pointer * and size arguments in function calls. * @@ -48,20 +58,28 @@ namespace ble { * @note You can create ArrayView instances with the help of the function * template make_ArrayView() and make_const_ArrayView(). * + * @note ArrayView objects can be implicitly converted to ArrayView + * objects where required. + * * @tparam T type of objects held by the array. + * @tparam Size The size of the array viewed. The default value + * ARRAY_VIEW_DYNAMIC_SIZE is special as it allows construction of ArrayView + * objects of any size (set at runtime). */ -template +template struct ArrayView { + MBED_STATIC_ASSERT(Size >= 0, "Invalid size for an ArrayView"); + /** * Construct a view to an empty array. * * @post a call to size() will return 0, and data() will return NULL. */ - ArrayView() : _array(0), _size(0) { } + ArrayView() : _array(NULL) { } /** - * Construct an array view from a pointer to a buffer and its size. + * Construct an array view from a pointer to a buffer. * * @param array_ptr Pointer to the array data * @param array_size Number of elements of T present in the array. @@ -70,7 +88,9 @@ struct ArrayView { * array_tpr. */ ArrayView(T* array_ptr, size_t array_size) : - _array(array_ptr), _size(array_size) { } + _array(array_ptr) { + MBED_ASSERT(array_size >= (size_t) Size); + } /** * Construct an array view from the reference to an array. @@ -82,9 +102,8 @@ struct ArrayView { * @post a call to size() will return Size, and data() will return * a pointer to elements. */ - template ArrayView(T (&elements)[Size]): - _array(elements), _size(Size) { } + _array(elements) { } /** * Return the size of the array viewed. @@ -93,7 +112,7 @@ struct ArrayView { */ size_t size() const { - return _size; + return _array ? Size : 0; } /** @@ -144,40 +163,115 @@ struct ArrayView { return _array; } +private: + T* const _array; +}; + +/** + * ArrayView specialisation that handle dynamic array size. + */ +template +struct ArrayView { + /** - * Equality operator. + * Construct a view to an empty array. * - * @param lhs Left hand side of the binary operation. - * @param rhs Right hand side of the binary operation. - * - * @return True if arrays in input have the same size and the same content - * and false otherwise. + * @post a call to size() will return 0, and data() will return NULL. */ - friend bool operator==(const ArrayView& lhs, const ArrayView& rhs) + ArrayView() : _array(0), _size(0) { } + + /** + * Construct an array view from a pointer to a buffer and its size. + * + * @param array_ptr Pointer to the array data + * @param array_size Number of elements of T present in the array. + * + * @post a call to size() will return array_size and data() will return + * array_tpr. + */ + ArrayView(T* array_ptr, size_t array_size) : + _array(array_ptr), _size(array_size) { } + + /** + * Construct an array view from the reference to an array. + * + * @param elements Reference to the array viewed. + * + * @tparam Size Number of elements of T presents in the array. + * + * @post a call to size() will return Size, and data() will return + * a pointer to elements. + */ + template + ArrayView(T (&elements)[Size]): + _array(elements), _size(Size) { } + + + /** + * Construct a ArrayView object with a dynamic size from an ArrayView object + * with a static size. + * @param other The ArrayView object used to construct this. + */ + template + ArrayView(ArrayView other): + _array(other.data()), _size(other.size()) { } + + /** + * Return the size of the array viewed. + * + * @return The number of elements present in the array viewed. + */ + size_t size() const { - if (lhs.size() != rhs.size()) { - return false; - } - - if (lhs.data() == rhs.data()) { - return true; - } - - return memcmp(lhs.data(), rhs.data(), lhs.size()) == 0; + return _size; } /** - * Not equal operator + * Access to a mutable element of the array. * - * @param lhs Left hand side of the binary operation. - * @param rhs Right hand side of the binary operation. + * @param index Element index to access. * - * @return True if arrays in input do not have the same size or the same - * content and false otherwise. + * @return A reference to the element at the index specified in input. + * + * @pre index shall be less than size(). */ - friend bool operator!=(const ArrayView& lhs, const ArrayView& rhs) + T& operator[](size_t index) { - return !(lhs == rhs); + return _array[index]; + } + + /** + * Access to an immutable element of the array. + * + * @param index Element index to access. + * + * @return A const reference to the element at the index specified in input. + * + * @pre index shall be less than size(). + */ + const T& operator[](size_t index) const + { + return _array[index]; + } + + /** + * Get the raw pointer to the array. + * + * @return The raw pointer to the array. + */ + T* data() + { + return _array; + } + + /** + * Get the raw const pointer to the array. + * + * @return The raw pointer to the array. + */ + const T* data() const + { + return _array; } private: @@ -186,6 +280,45 @@ private: }; +/** + * Equality operator. + * + * @param lhs Left hand side of the binary operation. + * @param rhs Right hand side of the binary operation. + * + * @return True if arrays in input have the same size and the same content + * and false otherwise. + */ +template +bool operator==(const ArrayView& lhs, const ArrayView& rhs) +{ + if (lhs.size() != rhs.size()) { + return false; + } + + if (lhs.data() == rhs.data()) { + return true; + } + + return std::equal(lhs.data(), lhs.data() + lhs.size(), rhs.data()); +} + +/** + * Not equal operator + * + * @param lhs Left hand side of the binary operation. + * @param rhs Right hand side of the binary operation. + * + * @return True if arrays in input do not have the same size or the same + * content and false otherwise. + */ +template +bool operator!=(const ArrayView& lhs, const ArrayView& rhs) +{ + return !(lhs == rhs); +} + + /** * Generate an array view from a reference to a C/C++ array. * @@ -200,9 +333,28 @@ private: * created 'inline'. */ template -ArrayView make_ArrayView(T (&elements)[Size]) +ArrayView make_ArrayView(T (&elements)[Size]) { - return ArrayView(elements); + return ArrayView(elements); +} + +/** + * Generate an array view from a pointer to a C/C++ array. + * + * @tparam Size Number of items held in elements. + * @tparam T Type of elements held in elements. + * + * @param elements The reference to the array viewed. + * + * @return The ArrayView to elements. + * + * @note This helper avoids the typing of template parameter when ArrayView is + * created 'inline'. + */ +template +ArrayView make_ArrayView(T* elements) +{ + return ArrayView(elements, Size); } /** @@ -237,9 +389,28 @@ ArrayView make_ArrayView(T* array_ptr, size_t array_size) * created 'inline'. */ template -ArrayView make_const_ArrayView(T (&elements)[Size]) +ArrayView make_const_ArrayView(T (&elements)[Size]) { - return ArrayView(elements); + return ArrayView(elements); +} + +/** + * Generate a const array view from a pointer to a C/C++ array. + * + * @tparam Size Number of items held in elements. + * @tparam T Type of elements held in elements. + * + * @param elements The reference to the array viewed. + * + * @return The ArrayView to elements. + * + * @note This helper avoids the typing of template parameter when ArrayView is + * created 'inline'. + */ +template +ArrayView make_const_ArrayView(const T* elements) +{ + return ArrayView(elements, Size); } /** From cd39406d2075a463d386887c7e18fa0aa06702fc Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 15:28:52 +0100 Subject: [PATCH 10/14] BLE: Add conversion function from byte_array_t to ArrayView. --- features/FEATURE_BLE/ble/BLETypes.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/features/FEATURE_BLE/ble/BLETypes.h b/features/FEATURE_BLE/ble/BLETypes.h index 5f20c232a4..bbd6785bc2 100644 --- a/features/FEATURE_BLE/ble/BLETypes.h +++ b/features/FEATURE_BLE/ble/BLETypes.h @@ -21,6 +21,7 @@ #include #include #include "ble/SafeEnum.h" +#include "ble/ArrayView.h" /** * @addtogroup ble @@ -355,6 +356,32 @@ protected: uint8_t _value[array_size]; }; +/** + * Construct a fixed size ArrayView from a byte_array_t. + * + * @param src byte_array_t to create a view from. + * + * @return An ArrayView to @p src. + */ +template +ArrayView make_ArrayView(byte_array_t& src) +{ + return ArrayView(src.data(), src.size()); +} + +/** + * Construct a fixed size ArrayView from a const byte_array_t. + * + * @param src byte_array_t to create a view from. + * + * @return An ArrayView to @p src. + */ +template +ArrayView make_const_ArrayView(const byte_array_t& src) +{ + return ArrayView(src.data(), src.size()); +} + /** 128 bit keys used by paired devices */ typedef byte_array_t<16> irk_t; typedef byte_array_t<16> csrk_t; From 59a301a256f6ec915c3c16b75c94c282294f386c Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 15:31:31 +0100 Subject: [PATCH 11/14] BLE: CryptoToolbox enhancement - Rename LescCrypto into CryptoToolbox - Use ArrayView of fixed size as parameters - Add licence --- .../TARGET_NRF5/source/nRF5XCrypto.cpp | 53 +++++++++++-------- .../TARGET_NRF5/source/nRF5xCrypto.h | 45 +++++++++++----- .../source/nRF5xPalSecurityManager.cpp | 14 ++--- .../source/nRF5xPalSecurityManager.h | 2 +- 4 files changed, 69 insertions(+), 45 deletions(-) diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp index 7e4c0de9d0..c5a4012de2 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp @@ -1,3 +1,19 @@ +/* mbed Microcontroller Library + * Copyright (c) 2018-2018 ARM Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #include #if !defined(MBEDTLS_CONFIG_FILE) @@ -27,7 +43,7 @@ namespace pal { namespace vendor { namespace nordic { -LescCrypto::LescCrypto() : _initialized(false) { +CryptoToolbox::CryptoToolbox() : _initialized(false) { mbedtls_entropy_init(&_entropy_context); mbedtls_ecp_group_init(&_group); int err = mbedtls_ecp_group_load( @@ -37,20 +53,16 @@ LescCrypto::LescCrypto() : _initialized(false) { _initialized = err ? false : true; } -LescCrypto::~LescCrypto() { +CryptoToolbox::~CryptoToolbox() { mbedtls_ecp_group_free(&_group); mbedtls_entropy_free(&_entropy_context); } -bool LescCrypto::generate_keys( - ArrayView X, - ArrayView Y, - ArrayView secret +bool CryptoToolbox::generate_keys( + ArrayView X, + ArrayView Y, + ArrayView secret ) { - MBED_ASSERT(X.size() == public_key_coord_t::size()); - MBED_ASSERT(Y.size() == public_key_coord_t::size()); - MBED_ASSERT(secret.size() == public_key_coord_t::size()); - mbedtls_mpi secret_key; mbedtls_ecp_point public_keys; @@ -77,17 +89,12 @@ bool LescCrypto::generate_keys( return err ? false : true; } -bool LescCrypto::generate_shared_secret( - const ArrayView& peer_X, - const ArrayView& peer_Y, - const ArrayView& own_secret, - ArrayView shared_secret +bool CryptoToolbox::generate_shared_secret( + const ArrayView& peer_X, + const ArrayView& peer_Y, + const ArrayView& own_secret, + ArrayView shared_secret ) { - MBED_ASSERT(peer_X.size() == public_key_coord_t::size()); - MBED_ASSERT(peer_Y.size() == public_key_coord_t::size()); - MBED_ASSERT(own_secret.size() == public_key_coord_t::size()); - MBED_ASSERT(shared_secret.size() == dhkey_t::size()); - mbedtls_mpi result; mbedtls_mpi secret_key; mbedtls_ecp_point public_keys; @@ -122,18 +129,18 @@ bool LescCrypto::generate_shared_secret( } -void LescCrypto::load_mpi(mbedtls_mpi& dest, const ArrayView& src) { +void CryptoToolbox::load_mpi(mbedtls_mpi& dest, const ArrayView& src) { ble::public_key_coord_t src_be = src.data(); swap_endian(src_be.buffer(), src_be.size()); mbedtls_mpi_read_binary(&dest, src_be.data(), src_be.size()); } -void LescCrypto::store_mpi(ArrayView& dest, const mbedtls_mpi& src) { +void CryptoToolbox::store_mpi(ArrayView& dest, const mbedtls_mpi& src) { mbedtls_mpi_write_binary(&src, dest.data(), dest.size()); swap_endian(dest.data(), dest.size()); } -void LescCrypto::swap_endian(uint8_t* buf, size_t len) { +void CryptoToolbox::swap_endian(uint8_t* buf, size_t len) { for(size_t low = 0, high = (len - 1); high > low; --high, ++low) { std::swap(buf[low], buf[high]); } diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h index afbb37b898..1d1f432d4f 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h @@ -1,3 +1,19 @@ +/* mbed Microcontroller Library + * Copyright (c) 2018-2018 ARM Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #ifndef NRF5X_CRYPTO_ #define NRF5X_CRYPTO_ @@ -29,33 +45,34 @@ namespace pal { namespace vendor { namespace nordic { -class LescCrypto : mbed::NonCopyable { +class CryptoToolbox : mbed::NonCopyable { public: - LescCrypto(); + static const ptrdiff_t lesc_key_size_ = public_key_coord_t::size_; - ~LescCrypto(); + + CryptoToolbox(); + + ~CryptoToolbox(); bool generate_keys( - ArrayView X, - ArrayView Y, - ArrayView secret + ArrayView X, + ArrayView Y, + ArrayView secret ); bool generate_shared_secret( - const ArrayView& peer_X, - const ArrayView& peer_Y, - const ArrayView& own_secret, - ArrayView shared_secret + const ArrayView& peer_X, + const ArrayView& peer_Y, + const ArrayView& own_secret, + ArrayView shared_secret ); private: - void load_mpi(mbedtls_mpi& dest, const ArrayView& src); + void load_mpi(mbedtls_mpi& dest, const ArrayView& src); - void store_mpi(ArrayView& dest, const mbedtls_mpi& src); - - void swap_endian(ArrayView& to_swap); + void store_mpi(ArrayView& dest, const mbedtls_mpi& src); void swap_endian(uint8_t* buf, size_t len); diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp index e6f00eccc5..e91de1b168 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp @@ -105,9 +105,9 @@ nRF5xSecurityManager::~nRF5xSecurityManager() ble_error_t nRF5xSecurityManager::initialize() { if (_crypto.generate_keys( - make_ArrayView(X.buffer(), X.size()), - make_ArrayView(Y.buffer(), Y.size()), - make_ArrayView(secret.buffer(), secret.size()) + make_ArrayView(X), + make_ArrayView(Y), + make_ArrayView(secret) )) { return BLE_ERROR_NONE; } @@ -800,13 +800,13 @@ bool nRF5xSecurityManager::sm_handler(const ble_evt_t *evt) const ble_gap_evt_lesc_dhkey_request_t& dhkey_request = gap_evt.params.lesc_dhkey_request; - size_t key_size = public_key_coord_t::size(); + static const size_t key_size = public_key_coord_t::size_; ble_gap_lesc_dhkey_t shared_secret; _crypto.generate_shared_secret( - make_const_ArrayView(dhkey_request.p_pk_peer->pk, key_size), - make_const_ArrayView(dhkey_request.p_pk_peer->pk + key_size, key_size), - make_const_ArrayView(secret.data(), secret.size()), + make_const_ArrayView(dhkey_request.p_pk_peer->pk), + make_const_ArrayView(dhkey_request.p_pk_peer->pk + key_size), + make_const_ArrayView(secret), shared_secret.key ); diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.h b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.h index f9263998ad..31bda5bbcc 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.h +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.h @@ -337,7 +337,7 @@ private: void release_all_pairing_cb(); pairing_control_block_t* _control_blocks; - LescCrypto _crypto; + CryptoToolbox _crypto; ble::public_key_coord_t X; ble::public_key_coord_t Y; ble::public_key_coord_t secret; From 04ab07ff16de758002eafb363a0bbd3b8de119ec Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 15:41:20 +0100 Subject: [PATCH 12/14] BLE: Add documentation to CryptoToolbox. --- .../TARGET_NRF5/source/nRF5xCrypto.h | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h index 1d1f432d4f..0189e44ddc 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xCrypto.h @@ -17,50 +17,70 @@ #ifndef NRF5X_CRYPTO_ #define NRF5X_CRYPTO_ -#include +#include #if !defined(MBEDTLS_CONFIG_FILE) #include "mbedtls/config.h" #else #include MBEDTLS_CONFIG_FILE #endif -#include -#include - #include "mbedtls/platform.h" -#include "mbedtls/ecdh.h" -#include "mbedtls/memory_buffer_alloc.h" #include "mbedtls/entropy.h" #include "mbedtls/ecp.h" #include "platform/NonCopyable.h" -#include "platform/CriticalSectionLock.h" #include "ble/BLETypes.h" -#include "cmsis.h" -#include "ble/ArrayView.h" namespace ble { namespace pal { namespace vendor { namespace nordic { +/** + * Toolbox of cryptographic functions used in BLE. + */ class CryptoToolbox : mbed::NonCopyable { public: + /** + * Size of the Key used in lesc crypto operations. + */ static const ptrdiff_t lesc_key_size_ = public_key_coord_t::size_; - + /** + * Create a new CryptoToolbox. + */ CryptoToolbox(); + /** + * Destroy a CryptoTioolbox object. + */ ~CryptoToolbox(); + /** + * Generate lesc public and private keys. + * @param[out] X The component X of the public key. + * @param[out] Y The component Y of the public key. + * @param[out] secret The secret key. + * @return true if the shared secret has been successfully generated and + * false otherwise. + */ bool generate_keys( ArrayView X, ArrayView Y, ArrayView secret ); + /** + * Generate a shared secret from a peer public key and a local secret key. + * @param[in] peer_X The component X of the peer public key. + * @param[in] peer_Y The component Y of the peer public key. + * @param[in] own_secret The local secret key. + * @param[out] shared_secret The shared secret generated. + * @return true if the shared secret has been successfully generated and + * false otherwise. + */ bool generate_shared_secret( const ArrayView& peer_X, const ArrayView& peer_Y, @@ -69,7 +89,6 @@ public: ); private: - void load_mpi(mbedtls_mpi& dest, const ArrayView& src); void store_mpi(ArrayView& dest, const mbedtls_mpi& src); From 17b3e2642977fafe2a8be661f2d4d49ff9ba28ce Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 15:42:47 +0100 Subject: [PATCH 13/14] BLE: remove trace in Nordic PAL security manager --- .../TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp index e91de1b168..9aa8b61b58 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp @@ -179,7 +179,6 @@ ble_error_t nRF5xSecurityManager::send_pairing_request( KeyDistribution initiator_dist, KeyDistribution responder_dist ) { - printf("nRF5xSecurityManager::send_pairing_request\r\n"); // allocate the control block required for the procedure completion pairing_control_block_t* pairing_cb = allocate_pairing_cb(connection); if (!pairing_cb) { From dcff8104574842268573700710c1b7d48c125a08 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Wed, 4 Apr 2018 17:20:08 +0100 Subject: [PATCH 14/14] BLE: replace byte_array_t::buffer with byte_array_t::data --- features/FEATURE_BLE/ble/BLETypes.h | 8 -------- .../FEATURE_BLE/source/generic/GenericSecurityManager.cpp | 6 +++--- .../TARGET_CORDIO/source/CordioPalSecurityManager.cpp | 2 +- .../TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp | 2 +- .../TARGET_NRF5/source/nRF5xPalSecurityManager.cpp | 4 ++-- 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/features/FEATURE_BLE/ble/BLETypes.h b/features/FEATURE_BLE/ble/BLETypes.h index bbd6785bc2..a4b0c1da05 100644 --- a/features/FEATURE_BLE/ble/BLETypes.h +++ b/features/FEATURE_BLE/ble/BLETypes.h @@ -337,14 +337,6 @@ struct byte_array_t { return _value; } - - /** - * Return the pointer to the buffer holding data. - */ - uint8_t* buffer() { - return _value; - } - /** * 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 2acca2755f..50a77c870b 100644 --- a/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp +++ b/features/FEATURE_BLE/source/generic/GenericSecurityManager.cpp @@ -520,7 +520,7 @@ ble_error_t GenericSecurityManager::setOOBDataUsage( cb->oob_mitm_protection = OOBProvidesMITM; _oob_temporary_key_creator_address = cb->local_address; - get_random_data(_oob_temporary_key.buffer(), 16); + get_random_data(_oob_temporary_key.data(), 16); eventHandler->legacyPairingOobGenerated( &_oob_temporary_key_creator_address, @@ -608,7 +608,7 @@ ble_error_t GenericSecurityManager::init_signing() { if (!pcsrk) { csrk_t csrk; - ble_error_t ret = get_random_data(csrk.buffer(), csrk.size()); + ble_error_t ret = get_random_data(csrk.data(), csrk.size()); if (ret != BLE_ERROR_NONE) { return ret; } @@ -630,7 +630,7 @@ ble_error_t GenericSecurityManager::get_random_data(uint8_t *buffer, size_t size if (ret != BLE_ERROR_NONE) { return ret; } - memcpy(buffer, random_data.buffer(), copy_size); + memcpy(buffer, random_data.data(), copy_size); size -= copy_size; buffer += copy_size; } diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp index 2935fdcc41..ea07b71c48 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalSecurityManager.cpp @@ -357,7 +357,7 @@ ble_error_t CordioSecurityManager::cancel_pairing( ble_error_t CordioSecurityManager::get_random_data(byte_array_t<8> &random_data) { - SecRand(random_data.buffer(), random_data.size()); + SecRand(random_data.data(), random_data.size()); return BLE_ERROR_NOT_IMPLEMENTED; } diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp index c5a4012de2..ca875d805f 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5XCrypto.cpp @@ -131,7 +131,7 @@ bool CryptoToolbox::generate_shared_secret( void CryptoToolbox::load_mpi(mbedtls_mpi& dest, const ArrayView& src) { ble::public_key_coord_t src_be = src.data(); - swap_endian(src_be.buffer(), src_be.size()); + swap_endian(src_be.data(), src_be.size()); mbedtls_mpi_read_binary(&dest, src_be.data(), src_be.size()); } diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp index 9aa8b61b58..08acfc4bdb 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xPalSecurityManager.cpp @@ -521,7 +521,7 @@ ble_error_t nRF5xSecurityManager::set_csrk(const csrk_t& csrk) ble_error_t nRF5xSecurityManager::get_random_data(byte_array_t<8> &random_data) { uint32_t err = sd_rand_application_vector_get( - random_data.buffer(), random_data.size() + random_data.data(), random_data.size() ); return convert_sd_error(err); } @@ -636,7 +636,7 @@ ble_error_t nRF5xSecurityManager::generate_secure_connections_oob( ble_gap_lesc_p256_pk_t own_secret; ble_gap_lesc_oob_data_t oob_data; - memcpy(own_secret.pk, secret.buffer(), secret.size()); + memcpy(own_secret.pk, secret.data(), secret.size()); uint32_t err = sd_ble_gap_lesc_oob_data_get( connection,