You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/03/18 12:22:46 UTC
[GitHub] [mynewt-nimble] KKopyscinski opened a new pull request #941: ble_sm: add Secure Connections Only mode
KKopyscinski opened a new pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941
Added mode allowing to enforce pairing only in SC mode 1 level 4. This mode is required to pass
GAP/SEC/SEM/BI testcases. Added BLE_SM_SC_LVL config to allow pairing only in selected levels.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] KKopyscinski commented on a change in pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
KKopyscinski commented on a change in pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#discussion_r630824650
##########
File path: nimble/host/src/ble_att_svr.c
##########
@@ -284,6 +284,16 @@ ble_att_svr_check_perms(uint16_t conn_handle, int is_read,
}
ble_att_svr_get_sec_state(conn_handle, &sec_state);
+ /* In SC Only mode all characteristics requiring security
+ * require it on level 4
+ */
+ if (MYNEWT_VAL(BLE_SM_SC_ONLY)) {
+ if (sec_state.key_size != 128 ||
Review comment:
Makes sense. It wasn't caught by tests as they only checked if key shorter than needed returns the error. I submitted a fix https://github.com/apache/mynewt-nimble/pull/977
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-801920937
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-802631404
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_sm.c
<details>
```diff
@@ -1837,8 +1836,8 @@
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_AUTHREQ);
} else if (MYNEWT_VAL(BLE_SM_SC_ONLY) && (req->max_enc_key_size != BLE_SM_PAIR_KEY_SZ_MAX)) {
/* Fail if Secure Connections Only mode is on and remote does not meet
- * key size requirements - MITM was checked in last step
- */
+ * key size requirements - MITM was checked in last step
+ */
res->sm_err = BLE_SM_ERR_ENC_KEY_SZ;
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_ENC_KEY_SZ);
} else {
@@ -1906,8 +1905,8 @@
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_AUTHREQ);
} else if (MYNEWT_VAL(BLE_SM_SC_ONLY) && (rsp->max_enc_key_size != BLE_SM_PAIR_KEY_SZ_MAX)) {
/* Fail if Secure Connections Only mode is on and remote does not meet
- * key size requirements - MITM was checked in last step
- */
+ * key size requirements - MITM was checked in last step
+ */
res->sm_err = BLE_SM_ERR_ENC_KEY_SZ;
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_ENC_KEY_SZ);
} else {
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] KKopyscinski commented on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
KKopyscinski commented on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-802639229
Test results and configs used in tests
<details>
'BLE_SM_LEGACY': '0',
'BLE_SM_SC': '1',
'BLE_SM_SC_ONLY': '0',
'BLE_SM_SC_LVL': '4',
1/8 GAP GAP/SEC/SEM/BV-21-C PASS 55.185
2/8 GAP GAP/SEC/SEM/BV-22-C PASS 36.079
3/8 GAP GAP/SEC/SEM/BV-23-C PASS 46.85
4/8 GAP GAP/SEC/SEM/BV-24-C PASS 27.714
5/8 GAP GAP/SEC/SEM/BV-26-C PASS 55.983
6/8 GAP GAP/SEC/SEM/BV-27-C PASS 47.475
7/8 GAP GAP/SEC/SEM/BV-28-C PASS 51.762
8/8 GAP GAP/SEC/SEM/BV-29-C FAIL 51.885 PTS Support Request CASE0070978
'BLE_SM_LEGACY': '0',
'BLE_SM_SC': '1',
'BLE_SM_SC_ONLY': '0',
'BLE_SM_SC_LVL': '2',
1/4 GAP GAP/SEC/SEM/BV-37-C PASS 47.913
2/4 GAP GAP/SEC/SEM/BV-39-C PASS 29.547
3/4 GAP GAP/SEC/SEM/BV-41-C PASS 53.756
4/4 GAP GAP/SEC/SEM/BV-43-C PASS 38.478
'BLE_SM_LEGACY': '0',
'BLE_SM_SC': '1',
'BLE_SM_SC_ONLY': '0',
'BLE_SM_SC_LVL': '3',
1/4 GAP GAP/SEC/SEM/BV-38-C PASS 55.274
2/4 GAP GAP/SEC/SEM/BV-40-C PASS 36.164
3/4 GAP GAP/SEC/SEM/BV-42-C PASS 56.499
4/4 GAP GAP/SEC/SEM/BV-44-C PASS 46.12
'BLE_SM_LEGACY': '0',
'BLE_SM_SC': '1',
'BLE_SM_SC_ONLY': '1',
'BLE_SM_SC_LVL': '4',
1/6 GAP GAP/SEC/SEM/BI-09-C PASS 56.987
2/6 GAP GAP/SEC/SEM/BI-20-C PASS 57.499
3/6 GAP GAP/SEC/SEM/BI-21-C PASS 55.16
4/6 GAP GAP/SEC/SEM/BI-10-C PASS 39.177
5/6 GAP GAP/SEC/SEM/BI-22-C PASS 38.404
6/6 GAP GAP/SEC/SEM/BI-23-C PASS 40.198
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-801920937
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-801890438
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#discussion_r630438620
##########
File path: nimble/host/src/ble_att_svr.c
##########
@@ -284,6 +284,16 @@ ble_att_svr_check_perms(uint16_t conn_handle, int is_read,
}
ble_att_svr_get_sec_state(conn_handle, &sec_state);
+ /* In SC Only mode all characteristics requiring security
+ * require it on level 4
+ */
+ if (MYNEWT_VAL(BLE_SM_SC_ONLY)) {
+ if (sec_state.key_size != 128 ||
Review comment:
@rymanluk @KKopyscinski I think this should be `16`(Bytes) and not `128`(bits !!)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] KKopyscinski edited a comment on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
KKopyscinski edited a comment on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-802592107
Tests GAP/SEC/SEM-BI-{20-23}-C require security mode described as "Security Mode 1 Level 2/3 with LE
Secure Connections Pairing only", level depending on test. For now, Secure Connections Pairing Only mode is implemented as described in BT Core Specification, Version 5.2, Vol 3, Part C 10.2.4 Secure Connections Only mode. These tests pass with SC Only enabled with level 4, as they just check if key is not shorter than 16 bytes, by checking if pairing fails for keys 7-15 bytes long. I wonder how consistent these specs really are? Do these tests just check if MITM flag has influence on pairing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-802631404
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_sm.c
<details>
```diff
@@ -1837,8 +1836,8 @@
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_AUTHREQ);
} else if (MYNEWT_VAL(BLE_SM_SC_ONLY) && (req->max_enc_key_size != BLE_SM_PAIR_KEY_SZ_MAX)) {
/* Fail if Secure Connections Only mode is on and remote does not meet
- * key size requirements - MITM was checked in last step
- */
+ * key size requirements - MITM was checked in last step
+ */
res->sm_err = BLE_SM_ERR_ENC_KEY_SZ;
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_ENC_KEY_SZ);
} else {
@@ -1906,8 +1905,8 @@
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_AUTHREQ);
} else if (MYNEWT_VAL(BLE_SM_SC_ONLY) && (rsp->max_enc_key_size != BLE_SM_PAIR_KEY_SZ_MAX)) {
/* Fail if Secure Connections Only mode is on and remote does not meet
- * key size requirements - MITM was checked in last step
- */
+ * key size requirements - MITM was checked in last step
+ */
res->sm_err = BLE_SM_ERR_ENC_KEY_SZ;
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_ENC_KEY_SZ);
} else {
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] KKopyscinski commented on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
KKopyscinski commented on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-802592107
Tests GAP/SEC/SEM-BI-{20-23}-C require security mode described as "Security Mode 1 Level 2/3 with LE
Secure Connections Pairing only", level depending on test. For now, Secure Connections Pairing Only mode is implemented as described in BT Core Specification, Version 5.2, Vol 3, Part C 10.2.4 Secure Connections Only mode. These tests pass with SC Only enabled with level 4, as they just check if key is not shorter than 16 bytes, by checking if pairing fails for keys 7-15 bytes long. I wonder how consistent these specs really are?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-802640427
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_sm.c
<details>
```diff
@@ -1834,8 +1833,8 @@
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_INVAL);
} else if (MYNEWT_VAL(BLE_SM_SC_ONLY) && (req->max_enc_key_size != BLE_SM_PAIR_KEY_SZ_MAX)) {
/* Fail if Secure Connections Only mode is on and remote does not meet
- * key size requirements - MITM was checked in last step
- */
+ * key size requirements - MITM was checked in last step
+ */
res->sm_err = BLE_SM_ERR_ENC_KEY_SZ;
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_ENC_KEY_SZ);
} else if (!ble_sm_verify_auth_requirements(req->authreq)) {
@@ -1903,8 +1902,8 @@
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_INVAL);
} else if (MYNEWT_VAL(BLE_SM_SC_ONLY) && (rsp->max_enc_key_size != BLE_SM_PAIR_KEY_SZ_MAX)) {
/* Fail if Secure Connections Only mode is on and remote does not meet
- * key size requirements - MITM was checked in last step
- */
+ * key size requirements - MITM was checked in last step
+ */
res->sm_err = BLE_SM_ERR_ENC_KEY_SZ;
res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_ENC_KEY_SZ);
} else if (!ble_sm_verify_auth_requirements(rsp->authreq)) {
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-801883954
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] KKopyscinski commented on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
KKopyscinski commented on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-802632305
Corrected SC only keysize check, as it should return BLE_SM_ERR_ENC_KEY_SZ, not BLE_SM_ERR_AUTHREQ. It also should be checked before SC/legacy and MITM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] rymanluk merged pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
rymanluk merged pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-801883954
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on pull request #941: ble_sm: add Secure Connections Only mode
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #941:
URL: https://github.com/apache/mynewt-nimble/pull/941#issuecomment-801890438
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org