Bluetooth SMP LE SC should defer sending confirm after peer's Public Key is validated

[sjanc]

Describe the bug Currently SMP Confirm is send regardless if peer's public key is valid or not, as PK validation is done asynchronously when DHKeyCheck is computed. This results in SMP qualification failures for new TCRL 2024-2 tests which validate this behavior.

To Reproduce Execute SM/CEN/KDU/BI-04-C qualification test case with AutoPTS. SM/PER/KDU/BI-04-C is probably also affected but mentioned issue is not triggered by current setup (fails due to other issue).

Expected behavior Test pass and IUT is not sending own confirm value if lower tester is using illegal Public Key.

Impact Bluetooth qualification for Core 6.0 is affected.

Additional context I looked at current SMP implementation and see two possible solution for this problem: 1) Add set of extra states and flags to delay confirm until PK is validated, this is non-trivial as there is also user interaction involved and we already have set of flags that handle delayed DHKeyCheck. Not sure if we should add more complexity into this.

2) Provide synchronous API in ecc.h that would allow for PK validation directly in PK rx handler (this would probably also fix SM/PER/KDU/BI-04-C as we would not hit test spec issue if rejected early). This would be similar to how debug key is checked.

I'm more in favor of 2) but I'm not sure on how feasible it is eg. is PSA is used.

[jhedberg]

What context (thread) would we be blocking in 2)?

[Thalley]

What context (thread) would we be blocking in 2)?

I would expect the BT RX thread, as we are responding to an incoming SMP PDU.

@sjanc Is this also an issue as the central, or just as the peripheral?

[Thalley]

Do we even have an API to validate the remote key?

Today it looks like we are calling smp_dhkey_generate and then on the callback from that, we check if there is an error. How performance intensive is it to do a validation? If it's a fairly trivial thing to do (once implemented), then I don't see any issues with option 2).

[sjanc]

With current autopts setup I hit this only in central here (passkey entry is being used), but I think all methods where confirm is send before DHKeyCheck is suspect for this (maybe timing dependent)

[sjanc]

yes, this API could simply be wrap for uECC_valid_public_key() (I think)

[Thalley]

yes, this API could simply be wrap for uECC_valid_public_key() (I think)

Not familiar with PSA, but if we use something like that we need to ensure that PSA also has a similar function if Zephyr is switching to using that.

[sjanc]

or we can always use uECC_valid_public_key() for validation maybe? regardless if later on PSA or TinyCrypt is being used

[Thalley]

or we can always use uECC_valid_public_key() for validation maybe? regardless if later on PSA or TinyCrypt is being used

But isn't that a TinyCrypt function?

It's defined in a header with the title /* ecc.h - TinyCrypt interface to common ECC functions */.

Or can we consider that API generic?

[sjanc]

it is not generic, if you look at emulate_le_generate_dhkey() you'll see that PSA implmentation is different (using PSA API which doesn't provide explicit validation function)

[Thalley]

it is not generic, if you look at emulate_le_generate_dhkey() you'll see that PSA implmentation is different (using PSA API which doesn't provide explicit validation function)

Yeah, that was what I was thinking.

It should nevertheless be possible to implement a synchronous function to validate a remote public key. IIRC the BT Core spec has a few requirements to implement and verify, so possibly we can even do this without the use of any crypto library.