Re: [PATCH 1/9] crypto,fs: Separate out hkdf_extract() and hkdf_expand()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/27/24 19:52, Eric Biggers wrote:
On Tue, Aug 13, 2024 at 01:15:04PM +0200, Hannes Reinecke wrote:
Separate out the HKDF functions into a separate module to
to make them available to other callers.
And add a testsuite to the module with test vectors
from RFC 5869 to ensure the integrity of the algorithm.
[ .. ]
+	desc->tfm = hmac_tfm;
+
+	for (i = 0; i < okmlen; i += hashlen) {
+
+		err = crypto_shash_init(desc);
+		if (err)
+			goto out;
+
+		if (prev) {
+			err = crypto_shash_update(desc, prev, hashlen);
+			if (err)
+				goto out;
+		}
+
+		if (info && infolen) {

'if (infolen)' instead of 'if (info && infolen)'.  The latter is a bad practice
because it can hide bugs.

Do I need to set a 'WARN_ON(!info)' (or something) in this case? Or are the '->update' callbacks expected to handle it themselves?

[ .. ]
diff --git a/include/crypto/hkdf.h b/include/crypto/hkdf.h
new file mode 100644
index 000000000000..ee3e7d21a5fe
--- /dev/null
+++ b/include/crypto/hkdf.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * HKDF: HMAC-based Key Derivation Function (HKDF), RFC 5869
+ *
+ * Extracted from fs/crypto/hkdf.c, which has
+ * Copyright 2019 Google LLC
+ */
+
+#ifndef _CRYPTO_HKDF_H
+#define _CRYPTO_HKDF_H
+
+#ifdef CONFIG_CRYPTO_HKDF
+int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm,
+		 unsigned int ikmlen, const u8 *salt, unsigned int saltlen,
+		 u8 *prk);
+int hkdf_expand(struct crypto_shash *hmac_tfm,
+		const u8 *info, unsigned int infolen,
+		u8 *okm, unsigned int okmlen);
+#else
+static inline int hkdf_extract(struct crypto_shash *hmac_tfm,
+			       const u8 *ikm, unsigned int ikmlen,
+			       const u8 *salt, unsigned int saltlen,
+			       u8 *prk)
+{
+	return -ENOTSUP;
+}
+static inline int hkdf_expand(struct crypto_shash *hmac_tfm,
+			      const u8 *info, unsigned int infolen,
+			      u8 *okm, unsigned int okmlen)
+{
+	return -ENOTSUP;
+}
+#endif
+#endif

This header is missing <crypto/hash.h> which it depends on.

Also the !CONFIG_CRYPTO_HKDF stubs are unnecessary and should not be included.

But that would mean that every call to '#include <crypto/hkdf.h>' would need to be encapsulated by 'CONFIG_CRYPTO_HKDF' (or the file itself is
conditionally compiled on that symbol).
Is that the direction you want to head?

Thanks for the review!

Cheers,

Hannes
--
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@xxxxxxx                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux