[PATCH] lib/crypto: add prompts back to crypto libraries

[PATCH] lib/crypto: add prompts back to crypto libraries

[Justin M. Forbes]

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 <jforbes@fedoraproject.org>
---
 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

Re: [PATCH] lib/crypto: add prompts back to crypto libraries

[Jason A. Donenfeld]

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

Re: [PATCH] lib/crypto: add prompts back to crypto libraries

[Justin Forbes]

On Tue, Jan 11, 2022 at 4:12 PM Jason A. Donenfeld <Jason@zx2c4.com> 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

Re: [PATCH] lib/crypto: add prompts back to crypto libraries

[Ard Biesheuvel]

On Tue, 11 Jan 2022 at 23:12, Jason A. Donenfeld <Jason@zx2c4.com> 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

Re: [PATCH] lib/crypto: add prompts back to crypto libraries

[Jason A. Donenfeld]

On Tue, Jan 11, 2022 at 11:25 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 11 Jan 2022 at 23:12, Jason A. Donenfeld <Jason@zx2c4.com> 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

Re: [PATCH] lib/crypto: add prompts back to crypto libraries

[Ard Biesheuvel]

On Tue, 11 Jan 2022 at 23:27, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Tue, Jan 11, 2022 at 11:25 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 11 Jan 2022 at 23:12, Jason A. Donenfeld <Jason@zx2c4.com> 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

Re: [PATCH] lib/crypto: add prompts back to crypto libraries

[Justin Forbes]

On Tue, Jan 11, 2022 at 4:41 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 11 Jan 2022 at 23:27, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > On Tue, Jan 11, 2022 at 11:25 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Tue, 11 Jan 2022 at 23:12, Jason A. Donenfeld <Jason@zx2c4.com> 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

That works, I will send a V2 with the menu.

Justin

Re: [PATCH] lib/crypto: add prompts back to crypto libraries

[Jason A. Donenfeld]

I think I've actually got a patch for the root problem here. Testing
now and it should be out shortly.

Re: [PATCH] lib/crypto: add prompts back to crypto libraries

[Jason A. Donenfeld]

On Wed, Jan 12, 2022 at 2:59 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> I think I've actually got a patch for the root problem here. Testing
> now and it should be out shortly.

Actually, scratch that, it's a lot harder than I anticipated. I just
saw your v2. Let's roll with that. Sent you a note there for a v3.

Jason