Nordic BLE: Backport security fixes from nRF5 to nRF5x

pull/6932/head
Vincent Coubard 2018-05-15 15:11:33 +01:00
parent ca5a9f359a
commit 2cb6e659a9
5 changed files with 136 additions and 120 deletions

View File

@ -31,6 +31,32 @@ typedef struct {
static unsigned uuidTableEntries = 0; /* current usage of the table */
converted_uuid_table_entry_t convertedUUIDTable[UUID_TABLE_MAX_ENTRIES];
namespace {
static void set_perm(ble_gap_conn_sec_mode_t& dest, GattAttribute::Security_t src) {
switch (src.value()) {
case GattAttribute::Security_t::NONE:
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&dest);
break;
case GattAttribute::Security_t::UNAUTHENTICATED:
BLE_GAP_CONN_SEC_MODE_SET_ENC_NO_MITM(&dest);
break;
case GattAttribute::Security_t::AUTHENTICATED:
BLE_GAP_CONN_SEC_MODE_SET_ENC_WITH_MITM(&dest);
break;
case GattAttribute::Security_t::SC_AUTHENTICATED:
BLE_GAP_CONN_SEC_MODE_SET_LESC_ENC_WITH_MITM(&dest);
break;
default:
break;
}
}
}
void custom_reset_128bits_uuid_table() {
uuidTableEntries = 0;
}
@ -204,7 +230,9 @@ error_t custom_decode_uuid_base(uint8_t const *const p_uuid_base,
error_t custom_add_in_characteristic(uint16_t service_handle,
ble_uuid_t *p_uuid,
uint8_t properties,
SecurityManager::SecurityMode_t requiredSecurity,
GattAttribute::Security_t read_security,
GattAttribute::Security_t write_security,
GattAttribute::Security_t update_security,
uint8_t *p_data,
uint16_t length,
uint16_t max_length,
@ -227,8 +255,8 @@ error_t custom_add_in_characteristic(uint16_t service_handle,
/* Notification requires cccd */
memclr_( &cccd_md, sizeof(ble_gatts_attr_md_t));
cccd_md.vloc = BLE_GATTS_VLOC_STACK;
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&cccd_md.read_perm);
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&cccd_md.write_perm);
set_perm(cccd_md.read_perm, GattAttribute::Security_t::NONE);
set_perm(cccd_md.write_perm, update_security);
}
ble_gatts_char_md_t char_md = {0};
@ -257,49 +285,8 @@ error_t custom_add_in_characteristic(uint16_t service_handle,
/* Always set variable size */
attr_md.vlen = has_variable_len;
if (char_props.read || char_props.notify || char_props.indicate) {
switch (requiredSecurity) {
case SecurityManager::SECURITY_MODE_ENCRYPTION_OPEN_LINK :
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&attr_md.read_perm);
break;
case SecurityManager::SECURITY_MODE_ENCRYPTION_NO_MITM :
BLE_GAP_CONN_SEC_MODE_SET_ENC_NO_MITM(&attr_md.read_perm);
break;
case SecurityManager::SECURITY_MODE_ENCRYPTION_WITH_MITM :
BLE_GAP_CONN_SEC_MODE_SET_ENC_WITH_MITM(&attr_md.read_perm);
break;
case SecurityManager::SECURITY_MODE_SIGNED_NO_MITM :
BLE_GAP_CONN_SEC_MODE_SET_SIGNED_NO_MITM(&attr_md.read_perm);
break;
case SecurityManager::SECURITY_MODE_SIGNED_WITH_MITM :
BLE_GAP_CONN_SEC_MODE_SET_SIGNED_WITH_MITM(&attr_md.read_perm);
break;
default:
break;
};
}
if (char_props.write || char_props.write_wo_resp) {
switch (requiredSecurity) {
case SecurityManager::SECURITY_MODE_ENCRYPTION_OPEN_LINK :
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&attr_md.write_perm);
break;
case SecurityManager::SECURITY_MODE_ENCRYPTION_NO_MITM :
BLE_GAP_CONN_SEC_MODE_SET_ENC_NO_MITM(&attr_md.write_perm);
break;
case SecurityManager::SECURITY_MODE_ENCRYPTION_WITH_MITM :
BLE_GAP_CONN_SEC_MODE_SET_ENC_WITH_MITM(&attr_md.write_perm);
break;
case SecurityManager::SECURITY_MODE_SIGNED_NO_MITM :
BLE_GAP_CONN_SEC_MODE_SET_SIGNED_NO_MITM(&attr_md.write_perm);
break;
case SecurityManager::SECURITY_MODE_SIGNED_WITH_MITM :
BLE_GAP_CONN_SEC_MODE_SET_SIGNED_WITH_MITM(&attr_md.write_perm);
break;
default:
break;
};
}
set_perm(attr_md.read_perm, read_security);
set_perm(attr_md.write_perm, write_security);
ble_gatts_attr_t attr_char_value = {0};
@ -343,7 +330,9 @@ error_t custom_add_in_descriptor(uint16_t char_handle,
uint16_t length,
uint16_t max_length,
bool has_variable_len,
uint16_t *p_desc_handle)
uint16_t *p_desc_handle,
GattAttribute::Security_t read_security,
GattAttribute::Security_t write_security)
{
/* Descriptor metadata */
ble_gatts_attr_md_t desc_md = {0};
@ -353,8 +342,8 @@ error_t custom_add_in_descriptor(uint16_t char_handle,
desc_md.vlen = has_variable_len;
/* Make it readable and writable */
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&desc_md.read_perm);
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&desc_md.write_perm);
set_perm(desc_md.read_perm, read_security);
set_perm(desc_md.write_perm, write_security);
ble_gatts_attr_t attr_desc = {0};

View File

@ -42,7 +42,9 @@ ble_uuid_t custom_convert_to_nordic_uuid(const UUID &uuid);
error_t custom_add_in_characteristic(uint16_t service_handle,
ble_uuid_t *p_uuid,
uint8_t properties,
SecurityManager::SecurityMode_t requiredSecurity,
GattAttribute::Security_t read_security,
GattAttribute::Security_t write_security,
GattAttribute::Security_t update_security,
uint8_t *p_data,
uint16_t length,
uint16_t max_length,
@ -61,7 +63,9 @@ error_t custom_add_in_descriptor(uint16_t char_handle,
uint16_t length,
uint16_t max_length,
bool has_variable_len,
uint16_t *p_desc_handle);
uint16_t *p_desc_handle,
GattAttribute::Security_t read_security,
GattAttribute::Security_t write_security);
#ifdef __cplusplus
}

View File

@ -29,38 +29,38 @@
namespace {
static const ble_gatts_rw_authorize_reply_params_t write_auth_queue_full_reply = {
.type = BLE_GATTS_AUTHORIZE_TYPE_WRITE,
.params = {
.write = {
.gatt_status = BLE_GATT_STATUS_ATTERR_PREPARE_QUEUE_FULL
/* .type = */ BLE_GATTS_AUTHORIZE_TYPE_WRITE,
/* .params = */ {
/* .write = */ {
/* .gatt_status = */ BLE_GATT_STATUS_ATTERR_PREPARE_QUEUE_FULL
}
}
};
static const ble_gatts_rw_authorize_reply_params_t write_auth_invalid_offset_reply = {
.type = BLE_GATTS_AUTHORIZE_TYPE_WRITE,
.params = {
.write = {
.gatt_status = BLE_GATT_STATUS_ATTERR_INVALID_OFFSET
/* .type = */ BLE_GATTS_AUTHORIZE_TYPE_WRITE,
/* .params = */ {
/* .write = */ {
/* .gatt_status = */ BLE_GATT_STATUS_ATTERR_INVALID_OFFSET
}
}
};
static const ble_gatts_rw_authorize_reply_params_t write_auth_succes_reply = {
.type = BLE_GATTS_AUTHORIZE_TYPE_WRITE,
.params = {
.write = {
.gatt_status = BLE_GATT_STATUS_SUCCESS,
.update = 0
/* .type = */ BLE_GATTS_AUTHORIZE_TYPE_WRITE,
/* .params = */ {
/* .write = */ {
/* .gatt_status = */ BLE_GATT_STATUS_SUCCESS,
/* .update = */ 0
}
}
};
static const ble_gatts_rw_authorize_reply_params_t write_auth_invalid_reply = {
.type = BLE_GATTS_AUTHORIZE_TYPE_WRITE,
.params = {
.write = {
.gatt_status = BLE_GATT_STATUS_ATTERR_INVALID
/* .type = */ BLE_GATTS_AUTHORIZE_TYPE_WRITE,
/* .params = */ {
/* .write = */ {
/* .gatt_status = */ BLE_GATT_STATUS_ATTERR_INVALID
}
}
};
@ -164,10 +164,13 @@ ble_error_t nRF5xGattServer::addService(GattService &service)
}
ASSERT_TRUE ( ERROR_NONE ==
custom_add_in_characteristic(BLE_GATT_HANDLE_INVALID,
custom_add_in_characteristic(
BLE_GATT_HANDLE_INVALID,
&nordicUUID,
p_char->getProperties(),
p_char->getRequiredSecurity(),
p_char->getReadSecurityRequirement(),
p_char->getWriteSecurityRequirement(),
p_char->getUpdateSecurityRequirement(),
p_char->getValueAttribute().getValuePtr(),
p_char->getValueAttribute().getLength(),
p_char->getValueAttribute().getMaxLength(),
@ -178,7 +181,8 @@ ble_error_t nRF5xGattServer::addService(GattService &service)
presentationFormatDescriptorValueLen,
p_char->isReadAuthorizationEnabled(),
p_char->isWriteAuthorizationEnabled(),
&nrfCharacteristicHandles[characteristicCount]),
&nrfCharacteristicHandles[characteristicCount]
),
BLE_ERROR_PARAM_OUT_OF_RANGE );
/* Update the characteristic handle */
@ -218,7 +222,9 @@ ble_error_t nRF5xGattServer::addService(GattService &service)
p_desc->getLength(),
p_desc->getMaxLength(),
p_desc->hasVariableLength(),
&nrfDescriptorHandles[descriptorCount]),
&nrfDescriptorHandles[descriptorCount],
p_desc->getReadSecurityRequirement(),
p_desc->getWriteSecurityRequirement()),
BLE_ERROR_PARAM_OUT_OF_RANGE);
p_descriptors[descriptorCount] = p_desc;
@ -260,9 +266,9 @@ ble_error_t nRF5xGattServer::read(GattAttribute::Handle_t attributeHandle, uint8
ble_error_t nRF5xGattServer::read(Gap::Handle_t connectionHandle, GattAttribute::Handle_t attributeHandle, uint8_t buffer[], uint16_t *lengthP)
{
ble_gatts_value_t value = {
.len = *lengthP,
.offset = 0,
.p_value = buffer,
/* .len = */ *lengthP,
/* .offset = */ 0,
/* .p_value = */ buffer,
};
ASSERT_TRUE( ERROR_NONE ==
@ -302,9 +308,9 @@ ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute
ble_error_t returnValue = BLE_ERROR_NONE;
ble_gatts_value_t value = {
.len = len,
.offset = 0,
.p_value = const_cast<uint8_t *>(buffer),
/* .len = */ len,
/* .offset = */ 0,
/* .p_value = */ const_cast<uint8_t *>(buffer),
};
if (localOnly) {
@ -345,7 +351,16 @@ ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute
}
}
if (updatesEnabled) {
bool updates_permitted = false;
ble_gap_conn_sec_t connection_security;
uint32_t err = sd_ble_gap_conn_sec_get(connectionHandle, &connection_security);
if (!err &&
(connection_security.sec_mode.sm == 1) &&
(connection_security.sec_mode.lv >= p_characteristics[characteristicIndex]->getUpdateSecurityRequirement().value())) {
updates_permitted = true;
}
if (updatesEnabled && updates_permitted) {
error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params);
if (error != ERROR_NONE) {
switch (error) {
@ -618,14 +633,14 @@ void nRF5xGattServer::hwCallback(const ble_evt_t *p_ble_evt)
// success, signal it to the softdevice
ble_gatts_rw_authorize_reply_params_t reply = {
.type = BLE_GATTS_AUTHORIZE_TYPE_WRITE,
.params = {
.write = {
.gatt_status = BLE_GATT_STATUS_SUCCESS,
.update = 1,
.offset = input_req.offset,
.len = input_req.len,
.p_data = input_req.data
/* .type = */ BLE_GATTS_AUTHORIZE_TYPE_WRITE,
/* .params = */ {
/* .write = */ {
/* .gatt_status = */ BLE_GATT_STATUS_SUCCESS,
/* .update = */ 1,
/* .offset = */ input_req.offset,
/* .len = */ input_req.len,
/* .p_data = */ input_req.data
}
}
};
@ -646,12 +661,12 @@ void nRF5xGattServer::hwCallback(const ble_evt_t *p_ble_evt)
}
GattWriteAuthCallbackParams cbParams = {
.connHandle = conn_handle,
.handle = req->attr_handle,
.offset = req->offset,
.len = req->length,
.data = req->data,
.authorizationReply = AUTH_CALLBACK_REPLY_SUCCESS /* the callback handler must leave this member
/* .connHandle = */ conn_handle,
/* .handle = */ req->attr_handle,
/* .offset = */ req->offset,
/* .len = */ req->length,
/* .data = */ req->data,
/* .authorizationReply = */ AUTH_CALLBACK_REPLY_SUCCESS /* the callback handler must leave this member
* set to AUTH_CALLBACK_REPLY_SUCCESS if the client
* request is to proceed. */
};
@ -668,9 +683,9 @@ void nRF5xGattServer::hwCallback(const ble_evt_t *p_ble_evt)
// FIXME can't use ::write here, this function doesn't take the offset into account ...
ble_gatts_value_t value = {
.len = req->length,
.offset = req->offset,
.p_value = req->data
/* .len = */ req->length,
/* .offset = */ req->offset,
/* .p_value = */ req->data
};
uint32_t update_err = sd_ble_gatts_value_set(conn_handle, req->attr_handle, &value);
if (update_err) {
@ -695,25 +710,25 @@ void nRF5xGattServer::hwCallback(const ble_evt_t *p_ble_evt)
}
GattWriteAuthCallbackParams cbParams = {
.connHandle = gattsEventP->conn_handle,
.handle = handle_value,
.offset = gattsEventP->params.authorize_request.request.write.offset,
.len = gattsEventP->params.authorize_request.request.write.len,
.data = gattsEventP->params.authorize_request.request.write.data,
.authorizationReply = AUTH_CALLBACK_REPLY_SUCCESS /* the callback handler must leave this member
/* .connHandle = */ gattsEventP->conn_handle,
/* .handle = */ handle_value,
/* .offset = */ gattsEventP->params.authorize_request.request.write.offset,
/* .len = */ gattsEventP->params.authorize_request.request.write.len,
/* .data = */ gattsEventP->params.authorize_request.request.write.data,
/* .authorizationReply = */ AUTH_CALLBACK_REPLY_SUCCESS /* the callback handler must leave this member
* set to AUTH_CALLBACK_REPLY_SUCCESS if the client
* request is to proceed. */
};
ble_gatts_rw_authorize_reply_params_t reply = {
.type = BLE_GATTS_AUTHORIZE_TYPE_WRITE,
.params = {
.write = {
.gatt_status = p_characteristics[characteristicIndex]->authorizeWrite(&cbParams),
.update = 1,
.offset = cbParams.offset,
.len = cbParams.len,
.p_data = cbParams.data
/* .type = */ BLE_GATTS_AUTHORIZE_TYPE_WRITE,
/* .params = */ {
/* .write = */ {
/* .gatt_status = */ p_characteristics[characteristicIndex]->authorizeWrite(&cbParams),
/* .update = */ 1,
/* .offset = */ cbParams.offset,
/* .len = */ cbParams.len,
/* .p_data = */ cbParams.data
}
}
};
@ -747,21 +762,21 @@ void nRF5xGattServer::hwCallback(const ble_evt_t *p_ble_evt)
}
case GattServerEvents::GATT_EVENT_READ_AUTHORIZATION_REQ: {
GattReadAuthCallbackParams cbParams = {
.connHandle = gattsEventP->conn_handle,
.handle = handle_value,
.offset = gattsEventP->params.authorize_request.request.read.offset,
.len = 0,
.data = NULL,
.authorizationReply = AUTH_CALLBACK_REPLY_SUCCESS /* the callback handler must leave this member
/* .connHandle = */ gattsEventP->conn_handle,
/* .handle = */ handle_value,
/* .offset = */ gattsEventP->params.authorize_request.request.read.offset,
/* .len = */ 0,
/* .data = */ NULL,
/* .authorizationReply = */ AUTH_CALLBACK_REPLY_SUCCESS /* the callback handler must leave this member
* set to AUTH_CALLBACK_REPLY_SUCCESS if the client
* request is to proceed. */
};
ble_gatts_rw_authorize_reply_params_t reply = {
.type = BLE_GATTS_AUTHORIZE_TYPE_READ,
.params = {
.read = {
.gatt_status = p_characteristics[characteristicIndex]->authorizeRead(&cbParams)
/* .type = */ BLE_GATTS_AUTHORIZE_TYPE_READ,
/* .params = */ {
/* .read = */ {
/* .gatt_status = */ p_characteristics[characteristicIndex]->authorizeRead(&cbParams)
}
}
};

View File

@ -616,6 +616,10 @@ ble_error_t nRF5xSecurityManager::set_peer_csrk(
return BLE_ERROR_NOT_IMPLEMENTED;
}
ble_error_t nRF5xSecurityManager::remove_peer_csrk(connection_handle_t connection)
{
return BLE_ERROR_NOT_IMPLEMENTED;
}
////////////////////////////////////////////////////////////////////////////
// Authentication
//

View File

@ -284,6 +284,10 @@ public:
sign_count_t sign_counter
);
/**
* @see ::ble::pal::SecurityManager::remove_peer_csrk
*/
virtual ble_error_t remove_peer_csrk(connection_handle_t connection);
////////////////////////////////////////////////////////////////////////////
// Authentication