Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: simd ciphers
From: Stephan Müller @ 2016-12-28 10:00 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20161228085938.GA11904@gondor.apana.org.au>

Am Mittwoch, 28. Dezember 2016, 16:59:38 CET schrieb Herbert Xu:

Hi Herbert,

> > With 4.10-rc1, I also do not get the AES-NI implementations to work. do
> > you
> > have any ideas what may be the issue?
> 
> I'm having problems reproducing this.  Does it work for you if you
> build them as modules?

When I configure AES-NI as module, it works. When compiling it statically, the 
allocation returns an -ENOENT. 

Now, when I diff both config files after a simple toggle from AES-NI=y to =m I 
see (linux-4.10.config contains the static config, .config contains the module 
configuration):

$ diff -urN ../linux-4.10.config .config
--- ../linux-4.10.config        2016-12-28 10:56:51.805526584 +0100
+++ .config     2016-12-28 11:00:17.959608195 +0100
@@ -4062,8 +4062,8 @@
 CONFIG_CRYPTO_AUTHENC=m
 CONFIG_CRYPTO_TEST=m
 CONFIG_CRYPTO_ABLK_HELPER=m
-CONFIG_CRYPTO_SIMD=y
-CONFIG_CRYPTO_GLUE_HELPER_X86=y
+CONFIG_CRYPTO_SIMD=m
+CONFIG_CRYPTO_GLUE_HELPER_X86=m
 
 #
 # Authenticated Encryption with Associated Data
@@ -4132,7 +4132,7 @@
 #
 CONFIG_CRYPTO_AES=y
 CONFIG_CRYPTO_AES_X86_64=y
-CONFIG_CRYPTO_AES_NI_INTEL=y
+CONFIG_CRYPTO_AES_NI_INTEL=m
 CONFIG_CRYPTO_ANUBIS=m
 CONFIG_CRYPTO_ARC4=m
 CONFIG_CRYPTO_BLOWFISH=m

Ciao
Stephan

^ permalink raw reply

* Re: simd ciphers
From: Herbert Xu @ 2016-12-28  8:59 UTC (permalink / raw)
  To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <1497014.RmdRyQUAOo@positron.chronox.de>

On Wed, Dec 28, 2016 at 09:37:04AM +0100, Stephan Müller wrote:
> Am Donnerstag, 8. Dezember 2016, 19:40:38 CET schrieb Stephan Müller:
> 
> Hi Herbert,
> 
> > Am Donnerstag, 8. Dezember 2016, 19:32:03 CET schrieb Stephan Müller:
> > 
> > Hi Herbert,
> > 
> > > Hi Herbert,
> > > 
> > > with the addition of the simd cipher change I seem to be unable to use the
> > > AESNI ciphers. My .config contains CONFIG_CRYPTO_SIMD=y and
> > > CONFIG_CRYPTO_AES_NI_INTEL=y. Those options always worked to load the
> > > AES-NI
> > (with "always worked" I meant the CONFIG_CRYPTO_AES_NI_INTEL=y option of
> > course, that always ensured AES-NI to be present and usable)
> 
> With 4.10-rc1, I also do not get the AES-NI implementations to work. do you 
> have any ideas what may be the issue?

I'm having problems reproducing this.  Does it work for you if you
build them as modules?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v2 1/3] crypto: chacha20 - convert generic and x86 versions to skcipher
From: Herbert Xu @ 2016-12-28  9:25 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <C46A3CDE-2329-4171-B4C5-071423EC3B4B@linaro.org>

On Wed, Dec 28, 2016 at 09:23:07AM +0000, Ard Biesheuvel wrote:
>
> Ehm, could you keep this one and revert the other two instead please?  This patch will not change in the respin, and the other two are blkciphers rather than skciphers (and in the arm64 version, I optimized the asm code as well) so they will look rather different ( as they do in the nacked v2)

OK I will keep this patch and revert the other two.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] crypto: arm/aes-neonbs - process 8 blocks in parallel if we can
From: Herbert Xu @ 2016-12-28  9:23 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <B32240B5-953F-42A2-A010-6FF64AC45859@linaro.org>

On Wed, Dec 28, 2016 at 09:19:32AM +0000, Ard Biesheuvel wrote:
>
> Ok, so that implies a field in the skcipher algo struct then, rather than some definition internal to the driver?

Oh yes it should definitely be visible to other crypto API drivers
and algorithms.  It's just that we don't want to export it outside
of the crypto API, e.g., to IPsec or algif.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v2 1/3] crypto: chacha20 - convert generic and x86 versions to skcipher
From: Ard Biesheuvel @ 2016-12-28  9:23 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <20161228091801.GA12318@gondor.apana.org.au>



> On 28 Dec 2016, at 09:18, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
>> On Tue, Dec 27, 2016 at 06:04:52PM +0800, Herbert Xu wrote:
>>> On Fri, Dec 09, 2016 at 02:33:51PM +0000, Ard Biesheuvel wrote:
>>> This converts the ChaCha20 code from a blkcipher to a skcipher, which
>>> is now the preferred way to implement symmetric block and stream ciphers.
>>> 
>>> This ports the generic and x86 versions at the same time because the
>>> latter reuses routines of the former.
>>> 
>>> Note that the skcipher_walk() API guarantees that all presented blocks
>>> except the final one are a multiple of the chunk size, so we can simplify
>>> the encrypt() routine somewhat.
>>> 
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> 
>> Patch applied.  Thanks.
> 
> I'm reverting this patch because it breaks the build on ARM without
> the two subsequent patches.
> 
> When resubmitting please make sure that the kernel can build and
> work after each patch.  Otherwise git bisections will fail.
> 

Ehm, could you keep this one and revert the other two instead please?  This patch will not change in the respin, and the other two are blkciphers rather than skciphers (and in the arm64 version, I optimized the asm code as well) so they will look rather different ( as they do in the nacked v2)

> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v2 1/3] crypto: chacha20 - convert generic and x86 versions to skcipher
From: Herbert Xu @ 2016-12-28  9:20 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <20161228091801.GA12318@gondor.apana.org.au>

On Wed, Dec 28, 2016 at 05:18:01PM +0800, Herbert Xu wrote:
> On Tue, Dec 27, 2016 at 06:04:52PM +0800, Herbert Xu wrote:
> > On Fri, Dec 09, 2016 at 02:33:51PM +0000, Ard Biesheuvel wrote:
> > > This converts the ChaCha20 code from a blkcipher to a skcipher, which
> > > is now the preferred way to implement symmetric block and stream ciphers.
> > > 
> > > This ports the generic and x86 versions at the same time because the
> > > latter reuses routines of the former.
> > > 
> > > Note that the skcipher_walk() API guarantees that all presented blocks
> > > except the final one are a multiple of the chunk size, so we can simplify
> > > the encrypt() routine somewhat.
> > > 
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > 
> > Patch applied.  Thanks.
> 
> I'm reverting this patch because it breaks the build on ARM without
> the two subsequent patches.
> 
> When resubmitting please make sure that the kernel can build and
> work after each patch.  Otherwise git bisections will fail.

Sorry, it only broke the build because I applied your other two
patches first.

So I'll just revert all three and it should work.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] crypto: arm/aes-neonbs - process 8 blocks in parallel if we can
From: Ard Biesheuvel @ 2016-12-28  9:19 UTC (permalink / raw)
  To: Herbert Xu
  Cc: linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20161228091020.GE11904@gondor.apana.org.au>


> On 28 Dec 2016, at 09:10, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
>> On Tue, Dec 27, 2016 at 06:35:46PM +0000, Ard Biesheuvel wrote:
>> 
>> OK, I will try to hack something up.
>> 
>> One thing to keep in mind though is that stacked chaining modes should
>> present the data with the same granularity for optimal performance.
>> E.g., xts(ecb(aes)) should pass 8 blocks at a time. How should this
>> requirement be incorporated according to you?
> 
> xts tries to do a page at a time, which should be good enough, no?
> 

Yes, if the xts chaining mode driver invokes the ecb transform with that granularity, it should be fine. Note that this is a theoretical
concern for this mode in particular, given that the bit sliced aes code implements the xts bits as well

> In general this parameter should be visible to internal API users
> such as xts so they could use it to determine how it wants to
> structure its walks.
> 

Ok, so that implies a field in the skcipher algo struct then, rather than some definition internal to the driver?

> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v2 1/3] crypto: chacha20 - convert generic and x86 versions to skcipher
From: Herbert Xu @ 2016-12-28  9:18 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <20161227100452.GB10629@gondor.apana.org.au>

On Tue, Dec 27, 2016 at 06:04:52PM +0800, Herbert Xu wrote:
> On Fri, Dec 09, 2016 at 02:33:51PM +0000, Ard Biesheuvel wrote:
> > This converts the ChaCha20 code from a blkcipher to a skcipher, which
> > is now the preferred way to implement symmetric block and stream ciphers.
> > 
> > This ports the generic and x86 versions at the same time because the
> > latter reuses routines of the former.
> > 
> > Note that the skcipher_walk() API guarantees that all presented blocks
> > except the final one are a multiple of the chunk size, so we can simplify
> > the encrypt() routine somewhat.
> > 
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Patch applied.  Thanks.

I'm reverting this patch because it breaks the build on ARM without
the two subsequent patches.

When resubmitting please make sure that the kernel can build and
work after each patch.  Otherwise git bisections will fail.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 0/2] crypto: arm64/ARM: NEON accelerated ChaCha20
From: Herbert Xu @ 2016-12-28  9:13 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <AA6F25FA-C998-462F-9E25-FF543BBE3EA7@linaro.org>

On Wed, Dec 28, 2016 at 09:11:03AM +0000, Ard Biesheuvel wrote:
> 
> > On 28 Dec 2016, at 09:03, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > 
> >> On Tue, Dec 27, 2016 at 02:26:35PM +0000, Ard Biesheuvel wrote:
> >> 
> >> You just nacked the v2 of this series (due to the chunksize/walksize) and i rewrote them as skciphers as well
> > 
> > Sorry.  Would you like me to revert or just send a new series
> > on top of this?
> > 
> 
> No worries. If you can, please drop them entirely, or revert them otherwise. I will resend after the holidays

OK I will revert.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 0/2] crypto: arm64/ARM: NEON accelerated ChaCha20
From: Ard Biesheuvel @ 2016-12-28  9:11 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <20161228090350.GD11904@gondor.apana.org.au>


> On 28 Dec 2016, at 09:03, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
>> On Tue, Dec 27, 2016 at 02:26:35PM +0000, Ard Biesheuvel wrote:
>> 
>> You just nacked the v2 of this series (due to the chunksize/walksize) and i rewrote them as skciphers as well
> 
> Sorry.  Would you like me to revert or just send a new series
> on top of this?
> 

No worries. If you can, please drop them entirely, or revert them otherwise. I will resend after the holidays


> Thanks,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] crypto: arm/aes-neonbs - process 8 blocks in parallel if we can
From: Herbert Xu @ 2016-12-28  9:10 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKv+Gu8OkOR3Xzx=XEOWyFpo6sLiq=WQrq0SauaY0V57uYetCQ@mail.gmail.com>

On Tue, Dec 27, 2016 at 06:35:46PM +0000, Ard Biesheuvel wrote:
>
> OK, I will try to hack something up.
> 
> One thing to keep in mind though is that stacked chaining modes should
> present the data with the same granularity for optimal performance.
> E.g., xts(ecb(aes)) should pass 8 blocks at a time. How should this
> requirement be incorporated according to you?

xts tries to do a page at a time, which should be good enough, no?

In general this parameter should be visible to internal API users
such as xts so they could use it to determine how it wants to
structure its walks.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 0/2] crypto: arm64/ARM: NEON accelerated ChaCha20
From: Herbert Xu @ 2016-12-28  9:03 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <6124F827-BA28-4A3D-B05F-EBCB1323955D@linaro.org>

On Tue, Dec 27, 2016 at 02:26:35PM +0000, Ard Biesheuvel wrote:
>
> You just nacked the v2 of this series (due to the chunksize/walksize) and i rewrote them as skciphers as well

Sorry.  Would you like me to revert or just send a new series
on top of this?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [RFC PATCH 0/3] Cavium ThunderX ZIP driver
From: Herbert Xu @ 2016-12-28  9:01 UTC (permalink / raw)
  To: Challa, Mahipal
  Cc: Jan Glauber, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org, David S . Miller, Nair, Vishnu
In-Reply-To: <DM5PR07MB3148D87C62159523B750CD20E5690@DM5PR07MB3148.namprd07.prod.outlook.com>

On Tue, Dec 27, 2016 at 11:39:21AM +0000, Challa, Mahipal wrote:
>
> Mahipal: One major issue is, the kernel use cases to validate the Cavium ThunderX ZIP driver are "ZSWAP" and "IPComp" which are not yet supported with scomp-acomp framework. 

OK I'll look into converting IPcomp.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: xts(ecb(aes-asm))
From: Herbert Xu @ 2016-12-28  9:01 UTC (permalink / raw)
  To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <4308953.jQs16UQPE9@tauon.atsec.com>

On Fri, Dec 09, 2016 at 07:02:25AM +0100, Stephan Müller wrote:
> Hi Herbert,
> 
> while testing the current cryptodev-2.6 tree, I noticed that instead of the 
> driver name of xts(aes-asm) (which used to be there), I now see xts(ecb(aes-
> asm)).
> 
> Is that intentional?

Yes this is a result of the xts template switching over to skciphers
instead of simple ciphers as its base.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: simd ciphers
From: Stephan Müller @ 2016-12-28  8:37 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <10079298.uODr7IfrOJ@tauon.atsec.com>

Am Donnerstag, 8. Dezember 2016, 19:40:38 CET schrieb Stephan Müller:

Hi Herbert,

> Am Donnerstag, 8. Dezember 2016, 19:32:03 CET schrieb Stephan Müller:
> 
> Hi Herbert,
> 
> > Hi Herbert,
> > 
> > with the addition of the simd cipher change I seem to be unable to use the
> > AESNI ciphers. My .config contains CONFIG_CRYPTO_SIMD=y and
> > CONFIG_CRYPTO_AES_NI_INTEL=y. Those options always worked to load the
> > AES-NI
> (with "always worked" I meant the CONFIG_CRYPTO_AES_NI_INTEL=y option of
> course, that always ensured AES-NI to be present and usable)

With 4.10-rc1, I also do not get the AES-NI implementations to work. do you 
have any ideas what may be the issue?

Ciao
Stephan

^ permalink raw reply

* Re: George's crazy full state idea (Re: HalfSipHash Acceptable Usage)
From: Hannes Frederic Sowa @ 2016-12-28  5:23 UTC (permalink / raw)
  To: George Spelvin, daniel
  Cc: ak, davem, David.Laight, ebiggers3, eric.dumazet, Jason,
	kernel-hardening, linux-crypto, linux-kernel, luto, netdev, tom,
	tytso, vegard.nossum
In-Reply-To: <20161224011754.14207.qmail@ns.sciencehorizons.net>

Hello,

On Fri, 2016-12-23 at 20:17 -0500, George Spelvin wrote:
> Hannes Frederic Sowa wrote:
> > On 24.12.2016 00:39, George Spelvin wrote:
> > > We just finished discussing why 8 bytes isn't enough.  If you only
> > > feed back 8 bytes, an attacker who can do 2^64 computation can find it
> > > (by guessing and computing forward to verify the guess) and recover the
> > > previous state.  You need to feed back at least as much output as your
> > > security targete.  For /dev/urandom's ChaCha20, that's 256 bits.
> > I followed the discussion but it appeared to me that this has the
> > additional constraint of time until the next reseeding event happenes,
> > which is 300s (under the assumption that calls to get_random_int happen
> > regularly, which I expect right now). After that the existing reseeding
> > mechansim will ensure enough backtracking protection. The number of
> > bytes can easily be increased here, given that reseeding was shown to be
> > quite fast already and we produce enough output. But I am not sure if
> > this is a bit overengineered in the end?
> 
> I'm not following your description of how the time-based and call-based
> mechanisms interact, but for any mix-back, you should either do enough
> or none at all.  (Also called "catastrophic reseeding".)

We call extract_crng when we run out of batched entropy and reseed. How
often we call down to extract_crng depends on how much entropy we
extracted by calls to get_random_int/long, so the number of calls into
those functions matter.

In extract_crng we have a timer which reseeds every 300s the CPRNG and
either uses completely new entropy from the CRNG or calls down into the
CPRNG while also doing backtracing protection (which feeds chacha's
block size / 2 back into chacha, if I read the code correctly, thus
1024 bits, which should be enough).

> For example, two mix-backs of 64 bits gives you 65 bit security, not 128.
> (Because each mixback can be guessed and verified separately.)

Exactly, but the full reseed after running out of entropy is strong
enough to not be defeated by your argumentation. Neither the reseed
from the CRNG.

> > Also agreed. Given your solution below to prandom_u32, I do think it
> > might also work without the seqlock now.
> 
> It's not technically a seqlock; in particular the reader doesn't
> spin.  But the write side, and general logic is so similar it's
> a good mental model.
> 
> Basically, assume a 64-byte buffer.  The reader has gone through
> 32 bytes of it, and has 32 left, and when he reads another 8 bytes,
> has to distinguish three cases:
> 
> 1) No update; we read the old bytes and there are now 32 - 24 bytes left.
> 2) Update completed while we weren't looking.  There are now new
>    bytes in the buffer, and we took 8 leaving 64 - 8 = 56.
> 3) Update in progress at the time of the read.  We don't know if we
>    are seeing old bytes or new bytes, so we have to assume the worst
>    and not proceeed unless 32 >= 8, but assume at the end there are
>    64 - 8 = 56 new bytes left.
> 
> > I wouldn't have added a disable irqs, but given that I really like your
> > proposal, I would take it in my todo branch and submit it when net-next
> > window opens.
> 
> If you want that, I have a pile of patches to prandom I really
> should push upstream.  Shall I refresh them and send them to you?

I would like to have a look at them in the new year, certainly! I can
also take care about the core prandom patches, but don't know if I have
time to submit the others to the different subsystems.

Maybe, if David would be okay with that, we can submit all patches
through his tree, as he is also the dedicated maintainer for prandom.

> [... patch descriptions ...]

Thanks,
Hannes

^ permalink raw reply

* [PATCH 0/8] random: cleanup of code after removal of nonblocking pool
From: Stephan Müller @ 2016-12-27 22:38 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto

Hi Ted,

with the removal of the nonblocking_pool, several code paths are now unused
which were only applicable to the nonblocking pool. This patch set removes
these unused code paths.

Also, a code path in the add_interrupt_randomness function that is never used
is removed.

In addition, the FIPS 140-2 continuous self tests are required to process
the output data of the RNG given to a caller. A patch is added to cover this
requirement.

Ciao
Stephan

Stephan Mueller (8):
  random: remove stale maybe_reseed_primary_crng
  random: remove stale urandom_init_wait
  random: trigger random_ready callback upon crng_init == 1
  random: remove unused branch in hot code path
  random: remove variable limit
  random: fix comment for unused random_min_urandom_seed
  random: remove noop function call to xfer_secondary_pool
  random: move FIPS continuous test to output functions

 drivers/char/random.c | 118 +++++++++++++++++++++++---------------------------
 1 file changed, 53 insertions(+), 65 deletions(-)

-- 
2.9.3

^ permalink raw reply

* [PATCH 3/8] random: trigger random_ready callback upon crng_init == 1
From: Stephan Müller @ 2016-12-27 22:39 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto
In-Reply-To: <3254875.f5A5oHPdxF@positron.chronox.de>

The random_ready callback mechanism is intended to replicate the
getrandom system call behavior to in-kernel users. As the getrandom
system call unblocks with crng_init == 1, trigger the random_ready
wakeup call at the same time.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 drivers/char/random.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 482531d..5c26b1c 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -810,6 +810,7 @@ static int crng_fast_load(const char *cp, size_t len)
 	}
 	if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
 		crng_init = 1;
+		process_random_ready_list();
 		wake_up_interruptible(&crng_init_wait);
 		pr_notice("random: fast init done\n");
 	}
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/8] random: remove stale urandom_init_wait
From: Stephan Müller @ 2016-12-27 22:39 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto
In-Reply-To: <3254875.f5A5oHPdxF@positron.chronox.de>

The urandom_init_wait wait queue is a left over from the pre-ChaCha20
times and can therefore be savely removed.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 drivers/char/random.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 8e5ab20..482531d 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -409,7 +409,6 @@ static struct poolinfo {
  */
 static DECLARE_WAIT_QUEUE_HEAD(random_read_wait);
 static DECLARE_WAIT_QUEUE_HEAD(random_write_wait);
-static DECLARE_WAIT_QUEUE_HEAD(urandom_init_wait);
 static struct fasync_struct *fasync;
 
 static DEFINE_SPINLOCK(random_ready_list_lock);
-- 
2.9.3

^ permalink raw reply related

* [PATCH 4/8] random: remove unused branch in hot code path
From: Stephan Müller @ 2016-12-27 22:40 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto
In-Reply-To: <3254875.f5A5oHPdxF@positron.chronox.de>

The variable ip is defined to be a __u64 which is always 8 bytes on any
architecture. Thus, the check for sizeof(ip) > 4 will always be true.

As the check happens in a hot code path, remove the branch.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 drivers/char/random.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 5c26b1c..8d4d720 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1136,8 +1136,7 @@ void add_interrupt_randomness(int irq, int irq_flags)
 	fast_pool->pool[1] ^= now ^ c_high;
 	ip = regs ? instruction_pointer(regs) : _RET_IP_;
 	fast_pool->pool[2] ^= ip;
-	fast_pool->pool[3] ^= (sizeof(ip) > 4) ? ip >> 32 :
-		get_reg(fast_pool, regs);
+	fast_pool->pool[3] ^= ip >> 32;
 
 	fast_mix(fast_pool);
 	add_interrupt_bench(cycles);
-- 
2.9.3

^ permalink raw reply related

* [PATCH 5/8] random: remove variable limit
From: Stephan Müller @ 2016-12-27 22:40 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto
In-Reply-To: <3254875.f5A5oHPdxF@positron.chronox.de>

The variable limit was used to identify the nonblocking pool's unlimited
random number generation. As the nonblocking pool is a thing of the
past, remove the limit variable and any conditions around it (i.e.
preserve the branches for limit == 1).

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 drivers/char/random.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 8d4d720..d19108c 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -466,7 +466,6 @@ struct entropy_store {
 	int entropy_count;
 	int entropy_total;
 	unsigned int initialized:1;
-	unsigned int limit:1;
 	unsigned int last_data_init:1;
 	__u8 last_data[EXTRACT_SIZE];
 };
@@ -484,7 +483,6 @@ static __u32 blocking_pool_data[OUTPUT_POOL_WORDS] __latent_entropy;
 static struct entropy_store input_pool = {
 	.poolinfo = &poolinfo_table[0],
 	.name = "input",
-	.limit = 1,
 	.lock = __SPIN_LOCK_UNLOCKED(input_pool.lock),
 	.pool = input_pool_data
 };
@@ -492,7 +490,6 @@ static struct entropy_store input_pool = {
 static struct entropy_store blocking_pool = {
 	.poolinfo = &poolinfo_table[1],
 	.name = "blocking",
-	.limit = 1,
 	.pull = &input_pool,
 	.lock = __SPIN_LOCK_UNLOCKED(blocking_pool.lock),
 	.pool = blocking_pool_data,
@@ -1212,15 +1209,6 @@ static void xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
 	    r->entropy_count > r->poolinfo->poolfracbits)
 		return;
 
-	if (r->limit == 0 && random_min_urandom_seed) {
-		unsigned long now = jiffies;
-
-		if (time_before(now,
-				r->last_pulled + random_min_urandom_seed * HZ))
-			return;
-		r->last_pulled = now;
-	}
-
 	_xfer_secondary_pool(r, nbytes);
 }
 
@@ -1228,8 +1216,6 @@ static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
 {
 	__u32	tmp[OUTPUT_POOL_WORDS];
 
-	/* For /dev/random's pool, always leave two wakeups' worth */
-	int rsvd_bytes = r->limit ? 0 : random_read_wakeup_bits / 4;
 	int bytes = nbytes;
 
 	/* pull at least as much as a wakeup */
@@ -1240,7 +1226,7 @@ static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
 	trace_xfer_secondary_pool(r->name, bytes * 8, nbytes * 8,
 				  ENTROPY_BITS(r), ENTROPY_BITS(r->pull));
 	bytes = extract_entropy(r->pull, tmp, bytes,
-				random_read_wakeup_bits / 8, rsvd_bytes);
+				random_read_wakeup_bits / 8, 0);
 	mix_pool_bytes(r, tmp, bytes);
 	credit_entropy_bits(r, bytes*8);
 }
@@ -1268,7 +1254,7 @@ static void push_to_pool(struct work_struct *work)
 static size_t account(struct entropy_store *r, size_t nbytes, int min,
 		      int reserved)
 {
-	int entropy_count, orig;
+	int entropy_count, orig, have_bytes;
 	size_t ibytes, nfrac;
 
 	BUG_ON(r->entropy_count > r->poolinfo->poolfracbits);
@@ -1277,14 +1263,12 @@ static size_t account(struct entropy_store *r, size_t nbytes, int min,
 retry:
 	entropy_count = orig = ACCESS_ONCE(r->entropy_count);
 	ibytes = nbytes;
-	/* If limited, never pull more than available */
-	if (r->limit) {
-		int have_bytes = entropy_count >> (ENTROPY_SHIFT + 3);
+	/* never pull more than available */
+	have_bytes = entropy_count >> (ENTROPY_SHIFT + 3);
 
-		if ((have_bytes -= reserved) < 0)
-			have_bytes = 0;
-		ibytes = min_t(size_t, ibytes, have_bytes);
-	}
+	if ((have_bytes -= reserved) < 0)
+		have_bytes = 0;
+	ibytes = min_t(size_t, ibytes, have_bytes);
 	if (ibytes < min)
 		ibytes = 0;
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 7/8] random: remove noop function call to xfer_secondary_pool
From: Stephan Müller @ 2016-12-27 22:41 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto
In-Reply-To: <3254875.f5A5oHPdxF@positron.chronox.de>

Since the introduction of the ChaCha20 DRNG, extract_entropy is only
invoked with the input_pool. For this entropy pool, xfer_secondary_pool
is a no-op and can therefore be safely removed.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 drivers/char/random.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 89d67c0..7b72a01 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1417,7 +1417,6 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 	}
 
 	trace_extract_entropy(r->name, nbytes, ENTROPY_BITS(r), _RET_IP_);
-	xfer_secondary_pool(r, nbytes);
 	nbytes = account(r, nbytes, min, reserved);
 
 	return _extract_entropy(r, buf, nbytes, fips_enabled);
-- 
2.9.3

^ permalink raw reply related

* [PATCH 8/8] random: move FIPS continuous test to output functions
From: Stephan Müller @ 2016-12-27 22:42 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto
In-Reply-To: <3254875.f5A5oHPdxF@positron.chronox.de>

The current lockation of the FIPS continuous self test covers the
input_pool only. However, the FIPS continuous self test shall cover the
output of the random number generator, i.e. the blocking pool and the
ChaCha20 DRNG.

This patch therefore moves the continuous test to the output function
used for /dev/random. In addition, it adds the continuous test to the
ChaCha20 output function.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 drivers/char/random.c | 71 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 43 insertions(+), 28 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 7b72a01..d185d1f 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -416,6 +416,8 @@ struct crng_state {
 	__u32		state[16];
 	unsigned long	init_time;
 	spinlock_t	lock;
+	unsigned int last_data_init:1;
+	__u8 last_data[CHACHA20_BLOCK_SIZE];
 };
 
 struct crng_state primary_crng = {
@@ -471,7 +473,7 @@ struct entropy_store {
 static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 			       size_t nbytes, int min, int rsvd);
 static ssize_t _extract_entropy(struct entropy_store *r, void *buf,
-				size_t nbytes, int fips);
+				size_t nbytes);
 
 static void crng_reseed(struct crng_state *crng, struct entropy_store *r);
 static void push_to_pool(struct work_struct *work);
@@ -775,7 +777,7 @@ static void crng_initialize(struct crng_state *crng)
 	memcpy(&crng->state[0], "expand 32-byte k", 16);
 	if (crng == &primary_crng)
 		_extract_entropy(&input_pool, &crng->state[4],
-				 sizeof(__u32) * 12, 0);
+				 sizeof(__u32) * 12);
 	else
 		get_random_bytes(&crng->state[4], sizeof(__u32) * 12);
 	for (i = 4; i < 16; i++) {
@@ -864,11 +866,25 @@ static void _extract_crng(struct crng_state *crng,
 	    time_after(jiffies, crng->init_time + CRNG_RESEED_INTERVAL))
 		crng_reseed(crng, crng == &primary_crng ? &input_pool : NULL);
 	spin_lock_irqsave(&crng->lock, flags);
+
+	if (fips_enabled && !crng->last_data_init) {
+		crng->last_data_init = 1;
+		chacha20_block(&crng->state[0], out);
+		memcpy(crng->last_data, out, CHACHA20_BLOCK_SIZE);
+	}
+
 	if (arch_get_random_long(&v))
 		crng->state[14] ^= v;
 	chacha20_block(&crng->state[0], out);
 	if (crng->state[12] == 0)
 		crng->state[13]++;
+
+	if (fips_enabled) {
+		if (!memcmp(out, crng->last_data, CHACHA20_BLOCK_SIZE))
+			panic("ChaCha20 RNG duplicated output!\n");
+		memcpy(crng->last_data, out, CHACHA20_BLOCK_SIZE);
+	}
+
 	spin_unlock_irqrestore(&crng->lock, flags);
 }
 
@@ -1356,22 +1372,14 @@ static void extract_buf(struct entropy_store *r, __u8 *out)
 }
 
 static ssize_t _extract_entropy(struct entropy_store *r, void *buf,
-				size_t nbytes, int fips)
+				size_t nbytes)
 {
 	ssize_t ret = 0, i;
 	__u8 tmp[EXTRACT_SIZE];
-	unsigned long flags;
 
 	while (nbytes) {
 		extract_buf(r, tmp);
 
-		if (fips) {
-			spin_lock_irqsave(&r->lock, flags);
-			if (!memcmp(tmp, r->last_data, EXTRACT_SIZE))
-				panic("Hardware RNG duplicated output!\n");
-			memcpy(r->last_data, tmp, EXTRACT_SIZE);
-			spin_unlock_irqrestore(&r->lock, flags);
-		}
 		i = min_t(int, nbytes, EXTRACT_SIZE);
 		memcpy(buf, tmp, i);
 		nbytes -= i;
@@ -1397,7 +1405,22 @@ static ssize_t _extract_entropy(struct entropy_store *r, void *buf,
 static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 				 size_t nbytes, int min, int reserved)
 {
+	trace_extract_entropy(r->name, nbytes, ENTROPY_BITS(r), _RET_IP_);
+	nbytes = account(r, nbytes, min, reserved);
+
+	return _extract_entropy(r, buf, nbytes);
+}
+
+/*
+ * This function extracts randomness from the "entropy pool", and
+ * returns it in a userspace buffer.
+ */
+static ssize_t extract_entropy_user(struct entropy_store *r, void __user *buf,
+				    size_t nbytes)
+{
+	ssize_t ret = 0, i;
 	__u8 tmp[EXTRACT_SIZE];
+	int large_request = (nbytes > 256);
 	unsigned long flags;
 
 	/* if last_data isn't primed, we need EXTRACT_SIZE extra bytes */
@@ -1416,23 +1439,6 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 		spin_unlock_irqrestore(&r->lock, flags);
 	}
 
-	trace_extract_entropy(r->name, nbytes, ENTROPY_BITS(r), _RET_IP_);
-	nbytes = account(r, nbytes, min, reserved);
-
-	return _extract_entropy(r, buf, nbytes, fips_enabled);
-}
-
-/*
- * This function extracts randomness from the "entropy pool", and
- * returns it in a userspace buffer.
- */
-static ssize_t extract_entropy_user(struct entropy_store *r, void __user *buf,
-				    size_t nbytes)
-{
-	ssize_t ret = 0, i;
-	__u8 tmp[EXTRACT_SIZE];
-	int large_request = (nbytes > 256);
-
 	trace_extract_entropy_user(r->name, nbytes, ENTROPY_BITS(r), _RET_IP_);
 	xfer_secondary_pool(r, nbytes);
 	nbytes = account(r, nbytes, 0, 0);
@@ -1448,6 +1454,15 @@ static ssize_t extract_entropy_user(struct entropy_store *r, void __user *buf,
 		}
 
 		extract_buf(r, tmp);
+
+		if (fips_enabled) {
+			spin_lock_irqsave(&r->lock, flags);
+			if (!memcmp(tmp, r->last_data, EXTRACT_SIZE))
+				panic("Hardware RNG duplicated output!\n");
+			memcpy(r->last_data, tmp, EXTRACT_SIZE);
+			spin_unlock_irqrestore(&r->lock, flags);
+		}
+
 		i = min_t(int, nbytes, EXTRACT_SIZE);
 		if (copy_to_user(buf, tmp, i)) {
 			ret = -EFAULT;
-- 
2.9.3

^ permalink raw reply related

* [PATCH 6/8] random: fix comment for unused random_min_urandom_seed
From: Stephan Müller @ 2016-12-27 22:41 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto
In-Reply-To: <3254875.f5A5oHPdxF@positron.chronox.de>

The variable random_min_urandom_seed is not needed any more as it
defined the reseeding behavior of the nonblocking pool. Though it is not
needed any more, it is left in the code for user space interface
compatibility.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 drivers/char/random.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d19108c..89d67c0 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -313,9 +313,7 @@ static int random_read_wakeup_bits = 64;
 static int random_write_wakeup_bits = 28 * OUTPUT_POOL_WORDS;
 
 /*
- * The minimum number of seconds between urandom pool reseeding.  We
- * do this to limit the amount of entropy that can be drained from the
- * input pool even if there are heavy demands on /dev/urandom.
+ * Variable is currently unused by left for user space compatibility.
  */
 static int random_min_urandom_seed = 60;
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/8] random: remove stale maybe_reseed_primary_crng
From: Stephan Müller @ 2016-12-27 22:39 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-kernel, linux-crypto
In-Reply-To: <3254875.f5A5oHPdxF@positron.chronox.de>

>From 5e84a71d4c4b3c7f015878c0907102634603d270 Mon Sep 17 00:00:00 2001
From: Stephan Mueller <stephan.mueller@atsec.com>
Date: Thu, 15 Dec 2016 12:42:33 +0100
Subject: 

The function maybe_reseed_primary_crng is not used anywhere and thus can
be removed. Besides, it contains the check crng_init > 2 which will
never become true and thus would never trigger a reseed of the ChaCha20
primary DRNG.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 drivers/char/random.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 1ef2640..8e5ab20 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -855,13 +855,6 @@ static void crng_reseed(struct crng_state *crng, struct entropy_store *r)
 	spin_unlock_irqrestore(&primary_crng.lock, flags);
 }
 
-static inline void maybe_reseed_primary_crng(void)
-{
-	if (crng_init > 2 &&
-	    time_after(jiffies, primary_crng.init_time + CRNG_RESEED_INTERVAL))
-		crng_reseed(&primary_crng, &input_pool);
-}
-
 static inline void crng_wait_ready(void)
 {
 	wait_event_interruptible(crng_init_wait, crng_ready());
-- 
2.9.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox