Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1476363 > unrolled thread

[PATCH] security/keys: make BIG_KEYS dependent on stdrng.

Started byArtem Savkov <asavkov@redhat.com>
First post2016-09-05 14:40 +0200
Last post2016-09-06 15:20 +0200
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] security/keys: make BIG_KEYS dependent on stdrng. Artem Savkov <asavkov@redhat.com> - 2016-09-05 14:40 +0200
    Re: [PATCH] security/keys: make BIG_KEYS dependent on stdrng. David Howells <dhowells@redhat.com> - 2016-09-06 15:00 +0200
      Re: [PATCH] security/keys: make BIG_KEYS dependent on stdrng. Artem Savkov <asavkov@redhat.com> - 2016-09-06 15:10 +0200
        Re: [PATCH] security/keys: make BIG_KEYS dependent on stdrng. David Howells <dhowells@redhat.com> - 2016-09-06 15:20 +0200
          Re: [PATCH] security/keys: make BIG_KEYS dependent on stdrng. Artem Savkov <asavkov@redhat.com> - 2016-09-06 15:30 +0200
          Re: [PATCH] security/keys: make BIG_KEYS dependent on stdrng. Kirill Marinushkin <k.marinushkin@gmail.com> - 2016-09-06 18:40 +0200
            Re: [PATCH] security/keys: make BIG_KEYS dependent on stdrng. David Howells <dhowells@redhat.com> - 2016-09-06 20:20 +0200
      Re: [PATCH] security/keys: make BIG_KEYS dependent on stdrng. Stephan Mueller <smueller@chronox.de> - 2016-09-06 15:20 +0200

#1476363 — [PATCH] security/keys: make BIG_KEYS dependent on stdrng.

FromArtem Savkov <asavkov@redhat.com>
Date2016-09-05 14:40 +0200
Subject[PATCH] security/keys: make BIG_KEYS dependent on stdrng.
Message-ID<se1lT-3KI-1@gated-at.bofh.it>
Since BIG_KEYS can't be compiled as module it requires one of the "stdrng"
providers to be compiled into kernel. Otherwise big_key_crypto_init() fails
on crypto_alloc_rng step and next dereference of big_key_skcipher (e.g. in
big_key_preparse()) results in a NULL pointer dereference.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
 security/keys/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index f826e87..d942c7c 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -41,7 +41,7 @@ config BIG_KEYS
 	bool "Large payload keys"
 	depends on KEYS
 	depends on TMPFS
-	select CRYPTO
+	depends on (CRYPTO_ANSI_CPRNG = y || CRYPTO_DRBG = y)
 	select CRYPTO_AES
 	select CRYPTO_ECB
 	select CRYPTO_RNG
-- 
2.7.4

[toc] | [next] | [standalone]

#1477338

FromDavid Howells <dhowells@redhat.com>
Date2016-09-06 15:00 +0200
Message-ID<seo8T-29v-21@gated-at.bofh.it>
In reply to#1476363
Artem Savkov <asavkov@redhat.com> wrote:

> -	select CRYPTO
> +	depends on (CRYPTO_ANSI_CPRNG = y || CRYPTO_DRBG = y)

Should those be "==" not "="?

David

[toc] | [prev] | [next] | [standalone]

#1477353

FromArtem Savkov <asavkov@redhat.com>
Date2016-09-06 15:10 +0200
Message-ID<seoiu-2rH-27@gated-at.bofh.it>
In reply to#1477338
On Tue, Sep 06, 2016 at 01:58:49PM +0100, David Howells wrote:
> Artem Savkov <asavkov@redhat.com> wrote:
> 
> > -	select CRYPTO
> > +	depends on (CRYPTO_ANSI_CPRNG = y || CRYPTO_DRBG = y)
> 
> Should those be "==" not "="?

Accodring to Documentation/kbuild/kconfig-language.txt (line 173) it is
"=" and I can only see "=" being used in existing Kconfigs.

-- 
Regards,
  Artem

[toc] | [prev] | [next] | [standalone]

#1477363

FromDavid Howells <dhowells@redhat.com>
Date2016-09-06 15:20 +0200
Message-ID<seos9-2uF-13@gated-at.bofh.it>
In reply to#1477353
Artem Savkov <asavkov@redhat.com> wrote:

> > > -	select CRYPTO
> > > +	depends on (CRYPTO_ANSI_CPRNG = y || CRYPTO_DRBG = y)
> > 
> > Should those be "==" not "="?
> 
> Accodring to Documentation/kbuild/kconfig-language.txt (line 173) it is
> "=" and I can only see "=" being used in existing Kconfigs.

Okay.  The other thing is that I have been given a conflicting patch (see
below).  Is your fix preferable?

David
---
commit 69ed34b303f87a1a53470dd37149ac1573d79da2
Author: Kirill Marinushkin <k.marinushkin@gmail.com>
Date: Mon, 8 Aug 2016 23:19:32 +0200

KEYS: fix big_key dependency

Signed-off-by: Kirill Marinushkin <k.marinushkin@gmail.com>
cc: David Howells <dhowells@redhat.com>
cc: Peter Hlavaty <zer0mem@yahoo.com>
cc: Greg KH <gregkh@linuxfoundation.org>
cc: stable@vger.kernel.org
---
 security/keys/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index f826e87..8213221 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -44,7 +44,7 @@ config BIG_KEYS
 	select CRYPTO
 	select CRYPTO_AES
 	select CRYPTO_ECB
-	select CRYPTO_RNG
+	select CRYPTO_ANSI_CPRNG
 	help
 	  This option provides support for holding large keys within the kernel
 	  (for example Kerberos ticket caches).  The data may be stored out to

[toc] | [prev] | [next] | [standalone]

#1477368

FromArtem Savkov <asavkov@redhat.com>
Date2016-09-06 15:30 +0200
Message-ID<seoBP-2y1-5@gated-at.bofh.it>
In reply to#1477363
On Tue, Sep 06, 2016 at 02:11:56PM +0100, David Howells wrote:
> Artem Savkov <asavkov@redhat.com> wrote:
> 
> > > > -	select CRYPTO
> > > > +	depends on (CRYPTO_ANSI_CPRNG = y || CRYPTO_DRBG = y)
> > > 
> > > Should those be "==" not "="?
> > 
> > Accodring to Documentation/kbuild/kconfig-language.txt (line 173) it is
> > "=" and I can only see "=" being used in existing Kconfigs.
> 
> Okay.  The other thing is that I have been given a conflicting patch (see
> below).  Is your fix preferable?
> 
> David
> ---
> commit 69ed34b303f87a1a53470dd37149ac1573d79da2
> Author: Kirill Marinushkin <k.marinushkin@gmail.com>
> Date: Mon, 8 Aug 2016 23:19:32 +0200
> 
> KEYS: fix big_key dependency
> 
> Signed-off-by: Kirill Marinushkin <k.marinushkin@gmail.com>
> cc: David Howells <dhowells@redhat.com>
> cc: Peter Hlavaty <zer0mem@yahoo.com>
> cc: Greg KH <gregkh@linuxfoundation.org>
> cc: stable@vger.kernel.org
> ---
>  security/keys/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index f826e87..8213221 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -44,7 +44,7 @@ config BIG_KEYS
>  	select CRYPTO
>  	select CRYPTO_AES
>  	select CRYPTO_ECB
> -	select CRYPTO_RNG
> +	select CRYPTO_ANSI_CPRNG
>  	help
>  	  This option provides support for holding large keys within the kernel
>  	  (for example Kerberos ticket caches).  The data may be stored out to

I would argue that locking a user into a specific stdrng implementation
is not something that should be done when there are options available.

-- 
Regards,
  Artem

[toc] | [prev] | [next] | [standalone]

#1477594

FromKirill Marinushkin <k.marinushkin@gmail.com>
Date2016-09-06 18:40 +0200
Message-ID<serzI-4pL-7@gated-at.bofh.it>
In reply to#1477363
> The other thing is that I have been given a conflicting patch (see
> below).  Is your fix preferable?

The patch you attached previously is v1; I submitted v2 after review by
Stephan Mueller. I additionally attach it here below.

IMO, the preferable fix depends on your future plan.
If you plan to continue using both ANSI X9.31 DRNG and DRBG - I agree with the
patch suggested by Artem Savkov.
If you plan to reduce using ANSI X9.31 DRNG and use DRBG more widely - I
suggest my patch.

Best Regards,
Kirill

---
From	Kirill Marinushkin <k.marinushkin@gmail.com>
Subject	[PATCH v2] KEYS: fix big_key dependency
Date	Tue, 16 Aug 2016 21:51:12 +0200

This patch fixes the following bug:
[oss-security] - panic at big_key_preparse #4.7-r6/rc7 & master

Signed-off-by: Kirill Marinushkin <k.marinushkin@gmail.com>
---
 security/keys/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index f826e87..5bc5114 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -44,7 +44,7 @@ config BIG_KEYS
        select CRYPTO
        select CRYPTO_AES
        select CRYPTO_ECB
-       select CRYPTO_RNG
+       select CRYPTO_RNG_DEFAULT

[toc] | [prev] | [next] | [standalone]

#1477727

FromDavid Howells <dhowells@redhat.com>
Date2016-09-06 20:20 +0200
Message-ID<set8u-5vV-9@gated-at.bofh.it>
In reply to#1477594
Kirill Marinushkin <k.marinushkin@gmail.com> wrote:

> IMO, the preferable fix depends on your future plan.
> If you plan to continue using both ANSI X9.31 DRNG and DRBG - I agree with the
> patch suggested by Artem Savkov.
> If you plan to reduce using ANSI X9.31 DRNG and use DRBG more widely - I
> suggest my patch.

No such plans, TBH.

David

[toc] | [prev] | [next] | [standalone]

#1477361

FromStephan Mueller <smueller@chronox.de>
Date2016-09-06 15:20 +0200
Message-ID<seos9-2uF-23@gated-at.bofh.it>
In reply to#1477338
Am Dienstag, 6. September 2016, 14:11:56 CEST schrieb David Howells:

Hi David,

> Artem Savkov <asavkov@redhat.com> wrote:
> > > > -	select CRYPTO
> > > > +	depends on (CRYPTO_ANSI_CPRNG = y || CRYPTO_DRBG = y)
> > > 
> > > Should those be "==" not "="?
> > 
> > Accodring to Documentation/kbuild/kconfig-language.txt (line 173) it is
> > "=" and I can only see "=" being used in existing Kconfigs.
> 
> Okay.  The other thing is that I have been given a conflicting patch (see
> below).  Is your fix preferable?

The listed patch only selects the ANSI X9.31 DRNG and thus conflicts with FIPS 
mode and the current default stdrng which is the DRBG.

Ciao
Stephan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web