Commit 6048fdcc5f269 ("lib/crypto: blake2s: include as built-in") took
away a number of prompt texts from other crypto libraries. This makes
values flip from built-in to module when oldconfig runs, and causes
problems when these crypto libs need to be built in for thingslike
BIG_KEYS.
Fixes: 6048fdcc5f269 ("lib/crypto: blake2s: include as built-in")
Signed-off-by: Justin M. Forbes <[email protected]>
---
lib/crypto/Kconfig | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 8620f38e117c..a3e41b7a8054 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -40,7 +40,7 @@ config CRYPTO_LIB_CHACHA_GENERIC
of CRYPTO_LIB_CHACHA.
config CRYPTO_LIB_CHACHA
- tristate
+ tristate "ChaCha library interface"
depends on CRYPTO_ARCH_HAVE_LIB_CHACHA || !CRYPTO_ARCH_HAVE_LIB_CHACHA
select CRYPTO_LIB_CHACHA_GENERIC if CRYPTO_ARCH_HAVE_LIB_CHACHA=n
help
@@ -65,7 +65,7 @@ config CRYPTO_LIB_CURVE25519_GENERIC
of CRYPTO_LIB_CURVE25519.
config CRYPTO_LIB_CURVE25519
- tristate
+ tristate "Curve25519 scalar multiplication library"
depends on CRYPTO_ARCH_HAVE_LIB_CURVE25519 || !CRYPTO_ARCH_HAVE_LIB_CURVE25519
select CRYPTO_LIB_CURVE25519_GENERIC if CRYPTO_ARCH_HAVE_LIB_CURVE25519=n
help
@@ -100,7 +100,7 @@ config CRYPTO_LIB_POLY1305_GENERIC
of CRYPTO_LIB_POLY1305.
config CRYPTO_LIB_POLY1305
- tristate
+ tristate "Poly1305 library interface"
depends on CRYPTO_ARCH_HAVE_LIB_POLY1305 || !CRYPTO_ARCH_HAVE_LIB_POLY1305
select CRYPTO_LIB_POLY1305_GENERIC if CRYPTO_ARCH_HAVE_LIB_POLY1305=n
help
@@ -109,7 +109,7 @@ config CRYPTO_LIB_POLY1305
is available and enabled.
config CRYPTO_LIB_CHACHA20POLY1305
- tristate
+ tristate "ChaCha20-Poly1305 AEAD support (8-byte nonce library version)"
depends on CRYPTO_ARCH_HAVE_LIB_CHACHA || !CRYPTO_ARCH_HAVE_LIB_CHACHA
depends on CRYPTO_ARCH_HAVE_LIB_POLY1305 || !CRYPTO_ARCH_HAVE_LIB_POLY1305
select CRYPTO_LIB_CHACHA
--
2.34.1
Hi Justin,
These are library variables, which means they really have no sense in
being user selectable. Internal things to the kernel depend on them,
or they don't. They're always only dependencies.
It sounds like CONFIG_BIG_KEYS might be declaring its dependencies
wrong, and that's actually where the bug is? CC'ing David Howells just
in case. Maybe things should be changed to:
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 64b81abd087e..2f1624c9eed9 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -60,7 +60,7 @@ config BIG_KEYS
bool "Large payload keys"
depends on KEYS
depends on TMPFS
- depends on CRYPTO_LIB_CHACHA20POLY1305 = y
+ select CRYPTO_LIB_CHACHA20POLY1305
help
This option provides support for holding large keys within the kernel
(for example Kerberos ticket caches). The data may be stored out to
Jason
On Tue, Jan 11, 2022 at 4:12 PM Jason A. Donenfeld <[email protected]> wrote:
>
> Hi Justin,
>
> These are library variables, which means they really have no sense in
> being user selectable. Internal things to the kernel depend on them,
> or they don't. They're always only dependencies.
While this is correct in principle, it has not been that way in the
past. The change had nothing to do with the patch it came in on, and
breaks existing configs used by some distros. BIG_KEYS happened to be
our motivation for flipping those from module to built in, but I have
no way of knowing what else might be in a similar situation. Worse,
many users will never know it is happening. We simply have some
scripts that warn loudly if a config value is flipped from what it was
set to in our prebuild files.
> It sounds like CONFIG_BIG_KEYS might be declaring its dependencies
> wrong, and that's actually where the bug is? CC'ing David Howells just
> in case. Maybe things should be changed to:
>
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index 64b81abd087e..2f1624c9eed9 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -60,7 +60,7 @@ config BIG_KEYS
> bool "Large payload keys"
> depends on KEYS
> depends on TMPFS
> - depends on CRYPTO_LIB_CHACHA20POLY1305 = y
> + select CRYPTO_LIB_CHACHA20POLY1305
> help
> This option provides support for holding large keys within the kernel
> (for example Kerberos ticket caches). The data may be stored out to
This looks correct, and would likely alleviate our initial problem,
but it doesn't mean nothing else is impacted. I would still be in
favor of my patch going in to revert the dropped prompts for 5.17, and
then we can audit everything else that has any type of dep on the
crypto libs in question and provide a patch to fix everything up
correctly. I am happy to do that audit, but probably wouldn't get the
time to do so until rc2 or later. I do not see how the dropped
prompts for these libs have anything to do with the "blake2s: include
as built-in" that brought them in.
Justin
>
> Jason
On Tue, 11 Jan 2022 at 23:12, Jason A. Donenfeld <[email protected]> wrote:
>
> Hi Justin,
>
> These are library variables, which means they really have no sense in
> being user selectable. Internal things to the kernel depend on them,
> or they don't. They're always only dependencies.
>
But what does any of this have to do with blake2s? These are unrelated
changes that are not even described in the commit log of the original
patch, so let's just revert them now. If changes are needed here, we
can discuss them on the linux-crypto mailing list after the merge
window.
> It sounds like CONFIG_BIG_KEYS might be declaring its dependencies
> wrong, and that's actually where the bug is? CC'ing David Howells just
> in case. Maybe things should be changed to:
>
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index 64b81abd087e..2f1624c9eed9 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -60,7 +60,7 @@ config BIG_KEYS
> bool "Large payload keys"
> depends on KEYS
> depends on TMPFS
> - depends on CRYPTO_LIB_CHACHA20POLY1305 = y
> + select CRYPTO_LIB_CHACHA20POLY1305
> help
> This option provides support for holding large keys within the kernel
> (for example Kerberos ticket caches). The data may be stored out to
>
>
> Jason
On Tue, Jan 11, 2022 at 11:25 PM Ard Biesheuvel <[email protected]> wrote:
>
> On Tue, 11 Jan 2022 at 23:12, Jason A. Donenfeld <[email protected]> wrote:
> >
> > Hi Justin,
> >
> > These are library variables, which means they really have no sense in
> > being user selectable. Internal things to the kernel depend on them,
> > or they don't. They're always only dependencies.
> >
>
> But what does any of this have to do with blake2s? These are unrelated
> changes that are not even described in the commit log of the original
> patch, so let's just revert them now. If changes are needed here, we
> can discuss them on the linux-crypto mailing list after the merge
> window.
The lib crypto stuff moved outside of `if CRYPTO`, so if you add those
titles back, the root menu is going to be filled with things. I'm
working on some patches now moving lib/crypto/ things into lib
strictly, so the dependency is one way. I can try adding back the
labels there if you want.
Jason
On Tue, 11 Jan 2022 at 23:27, Jason A. Donenfeld <[email protected]> wrote:
>
> On Tue, Jan 11, 2022 at 11:25 PM Ard Biesheuvel <[email protected]> wrote:
> >
> > On Tue, 11 Jan 2022 at 23:12, Jason A. Donenfeld <[email protected]> wrote:
> > >
> > > Hi Justin,
> > >
> > > These are library variables, which means they really have no sense in
> > > being user selectable. Internal things to the kernel depend on them,
> > > or they don't. They're always only dependencies.
> > >
> >
> > But what does any of this have to do with blake2s? These are unrelated
> > changes that are not even described in the commit log of the original
> > patch, so let's just revert them now. If changes are needed here, we
> > can discuss them on the linux-crypto mailing list after the merge
> > window.
>
> The lib crypto stuff moved outside of `if CRYPTO`, so if you add those
> titles back, the root menu is going to be filled with things. I'm
> working on some patches now moving lib/crypto/ things into lib
> strictly, so the dependency is one way. I can try adding back the
> labels there if you want.
>
Ah, right. In that case, can we fold in something like the below?
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index a3e41b7a8054..179041b60294 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+menu "Crypto library routines"
+
config CRYPTO_LIB_AES
tristate
@@ -120,3 +122,5 @@ config CRYPTO_LIB_SHA256
config CRYPTO_LIB_SM4
tristate
+
+endmenu