Hi,
The key derviation functions are considered to be a cryptographic
operation. As cryptographic operations are provided via the kernel
crypto API, this patch set consolidates the SP800-108 KDF
implementation into the crypto API.
If this patch is accepted, another patch set will be published attempting
to move the HKDF implementation from the crypto file system code base
to the kernel crypto API.
The KDF implementation is provided as service functions. Yet, the
interface to the the provided KDF is modeled such, that additional
KDF implementation can use the same API style. The goal is to allow
the transformation from a service function into a crypto API template
eventually.
The KDF executes a power-on self test with test vectors from commonly
known sources.
Tbe SP800-108 KDF implementation is used to replace the implementation
in the keys subsystem. The implementation was verified using the
keyutils command line test code provided in
tests/keyctl/dh_compute/valid. All tests show that the expected values
are calculated with the new code.
Changes v3:
* port to kernel 5.16-rc1
* remove the HKDF patch to only leave the SP800-108 patch
Stephan Mueller (4):
crypto: Add key derivation self-test support code
crypto: add SP800-108 counter key derivation function
security: DH - remove dead code for zero padding
security: DH - use KDF implementation from crypto API
crypto/Kconfig | 7 ++
crypto/Makefile | 5 +
crypto/kdf_sp800108.c | 149 +++++++++++++++++++++++++
include/crypto/internal/kdf_selftest.h | 71 ++++++++++++
include/crypto/kdf_sp800108.h | 61 ++++++++++
security/keys/Kconfig | 2 +-
security/keys/dh.c | 118 +++-----------------
7 files changed, 310 insertions(+), 103 deletions(-)
create mode 100644 crypto/kdf_sp800108.c
create mode 100644 include/crypto/internal/kdf_selftest.h
create mode 100644 include/crypto/kdf_sp800108.h
--
2.33.1
As a preparation to add the key derivation implementations, the
self-test data structure definition and the common test code is made
available.
The test framework follows the testing applied by the NIST CAVP test
approach.
The structure of the test code follows the implementations found in
crypto/testmgr.c|h. In case the KDF implementations will be made
available via a kernel crypto API templates, the test code is intended
to be merged into testmgr.c|h.
Signed-off-by: Stephan Mueller <[email protected]>
---
include/crypto/internal/kdf_selftest.h | 71 ++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
create mode 100644 include/crypto/internal/kdf_selftest.h
diff --git a/include/crypto/internal/kdf_selftest.h b/include/crypto/internal/kdf_selftest.h
new file mode 100644
index 000000000000..4d03d2af57b7
--- /dev/null
+++ b/include/crypto/internal/kdf_selftest.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (C) 2021, Stephan Mueller <[email protected]>
+ */
+
+#ifndef _CRYPTO_KDF_SELFTEST_H
+#define _CRYPTO_KDF_SELFTEST_H
+
+#include <crypto/hash.h>
+#include <linux/uio.h>
+
+struct kdf_testvec {
+ unsigned char *key;
+ size_t keylen;
+ unsigned char *ikm;
+ size_t ikmlen;
+ struct kvec info;
+ unsigned char *expected;
+ size_t expectedlen;
+};
+
+static inline int
+kdf_test(const struct kdf_testvec *test, const char *name,
+ int (*crypto_kdf_setkey)(struct crypto_shash *kmd,
+ const u8 *key, size_t keylen,
+ const u8 *ikm, size_t ikmlen),
+ int (*crypto_kdf_generate)(struct crypto_shash *kmd,
+ const struct kvec *info,
+ unsigned int info_nvec,
+ u8 *dst, unsigned int dlen))
+{
+ struct crypto_shash *kmd;
+ int ret;
+ u8 *buf = kzalloc(test->expectedlen, GFP_KERNEL);
+
+ if (!buf)
+ return -ENOMEM;
+
+ kmd = crypto_alloc_shash(name, 0, 0);
+ if (IS_ERR(kmd)) {
+ pr_err("alg: kdf: could not allocate hash handle for %s\n",
+ name);
+ kfree(buf);
+ return -ENOMEM;
+ }
+
+ ret = crypto_kdf_setkey(kmd, test->key, test->keylen,
+ test->ikm, test->ikmlen);
+ if (ret) {
+ pr_err("alg: kdf: could not set key derivation key\n");
+ goto err;
+ }
+
+ ret = crypto_kdf_generate(kmd, &test->info, 1, buf, test->expectedlen);
+ if (ret) {
+ pr_err("alg: kdf: could not obtain key data\n");
+ goto err;
+ }
+
+ ret = memcmp(test->expected, buf, test->expectedlen);
+ if (ret)
+ ret = -EINVAL;
+
+err:
+ crypto_free_shash(kmd);
+ kfree(buf);
+ return ret;
+}
+
+#endif /* _CRYPTO_KDF_SELFTEST_H */
--
2.33.1
The kernel crypto API provides the SP800-108 counter KDF implementation.
Thus, the separate implementation provided as part of the keys subsystem
can be replaced with calls to the KDF offered by the kernel crypto API.
The keys subsystem uses the counter KDF with a hash primitive. Thus,
it only uses the call to crypto_kdf108_ctr_generate.
Signed-off-by: Stephan Mueller <[email protected]>
---
security/keys/Kconfig | 2 +-
security/keys/dh.c | 97 +++++++------------------------------------
2 files changed, 15 insertions(+), 84 deletions(-)
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 64b81abd087e..969122c7b92f 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -109,7 +109,7 @@ config KEY_DH_OPERATIONS
bool "Diffie-Hellman operations on retained keys"
depends on KEYS
select CRYPTO
- select CRYPTO_HASH
+ select CRYPTO_KDF800108_CTR
select CRYPTO_DH
help
This option provides support for calculating Diffie-Hellman
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 56e12dae4534..46fa442b81ec 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -11,6 +11,7 @@
#include <crypto/hash.h>
#include <crypto/kpp.h>
#include <crypto/dh.h>
+#include <crypto/kdf_sp800108.h>
#include <keys/user-type.h>
#include "internal.h"
@@ -79,16 +80,9 @@ static void dh_crypto_done(struct crypto_async_request *req, int err)
complete(&compl->completion);
}
-struct kdf_sdesc {
- struct shash_desc shash;
- char ctx[];
-};
-
-static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
+static int kdf_alloc(struct crypto_shash **hash, char *hashname)
{
struct crypto_shash *tfm;
- struct kdf_sdesc *sdesc;
- int size;
int err;
/* allocate synchronous hash */
@@ -102,14 +96,7 @@ static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
if (crypto_shash_digestsize(tfm) == 0)
goto out_free_tfm;
- err = -ENOMEM;
- size = sizeof(struct shash_desc) + crypto_shash_descsize(tfm);
- sdesc = kmalloc(size, GFP_KERNEL);
- if (!sdesc)
- goto out_free_tfm;
- sdesc->shash.tfm = tfm;
-
- *sdesc_ret = sdesc;
+ *hash = tfm;
return 0;
@@ -118,76 +105,20 @@ static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
return err;
}
-static void kdf_dealloc(struct kdf_sdesc *sdesc)
-{
- if (!sdesc)
- return;
-
- if (sdesc->shash.tfm)
- crypto_free_shash(sdesc->shash.tfm);
-
- kfree_sensitive(sdesc);
-}
-
-/*
- * Implementation of the KDF in counter mode according to SP800-108 section 5.1
- * as well as SP800-56A section 5.8.1 (Single-step KDF).
- *
- * SP800-56A:
- * The src pointer is defined as Z || other info where Z is the shared secret
- * from DH and other info is an arbitrary string (see SP800-56A section
- * 5.8.1.2).
- *
- * 'dlen' must be a multiple of the digest size.
- */
-static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
- u8 *dst, unsigned int dlen)
+static void kdf_dealloc(struct crypto_shash *hash)
{
- struct shash_desc *desc = &sdesc->shash;
- unsigned int h = crypto_shash_digestsize(desc->tfm);
- int err = 0;
- u8 *dst_orig = dst;
- __be32 counter = cpu_to_be32(1);
-
- while (dlen) {
- err = crypto_shash_init(desc);
- if (err)
- goto err;
-
- err = crypto_shash_update(desc, (u8 *)&counter, sizeof(__be32));
- if (err)
- goto err;
-
- if (src && slen) {
- err = crypto_shash_update(desc, src, slen);
- if (err)
- goto err;
- }
-
- err = crypto_shash_final(desc, dst);
- if (err)
- goto err;
-
- dlen -= h;
- dst += h;
- counter = cpu_to_be32(be32_to_cpu(counter) + 1);
- }
-
- return 0;
-
-err:
- memzero_explicit(dst_orig, dlen);
- return err;
+ if (hash)
+ crypto_free_shash(hash);
}
-static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
+static int keyctl_dh_compute_kdf(struct crypto_shash *hash,
char __user *buffer, size_t buflen,
uint8_t *kbuf, size_t kbuflen)
{
+ struct kvec kbuf_iov = { .iov_base = kbuf, .iov_len = kbuflen };
uint8_t *outbuf = NULL;
int ret;
- size_t outbuf_len = roundup(buflen,
- crypto_shash_digestsize(sdesc->shash.tfm));
+ size_t outbuf_len = roundup(buflen, crypto_shash_digestsize(hash));
outbuf = kmalloc(outbuf_len, GFP_KERNEL);
if (!outbuf) {
@@ -195,7 +126,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
goto err;
}
- ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len);
+ ret = crypto_kdf108_ctr_generate(hash, &kbuf_iov, 1, outbuf, outbuf_len);
if (ret)
goto err;
@@ -224,7 +155,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
struct kpp_request *req;
uint8_t *secret;
uint8_t *outbuf;
- struct kdf_sdesc *sdesc = NULL;
+ struct crypto_shash *hash = NULL;
if (!params || (!buffer && buflen)) {
ret = -EINVAL;
@@ -257,7 +188,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
}
/* allocate KDF from the kernel crypto API */
- ret = kdf_alloc(&sdesc, hashname);
+ ret = kdf_alloc(&hash, hashname);
kfree(hashname);
if (ret)
goto out1;
@@ -367,7 +298,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
goto out6;
}
- ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf,
+ ret = keyctl_dh_compute_kdf(hash, buffer, buflen, outbuf,
req->dst_len + kdfcopy->otherinfolen);
} else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) {
ret = req->dst_len;
@@ -386,7 +317,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
out2:
dh_free_data(&dh_inputs);
out1:
- kdf_dealloc(sdesc);
+ kdf_dealloc(hash);
return ret;
}
--
2.33.1
Remove the specific code that adds a zero padding that was intended
to be invoked when the DH operation result was smaller than the
modulus. However, this cannot occur any more these days because the
function mpi_write_to_sgl is used in the code path that calculates the
shared secret in dh_compute_value. This MPI service function guarantees
that leading zeros are introduced as needed to ensure the resulting data
is exactly as long as the modulus. This implies that the specific code
to add zero padding is dead code which can be safely removed.
Signed-off-by: Stephan Mueller <[email protected]>
---
security/keys/dh.c | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 1abfa70ed6e1..56e12dae4534 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -141,7 +141,7 @@ static void kdf_dealloc(struct kdf_sdesc *sdesc)
* 'dlen' must be a multiple of the digest size.
*/
static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
- u8 *dst, unsigned int dlen, unsigned int zlen)
+ u8 *dst, unsigned int dlen)
{
struct shash_desc *desc = &sdesc->shash;
unsigned int h = crypto_shash_digestsize(desc->tfm);
@@ -158,22 +158,6 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
if (err)
goto err;
- if (zlen && h) {
- u8 tmpbuffer[32];
- size_t chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
- memset(tmpbuffer, 0, chunk);
-
- do {
- err = crypto_shash_update(desc, tmpbuffer,
- chunk);
- if (err)
- goto err;
-
- zlen -= chunk;
- chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
- } while (zlen);
- }
-
if (src && slen) {
err = crypto_shash_update(desc, src, slen);
if (err)
@@ -198,7 +182,7 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
char __user *buffer, size_t buflen,
- uint8_t *kbuf, size_t kbuflen, size_t lzero)
+ uint8_t *kbuf, size_t kbuflen)
{
uint8_t *outbuf = NULL;
int ret;
@@ -211,7 +195,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
goto err;
}
- ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len, lzero);
+ ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len);
if (ret)
goto err;
@@ -384,8 +368,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
}
ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf,
- req->dst_len + kdfcopy->otherinfolen,
- outlen - req->dst_len);
+ req->dst_len + kdfcopy->otherinfolen);
} else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) {
ret = req->dst_len;
} else {
--
2.33.1
On Mon, 15 Nov 2021, Stephan M?ller wrote:
> Remove the specific code that adds a zero padding that was intended
> to be invoked when the DH operation result was smaller than the
> modulus. However, this cannot occur any more these days because the
> function mpi_write_to_sgl is used in the code path that calculates the
> shared secret in dh_compute_value. This MPI service function guarantees
> that leading zeros are introduced as needed to ensure the resulting data
> is exactly as long as the modulus. This implies that the specific code
> to add zero padding is dead code which can be safely removed.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> security/keys/dh.c | 25 ++++---------------------
> 1 file changed, 4 insertions(+), 21 deletions(-)
Hi Stephan -
Thanks for the cleanup!
Acked-by: Mat Martineau <[email protected]>
>
> diff --git a/security/keys/dh.c b/security/keys/dh.c
> index 1abfa70ed6e1..56e12dae4534 100644
> --- a/security/keys/dh.c
> +++ b/security/keys/dh.c
> @@ -141,7 +141,7 @@ static void kdf_dealloc(struct kdf_sdesc *sdesc)
> * 'dlen' must be a multiple of the digest size.
> */
> static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
> - u8 *dst, unsigned int dlen, unsigned int zlen)
> + u8 *dst, unsigned int dlen)
> {
> struct shash_desc *desc = &sdesc->shash;
> unsigned int h = crypto_shash_digestsize(desc->tfm);
> @@ -158,22 +158,6 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
> if (err)
> goto err;
>
> - if (zlen && h) {
> - u8 tmpbuffer[32];
> - size_t chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
> - memset(tmpbuffer, 0, chunk);
> -
> - do {
> - err = crypto_shash_update(desc, tmpbuffer,
> - chunk);
> - if (err)
> - goto err;
> -
> - zlen -= chunk;
> - chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
> - } while (zlen);
> - }
> -
> if (src && slen) {
> err = crypto_shash_update(desc, src, slen);
> if (err)
> @@ -198,7 +182,7 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
>
> static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
> char __user *buffer, size_t buflen,
> - uint8_t *kbuf, size_t kbuflen, size_t lzero)
> + uint8_t *kbuf, size_t kbuflen)
> {
> uint8_t *outbuf = NULL;
> int ret;
> @@ -211,7 +195,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
> goto err;
> }
>
> - ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len, lzero);
> + ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len);
> if (ret)
> goto err;
>
> @@ -384,8 +368,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
> }
>
> ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf,
> - req->dst_len + kdfcopy->otherinfolen,
> - outlen - req->dst_len);
> + req->dst_len + kdfcopy->otherinfolen);
> } else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) {
> ret = req->dst_len;
> } else {
> --
> 2.33.1
>
>
>
>
>
--
Mat Martineau
Intel
On Mon, 15 Nov 2021, Stephan M?ller wrote:
> The kernel crypto API provides the SP800-108 counter KDF implementation.
> Thus, the separate implementation provided as part of the keys subsystem
> can be replaced with calls to the KDF offered by the kernel crypto API.
>
> The keys subsystem uses the counter KDF with a hash primitive. Thus,
> it only uses the call to crypto_kdf108_ctr_generate.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> security/keys/Kconfig | 2 +-
> security/keys/dh.c | 97 +++++++------------------------------------
> 2 files changed, 15 insertions(+), 84 deletions(-)
>
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index 64b81abd087e..969122c7b92f 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -109,7 +109,7 @@ config KEY_DH_OPERATIONS
> bool "Diffie-Hellman operations on retained keys"
> depends on KEYS
> select CRYPTO
> - select CRYPTO_HASH
> + select CRYPTO_KDF800108_CTR
> select CRYPTO_DH
> help
> This option provides support for calculating Diffie-Hellman
> diff --git a/security/keys/dh.c b/security/keys/dh.c
> index 56e12dae4534..46fa442b81ec 100644
> --- a/security/keys/dh.c
> +++ b/security/keys/dh.c
> @@ -11,6 +11,7 @@
> #include <crypto/hash.h>
> #include <crypto/kpp.h>
> #include <crypto/dh.h>
> +#include <crypto/kdf_sp800108.h>
> #include <keys/user-type.h>
> #include "internal.h"
>
> @@ -79,16 +80,9 @@ static void dh_crypto_done(struct crypto_async_request *req, int err)
> complete(&compl->completion);
> }
>
> -struct kdf_sdesc {
> - struct shash_desc shash;
> - char ctx[];
> -};
> -
> -static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
> +static int kdf_alloc(struct crypto_shash **hash, char *hashname)
> {
> struct crypto_shash *tfm;
> - struct kdf_sdesc *sdesc;
> - int size;
> int err;
>
> /* allocate synchronous hash */
> @@ -102,14 +96,7 @@ static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
> if (crypto_shash_digestsize(tfm) == 0)
> goto out_free_tfm;
With the sdesc allocation failure path removed below, there's only one use
of the 'err' variable and the out_free_tfm code path. I suggest furthering
the cleanup by removing err and the goto path, and moving the
crypto_free_shash() and return -EINVAL here (where the goto currently is).
-Mat
>
> - err = -ENOMEM;
> - size = sizeof(struct shash_desc) + crypto_shash_descsize(tfm);
> - sdesc = kmalloc(size, GFP_KERNEL);
> - if (!sdesc)
> - goto out_free_tfm;
> - sdesc->shash.tfm = tfm;
> -
> - *sdesc_ret = sdesc;
> + *hash = tfm;
>
> return 0;
>
> @@ -118,76 +105,20 @@ static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
> return err;
> }
>
> -static void kdf_dealloc(struct kdf_sdesc *sdesc)
> -{
> - if (!sdesc)
> - return;
> -
> - if (sdesc->shash.tfm)
> - crypto_free_shash(sdesc->shash.tfm);
> -
> - kfree_sensitive(sdesc);
> -}
> -
> -/*
> - * Implementation of the KDF in counter mode according to SP800-108 section 5.1
> - * as well as SP800-56A section 5.8.1 (Single-step KDF).
> - *
> - * SP800-56A:
> - * The src pointer is defined as Z || other info where Z is the shared secret
> - * from DH and other info is an arbitrary string (see SP800-56A section
> - * 5.8.1.2).
> - *
> - * 'dlen' must be a multiple of the digest size.
> - */
> -static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
> - u8 *dst, unsigned int dlen)
> +static void kdf_dealloc(struct crypto_shash *hash)
> {
> - struct shash_desc *desc = &sdesc->shash;
> - unsigned int h = crypto_shash_digestsize(desc->tfm);
> - int err = 0;
> - u8 *dst_orig = dst;
> - __be32 counter = cpu_to_be32(1);
> -
> - while (dlen) {
> - err = crypto_shash_init(desc);
> - if (err)
> - goto err;
> -
> - err = crypto_shash_update(desc, (u8 *)&counter, sizeof(__be32));
> - if (err)
> - goto err;
> -
> - if (src && slen) {
> - err = crypto_shash_update(desc, src, slen);
> - if (err)
> - goto err;
> - }
> -
> - err = crypto_shash_final(desc, dst);
> - if (err)
> - goto err;
> -
> - dlen -= h;
> - dst += h;
> - counter = cpu_to_be32(be32_to_cpu(counter) + 1);
> - }
> -
> - return 0;
> -
> -err:
> - memzero_explicit(dst_orig, dlen);
> - return err;
> + if (hash)
> + crypto_free_shash(hash);
> }
>
> -static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
> +static int keyctl_dh_compute_kdf(struct crypto_shash *hash,
> char __user *buffer, size_t buflen,
> uint8_t *kbuf, size_t kbuflen)
> {
> + struct kvec kbuf_iov = { .iov_base = kbuf, .iov_len = kbuflen };
> uint8_t *outbuf = NULL;
> int ret;
> - size_t outbuf_len = roundup(buflen,
> - crypto_shash_digestsize(sdesc->shash.tfm));
> + size_t outbuf_len = roundup(buflen, crypto_shash_digestsize(hash));
>
> outbuf = kmalloc(outbuf_len, GFP_KERNEL);
> if (!outbuf) {
> @@ -195,7 +126,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
> goto err;
> }
>
> - ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len);
> + ret = crypto_kdf108_ctr_generate(hash, &kbuf_iov, 1, outbuf, outbuf_len);
> if (ret)
> goto err;
>
> @@ -224,7 +155,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
> struct kpp_request *req;
> uint8_t *secret;
> uint8_t *outbuf;
> - struct kdf_sdesc *sdesc = NULL;
> + struct crypto_shash *hash = NULL;
>
> if (!params || (!buffer && buflen)) {
> ret = -EINVAL;
> @@ -257,7 +188,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
> }
>
> /* allocate KDF from the kernel crypto API */
> - ret = kdf_alloc(&sdesc, hashname);
> + ret = kdf_alloc(&hash, hashname);
> kfree(hashname);
> if (ret)
> goto out1;
> @@ -367,7 +298,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
> goto out6;
> }
>
> - ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf,
> + ret = keyctl_dh_compute_kdf(hash, buffer, buflen, outbuf,
> req->dst_len + kdfcopy->otherinfolen);
> } else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) {
> ret = req->dst_len;
> @@ -386,7 +317,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
> out2:
> dh_free_data(&dh_inputs);
> out1:
> - kdf_dealloc(sdesc);
> + kdf_dealloc(hash);
> return ret;
> }
>
> --
> 2.33.1
>
>
>
>
>
--
Mat Martineau
Intel
Am Mittwoch, 17. November 2021, 22:28:46 CET schrieb Mat Martineau:
Hi Mat,
> On Mon, 15 Nov 2021, Stephan M?ller wrote:
> > Remove the specific code that adds a zero padding that was intended
> > to be invoked when the DH operation result was smaller than the
> > modulus. However, this cannot occur any more these days because the
> > function mpi_write_to_sgl is used in the code path that calculates the
> > shared secret in dh_compute_value. This MPI service function guarantees
> > that leading zeros are introduced as needed to ensure the resulting data
> > is exactly as long as the modulus. This implies that the specific code
> > to add zero padding is dead code which can be safely removed.
> >
> > Signed-off-by: Stephan Mueller <[email protected]>
> > ---
> > security/keys/dh.c | 25 ++++---------------------
> > 1 file changed, 4 insertions(+), 21 deletions(-)
>
> Hi Stephan -
>
> Thanks for the cleanup!
Thank you for the review.
>
> Acked-by: Mat Martineau <[email protected]>
I have added your signature to the patch.
Ciao
Stephan