From: Hannes Reinecke <hare@suse.de>
To: Nicolai Stange <nstange@suse.de>,
Herbert Xu <herbert@gondor.apana.org.au>,
"David S. Miller" <davem@davemloft.net>
Cc: "Stephan Müller" <smueller@chronox.de>,
"Torsten Duwe" <duwe@suse.de>, "Zaibo Xu" <xuzaibo@huawei.com>,
"Giovanni Cabiddu" <giovanni.cabiddu@intel.com>,
"David Howells" <dhowells@redhat.com>,
"Jarkko Sakkinen" <jarkko@kernel.org>,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
qat-linux@intel.com, keyrings@vger.kernel.org
Subject: Re: [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups
Date: Fri, 10 Dec 2021 12:33:34 +0100 [thread overview]
Message-ID: <86157a11-7daa-876a-d80b-e6bda36e6368@suse.de> (raw)
In-Reply-To: <20211209090358.28231-4-nstange@suse.de>
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
next prev parent reply other threads:[~2021-12-10 11:33 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 9:03 [PATCH v2 00/18] crypto: dh - infrastructure for NVM in-band auth and FIPS conformance Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 01/18] crypto: dh - remove struct dh's ->q member Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 02/18] crypto: dh - constify struct dh's pointer members Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups Nicolai Stange
2021-12-10 11:33 ` Hannes Reinecke [this message]
2021-12-13 10:06 ` Nicolai Stange
2021-12-13 10:10 ` Hannes Reinecke
2021-12-17 5:52 ` Herbert Xu
2021-12-20 15:27 ` Nicolai Stange
2021-12-29 2:14 ` Herbert Xu
2022-01-06 14:30 ` Stephan Mueller
2022-01-07 2:44 ` Herbert Xu
2022-01-07 6:37 ` Nicolai Stange
2022-01-11 6:13 ` [PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1) Herbert Xu
2022-01-11 7:50 ` Nicolai Stange
2022-01-11 10:34 ` Herbert Xu
2022-01-14 6:16 ` [v2 PATCH] " Herbert Xu
2022-01-14 9:09 ` Nicolai Stange
2022-01-14 10:55 ` Herbert Xu
2022-01-14 12:34 ` Nicolai Stange
2022-01-14 12:35 ` Stephan Mueller
2022-01-14 12:54 ` James Bottomley
2022-01-26 9:01 ` Stephan Mueller
2022-01-28 14:14 ` Nicolai Stange
2022-01-28 15:49 ` Stephan Mueller
2022-02-02 10:09 ` Nicolai Stange
2022-01-07 7:01 ` [PATCH v2 03/18] crypto: dh - optimize domain parameter serialization for well-known groups Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 04/18] crypto: dh - introduce RFC 7919 safe-prime groups Nicolai Stange
2021-12-10 11:34 ` Hannes Reinecke
2021-12-09 9:03 ` [PATCH v2 05/18] crypto: testmgr - add DH RFC 7919 ffdhe3072 test vector Nicolai Stange
2021-12-10 11:34 ` Hannes Reinecke
2021-12-09 9:03 ` [PATCH v2 06/18] crypto: dh - introduce RFC 3526 safe-prime groups Nicolai Stange
2021-12-10 11:35 ` Hannes Reinecke
2021-12-09 9:03 ` [PATCH v2 07/18] crypto: testmgr - add DH RFC 3526 modp2048 test vector Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 08/18] crypto: testmgr - run only subset of DH vectors based on config Nicolai Stange
2021-12-10 11:36 ` Hannes Reinecke
2021-12-09 9:03 ` [PATCH v2 09/18] crypto: dh - implement private key generation primitive Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 10/18] crypto: dh - introduce support for ephemeral key generation to dh-generic Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 11/18] crypto: dh - introduce support for ephemeral key generation to hpre driver Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 12/18] crypto: dh - introduce support for ephemeral key generation to qat driver Nicolai Stange
2021-12-15 21:54 ` Giovanni Cabiddu
2021-12-09 9:03 ` [PATCH v2 13/18] crypto: testmgr - add DH test vectors for key generation Nicolai Stange
2021-12-10 11:37 ` Hannes Reinecke
2021-12-09 9:03 ` [PATCH v2 14/18] lib/mpi: export mpi_rshift Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 15/18] crypto: dh - store group id in dh-generic's dh_ctx Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 16/18] crypto: dh - calculate Q from P for the full public key verification Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 17/18] crypto: dh - try to match domain parameters to a known safe-prime group Nicolai Stange
2021-12-09 9:03 ` [PATCH v2 18/18] crypto: dh - accept only approved safe-prime groups in FIPS mode Nicolai Stange
2021-12-10 11:37 ` Hannes Reinecke
2021-12-10 7:56 ` [PATCH v2 00/18] crypto: dh - infrastructure for NVM in-band auth and FIPS conformance Stephan Mueller
2021-12-10 10:00 ` Nicolai Stange
2021-12-10 11:38 ` Hannes Reinecke
2021-12-13 10:12 ` Nicolai Stange
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86157a11-7daa-876a-d80b-e6bda36e6368@suse.de \
--to=hare@suse.de \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=duwe@suse.de \
--cc=giovanni.cabiddu@intel.com \
--cc=herbert@gondor.apana.org.au \
--cc=jarkko@kernel.org \
--cc=keyrings@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nstange@suse.de \
--cc=qat-linux@intel.com \
--cc=smueller@chronox.de \
--cc=xuzaibo@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).