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