[toup] zephyr: crypto: Fix for embedtls #20
Conversation
src/crypto/tls_mbedtls_alt.c
Outdated
| @@ -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); | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
• 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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I can try and attempt a PR to MbedTLS
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Thanks for confirming. Can you please cherry-pick the MbedTLS and update
zephyrproject-rtos/zephyr#76826
There was a problem hiding this comment.
This is akin to fromlist
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Sorry it took so long, its done now zephyrproject-rtos/zephyr#78785
LiLongNXP
force-pushed
the
main_lldbg_test3
branch
from
ec6dbe5
to
9e1412c
Compare
|
@krish2718 / @LiLongNXP is this PR waiting some action from someone? |
|
hi @krish2718 , |
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 <[email protected]>
LiLongNXP
force-pushed
the
main_lldbg_test3
branch
from
9e1412c
to
196c6f6
Compare
|
Thanks @krish2718. I have updated the 2 PRs into zephyr west.yml PR: But after the checks done, it shows "Some checks were not successful" and marked as Do Not Merge. |
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 |
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. |
|
hi @krish2718 @jukkar , The PR depend on below PR from @krish2718 , which also need be merged meanwhile. This zephyr PR update both above the 2 PRs above in west.yml: |
|
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. |
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.