[PATCH v2 00/18] crypto: dh - infrastructure for NVM in-band auth and FIPS conformance

[PATCH v2 00/18] crypto: dh - infrastructure for NVM in-band auth and FIPS conformance

[Nicolai Stange]

Hi all,

first of all, to the people primarily interested in security/keys/, there's
a rather trivial change to security/keys/dh.c in patch 2/18. It would be
great to get ACKs for that...

This is v2, v1 can be found at

  https://lore.kernel.org/r/20211201004858.19831-1-nstange@suse.de

For a list of changes, see below.

Quote from v1's cover letter:
===
Hannes' recent work on NVME in-band authentication ([1]) needs access to
the RFC 7919 DH group parameters and also some means to generate ephemeral
keys. He currently implements both as part of his patchset (patches 3/12
and 8/12). After some internal discussion, we decided to split off the bits
needed from crypto/dh into a separate series, i.e. this one here:
 - for the RFC 7919 DH group parameters, it's undesirable from a
   performance POV to serialize the well-known domain parameters via
   crypto_dh_encode_key() just to deserialize them shortly after again,
 - from an architectural POV, it would be preferrable to have the key
   generation code in crypto/dh.c rather than in drivers/nvme/,
   just in analogy to how key generation is supported by crypto/ecdh.c
   already.

Patches 1-13/18 implement all that is needed for the NVME in-band
authentication support. 

Unfortunately, due to the lack of HW, I have not been able to test
the changes to the QAT or HPRE drivers (other than mere compile tests).
Yet I figured it would be a good idea to have them behave consistently with
dh_generic, and so I chose to introduce support for privkey generation to
these as well.


By coincidence, NIST SP800-56Arev3 compliance effectively requires that
the domain parameters are checked against an approved set, which happens
to consists of those safe-prime group parameters specified in RFC 7919,
among others. Thus, introducing the RFC 7919 parameters to the kernel
allows for making the DH implementation to conform to SP800-56Arev3 with
only little effort. I used the opportunity to work crypto/dh towards
SP800-56Arev3 conformance with the rest of this patch series, i.e.
patches 14-18/18. I can split these into another series on its own, if you
like. But as they depend on the earlier patches 1-13/18, I sent them
alongside for now.
===

This patchset has been tested with and without fips_enabled on x86_64,
ppc64le and s390x, the latter being big endian.


Changes v1 -> v2:
- Throughout the patchset:
  - Upcase enum group_id members and strip superfluous _RFCXYZ_ parts from
    the names.
  - Carry Hannes' Reviewed-bys from v1 over for those patches which
    have not changed (except for that group_id member renaming)
- [03/18] ("crypto: dh - optimize domain parameter serialization for
            well-known groups"):
  - For better portability, don't serialize/deserialize directly from/to
    an enum group_id, but use an intermediate int for that.
- [05/18] ("crypto: testmgr - add DH RFC 7919 ffdhe2048 test vector")
  - Use ffdhe3072 TVs rather than ones for ffdhe2048. Requested by Hannes,
    because "the NVMe spec mandates for its TLS profile the ffdhe3072
    group".
- [13/18] ("crypto: testmgr - add DH test vectors for key generation")
  - Use ffdhe3072 in place of ffdhe2048 here as well.
  - Rather than introducing completely new keypairs, reuse the ones
    from the known answer test introduced previously in this patchset.

Thanks,

Nicolai

[1] https://lkml.kernel.org/r/20211123123801.73197-1-hare@suse.de


Nicolai Stange (18):
  crypto: dh - remove struct dh's ->q member
  crypto: dh - constify struct dh's pointer members
  crypto: dh - optimize domain parameter serialization for well-known
    groups
  crypto: dh - introduce RFC 7919 safe-prime groups
  crypto: testmgr - add DH RFC 7919 ffdhe3072 test vector
  crypto: dh - introduce RFC 3526 safe-prime groups
  crypto: testmgr - add DH RFC 3526 modp2048 test vector
  crypto: testmgr - run only subset of DH vectors based on config
  crypto: dh - implement private key generation primitive
  crypto: dh - introduce support for ephemeral key generation to
    dh-generic
  crypto: dh - introduce support for ephemeral key generation to hpre
    driver
  crypto: dh - introduce support for ephemeral key generation to qat
    driver
  crypto: testmgr - add DH test vectors for key generation
  lib/mpi: export mpi_rshift
  crypto: dh - store group id in dh-generic's dh_ctx
  crypto: dh - calculate Q from P for the full public key verification
  crypto: dh - try to match domain parameters to a known safe-prime
    group
  crypto: dh - accept only approved safe-prime groups in FIPS mode

 crypto/Kconfig                                |  20 +-
 crypto/dh.c                                   |  73 +-
 crypto/dh_helper.c                            | 691 +++++++++++++++++-
 crypto/testmgr.h                              | 388 +++++++++-
 drivers/crypto/hisilicon/hpre/hpre_crypto.c   |  11 +
 drivers/crypto/qat/qat_common/qat_asym_algs.c |   9 +
 include/crypto/dh.h                           |  52 +-
 lib/mpi/mpi-bit.c                             |   1 +
 security/keys/dh.c                            |   2 +-
 9 files changed, 1189 insertions(+), 58 deletions(-)

-- 
2.26.2

[PATCH v2 01/18] crypto: dh - remove struct dh's ->q member

[Nicolai Stange]

The only current user of the DH KPP algorithm, the
keyctl(KEYCTL_DH_COMPUTE) syscall, doesn't set the domain parameter ->q
in struct dh. Remove it and any associated (de)serialization code in
crypto_dh_encode_key() and crypto_dh_decode_key. Adjust the encoded
->secret values in testmgr's DH test vectors accordingly.

Note that the dh-generic implementation would have initialized its
struct dh_ctx's ->q from the decoded struct dh's ->q, if present. If this
struct dh_ctx's ->q would ever have been non-NULL, it would have enabled a
full key validation as specified in NIST SP800-56A in dh_is_pubkey_valid().
However, as outlined above, ->q is always NULL in practice and the full key
validation code is effectively dead. A later patch will make
dh_is_pubkey_valid() to calculate Q from P on the fly, if possible, so
don't remove struct dh_ctx's ->q now, but leave it there until that has
happened.

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 crypto/dh.c         |  6 ------
 crypto/dh_helper.c  | 17 ++++-------------
 crypto/testmgr.h    | 16 ++++++----------
 include/crypto/dh.h |  4 ----
 4 files changed, 10 insertions(+), 33 deletions(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index cd4f32092e5c..131b80064cb1 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -59,12 +59,6 @@ static int dh_set_params(struct dh_ctx *ctx, struct dh *params)
 	if (!ctx->p)
 		return -EINVAL;
 
-	if (params->q && params->q_size) {
-		ctx->q = mpi_read_raw_data(params->q, params->q_size);
-		if (!ctx->q)
-			return -EINVAL;
-	}
-
 	ctx->g = mpi_read_raw_data(params->g, params->g_size);
 	if (!ctx->g)
 		return -EINVAL;
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 9fd5a42eea15..aabc91e4f63f 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -10,7 +10,7 @@
 #include <crypto/dh.h>
 #include <crypto/kpp.h>
 
-#define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 4 * sizeof(int))
+#define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 3 * sizeof(int))
 
 static inline u8 *dh_pack_data(u8 *dst, u8 *end, const void *src, size_t size)
 {
@@ -28,7 +28,7 @@ static inline const u8 *dh_unpack_data(void *dst, const void *src, size_t size)
 
 static inline unsigned int dh_data_size(const struct dh *p)
 {
-	return p->key_size + p->p_size + p->q_size + p->g_size;
+	return p->key_size + p->p_size + p->g_size;
 }
 
 unsigned int crypto_dh_key_len(const struct dh *p)
@@ -53,11 +53,9 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params)
 	ptr = dh_pack_data(ptr, end, &params->key_size,
 			   sizeof(params->key_size));
 	ptr = dh_pack_data(ptr, end, &params->p_size, sizeof(params->p_size));
-	ptr = dh_pack_data(ptr, end, &params->q_size, sizeof(params->q_size));
 	ptr = dh_pack_data(ptr, end, &params->g_size, sizeof(params->g_size));
 	ptr = dh_pack_data(ptr, end, params->key, params->key_size);
 	ptr = dh_pack_data(ptr, end, params->p, params->p_size);
-	ptr = dh_pack_data(ptr, end, params->q, params->q_size);
 	ptr = dh_pack_data(ptr, end, params->g, params->g_size);
 	if (ptr != end)
 		return -EINVAL;
@@ -79,7 +77,6 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 
 	ptr = dh_unpack_data(&params->key_size, ptr, sizeof(params->key_size));
 	ptr = dh_unpack_data(&params->p_size, ptr, sizeof(params->p_size));
-	ptr = dh_unpack_data(&params->q_size, ptr, sizeof(params->q_size));
 	ptr = dh_unpack_data(&params->g_size, ptr, sizeof(params->g_size));
 	if (secret.len != crypto_dh_key_len(params))
 		return -EINVAL;
@@ -89,7 +86,7 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	 * some drivers assume otherwise.
 	 */
 	if (params->key_size > params->p_size ||
-	    params->g_size > params->p_size || params->q_size > params->p_size)
+	    params->g_size > params->p_size)
 		return -EINVAL;
 
 	/* Don't allocate memory. Set pointers to data within
@@ -97,9 +94,7 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	 */
 	params->key = (void *)ptr;
 	params->p = (void *)(ptr + params->key_size);
-	params->q = (void *)(ptr + params->key_size + params->p_size);
-	params->g = (void *)(ptr + params->key_size + params->p_size +
-			     params->q_size);
+	params->g = (void *)(ptr + params->key_size + params->p_size);
 
 	/*
 	 * Don't permit 'p' to be 0.  It's not a prime number, and it's subject
@@ -109,10 +104,6 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	if (memchr_inv(params->p, 0, params->p_size) == NULL)
 		return -EINVAL;
 
-	/* It is permissible to not provide Q. */
-	if (params->q_size == 0)
-		params->q = NULL;
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(crypto_dh_decode_key);
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 779720bf9364..7f7d5ae48721 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -1244,17 +1244,15 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.secret =
 #ifdef __LITTLE_ENDIAN
 	"\x01\x00" /* type */
-	"\x15\x02" /* len */
+	"\x11\x02" /* len */
 	"\x00\x01\x00\x00" /* key_size */
 	"\x00\x01\x00\x00" /* p_size */
-	"\x00\x00\x00\x00" /* q_size */
 	"\x01\x00\x00\x00" /* g_size */
 #else
 	"\x00\x01" /* type */
-	"\x02\x15" /* len */
+	"\x02\x11" /* len */
 	"\x00\x00\x01\x00" /* key_size */
 	"\x00\x00\x01\x00" /* p_size */
-	"\x00\x00\x00\x00" /* q_size */
 	"\x00\x00\x00\x01" /* g_size */
 #endif
 	/* xa */
@@ -1344,7 +1342,7 @@ static const struct kpp_testvec dh_tv_template[] = {
 	"\xd3\x34\x49\xad\x64\xa6\xb1\xc0\x59\x28\x75\x60\xa7\x8a\xb0\x11"
 	"\x56\x89\x42\x74\x11\xf5\xf6\x5e\x6f\x16\x54\x6a\xb1\x76\x4d\x50"
 	"\x8a\x68\xc1\x5b\x82\xb9\x0d\x00\x32\x50\xed\x88\x87\x48\x92\x17",
-	.secret_size = 533,
+	.secret_size = 529,
 	.b_public_size = 256,
 	.expected_a_public_size = 256,
 	.expected_ss_size = 256,
@@ -1353,17 +1351,15 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.secret =
 #ifdef __LITTLE_ENDIAN
 	"\x01\x00" /* type */
-	"\x15\x02" /* len */
+	"\x11\x02" /* len */
 	"\x00\x01\x00\x00" /* key_size */
 	"\x00\x01\x00\x00" /* p_size */
-	"\x00\x00\x00\x00" /* q_size */
 	"\x01\x00\x00\x00" /* g_size */
 #else
 	"\x00\x01" /* type */
-	"\x02\x15" /* len */
+	"\x02\x11" /* len */
 	"\x00\x00\x01\x00" /* key_size */
 	"\x00\x00\x01\x00" /* p_size */
-	"\x00\x00\x00\x00" /* q_size */
 	"\x00\x00\x00\x01" /* g_size */
 #endif
 	/* xa */
@@ -1453,7 +1449,7 @@ static const struct kpp_testvec dh_tv_template[] = {
 	"\x5e\x5a\x64\xbd\xf6\x85\x04\xe8\x28\x6a\xac\xef\xce\x19\x8e\x9a"
 	"\xfe\x75\xc0\x27\x69\xe3\xb3\x7b\x21\xa7\xb1\x16\xa4\x85\x23\xee"
 	"\xb0\x1b\x04\x6e\xbd\xab\x16\xde\xfd\x86\x6b\xa9\x95\xd7\x0b\xfd",
-	.secret_size = 533,
+	.secret_size = 529,
 	.b_public_size = 256,
 	.expected_a_public_size = 256,
 	.expected_ss_size = 256,
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index d71e9858ab86..2585f0e6bb69 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -24,21 +24,17 @@
  *
  * @key:	Private DH key
  * @p:		Diffie-Hellman parameter P
- * @q:		Diffie-Hellman parameter Q
  * @g:		Diffie-Hellman generator G
  * @key_size:	Size of the private DH key
  * @p_size:	Size of DH parameter P
- * @q_size:	Size of DH parameter Q
  * @g_size:	Size of DH generator G
  */
 struct dh {
 	void *key;
 	void *p;
-	void *q;
 	void *g;
 	unsigned int key_size;
 	unsigned int p_size;
-	unsigned int q_size;
 	unsigned int g_size;
 };
 
-- 
2.26.2

[PATCH v2 02/18] crypto: dh - constify struct dh's pointer members

[Nicolai Stange]

struct dh contains several pointer members corresponding to DH parameters:
->key, ->p and ->g. A subsequent commit will make the struct dh
deserialization function, crypto_dh_decode_key(), to set these to
constant static storage arrays for some of the well-known safe-prime
groups.

Turn the struct dh pointer members' types into "pointer to const" in
preparation for this.

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 include/crypto/dh.h | 6 +++---
 security/keys/dh.c  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index 2585f0e6bb69..67f3f6bca527 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -30,9 +30,9 @@
  * @g_size:	Size of DH generator G
  */
 struct dh {
-	void *key;
-	void *p;
-	void *g;
+	const void *key;
+	const void *p;
+	const void *g;
 	unsigned int key_size;
 	unsigned int p_size;
 	unsigned int g_size;
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 1abfa70ed6e1..a58f6fb9f6db 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -14,7 +14,7 @@
 #include <keys/user-type.h>
 #include "internal.h"
 
-static ssize_t dh_data_from_key(key_serial_t keyid, void **data)
+static ssize_t dh_data_from_key(key_serial_t keyid, const void **data)
 {
 	struct key *key;
 	key_ref_t key_ref;
-- 
2.26.2

[PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Nicolai Stange]

DH users are supposed to set a struct dh instance's ->p and ->g domain
parameters (as well as the secret ->key), serialize the whole struct dh
instance via the crypto_dh_encode_key() helper and pass the encoded blob
on to the DH's ->set_secret(). All three currently available DH
implementations (generic, drivers/crypto/hisilicon/hpre/ and
drivers/crypto/qat/) would then proceed to call the crypto_dh_decode_key()
helper for unwrapping the encoded struct dh instance again.

Up to now, the only DH user has been the keyctl(KEYCTL_DH_COMPUTE) syscall
and thus, all domain parameters have been coming from userspace. The domain
parameter encoding scheme for DH's ->set_secret() has been a perfectly
reasonable approach in this setting and the potential extra copy of ->p
and ->g during the encoding phase didn't harm much.

However, recently, the need for working with the well-known safe-prime
groups' domain parameters from RFC 3526 and RFC 7919 resp. arose from two
independent developments:
- The NVME in-band authentication support currently being worked on ([1])
  needs to install the RFC 7919 ffdhe groups' domain parameters for DH
  tfms.
- In FIPS mode, there's effectively no sensible way for the DH
  implementation to conform to SP800-56Arev3 other than rejecting any
  parameter set not corresponding to some approved safe-prime group
  specified in either of these two RFCs.

As the ->p arrays' lengths are in the range from 256 to 1024 bytes, it
would be nice if that extra copy during the crypto_dh_encode_key() step
from the NVME in-band authentication code could be avoided. Likewise, it
would be great if the DH implementation's FIPS handling code could avoid
attempting to match the input ->p and ->g against the individual approved
groups' parameters via memcmp() if it's known in advance that the input
corresponds to such one, as is the case for NVME.

Introduce a enum dh_group_id for referring to any of the safe-prime groups
known to the kernel. The introduction of actual such safe-prime groups
alongside with their resp. P and G parameters will be deferred to later
patches. As of now, the new enum contains only a single member,
DH_GROUP_ID_UNKNOWN, which is meant to be associated with parameter sets
not corresponding to any of the groups known to the kernel, as is needed
to continue to support the current keyctl(KEYCTL_DH_COMPUTE) syscall
semantics.

Add a new 'group_id' member of type enum group_id to struct dh. Make
crypto_dh_encode_key() include it in the serialization and to encode
->p and ->g only if it equals DH_GROUP_ID_UNKNOWN. For all other possible
values of the encoded ->group_id, the receiving decoding primitive,
crypto_dh_decode_key(), is made to not decode ->p and ->g from the encoded
data, but to look them up in a central registry instead.

The intended usage pattern is that users like NVME wouldn't set any of
the struct dh's ->p or ->g directly, but only the ->group_id for the group
they're interested in. They'd then proceed as usual and call
crypto_dh_encode_key() on the struct dh instance, pass the encoded result
on to DH's ->set_secret() and the latter would then invoke
crypto_dh_decode_key(), which would then in turn lookup the parameters
associated with the passed ->group_id.

Note that this will avoid the extra copy of the ->p and ->g for the groups
(to be made) known to the kernel and also, that a future patch can easily
introduce a validation of ->group_id if in FIPS mode.

As mentioned above, the introduction of actual safe-prime groups will be
deferred to later patches, so for now, only introduce an empty placeholder
array safe_prime_groups[] to be queried by crypto_dh_decode_key() for
domain parameters associated with a given ->group_id as outlined above.
Make its elements to be of the new internal struct safe_prime_group type.
Among the members ->group_id, ->p and ->p_size with obvious meaning, there
will also be a ->max_strength member for storing the maximum security
strength supported by the associated group -- its value will be needed for
the upcoming private key generation support.

Finally, update the encoded secrets provided by the testmgr's DH test
vectors in order to account for the additional ->group_id field expected
by crypto_dh_decode_key() now.

[1] https://lkml.kernel.org/r/20211122074727.25988-4-hare@suse.de

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/dh_helper.c  | 90 ++++++++++++++++++++++++++++++++++++---------
 crypto/testmgr.h    | 16 +++++---
 include/crypto/dh.h |  6 +++
 3 files changed, 88 insertions(+), 24 deletions(-)

diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index aabc91e4f63f..9f21204e5dee 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -10,7 +10,31 @@
 #include <crypto/dh.h>
 #include <crypto/kpp.h>
 
-#define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 3 * sizeof(int))
+#define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 4 * sizeof(int))
+
+static const struct safe_prime_group
+{
+	enum dh_group_id group_id;
+	unsigned int max_strength;
+	unsigned int p_size;
+	const char *p;
+} safe_prime_groups[] = {};
+
+/* 2 is used as a generator for all safe-prime groups. */
+static const char safe_prime_group_g[]  = { 2 };
+
+static inline const struct safe_prime_group *
+get_safe_prime_group(enum dh_group_id group_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(safe_prime_groups); ++i) {
+		if (safe_prime_groups[i].group_id == group_id)
+			return &safe_prime_groups[i];
+	}
+
+	return NULL;
+}
 
 static inline u8 *dh_pack_data(u8 *dst, u8 *end, const void *src, size_t size)
 {
@@ -28,7 +52,10 @@ static inline const u8 *dh_unpack_data(void *dst, const void *src, size_t size)
 
 static inline unsigned int dh_data_size(const struct dh *p)
 {
-	return p->key_size + p->p_size + p->g_size;
+	if (p->group_id == DH_GROUP_ID_UNKNOWN)
+		return p->key_size + p->p_size + p->g_size;
+	else
+		return p->key_size;
 }
 
 unsigned int crypto_dh_key_len(const struct dh *p)
@@ -45,18 +72,24 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params)
 		.type = CRYPTO_KPP_SECRET_TYPE_DH,
 		.len = len
 	};
+	int group_id;
 
 	if (unlikely(!len))
 		return -EINVAL;
 
 	ptr = dh_pack_data(ptr, end, &secret, sizeof(secret));
+	group_id = (int)params->group_id;
+	ptr = dh_pack_data(ptr, end, &group_id, sizeof(group_id));
 	ptr = dh_pack_data(ptr, end, &params->key_size,
 			   sizeof(params->key_size));
 	ptr = dh_pack_data(ptr, end, &params->p_size, sizeof(params->p_size));
 	ptr = dh_pack_data(ptr, end, &params->g_size, sizeof(params->g_size));
 	ptr = dh_pack_data(ptr, end, params->key, params->key_size);
-	ptr = dh_pack_data(ptr, end, params->p, params->p_size);
-	ptr = dh_pack_data(ptr, end, params->g, params->g_size);
+	if (params->group_id == DH_GROUP_ID_UNKNOWN) {
+		ptr = dh_pack_data(ptr, end, params->p, params->p_size);
+		ptr = dh_pack_data(ptr, end, params->g, params->g_size);
+	}
+
 	if (ptr != end)
 		return -EINVAL;
 	return 0;
@@ -67,6 +100,7 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 {
 	const u8 *ptr = buf;
 	struct kpp_secret secret;
+	int group_id;
 
 	if (unlikely(!buf || len < DH_KPP_SECRET_MIN_SIZE))
 		return -EINVAL;
@@ -75,12 +109,46 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	if (secret.type != CRYPTO_KPP_SECRET_TYPE_DH)
 		return -EINVAL;
 
+	ptr = dh_unpack_data(&group_id, ptr, sizeof(group_id));
+	params->group_id = (enum dh_group_id)group_id;
 	ptr = dh_unpack_data(&params->key_size, ptr, sizeof(params->key_size));
 	ptr = dh_unpack_data(&params->p_size, ptr, sizeof(params->p_size));
 	ptr = dh_unpack_data(&params->g_size, ptr, sizeof(params->g_size));
 	if (secret.len != crypto_dh_key_len(params))
 		return -EINVAL;
 
+	if (params->group_id == DH_GROUP_ID_UNKNOWN) {
+		/* Don't allocate memory. Set pointers to data within
+		 * the given buffer
+		 */
+		params->key = (void *)ptr;
+		params->p = (void *)(ptr + params->key_size);
+		params->g = (void *)(ptr + params->key_size + params->p_size);
+
+		/*
+		 * Don't permit 'p' to be 0.  It's not a prime number,
+		 * and it's subject to corner cases such as 'mod 0'
+		 * being undefined or crypto_kpp_maxsize() returning
+		 * 0.
+		 */
+		if (memchr_inv(params->p, 0, params->p_size) == NULL)
+			return -EINVAL;
+
+	} else {
+		const struct safe_prime_group *g;
+
+		g = get_safe_prime_group(params->group_id);
+		if (!g)
+			return -EINVAL;
+
+		params->key = (void *)ptr;
+
+		params->p = g->p;
+		params->p_size = g->p_size;
+		params->g = safe_prime_group_g;
+		params->g_size = sizeof(safe_prime_group_g);
+	}
+
 	/*
 	 * Don't permit the buffer for 'key' or 'g' to be larger than 'p', since
 	 * some drivers assume otherwise.
@@ -89,20 +157,6 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	    params->g_size > params->p_size)
 		return -EINVAL;
 
-	/* Don't allocate memory. Set pointers to data within
-	 * the given buffer
-	 */
-	params->key = (void *)ptr;
-	params->p = (void *)(ptr + params->key_size);
-	params->g = (void *)(ptr + params->key_size + params->p_size);
-
-	/*
-	 * Don't permit 'p' to be 0.  It's not a prime number, and it's subject
-	 * to corner cases such as 'mod 0' being undefined or
-	 * crypto_kpp_maxsize() returning 0.
-	 */
-	if (memchr_inv(params->p, 0, params->p_size) == NULL)
-		return -EINVAL;
 
 	return 0;
 }
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 7f7d5ae48721..637913064c64 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -1244,13 +1244,15 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.secret =
 #ifdef __LITTLE_ENDIAN
 	"\x01\x00" /* type */
-	"\x11\x02" /* len */
+	"\x15\x02" /* len */
+	"\x00\x00\x00\x00" /* group_id == DH_GROUP_ID_UNKNOWN */
 	"\x00\x01\x00\x00" /* key_size */
 	"\x00\x01\x00\x00" /* p_size */
 	"\x01\x00\x00\x00" /* g_size */
 #else
 	"\x00\x01" /* type */
-	"\x02\x11" /* len */
+	"\x02\x15" /* len */
+	"\x00\x00\x00\x00" /* group_id == DH_GROUP_ID_UNKNOWN */
 	"\x00\x00\x01\x00" /* key_size */
 	"\x00\x00\x01\x00" /* p_size */
 	"\x00\x00\x00\x01" /* g_size */
@@ -1342,7 +1344,7 @@ static const struct kpp_testvec dh_tv_template[] = {
 	"\xd3\x34\x49\xad\x64\xa6\xb1\xc0\x59\x28\x75\x60\xa7\x8a\xb0\x11"
 	"\x56\x89\x42\x74\x11\xf5\xf6\x5e\x6f\x16\x54\x6a\xb1\x76\x4d\x50"
 	"\x8a\x68\xc1\x5b\x82\xb9\x0d\x00\x32\x50\xed\x88\x87\x48\x92\x17",
-	.secret_size = 529,
+	.secret_size = 533,
 	.b_public_size = 256,
 	.expected_a_public_size = 256,
 	.expected_ss_size = 256,
@@ -1351,13 +1353,15 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.secret =
 #ifdef __LITTLE_ENDIAN
 	"\x01\x00" /* type */
-	"\x11\x02" /* len */
+	"\x15\x02" /* len */
+	"\x00\x00\x00\x00" /* group_id == DH_GROUP_ID_UNKNOWN */
 	"\x00\x01\x00\x00" /* key_size */
 	"\x00\x01\x00\x00" /* p_size */
 	"\x01\x00\x00\x00" /* g_size */
 #else
 	"\x00\x01" /* type */
-	"\x02\x11" /* len */
+	"\x02\x15" /* len */
+	"\x00\x00\x00\x00" /* group_id == DH_GROUP_ID_UNKNOWN */
 	"\x00\x00\x01\x00" /* key_size */
 	"\x00\x00\x01\x00" /* p_size */
 	"\x00\x00\x00\x01" /* g_size */
@@ -1449,7 +1453,7 @@ static const struct kpp_testvec dh_tv_template[] = {
 	"\x5e\x5a\x64\xbd\xf6\x85\x04\xe8\x28\x6a\xac\xef\xce\x19\x8e\x9a"
 	"\xfe\x75\xc0\x27\x69\xe3\xb3\x7b\x21\xa7\xb1\x16\xa4\x85\x23\xee"
 	"\xb0\x1b\x04\x6e\xbd\xab\x16\xde\xfd\x86\x6b\xa9\x95\xd7\x0b\xfd",
-	.secret_size = 529,
+	.secret_size = 533,
 	.b_public_size = 256,
 	.expected_a_public_size = 256,
 	.expected_ss_size = 256,
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index 67f3f6bca527..f0ed899e2168 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -19,6 +19,11 @@
  * the KPP API function call of crypto_kpp_set_secret.
  */
 
+/** enum dh_group_id - identify well-known domain parameter sets */
+enum dh_group_id {
+	DH_GROUP_ID_UNKNOWN = 0, /* Constants are used in test vectors. */
+};
+
 /**
  * struct dh - define a DH private key
  *
@@ -30,6 +35,7 @@
  * @g_size:	Size of DH generator G
  */
 struct dh {
+	enum dh_group_id group_id;
 	const void *key;
 	const void *p;
 	const void *g;
-- 
2.26.2

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Hannes Reinecke]

On 12/9/21 10:03 AM, Nicolai Stange wrote:
> DH users are supposed to set a struct dh instance's ->p and ->g domain
> parameters (as well as the secret ->key), serialize the whole struct dh
> instance via the crypto_dh_encode_key() helper and pass the encoded blob
> on to the DH's ->set_secret(). All three currently available DH
> implementations (generic, drivers/crypto/hisilicon/hpre/ and
> drivers/crypto/qat/) would then proceed to call the crypto_dh_decode_key()
> helper for unwrapping the encoded struct dh instance again.
> 
> Up to now, the only DH user has been the keyctl(KEYCTL_DH_COMPUTE) syscall
> and thus, all domain parameters have been coming from userspace. The domain
> parameter encoding scheme for DH's ->set_secret() has been a perfectly
> reasonable approach in this setting and the potential extra copy of ->p
> and ->g during the encoding phase didn't harm much.
> 
> However, recently, the need for working with the well-known safe-prime
> groups' domain parameters from RFC 3526 and RFC 7919 resp. arose from two
> independent developments:
> - The NVME in-band authentication support currently being worked on ([1])
>   needs to install the RFC 7919 ffdhe groups' domain parameters for DH
>   tfms.
> - In FIPS mode, there's effectively no sensible way for the DH
>   implementation to conform to SP800-56Arev3 other than rejecting any
>   parameter set not corresponding to some approved safe-prime group
>   specified in either of these two RFCs.
> 
> As the ->p arrays' lengths are in the range from 256 to 1024 bytes, it
> would be nice if that extra copy during the crypto_dh_encode_key() step
> from the NVME in-band authentication code could be avoided. Likewise, it
> would be great if the DH implementation's FIPS handling code could avoid
> attempting to match the input ->p and ->g against the individual approved
> groups' parameters via memcmp() if it's known in advance that the input
> corresponds to such one, as is the case for NVME.
> 
> Introduce a enum dh_group_id for referring to any of the safe-prime groups
> known to the kernel. The introduction of actual such safe-prime groups
> alongside with their resp. P and G parameters will be deferred to later
> patches. As of now, the new enum contains only a single member,
> DH_GROUP_ID_UNKNOWN, which is meant to be associated with parameter sets
> not corresponding to any of the groups known to the kernel, as is needed
> to continue to support the current keyctl(KEYCTL_DH_COMPUTE) syscall
> semantics.
> 
> Add a new 'group_id' member of type enum group_id to struct dh. Make
> crypto_dh_encode_key() include it in the serialization and to encode
> ->p and ->g only if it equals DH_GROUP_ID_UNKNOWN. For all other possible
> values of the encoded ->group_id, the receiving decoding primitive,
> crypto_dh_decode_key(), is made to not decode ->p and ->g from the encoded
> data, but to look them up in a central registry instead.
> 
> The intended usage pattern is that users like NVME wouldn't set any of
> the struct dh's ->p or ->g directly, but only the ->group_id for the group
> they're interested in. They'd then proceed as usual and call
> crypto_dh_encode_key() on the struct dh instance, pass the encoded result
> on to DH's ->set_secret() and the latter would then invoke
> crypto_dh_decode_key(), which would then in turn lookup the parameters
> associated with the passed ->group_id.
> 
> Note that this will avoid the extra copy of the ->p and ->g for the groups
> (to be made) known to the kernel and also, that a future patch can easily
> introduce a validation of ->group_id if in FIPS mode.
> 
> As mentioned above, the introduction of actual safe-prime groups will be
> deferred to later patches, so for now, only introduce an empty placeholder
> array safe_prime_groups[] to be queried by crypto_dh_decode_key() for
> domain parameters associated with a given ->group_id as outlined above.
> Make its elements to be of the new internal struct safe_prime_group type.
> Among the members ->group_id, ->p and ->p_size with obvious meaning, there
> will also be a ->max_strength member for storing the maximum security
> strength supported by the associated group -- its value will be needed for
> the upcoming private key generation support.
> 
> Finally, update the encoded secrets provided by the testmgr's DH test
> vectors in order to account for the additional ->group_id field expected
> by crypto_dh_decode_key() now.
> 
> [1] https://lkml.kernel.org/r/20211122074727.25988-4-hare@suse.de
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  crypto/dh_helper.c  | 90 ++++++++++++++++++++++++++++++++++++---------
>  crypto/testmgr.h    | 16 +++++---
>  include/crypto/dh.h |  6 +++
>  3 files changed, 88 insertions(+), 24 deletions(-)
> 
> diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
> index aabc91e4f63f..9f21204e5dee 100644
> --- a/crypto/dh_helper.c
> +++ b/crypto/dh_helper.c
> @@ -10,7 +10,31 @@
>  #include <crypto/dh.h>
>  #include <crypto/kpp.h>
>  
> -#define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 3 * sizeof(int))
> +#define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 4 * sizeof(int))
> +
> +static const struct safe_prime_group
> +{
> +	enum dh_group_id group_id;
> +	unsigned int max_strength;
> +	unsigned int p_size;
> +	const char *p;
> +} safe_prime_groups[] = {};
> +
> +/* 2 is used as a generator for all safe-prime groups. */
> +static const char safe_prime_group_g[]  = { 2 };
> +
> +static inline const struct safe_prime_group *
> +get_safe_prime_group(enum dh_group_id group_id)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(safe_prime_groups); ++i) {
> +		if (safe_prime_groups[i].group_id == group_id)
> +			return &safe_prime_groups[i];
> +	}
> +
> +	return NULL;
> +}
>  
>  static inline u8 *dh_pack_data(u8 *dst, u8 *end, const void *src, size_t size)
>  {
> @@ -28,7 +52,10 @@ static inline const u8 *dh_unpack_data(void *dst, const void *src, size_t size)
>  
>  static inline unsigned int dh_data_size(const struct dh *p)
>  {
> -	return p->key_size + p->p_size + p->g_size;
> +	if (p->group_id == DH_GROUP_ID_UNKNOWN)
> +		return p->key_size + p->p_size + p->g_size;
> +	else
> +		return p->key_size;
>  }
>  
>  unsigned int crypto_dh_key_len(const struct dh *p)
> @@ -45,18 +72,24 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params)
>  		.type = CRYPTO_KPP_SECRET_TYPE_DH,
>  		.len = len
>  	};
> +	int group_id;
>  
>  	if (unlikely(!len))
>  		return -EINVAL;
>  
>  	ptr = dh_pack_data(ptr, end, &secret, sizeof(secret));
> +	group_id = (int)params->group_id;
> +	ptr = dh_pack_data(ptr, end, &group_id, sizeof(group_id));

Me being picky again.
To my knowledge, 'int' doesn't have a fixed width, but is rather only
guaranteed to hold certain values.
So as soon as one relies on any fixed size (as this one does) I tend to
use fixed size type like 'u32' to make it absolutely clear what is to be
expected here.

But the I don't know the conventions in the crypto code; if an 'int' is
assumed to be 32 bits throughout the crypto code I guess we should be fine.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Nicolai Stange]

Hannes Reinecke <hare@suse.de> writes:

> On 12/9/21 10:03 AM, Nicolai Stange wrote:
>> diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
>> index aabc91e4f63f..9f21204e5dee 100644
>> --- a/crypto/dh_helper.c
>> +++ b/crypto/dh_helper.c
>> @@ -45,18 +72,24 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params)
>>  		.type = CRYPTO_KPP_SECRET_TYPE_DH,
>>  		.len = len
>>  	};
>> +	int group_id;
>>  
>>  	if (unlikely(!len))
>>  		return -EINVAL;
>>  
>>  	ptr = dh_pack_data(ptr, end, &secret, sizeof(secret));
>> +	group_id = (int)params->group_id;
>> +	ptr = dh_pack_data(ptr, end, &group_id, sizeof(group_id));
>
> Me being picky again.
> To my knowledge, 'int' doesn't have a fixed width, but is rather only
> guaranteed to hold certain values.
> So as soon as one relies on any fixed size (as this one does) I tend to
> use fixed size type like 'u32' to make it absolutely clear what is to be
> expected here.
>
> But the I don't know the conventions in the crypto code; if an 'int' is
> assumed to be 32 bits throughout the crypto code I guess we should be fine.

Yes, I thought about this, too. However, the other, already existing
fields like ->key_size and ->p_size are getting serialized as unsigned
ints and I decided to stick to that for ->group_id as well. Except for
the testmgr vectors, the encoding is internal to the
crypto_dh_encode_key() and crypto_dh_decode_key() pair anyway -- all
that would happen if sizeof(int) != 4 is that the tests would fail.

So, IMO, making the serialization of struct dh to use u32 throughout is
not really in scope for this series and would probably deserve a patch
on its own, if desired.

Thanks,

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Hannes Reinecke]

On 12/13/21 11:06 AM, Nicolai Stange wrote:
> Hannes Reinecke <hare@suse.de> writes:
> 
>> On 12/9/21 10:03 AM, Nicolai Stange wrote:
>>> diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
>>> index aabc91e4f63f..9f21204e5dee 100644
>>> --- a/crypto/dh_helper.c
>>> +++ b/crypto/dh_helper.c
>>> @@ -45,18 +72,24 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params)
>>>  		.type = CRYPTO_KPP_SECRET_TYPE_DH,
>>>  		.len = len
>>>  	};
>>> +	int group_id;
>>>  
>>>  	if (unlikely(!len))
>>>  		return -EINVAL;
>>>  
>>>  	ptr = dh_pack_data(ptr, end, &secret, sizeof(secret));
>>> +	group_id = (int)params->group_id;
>>> +	ptr = dh_pack_data(ptr, end, &group_id, sizeof(group_id));
>>
>> Me being picky again.
>> To my knowledge, 'int' doesn't have a fixed width, but is rather only
>> guaranteed to hold certain values.
>> So as soon as one relies on any fixed size (as this one does) I tend to
>> use fixed size type like 'u32' to make it absolutely clear what is to be
>> expected here.
>>
>> But the I don't know the conventions in the crypto code; if an 'int' is
>> assumed to be 32 bits throughout the crypto code I guess we should be fine.
> 
> Yes, I thought about this, too. However, the other, already existing
> fields like ->key_size and ->p_size are getting serialized as unsigned
> ints and I decided to stick to that for ->group_id as well. Except for
> the testmgr vectors, the encoding is internal to the
> crypto_dh_encode_key() and crypto_dh_decode_key() pair anyway -- all
> that would happen if sizeof(int) != 4 is that the tests would fail.
> 
> So, IMO, making the serialization of struct dh to use u32 throughout is
> not really in scope for this series and would probably deserve a patch
> on its own, if desired.
> 
As I thought.

So that's okay, then.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Herbert Xu]

On Thu, Dec 09, 2021 at 10:03:43AM +0100, Nicolai Stange wrote:
>> diff --git a/include/crypto/dh.h b/include/crypto/dh.h
> index 67f3f6bca527..f0ed899e2168 100644
> --- a/include/crypto/dh.h
> +++ b/include/crypto/dh.h
> @@ -19,6 +19,11 @@
>   * the KPP API function call of crypto_kpp_set_secret.
>   */
>  
> +/** enum dh_group_id - identify well-known domain parameter sets */
> +enum dh_group_id {
> +	DH_GROUP_ID_UNKNOWN = 0, /* Constants are used in test vectors. */
> +};

We try to avoid hard-coded ID lists like these in the Crypto API.

I've had a look at your subsequent patches and I don't think you
really need this.

For instance, instead of shoehorning this into "dh", you could
instead create new kpp algorithms modpXXXX and ffdheXXXX which
can be templates around the underlying dh algorithm.  Sure this
might involve a copy of the parameters but given the speed of
the algorithms that we're talking about here I don't think it's
really relevant.

That way the underlying drivers don't need to be touched at all.

Yes I do realise that this means the keyrings DH user-space API
cannot be used in FIPS mode, but that is probably a good thing
as users who care about modp/ffdhe shouldn't really have to stuff
the raw vectors into this interface just to access the kernel DH
implementation.

On a side note, are there really keyrings DH users out there in
the wild? If not can we deprecate and remove this interface
completely?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Nicolai Stange]

Hello Herbert,

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Thu, Dec 09, 2021 at 10:03:43AM +0100, Nicolai Stange wrote:
>>> diff --git a/include/crypto/dh.h b/include/crypto/dh.h
>> index 67f3f6bca527..f0ed899e2168 100644
>> --- a/include/crypto/dh.h
>> +++ b/include/crypto/dh.h
>> @@ -19,6 +19,11 @@
>>   * the KPP API function call of crypto_kpp_set_secret.
>>   */
>>  
>> +/** enum dh_group_id - identify well-known domain parameter sets */
>> +enum dh_group_id {
>> +	DH_GROUP_ID_UNKNOWN = 0, /* Constants are used in test vectors. */
>> +};
>
> We try to avoid hard-coded ID lists like these in the Crypto API.

Just for my understanding: the problem here is having a (single) enum
for the representation of all the possible "known" groups in the first
place or more that the individual group id enum members have hard-coded
values assigned to them each?


> I've had a look at your subsequent patches and I don't think you
> really need this.
>
> For instance, instead of shoehorning this into "dh", you could
> instead create new kpp algorithms modpXXXX and ffdheXXXX which
> can be templates around the underlying dh algorithm.

FWIW, when implementing this, I considered aligning more to the ecdh
API, namely to register separate algorithms for each dh group as you
suggested above: "dh-ffdhe2048" etc. in analogy to "ecdh-nist-p192" and
alike.

The various (three in total) ecdh-nist-* kpp_alg variants differ only in
their ->init(), which would all set ->curve_id to the corresponding
ECC_CURVE_NIST_* constant as appropriate.

However, after some back and forth, I opted against doing something
similar for dh at the time, because there are quite some more possible
parameter sets than there are for ecdh, namely ten vs. three. If we were
to render the KEYCTL_DH_COMPUTE functionality unusable in FIPS mode
alltogether (see below), I could drop the MODP* group support, but that
would still leave me with the five FFDHE* kpp_alg variants I had to at
least provide separate test vectors for. I think that these five TVs
would be quite redundant as they'd all merely test the implementation of
dh_set_secret() + dh_compute_value() with different input
parameters. This might be acceptable though, I only wanted to bring it
up.


Anyway, just to make sure I'm getting it right: when you're saying
"template", you mean to implement a crypto_template for instantiating
patterns like "dh(ffdhe2048)", "dh(ffdhe3072)" and so on? The dh(...)
template instantiations would keep a crypto_spawn for referring to the
underlying, non-template "dh" kpp_alg so that "dh" implementations of
higher priority (hpre + qat) would take over once they'd become
available, correct?

AFAICS, it would make sense to introduce something like struct
kpp_instance, crypto_kpp_spawn and associated helpers as a prerequisite
then. Which wouldn't be a problem by me, just saying that it's not there
yet. I'm not sure there would be other potential users of such an
infrastructure, perhaps Stephan's SP800-108 KDF ([1]) is a candidate?


> Sure this might involve a copy of the parameters but given the speed
> of the algorithms that we're talking about here I don't think it's
> really relevant.

Ok, understood. I'm by no means a FIPS expert, but I bet I'd still have
to somehow convey a flag like "this group parameter P is approved" from
the frontend template instantiations like "dh(ffdhe2048)" to the
underlying "dh" implementation and make the latter reject any
non-approved groups. That is, with my limited experience with FIPS, I'd
expect that disabling the only known path to get non-approved parameters
into "dh", i.e. KEYCTL_DH_COMPUTE, would not be sufficient for achieving
conformance. Note that those dh_group_id's previously serialized via
crypto_dh_encode_key() served this purpose, in addition to enabling that
optimization of not copying the Ps when possible.

I'm not really sure, but simply introducing a flag like ->fips_approved
to struct dh and serializing that as an alternative would probably not
work out, because it would in theory still allow potential "dh" users to
set it (e.g. by accident) even when specifying non-approved Ps.

Stephan?


>
> That way the underlying drivers don't need to be touched at all.

FWIW, I touched the drivers only for consistency with ecdh and the
related drivers/crypto implementations, which all invoke the privkey
generation from their resp. ->set_secret().

As an alternative, I could have also made crypto_dh_encode_key() to
generate an ephemeral key on the fly for input ->key_size == 0. This
wouldn't be much different from how I'd imagine a dh(...) template based
approach to work: its ->set_secret() would take a private key, if any,
and
- generate a private key if none has been specified,
- kmalloc() a buffer for crypto_dh_encode_key(),
- serialize the key, P and G for the underlying "dh" implementation via
  crypto_dh_encode_key(),
- pass the encoded result onwards to the underlying "dh"'s
  ->set_secret() and
- kfree() the buffer again.

So instead of having the proposed template's ->set_secret() wrapper
around crypto_dh_encode_key() to take care of generating ephemeral keys,
I could have made the latter to do that as well, I think.


> Yes I do realise that this means the keyrings DH user-space API
> cannot be used in FIPS mode, but that is probably a good thing
> as users who care about modp/ffdhe shouldn't really have to stuff
> the raw vectors into this interface just to access the kernel DH
> implementation.

The sole purpose of introducing the MODP* parameters with this patchset
had been to keep KEYCTL_DH_COMPUTE functional in FIPS mode to the extent
possible: NVMe in-band authentication OTOH needs only FFHDE*. If it
would be acceptable or even desirable to render KEYCTL_DH_COMPUTE
unusable in FIPS mode, I'd drop the MODP* related patches.

However, it would perhaps be fair to reflect KEYCTL_DH_COMPUTE's new
dependency on !fips_enabled in keyctl_capabilities() then, but this can
probably be done with a separate follow-up patch.


>
> On a side note, are there really keyrings DH users out there in
> the wild? If not can we deprecate and remove this interface
> completely?

I wondered the same when first looking into this -- a web search
returned the Embedded Linux library ([2]), which seems to rely on
KEYCTL_DH_COMPUTE for implementing TLS (web servers for embedded
devices?). Then there's the keyctl(1) utility, which exposes this
interface via its "dh_compute" subcommand. Lastly, there exist some Rust
and Go bindings also -- hard to say if anything is using those.


Thanks!

Nicolai

[1] https://lore.kernel.org/r/4642773.OV4Wx5bFTl@positron.chronox.de
[2] https://git.kernel.org/pub/scm/libs/ell/ell.git/

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Herbert Xu]

On Mon, Dec 20, 2021 at 04:27:35PM +0100, Nicolai Stange wrote:
> 
> Just for my understanding: the problem here is having a (single) enum
> for the representation of all the possible "known" groups in the first
> place or more that the individual group id enum members have hard-coded
> values assigned to them each?

Yes the fact that you need to have a list of all "known" groups is
the issue.

> However, after some back and forth, I opted against doing something
> similar for dh at the time, because there are quite some more possible
> parameter sets than there are for ecdh, namely ten vs. three. If we were

I don't understand why we can't support ten or an even larger
number of parameter sets.

If you are concerned about code duplication then there are ways
around that.  Or do you have another specific concern in mind
with respect to a large number of parameter sets under this scheme?
 
> Anyway, just to make sure I'm getting it right: when you're saying
> "template", you mean to implement a crypto_template for instantiating
> patterns like "dh(ffdhe2048)", "dh(ffdhe3072)" and so on? The dh(...)
> template instantiations would keep a crypto_spawn for referring to the
> underlying, non-template "dh" kpp_alg so that "dh" implementations of
> higher priority (hpre + qat) would take over once they'd become
> available, correct?

The template would work in the other dirirection.  It would look
like ffdhe2048(dh) with dh being implemented in either software or
hardware.

The template wrapper would simply supply the relevant parameters.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Stephan Mueller]

Am Mittwoch, 29. Dezember 2021, 03:14:41 CET schrieb Herbert Xu:

Hi Herbert,

> On Mon, Dec 20, 2021 at 04:27:35PM +0100, Nicolai Stange wrote:
> > Just for my understanding: the problem here is having a (single) enum
> > for the representation of all the possible "known" groups in the first
> > place or more that the individual group id enum members have hard-coded
> > values assigned to them each?
> 
> Yes the fact that you need to have a list of all "known" groups is
> the issue.
> 
> > However, after some back and forth, I opted against doing something
> > similar for dh at the time, because there are quite some more possible
> > parameter sets than there are for ecdh, namely ten vs. three. If we were
> 
> I don't understand why we can't support ten or an even larger
> number of parameter sets.
> 
> If you are concerned about code duplication then there are ways
> around that.  Or do you have another specific concern in mind
> with respect to a large number of parameter sets under this scheme?
> 
> > Anyway, just to make sure I'm getting it right: when you're saying
> > "template", you mean to implement a crypto_template for instantiating
> > patterns like "dh(ffdhe2048)", "dh(ffdhe3072)" and so on? The dh(...)
> > template instantiations would keep a crypto_spawn for referring to the
> > underlying, non-template "dh" kpp_alg so that "dh" implementations of
> > higher priority (hpre + qat) would take over once they'd become
> > available, correct?
> 
> The template would work in the other dirirection.  It would look
> like ffdhe2048(dh) with dh being implemented in either software or
> hardware.
> 
> The template wrapper would simply supply the relevant parameters.

I fully agree with you. However, there is a small wrinkle we should consider. 
In FIPS mode, we must only allow DH together with the safe primes provided by 
the templates of ffdheXX and modpXX.

This means in FIPS mode, invoking the algo of "dh" should not be possible. 
Yet, on the other hand, we cannot mark "dh" as fips_allowed == 0 as the 
templates would not be able to instantiate them.

Therefore, I think we should mark "dh" as CRYPTO_ALG_INTERNAL if in FIPS mode. 
To do so, I would think that dh_init should contain something like:

if (fips_enabled)
	dh.base.cra_flags |= CRYPTO_ALG_INTERNAL;

When encapsulating this small flag setting code into a helper function (just 
like xts_check_key or xts_verify_key) this helper can be added to other / new 
DH implementations QAT to make them FIPS-compliant. This would be the same 
approach as we currently take for XTS where each XTS implementation must have 
a callback to xts_check_key.

Marking "dh" as INTERNAL implies it cannot be allocated in FIPS mode. Some may 
think this is a small inconsistency as this algo is marked fips_allowed == 1. 
I personally think there is no inconsistency because DH *is* allowed, however 
with a small policy caveat (the requirement to use it with safe-primes). So, 
having "dh" with fips_allowed == 1 but marking it as INTERNAL should be also 
consistent.

Yes, it is a small misuse of the INTERNAL flag. But the alternative would be 
to create a "__dh" implementation that is wrapped by "dh". In turn this 
implies a big churn as we would need to touch all drivers implementing "dh".

> 
> Cheers,


Ciao
Stephan

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Herbert Xu]

On Thu, Jan 06, 2022 at 03:30:04PM +0100, Stephan Mueller wrote:
>
> This means in FIPS mode, invoking the algo of "dh" should not be possible. 
> Yet, on the other hand, we cannot mark "dh" as fips_allowed == 0 as the 
> templates would not be able to instantiate them.

Right, we have exactly the same problem with sha1 where sha1
per se should be not be allowed in FIPS mode but hmac(sha1)
should be.

> Therefore, I think we should mark "dh" as CRYPTO_ALG_INTERNAL if in FIPS mode. 
I think the annotation should be added to testmgr.c.  We could
mark dh and sha1 as not fips_allowed but allowed as the parameter
of a template.  This could then be represented in the crypto_alg
object by a new flag.

This flag could then be set automatically in crypto_grab_* to
allow them to be picked up automatically for templates.

I'm already writing this up for sha1 anyway so let me polish it
off and I'll post it soon which you can then reuse it for dh.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Nicolai Stange]

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Thu, Jan 06, 2022 at 03:30:04PM +0100, Stephan Mueller wrote:
>>
>> This means in FIPS mode, invoking the algo of "dh" should not be possible. 
>> Yet, on the other hand, we cannot mark "dh" as fips_allowed == 0 as the 
>> templates would not be able to instantiate them.
>
> Right, we have exactly the same problem with sha1 where sha1
> per se should be not be allowed in FIPS mode but hmac(sha1)
> should be.
>
>> Therefore, I think we should mark "dh" as CRYPTO_ALG_INTERNAL if in FIPS mode. 
> I think the annotation should be added to testmgr.c.  We could
> mark dh and sha1 as not fips_allowed but allowed as the parameter
> of a template.  This could then be represented in the crypto_alg
> object by a new flag.
>
> This flag could then be set automatically in crypto_grab_* to
> allow them to be picked up automatically for templates.
>
> I'm already writing this up for sha1 anyway so let me polish it
> off and I'll post it soon which you can then reuse it for dh.

Perfect, this will solve my problem with how to handle "dh"
vs. fips_enabled quite nicely.

Many thanks!

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

[PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Herbert Xu]

On Fri, Jan 07, 2022 at 01:44:34PM +1100, Herbert Xu wrote:
>
> I'm already writing this up for sha1 anyway so let me polish it
> off and I'll post it soon which you can then reuse it for dh.

Here is something that seems to work for sha1/hmac.  Please let
me know if you see any issues with this approach for dh.

Thanks,

---8<---
Currently we do not distinguish between algorithms that fail on
the self-test vs. those which are disabled in FIPS mode (not allowed).
Both are marked as having failed the self-test.

As it has been requested that we need to disable sha1 in FIPS
mode while still allowing hmac(sha1) this approach needs to change.

This patch allows this scenario by adding a new flag FIPS_INTERNAL
to indicate those algorithms that have passed the self-test and are
not FIPS-allowed.  They can then be used for the self-testing of
other algorithms or by those that are explicitly allowed to use them
(currently just hmac).

Note that as a side-effect of this patch algorithms which are not
FIPS-allowed will now return ENOENT instead of ELIBBAD.  Hopefully
this is not an issue as some people were relying on this already.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/algapi.c b/crypto/algapi.c
index a366cb3e8aa1..649731856843 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -322,7 +322,13 @@ void crypto_alg_tested(const char *name, int err)
 found:
 	q->cra_flags |= CRYPTO_ALG_DEAD;
 	alg = test->adult;
-	if (err || list_empty(&alg->cra_list))
+
+	if (list_empty(&alg->cra_list))
+		goto complete;
+
+	if (err == -ECANCELED)
+		alg->cra_flags |= CRYPTO_ALG_FIPS_INTERNAL;
+	else if (err)
 		goto complete;
 
 	alg->cra_flags |= CRYPTO_ALG_TESTED;
diff --git a/crypto/api.c b/crypto/api.c
index cf0869dd130b..4c343585d4ea 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -314,6 +314,10 @@ struct crypto_alg *crypto_alg_mod_lookup(const char *name, u32 type, u32 mask)
 	if (!((type | mask) & CRYPTO_ALG_INTERNAL))
 		mask |= CRYPTO_ALG_INTERNAL;
 
+	/* Ditto for FIPS_INTERNAL. */
+	if (!((type | mask) & CRYPTO_ALG_FIPS_INTERNAL))
+		mask |= CRYPTO_ALG_FIPS_INTERNAL;
+
 	larval = crypto_larval_lookup(name, type, mask);
 	if (IS_ERR(larval) || !crypto_is_larval(larval))
 		return larval;
diff --git a/crypto/hmac.c b/crypto/hmac.c
index 25856aa7ccbf..fea669310f52 100644
--- a/crypto/hmac.c
+++ b/crypto/hmac.c
@@ -169,6 +169,7 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 	struct crypto_alg *alg;
 	struct shash_alg *salg;
 	u32 mask;
+	u32 type;
 	int err;
 	int ds;
 	int ss;
@@ -182,8 +183,9 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 		return -ENOMEM;
 	spawn = shash_instance_ctx(inst);
 
+	type = CRYPTO_ALG_FIPS_INTERNAL;
 	err = crypto_grab_shash(spawn, shash_crypto_instance(inst),
-				crypto_attr_alg_name(tb[1]), 0, mask);
+				crypto_attr_alg_name(tb[1]), type, mask);
 	if (err)
 		goto err_free_inst;
 	salg = crypto_spawn_shash_alg(spawn);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 5831d4bbc64f..6e2221a492e8 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1664,7 +1664,8 @@ static int test_hash_vs_generic_impl(const char *generic_driver,
 	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
 		return 0;
 
-	generic_tfm = crypto_alloc_shash(generic_driver, 0, 0);
+	generic_tfm = crypto_alloc_shash(generic_driver,
+					 CRYPTO_ALG_FIPS_INTERNAL, 0);
 	if (IS_ERR(generic_tfm)) {
 		err = PTR_ERR(generic_tfm);
 		if (err == -ENOENT) {
@@ -2387,7 +2388,8 @@ static int test_aead_vs_generic_impl(struct aead_extra_tests_ctx *ctx)
 	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
 		return 0;
 
-	generic_tfm = crypto_alloc_aead(generic_driver, 0, 0);
+	generic_tfm = crypto_alloc_aead(generic_driver,
+					CRYPTO_ALG_FIPS_INTERNAL, 0);
 	if (IS_ERR(generic_tfm)) {
 		err = PTR_ERR(generic_tfm);
 		if (err == -ENOENT) {
@@ -2986,7 +2988,8 @@ static int test_skcipher_vs_generic_impl(const char *generic_driver,
 	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
 		return 0;
 
-	generic_tfm = crypto_alloc_skcipher(generic_driver, 0, 0);
+	generic_tfm = crypto_alloc_skcipher(generic_driver,
+					    CRYPTO_ALG_FIPS_INTERNAL, 0);
 	if (IS_ERR(generic_tfm)) {
 		err = PTR_ERR(generic_tfm);
 		if (err == -ENOENT) {
@@ -5328,7 +5331,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 	}, {
 		.alg = "sha1",
 		.test = alg_test_hash,
-		.fips_allowed = 1,
 		.suite = {
 			.hash = __VECS(sha1_tv_template)
 		}
@@ -5637,9 +5639,6 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 		if (i < 0)
 			goto notest;
 
-		if (fips_enabled && !alg_test_descs[i].fips_allowed)
-			goto non_fips_alg;
-
 		rc = alg_test_cipher(alg_test_descs + i, driver, type, mask);
 		goto test_done;
 	}
@@ -5649,8 +5648,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 	if (i < 0 && j < 0)
 		goto notest;
 
-	if (fips_enabled && ((i >= 0 && !alg_test_descs[i].fips_allowed) ||
-			     (j >= 0 && !alg_test_descs[j].fips_allowed)))
+	if (fips_enabled && (j >= 0 && !alg_test_descs[j].fips_allowed))
 		goto non_fips_alg;
 
 	rc = 0;
@@ -5671,10 +5669,15 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 		}
 		WARN(1, "alg: self-tests for %s (%s) failed (rc=%d)",
 		     driver, alg, rc);
-	} else {
-		if (fips_enabled)
-			pr_info("alg: self-tests for %s (%s) passed\n",
+	} else if (fips_enabled) {
+		pr_info("alg: self-tests for %s (%s) passed\n",
+			driver, alg);
+
+		if (i >= 0 && !alg_test_descs[i].fips_allowed) {
+			pr_info("alg: %s (%s) is disabled due to FIPS\n",
 				driver, alg);
+			rc = -ECANCELED;
+		}
 	}
 
 	return rc;
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 855869e1fd32..df3f68dfe8c7 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -132,6 +132,15 @@
  */
 #define CRYPTO_ALG_ALLOCATES_MEMORY	0x00010000
 
+/*
+ * Mark an algorithm as a service implementation only usable by a
+ * template and never by a normal user of the kernel crypto API.
+ * This is intended to be used by algorithms that are themselves
+ * not FIPS-approved but may instead be used to implement parts of
+ * a FIPS-approved algorithm (e.g., sha1 vs. hmac(sha1)).
+ */
+#define CRYPTO_ALG_FIPS_INTERNAL	0x00020000
+
 /*
  * Transform masks and values (for crt_flags).
  */
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Nicolai Stange]

Hi Herbert,

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Fri, Jan 07, 2022 at 01:44:34PM +1100, Herbert Xu wrote:
>>
>> I'm already writing this up for sha1 anyway so let me polish it
>> off and I'll post it soon which you can then reuse it for dh.
>
> Here is something that seems to work for sha1/hmac.  Please let
> me know if you see any issues with this approach for dh.
>
> Thanks,
>
> ---8<---
> Currently we do not distinguish between algorithms that fail on
> the self-test vs. those which are disabled in FIPS mode (not allowed).
> Both are marked as having failed the self-test.
>
> As it has been requested that we need to disable sha1 in FIPS
> mode while still allowing hmac(sha1) this approach needs to change.
>
> This patch allows this scenario by adding a new flag FIPS_INTERNAL
> to indicate those algorithms that have passed the self-test and are
> not FIPS-allowed.  They can then be used for the self-testing of
> other algorithms or by those that are explicitly allowed to use them
> (currently just hmac).

I haven't tried, but wouldn't this allow the instantiation of e.g.
hmac(blake2s-256) in FIPS mode?

Thanks,

Nicolai

>
> Note that as a side-effect of this patch algorithms which are not
> FIPS-allowed will now return ENOENT instead of ELIBBAD.  Hopefully
> this is not an issue as some people were relying on this already.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

Re: [PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Herbert Xu]

On Tue, Jan 11, 2022 at 08:50:18AM +0100, Nicolai Stange wrote:
>
> I haven't tried, but wouldn't this allow the instantiation of e.g.
> hmac(blake2s-256) in FIPS mode?

You're right.  The real issue is that any algorithm with no tests
at all is allowed in FIPS mode.  That's clearly suboptimal.  But we
can't just ban every unknown algorithm because we rely on that
to let things like echainiv through.

Let me figure out a way to differentiate these two cases.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Herbert Xu]

On Tue, Jan 11, 2022 at 09:34:19PM +1100, Herbert Xu wrote:
>
> You're right.  The real issue is that any algorithm with no tests
> at all is allowed in FIPS mode.  That's clearly suboptimal.  But we
> can't just ban every unknown algorithm because we rely on that
> to let things like echainiv through.
> 
> Let me figure out a way to differentiate these two cases.

So what I've done is modify hmac so that if the underlying algo
is FIPS_INTERNAL, then we pre-emptively set FIPS_INTERNAL on the
hmac side as well.  This can then be cleared if the hmac algorithm
is explicitly listed as fips_allowed.

---8<---
Currently we do not distinguish between algorithms that fail on
the self-test vs. those which are disabled in FIPS mode (not allowed).
Both are marked as having failed the self-test.

As it has been requested that we need to disable sha1 in FIPS
mode while still allowing hmac(sha1) this approach needs to change.

This patch allows this scenario by adding a new flag FIPS_INTERNAL
to indicate those algorithms that have passed the self-test and are
not FIPS-allowed.  They can then be used for the self-testing of
other algorithms or by those that are explicitly allowed to use them
(currently just hmac).

Note that as a side-effect of this patch algorithms which are not
FIPS-allowed will now return ENOENT instead of ELIBBAD.  Hopefully
this is not an issue as some people were relying on this already.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/algapi.c b/crypto/algapi.c
index a366cb3e8aa1..09fb75806e87 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -322,8 +322,16 @@ void crypto_alg_tested(const char *name, int err)
 found:
 	q->cra_flags |= CRYPTO_ALG_DEAD;
 	alg = test->adult;
-	if (err || list_empty(&alg->cra_list))
+
+	if (list_empty(&alg->cra_list))
+		goto complete;
+
+	if (err == -ECANCELED)
+		alg->cra_flags |= CRYPTO_ALG_FIPS_INTERNAL;
+	else if (err)
 		goto complete;
+	else
+		alg->cra_flags &= ~CRYPTO_ALG_FIPS_INTERNAL;
 
 	alg->cra_flags |= CRYPTO_ALG_TESTED;
 
diff --git a/crypto/api.c b/crypto/api.c
index cf0869dd130b..549f9aced1da 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -223,6 +223,8 @@ static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg)
 	else if (crypto_is_test_larval(larval) &&
 		 !(alg->cra_flags & CRYPTO_ALG_TESTED))
 		alg = ERR_PTR(-EAGAIN);
+	else if (alg->cra_flags & CRYPTO_ALG_FIPS_INTERNAL)
+		alg = ERR_PTR(-EAGAIN);
 	else if (!crypto_mod_get(alg))
 		alg = ERR_PTR(-EAGAIN);
 	crypto_mod_put(&larval->alg);
@@ -233,6 +235,7 @@ static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg)
 static struct crypto_alg *crypto_alg_lookup(const char *name, u32 type,
 					    u32 mask)
 {
+	const u32 fips = CRYPTO_ALG_FIPS_INTERNAL;
 	struct crypto_alg *alg;
 	u32 test = 0;
 
@@ -240,8 +243,20 @@ static struct crypto_alg *crypto_alg_lookup(const char *name, u32 type,
 		test |= CRYPTO_ALG_TESTED;
 
 	down_read(&crypto_alg_sem);
-	alg = __crypto_alg_lookup(name, type | test, mask | test);
-	if (!alg && test) {
+	alg = __crypto_alg_lookup(name, (type | test) & ~fips,
+				  (mask | test) & ~fips);
+	if (alg) {
+		if (((type | mask) ^ fips) & fips)
+			mask |= fips;
+		mask &= fips;
+
+		if (!crypto_is_larval(alg) &&
+		    ((type ^ alg->cra_flags) & mask)) {
+			/* Algorithm is disallowed in FIPS mode. */
+			crypto_mod_put(alg);
+			alg = ERR_PTR(-ENOENT);
+		}
+	} else if (test) {
 		alg = __crypto_alg_lookup(name, type, mask);
 		if (alg && !crypto_is_larval(alg)) {
 			/* Test failed */
diff --git a/crypto/hmac.c b/crypto/hmac.c
index 25856aa7ccbf..af82e3eeb7d0 100644
--- a/crypto/hmac.c
+++ b/crypto/hmac.c
@@ -169,6 +169,7 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 	struct crypto_alg *alg;
 	struct shash_alg *salg;
 	u32 mask;
+	u32 type;
 	int err;
 	int ds;
 	int ss;
@@ -182,8 +183,9 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 		return -ENOMEM;
 	spawn = shash_instance_ctx(inst);
 
+	type = CRYPTO_ALG_FIPS_INTERNAL;
 	err = crypto_grab_shash(spawn, shash_crypto_instance(inst),
-				crypto_attr_alg_name(tb[1]), 0, mask);
+				crypto_attr_alg_name(tb[1]), type, mask);
 	if (err)
 		goto err_free_inst;
 	salg = crypto_spawn_shash_alg(spawn);
@@ -204,6 +206,7 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 	if (err)
 		goto err_free_inst;
 
+	inst->alg.base.cra_flags = alg->cra_flags & CRYPTO_ALG_FIPS_INTERNAL;
 	inst->alg.base.cra_priority = alg->cra_priority;
 	inst->alg.base.cra_blocksize = alg->cra_blocksize;
 	inst->alg.base.cra_alignmask = alg->cra_alignmask;
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 5831d4bbc64f..995d44db6154 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1664,7 +1664,8 @@ static int test_hash_vs_generic_impl(const char *generic_driver,
 	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
 		return 0;
 
-	generic_tfm = crypto_alloc_shash(generic_driver, 0, 0);
+	generic_tfm = crypto_alloc_shash(generic_driver,
+					 CRYPTO_ALG_FIPS_INTERNAL, 0);
 	if (IS_ERR(generic_tfm)) {
 		err = PTR_ERR(generic_tfm);
 		if (err == -ENOENT) {
@@ -2387,7 +2388,8 @@ static int test_aead_vs_generic_impl(struct aead_extra_tests_ctx *ctx)
 	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
 		return 0;
 
-	generic_tfm = crypto_alloc_aead(generic_driver, 0, 0);
+	generic_tfm = crypto_alloc_aead(generic_driver,
+					CRYPTO_ALG_FIPS_INTERNAL, 0);
 	if (IS_ERR(generic_tfm)) {
 		err = PTR_ERR(generic_tfm);
 		if (err == -ENOENT) {
@@ -2986,7 +2988,8 @@ static int test_skcipher_vs_generic_impl(const char *generic_driver,
 	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
 		return 0;
 
-	generic_tfm = crypto_alloc_skcipher(generic_driver, 0, 0);
+	generic_tfm = crypto_alloc_skcipher(generic_driver,
+					    CRYPTO_ALG_FIPS_INTERNAL, 0);
 	if (IS_ERR(generic_tfm)) {
 		err = PTR_ERR(generic_tfm);
 		if (err == -ENOENT) {
@@ -5328,7 +5331,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 	}, {
 		.alg = "sha1",
 		.test = alg_test_hash,
-		.fips_allowed = 1,
 		.suite = {
 			.hash = __VECS(sha1_tv_template)
 		}
@@ -5613,6 +5615,13 @@ static int alg_find_test(const char *alg)
 	return -1;
 }
 
+static int alg_fips_disabled(const char *driver, const char *alg)
+{
+	pr_info("alg: %s (%s) is disabled due to FIPS\n", alg, driver);
+
+	return -ECANCELED;
+}
+
 int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 {
 	int i;
@@ -5637,9 +5646,6 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 		if (i < 0)
 			goto notest;
 
-		if (fips_enabled && !alg_test_descs[i].fips_allowed)
-			goto non_fips_alg;
-
 		rc = alg_test_cipher(alg_test_descs + i, driver, type, mask);
 		goto test_done;
 	}
@@ -5649,8 +5655,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 	if (i < 0 && j < 0)
 		goto notest;
 
-	if (fips_enabled && ((i >= 0 && !alg_test_descs[i].fips_allowed) ||
-			     (j >= 0 && !alg_test_descs[j].fips_allowed)))
+	if (fips_enabled && (j >= 0 && !alg_test_descs[j].fips_allowed))
 		goto non_fips_alg;
 
 	rc = 0;
@@ -5671,16 +5676,22 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 		}
 		WARN(1, "alg: self-tests for %s (%s) failed (rc=%d)",
 		     driver, alg, rc);
-	} else {
-		if (fips_enabled)
-			pr_info("alg: self-tests for %s (%s) passed\n",
-				driver, alg);
+	} else if (fips_enabled) {
+		pr_info("alg: self-tests for %s (%s) passed\n",
+			driver, alg);
+
+		if (i >= 0 && !alg_test_descs[i].fips_allowed)
+			rc = alg_fips_disabled(driver, alg);
 	}
 
 	return rc;
 
 notest:
 	printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);
+
+	if (type & CRYPTO_ALG_FIPS_INTERNAL)
+		return alg_fips_disabled(driver, alg);
+
 	return 0;
 non_fips_alg:
 	return -EINVAL;
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 855869e1fd32..df3f68dfe8c7 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -132,6 +132,15 @@
  */
 #define CRYPTO_ALG_ALLOCATES_MEMORY	0x00010000
 
+/*
+ * Mark an algorithm as a service implementation only usable by a
+ * template and never by a normal user of the kernel crypto API.
+ * This is intended to be used by algorithms that are themselves
+ * not FIPS-approved but may instead be used to implement parts of
+ * a FIPS-approved algorithm (e.g., sha1 vs. hmac(sha1)).
+ */
+#define CRYPTO_ALG_FIPS_INTERNAL	0x00020000
+
 /*
  * Transform masks and values (for crt_flags).
  */
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Nicolai Stange]

Hi Herbert,

many thanks for this, I think this approach can be applied as-is to the
ffdheXYZ(dh) situation. I have some questions/comments inline.

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Tue, Jan 11, 2022 at 09:34:19PM +1100, Herbert Xu wrote:
>>
>> You're right.  The real issue is that any algorithm with no tests
>> at all is allowed in FIPS mode.  That's clearly suboptimal.  But we
>> can't just ban every unknown algorithm because we rely on that
>> to let things like echainiv through.
>> 
>> Let me figure out a way to differentiate these two cases.
>
> So what I've done is modify hmac so that if the underlying algo
> is FIPS_INTERNAL, then we pre-emptively set FIPS_INTERNAL on the
> hmac side as well.  This can then be cleared if the hmac algorithm
> is explicitly listed as fips_allowed.

I wonder whether this can be made more generic. I.e. would it be possible
to make crypto_grab_spawn() to or in FIPS_INTERNAL into type (iff !(mask
& FIPS_INTERNAL)) and to make crypto_register_instance() to propagate
FIPS_INTERNAL from the template instance's spawns upwards into the
instance's ->cra_flags?


> ---8<---
> Currently we do not distinguish between algorithms that fail on
> the self-test vs. those which are disabled in FIPS mode (not allowed).
> Both are marked as having failed the self-test.
>
> As it has been requested that we need to disable sha1 in FIPS
> mode while still allowing hmac(sha1) this approach needs to change.

On an unrelated note, this will break trusted_key_tpm_ops->init() in
FIPS mode, because trusted_shash_alloc() would fail to get a hold of
sha1. AFAICT, this could potentially make the init_trusted() module_init
to fail, and, as encrypted-keys.ko imports key_type_trusted, prevent the
loading of that one as well. Not sure that's desired...


> This patch allows this scenario by adding a new flag FIPS_INTERNAL
> to indicate those algorithms that have passed the self-test and are
> not FIPS-allowed.  They can then be used for the self-testing of
> other algorithms or by those that are explicitly allowed to use them
> (currently just hmac).
>
> Note that as a side-effect of this patch algorithms which are not
> FIPS-allowed will now return ENOENT instead of ELIBBAD.  Hopefully
> this is not an issue as some people were relying on this already.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/crypto/algapi.c b/crypto/algapi.c
> index a366cb3e8aa1..09fb75806e87 100644
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -322,8 +322,16 @@ void crypto_alg_tested(const char *name, int err)
>  found:
>  	q->cra_flags |= CRYPTO_ALG_DEAD;
>  	alg = test->adult;
> -	if (err || list_empty(&alg->cra_list))
> +
> +	if (list_empty(&alg->cra_list))
> +		goto complete;
> +
> +	if (err == -ECANCELED)
> +		alg->cra_flags |= CRYPTO_ALG_FIPS_INTERNAL;
> +	else if (err)
>  		goto complete;
> +	else
> +		alg->cra_flags &= ~CRYPTO_ALG_FIPS_INTERNAL;
>  
>  	alg->cra_flags |= CRYPTO_ALG_TESTED;
>  
> diff --git a/crypto/api.c b/crypto/api.c
> index cf0869dd130b..549f9aced1da 100644
> --- a/crypto/api.c
> +++ b/crypto/api.c
> @@ -223,6 +223,8 @@ static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg)
>  	else if (crypto_is_test_larval(larval) &&
>  		 !(alg->cra_flags & CRYPTO_ALG_TESTED))
>  		alg = ERR_PTR(-EAGAIN);
> +	else if (alg->cra_flags & CRYPTO_ALG_FIPS_INTERNAL)
> +		alg = ERR_PTR(-EAGAIN);

I might be mistaken, but I think this would cause hmac(sha1)
instantiation to fail if sha1 is not already there. I.e. if hmac(sha1)
instantiation would load sha1, then it would invoke crypto_larval_wait()
on the sha1 larval, see the FIPS_INTERNAL and fail?

However, wouldn't it be possible to simply implement FIPS_INTERNAL
lookups in analogy to the INTERNAL ones instead? That is, would adding

	if (!((type | mask) & CRYPTO_ALG_FIPS_INTERNAL))
		mask |= CRYPTO_ALG_FIPS_INTERNAL;

to the preamble of crypto_alg_mod_lookup() work instead?

AFAICS, ...


>  	else if (!crypto_mod_get(alg))
>  		alg = ERR_PTR(-EAGAIN);
>  	crypto_mod_put(&larval->alg);
> @@ -233,6 +235,7 @@ static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg)
>  static struct crypto_alg *crypto_alg_lookup(const char *name, u32 type,
>  					    u32 mask)
>  {
> +	const u32 fips = CRYPTO_ALG_FIPS_INTERNAL;
>  	struct crypto_alg *alg;
>  	u32 test = 0;
>  
> @@ -240,8 +243,20 @@ static struct crypto_alg *crypto_alg_lookup(const char *name, u32 type,
>  		test |= CRYPTO_ALG_TESTED;
>  
>  	down_read(&crypto_alg_sem);
> -	alg = __crypto_alg_lookup(name, type | test, mask | test);
> -	if (!alg && test) {
> +	alg = __crypto_alg_lookup(name, (type | test) & ~fips,
> +				  (mask | test) & ~fips);
> +	if (alg) {
> +		if (((type | mask) ^ fips) & fips)
> +			mask |= fips;
> +		mask &= fips;
> +
> +		if (!crypto_is_larval(alg) &&
> +		    ((type ^ alg->cra_flags) & mask)) {
> +			/* Algorithm is disallowed in FIPS mode. */
> +			crypto_mod_put(alg);
> +			alg = ERR_PTR(-ENOENT);
> +		}
> +	} else if (test) {

... this check here would not be needed anymore then? But I might be
missing something.


>  		alg = __crypto_alg_lookup(name, type, mask);
>  		if (alg && !crypto_is_larval(alg)) {
>  			/* Test failed */
> diff --git a/crypto/hmac.c b/crypto/hmac.c
> index 25856aa7ccbf..af82e3eeb7d0 100644
> --- a/crypto/hmac.c
> +++ b/crypto/hmac.c
> @@ -169,6 +169,7 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
>  	struct crypto_alg *alg;
>  	struct shash_alg *salg;
>  	u32 mask;
> +	u32 type;
>  	int err;
>  	int ds;
>  	int ss;
> @@ -182,8 +183,9 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
>  		return -ENOMEM;
>  	spawn = shash_instance_ctx(inst);
>  
> +	type = CRYPTO_ALG_FIPS_INTERNAL;
>  	err = crypto_grab_shash(spawn, shash_crypto_instance(inst),
> -				crypto_attr_alg_name(tb[1]), 0, mask);
> +				crypto_attr_alg_name(tb[1]), type, mask);
>  	if (err)
>  		goto err_free_inst;
>  	salg = crypto_spawn_shash_alg(spawn);
> @@ -204,6 +206,7 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
>  	if (err)
>  		goto err_free_inst;
>  
> +	inst->alg.base.cra_flags = alg->cra_flags & CRYPTO_ALG_FIPS_INTERNAL;
>  	inst->alg.base.cra_priority = alg->cra_priority;
>  	inst->alg.base.cra_blocksize = alg->cra_blocksize;
>  	inst->alg.base.cra_alignmask = alg->cra_alignmask;
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index 5831d4bbc64f..995d44db6154 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -1664,7 +1664,8 @@ static int test_hash_vs_generic_impl(const char *generic_driver,
>  	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
>  		return 0;
>  
> -	generic_tfm = crypto_alloc_shash(generic_driver, 0, 0);
> +	generic_tfm = crypto_alloc_shash(generic_driver,
> +					 CRYPTO_ALG_FIPS_INTERNAL, 0);
>  	if (IS_ERR(generic_tfm)) {
>  		err = PTR_ERR(generic_tfm);
>  		if (err == -ENOENT) {
> @@ -2387,7 +2388,8 @@ static int test_aead_vs_generic_impl(struct aead_extra_tests_ctx *ctx)
>  	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
>  		return 0;
>  
> -	generic_tfm = crypto_alloc_aead(generic_driver, 0, 0);
> +	generic_tfm = crypto_alloc_aead(generic_driver,
> +					CRYPTO_ALG_FIPS_INTERNAL, 0);
>  	if (IS_ERR(generic_tfm)) {
>  		err = PTR_ERR(generic_tfm);
>  		if (err == -ENOENT) {
> @@ -2986,7 +2988,8 @@ static int test_skcipher_vs_generic_impl(const char *generic_driver,
>  	if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
>  		return 0;
>  
> -	generic_tfm = crypto_alloc_skcipher(generic_driver, 0, 0);
> +	generic_tfm = crypto_alloc_skcipher(generic_driver,
> +					    CRYPTO_ALG_FIPS_INTERNAL, 0);
>  	if (IS_ERR(generic_tfm)) {
>  		err = PTR_ERR(generic_tfm);
>  		if (err == -ENOENT) {
> @@ -5328,7 +5331,6 @@ static const struct alg_test_desc alg_test_descs[] = {
>  	}, {
>  		.alg = "sha1",
>  		.test = alg_test_hash,
> -		.fips_allowed = 1,
>  		.suite = {
>  			.hash = __VECS(sha1_tv_template)
>  		}
> @@ -5613,6 +5615,13 @@ static int alg_find_test(const char *alg)
>  	return -1;
>  }
>  
> +static int alg_fips_disabled(const char *driver, const char *alg)
> +{
> +	pr_info("alg: %s (%s) is disabled due to FIPS\n", alg, driver);
> +
> +	return -ECANCELED;
> +}
> +
>  int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
>  {
>  	int i;
> @@ -5637,9 +5646,6 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
>  		if (i < 0)
>  			goto notest;
>  
> -		if (fips_enabled && !alg_test_descs[i].fips_allowed)
> -			goto non_fips_alg;
> -
>  		rc = alg_test_cipher(alg_test_descs + i, driver, type, mask);
>  		goto test_done;
>  	}
> @@ -5649,8 +5655,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
>  	if (i < 0 && j < 0)
>  		goto notest;
>  
> -	if (fips_enabled && ((i >= 0 && !alg_test_descs[i].fips_allowed) ||
> -			     (j >= 0 && !alg_test_descs[j].fips_allowed)))
> +	if (fips_enabled && (j >= 0 && !alg_test_descs[j].fips_allowed))
>  		goto non_fips_alg;
>  
>  	rc = 0;
> @@ -5671,16 +5676,22 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
>  		}
>  		WARN(1, "alg: self-tests for %s (%s) failed (rc=%d)",
>  		     driver, alg, rc);
> -	} else {
> -		if (fips_enabled)
> -			pr_info("alg: self-tests for %s (%s) passed\n",
> -				driver, alg);
> +	} else if (fips_enabled) {
> +		pr_info("alg: self-tests for %s (%s) passed\n",
> +			driver, alg);
> +
> +		if (i >= 0 && !alg_test_descs[i].fips_allowed)
> +			rc = alg_fips_disabled(driver, alg);
>  	}
>  
>  	return rc;
>  
>  notest:
>  	printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);
> +
> +	if (type & CRYPTO_ALG_FIPS_INTERNAL)
> +		return alg_fips_disabled(driver, alg);
> +
>  	return 0;
>  non_fips_alg:
>  	return -EINVAL;

This looks all good to me, but as !->fips_allowed tests aren't skipped
over anymore now, it would perhaps make sense to make their failure
non-fatal in FIPS mode. Because in FIPS mode a failure could mean a
panic and some of the existing TVs might not pass because of e.g. some
key length checks or so active only for fips_enabled...

Many thanks again!

Nicolai



> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> index 855869e1fd32..df3f68dfe8c7 100644
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -132,6 +132,15 @@
>   */
>  #define CRYPTO_ALG_ALLOCATES_MEMORY	0x00010000
>  
> +/*
> + * Mark an algorithm as a service implementation only usable by a
> + * template and never by a normal user of the kernel crypto API.
> + * This is intended to be used by algorithms that are themselves
> + * not FIPS-approved but may instead be used to implement parts of
> + * a FIPS-approved algorithm (e.g., sha1 vs. hmac(sha1)).
> + */
> +#define CRYPTO_ALG_FIPS_INTERNAL	0x00020000
> +
>  /*
>   * Transform masks and values (for crt_flags).
>   */

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Herbert Xu]

On Fri, Jan 14, 2022 at 10:09:02AM +0100, Nicolai Stange wrote:
>
> I wonder whether this can be made more generic. I.e. would it be possible
> to make crypto_grab_spawn() to or in FIPS_INTERNAL into type (iff !(mask
> & FIPS_INTERNAL)) and to make crypto_register_instance() to propagate
> FIPS_INTERNAL from the template instance's spawns upwards into the
> instance's ->cra_flags?

We could certainly do something like that in future.  But it
isn't that easy because crypto_register_instance doesn't know
what the paramter algorithm(s) is/are.

> On an unrelated note, this will break trusted_key_tpm_ops->init() in
> FIPS mode, because trusted_shash_alloc() would fail to get a hold of
> sha1. AFAICT, this could potentially make the init_trusted() module_init
> to fail, and, as encrypted-keys.ko imports key_type_trusted, prevent the
> loading of that one as well. Not sure that's desired...

Well if sha1 is supposed to be forbidden in FIPS mode why should
TPM be allowed to use it? If it must be allowed, then we could
change TPM to set the FIPS_INTERNAL flag.

I think I'll simply leave out the line that actually disables sha1
for now.

> > diff --git a/crypto/api.c b/crypto/api.c
> > index cf0869dd130b..549f9aced1da 100644
> > --- a/crypto/api.c
> > +++ b/crypto/api.c
> > @@ -223,6 +223,8 @@ static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg)
> >  	else if (crypto_is_test_larval(larval) &&
> >  		 !(alg->cra_flags & CRYPTO_ALG_TESTED))
> >  		alg = ERR_PTR(-EAGAIN);
> > +	else if (alg->cra_flags & CRYPTO_ALG_FIPS_INTERNAL)
> > +		alg = ERR_PTR(-EAGAIN);
> 
> I might be mistaken, but I think this would cause hmac(sha1)
> instantiation to fail if sha1 is not already there. I.e. if hmac(sha1)
> instantiation would load sha1, then it would invoke crypto_larval_wait()
> on the sha1 larval, see the FIPS_INTERNAL and fail?

When EAGAIN is returned the lookup is automatically retried.

> However, wouldn't it be possible to simply implement FIPS_INTERNAL
> lookups in analogy to the INTERNAL ones instead? That is, would adding
> 
> 	if (!((type | mask) & CRYPTO_ALG_FIPS_INTERNAL))
> 		mask |= CRYPTO_ALG_FIPS_INTERNAL;
> 
> to the preamble of crypto_alg_mod_lookup() work instead?

If you do that then you will end up creating an infinite number
of failed templates if you lookup something like hmac(md5).  Once
the first hmac(md5) is created it must then match all subsequent
lookups of hmac(md5) in order to prevent any further creation.

> This looks all good to me, but as !->fips_allowed tests aren't skipped
> over anymore now, it would perhaps make sense to make their failure
> non-fatal in FIPS mode. Because in FIPS mode a failure could mean a
> panic and some of the existing TVs might not pass because of e.g. some
> key length checks or so active only for fips_enabled...

You mean a buggy non-FIPS algorithm that fails when tested in
FIPS mode?  I guess we could skip the panic in that case if
everyone is happy with that.  Stephan?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Nicolai Stange]

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Fri, Jan 14, 2022 at 10:09:02AM +0100, Nicolai Stange wrote:
>>
>> I wonder whether this can be made more generic. I.e. would it be possible
>> to make crypto_grab_spawn() to or in FIPS_INTERNAL into type (iff !(mask
>> & FIPS_INTERNAL)) and to make crypto_register_instance() to propagate
>> FIPS_INTERNAL from the template instance's spawns upwards into the
>> instance's ->cra_flags?
>
> We could certainly do something like that in future.  But it
> isn't that easy because crypto_register_instance doesn't know
> what the paramter algorithm(s) is/are.

I was thinking of simply walking through the inst->spawns list for that.


>
>> On an unrelated note, this will break trusted_key_tpm_ops->init() in
>> FIPS mode, because trusted_shash_alloc() would fail to get a hold of
>> sha1. AFAICT, this could potentially make the init_trusted() module_init
>> to fail, and, as encrypted-keys.ko imports key_type_trusted, prevent the
>> loading of that one as well. Not sure that's desired...
>
> Well if sha1 is supposed to be forbidden in FIPS mode why should
> TPM be allowed to use it? 

Yes, I only wanted to point out that doing that could potentially have
unforseen consequences as it currently stands, like
e.g. encrypted-keys.ko loading failures, even though the latter doesn't
even seem to use sha1 by itself.

However, this scenario would be possible only for CONFIG_TRUSTED_KEYS=m,
CONFIG_TEE=n and tpm_default_chip() returning something.


> If it must be allowed, then we could change TPM to set the
> FIPS_INTERNAL flag.
>
> I think I'll simply leave out the line that actually disables sha1
> for now.
>
>> > diff --git a/crypto/api.c b/crypto/api.c
>> > index cf0869dd130b..549f9aced1da 100644
>> > --- a/crypto/api.c
>> > +++ b/crypto/api.c
>> > @@ -223,6 +223,8 @@ static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg)
>> >  	else if (crypto_is_test_larval(larval) &&
>> >  		 !(alg->cra_flags & CRYPTO_ALG_TESTED))
>> >  		alg = ERR_PTR(-EAGAIN);
>> > +	else if (alg->cra_flags & CRYPTO_ALG_FIPS_INTERNAL)
>> > +		alg = ERR_PTR(-EAGAIN);
>> 
>> I might be mistaken, but I think this would cause hmac(sha1)
>> instantiation to fail if sha1 is not already there. I.e. if hmac(sha1)
>> instantiation would load sha1, then it would invoke crypto_larval_wait()
>> on the sha1 larval, see the FIPS_INTERNAL and fail?
>
> When EAGAIN is returned the lookup is automatically retried.

Ah right, just found the loop in cryptomgr_probe().


>
>> However, wouldn't it be possible to simply implement FIPS_INTERNAL
>> lookups in analogy to the INTERNAL ones instead? That is, would adding
>> 
>> 	if (!((type | mask) & CRYPTO_ALG_FIPS_INTERNAL))
>> 		mask |= CRYPTO_ALG_FIPS_INTERNAL;
>> 
>> to the preamble of crypto_alg_mod_lookup() work instead?
>
> If you do that then you will end up creating an infinite number
> of failed templates if you lookup something like hmac(md5).  Once
> the first hmac(md5) is created it must then match all subsequent
> lookups of hmac(md5) in order to prevent any further creation.

Thanks for the explanation, it makes sense now.

>
>> This looks all good to me, but as !->fips_allowed tests aren't skipped
>> over anymore now, it would perhaps make sense to make their failure
>> non-fatal in FIPS mode. Because in FIPS mode a failure could mean a
>> panic and some of the existing TVs might not pass because of e.g. some
>> key length checks or so active only for fips_enabled...
>
> You mean a buggy non-FIPS algorithm that fails when tested in
> FIPS mode?

Yes, I can't tell how realistic that is though.


> I guess we could skip the panic in that case if everyone is happy with
> that.  Stephan?
>

Thanks,

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Stephan Mueller]

Am Freitag, 14. Januar 2022, 11:55:26 CET schrieb Herbert Xu:

Hi Herbert,

> > On an unrelated note, this will break trusted_key_tpm_ops->init() in
> > FIPS mode, because trusted_shash_alloc() would fail to get a hold of
> > sha1. AFAICT, this could potentially make the init_trusted() module_init
> > to fail, and, as encrypted-keys.ko imports key_type_trusted, prevent the
> > loading of that one as well. Not sure that's desired...
> 
> Well if sha1 is supposed to be forbidden in FIPS mode why should

SHA-1 is approved in all use cases except signatures.

Ciao
Stephan

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[James Bottomley]

On Fri, 2022-01-14 at 13:35 +0100, Stephan Mueller wrote:
> Am Freitag, 14. Januar 2022, 11:55:26 CET schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> > > On an unrelated note, this will break trusted_key_tpm_ops->init() 
> > > in FIPS mode, because trusted_shash_alloc() would fail to get a
> > > hold of sha1. AFAICT, this could potentially make the
> > > init_trusted() module_init to fail, and, as encrypted-keys.ko
> > > imports key_type_trusted, prevent the loading of that one as
> > > well. Not sure that's desired...
> > 
> > Well if sha1 is supposed to be forbidden in FIPS mode why should
> 
> SHA-1 is approved in all use cases except signatures.

Actually, even that's not quite true: you can't use it in a FIPS
compliant system to *generate* signatures, but you can still use it in
a FIPS compliant system to verify legacy signatures (signatures created
before sha-1 was deprecated).  It's still also completely acceptable as
a hash for HMAC.

The supporting document is this one:

https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf

The bottom line is removing SHA-1 to achieve "FIPS compliance" is the
wrong approach.  You just have to make sure you can never use it to
generate signatures.

James

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Stephan Mueller]

Am Freitag, 14. Januar 2022, 11:55:26 CET schrieb Herbert Xu:

Hi Herbert,

> 
> > This looks all good to me, but as !->fips_allowed tests aren't skipped
> > over anymore now, it would perhaps make sense to make their failure
> > non-fatal in FIPS mode. Because in FIPS mode a failure could mean a
> > panic and some of the existing TVs might not pass because of e.g. some
> > key length checks or so active only for fips_enabled...
> 
> You mean a buggy non-FIPS algorithm that fails when tested in
> FIPS mode?  I guess we could skip the panic in that case if
> everyone is happy with that.  Stephan?

As we consider FIPS 140-3, we can allow a "degrated mode of operation". A 
degraded mode of operation disables only the algorithm that caused the 
failure. With a failing self test and not having a panic(), the offending 
algorithm implementation will not be available to the kernel crypto API and 
thus to a user.

In this case, we can replace the panic with a graceful error.

If that change is applied, I would like to mention to anybody that wants to 
backport the change: this change is not appropriate for FIPS 140-2.

Ciao
Stephan

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Nicolai Stange]

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Fri, Jan 14, 2022 at 10:09:02AM +0100, Nicolai Stange wrote:
>
>> This looks all good to me, but as !->fips_allowed tests aren't skipped
>> over anymore now, it would perhaps make sense to make their failure
>> non-fatal in FIPS mode. Because in FIPS mode a failure could mean a
>> panic and some of the existing TVs might not pass because of e.g. some
>> key length checks or so active only for fips_enabled...
>
> You mean a buggy non-FIPS algorithm that fails when tested in
> FIPS mode?  I guess we could skip the panic in that case if
> everyone is happy with that.  Stephan?

One more thing I just realized: dracut's fips module ([1]) modprobes
tcrypt (*) and failure is considered fatal, i.e. the system would not
boot up.

First of all this would mean that tcrypt_test() needs to ignore
-ECANCELED return values from alg_test() in FIPS mode, in addition to
the -EINVAL it is already prepared for.

However, chances are that some of the !fips_allowed algorithms looped
over by tcrypt are not available (i.e. not enabled at build time) and as
this change here makes alg_test() to unconditionally attempt a test
execution now, this would fail with -ENOENT AFAICS.

One way to work around this is to make tcrypt_test() to ignore -ENOENT
in addition to -EINVAL and -ECANCELED.

It might be undesirable though that the test executions triggered from
tcrypt would still instantiate/load a ton of !fips_allowed algorithms at
boot, most of which will effectively be inaccessible (because they're
not used as FIPS_INTERNAL arguments to fips_allowed == 1 template
instances).

So how about making alg_test() to skip the !fips_allowed tests in FIPS
mode as before, but to return -ECANCELED and eventually set
FIPS_INTERNAL as implemented with this patch here.

This would imply that FIPS_INTERNAL algorithms by themselves remain
untested, but I think this might be Ok as they would be usable only as
template arguments in fips_allowed instantiations. That is, they will
still receive some form of testing when the larger construction they're
part of gets tested.

For example, going with the "dh" example, where "dh" and "ffdhe3072(dh)"
would have fips_allowed unset and set respecively, ffdhe3072(dh) as
a whole would get tested, but not the "dh" argument individually.

Stephan, would this approach work from a FIPS 140-3 perspective?

Thanks!

Nicolai

[1] https://git.kernel.org/pub/scm/boot/dracut/dracut.git/tree/modules.d/01fips/fips.sh#n106
(*) I'm not sure why this is being done, but it is what it is.

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Stephan Mueller]

Am Freitag, 28. Januar 2022, 15:14:39 CET schrieb Nicolai Stange:

Hi Nicolai,

> Herbert Xu <herbert@gondor.apana.org.au> writes:
> > On Fri, Jan 14, 2022 at 10:09:02AM +0100, Nicolai Stange wrote:
> >> This looks all good to me, but as !->fips_allowed tests aren't skipped
> >> over anymore now, it would perhaps make sense to make their failure
> >> non-fatal in FIPS mode. Because in FIPS mode a failure could mean a
> >> panic and some of the existing TVs might not pass because of e.g. some
> >> key length checks or so active only for fips_enabled...
> > 
> > You mean a buggy non-FIPS algorithm that fails when tested in
> > FIPS mode?  I guess we could skip the panic in that case if
> > everyone is happy with that.  Stephan?
> 
> One more thing I just realized: dracut's fips module ([1]) modprobes
> tcrypt (*) and failure is considered fatal, i.e. the system would not
> boot up.
> 
> First of all this would mean that tcrypt_test() needs to ignore
> -ECANCELED return values from alg_test() in FIPS mode, in addition to
> the -EINVAL it is already prepared for.
> 
> However, chances are that some of the !fips_allowed algorithms looped
> over by tcrypt are not available (i.e. not enabled at build time) and as
> this change here makes alg_test() to unconditionally attempt a test
> execution now, this would fail with -ENOENT AFAICS.
> 
> One way to work around this is to make tcrypt_test() to ignore -ENOENT
> in addition to -EINVAL and -ECANCELED.
> 
> It might be undesirable though that the test executions triggered from
> tcrypt would still instantiate/load a ton of !fips_allowed algorithms at
> boot, most of which will effectively be inaccessible (because they're
> not used as FIPS_INTERNAL arguments to fips_allowed == 1 template
> instances).
> 
> So how about making alg_test() to skip the !fips_allowed tests in FIPS
> mode as before, but to return -ECANCELED and eventually set
> FIPS_INTERNAL as implemented with this patch here.
> 
> This would imply that FIPS_INTERNAL algorithms by themselves remain
> untested, but I think this might be Ok as they would be usable only as
> template arguments in fips_allowed instantiations. That is, they will
> still receive some form of testing when the larger construction they're
> part of gets tested.
> 
> For example, going with the "dh" example, where "dh" and "ffdhe3072(dh)"
> would have fips_allowed unset and set respecively, ffdhe3072(dh) as
> a whole would get tested, but not the "dh" argument individually.
> 
> Stephan, would this approach work from a FIPS 140-3 perspective?

Are we sure that we always will have power-up tests of the compound algorithms 
when we disable the lower-level algorithm testing?

For example, consider the DH work you are preparing: we currently have a self 
test for dh - which then will be marked as FIPS_INTERNAL and not executed. 
Would we now have self tests for modpXXX(dh) or ffdheXXX(dh)? If not, how 
would it be guaranteed that DH is tested?

The important part is that the algorithm testing is guaranteed. I see a number 
of alg_test_null in testmgr.c. I see the potential that some algorithms do not 
get tested at all when we skip FIPS_INTERNAL algorithms.

From a FIPS perspective it is permissible that compound algo power up tests 
are claimed to cover respective lower-level algos.

> 
> Thanks!
> 
> Nicolai
> 
> [1]
> https://git.kernel.org/pub/scm/boot/dracut/dracut.git/tree/modules.d/01fips
> /fips.sh#n106 (*) I'm not sure why this is being done, but it is what it is.


Ciao
Stephan

Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Nicolai Stange]

Hi Stephan,

Stephan Mueller <smueller@chronox.de> writes:

> Am Freitag, 28. Januar 2022, 15:14:39 CET schrieb Nicolai Stange:
>
>> Herbert Xu <herbert@gondor.apana.org.au> writes:
>> > On Fri, Jan 14, 2022 at 10:09:02AM +0100, Nicolai Stange wrote:
>> >> This looks all good to me, but as !->fips_allowed tests aren't skipped
>> >> over anymore now, it would perhaps make sense to make their failure
>> >> non-fatal in FIPS mode. Because in FIPS mode a failure could mean a
>> >> panic and some of the existing TVs might not pass because of e.g. some
>> >> key length checks or so active only for fips_enabled...
>> > 
>> > You mean a buggy non-FIPS algorithm that fails when tested in
>> > FIPS mode?  I guess we could skip the panic in that case if
>> > everyone is happy with that.  Stephan?
>> 
>> One more thing I just realized: dracut's fips module ([1]) modprobes
>> tcrypt (*) and failure is considered fatal, i.e. the system would not
>> boot up.
>> 
>> First of all this would mean that tcrypt_test() needs to ignore
>> -ECANCELED return values from alg_test() in FIPS mode, in addition to
>> the -EINVAL it is already prepared for.
>> 
>> However, chances are that some of the !fips_allowed algorithms looped
>> over by tcrypt are not available (i.e. not enabled at build time) and as
>> this change here makes alg_test() to unconditionally attempt a test
>> execution now, this would fail with -ENOENT AFAICS.
>> 
>> One way to work around this is to make tcrypt_test() to ignore -ENOENT
>> in addition to -EINVAL and -ECANCELED.
>> 
>> It might be undesirable though that the test executions triggered from
>> tcrypt would still instantiate/load a ton of !fips_allowed algorithms at
>> boot, most of which will effectively be inaccessible (because they're
>> not used as FIPS_INTERNAL arguments to fips_allowed == 1 template
>> instances).
>> 
>> So how about making alg_test() to skip the !fips_allowed tests in FIPS
>> mode as before, but to return -ECANCELED and eventually set
>> FIPS_INTERNAL as implemented with this patch here.
>> 
>> This would imply that FIPS_INTERNAL algorithms by themselves remain
>> untested, but I think this might be Ok as they would be usable only as
>> template arguments in fips_allowed instantiations. That is, they will
>> still receive some form of testing when the larger construction they're
>> part of gets tested.
>> 
>> For example, going with the "dh" example, where "dh" and "ffdhe3072(dh)"
>> would have fips_allowed unset and set respecively, ffdhe3072(dh) as
>> a whole would get tested, but not the "dh" argument individually.
>> 
>> Stephan, would this approach work from a FIPS 140-3 perspective?
>
> Are we sure that we always will have power-up tests of the compound algorithms 
> when we disable the lower-level algorithm testing?

Yes. The compound algorithms having ->fips_allowed == 1 and alg_test_null
entries are:

  authenc(hmac(sha1),ctr(aes))
  authenc(hmac(sha1),rfc3686(ctr(aes)))
  authenc(hmac(sha256),ctr(aes))
  authenc(hmac(sha256),rfc3686(ctr(aes)))
  authenc(hmac(sha384),ctr(aes))
  authenc(hmac(sha384),rfc3686(ctr(aes)))
  authenc(hmac(sha512),ctr(aes))
  authenc(hmac(sha512),rfc3686(ctr(aes)))

The hmac(sha*), ctr(aes) and rfc3686(ctr(aes)) all have
->fips_allowed == 1 and proper non-alg_test_null test entries. So no
change here.

  cbc(paes)
  ctr(paes)
  ecb(paes)
  ofb(paes)
  xts(paes)
  xts4096(paes)
  xts512(paes)
  cts(cbc(paes))

As ecb(paes) has only a alg_test_null() entry, no test would have been
performed at all before this change for these. So no change here either.
  
  ecb(cipher_null)

No change here either.

  pkcs1pad(rsa,sha224)
  pkcs1pad(rsa,sha384)
  pkcs1pad(rsa,sha512)

The sha* and rsa all have ->fips_allowed == 1 and proper
non-alg_test_null test entries. So no change here.


>
> For example, consider the DH work you are preparing: we currently have a self 
> test for dh - which then will be marked as FIPS_INTERNAL and not executed. 
> Would we now have self tests for modpXXX(dh) or ffdheXXX(dh)?

Yes, exactly.

> If not, how would it be guaranteed that DH is tested?
>
> The important part is that the algorithm testing is guaranteed. I see a number 
> of alg_test_null in testmgr.c. I see the potential that some algorithms do not 
> get tested at all when we skip FIPS_INTERNAL algorithms.

See above. But of course one needs to be careful not to add
->fips_allowed + alg_test_null entries for compound algorithms where any
of the template arguments isn't approved in the future.


> From a FIPS perspective it is permissible that compound algo power up tests 
> are claimed to cover respective lower-level algos.

Perfect.

Thanks,

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups

[Nicolai Stange]

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Mon, Dec 20, 2021 at 04:27:35PM +0100, Nicolai Stange wrote:
>> 
>> Just for my understanding: the problem here is having a (single) enum
>> for the representation of all the possible "known" groups in the first
>> place or more that the individual group id enum members have hard-coded
>> values assigned to them each?
>
> Yes the fact that you need to have a list of all "known" groups is
> the issue.

Ok, understood. Thanks for the clarification.


>> However, after some back and forth, I opted against doing something
>> similar for dh at the time, because there are quite some more possible
>> parameter sets than there are for ecdh, namely ten vs. three. If we were
>
> I don't understand why we can't support ten or an even larger
> number of parameter sets.

There's no real reason. I just didn't dare to promote what I considered
mere input parameter sets to full-fledged crypto_alg instances with
their associated overhead each:
- the global crypto_alg_list will get longer, which might have an
  impact on the lookup searches,
- every ffdheXYZ(dh) template instance will need to have individual
  TVs associated with it.

However, I take it as that's fine and I'd be more than happy to
implement the ffhdheXYZ(dh) template approach you suggested in a v3.


>
> If you are concerned about code duplication then there are ways
> around that.  Or do you have another specific concern in mind
> with respect to a large number of parameter sets under this scheme?
>  
>> Anyway, just to make sure I'm getting it right: when you're saying
>> "template", you mean to implement a crypto_template for instantiating
>> patterns like "dh(ffdhe2048)", "dh(ffdhe3072)" and so on? The dh(...)
>> template instantiations would keep a crypto_spawn for referring to the
>> underlying, non-template "dh" kpp_alg so that "dh" implementations of
>> higher priority (hpre + qat) would take over once they'd become
>> available, correct?
>
> The template would work in the other dirirection.  It would look
> like ffdhe2048(dh) with dh being implemented in either software or
> hardware.
>
> The template wrapper would simply supply the relevant parameters.

Makes sense.

Thanks!

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

[PATCH v2 04/18] crypto: dh - introduce RFC 7919 safe-prime groups

[Nicolai Stange]

The FFDHE groups specified by RFC 7919 are needed for the current work
on NVME ([1]) and also among the safe-prime groups approved by
SP800-56Arev3. Make them known to the kernel.

More specifically, introduce corresponding members to enum dh_group_id
as well as entries with the resp. domain parameters to the
safe_prime_groups[] array queried by crypto_dh_decode_key(). The resp.
->max_strength value is set to the maximum supported security strength as
specified in SP800-56Arev3.

As the domain parameters consume an substantial amount of space, make
RFC 7919 safe-prime group support selectable by means of the new
CRYPTO_DH_GROUPS_RFC7919 Kconfig option.

[1] https://lkml.kernel.org/r/20211122074727.25988-4-hare@suse.de

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/Kconfig      |  11 ++-
 crypto/dh_helper.c  | 219 +++++++++++++++++++++++++++++++++++++++++++-
 include/crypto/dh.h |   7 ++
 3 files changed, 235 insertions(+), 2 deletions(-)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 285f82647d2b..0f039bbf36e2 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -224,13 +224,22 @@ config CRYPTO_RSA
 	help
 	  Generic implementation of the RSA public key algorithm.
 
-config CRYPTO_DH
+menuconfig CRYPTO_DH
 	tristate "Diffie-Hellman algorithm"
 	select CRYPTO_KPP
 	select MPILIB
 	help
 	  Generic implementation of the Diffie-Hellman algorithm.
 
+if CRYPTO_DH
+config CRYPTO_DH_GROUPS_RFC7919
+	bool "Support for RFC 7919 FFDHE group parameters"
+	help
+	  Enable to allow for the use of RFC 7919 DH parameters in FIPS mode,
+	  e.g. via keyctl(KEYCTL_DH_COMPUTE). Otherwise it's safe to say N.
+
+endif
+
 config CRYPTO_ECC
 	tristate
 	select CRYPTO_RNG_DEFAULT
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 9f21204e5dee..b6df9207bcfd 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -18,7 +18,224 @@ static const struct safe_prime_group
 	unsigned int max_strength;
 	unsigned int p_size;
 	const char *p;
-} safe_prime_groups[] = {};
+} safe_prime_groups[] = {
+#ifdef CONFIG_CRYPTO_DH_GROUPS_RFC7919
+	{
+	.group_id = DH_GROUP_ID_FFDHE2048,
+	.max_strength = 112,
+	.p_size = 256,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xad\xf8\x54\x58\xa2\xbb\x4a\x9a"
+	"\xaf\xdc\x56\x20\x27\x3d\x3c\xf1\xd8\xb9\xc5\x83\xce\x2d\x36\x95"
+	"\xa9\xe1\x36\x41\x14\x64\x33\xfb\xcc\x93\x9d\xce\x24\x9b\x3e\xf9"
+	"\x7d\x2f\xe3\x63\x63\x0c\x75\xd8\xf6\x81\xb2\x02\xae\xc4\x61\x7a"
+	"\xd3\xdf\x1e\xd5\xd5\xfd\x65\x61\x24\x33\xf5\x1f\x5f\x06\x6e\xd0"
+	"\x85\x63\x65\x55\x3d\xed\x1a\xf3\xb5\x57\x13\x5e\x7f\x57\xc9\x35"
+	"\x98\x4f\x0c\x70\xe0\xe6\x8b\x77\xe2\xa6\x89\xda\xf3\xef\xe8\x72"
+	"\x1d\xf1\x58\xa1\x36\xad\xe7\x35\x30\xac\xca\x4f\x48\x3a\x79\x7a"
+	"\xbc\x0a\xb1\x82\xb3\x24\xfb\x61\xd1\x08\xa9\x4b\xb2\xc8\xe3\xfb"
+	"\xb9\x6a\xda\xb7\x60\xd7\xf4\x68\x1d\x4f\x42\xa3\xde\x39\x4d\xf4"
+	"\xae\x56\xed\xe7\x63\x72\xbb\x19\x0b\x07\xa7\xc8\xee\x0a\x6d\x70"
+	"\x9e\x02\xfc\xe1\xcd\xf7\xe2\xec\xc0\x34\x04\xcd\x28\x34\x2f\x61"
+	"\x91\x72\xfe\x9c\xe9\x85\x83\xff\x8e\x4f\x12\x32\xee\xf2\x81\x83"
+	"\xc3\xfe\x3b\x1b\x4c\x6f\xad\x73\x3b\xb5\xfc\xbc\x2e\xc2\x20\x05"
+	"\xc5\x8e\xf1\x83\x7d\x16\x83\xb2\xc6\xf3\x4a\x26\xc1\xb2\xef\xfa"
+	"\x88\x6b\x42\x38\x61\x28\x5c\x97\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+	{
+	.group_id = DH_GROUP_ID_FFDHE3072,
+	.max_strength = 128,
+	.p_size = 384,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xad\xf8\x54\x58\xa2\xbb\x4a\x9a"
+	"\xaf\xdc\x56\x20\x27\x3d\x3c\xf1\xd8\xb9\xc5\x83\xce\x2d\x36\x95"
+	"\xa9\xe1\x36\x41\x14\x64\x33\xfb\xcc\x93\x9d\xce\x24\x9b\x3e\xf9"
+	"\x7d\x2f\xe3\x63\x63\x0c\x75\xd8\xf6\x81\xb2\x02\xae\xc4\x61\x7a"
+	"\xd3\xdf\x1e\xd5\xd5\xfd\x65\x61\x24\x33\xf5\x1f\x5f\x06\x6e\xd0"
+	"\x85\x63\x65\x55\x3d\xed\x1a\xf3\xb5\x57\x13\x5e\x7f\x57\xc9\x35"
+	"\x98\x4f\x0c\x70\xe0\xe6\x8b\x77\xe2\xa6\x89\xda\xf3\xef\xe8\x72"
+	"\x1d\xf1\x58\xa1\x36\xad\xe7\x35\x30\xac\xca\x4f\x48\x3a\x79\x7a"
+	"\xbc\x0a\xb1\x82\xb3\x24\xfb\x61\xd1\x08\xa9\x4b\xb2\xc8\xe3\xfb"
+	"\xb9\x6a\xda\xb7\x60\xd7\xf4\x68\x1d\x4f\x42\xa3\xde\x39\x4d\xf4"
+	"\xae\x56\xed\xe7\x63\x72\xbb\x19\x0b\x07\xa7\xc8\xee\x0a\x6d\x70"
+	"\x9e\x02\xfc\xe1\xcd\xf7\xe2\xec\xc0\x34\x04\xcd\x28\x34\x2f\x61"
+	"\x91\x72\xfe\x9c\xe9\x85\x83\xff\x8e\x4f\x12\x32\xee\xf2\x81\x83"
+	"\xc3\xfe\x3b\x1b\x4c\x6f\xad\x73\x3b\xb5\xfc\xbc\x2e\xc2\x20\x05"
+	"\xc5\x8e\xf1\x83\x7d\x16\x83\xb2\xc6\xf3\x4a\x26\xc1\xb2\xef\xfa"
+	"\x88\x6b\x42\x38\x61\x1f\xcf\xdc\xde\x35\x5b\x3b\x65\x19\x03\x5b"
+	"\xbc\x34\xf4\xde\xf9\x9c\x02\x38\x61\xb4\x6f\xc9\xd6\xe6\xc9\x07"
+	"\x7a\xd9\x1d\x26\x91\xf7\xf7\xee\x59\x8c\xb0\xfa\xc1\x86\xd9\x1c"
+	"\xae\xfe\x13\x09\x85\x13\x92\x70\xb4\x13\x0c\x93\xbc\x43\x79\x44"
+	"\xf4\xfd\x44\x52\xe2\xd7\x4d\xd3\x64\xf2\xe2\x1e\x71\xf5\x4b\xff"
+	"\x5c\xae\x82\xab\x9c\x9d\xf6\x9e\xe8\x6d\x2b\xc5\x22\x36\x3a\x0d"
+	"\xab\xc5\x21\x97\x9b\x0d\xea\xda\x1d\xbf\x9a\x42\xd5\xc4\x48\x4e"
+	"\x0a\xbc\xd0\x6b\xfa\x53\xdd\xef\x3c\x1b\x20\xee\x3f\xd5\x9d\x7c"
+	"\x25\xe4\x1d\x2b\x66\xc6\x2e\x37\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+	{
+	.group_id = DH_GROUP_ID_FFDHE4096,
+	.max_strength = 152,
+	.p_size = 512,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xad\xf8\x54\x58\xa2\xbb\x4a\x9a"
+	"\xaf\xdc\x56\x20\x27\x3d\x3c\xf1\xd8\xb9\xc5\x83\xce\x2d\x36\x95"
+	"\xa9\xe1\x36\x41\x14\x64\x33\xfb\xcc\x93\x9d\xce\x24\x9b\x3e\xf9"
+	"\x7d\x2f\xe3\x63\x63\x0c\x75\xd8\xf6\x81\xb2\x02\xae\xc4\x61\x7a"
+	"\xd3\xdf\x1e\xd5\xd5\xfd\x65\x61\x24\x33\xf5\x1f\x5f\x06\x6e\xd0"
+	"\x85\x63\x65\x55\x3d\xed\x1a\xf3\xb5\x57\x13\x5e\x7f\x57\xc9\x35"
+	"\x98\x4f\x0c\x70\xe0\xe6\x8b\x77\xe2\xa6\x89\xda\xf3\xef\xe8\x72"
+	"\x1d\xf1\x58\xa1\x36\xad\xe7\x35\x30\xac\xca\x4f\x48\x3a\x79\x7a"
+	"\xbc\x0a\xb1\x82\xb3\x24\xfb\x61\xd1\x08\xa9\x4b\xb2\xc8\xe3\xfb"
+	"\xb9\x6a\xda\xb7\x60\xd7\xf4\x68\x1d\x4f\x42\xa3\xde\x39\x4d\xf4"
+	"\xae\x56\xed\xe7\x63\x72\xbb\x19\x0b\x07\xa7\xc8\xee\x0a\x6d\x70"
+	"\x9e\x02\xfc\xe1\xcd\xf7\xe2\xec\xc0\x34\x04\xcd\x28\x34\x2f\x61"
+	"\x91\x72\xfe\x9c\xe9\x85\x83\xff\x8e\x4f\x12\x32\xee\xf2\x81\x83"
+	"\xc3\xfe\x3b\x1b\x4c\x6f\xad\x73\x3b\xb5\xfc\xbc\x2e\xc2\x20\x05"
+	"\xc5\x8e\xf1\x83\x7d\x16\x83\xb2\xc6\xf3\x4a\x26\xc1\xb2\xef\xfa"
+	"\x88\x6b\x42\x38\x61\x1f\xcf\xdc\xde\x35\x5b\x3b\x65\x19\x03\x5b"
+	"\xbc\x34\xf4\xde\xf9\x9c\x02\x38\x61\xb4\x6f\xc9\xd6\xe6\xc9\x07"
+	"\x7a\xd9\x1d\x26\x91\xf7\xf7\xee\x59\x8c\xb0\xfa\xc1\x86\xd9\x1c"
+	"\xae\xfe\x13\x09\x85\x13\x92\x70\xb4\x13\x0c\x93\xbc\x43\x79\x44"
+	"\xf4\xfd\x44\x52\xe2\xd7\x4d\xd3\x64\xf2\xe2\x1e\x71\xf5\x4b\xff"
+	"\x5c\xae\x82\xab\x9c\x9d\xf6\x9e\xe8\x6d\x2b\xc5\x22\x36\x3a\x0d"
+	"\xab\xc5\x21\x97\x9b\x0d\xea\xda\x1d\xbf\x9a\x42\xd5\xc4\x48\x4e"
+	"\x0a\xbc\xd0\x6b\xfa\x53\xdd\xef\x3c\x1b\x20\xee\x3f\xd5\x9d\x7c"
+	"\x25\xe4\x1d\x2b\x66\x9e\x1e\xf1\x6e\x6f\x52\xc3\x16\x4d\xf4\xfb"
+	"\x79\x30\xe9\xe4\xe5\x88\x57\xb6\xac\x7d\x5f\x42\xd6\x9f\x6d\x18"
+	"\x77\x63\xcf\x1d\x55\x03\x40\x04\x87\xf5\x5b\xa5\x7e\x31\xcc\x7a"
+	"\x71\x35\xc8\x86\xef\xb4\x31\x8a\xed\x6a\x1e\x01\x2d\x9e\x68\x32"
+	"\xa9\x07\x60\x0a\x91\x81\x30\xc4\x6d\xc7\x78\xf9\x71\xad\x00\x38"
+	"\x09\x29\x99\xa3\x33\xcb\x8b\x7a\x1a\x1d\xb9\x3d\x71\x40\x00\x3c"
+	"\x2a\x4e\xce\xa9\xf9\x8d\x0a\xcc\x0a\x82\x91\xcd\xce\xc9\x7d\xcf"
+	"\x8e\xc9\xb5\x5a\x7f\x88\xa4\x6b\x4d\xb5\xa8\x51\xf4\x41\x82\xe1"
+	"\xc6\x8a\x00\x7e\x5e\x65\x5f\x6a\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+	{
+	.group_id = DH_GROUP_ID_FFDHE6144,
+	.max_strength = 176,
+	.p_size = 768,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xad\xf8\x54\x58\xa2\xbb\x4a\x9a"
+	"\xaf\xdc\x56\x20\x27\x3d\x3c\xf1\xd8\xb9\xc5\x83\xce\x2d\x36\x95"
+	"\xa9\xe1\x36\x41\x14\x64\x33\xfb\xcc\x93\x9d\xce\x24\x9b\x3e\xf9"
+	"\x7d\x2f\xe3\x63\x63\x0c\x75\xd8\xf6\x81\xb2\x02\xae\xc4\x61\x7a"
+	"\xd3\xdf\x1e\xd5\xd5\xfd\x65\x61\x24\x33\xf5\x1f\x5f\x06\x6e\xd0"
+	"\x85\x63\x65\x55\x3d\xed\x1a\xf3\xb5\x57\x13\x5e\x7f\x57\xc9\x35"
+	"\x98\x4f\x0c\x70\xe0\xe6\x8b\x77\xe2\xa6\x89\xda\xf3\xef\xe8\x72"
+	"\x1d\xf1\x58\xa1\x36\xad\xe7\x35\x30\xac\xca\x4f\x48\x3a\x79\x7a"
+	"\xbc\x0a\xb1\x82\xb3\x24\xfb\x61\xd1\x08\xa9\x4b\xb2\xc8\xe3\xfb"
+	"\xb9\x6a\xda\xb7\x60\xd7\xf4\x68\x1d\x4f\x42\xa3\xde\x39\x4d\xf4"
+	"\xae\x56\xed\xe7\x63\x72\xbb\x19\x0b\x07\xa7\xc8\xee\x0a\x6d\x70"
+	"\x9e\x02\xfc\xe1\xcd\xf7\xe2\xec\xc0\x34\x04\xcd\x28\x34\x2f\x61"
+	"\x91\x72\xfe\x9c\xe9\x85\x83\xff\x8e\x4f\x12\x32\xee\xf2\x81\x83"
+	"\xc3\xfe\x3b\x1b\x4c\x6f\xad\x73\x3b\xb5\xfc\xbc\x2e\xc2\x20\x05"
+	"\xc5\x8e\xf1\x83\x7d\x16\x83\xb2\xc6\xf3\x4a\x26\xc1\xb2\xef\xfa"
+	"\x88\x6b\x42\x38\x61\x1f\xcf\xdc\xde\x35\x5b\x3b\x65\x19\x03\x5b"
+	"\xbc\x34\xf4\xde\xf9\x9c\x02\x38\x61\xb4\x6f\xc9\xd6\xe6\xc9\x07"
+	"\x7a\xd9\x1d\x26\x91\xf7\xf7\xee\x59\x8c\xb0\xfa\xc1\x86\xd9\x1c"
+	"\xae\xfe\x13\x09\x85\x13\x92\x70\xb4\x13\x0c\x93\xbc\x43\x79\x44"
+	"\xf4\xfd\x44\x52\xe2\xd7\x4d\xd3\x64\xf2\xe2\x1e\x71\xf5\x4b\xff"
+	"\x5c\xae\x82\xab\x9c\x9d\xf6\x9e\xe8\x6d\x2b\xc5\x22\x36\x3a\x0d"
+	"\xab\xc5\x21\x97\x9b\x0d\xea\xda\x1d\xbf\x9a\x42\xd5\xc4\x48\x4e"
+	"\x0a\xbc\xd0\x6b\xfa\x53\xdd\xef\x3c\x1b\x20\xee\x3f\xd5\x9d\x7c"
+	"\x25\xe4\x1d\x2b\x66\x9e\x1e\xf1\x6e\x6f\x52\xc3\x16\x4d\xf4\xfb"
+	"\x79\x30\xe9\xe4\xe5\x88\x57\xb6\xac\x7d\x5f\x42\xd6\x9f\x6d\x18"
+	"\x77\x63\xcf\x1d\x55\x03\x40\x04\x87\xf5\x5b\xa5\x7e\x31\xcc\x7a"
+	"\x71\x35\xc8\x86\xef\xb4\x31\x8a\xed\x6a\x1e\x01\x2d\x9e\x68\x32"
+	"\xa9\x07\x60\x0a\x91\x81\x30\xc4\x6d\xc7\x78\xf9\x71\xad\x00\x38"
+	"\x09\x29\x99\xa3\x33\xcb\x8b\x7a\x1a\x1d\xb9\x3d\x71\x40\x00\x3c"
+	"\x2a\x4e\xce\xa9\xf9\x8d\x0a\xcc\x0a\x82\x91\xcd\xce\xc9\x7d\xcf"
+	"\x8e\xc9\xb5\x5a\x7f\x88\xa4\x6b\x4d\xb5\xa8\x51\xf4\x41\x82\xe1"
+	"\xc6\x8a\x00\x7e\x5e\x0d\xd9\x02\x0b\xfd\x64\xb6\x45\x03\x6c\x7a"
+	"\x4e\x67\x7d\x2c\x38\x53\x2a\x3a\x23\xba\x44\x42\xca\xf5\x3e\xa6"
+	"\x3b\xb4\x54\x32\x9b\x76\x24\xc8\x91\x7b\xdd\x64\xb1\xc0\xfd\x4c"
+	"\xb3\x8e\x8c\x33\x4c\x70\x1c\x3a\xcd\xad\x06\x57\xfc\xcf\xec\x71"
+	"\x9b\x1f\x5c\x3e\x4e\x46\x04\x1f\x38\x81\x47\xfb\x4c\xfd\xb4\x77"
+	"\xa5\x24\x71\xf7\xa9\xa9\x69\x10\xb8\x55\x32\x2e\xdb\x63\x40\xd8"
+	"\xa0\x0e\xf0\x92\x35\x05\x11\xe3\x0a\xbe\xc1\xff\xf9\xe3\xa2\x6e"
+	"\x7f\xb2\x9f\x8c\x18\x30\x23\xc3\x58\x7e\x38\xda\x00\x77\xd9\xb4"
+	"\x76\x3e\x4e\x4b\x94\xb2\xbb\xc1\x94\xc6\x65\x1e\x77\xca\xf9\x92"
+	"\xee\xaa\xc0\x23\x2a\x28\x1b\xf6\xb3\xa7\x39\xc1\x22\x61\x16\x82"
+	"\x0a\xe8\xdb\x58\x47\xa6\x7c\xbe\xf9\xc9\x09\x1b\x46\x2d\x53\x8c"
+	"\xd7\x2b\x03\x74\x6a\xe7\x7f\x5e\x62\x29\x2c\x31\x15\x62\xa8\x46"
+	"\x50\x5d\xc8\x2d\xb8\x54\x33\x8a\xe4\x9f\x52\x35\xc9\x5b\x91\x17"
+	"\x8c\xcf\x2d\xd5\xca\xce\xf4\x03\xec\x9d\x18\x10\xc6\x27\x2b\x04"
+	"\x5b\x3b\x71\xf9\xdc\x6b\x80\xd6\x3f\xdd\x4a\x8e\x9a\xdb\x1e\x69"
+	"\x62\xa6\x95\x26\xd4\x31\x61\xc1\xa4\x1d\x57\x0d\x79\x38\xda\xd4"
+	"\xa4\x0e\x32\x9c\xd0\xe4\x0e\x65\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+	{
+	.group_id = DH_GROUP_ID_FFDHE8192,
+	.max_strength = 200,
+	.p_size = 1024,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xad\xf8\x54\x58\xa2\xbb\x4a\x9a"
+	"\xaf\xdc\x56\x20\x27\x3d\x3c\xf1\xd8\xb9\xc5\x83\xce\x2d\x36\x95"
+	"\xa9\xe1\x36\x41\x14\x64\x33\xfb\xcc\x93\x9d\xce\x24\x9b\x3e\xf9"
+	"\x7d\x2f\xe3\x63\x63\x0c\x75\xd8\xf6\x81\xb2\x02\xae\xc4\x61\x7a"
+	"\xd3\xdf\x1e\xd5\xd5\xfd\x65\x61\x24\x33\xf5\x1f\x5f\x06\x6e\xd0"
+	"\x85\x63\x65\x55\x3d\xed\x1a\xf3\xb5\x57\x13\x5e\x7f\x57\xc9\x35"
+	"\x98\x4f\x0c\x70\xe0\xe6\x8b\x77\xe2\xa6\x89\xda\xf3\xef\xe8\x72"
+	"\x1d\xf1\x58\xa1\x36\xad\xe7\x35\x30\xac\xca\x4f\x48\x3a\x79\x7a"
+	"\xbc\x0a\xb1\x82\xb3\x24\xfb\x61\xd1\x08\xa9\x4b\xb2\xc8\xe3\xfb"
+	"\xb9\x6a\xda\xb7\x60\xd7\xf4\x68\x1d\x4f\x42\xa3\xde\x39\x4d\xf4"
+	"\xae\x56\xed\xe7\x63\x72\xbb\x19\x0b\x07\xa7\xc8\xee\x0a\x6d\x70"
+	"\x9e\x02\xfc\xe1\xcd\xf7\xe2\xec\xc0\x34\x04\xcd\x28\x34\x2f\x61"
+	"\x91\x72\xfe\x9c\xe9\x85\x83\xff\x8e\x4f\x12\x32\xee\xf2\x81\x83"
+	"\xc3\xfe\x3b\x1b\x4c\x6f\xad\x73\x3b\xb5\xfc\xbc\x2e\xc2\x20\x05"
+	"\xc5\x8e\xf1\x83\x7d\x16\x83\xb2\xc6\xf3\x4a\x26\xc1\xb2\xef\xfa"
+	"\x88\x6b\x42\x38\x61\x1f\xcf\xdc\xde\x35\x5b\x3b\x65\x19\x03\x5b"
+	"\xbc\x34\xf4\xde\xf9\x9c\x02\x38\x61\xb4\x6f\xc9\xd6\xe6\xc9\x07"
+	"\x7a\xd9\x1d\x26\x91\xf7\xf7\xee\x59\x8c\xb0\xfa\xc1\x86\xd9\x1c"
+	"\xae\xfe\x13\x09\x85\x13\x92\x70\xb4\x13\x0c\x93\xbc\x43\x79\x44"
+	"\xf4\xfd\x44\x52\xe2\xd7\x4d\xd3\x64\xf2\xe2\x1e\x71\xf5\x4b\xff"
+	"\x5c\xae\x82\xab\x9c\x9d\xf6\x9e\xe8\x6d\x2b\xc5\x22\x36\x3a\x0d"
+	"\xab\xc5\x21\x97\x9b\x0d\xea\xda\x1d\xbf\x9a\x42\xd5\xc4\x48\x4e"
+	"\x0a\xbc\xd0\x6b\xfa\x53\xdd\xef\x3c\x1b\x20\xee\x3f\xd5\x9d\x7c"
+	"\x25\xe4\x1d\x2b\x66\x9e\x1e\xf1\x6e\x6f\x52\xc3\x16\x4d\xf4\xfb"
+	"\x79\x30\xe9\xe4\xe5\x88\x57\xb6\xac\x7d\x5f\x42\xd6\x9f\x6d\x18"
+	"\x77\x63\xcf\x1d\x55\x03\x40\x04\x87\xf5\x5b\xa5\x7e\x31\xcc\x7a"
+	"\x71\x35\xc8\x86\xef\xb4\x31\x8a\xed\x6a\x1e\x01\x2d\x9e\x68\x32"
+	"\xa9\x07\x60\x0a\x91\x81\x30\xc4\x6d\xc7\x78\xf9\x71\xad\x00\x38"
+	"\x09\x29\x99\xa3\x33\xcb\x8b\x7a\x1a\x1d\xb9\x3d\x71\x40\x00\x3c"
+	"\x2a\x4e\xce\xa9\xf9\x8d\x0a\xcc\x0a\x82\x91\xcd\xce\xc9\x7d\xcf"
+	"\x8e\xc9\xb5\x5a\x7f\x88\xa4\x6b\x4d\xb5\xa8\x51\xf4\x41\x82\xe1"
+	"\xc6\x8a\x00\x7e\x5e\x0d\xd9\x02\x0b\xfd\x64\xb6\x45\x03\x6c\x7a"
+	"\x4e\x67\x7d\x2c\x38\x53\x2a\x3a\x23\xba\x44\x42\xca\xf5\x3e\xa6"
+	"\x3b\xb4\x54\x32\x9b\x76\x24\xc8\x91\x7b\xdd\x64\xb1\xc0\xfd\x4c"
+	"\xb3\x8e\x8c\x33\x4c\x70\x1c\x3a\xcd\xad\x06\x57\xfc\xcf\xec\x71"
+	"\x9b\x1f\x5c\x3e\x4e\x46\x04\x1f\x38\x81\x47\xfb\x4c\xfd\xb4\x77"
+	"\xa5\x24\x71\xf7\xa9\xa9\x69\x10\xb8\x55\x32\x2e\xdb\x63\x40\xd8"
+	"\xa0\x0e\xf0\x92\x35\x05\x11\xe3\x0a\xbe\xc1\xff\xf9\xe3\xa2\x6e"
+	"\x7f\xb2\x9f\x8c\x18\x30\x23\xc3\x58\x7e\x38\xda\x00\x77\xd9\xb4"
+	"\x76\x3e\x4e\x4b\x94\xb2\xbb\xc1\x94\xc6\x65\x1e\x77\xca\xf9\x92"
+	"\xee\xaa\xc0\x23\x2a\x28\x1b\xf6\xb3\xa7\x39\xc1\x22\x61\x16\x82"
+	"\x0a\xe8\xdb\x58\x47\xa6\x7c\xbe\xf9\xc9\x09\x1b\x46\x2d\x53\x8c"
+	"\xd7\x2b\x03\x74\x6a\xe7\x7f\x5e\x62\x29\x2c\x31\x15\x62\xa8\x46"
+	"\x50\x5d\xc8\x2d\xb8\x54\x33\x8a\xe4\x9f\x52\x35\xc9\x5b\x91\x17"
+	"\x8c\xcf\x2d\xd5\xca\xce\xf4\x03\xec\x9d\x18\x10\xc6\x27\x2b\x04"
+	"\x5b\x3b\x71\xf9\xdc\x6b\x80\xd6\x3f\xdd\x4a\x8e\x9a\xdb\x1e\x69"
+	"\x62\xa6\x95\x26\xd4\x31\x61\xc1\xa4\x1d\x57\x0d\x79\x38\xda\xd4"
+	"\xa4\x0e\x32\x9c\xcf\xf4\x6a\xaa\x36\xad\x00\x4c\xf6\x00\xc8\x38"
+	"\x1e\x42\x5a\x31\xd9\x51\xae\x64\xfd\xb2\x3f\xce\xc9\x50\x9d\x43"
+	"\x68\x7f\xeb\x69\xed\xd1\xcc\x5e\x0b\x8c\xc3\xbd\xf6\x4b\x10\xef"
+	"\x86\xb6\x31\x42\xa3\xab\x88\x29\x55\x5b\x2f\x74\x7c\x93\x26\x65"
+	"\xcb\x2c\x0f\x1c\xc0\x1b\xd7\x02\x29\x38\x88\x39\xd2\xaf\x05\xe4"
+	"\x54\x50\x4a\xc7\x8b\x75\x82\x82\x28\x46\xc0\xba\x35\xc3\x5f\x5c"
+	"\x59\x16\x0c\xc0\x46\xfd\x82\x51\x54\x1f\xc6\x8c\x9c\x86\xb0\x22"
+	"\xbb\x70\x99\x87\x6a\x46\x0e\x74\x51\xa8\xa9\x31\x09\x70\x3f\xee"
+	"\x1c\x21\x7e\x6c\x38\x26\xe5\x2c\x51\xaa\x69\x1e\x0e\x42\x3c\xfc"
+	"\x99\xe9\xe3\x16\x50\xc1\x21\x7b\x62\x48\x16\xcd\xad\x9a\x95\xf9"
+	"\xd5\xb8\x01\x94\x88\xd9\xc0\xa0\xa1\xfe\x30\x75\xa5\x77\xe2\x31"
+	"\x83\xf8\x1d\x4a\x3f\x2f\xa4\x57\x1e\xfc\x8c\xe0\xba\x8a\x4f\xe8"
+	"\xb6\x85\x5d\xfe\x72\xb0\xa6\x6e\xde\xd2\xfb\xab\xfb\xe5\x8a\x30"
+	"\xfa\xfa\xbe\x1c\x5d\x71\xa8\x7e\x2f\x74\x1e\xf8\xc1\xfe\x86\xfe"
+	"\xa6\xbb\xfd\xe5\x30\x67\x7f\x0d\x97\xd1\x1d\x49\xf7\xa8\x44\x3d"
+	"\x08\x22\xe5\x06\xa9\xf4\x61\x4e\x01\x1e\x2a\x94\x83\x8f\xf8\x8c"
+	"\xd6\x8c\x8b\xb7\xc5\xc6\x42\x4c\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+#endif /* CONFIG_CRYPTO_DH_GROUPS_RFC7919 */
+};
 
 /* 2 is used as a generator for all safe-prime groups. */
 static const char safe_prime_group_g[]  = { 2 };
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index f0ed899e2168..2aee155f1e0b 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -22,6 +22,13 @@
 /** enum dh_group_id - identify well-known domain parameter sets */
 enum dh_group_id {
 	DH_GROUP_ID_UNKNOWN = 0, /* Constants are used in test vectors. */
+#ifdef CONFIG_CRYPTO_DH_GROUPS_RFC7919
+	DH_GROUP_ID_FFDHE2048 = 1,
+	DH_GROUP_ID_FFDHE3072 = 2,
+	DH_GROUP_ID_FFDHE4096 = 3,
+	DH_GROUP_ID_FFDHE6144 = 4,
+	DH_GROUP_ID_FFDHE8192 = 5,
+#endif
 };
 
 /**
-- 
2.26.2

Re: [PATCH v2 04/18] crypto: dh - introduce RFC 7919 safe-prime groups

[Hannes Reinecke]

On 12/9/21 10:03 AM, Nicolai Stange wrote:
> The FFDHE groups specified by RFC 7919 are needed for the current work
> on NVME ([1]) and also among the safe-prime groups approved by
> SP800-56Arev3. Make them known to the kernel.
> 
> More specifically, introduce corresponding members to enum dh_group_id
> as well as entries with the resp. domain parameters to the
> safe_prime_groups[] array queried by crypto_dh_decode_key(). The resp.
> ->max_strength value is set to the maximum supported security strength as
> specified in SP800-56Arev3.
> 
> As the domain parameters consume an substantial amount of space, make
> RFC 7919 safe-prime group support selectable by means of the new
> CRYPTO_DH_GROUPS_RFC7919 Kconfig option.
> 
> [1] https://lkml.kernel.org/r/20211122074727.25988-4-hare@suse.de
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  crypto/Kconfig      |  11 ++-
>  crypto/dh_helper.c  | 219 +++++++++++++++++++++++++++++++++++++++++++-
>  include/crypto/dh.h |   7 ++
>  3 files changed, 235 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

[PATCH v2 05/18] crypto: testmgr - add DH RFC 7919 ffdhe3072 test vector

[Nicolai Stange]

The previous patch introduced support for the safe-prime groups specified
by RFC 7919. In order to test this functionality, add a corresponding
ffdhe3072 test vector to testmgr. The choice of ffdhe3072 over e.g.
ffdhe2048 is justified by the fact that the NVMe spec mandates it for its
TLS profile.

The test data has been generated with OpenSSL.

Note that this new entry provides test coverage for the recent change to
crypto_dh_encode_key(), which made it to skip the serialization of domain
parameters for known groups, i.e. those with
->group_id != DH_GROUP_ID_UNKNOWN.

Moreover, a future patch will make the DH implementation to reject domain
parameters not corresponding to some safe-prime group approved by
SP800-56Arev3 in FIPS mode and the existing DH test vectors don't qualify.
So this patch here will ensure that there's still some suitable test vector
available.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/testmgr.h | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 637913064c64..26194db387db 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -1240,6 +1240,130 @@ static const struct akcipher_testvec pkcs1pad_rsa_tv_template[] = {
 };
 
 static const struct kpp_testvec dh_tv_template[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC7919)
+	{
+	.secret =
+#ifdef __LITTLE_ENDIAN
+	"\x01\x00" /* type */
+	"\x94\x01" /* len */
+	"\x02\x00\x00\x00" /* group_id == DH_GROUP_ID_FFDHE3072 */
+	"\x80\x01\x00\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* g_size */
+#else
+	"\x00\x01" /* type */
+	"\x01\x94" /* len */
+	"\x00\x00\x00\x02" /* group_id == DH_GROUP_ID_FFDHE3072 */
+	"\x00\x00\x01\x80" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* g_size */
+#endif
+	/* xa */
+	"\x6b\xb4\x97\x23\xfa\xc8\x5e\xa9\x7b\x63\xe7\x3e\x0e\x99\xc3\xb9"
+	"\xda\xb7\x48\x0d\xc3\xb1\xbf\x4f\x17\xc7\xa9\x51\xf6\x64\xff\xc4"
+	"\x31\x58\x87\x25\x83\x2c\x00\xf0\x41\x29\xf7\xee\xf9\xe6\x36\x76"
+	"\xd6\x3a\x24\xbe\xa7\x07\x0b\x93\xc7\x9f\x6c\x75\x0a\x26\x75\x76"
+	"\xe3\x0c\x42\xe0\x00\x04\x69\xd9\xec\x0b\x59\x54\x28\x8f\xd7\x9a"
+	"\x63\xf4\x5b\xdf\x85\x65\xc4\xe1\x95\x27\x4a\x42\xad\x36\x47\xa9"
+	"\x0a\xf8\x14\x1c\xf3\x94\x3b\x7e\x47\x99\x35\xa8\x18\xec\x70\x10"
+	"\xdf\xcb\xd2\x78\x88\xc1\x2d\x59\x93\xc1\xa4\x6d\xd7\x1d\xb9\xd5"
+	"\xf8\x30\x06\x7f\x98\x90\x0c\x74\x5e\x89\x2f\x64\x5a\xad\x5f\x53"
+	"\xb2\xa3\xa8\x83\xbf\xfc\x37\xef\xb8\x36\x0a\x5c\x62\x81\x64\x74"
+	"\x16\x2f\x45\x39\x2a\x91\x26\x87\xc0\x12\xcc\x75\x11\xa3\xa1\xc5"
+	"\xae\x20\xcf\xcb\x20\x25\x6b\x7a\x31\x93\x9d\x38\xb9\x57\x72\x46"
+	"\xd4\x84\x65\x87\xf1\xb5\xd3\xab\xfc\xc3\x4d\x40\x92\x94\x1e\xcd"
+	"\x1c\x87\xec\x3f\xcd\xbe\xd0\x95\x6b\x40\x02\xdd\x62\xeb\x0a\xda"
+	"\x4f\xbe\x8e\x32\x48\x8b\x6d\x83\xa0\x96\x62\x23\xec\x83\x91\x44"
+	"\xf9\x72\x01\xac\xa0\xe4\x72\x1d\x5a\x75\x05\x57\x90\xae\x7e\xb4"
+	"\x71\x39\x01\x05\xdc\xe9\xee\xcb\xf0\x61\x28\x91\x69\x8c\x31\x03"
+	"\x7a\x92\x15\xa1\x58\x67\x3d\x70\x82\xa6\x2c\xfe\x10\x56\x58\xd3"
+	"\x94\x67\xe1\xbe\xee\xc1\x64\x5c\x4b\xc8\x28\x3d\xc5\x66\x3a\xab"
+	"\x22\xc1\x7e\xa1\xbb\xf3\x19\x3b\xda\x46\x82\x45\xd4\x3c\x7c\xc6"
+	"\xce\x1f\x7f\x95\xa2\x17\xff\x88\xba\xd6\x4d\xdb\xd2\xea\xde\x39"
+	"\xd6\xa5\x18\x73\xbb\x64\x6e\x79\xe9\xdc\x3f\x92\x7f\xda\x1f\x49"
+	"\x33\x70\x65\x73\xa2\xd9\x06\xb8\x1b\x29\x29\x1a\xe0\xa3\xe6\x05"
+	"\x9a\xa8\xc2\x4e\x7a\x78\x1d\x22\x57\x21\xc8\xa3\x8d\x66\x3e\x23",
+	.b_public =
+	"\x73\x40\x8b\xce\xe8\x6a\x1c\x03\x50\x54\x42\x36\x22\xc6\x1d\xe8"
+	"\xe1\xef\x5c\x89\xa5\x55\xc1\xc4\x1c\xd7\x4f\xee\x5d\xba\x62\x60"
+	"\xfe\x93\x2f\xfd\x93\x2c\x8f\x70\xc6\x47\x17\x25\xb2\x95\xd7\x7d"
+	"\x41\x81\x4d\x52\x1c\xbe\x4d\x57\x3e\x26\x51\x28\x03\x8f\x67\xf5"
+	"\x22\x16\x1c\x67\xf7\x62\xcb\xfd\xa3\xee\x8d\xe0\xfa\x15\x9a\x53"
+	"\xbe\x7b\x9f\xc0\x12\x7a\xfc\x5e\x77\x2d\x60\x06\xba\x71\xc5\xca"
+	"\xd7\x26\xaf\x3b\xba\x6f\xd3\xc4\x82\x57\x19\x26\xb0\x16\x7b\xbd"
+	"\x83\xf2\x21\x03\x79\xff\x0a\x6f\xc5\x7b\x00\x15\xad\x5b\xf4\x42"
+	"\x1f\xcb\x7f\x3d\x34\x77\x3c\xc3\xe0\x38\xa5\x40\x51\xbe\x6f\xd9"
+	"\xc9\x77\x9c\xfc\x0d\xc1\x8e\xef\x0f\xaa\x5e\xa8\xbb\x16\x4a\x3e"
+	"\x26\x55\xae\xc1\xb6\x3e\xfd\x73\xf7\x59\xd2\xe5\x4b\x91\x8e\x28"
+	"\x77\x1e\x5a\xe2\xcd\xce\x92\x35\xbb\x1e\xbb\xcf\x79\x94\xdf\x31"
+	"\xde\x31\xa8\x75\xf6\xe0\xaa\x2e\xe9\x4f\x44\xc8\xba\xb9\xab\x80"
+	"\x29\xa1\xea\x58\x2e\x40\x96\xa0\x1a\xf5\x2c\x38\x47\x43\x5d\x26"
+	"\x2c\xd8\xad\xea\xd3\xad\xe8\x51\x49\xad\x45\x2b\x25\x7c\xde\xe4"
+	"\xaf\x03\x2a\x39\x26\x86\x66\x10\xbc\xa8\x71\xda\xe0\xe8\xf1\xdd"
+	"\x50\xff\x44\xb2\xd3\xc7\xff\x66\x63\xf6\x42\xe3\x97\x9d\x9e\xf4"
+	"\xa6\x89\xb9\xab\x12\x17\xf2\x85\x56\x9c\x6b\x24\x71\x83\x57\x7d"
+	"\x3c\x7b\x2b\x88\x92\x19\xd7\x1a\x00\xd5\x38\x94\x43\x60\x4d\xa7"
+	"\x12\x9e\x0d\xf6\x5c\x9a\xd3\xe2\x9e\xb1\x21\xe8\xe2\x9e\xe9\x1e"
+	"\x9d\xa5\x94\x95\xa6\x3d\x12\x15\xd8\x8b\xac\xe0\x8c\xde\xe6\x40"
+	"\x98\xaa\x5e\x55\x4f\x3d\x86\x87\x0d\xe3\xc6\x68\x15\xe6\xde\x17"
+	"\x78\x21\xc8\x6c\x06\xc7\x94\x56\xb4\xaf\xa2\x35\x0b\x0c\x97\xd7"
+	"\xa4\x12\xee\xf4\xd2\xef\x80\x28\xb3\xee\xe9\x15\x8b\x01\x32\x79",
+	.expected_a_public =
+	"\x1b\x6a\xba\xea\xa3\xcc\x50\x69\xa9\x41\x89\xaf\x04\xe1\x44\x22"
+	"\x97\x20\xd1\xf6\x1e\xcb\x64\x36\x6f\xee\x0b\x16\xc1\xd9\x91\xbe"
+	"\x57\xc8\xd9\xf2\xa1\x96\x91\xec\x41\xc7\x79\x00\x1a\x48\x25\x55"
+	"\xbe\xf3\x20\x8c\x38\xc6\x7b\xf2\x8b\x5a\xc3\xb5\x87\x0a\x86\x3d"
+	"\xb7\xd6\xce\xb0\x96\x2e\x5d\xc4\x00\x5e\x42\xe4\xe5\x50\x4f\xb8"
+	"\x6f\x18\xa4\xe1\xd3\x20\xfc\x3c\xf5\x0a\xff\x23\xa6\x5b\xb4\x17"
+	"\x3e\x7b\xdf\xb9\xb5\x3c\x1b\x76\x29\xcd\xb4\x46\x4f\x27\x8f\xd2"
+	"\xe8\x27\x66\xdb\xe8\xb3\xf5\xe1\xd0\x04\xcd\x89\xff\xba\x76\x67"
+	"\xe8\x4d\xcf\x86\x1c\x8a\xd1\xcf\x99\x27\xfb\xa9\x78\xcc\x94\xaf"
+	"\x3d\x04\xfd\x25\xc0\x47\xfa\x29\x80\x05\xf4\xde\xad\xdb\xab\x12"
+	"\xb0\x2b\x8e\xca\x02\x06\x6d\xad\x3e\x09\xb1\x22\xa3\xf5\x4c\x6d"
+	"\x69\x99\x58\x8b\xd8\x45\x2e\xe0\xc9\x3c\xf7\x92\xce\x21\x90\x6b"
+	"\x3b\x65\x9f\x64\x79\x8d\x67\x22\x1a\x37\xd3\xee\x51\xe2\xe7\x5a"
+	"\x93\x51\xaa\x3c\x4b\x04\x16\x32\xef\xe3\x66\xbe\x18\x94\x88\x64"
+	"\x79\xce\x06\x3f\xb8\xd6\xee\xdc\x13\x79\x6f\x20\x14\xc2\x6b\xce"
+	"\xc8\xda\x42\xa5\x93\x5b\xe4\x7f\x1a\xe6\xda\x0f\xb3\xc1\x5f\x30"
+	"\x50\x76\xe8\x37\x3d\xca\x77\x2c\xa8\xe4\x3b\xf9\x6f\xe0\x17\xed"
+	"\x0e\xef\xb7\x31\x14\xb5\xea\xd9\x39\x22\x89\xb6\x40\x57\xcc\x84"
+	"\xef\x73\xa7\xe9\x27\x21\x85\x89\xfa\xaf\x03\xda\x9c\x8b\xfd\x52"
+	"\x7d\xb0\xa4\xe4\xf9\xd8\x90\x55\xc4\x39\xd6\x9d\xaf\x3b\xce\xac"
+	"\xaa\x36\x14\x7a\x9b\x8b\x12\x43\xe1\xca\x61\xae\x46\x5b\xe7\xe5"
+	"\x88\x32\x80\xa0\x2d\x51\xbb\x2f\xea\xeb\x3c\x71\xb2\xae\xce\xca"
+	"\x61\xd2\x76\xe0\x45\x46\x78\x4e\x09\x2d\xc2\x54\xc2\xa9\xc7\xa8"
+	"\x55\x8e\x72\xa4\x8b\x8a\xc9\x01\xdb\xe9\x58\x11\xa1\xc4\xe7\x12",
+	.expected_ss =
+	"\x47\x8e\xb2\x19\x09\xf0\x46\x99\x6b\x41\x86\xf7\x34\xad\xbf\x2a"
+	"\x18\x1b\x7d\xec\xa9\xb2\x47\x2f\x40\xfb\x9a\x64\x30\x44\xf3\x4c"
+	"\x01\x67\xad\x57\x5a\xbc\xd4\xc8\xef\x7e\x8a\x14\x74\x1d\x6d\x8c"
+	"\x7b\xce\xc5\x57\x5f\x95\xe8\x72\xba\xdf\xa3\xcd\x00\xbe\x09\x4c"
+	"\x06\x72\xe7\x17\xb0\xe5\xe5\xb7\x20\xa5\xcb\xd9\x68\x99\xad\x3f"
+	"\xde\xf3\xde\x1d\x1c\x00\x74\xd2\xd1\x57\x55\x5d\xce\x76\x0c\xc4"
+	"\x7a\xc4\x65\x7c\x19\x17\x0a\x09\x66\x7d\x3a\xab\xf7\x61\x3a\xe3"
+	"\x5b\xac\xcf\x69\xb0\x8b\xee\x5d\x28\x36\xbb\x3f\x74\xce\x6e\x38"
+	"\x1e\x39\xab\x26\xca\x89\xdc\x58\x59\xcb\x95\xe4\xbc\xd6\x19\x48"
+	"\xd0\x55\x68\x7b\xb4\x27\x95\x3c\xd9\x58\x10\x4f\x8f\x55\x1c\x3f"
+	"\x04\xce\x89\x1f\x82\x28\xe9\x48\x17\x47\x8f\xee\xb7\x8f\xeb\xb1"
+	"\x29\xa8\x23\x18\x73\x33\x9f\x83\x08\xca\xcd\x54\x6e\xca\xec\x78"
+	"\x7b\x16\x83\x3f\xdb\x0a\xef\xfd\x87\x94\x19\x08\x6e\x6e\x22\x57"
+	"\xd7\xd2\x79\xf9\xf6\xeb\xe0\x6c\x93\x9d\x95\xfa\x41\x7a\xa9\xd6"
+	"\x2a\xa3\x26\x9b\x24\x1b\x8b\xa0\xed\x04\xb2\xe4\x6c\x4e\xc4\x3f"
+	"\x61\xe5\xe0\x4d\x09\x28\xaf\x58\x35\x25\x0b\xd5\x38\x18\x69\x51"
+	"\x18\x51\x73\x7b\x28\x19\x9f\xe4\x69\xfc\x2c\x25\x08\x99\x8f\x62"
+	"\x65\x62\xa5\x28\xf1\xf4\xfb\x02\x29\x27\xb0\x5e\xbb\x4f\xf9\x1a"
+	"\xa7\xc4\x38\x63\x5b\x01\xfe\x00\x66\xe3\x47\x77\x21\x85\x17\xd5"
+	"\x34\x19\xd3\x87\xab\x44\x62\x08\x59\xb2\x6b\x1f\x21\x0c\x23\x84"
+	"\xf7\xba\x92\x67\xf9\x16\x85\x6a\xe0\xeb\xe7\x4f\x06\x80\x81\x81"
+	"\x28\x9c\xe8\x2e\x71\x97\x48\xe0\xd1\xbc\xce\xe9\x42\x2c\x89\xdf"
+	"\x0b\xa9\xa1\x07\x84\x33\x78\x7f\x49\x2f\x1c\x55\xc3\x7f\xc3\x37"
+	"\x40\xdf\x13\xf4\xa0\x21\x79\x6e\x3a\xe3\xb8\x23\x9e\x8a\x6e\x9c",
+	.secret_size = 404,
+	.b_public_size = 384,
+	.expected_a_public_size = 384,
+	.expected_ss_size = 384,
+	},
+#endif /* IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC7919) */
 	{
 	.secret =
 #ifdef __LITTLE_ENDIAN
-- 
2.26.2

Re: [PATCH v2 05/18] crypto: testmgr - add DH RFC 7919 ffdhe3072 test vector

[Hannes Reinecke]

On 12/9/21 10:03 AM, Nicolai Stange wrote:
> The previous patch introduced support for the safe-prime groups specified
> by RFC 7919. In order to test this functionality, add a corresponding
> ffdhe3072 test vector to testmgr. The choice of ffdhe3072 over e.g.
> ffdhe2048 is justified by the fact that the NVMe spec mandates it for its
> TLS profile.
> 
> The test data has been generated with OpenSSL.
> 
> Note that this new entry provides test coverage for the recent change to
> crypto_dh_encode_key(), which made it to skip the serialization of domain
> parameters for known groups, i.e. those with
> ->group_id != DH_GROUP_ID_UNKNOWN.
> 
> Moreover, a future patch will make the DH implementation to reject domain
> parameters not corresponding to some safe-prime group approved by
> SP800-56Arev3 in FIPS mode and the existing DH test vectors don't qualify.
> So this patch here will ensure that there's still some suitable test vector
> available.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  crypto/testmgr.h | 124 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

[PATCH v2 06/18] crypto: dh - introduce RFC 3526 safe-prime groups

[Nicolai Stange]

A future patch will make the DH implementation to reject domain parameters
not corresponding to any of the safe-prime groups approved by SP800-56Arev3
in FIPS mode.

The MODP groups specified by RFC 3526 are among those approved safe-prime
groups. Make them known to the kernel in order to enable the DH
implementation to recognize those when passed in from e.g. the
keyctl(KEYCTL_DH_COMPUTE) syscall.

More specifically, introduce corresponding members to enum dh_group_id
as well as entries with the resp. domain parameters to the
safe_prime_groups[] array queried by crypto_dh_decode_key(). The resp.
->max_strength value is set to the maximum supported security strength as
specified in SP800-56Arev3.

As the domain parameters consume an substantial amount of space, make
RFC 3526 safe-prime group support selectable by means of the new
CRYPTO_DH_GROUPS_RFC3526 Kconfig option.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/Kconfig      |   6 ++
 crypto/dh_helper.c  | 216 ++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/dh.h |   7 ++
 3 files changed, 229 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 0f039bbf36e2..fcb044bdc90a 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -238,6 +238,12 @@ config CRYPTO_DH_GROUPS_RFC7919
 	  Enable to allow for the use of RFC 7919 DH parameters in FIPS mode,
 	  e.g. via keyctl(KEYCTL_DH_COMPUTE). Otherwise it's safe to say N.
 
+config CRYPTO_DH_GROUPS_RFC3526
+	bool "Support for RFC 3526 MODP group parameters"
+	help
+	  Enable to allow for the use of RFC 3526 DH parameters in FIPS mode,
+	  e.g. via keyctl(KEYCTL_DH_COMPUTE). Otherwise it's safe to say N.
+
 endif
 
 config CRYPTO_ECC
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index b6df9207bcfd..4541a4b1a92f 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -235,6 +235,222 @@ static const struct safe_prime_group
 	"\xd6\x8c\x8b\xb7\xc5\xc6\x42\x4c\xff\xff\xff\xff\xff\xff\xff\xff",
 	},
 #endif /* CONFIG_CRYPTO_DH_GROUPS_RFC7919 */
+#ifdef CONFIG_CRYPTO_DH_GROUPS_RFC3526
+	{
+	.group_id = DH_GROUP_ID_MODP2048,
+	.max_strength = 112,
+	.p_size = 256,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xc9\x0f\xda\xa2\x21\x68\xc2\x34"
+	"\xc4\xc6\x62\x8b\x80\xdc\x1c\xd1\x29\x02\x4e\x08\x8a\x67\xcc\x74"
+	"\x02\x0b\xbe\xa6\x3b\x13\x9b\x22\x51\x4a\x08\x79\x8e\x34\x04\xdd"
+	"\xef\x95\x19\xb3\xcd\x3a\x43\x1b\x30\x2b\x0a\x6d\xf2\x5f\x14\x37"
+	"\x4f\xe1\x35\x6d\x6d\x51\xc2\x45\xe4\x85\xb5\x76\x62\x5e\x7e\xc6"
+	"\xf4\x4c\x42\xe9\xa6\x37\xed\x6b\x0b\xff\x5c\xb6\xf4\x06\xb7\xed"
+	"\xee\x38\x6b\xfb\x5a\x89\x9f\xa5\xae\x9f\x24\x11\x7c\x4b\x1f\xe6"
+	"\x49\x28\x66\x51\xec\xe4\x5b\x3d\xc2\x00\x7c\xb8\xa1\x63\xbf\x05"
+	"\x98\xda\x48\x36\x1c\x55\xd3\x9a\x69\x16\x3f\xa8\xfd\x24\xcf\x5f"
+	"\x83\x65\x5d\x23\xdc\xa3\xad\x96\x1c\x62\xf3\x56\x20\x85\x52\xbb"
+	"\x9e\xd5\x29\x07\x70\x96\x96\x6d\x67\x0c\x35\x4e\x4a\xbc\x98\x04"
+	"\xf1\x74\x6c\x08\xca\x18\x21\x7c\x32\x90\x5e\x46\x2e\x36\xce\x3b"
+	"\xe3\x9e\x77\x2c\x18\x0e\x86\x03\x9b\x27\x83\xa2\xec\x07\xa2\x8f"
+	"\xb5\xc5\x5d\xf0\x6f\x4c\x52\xc9\xde\x2b\xcb\xf6\x95\x58\x17\x18"
+	"\x39\x95\x49\x7c\xea\x95\x6a\xe5\x15\xd2\x26\x18\x98\xfa\x05\x10"
+	"\x15\x72\x8e\x5a\x8a\xac\xaa\x68\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+	{
+	.group_id = DH_GROUP_ID_MODP3072,
+	.max_strength = 128,
+	.p_size = 384,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xc9\x0f\xda\xa2\x21\x68\xc2\x34"
+	"\xc4\xc6\x62\x8b\x80\xdc\x1c\xd1\x29\x02\x4e\x08\x8a\x67\xcc\x74"
+	"\x02\x0b\xbe\xa6\x3b\x13\x9b\x22\x51\x4a\x08\x79\x8e\x34\x04\xdd"
+	"\xef\x95\x19\xb3\xcd\x3a\x43\x1b\x30\x2b\x0a\x6d\xf2\x5f\x14\x37"
+	"\x4f\xe1\x35\x6d\x6d\x51\xc2\x45\xe4\x85\xb5\x76\x62\x5e\x7e\xc6"
+	"\xf4\x4c\x42\xe9\xa6\x37\xed\x6b\x0b\xff\x5c\xb6\xf4\x06\xb7\xed"
+	"\xee\x38\x6b\xfb\x5a\x89\x9f\xa5\xae\x9f\x24\x11\x7c\x4b\x1f\xe6"
+	"\x49\x28\x66\x51\xec\xe4\x5b\x3d\xc2\x00\x7c\xb8\xa1\x63\xbf\x05"
+	"\x98\xda\x48\x36\x1c\x55\xd3\x9a\x69\x16\x3f\xa8\xfd\x24\xcf\x5f"
+	"\x83\x65\x5d\x23\xdc\xa3\xad\x96\x1c\x62\xf3\x56\x20\x85\x52\xbb"
+	"\x9e\xd5\x29\x07\x70\x96\x96\x6d\x67\x0c\x35\x4e\x4a\xbc\x98\x04"
+	"\xf1\x74\x6c\x08\xca\x18\x21\x7c\x32\x90\x5e\x46\x2e\x36\xce\x3b"
+	"\xe3\x9e\x77\x2c\x18\x0e\x86\x03\x9b\x27\x83\xa2\xec\x07\xa2\x8f"
+	"\xb5\xc5\x5d\xf0\x6f\x4c\x52\xc9\xde\x2b\xcb\xf6\x95\x58\x17\x18"
+	"\x39\x95\x49\x7c\xea\x95\x6a\xe5\x15\xd2\x26\x18\x98\xfa\x05\x10"
+	"\x15\x72\x8e\x5a\x8a\xaa\xc4\x2d\xad\x33\x17\x0d\x04\x50\x7a\x33"
+	"\xa8\x55\x21\xab\xdf\x1c\xba\x64\xec\xfb\x85\x04\x58\xdb\xef\x0a"
+	"\x8a\xea\x71\x57\x5d\x06\x0c\x7d\xb3\x97\x0f\x85\xa6\xe1\xe4\xc7"
+	"\xab\xf5\xae\x8c\xdb\x09\x33\xd7\x1e\x8c\x94\xe0\x4a\x25\x61\x9d"
+	"\xce\xe3\xd2\x26\x1a\xd2\xee\x6b\xf1\x2f\xfa\x06\xd9\x8a\x08\x64"
+	"\xd8\x76\x02\x73\x3e\xc8\x6a\x64\x52\x1f\x2b\x18\x17\x7b\x20\x0c"
+	"\xbb\xe1\x17\x57\x7a\x61\x5d\x6c\x77\x09\x88\xc0\xba\xd9\x46\xe2"
+	"\x08\xe2\x4f\xa0\x74\xe5\xab\x31\x43\xdb\x5b\xfc\xe0\xfd\x10\x8e"
+	"\x4b\x82\xd1\x20\xa9\x3a\xd2\xca\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+	{
+	.group_id = DH_GROUP_ID_MODP4096,
+	.max_strength = 152,
+	.p_size = 512,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xc9\x0f\xda\xa2\x21\x68\xc2\x34"
+	"\xc4\xc6\x62\x8b\x80\xdc\x1c\xd1\x29\x02\x4e\x08\x8a\x67\xcc\x74"
+	"\x02\x0b\xbe\xa6\x3b\x13\x9b\x22\x51\x4a\x08\x79\x8e\x34\x04\xdd"
+	"\xef\x95\x19\xb3\xcd\x3a\x43\x1b\x30\x2b\x0a\x6d\xf2\x5f\x14\x37"
+	"\x4f\xe1\x35\x6d\x6d\x51\xc2\x45\xe4\x85\xb5\x76\x62\x5e\x7e\xc6"
+	"\xf4\x4c\x42\xe9\xa6\x37\xed\x6b\x0b\xff\x5c\xb6\xf4\x06\xb7\xed"
+	"\xee\x38\x6b\xfb\x5a\x89\x9f\xa5\xae\x9f\x24\x11\x7c\x4b\x1f\xe6"
+	"\x49\x28\x66\x51\xec\xe4\x5b\x3d\xc2\x00\x7c\xb8\xa1\x63\xbf\x05"
+	"\x98\xda\x48\x36\x1c\x55\xd3\x9a\x69\x16\x3f\xa8\xfd\x24\xcf\x5f"
+	"\x83\x65\x5d\x23\xdc\xa3\xad\x96\x1c\x62\xf3\x56\x20\x85\x52\xbb"
+	"\x9e\xd5\x29\x07\x70\x96\x96\x6d\x67\x0c\x35\x4e\x4a\xbc\x98\x04"
+	"\xf1\x74\x6c\x08\xca\x18\x21\x7c\x32\x90\x5e\x46\x2e\x36\xce\x3b"
+	"\xe3\x9e\x77\x2c\x18\x0e\x86\x03\x9b\x27\x83\xa2\xec\x07\xa2\x8f"
+	"\xb5\xc5\x5d\xf0\x6f\x4c\x52\xc9\xde\x2b\xcb\xf6\x95\x58\x17\x18"
+	"\x39\x95\x49\x7c\xea\x95\x6a\xe5\x15\xd2\x26\x18\x98\xfa\x05\x10"
+	"\x15\x72\x8e\x5a\x8a\xaa\xc4\x2d\xad\x33\x17\x0d\x04\x50\x7a\x33"
+	"\xa8\x55\x21\xab\xdf\x1c\xba\x64\xec\xfb\x85\x04\x58\xdb\xef\x0a"
+	"\x8a\xea\x71\x57\x5d\x06\x0c\x7d\xb3\x97\x0f\x85\xa6\xe1\xe4\xc7"
+	"\xab\xf5\xae\x8c\xdb\x09\x33\xd7\x1e\x8c\x94\xe0\x4a\x25\x61\x9d"
+	"\xce\xe3\xd2\x26\x1a\xd2\xee\x6b\xf1\x2f\xfa\x06\xd9\x8a\x08\x64"
+	"\xd8\x76\x02\x73\x3e\xc8\x6a\x64\x52\x1f\x2b\x18\x17\x7b\x20\x0c"
+	"\xbb\xe1\x17\x57\x7a\x61\x5d\x6c\x77\x09\x88\xc0\xba\xd9\x46\xe2"
+	"\x08\xe2\x4f\xa0\x74\xe5\xab\x31\x43\xdb\x5b\xfc\xe0\xfd\x10\x8e"
+	"\x4b\x82\xd1\x20\xa9\x21\x08\x01\x1a\x72\x3c\x12\xa7\x87\xe6\xd7"
+	"\x88\x71\x9a\x10\xbd\xba\x5b\x26\x99\xc3\x27\x18\x6a\xf4\xe2\x3c"
+	"\x1a\x94\x68\x34\xb6\x15\x0b\xda\x25\x83\xe9\xca\x2a\xd4\x4c\xe8"
+	"\xdb\xbb\xc2\xdb\x04\xde\x8e\xf9\x2e\x8e\xfc\x14\x1f\xbe\xca\xa6"
+	"\x28\x7c\x59\x47\x4e\x6b\xc0\x5d\x99\xb2\x96\x4f\xa0\x90\xc3\xa2"
+	"\x23\x3b\xa1\x86\x51\x5b\xe7\xed\x1f\x61\x29\x70\xce\xe2\xd7\xaf"
+	"\xb8\x1b\xdd\x76\x21\x70\x48\x1c\xd0\x06\x91\x27\xd5\xb0\x5a\xa9"
+	"\x93\xb4\xea\x98\x8d\x8f\xdd\xc1\x86\xff\xb7\xdc\x90\xa6\xc0\x8f"
+	"\x4d\xf4\x35\xc9\x34\x06\x31\x99\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+	{
+	.group_id = DH_GROUP_ID_MODP6144,
+	.max_strength = 176,
+	.p_size = 768,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xc9\x0f\xda\xa2\x21\x68\xc2\x34"
+	"\xc4\xc6\x62\x8b\x80\xdc\x1c\xd1\x29\x02\x4e\x08\x8a\x67\xcc\x74"
+	"\x02\x0b\xbe\xa6\x3b\x13\x9b\x22\x51\x4a\x08\x79\x8e\x34\x04\xdd"
+	"\xef\x95\x19\xb3\xcd\x3a\x43\x1b\x30\x2b\x0a\x6d\xf2\x5f\x14\x37"
+	"\x4f\xe1\x35\x6d\x6d\x51\xc2\x45\xe4\x85\xb5\x76\x62\x5e\x7e\xc6"
+	"\xf4\x4c\x42\xe9\xa6\x37\xed\x6b\x0b\xff\x5c\xb6\xf4\x06\xb7\xed"
+	"\xee\x38\x6b\xfb\x5a\x89\x9f\xa5\xae\x9f\x24\x11\x7c\x4b\x1f\xe6"
+	"\x49\x28\x66\x51\xec\xe4\x5b\x3d\xc2\x00\x7c\xb8\xa1\x63\xbf\x05"
+	"\x98\xda\x48\x36\x1c\x55\xd3\x9a\x69\x16\x3f\xa8\xfd\x24\xcf\x5f"
+	"\x83\x65\x5d\x23\xdc\xa3\xad\x96\x1c\x62\xf3\x56\x20\x85\x52\xbb"
+	"\x9e\xd5\x29\x07\x70\x96\x96\x6d\x67\x0c\x35\x4e\x4a\xbc\x98\x04"
+	"\xf1\x74\x6c\x08\xca\x18\x21\x7c\x32\x90\x5e\x46\x2e\x36\xce\x3b"
+	"\xe3\x9e\x77\x2c\x18\x0e\x86\x03\x9b\x27\x83\xa2\xec\x07\xa2\x8f"
+	"\xb5\xc5\x5d\xf0\x6f\x4c\x52\xc9\xde\x2b\xcb\xf6\x95\x58\x17\x18"
+	"\x39\x95\x49\x7c\xea\x95\x6a\xe5\x15\xd2\x26\x18\x98\xfa\x05\x10"
+	"\x15\x72\x8e\x5a\x8a\xaa\xc4\x2d\xad\x33\x17\x0d\x04\x50\x7a\x33"
+	"\xa8\x55\x21\xab\xdf\x1c\xba\x64\xec\xfb\x85\x04\x58\xdb\xef\x0a"
+	"\x8a\xea\x71\x57\x5d\x06\x0c\x7d\xb3\x97\x0f\x85\xa6\xe1\xe4\xc7"
+	"\xab\xf5\xae\x8c\xdb\x09\x33\xd7\x1e\x8c\x94\xe0\x4a\x25\x61\x9d"
+	"\xce\xe3\xd2\x26\x1a\xd2\xee\x6b\xf1\x2f\xfa\x06\xd9\x8a\x08\x64"
+	"\xd8\x76\x02\x73\x3e\xc8\x6a\x64\x52\x1f\x2b\x18\x17\x7b\x20\x0c"
+	"\xbb\xe1\x17\x57\x7a\x61\x5d\x6c\x77\x09\x88\xc0\xba\xd9\x46\xe2"
+	"\x08\xe2\x4f\xa0\x74\xe5\xab\x31\x43\xdb\x5b\xfc\xe0\xfd\x10\x8e"
+	"\x4b\x82\xd1\x20\xa9\x21\x08\x01\x1a\x72\x3c\x12\xa7\x87\xe6\xd7"
+	"\x88\x71\x9a\x10\xbd\xba\x5b\x26\x99\xc3\x27\x18\x6a\xf4\xe2\x3c"
+	"\x1a\x94\x68\x34\xb6\x15\x0b\xda\x25\x83\xe9\xca\x2a\xd4\x4c\xe8"
+	"\xdb\xbb\xc2\xdb\x04\xde\x8e\xf9\x2e\x8e\xfc\x14\x1f\xbe\xca\xa6"
+	"\x28\x7c\x59\x47\x4e\x6b\xc0\x5d\x99\xb2\x96\x4f\xa0\x90\xc3\xa2"
+	"\x23\x3b\xa1\x86\x51\x5b\xe7\xed\x1f\x61\x29\x70\xce\xe2\xd7\xaf"
+	"\xb8\x1b\xdd\x76\x21\x70\x48\x1c\xd0\x06\x91\x27\xd5\xb0\x5a\xa9"
+	"\x93\xb4\xea\x98\x8d\x8f\xdd\xc1\x86\xff\xb7\xdc\x90\xa6\xc0\x8f"
+	"\x4d\xf4\x35\xc9\x34\x02\x84\x92\x36\xc3\xfa\xb4\xd2\x7c\x70\x26"
+	"\xc1\xd4\xdc\xb2\x60\x26\x46\xde\xc9\x75\x1e\x76\x3d\xba\x37\xbd"
+	"\xf8\xff\x94\x06\xad\x9e\x53\x0e\xe5\xdb\x38\x2f\x41\x30\x01\xae"
+	"\xb0\x6a\x53\xed\x90\x27\xd8\x31\x17\x97\x27\xb0\x86\x5a\x89\x18"
+	"\xda\x3e\xdb\xeb\xcf\x9b\x14\xed\x44\xce\x6c\xba\xce\xd4\xbb\x1b"
+	"\xdb\x7f\x14\x47\xe6\xcc\x25\x4b\x33\x20\x51\x51\x2b\xd7\xaf\x42"
+	"\x6f\xb8\xf4\x01\x37\x8c\xd2\xbf\x59\x83\xca\x01\xc6\x4b\x92\xec"
+	"\xf0\x32\xea\x15\xd1\x72\x1d\x03\xf4\x82\xd7\xce\x6e\x74\xfe\xf6"
+	"\xd5\x5e\x70\x2f\x46\x98\x0c\x82\xb5\xa8\x40\x31\x90\x0b\x1c\x9e"
+	"\x59\xe7\xc9\x7f\xbe\xc7\xe8\xf3\x23\xa9\x7a\x7e\x36\xcc\x88\xbe"
+	"\x0f\x1d\x45\xb7\xff\x58\x5a\xc5\x4b\xd4\x07\xb2\x2b\x41\x54\xaa"
+	"\xcc\x8f\x6d\x7e\xbf\x48\xe1\xd8\x14\xcc\x5e\xd2\x0f\x80\x37\xe0"
+	"\xa7\x97\x15\xee\xf2\x9b\xe3\x28\x06\xa1\xd5\x8b\xb7\xc5\xda\x76"
+	"\xf5\x50\xaa\x3d\x8a\x1f\xbf\xf0\xeb\x19\xcc\xb1\xa3\x13\xd5\x5c"
+	"\xda\x56\xc9\xec\x2e\xf2\x96\x32\x38\x7f\xe8\xd7\x6e\x3c\x04\x68"
+	"\x04\x3e\x8f\x66\x3f\x48\x60\xee\x12\xbf\x2d\x5b\x0b\x74\x74\xd6"
+	"\xe6\x94\xf9\x1e\x6d\xcc\x40\x24\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+	{
+	.group_id = DH_GROUP_ID_MODP8192,
+	.max_strength = 200,
+	.p_size = 1024,
+	.p =
+	"\xff\xff\xff\xff\xff\xff\xff\xff\xc9\x0f\xda\xa2\x21\x68\xc2\x34"
+	"\xc4\xc6\x62\x8b\x80\xdc\x1c\xd1\x29\x02\x4e\x08\x8a\x67\xcc\x74"
+	"\x02\x0b\xbe\xa6\x3b\x13\x9b\x22\x51\x4a\x08\x79\x8e\x34\x04\xdd"
+	"\xef\x95\x19\xb3\xcd\x3a\x43\x1b\x30\x2b\x0a\x6d\xf2\x5f\x14\x37"
+	"\x4f\xe1\x35\x6d\x6d\x51\xc2\x45\xe4\x85\xb5\x76\x62\x5e\x7e\xc6"
+	"\xf4\x4c\x42\xe9\xa6\x37\xed\x6b\x0b\xff\x5c\xb6\xf4\x06\xb7\xed"
+	"\xee\x38\x6b\xfb\x5a\x89\x9f\xa5\xae\x9f\x24\x11\x7c\x4b\x1f\xe6"
+	"\x49\x28\x66\x51\xec\xe4\x5b\x3d\xc2\x00\x7c\xb8\xa1\x63\xbf\x05"
+	"\x98\xda\x48\x36\x1c\x55\xd3\x9a\x69\x16\x3f\xa8\xfd\x24\xcf\x5f"
+	"\x83\x65\x5d\x23\xdc\xa3\xad\x96\x1c\x62\xf3\x56\x20\x85\x52\xbb"
+	"\x9e\xd5\x29\x07\x70\x96\x96\x6d\x67\x0c\x35\x4e\x4a\xbc\x98\x04"
+	"\xf1\x74\x6c\x08\xca\x18\x21\x7c\x32\x90\x5e\x46\x2e\x36\xce\x3b"
+	"\xe3\x9e\x77\x2c\x18\x0e\x86\x03\x9b\x27\x83\xa2\xec\x07\xa2\x8f"
+	"\xb5\xc5\x5d\xf0\x6f\x4c\x52\xc9\xde\x2b\xcb\xf6\x95\x58\x17\x18"
+	"\x39\x95\x49\x7c\xea\x95\x6a\xe5\x15\xd2\x26\x18\x98\xfa\x05\x10"
+	"\x15\x72\x8e\x5a\x8a\xaa\xc4\x2d\xad\x33\x17\x0d\x04\x50\x7a\x33"
+	"\xa8\x55\x21\xab\xdf\x1c\xba\x64\xec\xfb\x85\x04\x58\xdb\xef\x0a"
+	"\x8a\xea\x71\x57\x5d\x06\x0c\x7d\xb3\x97\x0f\x85\xa6\xe1\xe4\xc7"
+	"\xab\xf5\xae\x8c\xdb\x09\x33\xd7\x1e\x8c\x94\xe0\x4a\x25\x61\x9d"
+	"\xce\xe3\xd2\x26\x1a\xd2\xee\x6b\xf1\x2f\xfa\x06\xd9\x8a\x08\x64"
+	"\xd8\x76\x02\x73\x3e\xc8\x6a\x64\x52\x1f\x2b\x18\x17\x7b\x20\x0c"
+	"\xbb\xe1\x17\x57\x7a\x61\x5d\x6c\x77\x09\x88\xc0\xba\xd9\x46\xe2"
+	"\x08\xe2\x4f\xa0\x74\xe5\xab\x31\x43\xdb\x5b\xfc\xe0\xfd\x10\x8e"
+	"\x4b\x82\xd1\x20\xa9\x21\x08\x01\x1a\x72\x3c\x12\xa7\x87\xe6\xd7"
+	"\x88\x71\x9a\x10\xbd\xba\x5b\x26\x99\xc3\x27\x18\x6a\xf4\xe2\x3c"
+	"\x1a\x94\x68\x34\xb6\x15\x0b\xda\x25\x83\xe9\xca\x2a\xd4\x4c\xe8"
+	"\xdb\xbb\xc2\xdb\x04\xde\x8e\xf9\x2e\x8e\xfc\x14\x1f\xbe\xca\xa6"
+	"\x28\x7c\x59\x47\x4e\x6b\xc0\x5d\x99\xb2\x96\x4f\xa0\x90\xc3\xa2"
+	"\x23\x3b\xa1\x86\x51\x5b\xe7\xed\x1f\x61\x29\x70\xce\xe2\xd7\xaf"
+	"\xb8\x1b\xdd\x76\x21\x70\x48\x1c\xd0\x06\x91\x27\xd5\xb0\x5a\xa9"
+	"\x93\xb4\xea\x98\x8d\x8f\xdd\xc1\x86\xff\xb7\xdc\x90\xa6\xc0\x8f"
+	"\x4d\xf4\x35\xc9\x34\x02\x84\x92\x36\xc3\xfa\xb4\xd2\x7c\x70\x26"
+	"\xc1\xd4\xdc\xb2\x60\x26\x46\xde\xc9\x75\x1e\x76\x3d\xba\x37\xbd"
+	"\xf8\xff\x94\x06\xad\x9e\x53\x0e\xe5\xdb\x38\x2f\x41\x30\x01\xae"
+	"\xb0\x6a\x53\xed\x90\x27\xd8\x31\x17\x97\x27\xb0\x86\x5a\x89\x18"
+	"\xda\x3e\xdb\xeb\xcf\x9b\x14\xed\x44\xce\x6c\xba\xce\xd4\xbb\x1b"
+	"\xdb\x7f\x14\x47\xe6\xcc\x25\x4b\x33\x20\x51\x51\x2b\xd7\xaf\x42"
+	"\x6f\xb8\xf4\x01\x37\x8c\xd2\xbf\x59\x83\xca\x01\xc6\x4b\x92\xec"
+	"\xf0\x32\xea\x15\xd1\x72\x1d\x03\xf4\x82\xd7\xce\x6e\x74\xfe\xf6"
+	"\xd5\x5e\x70\x2f\x46\x98\x0c\x82\xb5\xa8\x40\x31\x90\x0b\x1c\x9e"
+	"\x59\xe7\xc9\x7f\xbe\xc7\xe8\xf3\x23\xa9\x7a\x7e\x36\xcc\x88\xbe"
+	"\x0f\x1d\x45\xb7\xff\x58\x5a\xc5\x4b\xd4\x07\xb2\x2b\x41\x54\xaa"
+	"\xcc\x8f\x6d\x7e\xbf\x48\xe1\xd8\x14\xcc\x5e\xd2\x0f\x80\x37\xe0"
+	"\xa7\x97\x15\xee\xf2\x9b\xe3\x28\x06\xa1\xd5\x8b\xb7\xc5\xda\x76"
+	"\xf5\x50\xaa\x3d\x8a\x1f\xbf\xf0\xeb\x19\xcc\xb1\xa3\x13\xd5\x5c"
+	"\xda\x56\xc9\xec\x2e\xf2\x96\x32\x38\x7f\xe8\xd7\x6e\x3c\x04\x68"
+	"\x04\x3e\x8f\x66\x3f\x48\x60\xee\x12\xbf\x2d\x5b\x0b\x74\x74\xd6"
+	"\xe6\x94\xf9\x1e\x6d\xbe\x11\x59\x74\xa3\x92\x6f\x12\xfe\xe5\xe4"
+	"\x38\x77\x7c\xb6\xa9\x32\xdf\x8c\xd8\xbe\xc4\xd0\x73\xb9\x31\xba"
+	"\x3b\xc8\x32\xb6\x8d\x9d\xd3\x00\x74\x1f\xa7\xbf\x8a\xfc\x47\xed"
+	"\x25\x76\xf6\x93\x6b\xa4\x24\x66\x3a\xab\x63\x9c\x5a\xe4\xf5\x68"
+	"\x34\x23\xb4\x74\x2b\xf1\xc9\x78\x23\x8f\x16\xcb\xe3\x9d\x65\x2d"
+	"\xe3\xfd\xb8\xbe\xfc\x84\x8a\xd9\x22\x22\x2e\x04\xa4\x03\x7c\x07"
+	"\x13\xeb\x57\xa8\x1a\x23\xf0\xc7\x34\x73\xfc\x64\x6c\xea\x30\x6b"
+	"\x4b\xcb\xc8\x86\x2f\x83\x85\xdd\xfa\x9d\x4b\x7f\xa2\xc0\x87\xe8"
+	"\x79\x68\x33\x03\xed\x5b\xdd\x3a\x06\x2b\x3c\xf5\xb3\xa2\x78\xa6"
+	"\x6d\x2a\x13\xf8\x3f\x44\xf8\x2d\xdf\x31\x0e\xe0\x74\xab\x6a\x36"
+	"\x45\x97\xe8\x99\xa0\x25\x5d\xc1\x64\xf3\x1c\xc5\x08\x46\x85\x1d"
+	"\xf9\xab\x48\x19\x5d\xed\x7e\xa1\xb1\xd5\x10\xbd\x7e\xe7\x4d\x73"
+	"\xfa\xf3\x6b\xc3\x1e\xcf\xa2\x68\x35\x90\x46\xf4\xeb\x87\x9f\x92"
+	"\x40\x09\x43\x8b\x48\x1c\x6c\xd7\x88\x9a\x00\x2e\xd5\xee\x38\x2b"
+	"\xc9\x19\x0d\xa6\xfc\x02\x6e\x47\x95\x58\xe4\x47\x56\x77\xe9\xaa"
+	"\x9e\x30\x50\xe2\x76\x56\x94\xdf\xc8\x1f\x56\xe8\x80\xb9\x6e\x71"
+	"\x60\xc9\x80\xdd\x98\xed\xd3\xdf\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+#endif /* CONFIG_CRYPTO_DH_GROUPS_RFC3526 */
 };
 
 /* 2 is used as a generator for all safe-prime groups. */
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index 2aee155f1e0b..e238380dee01 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -29,6 +29,13 @@ enum dh_group_id {
 	DH_GROUP_ID_FFDHE6144 = 4,
 	DH_GROUP_ID_FFDHE8192 = 5,
 #endif
+#ifdef CONFIG_CRYPTO_DH_GROUPS_RFC3526
+	DH_GROUP_ID_MODP2048 = 6,
+	DH_GROUP_ID_MODP3072 = 7,
+	DH_GROUP_ID_MODP4096 = 8,
+	DH_GROUP_ID_MODP6144 = 9,
+	DH_GROUP_ID_MODP8192 = 10,
+#endif
 };
 
 /**
-- 
2.26.2

Re: [PATCH v2 06/18] crypto: dh - introduce RFC 3526 safe-prime groups

[Hannes Reinecke]

On 12/9/21 10:03 AM, Nicolai Stange wrote:
> A future patch will make the DH implementation to reject domain parameters
> not corresponding to any of the safe-prime groups approved by SP800-56Arev3
> in FIPS mode.
> 
> The MODP groups specified by RFC 3526 are among those approved safe-prime
> groups. Make them known to the kernel in order to enable the DH
> implementation to recognize those when passed in from e.g. the
> keyctl(KEYCTL_DH_COMPUTE) syscall.
> 
> More specifically, introduce corresponding members to enum dh_group_id
> as well as entries with the resp. domain parameters to the
> safe_prime_groups[] array queried by crypto_dh_decode_key(). The resp.
> ->max_strength value is set to the maximum supported security strength as
> specified in SP800-56Arev3.
> 
> As the domain parameters consume an substantial amount of space, make
> RFC 3526 safe-prime group support selectable by means of the new
> CRYPTO_DH_GROUPS_RFC3526 Kconfig option.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  crypto/Kconfig      |   6 ++
>  crypto/dh_helper.c  | 216 ++++++++++++++++++++++++++++++++++++++++++++
>  include/crypto/dh.h |   7 ++
>  3 files changed, 229 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

[PATCH v2 07/18] crypto: testmgr - add DH RFC 3526 modp2048 test vector

[Nicolai Stange]

The previous patch introduced support for the safe-prime groups specified
by RFC 3526. In order to test this functionality, add a corresponding test
vector to testmgr. The test data has been generated with OpenSSL.

Note that this new entry provides test coverage for the recent change to
crypto_dh_encode_key(), which made it to skip the serialization of domain
parameters for known groups, i.e. those with
->group_id != DH_GROUP_ID_UNKNOWN.

Moreover, a future patch will make the DH implementation to reject domain
parameters not corresponding to some safe-prime group approved by
SP800-56Arev3 in FIPS mode and the existing DH test vectors don't qualify.
So this patch here will ensure that there's still some suitable test vector
available.

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 crypto/testmgr.h | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 26194db387db..8658cf00ea1f 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -1364,6 +1364,98 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.expected_ss_size = 384,
 	},
 #endif /* IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC7919) */
+#if IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC3526)
+	{
+	.secret =
+#ifdef __LITTLE_ENDIAN
+	"\x01\x00" /* type */
+	"\x14\x01" /* len */
+	"\x06\x00\x00\x00" /* group_id == DH_GROUP_ID_MODP2048 */
+	"\x00\x01\x00\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* g_size */
+#else
+	"\x00\x01" /* type */
+	"\x01\x14" /* len */
+	"\x00\x00\x00\x06" /* group_id == DH_GROUP_ID_MODP2048 */
+	"\x00\x00\x01\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* g_size */
+#endif
+	/* xa */
+	"\x38\x77\xec\x02\xc5\xae\xc2\x1c\x4c\x5e\xf5\xa2\xfb\x7e\x06\xf2"
+	"\xa0\x32\x0f\x3d\xf5\xcb\x75\xd0\xd7\x47\x12\x06\xca\x11\x55\xe4"
+	"\x16\xff\x35\xd3\xda\x36\x69\x04\xc4\xd8\x63\x96\xd4\x1d\x92\x6d"
+	"\xd6\x1f\x4b\x22\x7f\xa8\x68\xae\x53\x46\x49\x5a\x06\xfd\x33\xb9"
+	"\x47\x7b\x2c\xaf\x5f\x52\x76\x2d\xe5\x46\x44\xd7\xf1\x5e\xdf\xaa"
+	"\x17\xb5\x3c\x86\x5e\x69\xf9\xf5\x4a\x86\xc6\x58\x77\x81\x88\x78"
+	"\x7d\x5b\xf6\xe3\xd7\x46\x4c\xaf\x75\xf8\x53\x76\xf6\xcc\x6d\xd2"
+	"\x8e\xb7\x0f\x4c\xea\x3e\x82\x55\x82\x34\x5c\x99\x32\x7c\x22\x4b"
+	"\xcc\xd7\xfd\x39\x72\x64\x27\xc6\x5a\x10\xc2\x97\x38\x20\x51\xd2"
+	"\xf3\xf0\x95\xe7\xe4\xfb\x5a\x1e\xb6\x08\x81\xda\xac\x7e\xdf\x85"
+	"\xad\xa5\xdb\xd1\x96\xc6\xab\x9c\x9b\x8e\xa5\x80\x0a\xf0\xce\xf6"
+	"\x60\xb2\x88\xc1\x3a\x77\xb3\x87\xd1\x39\x68\x56\x7b\x8c\x8a\xb4"
+	"\xb5\x35\xd6\x93\xdf\x8e\x43\x3c\x41\xb5\xb5\x5d\xdd\xd2\x36\x93"
+	"\xa3\x09\xeb\x9f\x6c\x13\xac\xcb\xa0\x50\x4e\x7c\x49\x20\xcf\xf7"
+	"\xa6\xfc\xd1\x1d\x50\x72\xdf\x76\x24\xc5\xb9\xb3\x68\x1d\xe2\xdd"
+	"\xd1\xcb\x1b\x53\x2c\xed\x75\xfc\xeb\x36\x20\x9d\x82\xca\xe5\xa7",
+	.b_public =
+	"\x75\x98\x23\x19\xc9\xc2\xe1\x59\x73\xc2\x1d\xc5\x2c\xad\x22\x90"
+	"\xa8\xa4\xb4\xfa\xd7\x67\x5b\xe9\xa1\x0e\x15\x3b\x5d\xae\xd3\x25"
+	"\x29\xfc\x26\x79\xd6\x86\xf2\x21\x20\x86\xd7\x17\xce\xe7\x6a\x74"
+	"\x3e\x2e\x8b\x62\x87\x62\xe9\x27\xc0\x57\xca\x5b\xaf\x86\x22\xd6"
+	"\xdd\xf6\x88\xd2\x86\x21\xf7\x39\x6a\x3f\x52\x17\x03\xdc\xb9\x44"
+	"\x03\xdf\xb5\x6e\x5d\x15\x50\x6f\xf8\x9a\x3c\xee\x9f\xc5\x01\x23"
+	"\xd8\x2d\xb8\x18\x37\xc8\xed\x7d\x46\x27\x03\xc9\xae\x3b\xbf\x9e"
+	"\x4e\x98\x91\x30\x56\xcb\x09\x6b\x8e\xd3\xe5\x87\xfe\x82\x66\x36"
+	"\x2c\xee\x88\x74\x00\x8a\x2d\x36\x39\x2b\xe7\xbd\x18\x21\x36\xd0"
+	"\x98\x34\x6c\xb1\x4f\xbf\xd0\x0c\xd3\x6c\x64\x2e\x04\xfa\x68\x13"
+	"\x51\xaf\x1b\xc8\xc3\xbd\x13\x44\x72\x89\xd5\xa3\xd8\x83\x22\xf1"
+	"\x92\xeb\x5a\x70\x5e\x91\x1e\x86\xb9\x2f\x18\x44\x8c\x5a\xe0\x18"
+	"\x6c\x7a\xc6\x20\x27\x27\xae\x6a\x9e\x1b\x9b\xae\x13\xc9\x73\x22"
+	"\x0c\x0d\xdf\x97\x9c\x87\x06\x48\xdc\xe0\x8d\x83\xe1\x32\x8a\x8f"
+	"\x80\x60\x70\x7c\x7e\x10\x10\xf0\xd7\x49\x09\xfc\xf0\x0e\x11\x3f"
+	"\xb4\x5a\x9e\x3d\x38\x28\x3d\x46\x5a\x63\x6c\x9e\x14\xe3\x7c\x13",
+	.expected_a_public =
+	"\xca\x88\x57\x90\x69\x2d\x30\x40\xbc\x97\xd0\x79\x4b\x9e\x8c\x3c"
+	"\x55\x78\x01\x81\x0c\x62\xa3\x51\x80\xcb\x83\x56\x70\x50\xe8\x41"
+	"\x2d\x72\x0c\x7a\x1d\x9b\xf7\x0d\xe6\x81\x2b\x51\xca\xf7\x6c\xf0"
+	"\x45\x92\x9d\x7e\x3c\xe3\x22\xbc\x16\x5a\x2f\x92\x79\xbe\xea\xbe"
+	"\xa5\x73\xf7\xfa\xbf\x86\x71\x9b\x28\x4f\x32\x86\x44\xdb\xc4\x0f"
+	"\xb6\x30\xdd\x95\xa5\xcb\xa8\x16\x96\x76\x51\x27\xfb\x6e\xc1\x06"
+	"\x19\x28\x8a\xf0\x3d\x92\xe8\x6b\x57\x2a\xfc\x63\x96\xea\xf0\x9b"
+	"\x4e\xbe\xeb\x42\x38\x66\x0d\x47\x6b\xc6\x2b\xb1\xe6\x49\xe4\x82"
+	"\xcf\x74\xb4\x5a\x13\x7b\xaf\x22\x53\x34\x5b\xf2\x6f\xda\x5e\x51"
+	"\x00\xd1\x37\x9d\x9c\x8b\x3e\xe9\x05\x37\x8d\x01\xb9\x64\x06\xdd"
+	"\xee\x10\xa2\x96\xa1\x18\xbf\xb8\xb5\x77\x24\xda\xb0\x7f\x07\x7e"
+	"\x98\xf4\xeb\x0e\x80\x39\x54\x1e\x7e\xf6\x5c\x6b\x02\xf5\x91\x5e"
+	"\x3e\xb2\xa5\xe0\x13\x25\x9b\x04\xf9\xb3\x42\x82\xfe\x6a\x11\x94"
+	"\x4b\x01\x35\x43\xb5\x32\x20\x6e\xc0\x91\xad\x1e\xbe\xdf\xb6\x11"
+	"\x5c\x91\x83\x66\xa0\xe5\x27\x82\x7d\x45\xa8\x70\xa1\x37\xcd\x24"
+	"\xab\xb3\xb5\x13\x97\x61\x72\x7b\x03\x58\x06\xd9\x90\x78\x3c\xd1",
+	.expected_ss =
+	"\xba\x1e\x8c\x44\x39\x9e\xab\xe4\xe2\x75\xae\x54\xe3\xa9\xde\xb8"
+	"\x21\x3f\x46\x54\xb8\xea\xe7\xe3\xd6\x1e\xd0\xf3\x33\x2c\xb9\xb7"
+	"\xbd\x76\x63\xf1\xec\x2e\xf9\xe7\x3b\xa4\xa8\x94\xba\x9b\x34\x1b"
+	"\xfc\xa8\xbd\x89\xd2\x11\xb1\xa0\x02\x76\xe1\xb3\xe9\x89\x63\xc0"
+	"\xc2\xda\x77\x53\xc5\x53\x2d\x1d\x0e\xa5\x14\xac\xf1\x91\xfa\x5b"
+	"\x52\x8e\xeb\x73\x54\x7f\x99\xa6\x39\x17\x32\xcc\x4d\x59\x3a\x4c"
+	"\xd7\xea\xb3\x70\x84\xb4\x04\xb8\xb2\xcd\x77\x6e\x2b\xa1\xc6\xeb"
+	"\xa1\x2e\x0c\x8f\xaa\xd1\x83\xe5\x66\x12\x2c\x99\x72\x52\x2a\xfd"
+	"\x67\x0d\x14\xd7\x11\xd3\xf1\x77\x5f\x86\x06\x21\xcb\x7a\x14\x78"
+	"\x94\x6f\x42\xe9\xa9\xf4\x22\x8e\x94\x6a\x74\xfb\x13\x30\xd3\x41"
+	"\xde\xd3\xac\x36\x88\xc9\x24\xe6\x55\x20\x79\xfb\xd7\x81\x6a\xac"
+	"\x3b\x91\xcb\x34\x33\xb8\x61\x86\xf6\x2c\x88\x14\xe7\x64\x23\xaf"
+	"\x05\x34\x31\x9a\x56\x1e\xe5\xd5\xb6\xe6\x79\xd0\x2d\xcf\x4c\x41"
+	"\x95\x16\x08\xa8\x2c\xdd\x7a\xde\xe0\x77\x10\x71\x9b\x98\xfc\xc1"
+	"\x2c\x48\xd4\xfa\x54\x45\x44\xed\x7f\x42\x92\x63\x9c\xf6\x81\x7f"
+	"\xe0\x66\x55\x6e\x69\xa5\x52\x0b\x4d\x86\x06\x85\xb2\xb0\x7e\x47",
+	.secret_size = 276,
+	.b_public_size = 256,
+	.expected_a_public_size = 256,
+	.expected_ss_size = 256,
+	},
+#endif /* IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC3526) */
 	{
 	.secret =
 #ifdef __LITTLE_ENDIAN
-- 
2.26.2

[PATCH v2 08/18] crypto: testmgr - run only subset of DH vectors based on config

[Nicolai Stange]

With the previous patches, the testmgr now has up to four test vectors for
DH which all test more or less the same thing:
- the two vectors from before this series,
- the vector for the ffdhe3072 group, enabled if
  CONFIG_CRYPTO_DH_GROUPS_RFC7919 is set and
- the vector for the modp2048 group, similarly enabled if
  CONFIG_CRYPTO_DH_GROUPS_RFC3526 is set.

In order to avoid too much redundancy during DH testing, enable only a
subset of these depending on the kernel config:
- if CONFIG_CRYPTO_DH_GROUPS_RFC7919 is set, enable only the ffdhe3072
  vector,
- otherwise, if CONFIG_CRYPTO_DH_GROUPS_RFC3526 is set, enable only
  the modp2048 vector and
- only enable the original two vectors if neither of these options
  has been selected.

Note that an upcoming patch will make the DH implementation to reject any
domain parameters not corresponding to some safe-prime group approved by
SP800-56Arev3 in FIPS mode. Thus, having CONFIG_FIPS enabled, but
both of CONFIG_CRYPTO_DH_GROUPS_RFC7919 and
CONFIG_CRYPTO_DH_GROUPS_RFC3526 unset wouldn't make much sense as it would
render the DH implementation unusable in FIPS mode. Conversely, any
reasonable configuration would ensure that the original, non-conforming
test vectors would not get to run in FIPS mode.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/testmgr.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 8658cf00ea1f..b10d5b9d49a1 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -1363,8 +1363,7 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.expected_a_public_size = 384,
 	.expected_ss_size = 384,
 	},
-#endif /* IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC7919) */
-#if IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC3526)
+#elif IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC3526)
 	{
 	.secret =
 #ifdef __LITTLE_ENDIAN
@@ -1455,7 +1454,7 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.expected_a_public_size = 256,
 	.expected_ss_size = 256,
 	},
-#endif /* IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC3526) */
+#else
 	{
 	.secret =
 #ifdef __LITTLE_ENDIAN
@@ -1674,6 +1673,7 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.expected_a_public_size = 256,
 	.expected_ss_size = 256,
 	}
+#endif
 };
 
 static const struct kpp_testvec curve25519_tv_template[] = {
-- 
2.26.2

Re: [PATCH v2 08/18] crypto: testmgr - run only subset of DH vectors based on config

[Hannes Reinecke]

On 12/9/21 10:03 AM, Nicolai Stange wrote:
> With the previous patches, the testmgr now has up to four test vectors for
> DH which all test more or less the same thing:
> - the two vectors from before this series,
> - the vector for the ffdhe3072 group, enabled if
>   CONFIG_CRYPTO_DH_GROUPS_RFC7919 is set and
> - the vector for the modp2048 group, similarly enabled if
>   CONFIG_CRYPTO_DH_GROUPS_RFC3526 is set.
> 
> In order to avoid too much redundancy during DH testing, enable only a
> subset of these depending on the kernel config:
> - if CONFIG_CRYPTO_DH_GROUPS_RFC7919 is set, enable only the ffdhe3072
>   vector,
> - otherwise, if CONFIG_CRYPTO_DH_GROUPS_RFC3526 is set, enable only
>   the modp2048 vector and
> - only enable the original two vectors if neither of these options
>   has been selected.
> 
> Note that an upcoming patch will make the DH implementation to reject any
> domain parameters not corresponding to some safe-prime group approved by
> SP800-56Arev3 in FIPS mode. Thus, having CONFIG_FIPS enabled, but
> both of CONFIG_CRYPTO_DH_GROUPS_RFC7919 and
> CONFIG_CRYPTO_DH_GROUPS_RFC3526 unset wouldn't make much sense as it would
> render the DH implementation unusable in FIPS mode. Conversely, any
> reasonable configuration would ensure that the original, non-conforming
> test vectors would not get to run in FIPS mode.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  crypto/testmgr.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

[PATCH v2 09/18] crypto: dh - implement private key generation primitive

[Nicolai Stange]

The support for NVME in-band authentication currently in the works ([1])
needs to generate ephemeral DH keys.

Implement crypto_dh_gen_privkey() which is intended to be used from
the DH implementations just in analogy to how ecc_gen_privkey() is used
for ECDH.

Make the new crypto_dh_gen_privkey() to follow the approach specified
in SP800-56Arev3, sec. 5.6.1.1.3 ("Key-Pair Generation Using Extra Random
Bits").

SP800-56Arev3 specifies a lower as well as an upper bound on the generated
key's length:
- it must be >= two times the maximum supported security strength of
  the group in question and
- it must be <= the length of the domain parameter Q.
Both of these are available only for the safe-prime groups from
RFC 3526 or RFC 7919, which had been introduced to the kernel with previous
patches: for any safe-prime group Q = (P - 1)/2 by definition and the
individual maximum supported security strength as specified by
SP800-56Arev3 has already been made available alongside the resp. domain
parameters with said previous patches. Restrict crypto_dh_gen_privkey() to
these safe-prime groups, i.e. to those groups with any group_id but
DH_GROUP_ID_UNKNOWN. Make it pick twice the maximum supported strength
rounded up to the next power of two for the output key size. This choice
respects both, the lower and upper bounds given by SP800-90Arev3 for
all safe-prime groups known to the kernel by now and is also in line with
the NVME base spec 2.0, which requires the key size to be >= 256bits.

[1] https://lkml.kernel.org/r/20211122074727.25988-4-hare@suse.de

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 crypto/Kconfig      |   1 +
 crypto/dh_helper.c  | 128 ++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/dh.h |  22 ++++++++
 3 files changed, 151 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index fcb044bdc90a..578711b02bb3 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -228,6 +228,7 @@ menuconfig CRYPTO_DH
 	tristate "Diffie-Hellman algorithm"
 	select CRYPTO_KPP
 	select MPILIB
+	select CRYPTO_RNG_DEFAULT
 	help
 	  Generic implementation of the Diffie-Hellman algorithm.
 
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 4541a4b1a92f..ec9c4cdf57b2 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -9,6 +9,7 @@
 #include <linux/string.h>
 #include <crypto/dh.h>
 #include <crypto/kpp.h>
+#include <crypto/rng.h>
 
 #define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 4 * sizeof(int))
 
@@ -594,3 +595,130 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(crypto_dh_decode_key);
+
+static u64 __add_u64_to_be(__be64 *dst, unsigned int n, u64 val)
+{
+	unsigned int i;
+
+	for (i = n; val && i > 0; --i) {
+		u64 tmp = be64_to_cpu(dst[i - 1]);
+
+		tmp += val;
+		val = tmp >= val ? 0 : 1;
+		dst[i - 1] = cpu_to_be64(tmp);
+	}
+
+	return val;
+}
+
+int crypto_dh_gen_privkey(enum dh_group_id group_id,
+			  char key[CRYPTO_DH_MAX_PRIVKEY_SIZE],
+			  unsigned int *key_size)
+{
+	const struct safe_prime_group *g;
+	unsigned int n, tmp_size;
+	__be64 *tmp;
+	int err;
+	u64 h, o;
+
+	/*
+	 * Generate a private key following NIST SP800-56Ar3,
+	 * sec. 5.6.1.1.1 and 5.6.1.1.3 resp.. This is supported only
+	 * for the (approved) safe-prime groups.
+	 */
+	g = get_safe_prime_group(group_id);
+	if (!g)
+		return -EINVAL;
+
+	/*
+	 * 5.6.1.1.1: choose key length N such that
+	 * 2 * ->max_strength <= N <= log2(q) + 1 = ->p_size * 8 - 1
+	 * with q = (p - 1) / 2 for the safe-prime groups.
+	 * Choose the lower bound's next power of two for N in order to
+	 * avoid excessively large private keys while still
+	 * maintaining some extra reserve beyond the bare minimum in
+	 * most cases. Note that for each entry in safe_prime_groups[],
+	 * the following holds for such N:
+	 * - N >= 256, in particular it is a multiple of 2^6 = 64
+	 *   bits and
+	 * - N < log2(q) + 1, i.e. N respects the upper bound.
+	 */
+	n = roundup_pow_of_two(2 * g->max_strength);
+	WARN_ON_ONCE(n & ((1u << 6) - 1));
+	n >>= 6; /* Convert N into units of u64. */
+
+	/*
+	 * Reserve one extra u64 to hold the extra random bits
+	 * required as per 5.6.1.1.3.
+	 */
+	tmp_size = (n + 1) * sizeof(__be64);
+	tmp = kmalloc(tmp_size, GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	/*
+	 * 5.6.1.1.3, step 3 (and implicitly step 4): obtain N + 64
+	 * random bits and interpret them as a big endian integer.
+	 */
+	err = -EFAULT;
+	if (crypto_get_default_rng())
+		goto out;
+
+	err = crypto_rng_get_bytes(crypto_default_rng, (u8 *)tmp, tmp_size);
+	crypto_put_default_rng();
+	if (err)
+		goto out;
+
+	/*
+	 * 5.6.1.1.3, step 5 is implicit: 2^N < q and thus,
+	 * M = min(2^N, q) = 2^N.
+	 *
+	 * For step 6, calculate
+	 * key = (tmp[] mod (M - 1)) + 1 = (tmp[] mod (2^N - 1)) + 1.
+	 *
+	 * In order to avoid expensive divisions, note that
+	 * 2^N mod (2^N - 1) = 1 and thus, for any integer h,
+	 * 2^N * h mod (2^N - 1) = h mod (2^N - 1) always holds.
+	 * The big endian integer tmp[] composed of n + 1 64bit words
+	 * may be written as tmp[] = h * 2^N + l, with h = tmp[0]
+	 * representing the 64 most significant bits and l
+	 * corresponding to the remaining 2^N bits. With the remark
+	 * from above,
+	 * h * 2^N + l mod (2^N - 1) = l + h mod (2^N - 1).
+	 * As both, l and h are less than 2^N, their sum after
+	 * this first reduction is guaranteed to be <= 2^(N + 1) - 2.
+	 * Or equivalently, that their sum can again be written as
+	 * h' * 2^N + l' with h' now either zero or one and if one,
+	 * then l' <= 2^N - 2. Thus, all bits at positions >= N will
+	 * be zero after a second reduction:
+	 * h' * 2^N + l' mod (2^N - 1) = l' + h' mod (2^N - 1).
+	 * At this point, it is still possible that
+	 * l' + h' = 2^N - 1, i.e. that l' + h' mod (2^N - 1)
+	 * is zero. This condition will be detected below by means of
+	 * the final increment overflowing in this case.
+	 */
+	h = be64_to_cpu(tmp[0]);
+	h = __add_u64_to_be(tmp + 1, n, h);
+	h = __add_u64_to_be(tmp + 1, n, h);
+	WARN_ON_ONCE(h);
+
+	/* Increment to obtain the final result. */
+	o = __add_u64_to_be(tmp + 1, n, 1);
+	/*
+	 * The overflow bit o from the increment is either zero or
+	 * one. If zero, tmp[1:n] holds the final result in big-endian
+	 * order. If one, tmp[1:n] is zero now, but needs to be set to
+	 * one, c.f. above.
+	 */
+	if (o)
+		tmp[n] = cpu_to_be64(1);
+
+	/* n is in units of u64, convert to bytes. */
+	*key_size = n << 3;
+	memcpy(key, &tmp[1], *key_size);
+
+out:
+	kfree_sensitive(tmp);
+	return err;
+}
+EXPORT_SYMBOL_GPL(crypto_dh_gen_privkey);
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index e238380dee01..b1917cc98867 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -99,4 +99,26 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params);
  */
 int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params);
 
+/*
+ * The maximum key length is two times the max. sec. strength of the
+ * safe-prime groups, rounded up to the next power of two.
+ */
+#define CRYPTO_DH_MAX_PRIVKEY_SIZE (512 / 8)
+
+/**
+ * crypto_dh_gen_privkey() - generate a DH private key
+ * @buf:	The DH group to generate a key for
+ * @key:	Buffer provided by the caller to receive the generated
+ *		key
+ * @key_size:	Pointer to an unsigned integer the generated key's length
+ *		will be stored in
+ *
+ * This function is intended to generate an ephemeral DH key.
+ *
+ * Return:	Negative error code on failure, 0 on success
+ */
+int crypto_dh_gen_privkey(enum dh_group_id group_id,
+			  char key[CRYPTO_DH_MAX_PRIVKEY_SIZE],
+			  unsigned int *key_size);
+
 #endif
-- 
2.26.2

[PATCH v2 10/18] crypto: dh - introduce support for ephemeral key generation to dh-generic

[Nicolai Stange]

The support for NVME in-band authentication currently in the works ([1])
needs to generate ephemeral DH keys. Make dh-generic's ->set_secret()
to generate an ephemeral key via the recently added crypto_dh_gen_privkey()
in case the input ->key_size is zero. Note that this behaviour is in
analogy to ecdh's ->set_secret().

[1] https://lkml.kernel.org/r/20211122074727.25988-4-hare@suse.de

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 crypto/dh.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index 131b80064cb1..2e49b114e038 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -71,25 +71,41 @@ static int dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 {
 	struct dh_ctx *ctx = dh_get_ctx(tfm);
 	struct dh params;
+	char key[CRYPTO_DH_MAX_PRIVKEY_SIZE];
+	int err;
 
 	/* Free the old MPI key if any */
 	dh_clear_ctx(ctx);
 
-	if (crypto_dh_decode_key(buf, len, &params) < 0)
+	err = crypto_dh_decode_key(buf, len, &params);
+	if (err)
 		goto err_clear_ctx;
 
-	if (dh_set_params(ctx, &params) < 0)
+	if (!params.key_size) {
+		err = crypto_dh_gen_privkey(params.group_id, key,
+					    &params.key_size);
+		if (err)
+			goto err_clear_ctx;
+		params.key = key;
+	}
+
+	err = dh_set_params(ctx, &params);
+	if (err)
 		goto err_clear_ctx;
 
 	ctx->xa = mpi_read_raw_data(params.key, params.key_size);
-	if (!ctx->xa)
+	if (!ctx->xa) {
+		err = -EINVAL;
 		goto err_clear_ctx;
+	}
+
+	memzero_explicit(key, sizeof(key));
 
 	return 0;
 
 err_clear_ctx:
 	dh_clear_ctx(ctx);
-	return -EINVAL;
+	return err;
 }
 
 /*
-- 
2.26.2

[PATCH v2 11/18] crypto: dh - introduce support for ephemeral key generation to hpre driver

[Nicolai Stange]

A previous patch made the dh-generic implementation's ->set_secret() to
generate an ephemeral key in case the input ->key_size is zero, just in
analogy with ecdh. Make the hpre crypto driver's DH implementation to
behave consistently by doing the same.

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/crypto/hisilicon/hpre/hpre_crypto.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/crypto/hisilicon/hpre/hpre_crypto.c b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
index a032c192ef1d..02ca79e263f1 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_crypto.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
@@ -701,11 +701,20 @@ static int hpre_dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 {
 	struct hpre_ctx *ctx = kpp_tfm_ctx(tfm);
 	struct dh params;
+	char key[CRYPTO_DH_MAX_PRIVKEY_SIZE];
 	int ret;
 
 	if (crypto_dh_decode_key(buf, len, &params) < 0)
 		return -EINVAL;
 
+	if (!params.key_size) {
+		ret = crypto_dh_gen_privkey(params.group_id, key,
+					    &params.key_size);
+		if (ret)
+			return ret;
+		params.key = key;
+	}
+
 	/* Free old secret if any */
 	hpre_dh_clear_ctx(ctx, false);
 
@@ -716,6 +725,8 @@ static int hpre_dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 	memcpy(ctx->dh.xa_p + (ctx->key_sz - params.key_size), params.key,
 	       params.key_size);
 
+	memzero_explicit(key, sizeof(key));
+
 	return 0;
 
 err_clear_ctx:
-- 
2.26.2

[PATCH v2 12/18] crypto: dh - introduce support for ephemeral key generation to qat driver

[Nicolai Stange]

A previous patch made the dh-generic implementation's ->set_secret() to
generate an ephemeral key in case the input ->key_size is zero, just in
analogy with ecdh. Make the qat crypto driver's DH implementation to
behave consistently by doing the same.

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/crypto/qat/qat_common/qat_asym_algs.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/crypto/qat/qat_common/qat_asym_algs.c b/drivers/crypto/qat/qat_common/qat_asym_algs.c
index b0b78445418b..e0d3a70fa6b1 100644
--- a/drivers/crypto/qat/qat_common/qat_asym_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_asym_algs.c
@@ -441,11 +441,20 @@ static int qat_dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 	struct qat_dh_ctx *ctx = kpp_tfm_ctx(tfm);
 	struct device *dev = &GET_DEV(ctx->inst->accel_dev);
 	struct dh params;
+	char key[CRYPTO_DH_MAX_PRIVKEY_SIZE];
 	int ret;
 
 	if (crypto_dh_decode_key(buf, len, &params) < 0)
 		return -EINVAL;
 
+	if (!params.key_size) {
+		ret = crypto_dh_gen_privkey(params.group_id, key,
+					    &params.key_size);
+		if (ret)
+			return ret;
+		params.key = key;
+	}
+
 	/* Free old secret if any */
 	qat_dh_clear_ctx(dev, ctx);
 
-- 
2.26.2

Re: [PATCH v2 12/18] crypto: dh - introduce support for ephemeral key generation to qat driver

[Giovanni Cabiddu]

On Thu, Dec 09, 2021 at 09:03:52AM +0000, Nicolai Stange wrote:
> A previous patch made the dh-generic implementation's ->set_secret() to
> generate an ephemeral key in case the input ->key_size is zero, just in
> analogy with ecdh. Make the qat crypto driver's DH implementation to
> behave consistently by doing the same.
I ran a few tests on QAT GEN2 HW and this patch/set does not causes
regressions.

On the headline of the commit, should this be crypto: qat - ... ?

> Signed-off-by: Nicolai Stange <nstange@suse.de>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
 Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>

-- 
Giovanni

[PATCH v2 13/18] crypto: testmgr - add DH test vectors for key generation

[Nicolai Stange]

Now that all DH implementations support ephemeral key generation triggered
by passing a ->key_size of zero to ->set_secret(), it's certainly
worthwhile to build upon the testmgr's do_test_kpp() ->genkey facility to
test it.

Add two ->genkey DH test vectors to the testmgr, one for the RFC 7919
ffdhe3072 group and another one for the RFC 3526 modp2048 group. For the
resp. party B's keypair, just reuse the already available values
previously specified for party A in the existing known answer tests. This
will enable the compiler to merge these rather large data strings.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/testmgr.h | 164 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index b10d5b9d49a1..b6aa4905c5eb 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -1363,6 +1363,96 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.expected_a_public_size = 384,
 	.expected_ss_size = 384,
 	},
+	{
+	.secret =
+#ifdef __LITTLE_ENDIAN
+	"\x01\x00" /* type */
+	"\x14\x00" /* len */
+	"\x02\x00\x00\x00" /* group_id == DH_GROUP_ID_FFDHE3072 */
+	"\x00\x00\x00\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00", /* g_size */
+#else
+	"\x00\x01" /* type */
+	"\x00\x14" /* len */
+	"\x00\x00\x00\x02" /* group_id == DH_GROUP_ID_FFDHE3072 */
+	"\x00\x00\x00\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00", /* g_size */
+#endif
+	.b_secret =
+#ifdef __LITTLE_ENDIAN
+	"\x01\x00" /* type */
+	"\x94\x01" /* len */
+	"\x02\x00\x00\x00" /* group_id == DH_GROUP_ID_FFDHE3072 */
+	"\x80\x01\x00\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* g_size */
+#else
+	"\x00\x01" /* type */
+	"\x01\x94" /* len */
+	"\x00\x00\x00\x02" /* group_id == DH_GROUP_ID_FFDHE3072 */
+	"\x00\x00\x01\x80" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* g_size */
+#endif
+	/* xa */
+	"\x6b\xb4\x97\x23\xfa\xc8\x5e\xa9\x7b\x63\xe7\x3e\x0e\x99\xc3\xb9"
+	"\xda\xb7\x48\x0d\xc3\xb1\xbf\x4f\x17\xc7\xa9\x51\xf6\x64\xff\xc4"
+	"\x31\x58\x87\x25\x83\x2c\x00\xf0\x41\x29\xf7\xee\xf9\xe6\x36\x76"
+	"\xd6\x3a\x24\xbe\xa7\x07\x0b\x93\xc7\x9f\x6c\x75\x0a\x26\x75\x76"
+	"\xe3\x0c\x42\xe0\x00\x04\x69\xd9\xec\x0b\x59\x54\x28\x8f\xd7\x9a"
+	"\x63\xf4\x5b\xdf\x85\x65\xc4\xe1\x95\x27\x4a\x42\xad\x36\x47\xa9"
+	"\x0a\xf8\x14\x1c\xf3\x94\x3b\x7e\x47\x99\x35\xa8\x18\xec\x70\x10"
+	"\xdf\xcb\xd2\x78\x88\xc1\x2d\x59\x93\xc1\xa4\x6d\xd7\x1d\xb9\xd5"
+	"\xf8\x30\x06\x7f\x98\x90\x0c\x74\x5e\x89\x2f\x64\x5a\xad\x5f\x53"
+	"\xb2\xa3\xa8\x83\xbf\xfc\x37\xef\xb8\x36\x0a\x5c\x62\x81\x64\x74"
+	"\x16\x2f\x45\x39\x2a\x91\x26\x87\xc0\x12\xcc\x75\x11\xa3\xa1\xc5"
+	"\xae\x20\xcf\xcb\x20\x25\x6b\x7a\x31\x93\x9d\x38\xb9\x57\x72\x46"
+	"\xd4\x84\x65\x87\xf1\xb5\xd3\xab\xfc\xc3\x4d\x40\x92\x94\x1e\xcd"
+	"\x1c\x87\xec\x3f\xcd\xbe\xd0\x95\x6b\x40\x02\xdd\x62\xeb\x0a\xda"
+	"\x4f\xbe\x8e\x32\x48\x8b\x6d\x83\xa0\x96\x62\x23\xec\x83\x91\x44"
+	"\xf9\x72\x01\xac\xa0\xe4\x72\x1d\x5a\x75\x05\x57\x90\xae\x7e\xb4"
+	"\x71\x39\x01\x05\xdc\xe9\xee\xcb\xf0\x61\x28\x91\x69\x8c\x31\x03"
+	"\x7a\x92\x15\xa1\x58\x67\x3d\x70\x82\xa6\x2c\xfe\x10\x56\x58\xd3"
+	"\x94\x67\xe1\xbe\xee\xc1\x64\x5c\x4b\xc8\x28\x3d\xc5\x66\x3a\xab"
+	"\x22\xc1\x7e\xa1\xbb\xf3\x19\x3b\xda\x46\x82\x45\xd4\x3c\x7c\xc6"
+	"\xce\x1f\x7f\x95\xa2\x17\xff\x88\xba\xd6\x4d\xdb\xd2\xea\xde\x39"
+	"\xd6\xa5\x18\x73\xbb\x64\x6e\x79\xe9\xdc\x3f\x92\x7f\xda\x1f\x49"
+	"\x33\x70\x65\x73\xa2\xd9\x06\xb8\x1b\x29\x29\x1a\xe0\xa3\xe6\x05"
+	"\x9a\xa8\xc2\x4e\x7a\x78\x1d\x22\x57\x21\xc8\xa3\x8d\x66\x3e\x23",
+	.b_public =
+	"\x1b\x6a\xba\xea\xa3\xcc\x50\x69\xa9\x41\x89\xaf\x04\xe1\x44\x22"
+	"\x97\x20\xd1\xf6\x1e\xcb\x64\x36\x6f\xee\x0b\x16\xc1\xd9\x91\xbe"
+	"\x57\xc8\xd9\xf2\xa1\x96\x91\xec\x41\xc7\x79\x00\x1a\x48\x25\x55"
+	"\xbe\xf3\x20\x8c\x38\xc6\x7b\xf2\x8b\x5a\xc3\xb5\x87\x0a\x86\x3d"
+	"\xb7\xd6\xce\xb0\x96\x2e\x5d\xc4\x00\x5e\x42\xe4\xe5\x50\x4f\xb8"
+	"\x6f\x18\xa4\xe1\xd3\x20\xfc\x3c\xf5\x0a\xff\x23\xa6\x5b\xb4\x17"
+	"\x3e\x7b\xdf\xb9\xb5\x3c\x1b\x76\x29\xcd\xb4\x46\x4f\x27\x8f\xd2"
+	"\xe8\x27\x66\xdb\xe8\xb3\xf5\xe1\xd0\x04\xcd\x89\xff\xba\x76\x67"
+	"\xe8\x4d\xcf\x86\x1c\x8a\xd1\xcf\x99\x27\xfb\xa9\x78\xcc\x94\xaf"
+	"\x3d\x04\xfd\x25\xc0\x47\xfa\x29\x80\x05\xf4\xde\xad\xdb\xab\x12"
+	"\xb0\x2b\x8e\xca\x02\x06\x6d\xad\x3e\x09\xb1\x22\xa3\xf5\x4c\x6d"
+	"\x69\x99\x58\x8b\xd8\x45\x2e\xe0\xc9\x3c\xf7\x92\xce\x21\x90\x6b"
+	"\x3b\x65\x9f\x64\x79\x8d\x67\x22\x1a\x37\xd3\xee\x51\xe2\xe7\x5a"
+	"\x93\x51\xaa\x3c\x4b\x04\x16\x32\xef\xe3\x66\xbe\x18\x94\x88\x64"
+	"\x79\xce\x06\x3f\xb8\xd6\xee\xdc\x13\x79\x6f\x20\x14\xc2\x6b\xce"
+	"\xc8\xda\x42\xa5\x93\x5b\xe4\x7f\x1a\xe6\xda\x0f\xb3\xc1\x5f\x30"
+	"\x50\x76\xe8\x37\x3d\xca\x77\x2c\xa8\xe4\x3b\xf9\x6f\xe0\x17\xed"
+	"\x0e\xef\xb7\x31\x14\xb5\xea\xd9\x39\x22\x89\xb6\x40\x57\xcc\x84"
+	"\xef\x73\xa7\xe9\x27\x21\x85\x89\xfa\xaf\x03\xda\x9c\x8b\xfd\x52"
+	"\x7d\xb0\xa4\xe4\xf9\xd8\x90\x55\xc4\x39\xd6\x9d\xaf\x3b\xce\xac"
+	"\xaa\x36\x14\x7a\x9b\x8b\x12\x43\xe1\xca\x61\xae\x46\x5b\xe7\xe5"
+	"\x88\x32\x80\xa0\x2d\x51\xbb\x2f\xea\xeb\x3c\x71\xb2\xae\xce\xca"
+	"\x61\xd2\x76\xe0\x45\x46\x78\x4e\x09\x2d\xc2\x54\xc2\xa9\xc7\xa8"
+	"\x55\x8e\x72\xa4\x8b\x8a\xc9\x01\xdb\xe9\x58\x11\xa1\xc4\xe7\x12",
+	.secret_size = 20,
+	.b_secret_size = 404,
+	.b_public_size = 384,
+	.expected_a_public_size = 384,
+	.expected_ss_size = 384,
+	.genkey = true,
+	},
 #elif IS_ENABLED(CONFIG_CRYPTO_DH_GROUPS_RFC3526)
 	{
 	.secret =
@@ -1454,6 +1544,80 @@ static const struct kpp_testvec dh_tv_template[] = {
 	.expected_a_public_size = 256,
 	.expected_ss_size = 256,
 	},
+	{
+	.secret =
+#ifdef __LITTLE_ENDIAN
+	"\x01\x00" /* type */
+	"\x14\x00" /* len */
+	"\x06\x00\x00\x00" /* group_id == DH_GROUP_ID_MODP2048 */
+	"\x00\x00\x00\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00", /* g_size */
+#else
+	"\x00\x01" /* type */
+	"\x00\x14" /* len */
+	"\x00\x00\x00\x06" /* group_id == DH_GROUP_ID_MODP2048 */
+	"\x00\x00\x00\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00", /* g_size */
+#endif
+	.b_secret =
+#ifdef __LITTLE_ENDIAN
+	"\x01\x00" /* type */
+	"\x14\x01" /* len */
+	"\x06\x00\x00\x00" /* group_id == DH_GROUP_ID_MODP2048 */
+	"\x00\x01\x00\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* g_size */
+#else
+	"\x00\x01" /* type */
+	"\x01\x14" /* len */
+	"\x00\x00\x00\x06" /* group_id == DH_GROUP_ID_MODP2048 */
+	"\x00\x00\x01\x00" /* key_size */
+	"\x00\x00\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* g_size */
+#endif
+	/* xa */
+	"\x38\x77\xec\x02\xc5\xae\xc2\x1c\x4c\x5e\xf5\xa2\xfb\x7e\x06\xf2"
+	"\xa0\x32\x0f\x3d\xf5\xcb\x75\xd0\xd7\x47\x12\x06\xca\x11\x55\xe4"
+	"\x16\xff\x35\xd3\xda\x36\x69\x04\xc4\xd8\x63\x96\xd4\x1d\x92\x6d"
+	"\xd6\x1f\x4b\x22\x7f\xa8\x68\xae\x53\x46\x49\x5a\x06\xfd\x33\xb9"
+	"\x47\x7b\x2c\xaf\x5f\x52\x76\x2d\xe5\x46\x44\xd7\xf1\x5e\xdf\xaa"
+	"\x17\xb5\x3c\x86\x5e\x69\xf9\xf5\x4a\x86\xc6\x58\x77\x81\x88\x78"
+	"\x7d\x5b\xf6\xe3\xd7\x46\x4c\xaf\x75\xf8\x53\x76\xf6\xcc\x6d\xd2"
+	"\x8e\xb7\x0f\x4c\xea\x3e\x82\x55\x82\x34\x5c\x99\x32\x7c\x22\x4b"
+	"\xcc\xd7\xfd\x39\x72\x64\x27\xc6\x5a\x10\xc2\x97\x38\x20\x51\xd2"
+	"\xf3\xf0\x95\xe7\xe4\xfb\x5a\x1e\xb6\x08\x81\xda\xac\x7e\xdf\x85"
+	"\xad\xa5\xdb\xd1\x96\xc6\xab\x9c\x9b\x8e\xa5\x80\x0a\xf0\xce\xf6"
+	"\x60\xb2\x88\xc1\x3a\x77\xb3\x87\xd1\x39\x68\x56\x7b\x8c\x8a\xb4"
+	"\xb5\x35\xd6\x93\xdf\x8e\x43\x3c\x41\xb5\xb5\x5d\xdd\xd2\x36\x93"
+	"\xa3\x09\xeb\x9f\x6c\x13\xac\xcb\xa0\x50\x4e\x7c\x49\x20\xcf\xf7"
+	"\xa6\xfc\xd1\x1d\x50\x72\xdf\x76\x24\xc5\xb9\xb3\x68\x1d\xe2\xdd"
+	"\xd1\xcb\x1b\x53\x2c\xed\x75\xfc\xeb\x36\x20\x9d\x82\xca\xe5\xa7",
+	.b_public =
+	"\xca\x88\x57\x90\x69\x2d\x30\x40\xbc\x97\xd0\x79\x4b\x9e\x8c\x3c"
+	"\x55\x78\x01\x81\x0c\x62\xa3\x51\x80\xcb\x83\x56\x70\x50\xe8\x41"
+	"\x2d\x72\x0c\x7a\x1d\x9b\xf7\x0d\xe6\x81\x2b\x51\xca\xf7\x6c\xf0"
+	"\x45\x92\x9d\x7e\x3c\xe3\x22\xbc\x16\x5a\x2f\x92\x79\xbe\xea\xbe"
+	"\xa5\x73\xf7\xfa\xbf\x86\x71\x9b\x28\x4f\x32\x86\x44\xdb\xc4\x0f"
+	"\xb6\x30\xdd\x95\xa5\xcb\xa8\x16\x96\x76\x51\x27\xfb\x6e\xc1\x06"
+	"\x19\x28\x8a\xf0\x3d\x92\xe8\x6b\x57\x2a\xfc\x63\x96\xea\xf0\x9b"
+	"\x4e\xbe\xeb\x42\x38\x66\x0d\x47\x6b\xc6\x2b\xb1\xe6\x49\xe4\x82"
+	"\xcf\x74\xb4\x5a\x13\x7b\xaf\x22\x53\x34\x5b\xf2\x6f\xda\x5e\x51"
+	"\x00\xd1\x37\x9d\x9c\x8b\x3e\xe9\x05\x37\x8d\x01\xb9\x64\x06\xdd"
+	"\xee\x10\xa2\x96\xa1\x18\xbf\xb8\xb5\x77\x24\xda\xb0\x7f\x07\x7e"
+	"\x98\xf4\xeb\x0e\x80\x39\x54\x1e\x7e\xf6\x5c\x6b\x02\xf5\x91\x5e"
+	"\x3e\xb2\xa5\xe0\x13\x25\x9b\x04\xf9\xb3\x42\x82\xfe\x6a\x11\x94"
+	"\x4b\x01\x35\x43\xb5\x32\x20\x6e\xc0\x91\xad\x1e\xbe\xdf\xb6\x11"
+	"\x5c\x91\x83\x66\xa0\xe5\x27\x82\x7d\x45\xa8\x70\xa1\x37\xcd\x24"
+	"\xab\xb3\xb5\x13\x97\x61\x72\x7b\x03\x58\x06\xd9\x90\x78\x3c\xd1",
+	.secret_size = 20,
+	.b_secret_size = 276,
+	.b_public_size = 256,
+	.expected_a_public_size = 256,
+	.expected_ss_size = 256,
+	.genkey = true,
+	},
 #else
 	{
 	.secret =
-- 
2.26.2

Re: [PATCH v2 13/18] crypto: testmgr - add DH test vectors for key generation

[Hannes Reinecke]

On 12/9/21 10:03 AM, Nicolai Stange wrote:
> Now that all DH implementations support ephemeral key generation triggered
> by passing a ->key_size of zero to ->set_secret(), it's certainly
> worthwhile to build upon the testmgr's do_test_kpp() ->genkey facility to
> test it.
> 
> Add two ->genkey DH test vectors to the testmgr, one for the RFC 7919
> ffdhe3072 group and another one for the RFC 3526 modp2048 group. For the
> resp. party B's keypair, just reuse the already available values
> previously specified for party A in the existing known answer tests. This
> will enable the compiler to merge these rather large data strings.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  crypto/testmgr.h | 164 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 164 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

[PATCH v2 14/18] lib/mpi: export mpi_rshift

[Nicolai Stange]

A subsequent patch will make the crypto/dh's dh_is_pubkey_valid() to
calculate a safe-prime groups Q parameter from P: Q = (P - 1) / 2. For
implementing this, mpi_rshift() will be needed. Export it so that it's
accessible from crypto/dh.

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 lib/mpi/mpi-bit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/mpi/mpi-bit.c b/lib/mpi/mpi-bit.c
index 142b680835df..070ba784c9f1 100644
--- a/lib/mpi/mpi-bit.c
+++ b/lib/mpi/mpi-bit.c
@@ -242,6 +242,7 @@ void mpi_rshift(MPI x, MPI a, unsigned int n)
 	}
 	MPN_NORMALIZE(x->d, x->nlimbs);
 }
+EXPORT_SYMBOL_GPL(mpi_rshift);
 
 /****************
  * Shift A by COUNT limbs to the left
-- 
2.26.2

[PATCH v2 15/18] crypto: dh - store group id in dh-generic's dh_ctx

[Nicolai Stange]

A subsequent patch will make the crypto/dh's dh_is_pubkey_valid() to
calculate the Q value from the P domain parameter for safe-prime groups,
for which by definition Q = (P - 1)/2. However, dh_is_pubkey_valid() will
need to check first whether the group in question is actually a safe-prime
group. In order to make this information available, introduce a new
->group_id member to struct dh_ctx and let dh_set_params() set it to the
value found in the struct dh as deserialized via crypto_dh_decode_key().

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 crypto/dh.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crypto/dh.c b/crypto/dh.c
index 2e49b114e038..38547c5301da 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -13,6 +13,7 @@
 #include <linux/mpi.h>
 
 struct dh_ctx {
+	enum dh_group_id group_id;
 	MPI p;	/* Value is guaranteed to be set. */
 	MPI q;	/* Value is optional. */
 	MPI g;	/* Value is guaranteed to be set. */
@@ -55,6 +56,8 @@ static int dh_set_params(struct dh_ctx *ctx, struct dh *params)
 	if (dh_check_params_length(params->p_size << 3))
 		return -EINVAL;
 
+	ctx->group_id = params->group_id;
+
 	ctx->p = mpi_read_raw_data(params->p, params->p_size);
 	if (!ctx->p)
 		return -EINVAL;
-- 
2.26.2

[PATCH v2 16/18] crypto: dh - calculate Q from P for the full public key verification

[Nicolai Stange]

As the ->q in struct dh_ctx gets never set anywhere, the code
in dh_is_pubkey_valid() for doing the full public key validation in
accordance to SP800-56Arev3 is effectively dead.

However, for safe-prime groups, Q = (P - 1)/2 by definition and this
enables dh_is_pubkey_valid() to calculate Q on the fly for these groups.
Implement this.

With this change, the last code accessing struct dh_ctx's ->q is now gone.
Remove this member from struct dh_ctx.

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 crypto/dh.c | 40 +++++++++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index 38547c5301da..bd3ea51fbeb0 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -15,7 +15,6 @@
 struct dh_ctx {
 	enum dh_group_id group_id;
 	MPI p;	/* Value is guaranteed to be set. */
-	MPI q;	/* Value is optional. */
 	MPI g;	/* Value is guaranteed to be set. */
 	MPI xa;	/* Value is guaranteed to be set. */
 };
@@ -23,7 +22,6 @@ struct dh_ctx {
 static void dh_clear_ctx(struct dh_ctx *ctx)
 {
 	mpi_free(ctx->p);
-	mpi_free(ctx->q);
 	mpi_free(ctx->g);
 	mpi_free(ctx->xa);
 	memset(ctx, 0, sizeof(*ctx));
@@ -114,11 +112,12 @@ static int dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 /*
  * SP800-56A public key verification:
  *
- * * If Q is provided as part of the domain paramenters, a full validation
- *   according to SP800-56A section 5.6.2.3.1 is performed.
+ * * For safe-prime groups, Q can be computed trivially from P and a
+ *   full validation according to SP800-56A section 5.6.2.3.1 is
+ *   performed.
  *
- * * If Q is not provided, a partial validation according to SP800-56A section
- *   5.6.2.3.2 is performed.
+ * * For all other sets of group parameters, only a partial validation
+ *   according to SP800-56A section 5.6.2.3.2 is performed.
  */
 static int dh_is_pubkey_valid(struct dh_ctx *ctx, MPI y)
 {
@@ -129,21 +128,40 @@ static int dh_is_pubkey_valid(struct dh_ctx *ctx, MPI y)
 	 * Step 1: Verify that 2 <= y <= p - 2.
 	 *
 	 * The upper limit check is actually y < p instead of y < p - 1
-	 * as the mpi_sub_ui function is yet missing.
+	 * in order to save one mpi_sub_ui() invocation here. Note that
+	 * p - 1 is the non-trivial element of the subgroup of order 2 and
+	 * thus, the check on y^q below would fail if y == p - 1.
 	 */
 	if (mpi_cmp_ui(y, 1) < 1 || mpi_cmp(y, ctx->p) >= 0)
 		return -EINVAL;
 
-	/* Step 2: Verify that 1 = y^q mod p */
-	if (ctx->q) {
-		MPI val = mpi_alloc(0);
+	/*
+	 * Step 2: Verify that 1 = y^q mod p
+	 *
+	 * For the safe-prime groups q = (p - 1)/2.
+	 */
+	if (ctx->group_id != DH_GROUP_ID_UNKNOWN) {
+		MPI val, q;
 		int ret;
 
+		val = mpi_alloc(0);
 		if (!val)
 			return -ENOMEM;
 
-		ret = mpi_powm(val, y, ctx->q, ctx->p);
+		q = mpi_alloc(mpi_get_nlimbs(ctx->p));
+		if (!q) {
+			mpi_free(val);
+			return -ENOMEM;
+		}
+
+		/*
+		 * ->p is odd, so no need to explicitly subtract one
+		 * from it before shifting to the right.
+		 */
+		mpi_rshift(q, ctx->p, 1);
 
+		ret = mpi_powm(val, y, q, ctx->p);
+		mpi_free(q);
 		if (ret) {
 			mpi_free(val);
 			return ret;
-- 
2.26.2

[PATCH v2 17/18] crypto: dh - try to match domain parameters to a known safe-prime group

[Nicolai Stange]

A subsequent patch will make the DH implementation to reject any input
domain parameter set with ->group_id == DH_GROUP_ID_UNKNOWN in FIPS mode.
However, as the keyctl(KEYCTL_DH_COMPUTE) implementation simply passes
forward keys from userspace, it does not (and cannot) set ->group_id to
anything else than DH_GROUP_ID_UNKNOWN.

In order to still allow for keyctl(KEYCTL_DH_COMPUTE) to work on approved
domain parameters passed in from userspace in FIPS mode, make
crypto_dh_decode_key() to compare them against any of the known groups and
set ->group_id upon having found a match, if any.

Signed-off-by: Nicolai Stange <nstange@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 crypto/dh_helper.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index ec9c4cdf57b2..b8a726b610a2 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -470,6 +470,36 @@ get_safe_prime_group(enum dh_group_id group_id)
 	return NULL;
 }
 
+static enum dh_group_id lookup_group_id(const char *g, size_t g_size,
+					const char *p, size_t p_size)
+{
+	int i;
+
+	/* All safe-prime groups use a generator of g == 2. */
+	while (g_size && !*g) {
+		++g;
+		--g_size;
+	}
+
+	if (g_size != 1 || *g != 2)
+		return DH_GROUP_ID_UNKNOWN;
+
+	while (p_size && !*p) {
+		++p;
+		--p_size;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(safe_prime_groups); ++i) {
+		if (safe_prime_groups[i].p_size != p_size)
+			continue;
+
+		if (!memcmp(safe_prime_groups[i].p, p, p_size))
+			return safe_prime_groups[i].group_id;
+	}
+
+	return DH_GROUP_ID_UNKNOWN;
+}
+
 static inline u8 *dh_pack_data(u8 *dst, u8 *end, const void *src, size_t size)
 {
 	if (!dst || size > end - dst)
@@ -568,6 +598,9 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 		if (memchr_inv(params->p, 0, params->p_size) == NULL)
 			return -EINVAL;
 
+		params->group_id = lookup_group_id(params->g, params->g_size,
+						   params->p, params->p_size);
+
 	} else {
 		const struct safe_prime_group *g;
 
-- 
2.26.2

[PATCH v2 18/18] crypto: dh - accept only approved safe-prime groups in FIPS mode

[Nicolai Stange]

SP800-56Arev3, sec. 5.5.2 ("Assurance of Domain-Parameter Validity")
asserts that an implementation needs to verify domain paramtere validity,
which boils down to either
- the domain parameters corresponding to some known safe-prime group
  explicitly listed to be approved in the document or
- for parameters conforming to a "FIPS 186-type parameter-size set",
  that the implementation needs to perform an explicit domain parameter
  verification, which would require access to the "seed" and "counter"
  values used in their generation.

The latter is not easily feasible and moreover, SP800-56Arev3 states that
safe-prime groups are preferred and that FIPS 186-type parameter sets
should only be supported for backward compatibility, if it all.

Make the dh implementations reject any domain parameters which don't
correspond to any of the approved safe-prime groups in FIPS mode. The
approved safe-prime groups are the ones specified in RFC 7919 and RFC 3526,
and given that all possible values of enum dh_group_id correspond to
either groups from these RFCs or to DH_GROUP_ID_UNKNOWN, it suffices to
make crypto_dh_decode_key() to reject any parameter set where
->group_id == DH_GROUP_ID_UNKNOWN.

As this change will effectively render the dh implementation unusable in
FIPS mode if neither of the CRYPTO_DH_GROUPS_RFC7919 or
CRYPTO_DH_GROUPS_RFC3526 Kconfig options enabled, make CRYPTO_DH imply
these two if CRYPTO_FIPS is set.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/Kconfig     | 2 ++
 crypto/dh_helper.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 578711b02bb3..571f2271ad2e 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -229,6 +229,8 @@ menuconfig CRYPTO_DH
 	select CRYPTO_KPP
 	select MPILIB
 	select CRYPTO_RNG_DEFAULT
+	imply CRYPTO_DH_GROUPS_RFC7919 if CRYPTO_FIPS
+	imply CRYPTO_DH_GROUPS_RFC3526 if CRYPTO_FIPS
 	help
 	  Generic implementation of the Diffie-Hellman algorithm.
 
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index b8a726b610a2..fafaf3c30bb1 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -7,6 +7,7 @@
 #include <linux/export.h>
 #include <linux/err.h>
 #include <linux/string.h>
+#include <linux/fips.h>
 #include <crypto/dh.h>
 #include <crypto/kpp.h>
 #include <crypto/rng.h>
@@ -624,6 +625,9 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	    params->g_size > params->p_size)
 		return -EINVAL;
 
+	/* Only safe-prime groups are allowed in FIPS mode. */
+	if (fips_enabled && params->group_id == DH_GROUP_ID_UNKNOWN)
+		return -EINVAL;
 
 	return 0;
 }
-- 
2.26.2

Re: [PATCH v2 18/18] crypto: dh - accept only approved safe-prime groups in FIPS mode

[Hannes Reinecke]

On 12/9/21 10:03 AM, Nicolai Stange wrote:
> SP800-56Arev3, sec. 5.5.2 ("Assurance of Domain-Parameter Validity")
> asserts that an implementation needs to verify domain paramtere validity,
> which boils down to either
> - the domain parameters corresponding to some known safe-prime group
>   explicitly listed to be approved in the document or
> - for parameters conforming to a "FIPS 186-type parameter-size set",
>   that the implementation needs to perform an explicit domain parameter
>   verification, which would require access to the "seed" and "counter"
>   values used in their generation.
> 
> The latter is not easily feasible and moreover, SP800-56Arev3 states that
> safe-prime groups are preferred and that FIPS 186-type parameter sets
> should only be supported for backward compatibility, if it all.
> 
> Make the dh implementations reject any domain parameters which don't
> correspond to any of the approved safe-prime groups in FIPS mode. The
> approved safe-prime groups are the ones specified in RFC 7919 and RFC 3526,
> and given that all possible values of enum dh_group_id correspond to
> either groups from these RFCs or to DH_GROUP_ID_UNKNOWN, it suffices to
> make crypto_dh_decode_key() to reject any parameter set where
> ->group_id == DH_GROUP_ID_UNKNOWN.
> 
> As this change will effectively render the dh implementation unusable in
> FIPS mode if neither of the CRYPTO_DH_GROUPS_RFC7919 or
> CRYPTO_DH_GROUPS_RFC3526 Kconfig options enabled, make CRYPTO_DH imply
> these two if CRYPTO_FIPS is set.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  crypto/Kconfig     | 2 ++
>  crypto/dh_helper.c | 4 ++++
>  2 files changed, 6 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Re: [PATCH v2 00/18] crypto: dh - infrastructure for NVM in-band auth and FIPS conformance

[Stephan Mueller]

Am Donnerstag, 9. Dezember 2021, 10:03:40 CET schrieb Nicolai Stange:

Hi Nicolai,

I successfully tested the entire patch set with the NIST ACVP reference 
implementation which covers key generation for all safe prime groups defined 
in your patch set.

Tested-by: Stephan Mueller <smueller@chronox.de>

Ciao
Stephan

Re: [PATCH v2 00/18] crypto: dh - infrastructure for NVM in-band auth and FIPS conformance

[Nicolai Stange]

Stephan Mueller <smueller@chronox.de> writes:

> Am Donnerstag, 9. Dezember 2021, 10:03:40 CET schrieb Nicolai Stange:
>
> I successfully tested the entire patch set with the NIST ACVP reference 
> implementation which covers key generation for all safe prime groups defined 
> in your patch set.
>
> Tested-by: Stephan Mueller <smueller@chronox.de>

Thank you!

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev

Re: [PATCH v2 00/18] crypto: dh - infrastructure for NVM in-band auth and FIPS conformance

[Hannes Reinecke]

On 12/9/21 10:03 AM, Nicolai Stange wrote:
> Hi all,
> 
> first of all, to the people primarily interested in security/keys/, there's
> a rather trivial change to security/keys/dh.c in patch 2/18. It would be
> great to get ACKs for that...
> 
> This is v2, v1 can be found at
> 
>   https://lore.kernel.org/r/20211201004858.19831-1-nstange@suse.de
> 
> For a list of changes, see below.
> 
> Quote from v1's cover letter:
> ===
> Hannes' recent work on NVME in-band authentication ([1]) needs access to
> the RFC 7919 DH group parameters and also some means to generate ephemeral
> keys. He currently implements both as part of his patchset (patches 3/12
> and 8/12). After some internal discussion, we decided to split off the bits
> needed from crypto/dh into a separate series, i.e. this one here:
>  - for the RFC 7919 DH group parameters, it's undesirable from a
>    performance POV to serialize the well-known domain parameters via
>    crypto_dh_encode_key() just to deserialize them shortly after again,
>  - from an architectural POV, it would be preferrable to have the key
>    generation code in crypto/dh.c rather than in drivers/nvme/,
>    just in analogy to how key generation is supported by crypto/ecdh.c
>    already.
> 
> Patches 1-13/18 implement all that is needed for the NVME in-band
> authentication support. 
> 
> Unfortunately, due to the lack of HW, I have not been able to test
> the changes to the QAT or HPRE drivers (other than mere compile tests).
> Yet I figured it would be a good idea to have them behave consistently with
> dh_generic, and so I chose to introduce support for privkey generation to
> these as well.
> 
> 
> By coincidence, NIST SP800-56Arev3 compliance effectively requires that
> the domain parameters are checked against an approved set, which happens
> to consists of those safe-prime group parameters specified in RFC 7919,
> among others. Thus, introducing the RFC 7919 parameters to the kernel
> allows for making the DH implementation to conform to SP800-56Arev3 with
> only little effort. I used the opportunity to work crypto/dh towards
> SP800-56Arev3 conformance with the rest of this patch series, i.e.
> patches 14-18/18. I can split these into another series on its own, if you
> like. But as they depend on the earlier patches 1-13/18, I sent them
> alongside for now.
> ===
> 
> This patchset has been tested with and without fips_enabled on x86_64,
> ppc64le and s390x, the latter being big endian.
> 
> 
> Changes v1 -> v2:
> - Throughout the patchset:
>   - Upcase enum group_id members and strip superfluous _RFCXYZ_ parts from
>     the names.
>   - Carry Hannes' Reviewed-bys from v1 over for those patches which
>     have not changed (except for that group_id member renaming)
> - [03/18] ("crypto: dh - optimize domain parameter serialization for
>             well-known groups"):
>   - For better portability, don't serialize/deserialize directly from/to
>     an enum group_id, but use an intermediate int for that.
> - [05/18] ("crypto: testmgr - add DH RFC 7919 ffdhe2048 test vector")
>   - Use ffdhe3072 TVs rather than ones for ffdhe2048. Requested by Hannes,
>     because "the NVMe spec mandates for its TLS profile the ffdhe3072
>     group".
> - [13/18] ("crypto: testmgr - add DH test vectors for key generation")
>   - Use ffdhe3072 in place of ffdhe2048 here as well.
>   - Rather than introducing completely new keypairs, reuse the ones
>     from the known answer test introduced previously in this patchset.
> 
> Thanks,
> 
> Nicolai
> 
> [1] https://lkml.kernel.org/r/20211123123801.73197-1-hare@suse.de
> 
> 
> Nicolai Stange (18):
>   crypto: dh - remove struct dh's ->q member
>   crypto: dh - constify struct dh's pointer members
>   crypto: dh - optimize domain parameter serialization for well-known
>     groups
>   crypto: dh - introduce RFC 7919 safe-prime groups
>   crypto: testmgr - add DH RFC 7919 ffdhe3072 test vector
>   crypto: dh - introduce RFC 3526 safe-prime groups
>   crypto: testmgr - add DH RFC 3526 modp2048 test vector
>   crypto: testmgr - run only subset of DH vectors based on config
>   crypto: dh - implement private key generation primitive
>   crypto: dh - introduce support for ephemeral key generation to
>     dh-generic
>   crypto: dh - introduce support for ephemeral key generation to hpre
>     driver
>   crypto: dh - introduce support for ephemeral key generation to qat
>     driver
>   crypto: testmgr - add DH test vectors for key generation
>   lib/mpi: export mpi_rshift
>   crypto: dh - store group id in dh-generic's dh_ctx
>   crypto: dh - calculate Q from P for the full public key verification
>   crypto: dh - try to match domain parameters to a known safe-prime
>     group
>   crypto: dh - accept only approved safe-prime groups in FIPS mode
> 
>  crypto/Kconfig                                |  20 +-
>  crypto/dh.c                                   |  73 +-
>  crypto/dh_helper.c                            | 691 +++++++++++++++++-
>  crypto/testmgr.h                              | 388 +++++++++-
>  drivers/crypto/hisilicon/hpre/hpre_crypto.c   |  11 +
>  drivers/crypto/qat/qat_common/qat_asym_algs.c |   9 +
>  include/crypto/dh.h                           |  52 +-
>  lib/mpi/mpi-bit.c                             |   1 +
>  security/keys/dh.c                            |   2 +-
>  9 files changed, 1189 insertions(+), 58 deletions(-)
> 
I have run this implementation against my NVMe In-band authentication
test suite and have found no issues.

Tested-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Re: [PATCH v2 00/18] crypto: dh - infrastructure for NVM in-band auth and FIPS conformance

[Nicolai Stange]

Hannes Reinecke <hare@suse.de> writes:

> I have run this implementation against my NVMe In-band authentication
> test suite and have found no issues.
>
> Tested-by: Hannes Reinecke <hare@suse.de>

Thank you!

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev