Re: [PATCH v4 11/14] treewide: Prepare to remove VLA usage for AHASH_REQUEST_ON_STACK
On Fri, Jul 13, 2018 at 3:14 AM, Arnd Bergmann wrote: > On Fri, Jul 13, 2018 at 8:00 AM, Kees Cook wrote: >> On Thu, Jul 12, 2018 at 10:20 PM, Herbert Xu >> wrote: >>> On Thu, Jul 12, 2018 at 10:17:29PM -0700, Kees Cook wrote: What is the correct value to use for AHASH_REQUEST_ON_STACK? >>> >>> As I said to arrive at a fixed value you should examine all sync >>> ahash algorithms (e.g., all shash ones plus ahash ones marked as >>> sync if there are any). >> >> The "value" for the ahash I understand: it has a request size >> (tfm->reqsize, as set by crypto_ahash_set_reqsize()) what should be >> used to measure the shash value? (And how does this relate to the >> value returned by crypto_ahash_reqsize()?) The closest clue I can find >> is this: >> >> crypto_init_shash_ops_async() does: >> crt->reqsize = sizeof(struct shash_desc) + >> crypto_shash_descsize(shash); >> >> and that gets called from crypto_ahash_init_tfm(), so if it starts >> with the above reqsize and adds to it with a call to >> crypto_ahash_set_reqsize() later, we'll have that maximum? >> >> So, do I want to calculate this answer as: >> >> sizeof(struct shash_desc) + max(shash descsize) + max(ahash-sync reqsize) ? >> 16 + 360 + 0 > > I arrived at the same number, looking at all the sizes in shash, > The largest I found are sha3_state (360 bytes) and s390_sha_ctx > (336 bytes), everything else is way smaller. Excellent. Thanks for double-checking this. :) > >> It's 0 above because if I look at all the callers of >> crypto_ahash_set_reqsize() that do wrapping, all are ASYNC. >> >> So, should this really just be 376? Where is best to validate this >> size, as it seems checking in crypto_ahash_set_reqsize() is >> inappropriate? > > How about crypto_init_shash_ops_async()? Ah yes, that looks good. Nice find! After my ahash to shash conversions, only ccm is left as an ahash user, since it actually uses sg. But with the hard-coded value reduced to 376, this doesn't trip the frame warnings any more. :) I'll send an updated series soon. -Kees -- Kees Cook Pixel Security
[PATCH] random: mix rdrand with entropy sent in from userspace
Fedora has integrated the jitter entropy daemon to work around slow
boot problems, especially on VM's that don't support virtio-rng:
https://bugzilla.redhat.com/show_bug.cgi?id=1572944
It's understandable why they did this, but the Jitter entropy daemon
works fundamentally on the principle: "the CPU microarchitecture is
**so** complicated and we can't figure it out, so it *must* be
random". Yes, it uses statistical tests to "prove" it is secure, but
AES_ENCRYPT(NSA_KEY, COUNTER++) will also pass statistical tests with
flying colors.
So if RDRAND is available, mix it into entropy submitted from
userspace. It can't hurt, and if you believe the NSA has backdoored
RDRAND, then they probably have enough details about the Intel
microarchitecture that they can reverse engineer how the Jitter
entropy daemon affects the microarchitecture, and attack its output
stream. And if RDRAND is in fact an honest DRNG, it will immeasurably
improve on what the Jitter entropy daemon might produce.
This also provides some protection against someone who is able to read
or set the entropy seed file.
Signed-off-by: Theodore Ts'o
Cc: sta...@vger.kernel.org
---
drivers/char/random.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0706646b018d..283fe390e878 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1896,14 +1896,22 @@ static int
write_pool(struct entropy_store *r, const char __user *buffer, size_t count)
{
size_t bytes;
- __u32 buf[16];
+ __u32 t, buf[16];
const char __user *p = buffer;
while (count > 0) {
+ int b, i = 0;
+
bytes = min(count, sizeof(buf));
if (copy_from_user(&buf, p, bytes))
return -EFAULT;
+ for (b = bytes ; b > 0 ; b -= sizeof(__u32), i++) {
+ if (arch_get_random_int(&t))
+ continue;
+ buf[i] ^= t;
+ }
+
count -= bytes;
p += bytes;
--
2.18.0.rc0
Re: [PATCH v4 11/14] treewide: Prepare to remove VLA usage for AHASH_REQUEST_ON_STACK
On Sat, Jul 14, 2018 at 7:44 PM, Herbert Xu wrote: > On Fri, Jul 13, 2018 at 08:07:10PM -0700, Kees Cook wrote: >> >> On a plane today I started converting all these to shash. IIUC, it >> just looks like this (apologies for whitespace damage): > > Yes if it doesn't actually make use of SGs then shash would be > the way to go. However, for SG users ahash is the best interface. Nearly all of them artificially build an sg explicitly to use the ahash interface. :P So, I'll take that as a "yes, do these conversions." :) Thanks! -Kees -- Kees Cook Pixel Security
Re: [PATCH v4 11/14] treewide: Prepare to remove VLA usage for AHASH_REQUEST_ON_STACK
On Fri, Jul 13, 2018 at 08:07:10PM -0700, Kees Cook wrote: > > On a plane today I started converting all these to shash. IIUC, it > just looks like this (apologies for whitespace damage): Yes if it doesn't actually make use of SGs then shash would be the way to go. However, for SG users ahash is the best interface. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[PATCH] crypto: sharah: Unregister correct algorithms for SAHARA 3
This patch fixes two typos related to unregistering algorithms supported by SAHARAH 3. In sahara_register_algs the wrong algorithms are unregistered in case of an error. In sahara_unregister_algs the wrong array is used to determine the iteration count. Signed-off-by: Michael Müller --- Sorry for the duplicate email. I forgot the [PATCH] prefix, which is automatically filtered out by the git tools that I use for other open source projects. drivers/crypto/sahara.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c index 4379f016fbee..e7540a5b8197 100644 --- a/drivers/crypto/sahara.c +++ b/drivers/crypto/sahara.c @@ -1349,7 +1349,7 @@ static int sahara_register_algs(struct sahara_dev *dev) err_sha_v3_algs: for (j = 0; j < k; j++) - crypto_unregister_ahash(&sha_v4_algs[j]); + crypto_unregister_ahash(&sha_v3_algs[j]); err_aes_algs: for (j = 0; j < i; j++) @@ -1365,7 +1365,7 @@ static void sahara_unregister_algs(struct sahara_dev *dev) for (i = 0; i < ARRAY_SIZE(aes_algs); i++) crypto_unregister_alg(&aes_algs[i]); - for (i = 0; i < ARRAY_SIZE(sha_v4_algs); i++) + for (i = 0; i < ARRAY_SIZE(sha_v3_algs); i++) crypto_unregister_ahash(&sha_v3_algs[i]); if (dev->version > SAHARA_VERSION_3) -- 2.18.0
crypto: sharah: Unregister correct algorithms for SAHARA 3
This patch fixes two typos related to unregistering algorithms supported by SAHARAH 3. In sahara_register_algs the wrong algorithms are unregistered in case of an error. In sahara_unregister_algs the wrong array is used to determine the iteration count. Signed-off-by: Michael Müller --- drivers/crypto/sahara.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c index 4379f016fbee..e7540a5b8197 100644 --- a/drivers/crypto/sahara.c +++ b/drivers/crypto/sahara.c @@ -1349,7 +1349,7 @@ static int sahara_register_algs(struct sahara_dev *dev) err_sha_v3_algs: for (j = 0; j < k; j++) - crypto_unregister_ahash(&sha_v4_algs[j]); + crypto_unregister_ahash(&sha_v3_algs[j]); err_aes_algs: for (j = 0; j < i; j++) @@ -1365,7 +1365,7 @@ static void sahara_unregister_algs(struct sahara_dev *dev) for (i = 0; i < ARRAY_SIZE(aes_algs); i++) crypto_unregister_alg(&aes_algs[i]); - for (i = 0; i < ARRAY_SIZE(sha_v4_algs); i++) + for (i = 0; i < ARRAY_SIZE(sha_v3_algs); i++) crypto_unregister_ahash(&sha_v3_algs[i]); if (dev->version > SAHARA_VERSION_3) -- 2.18.0
Re: [PATCH v5 3/6] crypto: Add Qcom prng driver
On Mon, Jul 9, 2018 at 8:20 AM Vinod Koul wrote: > This ports the Qcom prng from older hw_random driver. > > No change of functionality and move from hw_random to crypto > APIs is done. > > Signed-off-by: Vinod Koul FWIW: Reviewed-by: Linus Walleij > +config CRYPTO_DEV_QCOM_RNG > + tristate "Qualcomm Randon Number Generator Driver" Random speling error. > +#include Do you need this? It seems like a platform device fair and square. Yours, Linus Walleij
