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 2020/01/09 02:09:22 UTC

[GitHub] [mynewt-nimble] ccollins476ad opened a new pull request #726: host: Don't allow unauth pairing if MITM protection required

ccollins476ad opened a new pull request #726: host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726
 
 
   I think this PR identifies a real bug, but I am not sure if the fix conforms to the letter of the Bluetooth spec.  Hopefully someone more knowledgeable can take a look.
   
   Before this PR, the following scenario was possible:
   
   1. We send pair request with mitm=1
   2. Peer ignores our mitm flag and sends a pair response attempting just works.
   3. We accept the pair response and proceed with just works pairing.
   
   This commit changes step 3 such that we abort the pairing procedure if either side has mitm=1 but the pairing algorithm is unauthenticated.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] ccollins476ad commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

ccollins476ad commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726#issuecomment-572586514
 
 
   The use case is this:
   
   A device ships locked.  The user can only unlock the device by pairing with it.  Pairing requires a passkey that the user gets from the vendor web site after logging in and authenticating.
   
   The application sets mitm=1.  I assumed this meant that JustWorks pairing would never occur.  The application unlocks the device as soon as the connection changes its state to "encrypted".
   
   I don't see a means for the application to abort the pairing procedure when the peer indicates that it will be using an unauthenticated algorithm.  The application has to allow pairing to succeed, but then it can inspect the `authenticated` flag to determine if it should unlock the device, is this correct?  I hadn't noticed the `authenticated` member of `struct ble_gap_sec_state`! 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] rymanluk commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

rymanluk commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726#issuecomment-572476883
 
 
   @ccollins476ad I agree with @andrzej-kaczmarek . Out of curiosity, what is your use case?
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] ccollins476ad commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

ccollins476ad commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726#issuecomment-572651883
 
 
   Thanks for the clarification @andrzej-kaczmarek.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] ccollins476ad edited a comment on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

ccollins476ad edited a comment on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726#issuecomment-572586514
 
 
   The use case is this:
   
   A device ships locked.  The user can only unlock the device by pairing with it.  Pairing requires a passkey that the user gets from the vendor web site after logging in and authenticating.
   
   The application sets mitm=1.  I assumed this meant that JustWorks pairing would never occur.  The application unlocks the device as soon as the connection changes its state to "encrypted".
   
   I don't see a means for the application to abort the pairing procedure when the peer indicates that it will be using an unauthenticated algorithm.
   
   The application has to allow pairing to succeed, but then it can inspect the `authenticated` flag to determine if it should unlock the device.  Is this correct?  I didn't notice the `authenticated` member of `struct ble_gap_sec_state`! 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] andrzej-kaczmarek commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

andrzej-kaczmarek commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726#issuecomment-572475553

This is partially correct.

We can set our mitm=1 if we either mandate MITM or if we allow to downgrade to mitm=0 and proceed with unauthenticated pairing - this is up to implementation and we do the latter. However, if remote device set mitm=1 we cannot assume that it will not accept pairing without MITM.

That said, we either keep current behavior or add some setting to control our MITM policy (either separate setting or just say mitm=2 means we require MITM). I'd say current behavior is ok since in case we have unauthenticated pairing subsequent access to GATT database which requires authenticated key will be rejected or application can simply disconnect link if it does not want unauthenticated link.

The relevant test case which verifies this behavior is GAP/SEC/AUT/BV-17-C. It has 2 alternatives depending on mentioned policy of IUT. We now pass this test case because we set IXIT_iut_mandates_mitm=0. After changing the behavior we would need to also adjust this settings in PTS.

----------------------------------------------------------------
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

With regards,
Apache Git Services

[GitHub] [mynewt-nimble] andrzej-kaczmarek commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

andrzej-kaczmarek commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726#issuecomment-572612461

Correct, you can check `authenticated` flag to see if link is authenticated and unlock device. Also if at least one device set mitm=1, then pairing method is determined by IO capabilities so assuming you pair with a phone which should have `KeyboardDisplay`, setting `DisplayOnly` on your side should always trigger Passkey Entry pairing which results in an authenticated key. If remote device forced Just Works, you should be able to force MITM by initiating pairing once more, as per spec (Core 5.1, Vol 3, Part C, 10.3.2):

> If an authenticated pairing is required but only an unauthenticated pairing has occurred and the link is currently encrypted, the pairing procedure should be executed with the required authentication settings. If the pairing procedure fails, or an authenticated pairing cannot be performed with the IO capabilities of the local device and remote device, then the service request shall be aborted.

I do not think there is an issue here. IMO it's more user-friendly that Just Works is triggered if service does not require authenticated link. If it does, authenticated pairing should be triggered automatically. In your case it looks like an issue because as I understand you do not need to access any particular service and just want to trigger certain pairing method - that is not really an use-case for LE link in GAP spec since it defines behavior for either initiating local or remote service, i.e. accessing L2CAP channel or GATT service.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [mynewt-nimble] ccollins476ad edited a comment on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

ccollins476ad edited a comment on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726#issuecomment-572598704

I think the fix I described in my last comment works, so we can close this PR.

I wonder if there is a user-friendliness issue here though (I have no idea what the solution is). It might just be me, but I don't think it's obvious that the stack allows unauthenticated pairing when it is configured to use authentication. In my mind if you configure the stack to use authentication, you want to reject unauthenticated attempts (otherwise why even allow authentication?). EDIT: Actually, I can understand why a device might allow both authenticated and unauthenticated pairing. Some characteristics might require more security than others.

That alone isn't too bad, but my real concern is that it is easy to overlook such a security vulnerability. Most Bluetooth devices won't attempt JustWorks if the nimble device sets `mitm=1`, so this problem never gets detected.

Again, I don't know what the solution is. Maybe just a big disclaimer in the documentation about it being the application's responsibilty to check the authenticated state if it cares about it?

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [mynewt-nimble] ccollins476ad commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

ccollins476ad commented on issue #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726#issuecomment-572598704
 
 
   I think the fix I described in my last comment works, so we can close this PR.
   
   I wonder if there is a user-friendliness issue here though (I have no idea what the solution is).  It might just be me, but I don't think it's obvious that the stack allows unauthenticated pairing when it is configured to use authentication.  In my mind if you configure the stack to use authentication, you want to reject unauthenticated attempts (otherwise why even allow authentication?).
   
   That alone isn't too bad, but my real concern is that it is easy to overlook such a security vulnerability.  Most Bluetooth devices won't attempt JustWorks if the nimble device sets `mitm=1`, so this problem never gets detected.
   
   Again, I don't know what the solution is.  Maybe just a big disclaimer in the documentation about it being the application's responsibilty to check the authenticated state if it cares about it?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] ccollins476ad closed pull request #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required

Posted by GitBox <gi...@apache.org>.

ccollins476ad closed pull request #726: [DO NOT MERGE] host: Don't allow unauth pairing if MITM protection required
URL: https://github.com/apache/mynewt-nimble/pull/726
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services