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

Fix nordic security cancellation #7210

Merged
merged 7 commits into from Jun 25, 2018

Conversation

Projects
None yet
5 participants
@pan-
Member

pan- commented Jun 13, 2018

Description

This PR fix the way Bluetooth pairing is cancelled on Nordic targets. Depending on the state of the device, different calls must be issued to cancel Bluetooth pairing. One state (Bluetooth central) was not managed and a regression in the Nordic stack forced this patch.

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

pan- added some commits Jun 13, 2018

Nordic BLE: Improve return of nRF5xn::getGap.
Return the derived type instead of the abstract one. This is legal as C++ supports covariant returns.
Nordic BLE: Fix pairing cancellation.
Depending on the role and the current state of the local device; pairing cancelation should be made with a call to a specific function. Normally the Nordic stack would reject invalid calls if the device is not in the correct state; therefore it was assumed that it was possible to detect the state from sd errors. Unfortunatelly this is not true with the latest softdevices as some calls succeed even if the device is not in the right state.

To solve that issue cancelation looks at the current state of the device first to select the right function that will trigger the pairing cancellation.

Note: the call to sd_ble_gap_authenticate was missing in the previous algorithm
Nordic BLE:
Cancel pairing if the device fail to allocate the resources necessary for the pairing operation.

@cmonr cmonr added needs: CI and removed needs: review labels Jun 13, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 13, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jun 13, 2018

Build : SUCCESS

Build number : 2347
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7210/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@paul-szczepanek-arm

the reject command returns an error during tests

@mbed-ci

This comment has been minimized.

@cmonr cmonr added needs: review and removed needs: CI labels Jun 15, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 15, 2018

@paul-szczepanek-arm Mind elaborating on which tests these are?

@paul-szczepanek-arm

This comment has been minimized.

Member

paul-szczepanek-arm commented Jun 15, 2018

I used clitest SM_persistent_db_test_01, but the checked in version has been modified to work around this problem

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 15, 2018

the checked in version has been modified to work around this problem

@paul-szczepanek-arm Does that mean that this PR is good to go? If so, would you mind approving the PR?

@paul-szczepanek-arm

This comment has been minimized.

Member

paul-szczepanek-arm commented Jun 15, 2018

No, I mean the test has a work around since it's a pre-existing problem. This PR aims to fix it but doesn't. Yet. I used that test without the workaround to check.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 15, 2018

OH, ok. Thanks for the clarification.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 15, 2018

@paul-szczepanek-arm Would you mind posting the test results to help with tracking down the issue(s)?

@cmonr cmonr added needs: work and removed needs: review labels Jun 19, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 21, 2018

Making a note here that a fix is still incoming from @pan- before this can move forward.

@pan-

This comment has been minimized.

Member

pan- commented Jun 22, 2018

@cmonr I've pushed a fix and added a test ARMmbed/ble-tests#44 .

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 23, 2018

/morph build

@cmonr cmonr added needs: CI and removed needs: work labels Jun 23, 2018

@cmonr

cmonr approved these changes Jun 23, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Jun 23, 2018

Build : SUCCESS

Build number : 2436
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7210/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr merged commit 2e233a9 into ARMmbed:master Jun 25, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 919 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10030 cycles (+220 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment