Skip to contentSkip to conversation

certs: Move RSA self-test data to separate file

PATCH In review Joachim Vandersmissen 2 participants 6 messages May 13, 2024 – May 14, 2024

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v5 1/2 certs: Move RSA self-test data to separate file

In preparation of adding new ECDSA self-tests, the existing data for the RSA self-tests is moved to a separate file. This file is only compiled if the new CONFIG_FIPS_SIGNATURE_SELFTEST_RSA configuration option is set, which ensures that the required dependencies (RSA, SHA-256) are present. Otherwise, the kernel would panic when trying to execute the self-test.
The introduction of this new option, rather than adding the dependencies to the existing CONFIG_FIPS_SIGNATURE_SELFTEST option, allows for additional self-tests to be added for different algorithms. The kernel can then be configured to only execute the self-tests for those algorithms that are included.

---
v5: update commit message to be more descriptive and move the changelog to the right place.

v4: FIPS_SIGNATURE_SELFTEST_RSA is no longer user-configurable and will be set when the dependencies are fulfilled.

v3: adding the FIPS_SIGNATURE_SELFTEST_RSA configuration option to enforce that RSA (and SHA-256) are either built-in; or RSA (and SHA-256) and x509_selftest are modules. This will prevent a misconfiguration where RSA (or SHA-256) are modules but x509_selftest is built-in (and thus panics on boot).

v2: move the RSA self-test to a separate file as suggested by Herbert.

---
 crypto/asymmetric_keys/Kconfig        |   7 +
 crypto/asymmetric_keys/Makefile       |   1 +
 crypto/asymmetric_keys/selftest.c     | 218 ++++----------------------
 crypto/asymmetric_keys/selftest.h     |  16 ++
 crypto/asymmetric_keys/selftest_rsa.c | 172 ++++++++++++++++++++
 5 files changed, 225 insertions(+), 189 deletions(-)
 create mode 100644 crypto/asymmetric_keys/selftest.h
 create mode 100644 crypto/asymmetric_keys/selftest_rsa.c
crypto/asymmetric_keys/Kconfig +7 -0
index 59ec726b7c77..33bbfd0d8367 100644
@@ -86,4 +86,11 @@ config FIPS_SIGNATURE_SELFTEST
 	depends on PKCS7_MESSAGE_PARSER=X509_CERTIFICATE_PARSER
 	depends on X509_CERTIFICATE_PARSER
 
+config FIPS_SIGNATURE_SELFTEST_RSA
+	bool
+	default y
+	depends on FIPS_SIGNATURE_SELFTEST
+	depends on CRYPTO_SHA256=y || CRYPTO_SHA256=FIPS_SIGNATURE_SELFTEST
+	depends on CRYPTO_RSA=y || CRYPTO_RSA=FIPS_SIGNATURE_SELFTEST
+
 endif # ASYMMETRIC_KEY_TYPE
crypto/asymmetric_keys/Makefile +1 -0
index 1a273d6df3eb..ac1402e27324 100644
@@ -24,6 +24,7 @@ x509_key_parser-y := \
 	x509_public_key.o
 obj-$(CONFIG_FIPS_SIGNATURE_SELFTEST) += x509_selftest.o
 x509_selftest-y += selftest.o
+x509_selftest-$(CONFIG_FIPS_SIGNATURE_SELFTEST_RSA) += selftest_rsa.o
 
 $(obj)/x509_cert_parser.o: \
 	$(obj)/x509.asn1.h \
crypto/asymmetric_keys/selftest.c +29 -189

251 lines not on this page · raw message

crypto/asymmetric_keys/selftest.h new file +16 -0
new file mode 100644
index 000000000000..842ac3cf86b4
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Helper function for self-testing PKCS#7 signature verification.
+ *
+ * Copyright (C) 2024 Joachim Vandersmissen <[email protected]>
+ */
+
+void fips_signature_selftest(const char *name,
+			     const u8 *keys, size_t keys_len,
+			     const u8 *data, size_t data_len,
+			     const u8 *sig, size_t sig_len);
+
+#ifdef CONFIG_FIPS_SIGNATURE_SELFTEST_RSA
+void __init fips_signature_selftest_rsa(void);
+#else
+static inline void __init fips_signature_selftest_rsa(void) { }
+#endif
crypto/asymmetric_keys/selftest_rsa.c new file +172 -0

172 lines not on this page · raw message

Signature · 1 line
2.45.0
#1 lore raw

PATCH v5 2/2 certs: Add ECDSA signature verification self-test

Commit c27b2d2012e1 ("crypto: testmgr - allow ecdsa-nist-p256 and -p384 in FIPS mode") enabled support for ECDSA in crypto/testmgr.c. The PKCS#7 signature verification API builds upon the KCAPI primitives to perform its high-level operations. Therefore, this change in testmgr.c also allows ECDSA to be used by the PKCS#7 signature verification API (in FIPS mode).

However, from a FIPS perspective, the PKCS#7 signature verification API is a distinct "service" from the KCAPI primitives. This is because the PKCS#7 API performs a "full" signature verification, which consists of both hashing the data to be verified, and the public key operation. On the other hand, the KCAPI primitive does not perform this hashing step - it accepts pre-hashed data from the caller and only performs the public key operation.

For this reason, the ECDSA self-tests in crypto/testmgr.c are not sufficient to cover ECDSA signature verification offered by the PKCS#7 API. This is reflected by the self-test already present in this file for RSA PKCS#1 v1.5 signature verification.

The solution is simply to add a second self-test here for ECDSA. P-256 with SHA-256 hashing was chosen as those parameters should remain FIPS-approved for the foreseeable future, while keeping the performance impact to a minimum. The ECDSA certificate and PKCS#7 signed data was generated using OpenSSL. The input data is identical to the input data for the existing RSA self-test.

---
v5: move the changelog to the right place.

v4: FIPS_SIGNATURE_SELFTEST_ECDSA is no longer user-configurable and will be set when the dependencies are fulfilled.

v3: adding the FIPS_SIGNATURE_SELFTEST_RSA configuration option to enforce that ECDSA (and SHA-256) are either built-in; or ECDSA (and SHA-256) and x509_selftest are modules.

v2: add a separate file for the ECDSA self-test.

---
 crypto/asymmetric_keys/Kconfig          |  7 ++
 crypto/asymmetric_keys/Makefile         |  1 +
 crypto/asymmetric_keys/selftest.c       |  1 +
 crypto/asymmetric_keys/selftest.h       |  6 ++
 crypto/asymmetric_keys/selftest_ecdsa.c | 89 +++++++++++++++++++++++++
 5 files changed, 104 insertions(+)
 create mode 100644 crypto/asymmetric_keys/selftest_ecdsa.c
crypto/asymmetric_keys/Kconfig +7 -0
index 33bbfd0d8367..06e64e29587e 100644
@@ -93,4 +93,11 @@ config FIPS_SIGNATURE_SELFTEST_RSA
 	depends on CRYPTO_SHA256=y || CRYPTO_SHA256=FIPS_SIGNATURE_SELFTEST
 	depends on CRYPTO_RSA=y || CRYPTO_RSA=FIPS_SIGNATURE_SELFTEST
 
+config FIPS_SIGNATURE_SELFTEST_ECDSA
+	bool
+	default y
+	depends on FIPS_SIGNATURE_SELFTEST
+	depends on CRYPTO_SHA256=y || CRYPTO_SHA256=FIPS_SIGNATURE_SELFTEST
+	depends on CRYPTO_ECDSA=y || CRYPTO_ECDSA=FIPS_SIGNATURE_SELFTEST
+
 endif # ASYMMETRIC_KEY_TYPE
crypto/asymmetric_keys/Makefile +1 -0
index ac1402e27324..bc65d3b98dcb 100644
@@ -25,6 +25,7 @@ x509_key_parser-y := \
 obj-$(CONFIG_FIPS_SIGNATURE_SELFTEST) += x509_selftest.o
 x509_selftest-y += selftest.o
 x509_selftest-$(CONFIG_FIPS_SIGNATURE_SELFTEST_RSA) += selftest_rsa.o
+x509_selftest-$(CONFIG_FIPS_SIGNATURE_SELFTEST_ECDSA) += selftest_ecdsa.o
 
 $(obj)/x509_cert_parser.o: \
 	$(obj)/x509.asn1.h \
crypto/asymmetric_keys/selftest.c +1 -0
index ec289d2d065c..98dc5cdfdebe 100644
@@ -61,6 +61,7 @@ void fips_signature_selftest(const char *name,
 static int __init fips_signature_selftest_init(void)
 {
 	fips_signature_selftest_rsa();
+	fips_signature_selftest_ecdsa();
 	return 0;
 }
 
crypto/asymmetric_keys/selftest.h +6 -0
index 842ac3cf86b4..4139f05906cb 100644
@@ -14,3 +14,9 @@ void __init fips_signature_selftest_rsa(void);
 #else
 static inline void __init fips_signature_selftest_rsa(void) { }
 #endif
+
+#ifdef CONFIG_FIPS_SIGNATURE_SELFTEST_ECDSA
+void __init fips_signature_selftest_ecdsa(void);
+#else
+static inline void __init fips_signature_selftest_ecdsa(void) { }
+#endif
crypto/asymmetric_keys/selftest_ecdsa.c new file +89 -0

89 lines not on this page · raw message

Signature · 1 line
2.45.0
Reviewed-by on v5 2/2
#2 lore raw

Re: [PATCH v5 2/2] certs: Add ECDSA signature verification self-test

Quoted 20 lines · Joachim Vandersmissen

Commit c27b2d2012e1 ("crypto: testmgr - allow ecdsa-nist-p256 and -p384 in FIPS mode") enabled support for ECDSA in crypto/testmgr.c. The PKCS#7 signature verification API builds upon the KCAPI primitives to perform its high-level operations. Therefore, this change in testmgr.c also allows ECDSA to be used by the PKCS#7 signature verification API (in FIPS mode).

However, from a FIPS perspective, the PKCS#7 signature verification API is a distinct "service" from the KCAPI primitives. This is because the PKCS#7 API performs a "full" signature verification, which consists of both hashing the data to be verified, and the public key operation. On the other hand, the KCAPI primitive does not perform this hashing step - it accepts pre-hashed data from the caller and only performs the public key operation.

For this reason, the ECDSA self-tests in crypto/testmgr.c are not sufficient to cover ECDSA signature verification offered by the PKCS#7 API. This is reflected by the self-test already present in this file for RSA PKCS#1 v1.5 signature verification.

Quoted 8 lines

The solution is simply to add a second self-test here for ECDSA. P-256 with SHA-256 hashing was chosen as those parameters should remain FIPS-approved for the foreseeable future, while keeping the performance impact to a minimum. The ECDSA certificate and PKCS#7 signed data was generated using OpenSSL. The input data is identical to the input data for the existing RSA self-test.

Unfortunately I don't have anything to complain about so:

Linus pulled 3/4 of my previous pull requests (TPM, trusted keys and keyring) so the road is clear for asymmetric keys pull request.

BR, Jarkko

Reviewed-by on v5 1/2
#3 lore raw
In reply to Joachim Vandersmissen

crypto/asymmetric_keys/selftest.c static int __init fips_signature_selftest(void) · line 44

Quoted 3 lines · Joachim Vandersmissen
crypto/asymmetric_keys/selftest.c +3 -0
+	pkcs7 = pkcs7_parse_message(sig, sig_len);
+	if (IS_ERR(pkcs7))
+		panic("Certs %s selftest: pkcs7_parse_message() = %d\n", name, ret);

Off-topic: wondering if Linux had similar helpers for PKCS#1 padding (and if not, are they difficult to add)?

Anyway, looks good to me:

BR, Jarkko

#4 lore raw
Quoted 6 lines · Jarkko Sakkinen

crypto/asymmetric_keys/selftest.c static int __init fips_signature_selftest(void) · line 44

Quoted 3 lines · Joachim Vandersmissen
crypto/asymmetric_keys/selftest.c +3 -0
+	pkcs7 = pkcs7_parse_message(sig, sig_len);
+	if (IS_ERR(pkcs7))
+		panic("Certs %s selftest: pkcs7_parse_message() = %d\n", name, ret);

Off-topic: wondering if Linux had similar helpers for PKCS#1 padding (and if not, are they difficult to add)?

PKCS#7 here refers to the message container format, rather than the padding. Internally, the PKCS#1 v1.5 padding scheme will be used (see software_key_determine_akcipher). Unless you are referring to PSS padding (also defined in PKCS#1)?

Quoted 6 lines

Anyway, looks good to me:

BR, Jarkko

#5 lore raw
Quoted 11 lines · Joachim Vandersmissen
Quoted 6 lines · Jarkko Sakkinen

crypto/asymmetric_keys/selftest.c static int __init fips_signature_selftest(void) · line 44

Quoted 3 lines · Joachim Vandersmissen
crypto/asymmetric_keys/selftest.c +3 -0
+	pkcs7 = pkcs7_parse_message(sig, sig_len);
+	if (IS_ERR(pkcs7))
+		panic("Certs %s selftest: pkcs7_parse_message() = %d\n", name, ret);

Off-topic: wondering if Linux had similar helpers for PKCS#1 padding (and if not, are they difficult to add)?

PKCS#7 here refers to the message container format, rather than the padding. Internally, the PKCS#1 v1.5 padding scheme will be used (see software_key_determine_akcipher). Unless you are referring to PSS padding (also defined in PKCS#1)?

I think it should be PCKS#1 v1.5 padding as described in RFC 8017 [1] but just for doing step 5:

https://www.rfc-editor.org/rfc/rfc8017#section-9.2.

This is for refreshing this old patch:

https://lore.kernel.org/all/[email protected]/

I asked James if he could refresh it and one of the remarks was that there is duplicate snippets with:

https://elixir.bootlin.com/linux/v6.9-rc6/source/crypto/rsa-pkcs1pad.c

But now that I look at this padding is not the issue here, but it is the duplicate digest_info instances.

James has this construct in the old patch:

static const struct asn1_template {
	const char	*name;
	const u8	*data;
	size_t		size;
} asn1_templates[] = {
#define _(X) { #X, digest_info_##X, sizeof(digest_info_##X) }
	_(md5),
	_(sha1),
	_(rmd160),
	_(sha256),
	_(sha384),
	_(sha512),
	_(sha224),
	{ NULL }

#undef _
};

static const struct asn1_template *lookup_asn1(const char *name)

{
	const struct asn1_template *p;

	for (p = asn1_templates; p->name; p++)
		if (strcmp(name, p->name) = 0)
			return p;
	return NULL;
}

Looking at this the very first thing I spot is that the last field is redundant so let's scrape that away. I neither get why use u8* instead of struct digest_info * so let's switch to that.

So with those substitutions, renaming and a bit of polishing (but not yet compiling ;-)) this what I end up with:

static const struct digest_info_mapping {
	char *name;
	struct digest_info *info;
} digest_info_map[] = {
#define _(X) { #X, digest_info_##X, }
	_(md5),
	_(sha1),
	_(rmd160),
	_(sha256),
	_(sha384),
	_(sha512),
	_(sha224),
	{ NULL }

#undef _
};

/**
* find_digest_info() - Find digest info by the hash name
* @name: hash name
*
* Returns the digest info on success, and NULL on failure. *
struct digest_info *find_digest_info(const char *name)

{
	struct digest_info *mapping;
	int i;

	for (i = 0; digest_info_map[i] != NULL; i++) {
		mapping = digest_info_map[i];
	
		if (!strcmp(name, mapping->name))
			return mapping->info;
	}

	return NULL;
}

EXPORT_SYMBOL_GPL(find_digest_info);

The instances live in rsa-pcks1pad.c so it is the most trivial place to add this.

BR, Jarkko