modules: mbedtls: Expose MBEDTLS_RSA_C

[de-nordic]

Allow enabling MBEDTLS_RSA_C without key exchange enabled. This allows to use RSA without enabling x509 support too.

[valeriosetti]

This is a nice addition, but I think we can shape it slightly better than this, so I wrote my proposal in comments.

[valeriosetti]

I would prefer to have a Kconfig for both PKCS1 v1.5 and v2.1 as follows:

if MBEDTLS_RSA_C

config MBEDTLS_PKCS1_V15
    bool "RSA PKCS1 v1.5"

config MBEDTLS_PKCS1_V21
    bool "RSA PKCS1 v2.1"

endif # MBEDTLS_RSA_C

[OPTIONAL]
config MBEDTLS_RSA_PKCS1_V15
     select MBEDTLS_RSA_C
     select MBEDTLS_PKCS1_V15

[OPTIONAL]
config MBEDTLS_RSA_PKCS1_V21
     select MBEDTLS_RSA_C
     select MBEDTLS_PKCS1_V21

The last 2 items that I marked as [OPTIONAL] are meant to ease your life in the TLS key exchanges so that you can simply select MBEDTLS_RSA_PKCS1_V21 to have what you need.

[valeriosetti]

Extra nice thing that would be nice to do in this PR: Kconfig MBEDTLS_GENPRIME_ENABLED only makes sense when RSA is enabled, so it would be nice to bring the definition inside the

if MBEDTLS_RSA_C
....
endif 

block

[de-nordic]

Sure. Don't you want choice for

config MBEDTLS_PKCS1_V15
    bool "RSA PKCS1 v1.5"

config MBEDTLS_PKCS1_V21
    bool "RSA PKCS1 v2.1"

?
I guess then MBEDTLS_RSA_PKCS1_V15 and MBEDTLS_RSA_PKCS1_V21 would not be possible.

[valeriosetti]

Following the suggestion on Kconfig here you should have 1 #define for each Kconfig, i.e.

#if defined(CONFIG_MBEDTLS_RSA_C)
#define MBEDTLS_RSA_C
#endif

#if defined(CONFIG_MBEDTLS_PKCS1_V15)
#define MBEDTLS_PKCS1_V15
#endif

#if defined(CONFIG_MBEDTLS_PKCS1_V15)
#define MBEDTLS_PKCS1_V21
#endif

[valeriosetti]

If you add the Kconfig also for PKCS v1.5 as I suggested and resolve all the dependencies in Kconfig, we can get finally rid of this #if def block

[valeriosetti]

Thanks for addressing my comments. LGTM now :)

[tomi-font]

Previously MBEDTLS_PKCS1_V21 would get defined as well, but now you are making the Kconfig options select MBEDTLS_RSA_PKCS1_V15 which will result in only MBEDTLS_PKCS1_V15 always getting defined. Don't know what's the right solution but that seems like quite a change?
@valeriosetti what do you think?

[valeriosetti]

Ops, you're right, I missed that part, thanks!
Perhaps we can add another Kconfig like:

config MBEDTLS_RSA_FULL
	bool
	select MBEDTLS_RSA_C
	select MBEDTLS_PKCS1_V21
	select MBEDTLS_PKCS1_V15

which can be used for TLS key-exchanges. If so, then we can also remove Kconfigs CONFIG_MBEDTLS_RSA_PKCS1_V15 and CONFIG_MBEDTLS_RSA_PKCS1_V21 since they would not be used anywhere. But this latter part is optional of course.

[tomi-font]

Yeah I think better remove those extra helper Kconfig options if they're not used.

[tomi-font]

#87355 (comment)

[valeriosetti]

The PR looks good to me. There's only one small conflict because my PR renaming Mbed TLS' headers got merged yesterday. However the conflict should be extremely easy to fix.

[d3zd3z]

So nothing in particular wrong with this change. I don't know what the use is, but pkcs 1 1.5 should not be used for new applications or in new designs. The PSS from v2.1 should be used for new designs. But, otherwise this looks good.

Also, there is a pkcs1 v2.2 now, which as far as I can tell just adds additional signature algorithms. I'm not sure the approach mbedtls is taking on this.