Skip to content

Fix nordic security cancellation #7210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ nRF5xGap::nRF5xGap() : Gap(),
_privacy_enabled(false),
_peripheral_privacy_configuration(default_peripheral_privacy_configuration),
_central_privacy_configuration(default_central_privacy_configuration),
_non_private_address_type(LegacyAddressType::RANDOM_STATIC)
_non_private_address_type(LegacyAddressType::RANDOM_STATIC),
_connections_role()
{
m_connectionHandle = BLE_CONN_HANDLE_INVALID;
}
Expand Down Expand Up @@ -682,6 +683,8 @@ ble_error_t nRF5xGap::reset(void)
/* Clear the internal whitelist */
whitelistAddressesSize = 0;

/* Reset existing mapping between a connection and its role */
release_all_connections_role();

return BLE_ERROR_NONE;
}
Expand Down Expand Up @@ -1195,6 +1198,8 @@ void nRF5xGap::processDisconnectionEvent(
Handle_t handle,
DisconnectionReason_t reason
) {
release_connection_role(handle);

if (_connection_event_handler) {
_connection_event_handler->on_disconnected(
handle,
Expand All @@ -1214,6 +1219,9 @@ void nRF5xGap::on_connection(Gap::Handle_t handle, const ble_gap_evt_connected_t
// set the new connection handle as the _default_ handle in gap
setConnectionHandle(handle);

// add the connection and the role of the device in the local table
allocate_connection_role(handle, static_cast<Role_t>(evt.role));

// deal with own address
LegacyAddressType_t own_addr_type;
Address_t own_address;
Expand Down Expand Up @@ -1375,5 +1383,45 @@ void nRF5xGap::on_advertising_packet(const ble_gap_evt_adv_report_t &evt) {
);
}

ble_error_t nRF5xGap::get_role(ble::connection_handle_t connection, Role_t& role) {
for (size_t i = 0; i < max_connections_count; ++i) {
connection_role_t& c = _connections_role[i];
if (c.is_allocated && c.connection == connection) {
role = c.is_peripheral ? PERIPHERAL : CENTRAL;
return BLE_ERROR_NONE;
}
}

return BLE_ERROR_INVALID_PARAM;
}

void nRF5xGap::allocate_connection_role(
ble::connection_handle_t connection,
Role_t role
) {
for (size_t i = 0; i < max_connections_count; ++i) {
connection_role_t& c = _connections_role[i];
if (c.is_allocated == false) {
c.connection = connection;
c.is_peripheral = (role == Gap::PERIPHERAL);
c.is_allocated = true;
return;
}
}
}
void nRF5xGap::release_connection_role(ble::connection_handle_t connection) {
for (size_t i = 0; i < max_connections_count; ++i) {
connection_role_t& c = _connections_role[i];
if (c.is_allocated && c.connection == connection) {
c.is_allocated = false;
return;
}
}
}

void nRF5xGap::release_all_connections_role() {
for (size_t i = 0; i < max_connections_count; ++i) {
_connections_role[i].is_allocated = false;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,26 @@ class nRF5xGap : public ::Gap, public ble::pal::ConnectionEventMonitor {
DisconnectionReason_t reason
);

/**
* Return the role of the local peripheral for a given connection.
*
* @param[in] connection The connection queried.
* @param[out] role The role of the local device in the connection.
*
* @return BLE_ERROR_NONE in case of success or an appropriate error code.
*/
ble_error_t get_role(ble::connection_handle_t connection, Role_t& role);

private:
friend void btle_handler(ble_evt_t *p_ble_evt);

void on_connection(Handle_t handle, const ble_gap_evt_connected_t& evt);
void on_advertising_packet(const ble_gap_evt_adv_report_t &evt);

void allocate_connection_role(ble::connection_handle_t, Role_t);
void release_connection_role(ble::connection_handle_t);
void release_all_connections_role();

uint16_t m_connectionHandle;

ConnectionEventMonitor::EventHandler* _connection_event_handler;
Expand All @@ -275,6 +289,23 @@ class nRF5xGap : public ::Gap, public ble::pal::ConnectionEventMonitor {
AddressType_t _non_private_address_type;
Address_t _non_private_address;

struct connection_role_t {
connection_role_t() :
connection(),
is_peripheral(false),
is_allocated(false)
{ }

ble::connection_handle_t connection;
uint8_t is_peripheral:1;
uint8_t is_allocated:1;
};

static const size_t max_connections_count =
NRF_SDH_BLE_PERIPHERAL_LINK_COUNT + NRF_SDH_BLE_CENTRAL_LINK_COUNT;

connection_role_t _connections_role[max_connections_count];

/*
* Allow instantiation from nRF5xn when required.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

#include <stdint.h>
#include "nRF5xPalSecurityManager.h"
#include "nRF5xn.h"
#include "ble/Gap.h"
#include "nRF5xGap.h"
#include "nrf_ble.h"
#include "nrf_ble_gap.h"
#include "nrf_soc.h"
Expand Down Expand Up @@ -297,6 +300,9 @@ ble_error_t nRF5xSecurityManager::send_pairing_response(
) {
pairing_control_block_t* pairing_cb = allocate_pairing_cb(connection);
if (!pairing_cb) {
// not enough memory; try to reject the pairing request instead of
// waiting for timeout.
cancel_pairing(connection, pairing_failure_t::UNSPECIFIED_REASON);
return BLE_ERROR_NO_MEM;
}
pairing_cb->role = PAIRING_RESPONDER;
Expand Down Expand Up @@ -339,22 +345,33 @@ 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 entered by
// the user.
uint32_t err = sd_ble_gap_sec_params_reply(
connection,
reason.value() | 0x80,
/* sec params */ NULL,
/* keyset */ NULL
);
uint32_t err = 0;

if (!err) {
return BLE_ERROR_NONE;
}
pairing_control_block_t* pairing_cb = get_pairing_cb(connection);

// Failed because we're in the wrong state; try to cancel pairing with
// sd_ble_gap_auth_key_reply
if (err == NRF_ERROR_INVALID_STATE) {
// If there is no control block yet then if the local device is a central
// then we must reject the security request otherwise it is a response to
// a pairing feature exchange from a central.
if (!pairing_cb) {
::Gap::Role_t current_role;
if (nRF5xn::Instance().getGap().get_role(connection, current_role) != BLE_ERROR_NONE) {
return BLE_ERROR_INVALID_PARAM;
}

if (current_role == ::Gap::PERIPHERAL) {
// response to a pairing feature request
err = sd_ble_gap_sec_params_reply(
connection,
reason.value() | 0x80,
/* sec params */ NULL,
/* keyset */ NULL
);
} else {
// response to a peripheral security request
err = sd_ble_gap_authenticate(connection, NULL);
}
} else {
// At this point this must be a response to a key
err = sd_ble_gap_auth_key_reply(
connection,
/* key type */ BLE_GAP_AUTH_KEY_TYPE_NONE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class nRF5xn : public BLEInstanceBase
* always needed in a BLE application. Therefore it is allocated
* statically.
*/
virtual Gap &getGap() {
virtual nRF5xGap &getGap() {
return gapInstance;
};

Expand Down Expand Up @@ -134,7 +134,7 @@ class nRF5xn : public BLEInstanceBase
virtual void processEvents();

public:
static nRF5xn& Instance(BLE::InstanceID_t instanceId);
static nRF5xn& Instance(BLE::InstanceID_t instanceId = BLE::DEFAULT_INSTANCE);

private:
bool initialized;
Expand Down
Loading