Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [PATCH] hwrng: core - Allow for multiple simultaneous active hwrng devices
From: Jason Cooper @ 2016-08-09 16:57 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Keith Packard, linux-crypto, linux-kernel
In-Reply-To: <20160809095058.GA6618@gondor.apana.org.au>

Hi Keith, Herbert,

On Tue, Aug 09, 2016 at 05:50:58PM +0800, Herbert Xu wrote:
> On Mon, Jul 25, 2016 at 01:07:35PM -0700, Keith Packard wrote:
> > Instead of having only one hwrng feeding /dev/random at a time, maintain
> > a list of devices and cycle between them when filling the entropy pool.
> > 
> > Signed-off-by: Keith Packard <keithp@keithp.com>
> 
> So you're cycling RNGs even for user-space reads? That could be
> problematic because not all hardware RNGs carry the maximum amount
> of entropy.  It would be rather annoying to be cycling between
> RNGs of different qualities.
> 
> Perhaps only cycle for the kernel hwrngd?

Perhaps a /dev/hwrng[0-9] per rng?  That would lend itself nicely to a
sysfs interface for per device quality, rate, and enabled attributes.
e.g. /sys/class/hw_random/hwrng0/{device/,quality,rate,enabled}

/dev/hwrng could pull from the one with the highest quality, or user
specified for backwards compatibility.

thx,

Jason.

^ permalink raw reply

* Re: FIPS mode: modprobe: ERROR: could not insert 'drbg'
From: Stephan Mueller @ 2016-08-09 17:05 UTC (permalink / raw)
  To: Tapas Sarangi; +Cc: linux-crypto@vger.kernel.org
In-Reply-To: <D3CF7065.28F3%tsarangi@trustwave.com>

Am Dienstag, 9. August 2016, 16:34:59 CEST schrieb Tapas Sarangi:

Hi Tapas,

> Hi Stephan,
> 
> Following up from the other thread:
> 
> While trying to boot in FIPS mode, kernel panics with the following
> message. So far, I don¹t have success to get more information about which
> module or symbol is causing this. I haven¹t found any errors or warnings
> in kernel compilation. It boots fine in a non-fips mode.
> 
> I am also pasting the CRYPTO related configs that I have enabled (See
> below).

I do not see the issue immediately. Could you boot without fips=1 and do a 
modprobe drbg ?

I am also testing fips=1 now.

Ciao
Stephan

^ permalink raw reply

* Re: FIPS mode: modprobe: ERROR: could not insert 'drbg'
From: Tapas Sarangi @ 2016-08-09 17:11 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto@vger.kernel.org
In-Reply-To: <1699099.iu8Xr4xQ6X@tauon.atsec.com>

Hi Stephan,

Thanks. I have already tried that. ‘drbg’ module is loaded fine in a
non-fips mode. Here are output from some commands.

I see that at some point you had a patch to use CONFIG_CRYPTO_LRNG. I am
not using that, could that be a problem ?

-Tapas


[root@localhost ~]# modprobe drbg
[root@localhost ~]# echo $?
0

[root@localhost ~]# dmesg | tail -5
[    3.636174] nf_conntrack version 0.5.0 (7168 buckets, 28672 max)
[    3.738645] NET: Registered protocol family 10
[    3.743004] ip6_tables: (C) 2000-2006 Netfilter Core Team
[    3.773384] input: ImExPS/2 BYD TouchPad as
/devices/platform/i8042/serio1/input/input3
[    3.776803] mousedev: PS/2 mouse device common for all mice

[root@localhost ~]# lsmod | grep drbg
drbg                   14147  1

[root@localhost ~]# modinfo drbg
filename:       /lib/modules/4.7.0-1.tos2_5/kernel/crypto/drbg.ko.gz
alias:          crypto-stdrng
alias:          stdrng
description:    NIST SP800-90A Deterministic Random Bit Generator (DRBG)
using following cores: HMAC
author:         Stephan Mueller <smueller@chronox.de>
license:        GPL
alias:          crypto-drbg_nopr_hmac_sha1
alias:          drbg_nopr_hmac_sha1
alias:          crypto-drbg_pr_hmac_sha1
alias:          drbg_pr_hmac_sha1
alias:          crypto-drbg_nopr_hmac_sha256
alias:          drbg_nopr_hmac_sha256
alias:          crypto-drbg_pr_hmac_sha256
alias:          drbg_pr_hmac_sha256
alias:          crypto-drbg_nopr_hmac_sha384
alias:          drbg_nopr_hmac_sha384
alias:          crypto-drbg_pr_hmac_sha384
alias:          drbg_pr_hmac_sha384
alias:          crypto-drbg_nopr_hmac_sha512
alias:          drbg_nopr_hmac_sha512
alias:          crypto-drbg_pr_hmac_sha512
alias:          drbg_pr_hmac_sha512
depends:
intree:         Y
vermagic:       4.7.0-1.tos2_5 SMP mod_unload modversions







On 8/9/16, 12:05 PM, "Stephan Mueller" <smueller@chronox.de> wrote:

>Am Dienstag, 9. August 2016, 16:34:59 CEST schrieb Tapas Sarangi:
>
>Hi Tapas,
>
>> Hi Stephan,
>>
>> Following up from the other thread:
>>
>> While trying to boot in FIPS mode, kernel panics with the following
>> message. So far, I don¹t have success to get more information about
>>which
>> module or symbol is causing this. I haven¹t found any errors or warnings
>> in kernel compilation. It boots fine in a non-fips mode.
>>
>> I am also pasting the CRYPTO related configs that I have enabled (See
>> below).
>
>I do not see the issue immediately. Could you boot without fips=1 and do
>a
>modprobe drbg ?
>
>I am also testing fips=1 now.
>
>Ciao
>Stephan


________________________________

This transmission may contain information that is privileged, confidential, and/or exempt from disclosure under applicable law. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein (including any reliance thereon) is strictly prohibited. If you received this transmission in error, please immediately contact the sender and destroy the material in its entirety, whether in electronic or hard copy format.

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Jason Cooper @ 2016-08-09 17:48 UTC (permalink / raw)
  To: miaoqing-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: kvalo-A+ZNKFmMK5xy9aJCnZT0Uw,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	ath9k-devel-A+ZNKFmMK5xy9aJCnZT0Uw,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	smueller-T9tCv8IpfcWELgA04lAiVw, pouyans-Rm6X0d1/PG5y9aJCnZT0Uw
In-Reply-To: <1470726147-30095-2-git-send-email-miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On Tue, Aug 09, 2016 at 03:02:27PM +0800, miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org wrote:
> From: Miaoqing Pan <miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> 
> ath9k RNG will dominates all the noise sources from the real HW
> RNG, disable it by default. But we strongly recommand to enable
> it if the system without HW RNG, especially on embedded systems.
> 
> Signed-off-by: Miaoqing Pan <miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Until we get the hw_random/get_device_randomness question sorted...

Reviewed-by: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: FIPS mode: modprobe: ERROR: could not insert 'drbg'
From: Stephan Mueller @ 2016-08-09 17:52 UTC (permalink / raw)
  To: Tapas Sarangi, herbert; +Cc: linux-crypto@vger.kernel.org
In-Reply-To: <D3CF7805.2937%tsarangi@trustwave.com>

Am Dienstag, 9. August 2016, 17:11:09 CEST schrieb Tapas Sarangi:

Hi Tapas, Herbert,

> Hi Stephan,
> 
> Thanks. I have already tried that. ‘drbg’ module is loaded fine in a
> non-fips mode. Here are output from some commands.

There is something strange going on. I have to compile the DRBG statically. 
When booting the kernel with fips=1 (of course after changing the key size to 
2k or 3k in certs/x509.genkey), the DRBG does not show up in /proc/crypto nor 
can I find testmgr entries about the DRBG.

When I reboot the kernel without fips=1, all works as expected.

When I create a copy of the drbg.c code and have it compiled as a module to 
ensure it is signed, I can insmod it and the testmgr successfully tests it.

Note, with fips=1, my kernel crashes randomly somewhere in the elf loading 
code -- I guess it is because there is no stdrng.

> 
> I see that at some point you had a patch to use CONFIG_CRYPTO_LRNG. I am
> not using that, could that be a problem ?

Nope, this LRNG is something completely different -- it is my proposal to 
replace the current /dev/random and /dev/urandom implementation as documented 
in [1].

[1] http://www.chronox.de/lrng.html

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Jason Cooper @ 2016-08-09 17:51 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Stephan Mueller, Herbert Xu, Pan, Miaoqing, Matt Mackall,
	miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, Sepehrdad, Pouyan
In-Reply-To: <20160809102458.GA20214@khazad-dum.debian.net>

Hi Henrique,

On Tue, Aug 09, 2016 at 07:24:58AM -0300, Henrique de Moraes Holschuh wrote:
> On Tue, 09 Aug 2016, Stephan Mueller wrote:
> > RHEL 7 and Fedora do not adjust it. So, shall we consider those rng-tools then 
> > broken (at least in those large distros)?
> 
> Might I humbly suggest that the kernel start providing some metatada
> about the quality of the random source that userspace can consume?
> Preferably by a new ioctl, so that it will fit naturally if we ever
> extend /dev/hwrng to support more than one source?
> 
> That would allow for auto tunning to be implemented in userspace...

See my reply to Keith Packard's proposed multi-device hw_random patch:

  https://lkml.kernel.org/r/20160809165710.GC2013@io.lakedaemon.net

and top of thread:

  https://lkml.kernel.org/r/1469477255-26824-1-git-send-email-keithp@keithp.com

thx,

Jason.

^ permalink raw reply

* Re: [PATCH] hwrng: core - Allow for multiple simultaneous active hwrng devices
From: Keith Packard @ 2016-08-09 17:58 UTC (permalink / raw)
  To: Jason Cooper, Herbert Xu; +Cc: linux-crypto, linux-kernel
In-Reply-To: <20160809165710.GC2013@io.lakedaemon.net>

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]

Jason Cooper <jason@lakedaemon.net> writes:

> Perhaps a /dev/hwrng[0-9] per rng?  That would lend itself nicely to a
> sysfs interface for per device quality, rate, and enabled attributes.
> e.g. /sys/class/hw_random/hwrng0/{device/,quality,rate,enabled}

I was interested in the data being provided for /dev/random; that seems
like the most important interface to me.  But, exposing all of the
devices using consistent names does seem like a useful idea at some
level.

> /dev/hwrng could pull from the one with the highest quality, or user
> specified for backwards compatibility.

I like the notion of using all of them in turn; if one of them turns out
to be broken, you're still stirring in data from the others. After all,
the quality metric is provided by the device, we aren't doing any
analysis on the data to determine it independently.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 810 bytes --]

^ permalink raw reply

* Re: [PATCH] hwrng: core - Allow for multiple simultaneous active hwrng devices
From: Jason Cooper @ 2016-08-09 18:26 UTC (permalink / raw)
  To: Keith Packard; +Cc: Herbert Xu, linux-crypto, linux-kernel
In-Reply-To: <86y445rfiq.fsf@hiro.keithp.com>

Hi Keith,

On Tue, Aug 09, 2016 at 10:58:05AM -0700, Keith Packard wrote:
> Jason Cooper <jason@lakedaemon.net> writes:
> > Perhaps a /dev/hwrng[0-9] per rng?  That would lend itself nicely to a
> > sysfs interface for per device quality, rate, and enabled attributes.
> > e.g. /sys/class/hw_random/hwrng0/{device/,quality,rate,enabled}
> 
> I was interested in the data being provided for /dev/random; that seems
> like the most important interface to me.

Me too, agreed.

> But, exposing all of the devices using consistent names does seem like
> a useful idea at some level.

On another thread, regarding the ath9k-rng (actually just the adc
registers), Henrique asked about per-source knobs.  My suggestion
follows from that.

> > /dev/hwrng could pull from the one with the highest quality, or user
> > specified for backwards compatibility.
> 
> I like the notion of using all of them in turn; if one of them turns out
> to be broken, you're still stirring in data from the others. After all,
> the quality metric is provided by the device, we aren't doing any
> analysis on the data to determine it independently.

Sure, but /dev/hwrng is a user interface.  Typically to rngd, but not
necessarily.  We need to make sure it's behavior is consistent with
existing expectations.

We shouldn't attach first-probed to /dev/hwrng, because that may not be
what the user is expecting.  If I bought a raw entropy source, and knew
nothing of the proposed multi-source interfaces, I'd expect the USB
dongle to be attached to /dev/hwrng.  Despite the fact that my pcie wifi
card was probed first and has adc registers providing an entropy source.

I'm not sure how we ensure that.  Perhaps an 'environmental' flag in the
hw_random source attributes?  Or a 'not-designed-to-be-an-rng' flag? :)
Maybe those would be /dev/envrng[0-9]...

thx,

Jason.

^ permalink raw reply

* Re: [PATCH] hwrng: core - Allow for multiple simultaneous active hwrng devices
From: Keith Packard @ 2016-08-09 19:01 UTC (permalink / raw)
  To: Jason Cooper; +Cc: Herbert Xu, linux-crypto, linux-kernel
In-Reply-To: <20160809182620.GF2013@io.lakedaemon.net>

[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]

Jason Cooper <jason@lakedaemon.net> writes:

> On another thread, regarding the ath9k-rng (actually just the adc
> registers), Henrique asked about per-source knobs.  My suggestion
> follows from that.

I'd do that with the source-specific driver instead of attempting to
route controls through hwrng. Anything else seems like 'ioctl' to me.

> Sure, but /dev/hwrng is a user interface.  Typically to rngd, but not
> necessarily.  We need to make sure it's behavior is consistent with
> existing expectations.

Hrm. Maybe /dev/hwrng should use a different policy than how we feed
/dev/random -- we could use the existing behaviour for /dev/hwrng, but
use a round-robin for /dev/random. That way, the latest device would
always end up in /dev/hwrng (unless configured otherwise), and we'd
still use all of the available sources to help stir the kernel entropy
pool.

> We shouldn't attach first-probed to /dev/hwrng, because that may not be
> what the user is expecting.  If I bought a raw entropy source, and knew
> nothing of the proposed multi-source interfaces, I'd expect the USB
> dongle to be attached to /dev/hwrng.  Despite the fact that my pcie wifi
> card was probed first and has adc registers providing an entropy source.

That seems like a fragile interface as it depends on discovery order,
but it is what we have currently.

The chaoskey driver also exposes it's own device; that provides a simple
way to ensure that the application is getting bits from the desired
entropy source.

> I'm not sure how we ensure that.  Perhaps an 'environmental' flag in the
> hw_random source attributes?  Or a 'not-designed-to-be-an-rng' flag? :)
> Maybe those would be /dev/envrng[0-9]...

Or some set of query ioctls on /dev/hwrng[0-9]+ that would provide
information about the capabilities of the underlying device.

There are lots of things we could do, I guess the question I have is how
much of this would applications actually use effectively? You're
probably right that /dev/hwrng should point at a single source and not
change though; otherwise figuring out what the quality of the bits
you're getting isn't possible.x

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 810 bytes --]

^ permalink raw reply

* [PATCH] crypto: DRBG: do not call drbg_instantiate in healt test
From: Stephan Mueller @ 2016-08-09 19:02 UTC (permalink / raw)
  To: Tapas Sarangi; +Cc: herbert, linux-crypto@vger.kernel.org
In-Reply-To: <2671088.6l5BeQPtPO@positron.chronox.de>

Am Dienstag, 9. August 2016, 19:52:46 CEST schrieb Stephan Mueller:

Hi Tapas,

I think I found the issue. Can you please test the attached patch?

---8<---

When calling the DRBG health test in FIPS mode, the Jitter RNG is not
yet present in the kernel crypto API which will cause the instantiation
to fail and thus the health test to fail.

As the health tests cover the enforcement of various thresholds, invoke
the functions that are supposed to enforce the thresholds directly.

This patch also saves precious seed.

Reported-by: Tapas Sarangi <TSarangi@trustwave.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index f752da3..edf3ce0 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1917,6 +1917,8 @@ static inline int __init drbg_healthcheck_sanity(void)
 		return -ENOMEM;
 
 	mutex_init(&drbg->drbg_mutex);
+	drbg->core = &drbg_cores[coreref];
+	drbg->reseed_threshold = drbg_max_requests(drbg);
 
 	/*
 	 * if the following tests fail, it is likely that there is a buffer
@@ -1926,12 +1928,6 @@ static inline int __init drbg_healthcheck_sanity(void)
 	 * grave bug.
 	 */
 
-	/* get a valid instance of DRBG for following tests */
-	ret = drbg_instantiate(drbg, NULL, coreref, pr);
-	if (ret) {
-		rc = ret;
-		goto outbuf;
-	}
 	max_addtllen = drbg_max_addtl(drbg);
 	max_request_bytes = drbg_max_request_bytes(drbg);
 	drbg_string_fill(&addtl, buf, max_addtllen + 1);
@@ -1941,10 +1937,9 @@ static inline int __init drbg_healthcheck_sanity(void)
 	/* overflow max_bits */
 	len = drbg_generate(drbg, buf, (max_request_bytes + 1), NULL);
 	BUG_ON(0 < len);
-	drbg_uninstantiate(drbg);
 
 	/* overflow max addtllen with personalization string */
-	ret = drbg_instantiate(drbg, &addtl, coreref, pr);
+	ret = drbg_seed(drbg, &addtl, false);
 	BUG_ON(0 == ret);
 	/* all tests passed */
 	rc = 0;
@@ -1952,9 +1947,7 @@ static inline int __init drbg_healthcheck_sanity(void)
 	pr_devel("DRBG: Sanity tests for failure code paths successfully "
 		 "completed\n");
 
-	drbg_uninstantiate(drbg);
-outbuf:
-	kzfree(drbg);
+	kfree(drbg);
 	return rc;
 }
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] hwrng: core - Allow for multiple simultaneous active hwrng devices
From: Henrique de Moraes Holschuh @ 2016-08-09 20:18 UTC (permalink / raw)
  To: Jason Cooper; +Cc: Herbert Xu, Keith Packard, linux-crypto, linux-kernel
In-Reply-To: <20160809165710.GC2013@io.lakedaemon.net>

On Tue, 09 Aug 2016, Jason Cooper wrote:
> Perhaps a /dev/hwrng[0-9] per rng?  That would lend itself nicely to a
> sysfs interface for per device quality, rate, and enabled attributes.
> e.g. /sys/class/hw_random/hwrng0/{device/,quality,rate,enabled}

IMHO, this is mightly annoying to use from inside a rngd-like utility in
a race-free, safe way.  It looks to me that ioctl() would be a much
better interface for everything but the "enabled" functionality (which
should be reported to the rngd-like utility as open() on the real device
failing with, e.g., ENXIO, when that source is disabled).

-- 
  Henrique Holschuh

^ permalink raw reply

* Re: [PATCH] crypto: DRBG: do not call drbg_instantiate in healt test
From: Tapas Sarangi @ 2016-08-09 21:59 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org
In-Reply-To: <6079382.fWeFLzBGfE@positron.chronox.de>

Hi Stephan,

Thanks. The patch that I applied have different line numbers than yours.
In any case, patch worked to get rid of Œdrbg¹ related error.

Now, fips mode is failing on self-test:

/boot/vmlinuz-4.7.0-1.tos2_5: OK
[    1.296714] alg: skcipher: setkey failed on test 1 for xts(aes-asm):
flags=100000
[    1.297665] Kernel panic - not syncing: xts(aes): xts(aes) alg self
test failed in fips mode!
[    1.297665]




Thanks
-Tapas



On 8/9/16, 2:02 PM, "Stephan Mueller" <smueller@chronox.de> wrote:

>Am Dienstag, 9. August 2016, 19:52:46 CEST schrieb Stephan Mueller:
>
>Hi Tapas,
>
>I think I found the issue. Can you please test the attached patch?
>
>---8<---
>
>When calling the DRBG health test in FIPS mode, the Jitter RNG is not
>yet present in the kernel crypto API which will cause the instantiation
>to fail and thus the health test to fail.
>
>As the health tests cover the enforcement of various thresholds, invoke
>the functions that are supposed to enforce the thresholds directly.
>
>This patch also saves precious seed.
>
>Reported-by: Tapas Sarangi <TSarangi@trustwave.com>
>Signed-off-by: Stephan Mueller <smueller@chronox.de>
>---
> crypto/drbg.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
>diff --git a/crypto/drbg.c b/crypto/drbg.c
>index f752da3..edf3ce0 100644
>--- a/crypto/drbg.c
>+++ b/crypto/drbg.c
>@@ -1917,6 +1917,8 @@ static inline int __init
>drbg_healthcheck_sanity(void)
>               return -ENOMEM;
>
>       mutex_init(&drbg->drbg_mutex);
>+      drbg->core = &drbg_cores[coreref];
>+      drbg->reseed_threshold = drbg_max_requests(drbg);
>
>       /*
>        * if the following tests fail, it is likely that there is a buffer
>@@ -1926,12 +1928,6 @@ static inline int __init
>drbg_healthcheck_sanity(void)
>        * grave bug.
>        */
>
>-      /* get a valid instance of DRBG for following tests */
>-      ret = drbg_instantiate(drbg, NULL, coreref, pr);
>-      if (ret) {
>-              rc = ret;
>-              goto outbuf;
>-      }
>       max_addtllen = drbg_max_addtl(drbg);
>       max_request_bytes = drbg_max_request_bytes(drbg);
>       drbg_string_fill(&addtl, buf, max_addtllen + 1);
>@@ -1941,10 +1937,9 @@ static inline int __init
>drbg_healthcheck_sanity(void)
>       /* overflow max_bits */
>       len = drbg_generate(drbg, buf, (max_request_bytes + 1), NULL);
>       BUG_ON(0 < len);
>-      drbg_uninstantiate(drbg);
>
>       /* overflow max addtllen with personalization string */
>-      ret = drbg_instantiate(drbg, &addtl, coreref, pr);
>+      ret = drbg_seed(drbg, &addtl, false);
>       BUG_ON(0 == ret);
>       /* all tests passed */
>       rc = 0;
>@@ -1952,9 +1947,7 @@ static inline int __init
>drbg_healthcheck_sanity(void)
>       pr_devel("DRBG: Sanity tests for failure code paths successfully "
>                "completed\n");
>
>-      drbg_uninstantiate(drbg);
>-outbuf:
>-      kzfree(drbg);
>+      kfree(drbg);
>       return rc;
> }
>
>--
>2.7.4
>
>


________________________________

This transmission may contain information that is privileged, confidential, and/or exempt from disclosure under applicable law. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein (including any reliance thereon) is strictly prohibited. If you received this transmission in error, please immediately contact the sender and destroy the material in its entirety, whether in electronic or hard copy format.

^ permalink raw reply

* Re: [PATCH v3] DH support: add KDF handling support
From: Mat Martineau @ 2016-08-09 22:38 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: David Howells, keyrings, linux-crypto
In-Reply-To: <3715151.LOyK4NLlGi@positron.chronox.de>


On Sat, 6 Aug 2016, Stephan Mueller wrote:

> diff --git a/man/keyctl_dh_compute.3 b/man/keyctl_dh_compute.3
> index b06d39e..92d358f 100644
> --- a/man/keyctl_dh_compute.3
> +++ b/man/keyctl_dh_compute.3
> @@ -11,6 +11,8 @@
> .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> .SH NAME
> keyctl_dh_compute \- Compute a Diffie-Hellman shared secret or public key
> +.br
> +keyctl_dh_compute_kdf \- Derive key from a Diffie-Hellman shared secret
> .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> .SH SYNOPSIS
> .nf
> @@ -21,6 +23,10 @@ keyctl_dh_compute \- Compute a Diffie-Hellman shared secret or public key
> .sp
> .BI "long keyctl_dh_compute_alloc(key_serial_t " private,
> .BI "key_serial_t " prime ", key_serial_t " base ", void **" _buffer ");"
> +.sp
> +.BI "long keyctl_dh_compute_kdf(key_serial_t " private ", key_serial_t " prime ,
> +.BI "key_serial_t " base ", char *" kdfname ", char *" otherinfo ",
> +.BI "size_t " otherinfolen ", char *" buffer ", size_t " buflen ");"
> .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> .SH DESCRIPTION
> .BR keyctl_dh_compute ()
> @@ -64,6 +70,49 @@ places the data in it.  If successful, a pointer to the buffer is placed in
> .IR *_buffer .
> The caller must free the buffer.
> .P
> +.BR keyctl_dh_compute_kdf ()
> +derives a key from a Diffie-Hellman shared secret according to the protocol
> +specified in SP800-56A. The Diffie-Hellman computation is based on the same
> +primitives as discussed
> +for
> +.BR keyctl_dy_compute ().

Minor typo: dy->dh


Regards,

--
Mat Martineau
Intel OTC

^ permalink raw reply

* Re: [PATCH v3] KEYS: add SP800-56A KDF support for DH
From: Mat Martineau @ 2016-08-09 22:48 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: David Howells, keyrings, linux-crypto
In-Reply-To: <2623005.lF82gu89UT@positron.chronox.de>


On Sat, 6 Aug 2016, Stephan Mueller wrote:

> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index a705a7d..7659b52 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -259,15 +259,32 @@ static inline long keyctl_get_persistent(uid_t uid, key_serial_t destring)
> #endif
>
> #ifdef CONFIG_KEY_DH_OPERATIONS
> +#include <crypto/rng.h>
> +#include <linux/compat.h>

These may belong at the top of the file, even if they are only used when 
CONFIG_KEY_DH_OPERATIONS is defined.

> extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
> -			      size_t, void __user *);
> +			      size_t, struct keyctl_kdf_params __user *);
> +extern long __keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
> +				size_t, struct keyctl_kdf_params *);
> +extern long compat_keyctl_dh_compute(struct keyctl_dh_params __user *params,
> +				char __user *buffer, size_t buflen,
> +				struct compat_keyctl_kdf_params __user *kdf);
> +#define KEYCTL_KDF_MAX_OUTPUT_LEN	1024	/* max length of KDF output */
> +#define KEYCTL_KDF_MAX_OI_LEN		64	/* max length of otherinfo */
> #else
> static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params,
> 				     char __user *buffer, size_t buflen,
> -				     void __user *reserved)
> +				     struct keyctl_kdf_params __user *kdf)
> {
> 	return -EOPNOTSUPP;
> }
> +
> +static inline long compat_keyctl_dh_compute(
> +				struct keyctl_dh_params __user *params,
> +				char __user *buffer, size_t buflen,
> +				struct keyctl_kdf_params __user *kdf)
> +{
> +	return -EOPNOTSUPP
> +}
> #endif
>
> /*
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index d580ad0..b106898 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1689,7 +1689,7 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
> 	case KEYCTL_DH_COMPUTE:
> 		return keyctl_dh_compute((struct keyctl_dh_params __user *) arg2,
> 					 (char __user *) arg3, (size_t) arg4,
> -					 (void __user *) arg5);
> +					 (struct keyctl_kdf_params __user *) arg5);
>
> 	default:
> 		return -EOPNOTSUPP;

Regards,
--
Mat Martineau
Intel OTC

^ permalink raw reply

* Re: [PATCH] hwrng: core - Allow for multiple simultaneous active hwrng devices
From: Keith Packard @ 2016-08-09 23:26 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh, Jason Cooper
  Cc: Herbert Xu, linux-crypto, linux-kernel
In-Reply-To: <20160809201846.GA4684@khazad-dum.debian.net>

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

Henrique de Moraes Holschuh <hmh@hmh.eng.br> writes:

> IMHO, this is mightly annoying to use from inside a rngd-like utility in
> a race-free, safe way.  It looks to me that ioctl() would be a much
> better interface for everything but the "enabled" functionality (which
> should be reported to the rngd-like utility as open() on the real device
> failing with, e.g., ENXIO, when that source is disabled).

What information does an rngd-like program actually want? All I can
think that it would need is the stream of random data. I guess some
estimate of the entropy available would be nice, but surely it would
want to verify that in any case.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 810 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Pan, Miaoqing @ 2016-08-10  2:35 UTC (permalink / raw)
  To: Stephan Mueller, Herbert Xu
  Cc: Matt Mackall, miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <1645997.7cVzaEi3NG-gNvIQDDl/k7Ia13z/PHSgg@public.gmane.org>

Hi Stephan,

For those less perfect noise source, can't pass the FIPS test.

static int update_kernel_random(int random_step,
        unsigned char *buf, fips_ctx_t *fipsctx_in)
{
        unsigned char *p; 
        int fips;

        fips = fips_run_rng_test(fipsctx_in, buf);
        if (fips)
                return 1;

        for (p = buf; p + random_step <= &buf[FIPS_RNG_BUFFER_SIZE];
                 p += random_step) {
                random_add_entropy(p, random_step);
                random_sleep();
        }
        return 0;
}

--
Miaoqing
________________________________________
From: Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
Sent: Tuesday, August 9, 2016 5:37 PM
To: Herbert Xu
Cc: Pan, Miaoqing; Matt Mackall; miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; Valo, Kalle; linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ath9k-devel; linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org; Sepehrdad, Pouyan
Subject: Re: [PATCH 2/2] ath9k: disable RNG by default

Am Dienstag, 9. August 2016, 17:17:55 CEST schrieb Herbert Xu:

Hi Herbert,

> On Tue, Aug 09, 2016 at 11:02:58AM +0200, Stephan Mueller wrote:
> > But shouldn't the default of the rngd then be adjusted a bit?
>
> Please elaborate.

in rngd_linux.c:random_add_entropy(void *buf, size_t size):

        entropy.ent_count = size * 8;
        entropy.size = size;
        memcpy(entropy.data, buf, size);

        if (ioctl(random_fd, RNDADDENTROPY, &entropy) != 0) {

...


in rngd.c:do_loop():

                        retval = iter->xread(buf, sizeof buf, iter);
...
                        rc = update_kernel_random(random_step,
                                             buf, iter->fipsctx);

where update_kernel_random simply invokes random_add_entropy in chunks.

Hence, the rngd reads some bytes from /dev/hwrand and injects it into /dev/
random with an entropy estimate that is equal to the read bytes.

With less than perfect noise sources, entropy.ent_count should be much
smaller.
>
> Thanks,



Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Stephan Mueller @ 2016-08-10  5:29 UTC (permalink / raw)
  To: Pan, Miaoqing
  Cc: Herbert Xu, Matt Mackall,
	miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <1470796501856.53342-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>

Am Mittwoch, 10. August 2016, 02:35:04 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> For those less perfect noise source, can't pass the FIPS test.
> 
> static int update_kernel_random(int random_step,
>         unsigned char *buf, fips_ctx_t *fipsctx_in)
> {
>         unsigned char *p;
>         int fips;
> 
>         fips = fips_run_rng_test(fipsctx_in, buf);
>         if (fips)
>                 return 1;
> 
>         for (p = buf; p + random_step <= &buf[FIPS_RNG_BUFFER_SIZE];
>                  p += random_step) {
>                 random_add_entropy(p, random_step);
>                 random_sleep();
>         }
>         return 0;
> }

Not even the poor cheap AIS20 statistical tests from rngd pass?

I guess the only sensible solution is what Ted suggested to use 
add_device_randomness.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH 2/2] ath9k: disable RNG by default
From: Pan, Miaoqing @ 2016-08-10  6:04 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Herbert Xu, Matt Mackall,
	miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <1543667.vXsZDTRgbm-jJGQKZiSfeo1haGO/jJMPxvVK+yQ3ZXh@public.gmane.org>

Hi Stephan,

FIPS RNG test is supposed to be run on the output of an RNG, and not on the RNG entropy source. It is not surprising that the RNG input fails the entropy tests from NIST. Check the following example.

Imagine you have a perfectly random sequence, a_1, a_2, .., a_n, where each a_i is a byte. And imagine, this sequence passes all randomness tests.

Now, let's say I create a new sequence a_1, 0, a_2, 0, a_3, 0, ..., 0, a_n, where each zero is a byte

If you give this sequence (as an entropy source) to a randomness test, it will fail most of the tests, if not all. This does not mean this sequence is not appropriate as an entropy source, it just means we need twice more bytes to gain the same amount of entropy.

I can give this 2n byte sequence to an RNG as an entropy source and it provides the same amount of security as if I give the n byte stream.

Thanks,
Miaoqing

-----Original Message-----
From: Stephan Mueller [mailto:smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org] 
Sent: Wednesday, August 10, 2016 1:29 PM
To: Pan, Miaoqing <miaoqing-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Cc: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>; Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>; miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; Valo, Kalle <kvalo-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ath9k-devel <ath9k-devel-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org; Sepehrdad, Pouyan <pouyans-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Subject: Re: [PATCH 2/2] ath9k: disable RNG by default

Am Mittwoch, 10. August 2016, 02:35:04 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> For those less perfect noise source, can't pass the FIPS test.
> 
> static int update_kernel_random(int random_step,
>         unsigned char *buf, fips_ctx_t *fipsctx_in) {
>         unsigned char *p;
>         int fips;
> 
>         fips = fips_run_rng_test(fipsctx_in, buf);
>         if (fips)
>                 return 1;
> 
>         for (p = buf; p + random_step <= &buf[FIPS_RNG_BUFFER_SIZE];
>                  p += random_step) {
>                 random_add_entropy(p, random_step);
>                 random_sleep();
>         }
>         return 0;
> }

Not even the poor cheap AIS20 statistical tests from rngd pass?

I guess the only sensible solution is what Ted suggested to use add_device_randomness.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH 2/2] ath9k: disable RNG by default
From: Pan, Miaoqing @ 2016-08-10  6:46 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Herbert Xu, Matt Mackall,
	miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <14565196.xaXq375WQg-gNvIQDDl/k7Ia13z/PHSgg@public.gmane.org>

Hi Stephan,

Would you please provide a recent NIST document which asks the entropy source to pass the NIST randomness tests ?

Thanks,
Miaoqing

-----Original Message-----
From: Stephan Mueller [mailto:smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org] 
Sent: Wednesday, August 10, 2016 2:25 PM
To: Pan, Miaoqing <miaoqing-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Cc: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>; Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>; miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; Valo, Kalle <kvalo-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ath9k-devel <ath9k-devel-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org; Sepehrdad, Pouyan <pouyans-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Subject: Re: [PATCH 2/2] ath9k: disable RNG by default

Am Mittwoch, 10. August 2016, 06:04:32 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> FIPS RNG test is supposed to be run on the output of an RNG, and not 
> on the RNG entropy source. It is not surprising that the RNG input 
> fails the entropy tests from NIST. Check the following example.
> 
> Imagine you have a perfectly random sequence, a_1, a_2, .., a_n, where 
> each a_i is a byte. And imagine, this sequence passes all randomness tests.
> 
> Now, let's say I create a new sequence a_1, 0, a_2, 0, a_3, 0, ..., 0, 
> a_n, where each zero is a byte
> 
> If you give this sequence (as an entropy source) to a randomness test, 
> it will fail most of the tests, if not all. This does not mean this 
> sequence is not appropriate as an entropy source, it just means we 
> need twice more bytes to gain the same amount of entropy.

Agreed. But that is a very simplistic view.
> 
> I can give this 2n byte sequence to an RNG as an entropy source and it 
> provides the same amount of security as if I give the n byte stream.

Well, I am working with standards bodies like NIST and BSI on RNG assessments. They all require that the noise source (pre-whitening, of
course) pass statistical tests like the AIS20 tests, SP800-22 and similar. If you fail, you better have a good argument.

And the only argument that is kind of allowed is that you oversample your noise source to seed a DRNG from (i.e. have an entropy to data ratio of significantly below 1). And the argument for the oversampling rate is always a very interesting discussion. You apply 10/32. In private, I am wondering about that ratio, but this should not be discussed here as I hope you have a valid argument for that.

As we are talking about the current rngd, we have to consider that it does
*not* perform an oversampling (yet) as mentioned in the previous emails.

Do not get me wrong on my initial patch: your RNG may provide some entropy. 
But there are quite some folks who want to understand and audit a noise source before using it. Your current implementation simply does not allow switching the noise source off to feed the input_pool with data that increases the entropy estimator (at runtime).

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Stephan Mueller @ 2016-08-10  7:27 UTC (permalink / raw)
  To: Pan, Miaoqing
  Cc: Herbert Xu, Matt Mackall,
	miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <c0951e085764481d8d85637734e2e14b-fhY3XlRGNI1pWAYlkNb9jaRtKmQZhJ7pQQ4Iyu8u01E@public.gmane.org>

Am Mittwoch, 10. August 2016, 07:15:49 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> NIST SP 800-22-rev1a and NIST SP 800-90B are used together to evaluate the
> amount of min entropy the source provides, and not to decide if the source
> has passed the tests or failed. See
> 
> https://github.com/usnistgov/SP800-90B_EntropyAssessment
> 
> The goal is often to make sure the input entropy is more than the entropy we
> expect from the output.

You are correct on the SP800-90B tests (hence I did not refer to them for the 
binary decision). Yet, SP800-22 with the associated tool delivers a binary 
decision.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH 2/2] ath9k: disable RNG by default
From: Pan, Miaoqing @ 2016-08-10  7:40 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Herbert Xu, Matt Mackall,
	miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <4321952.1nMxxDi7Wz-jJGQKZiSfeo1haGO/jJMPxvVK+yQ3ZXh@public.gmane.org>

Hi Stephan,

That is set as "optional but highly recommended" in the FIPS doc, plus the fact that we do not have a requirement to have a FIP-approved RNG in our case. Although FIPS might impose higher and stronger requirements on the source of entropy, but not passing those tests does not mean the source of entropy is of bad quality. As I mentioned earlier, we just need to evaluate the amount of entropy it provides correctly and use it accordingly. If we are dealing with a chip which has a HW RNG, we expect extremely high entropy close to full from our source, but this patch is for chips which do not have a dedicated HW RNG in place to improve the quality of random number generation on the platform.

Thanks,
Miaoqing

-----Original Message-----
From: Stephan Mueller [mailto:smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org] 
Sent: Wednesday, August 10, 2016 3:27 PM
To: Pan, Miaoqing <miaoqing-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Cc: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>; Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>; miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; Valo, Kalle <kvalo-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ath9k-devel <ath9k-devel-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org; Sepehrdad, Pouyan <pouyans-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Subject: Re: [PATCH 2/2] ath9k: disable RNG by default

Am Mittwoch, 10. August 2016, 07:15:49 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> NIST SP 800-22-rev1a and NIST SP 800-90B are used together to evaluate 
> the amount of min entropy the source provides, and not to decide if 
> the source has passed the tests or failed. See
> 
> https://github.com/usnistgov/SP800-90B_EntropyAssessment
> 
> The goal is often to make sure the input entropy is more than the 
> entropy we expect from the output.

You are correct on the SP800-90B tests (hence I did not refer to them for the binary decision). Yet, SP800-22 with the associated tool delivers a binary decision.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH 2/2] ath9k: disable RNG by default
From: Pan, Miaoqing @ 2016-08-10  7:43 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Herbert Xu, Matt Mackall,
	miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <1e8e88ad7de64c528f08c75ff9176ab8-fhY3XlRGNI1pWAYlkNb9jaRtKmQZhJ7pQQ4Iyu8u01E@public.gmane.org>

Hi Stephan,

The problem with using the add_device_randomness is that we do not know when to call that API, and we have to make our solution either timer-based or interrupt based, which is not really the correct way of implementing this feature.

Thanks,
Miaoqing

-----Original Message-----
From: Pan, Miaoqing 
Sent: Wednesday, August 10, 2016 3:41 PM
To: Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
Cc: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>; Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>; miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; Valo, Kalle <kvalo-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ath9k-devel <ath9k-devel-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org; Sepehrdad, Pouyan <pouyans-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Subject: RE: [PATCH 2/2] ath9k: disable RNG by default

Hi Stephan,

That is set as "optional but highly recommended" in the FIPS doc, plus the fact that we do not have a requirement to have a FIP-approved RNG in our case. Although FIPS might impose higher and stronger requirements on the source of entropy, but not passing those tests does not mean the source of entropy is of bad quality. As I mentioned earlier, we just need to evaluate the amount of entropy it provides correctly and use it accordingly. If we are dealing with a chip which has a HW RNG, we expect extremely high entropy close to full from our source, but this patch is for chips which do not have a dedicated HW RNG in place to improve the quality of random number generation on the platform.

Thanks,
Miaoqing

-----Original Message-----
From: Stephan Mueller [mailto:smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org]
Sent: Wednesday, August 10, 2016 3:27 PM
To: Pan, Miaoqing <miaoqing-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Cc: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>; Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>; miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; Valo, Kalle <kvalo-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ath9k-devel <ath9k-devel-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>; linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org; Sepehrdad, Pouyan <pouyans-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org>
Subject: Re: [PATCH 2/2] ath9k: disable RNG by default

Am Mittwoch, 10. August 2016, 07:15:49 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> NIST SP 800-22-rev1a and NIST SP 800-90B are used together to evaluate 
> the amount of min entropy the source provides, and not to decide if 
> the source has passed the tests or failed. See
> 
> https://github.com/usnistgov/SP800-90B_EntropyAssessment
> 
> The goal is often to make sure the input entropy is more than the 
> entropy we expect from the output.

You are correct on the SP800-90B tests (hence I did not refer to them for the binary decision). Yet, SP800-22 with the associated tool delivers a binary decision.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* How best to {en,de}crypt between sk_buff and iov_iter?
From: David Howells @ 2016-08-10  9:12 UTC (permalink / raw)
  To: netdev, linux-crypto; +Cc: dhowells, viro

Is there a good way to encrypt data held in an iov_iter directly into an
sk_buff and decrypt data held in an sk_buff back into an iov_iter?

What I would like to avoid is:

 (a) Invoking skb_cow_data() to potentially take an unnecessary copy of the
     data I shouldn't need to change, but I need to do this to decrypt in
     place.

 (b) Having to copy the unencrypted data between the sk_buff and the iov_iter
     when the crypto process ought to get me a free copy.

One problem, though, is that I might not be able to do drain/fill a complete
sk_buff in a single operation because the iov_iter might not give me sufficient
bufferage/data to do that, so it may take multiple operations.  However, since
I'm using an skcipher, I think it should be fine to call
crypto_skcipher_encrypt() multiple times on the same skcipher.

I can see a couple of alternatives:

 (1) Duplicate skb_copy_datagram_iter(), give it an initialised
     skcipher_request and let it set the crypto parameters for each block it
     transfers.  copy_to_iter() would then need to be replaced with something
     that sets up an sglist each time from the iov.

     Something similar would need doing for skb_copy_datagram_from_iter().

 (2) Create an sglist for the skb and one for the iov_iter and encrypt/decrypt
     between them.  Unfortunately, if the iov_iter is a userspace reference
     then this would mean pinning userspace pages.

 (3) Add an {en,de}crypt-to-iov_iter capability to the crypto layer.  I'm not
     sure how well this would work for hardware support, though.  I think we'd
     come back to pinning userspace pages.

David

^ permalink raw reply

* [PATCH] crypto: fix a little typo
From: LABBE Corentin @ 2016-08-10  9:29 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto, linux-kernel, LABBE Corentin

The sentence 'Based on' is misspelled, respell it.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 crypto/xts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/xts.c b/crypto/xts.c
index 26ba583..305343f 100644
--- a/crypto/xts.c
+++ b/crypto/xts.c
@@ -5,7 +5,7 @@
  *
  * Copyright (c) 2007 Rik Snel <rsnel@cube.dyndns.org>
  *
- * Based om ecb.c
+ * Based on ecb.c
  * Copyright (c) 2006 Herbert Xu <herbert@gondor.apana.org.au>
  *
  * This program is free software; you can redistribute it and/or modify it
-- 
2.7.3

^ permalink raw reply related

* [PATCH 2/6] crypto: sun4i-ss: unify update/final function
From: LABBE Corentin @ 2016-08-10  9:45 UTC (permalink / raw)
  To: herbert, davem, maxime.ripard, wens
  Cc: linux-crypto, linux-kernel, LABBE Corentin
In-Reply-To: <1470822334-20672-1-git-send-email-clabbe.montjoie@gmail.com>

The update and final functions have lots of common action.
This patch mix them in one function.
This will give some improvements:
- This will permit asynchronous support more easily
- This will permit to use finup/digest functions with some performance
  improvements

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/crypto/sunxi-ss/sun4i-ss-hash.c | 147 ++++++++++++++++++--------------
 drivers/crypto/sunxi-ss/sun4i-ss.h      |   1 +
 2 files changed, 85 insertions(+), 63 deletions(-)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
index ff80314..2fb0684 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
@@ -129,6 +129,9 @@ int sun4i_hash_import_sha1(struct ahash_request *areq, const void *in)
 	return 0;
 }
 
+#define SS_HASH_UPDATE 1
+#define SS_HASH_FINAL 2
+
 /*
  * sun4i_hash_update: update hash engine
  *
@@ -156,7 +159,7 @@ int sun4i_hash_import_sha1(struct ahash_request *areq, const void *in)
  * write remaining data in op->buf
  * final state op->len=56
  */
-int sun4i_hash_update(struct ahash_request *areq)
+int sun4i_hash(struct ahash_request *areq)
 {
 	u32 v, ivmode = 0;
 	unsigned int i = 0;
@@ -180,22 +183,30 @@ int sun4i_hash_update(struct ahash_request *areq)
 	u32 spaces, rx_cnt = SS_RX_DEFAULT;
 	size_t copied = 0;
 	struct sg_mapping_iter mi;
+	unsigned int j = 0;
+	int zeros;
+	unsigned int index, padlen;
+	__be64 bits;
+	u32 bf[32];
+	u32 wb = 0;
+	unsigned int nwait, nbw = 0;
+	struct scatterlist *in_sg = areq->src;
 
 	dev_dbg(ss->dev, "%s %s bc=%llu len=%u mode=%x wl=%u h0=%0x",
 		__func__, crypto_tfm_alg_name(areq->base.tfm),
 		op->byte_count, areq->nbytes, op->mode,
 		op->len, op->hash[0]);
 
-	if (areq->nbytes == 0)
+	if (unlikely(areq->nbytes == 0) && (op->flags & SS_HASH_FINAL) == 0)
 		return 0;
 
 	/* protect against overflow */
-	if (areq->nbytes > UINT_MAX - op->len) {
+	if (unlikely(areq->nbytes > UINT_MAX - op->len)) {
 		dev_err(ss->dev, "Cannot process too large request\n");
 		return -EINVAL;
 	}
 
-	if (op->len + areq->nbytes < 64) {
+	if (op->len + areq->nbytes < 64 && (op->flags & SS_HASH_FINAL) == 0) {
 		/* linearize data to op->buf */
 		copied = sg_pcopy_to_buffer(areq->src, sg_nents(areq->src),
 					    op->buf + op->len, areq->nbytes, 0);
@@ -203,14 +214,6 @@ int sun4i_hash_update(struct ahash_request *areq)
 		return 0;
 	}
 
-	end = ((areq->nbytes + op->len) / 64) * 64 - op->len;
-
-	if (end > areq->nbytes || areq->nbytes - end > 63) {
-		dev_err(ss->dev, "ERROR: Bound error %u %u\n",
-			end, areq->nbytes);
-		return -EINVAL;
-	}
-
 	spin_lock_bh(&ss->slock);
 
 	/*
@@ -225,6 +228,33 @@ int sun4i_hash_update(struct ahash_request *areq)
 	/* Enable the device */
 	writel(op->mode | SS_ENABLED | ivmode, ss->base + SS_CTL);
 
+	if ((op->flags & SS_HASH_UPDATE) == 0)
+		goto hash_final;
+
+	/* start of handling data */
+	if ((op->flags & SS_HASH_FINAL) == 0) {
+		end = ((areq->nbytes + op->len) / 64) * 64 - op->len;
+
+		if (end > areq->nbytes || areq->nbytes - end > 63) {
+			dev_err(ss->dev, "ERROR: Bound error %u %u\n",
+				end, areq->nbytes);
+			return -EINVAL;
+		}
+	} else {
+		/* Since we have the flag final, we can go up to modulo 4 */
+		end = ((areq->nbytes + op->len) / 4) * 4 - op->len;
+	}
+
+	/* TODO if SGlen % 4 and op->len == 0 then DMA */
+	i = 1;
+	while (in_sg && i == 1) {
+		if ((in_sg->length % 4) != 0)
+			i = 0;
+		in_sg = sg_next(in_sg);
+	}
+	if (i == 1 && op->len == 0)
+		dev_dbg(ss->dev, "We can DMA\n");
+
 	i = 0;
 	sg_miter_start(&mi, areq->src, sg_nents(areq->src),
 		       SG_MITER_FROM_SG | SG_MITER_ATOMIC);
@@ -285,7 +315,11 @@ int sun4i_hash_update(struct ahash_request *areq)
 			}
 		}
 	} while (i < end);
-	/* final linear */
+
+	/*
+	 * Now we have written to the device all that we can,
+	 * store the remaining bytes in op->buf
+	 */
 	if ((areq->nbytes - i) < 64) {
 		while (i < areq->nbytes && in_i < mi.length && op->len < 64) {
 			/* how many bytes we can read from current SG */
@@ -304,13 +338,21 @@ int sun4i_hash_update(struct ahash_request *areq)
 
 	sg_miter_stop(&mi);
 
+	/*
+	 * End of data process
+	 * Now if we have the flag final go to finalize part
+	 * If not, store the partial hash
+	 */
+	if ((op->flags & SS_HASH_FINAL) > 0)
+		goto hash_final;
+
 	writel(op->mode | SS_ENABLED | SS_DATA_END, ss->base + SS_CTL);
 	i = 0;
 	do {
 		v = readl(ss->base + SS_CTL);
 		i++;
 	} while (i < SS_TIMEOUT && (v & SS_DATA_END) > 0);
-	if (i >= SS_TIMEOUT) {
+	if (unlikely(i >= SS_TIMEOUT)) {
 		dev_err_ratelimited(ss->dev,
 				    "ERROR: hash end timeout %d>%d ctl=%x len=%u\n",
 				    i, SS_TIMEOUT, v, areq->nbytes);
@@ -318,56 +360,24 @@ int sun4i_hash_update(struct ahash_request *areq)
 		goto release_ss;
 	}
 
-	/* get the partial hash only if something was written */
 	for (i = 0; i < crypto_ahash_digestsize(tfm) / 4; i++)
 		op->hash[i] = readl(ss->base + SS_MD0 + i * 4);
 
-release_ss:
-	writel(0, ss->base + SS_CTL);
-	spin_unlock_bh(&ss->slock);
-	return err;
-}
+	goto release_ss;
 
 /*
- * sun4i_hash_final: finalize hashing operation
+ * hash_final: finalize hashing operation
  *
  * If we have some remaining bytes, we write them.
  * Then ask the SS for finalizing the hashing operation
  *
  * I do not check RX FIFO size in this function since the size is 32
  * after each enabling and this function neither write more than 32 words.
+ * If we come from the update part, we cannot have more than
+ * 3 remainings bytes to write and SS is fast enought to not care about it.
  */
-int sun4i_hash_final(struct ahash_request *areq)
-{
-	u32 v, ivmode = 0;
-	unsigned int i;
-	unsigned int j = 0;
-	int zeros, err = 0;
-	unsigned int index, padlen;
-	__be64 bits;
-	struct sun4i_req_ctx *op = ahash_request_ctx(areq);
-	struct sun4i_ss_ctx *ss = op->ss;
-	struct crypto_ahash *tfm = crypto_ahash_reqtfm(areq);
-	u32 bf[32];
-	u32 wb = 0;
-	unsigned int nwait, nbw = 0;
 
-	dev_dbg(ss->dev, "%s: byte=%llu len=%u mode=%x wl=%u h=%x",
-		__func__, op->byte_count, areq->nbytes, op->mode,
-		op->len, op->hash[0]);
-
-	spin_lock_bh(&ss->slock);
-
-	/*
-	 * if we have already written something,
-	 * restore the partial hash state
-	 */
-	if (op->byte_count > 0) {
-		ivmode = SS_IV_ARBITRARY;
-		for (i = 0; i < crypto_ahash_digestsize(tfm) / 4; i++)
-			writel(op->hash[i], ss->base + SS_IV0 + i * 4);
-	}
-	writel(op->mode | SS_ENABLED | ivmode, ss->base + SS_CTL);
+hash_final:
 
 	/* write the remaining words of the wait buffer */
 	if (op->len > 0) {
@@ -436,7 +446,7 @@ int sun4i_hash_final(struct ahash_request *areq)
 		v = readl(ss->base + SS_CTL);
 		i++;
 	} while (i < SS_TIMEOUT && (v & SS_DATA_END) > 0);
-	if (i >= SS_TIMEOUT) {
+	if (unlikely(i >= SS_TIMEOUT)) {
 		dev_err_ratelimited(ss->dev,
 				    "ERROR: hash end timeout %d>%d ctl=%x len=%u\n",
 				    i, SS_TIMEOUT, v, areq->nbytes);
@@ -463,30 +473,41 @@ release_ss:
 	return err;
 }
 
+int sun4i_hash_final(struct ahash_request *areq)
+{
+	struct sun4i_req_ctx *op = ahash_request_ctx(areq);
+
+	op->flags = SS_HASH_FINAL;
+	return sun4i_hash(areq);
+}
+
+int sun4i_hash_update(struct ahash_request *areq)
+{
+	struct sun4i_req_ctx *op = ahash_request_ctx(areq);
+
+	op->flags = SS_HASH_UPDATE;
+	return sun4i_hash(areq);
+}
+
 /* sun4i_hash_finup: finalize hashing operation after an update */
 int sun4i_hash_finup(struct ahash_request *areq)
 {
-	int err;
-
-	err = sun4i_hash_update(areq);
-	if (err != 0)
-		return err;
+	struct sun4i_req_ctx *op = ahash_request_ctx(areq);
 
-	return sun4i_hash_final(areq);
+	op->flags = SS_HASH_UPDATE | SS_HASH_FINAL;
+	return sun4i_hash(areq);
 }
 
 /* combo of init/update/final functions */
 int sun4i_hash_digest(struct ahash_request *areq)
 {
 	int err;
+	struct sun4i_req_ctx *op = ahash_request_ctx(areq);
 
 	err = sun4i_hash_init(areq);
 	if (err != 0)
 		return err;
 
-	err = sun4i_hash_update(areq);
-	if (err != 0)
-		return err;
-
-	return sun4i_hash_final(areq);
+	op->flags = SS_HASH_UPDATE | SS_HASH_FINAL;
+	return sun4i_hash(areq);
 }
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss.h b/drivers/crypto/sunxi-ss/sun4i-ss.h
index 8e9c05f..ece5a1c 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss.h
+++ b/drivers/crypto/sunxi-ss/sun4i-ss.h
@@ -164,6 +164,7 @@ struct sun4i_req_ctx {
 	char buf[64];
 	unsigned int len;
 	struct sun4i_ss_ctx *ss;
+	int flags;
 };
 
 int sun4i_hash_crainit(struct crypto_tfm *tfm);
-- 
2.7.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