diff mbox series

[2/2] certs: Guard RSA signature verification self-test

Message ID 20240416032347.72663-2-git@jvdsn.com (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show
Series [1/2] certs: Add ECDSA signature verification self-test | expand

Commit Message

Joachim Vandersmissen April 16, 2024, 3:23 a.m. UTC
Currently it is possible to configure the kernel (albeit in a very
contrived manner) such that CRYPTO_RSA is not set, yet
FIPS_SIGNATURE_SELFTEST is set. This would cause a false kernel panic
when executing the RSA PKCS#7 self-test. Guard against this by
introducing a compile-time check.

Signed-off-by: Joachim Vandersmissen <git@jvdsn.com>
---
 crypto/asymmetric_keys/selftest.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Herbert Xu April 16, 2024, 8:59 a.m. UTC | #1
On Mon, Apr 15, 2024 at 10:23:47PM -0500, Joachim Vandersmissen wrote:
> Currently it is possible to configure the kernel (albeit in a very
> contrived manner) such that CRYPTO_RSA is not set, yet
> FIPS_SIGNATURE_SELFTEST is set. This would cause a false kernel panic
> when executing the RSA PKCS#7 self-test. Guard against this by
> introducing a compile-time check.
> 
> Signed-off-by: Joachim Vandersmissen <git@jvdsn.com>

The usual way to handle this is to add a select to the Kconfig file.

Thanks,
Joachim Vandersmissen April 16, 2024, 1:39 p.m. UTC | #2
Hi Herbert,

On 4/16/24 3:59 AM, Herbert Xu wrote:
> On Mon, Apr 15, 2024 at 10:23:47PM -0500, Joachim Vandersmissen wrote:
>> Currently it is possible to configure the kernel (albeit in a very
>> contrived manner) such that CRYPTO_RSA is not set, yet
>> FIPS_SIGNATURE_SELFTEST is set. This would cause a false kernel panic
>> when executing the RSA PKCS#7 self-test. Guard against this by
>> introducing a compile-time check.
>>
>> Signed-off-by: Joachim Vandersmissen <git@jvdsn.com>
> The usual way to handle this is to add a select to the Kconfig file.

I did consider that initially, but I was unsure if this was the right 
path. From a conceptual standpoint, this module doesn't need the RSA (or 
ECDSA) functionality. If the algorithm is not present, it would be 
perfectly valid for the module to do nothing. However, I'm not opposed 
to removing the current check and adding the select to the Kconfig.

If I add a `select CRYPTO_RSA` to FIPS_SIGNATURE_SELFTEST, do you think 
I should do something similar for ECDSA as well (considering the other 
patch in this series)?

>
> Thanks,
Herbert Xu April 18, 2024, 4:01 a.m. UTC | #3
On Tue, Apr 16, 2024 at 08:39:28AM -0500, Joachim Vandersmissen wrote:
> 
> I did consider that initially, but I was unsure if this was the right path.
> From a conceptual standpoint, this module doesn't need the RSA (or ECDSA)
> functionality. If the algorithm is not present, it would be perfectly valid
> for the module to do nothing. However, I'm not opposed to removing the
> current check and adding the select to the Kconfig.
> 
> If I add a `select CRYPTO_RSA` to FIPS_SIGNATURE_SELFTEST, do you think I
> should do something similar for ECDSA as well (considering the other patch
> in this series)?

I think we should split the data out into individual files, leaving
only the test code in selftest.c.  Each individual file could then
invoke the test function directly on its data.

Cheers,
diff mbox series

Patch

diff --git a/crypto/asymmetric_keys/selftest.c b/crypto/asymmetric_keys/selftest.c
index 68620a9ab974..d2781d0b87d9 100644
--- a/crypto/asymmetric_keys/selftest.c
+++ b/crypto/asymmetric_keys/selftest.c
@@ -23,6 +23,7 @@  struct certs_test {
  * be loaded into a temporary keyring for the duration of the testing.
  */
 static const u8 certs_selftest_keys[] __initconst = {
+#if IS_ENABLED(CONFIG_CRYPTO_RSA)
 	/* 4096-bit RSA certificate */
 	"\x30\x82\x05\x55\x30\x82\x03\x3d\xa0\x03\x02\x01\x02\x02\x14\x73"
 	"\x98\xea\x98\x2d\xd0\x2e\xa8\xb1\xcf\x57\xc7\xf2\x97\xb3\xe6\x1a"
@@ -110,6 +111,7 @@  static const u8 certs_selftest_keys[] __initconst = {
 	"\xad\x5a\xf5\xb3\xdb\x69\x21\x04\xfd\xd3\x1c\xdf\x94\x9d\x56\xb0"
 	"\x0a\xd1\x95\x76\x8d\xec\x9e\xdd\x0b\x15\x97\x64\xad\xe5\xf2\x62"
 	"\x02\xfc\x9e\x5f\x56\x42\x39\x05\xb3"
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_ECDSA)
 	/* P-256 ECDSA certificate */
 	"\x30\x82\x01\xd4\x30\x82\x01\x7b\xa0\x03\x02\x01\x02\x02\x14\x2e"
@@ -228,7 +230,9 @@  static const u8 certs_selftest_ecdsa_pkcs7[] __initconst = {
  */
 #define TEST(data, pkcs7) { data, sizeof(data) - 1, pkcs7, sizeof(pkcs7) - 1 }
 static const struct certs_test certs_tests[] __initconst = {
+#if IS_ENABLED(CONFIG_CRYPTO_RSA)
 	TEST(certs_selftest_data, certs_selftest_rsa_pkcs7),
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_ECDSA)
 	TEST(certs_selftest_data, certs_selftest_ecdsa_pkcs7),
 #endif