[toup] zephyr: crypto: Fix for embedtls #20

LiLongNXP · 2024-07-26T03:07:58Z

Fix mbedtls for WPA3 enterprise suiteb192 rsa3k connect fail.

Let default config not use MBEDTLS_SSL_PRESET_SUITEB as input mbedtls_ssl_config_defaults().
For rsa3k case has TLS_CONN_SUITEB flag and will
choose MBEDTLS_SSL_PRESET_SUITEB as input.
Then the signature algorithm will set to
ssl_tls12_preset_suiteb_sig_algs which removed rsa. Then will cause EAP Hello packet not include rsa
in sig_alg and AP will return EAP failure.
Use MBEDTLS_SSL_PRESET_DEFAULT as input.

krish2718 · 2024-07-26T08:16:05Z

src/crypto/tls_mbedtls_alt.c

@@ -1730,7 +1730,7 @@ static int tls_mbedtls_set_params(struct tls_conf *tls_conf, const struct tls_co
    int ret = mbedtls_ssl_config_defaults(
        &tls_conf->conf, tls_ctx_global.tls_conf ? MBEDTLS_SSL_IS_SERVER : MBEDTLS_SSL_IS_CLIENT,
        MBEDTLS_SSL_TRANSPORT_STREAM,
-        (tls_conf->flags & TLS_CONN_SUITEB) ? MBEDTLS_SSL_PRESET_SUITEB : MBEDTLS_SSL_PRESET_DEFAULT);
+        MBEDTLS_SSL_PRESET_DEFAULT);


By RSA-3K, I guess you are referring to https://github.com/zephyrproject-rtos/hostap/blob/main/src/crypto/tls_mbedtls_alt.c#L1679. AFAIK, RSA isn't part of Suite-B (even the 3K), at least as per https://en.wikipedia.org/wiki/NSA_Suite_B_Cryptography# and the MbedTLS behaviour seems to be correct. And if it is supported in Suite-B, then this is a MbedTLS bug, the RSA-3K should be added to Suite-B list.

Can you point to some literature about RSA-3K being part of SUITE-B?

hi @krish2718 ,

Yes, RSA isn't part of EmbedTLS Suite-B.

At first EmbedTLS Suite B profile code has RSA included.
Then it appears that this bug report triggered the RSA removal:
Mbed-TLS/mbedtls#8221.

Some literature for your reference:
• Initially as per https://datatracker.ietf.org/doc/html/rfc5430 and https://datatracker.ietf.org/doc/html/rfc6460, only ECC is supported
• DH and RSA was set as “legacy” to be discontinued by October 1st 2015
• This was revised by CNSSP No. 15 from Oct 2016:
https://www.cnss.gov/CNSS/openDoc.cfm?a=nQUTpKgoOqqkPFoNb%2BoF3w%3D%3D&b=467C780853881A895A4162494D0AA5D3DE499CE5938855DCE183B428C4F217E570B30FBF762CA332098990819F0D808A

According to https://datatracker.ietf.org/doc/html/rfc5430 :
3. Suite B Requirements
The Fact Sheet on Suite B Cryptography requires that key
establishment and authentication algorithms be based on Elliptic
Curve Cryptography, and that the encryption algorithm be AES [AES].
Suite B defines two security levels, of 128 and 192 bits.

There is no RSA in suite B. So the removal of RSA to MbedTLS is correct.

Then for WPA3 enterprise suiteb192 rsa3k:
We will use MBEDTLS_SSL_PRESET_DEFAULT to make conf->sig_algs include RSA algorithm,
instead of use MBEDTLS_SSL_PRESET_SUITEB which profile is without RSA algorithm.

Thanks.
Li Long

• This was revised by CNSSP No. 15 from Oct 2016:
https://www.cnss.gov/CNSS/openDoc.cfm?a=nQUTpKgoOqqkPFoNb%2BoF3w%3D%3D&b=467C780853881A895A4162494D0AA5D3DE499CE5938855DCE183B428C4F217E570B30FBF762CA332098990819F0D808A

Yes, even Wiki link above says In 2018, NSA replaced Suite B with the Commercial National Security Algorithm Suite (CNSA).[2].

So, technically suiteb192 rsa3k is wrong, it should be CNSA-RSA3K, I still think a proper way would be to introduce a new preset called CNSA and add RSA3K in MbedTLS https://github.com/Mbed-TLS/mbedtls/blob/195e1647b24d231521675c0aab6d4715bb579be5/library/ssl_tls.c#L5978

Mbed-TLS/mbedtls#4602 3 year old issue :) and first task

Define presets for CNSA, similar to the existing presets for NSA Suite B.

This is exactly what we need. WDYT?

I can try and attempt a PR to MbedTLS

Here's my attempt: Mbed-TLS/mbedtls#9460

hi @krish2718 ,
please review the new change to run with your attempt which add CNSA presets, it could success do STA connect with WPA3 enterprise suiteb192 RSA 3k case as in WPA3 spec 19.5.2 step 4.

Thanks for confirming. Can you please cherry-pick the MbedTLS and update
zephyrproject-rtos/zephyr#76826

This is akin to fromlist

Thanks for confirming. Can you please cherry-pick the MbedTLS and update zephyrproject-rtos/zephyr#76826

hi @krish2718 ,
Since your are the author of CNSA preset change in MbedTLS, could you please cherry-pick the change.
Then I could update zephyrproject-rtos/zephyr#76826 to include both PRs (mbedtls PR and hostap PR).
Thanks.

Sorry it took so long, its done now zephyrproject-rtos/zephyr#78785

jukkar · 2024-09-06T05:56:07Z

@krish2718 / @LiLongNXP is this PR waiting some action from someone?

LiLongNXP · 2024-09-06T08:24:31Z

hi @krish2718 ,
Have you done cherry-pick the MbedTLS CNSA preset change? Could you please paste the PR here.
Then I could update zephyrproject-rtos/zephyr#76826 to include both PRs.
Thanks.

krish2718 · 2024-09-06T10:38:17Z

hi @krish2718 , Have you done cherry-pick the MbedTLS CNSA preset change? Could you please paste the PR here. Then I could update zephyrproject-rtos/zephyr#76826 to include both PRs. Thanks.

Sorry, will do it over this weekend.

Fix mbedtls for WPA3 enterprise suiteb192 rsa3k connect fail. Use MBEDTLS_SSL_PRESET_DEFAULT as input. Signed-off-by: Li Long <li.long@nxp.com>

krish2718 · 2024-09-27T08:11:03Z

hi @krish2718 , Have you done cherry-pick the MbedTLS CNSA preset change? Could you please paste the PR here. Then I could update zephyrproject-rtos/zephyr#76826 to include both PRs. Thanks.

Sorry, will do it over this weekend.

FYI zephyrproject-rtos/zephyr#78785

LiLongNXP · 2024-10-09T06:34:20Z

hi @krish2718 , Have you done cherry-pick the MbedTLS CNSA preset change? Could you please paste the PR here. Then I could update zephyrproject-rtos/zephyr#76826 to include both PRs. Thanks.

Sorry, will do it over this weekend.

FYI zephyrproject-rtos/zephyr#78785

Thanks @krish2718.

I have updated the 2 PRs into zephyr west.yml PR:
zephyrproject-rtos/zephyr#76826

But after the checks done, it shows "Some checks were not successful" and marked as Do Not Merge.
I did not find any fail reason in it, do you know why it fails?

krish2718 · 2024-10-09T07:43:46Z

But after the checks done, it shows "Some checks were not successful" and marked as Do Not Merge.
I did not find any fail reason in it, do you know why it fails

It didn't fail, as there are updates to manifest with PR references it was labelled as DNM till PR references are replaced with proper SHAs

LiLongNXP · 2024-10-09T08:01:57Z

re are updates to manifest with PR references it was labelled as DNM til

Thanks @krish2718, then can this PR be approved and merged later after that?

krish2718 · 2024-10-09T08:48:13Z

re are updates to manifest with PR references it was labelled as DNM til

Thanks @krish2718, then can this PR be approved and merged later after that?

Yes, we need to merge PRs of hostap in an order, so, as CI is green this will be merged too.

LiLongNXP · 2024-10-23T09:23:33Z

hi @krish2718 @jukkar ,
Could you please merge this PR if it reviewed done.

The PR depend on below PR from @krish2718 , which also need be merged meanwhile.
zephyrproject-rtos/mbedtls#62

This zephyr PR update both above the 2 PRs above in west.yml:
zephyrproject-rtos/zephyr#76826

krish2718 · 2024-10-23T09:25:53Z

My PR needs more work as I got more comments on my upstream PR, I don't think it would be merged before RC1, but I will try and address those comments ASAP.

jukkar · 2024-11-18T09:09:33Z

What is the status of this PR, is this still a valid change?

krish2718 · 2024-11-18T09:22:28Z

I guess this depends on zephyrproject-rtos/zephyr#78785 which I need to work, will try and get that merged soon.

LiLongNXP · 2024-12-03T03:02:40Z

I guess this depends on zephyrproject-rtos/zephyr#78785 which I need to work, will try and get that merged soon.

hi @krish2718 , this PR is pending long time and depend on below PR pull 62 in mbedtls, could you please merge below mbedtls pull 62 PR and then this pull 20 PR could be merged.
PR for mbedtls from @krish2718 : Pull support for CNSA suite
zephyrproject-rtos/mbedtls#62
zephyrproject-rtos/zephyr#78785 (update west.yml for pull 62)

Below PR update west.yml for both above mbedtls PR and this PR in west.yml:
zephyrproject-rtos/zephyr#76826 (update west.yml for pull 62 and pull 20, for pull 20 rely on pull 62)

Thanks
Li Long

krish2718 · 2024-12-03T07:12:57Z

Apologies, my CNSA PR has got a few comments upstream and I need to resolve them, will try and do it ASAP.

LiLongNXP · 2024-12-03T07:21:51Z

#20

Apologies, my CNSA PR has got a few comments upstream and I need to resolve them, will try and do it ASAP.

Thanks :) @krish2718.

krish2718 · 2024-12-03T19:30:13Z

I finally looked at the review comments, fixed some and responded to some, I still need time to address a few (adding tests), so, you can go with the original WAR without depending on my patch if this is urgent/blocking. We can fix it later.

krish2718 · 2024-12-17T19:53:06Z

Closing, will revisit once MbedTLS upstream PR is merged.

LiLongNXP requested review from jukkar, krish2718 and rlubos as code owners July 26, 2024 03:07

krish2718 reviewed Jul 26, 2024

View reviewed changes

zephyrbot mentioned this pull request Aug 8, 2024

west: hostap: update hostap revision to PR zephyrproject-rtos/zephyr#76826

Open

LiLongNXP force-pushed the main_lldbg_test3 branch from ec6dbe5 to 9e1412c Compare August 21, 2024 10:30

[toup] zephyr: crypto: Fix for embedtls

196c6f6

Fix mbedtls for WPA3 enterprise suiteb192 rsa3k connect fail. Use MBEDTLS_SSL_PRESET_DEFAULT as input. Signed-off-by: Li Long <li.long@nxp.com>

LiLongNXP force-pushed the main_lldbg_test3 branch from 9e1412c to 196c6f6 Compare September 26, 2024 06:01

jukkar requested a review from krish2718 September 26, 2024 06:39

LiLongNXP mentioned this pull request Oct 23, 2024

modules: hostap: add tls_cipher param zephyrproject-rtos/zephyr#80214

Merged

LiLongNXP mentioned this pull request Dec 9, 2024

[toup] mbedtls: Fix for WPA3 enterprise suiteb192 rsa3k #67

Merged

krish2718 closed this Dec 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[toup] zephyr: crypto: Fix for embedtls #20

[toup] zephyr: crypto: Fix for embedtls #20

LiLongNXP commented Jul 26, 2024

krish2718 Jul 26, 2024 •

edited

Loading

LiLongNXP Aug 8, 2024

krish2718 Aug 8, 2024

krish2718 Aug 8, 2024

krish2718 Aug 8, 2024

LiLongNXP Aug 21, 2024

krish2718 Aug 21, 2024

krish2718 Aug 21, 2024

LiLongNXP Aug 23, 2024

krish2718 Sep 20, 2024

jukkar commented Sep 6, 2024

LiLongNXP commented Sep 6, 2024

krish2718 commented Sep 6, 2024

krish2718 commented Sep 27, 2024

LiLongNXP commented Oct 9, 2024

krish2718 commented Oct 9, 2024

LiLongNXP commented Oct 9, 2024

krish2718 commented Oct 9, 2024

LiLongNXP commented Oct 23, 2024

krish2718 commented Oct 23, 2024

jukkar commented Nov 18, 2024

krish2718 commented Nov 18, 2024

LiLongNXP commented Dec 3, 2024

krish2718 commented Dec 3, 2024

LiLongNXP commented Dec 3, 2024

krish2718 commented Dec 3, 2024

krish2718 commented Dec 17, 2024

[toup] zephyr: crypto: Fix for embedtls #20

[toup] zephyr: crypto: Fix for embedtls #20

Conversation

LiLongNXP commented Jul 26, 2024

krish2718 Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jukkar commented Sep 6, 2024

LiLongNXP commented Sep 6, 2024

krish2718 commented Sep 6, 2024

krish2718 commented Sep 27, 2024

LiLongNXP commented Oct 9, 2024

krish2718 commented Oct 9, 2024

LiLongNXP commented Oct 9, 2024

krish2718 commented Oct 9, 2024

LiLongNXP commented Oct 23, 2024

krish2718 commented Oct 23, 2024

jukkar commented Nov 18, 2024

krish2718 commented Nov 18, 2024

LiLongNXP commented Dec 3, 2024

krish2718 commented Dec 3, 2024

LiLongNXP commented Dec 3, 2024

krish2718 commented Dec 3, 2024

krish2718 commented Dec 17, 2024

krish2718 Jul 26, 2024 •

edited

Loading