Re: [PATCH 3/4] crypto: drbg - replace spinlock with mutex
On Sun, Apr 19, 2015 at 05:37:21PM +0200, Stephan Mueller wrote:
I am not sure I understand you correctly: shall the DRBG have these
precautions? If so, wouldn't we break the requirements in SP800-90A where the
DRBG is intended to seed itself?
Or would you want to update the crypto_alloc_rng routine?
No. Our API doesn't provide the Instantiate_function obviously.
If you really want to have an explicit instantiate function then
you can provide a wrapper:
crypto_instantiate_drbg(...)
{
struct crypto_rng *drbg;
drbg = crypto_alloc_rng(...);
crypto_rng_reset(drbg, ...);
return drbg;
}
The fact that crypto_alloc_rng currently instantiates the RNG
is wrong because there is no provision for the personalisation
string.
Cheers,
--
Email: Herbert Xu herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] crypto: drbg - replace spinlock with mutex
On Mon, Apr 20, 2015 at 02:45:02AM +0200, Stephan Mueller wrote: I do not want to deviate from the kernel crypto API by adding some additional wrapper. But what we can do is to leave the DRBG unseeded during alloc time. As long as the DRBG is unseeded, it will return EAGAIN to any request for random numbers, forcing the caller to use crypto_rng_reset to activate the DRBG. When the DRBG receives a reset, it will always obtain the seed and treat any user-provided data as personalization string / additional data. That's exactly what I was suggesting. I already have two patches that I will post once I finish testing. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] crypto: drbg - replace spinlock with mutex
Am Montag, 20. April 2015, 08:48:55 schrieb Herbert Xu: Hi Herbert, On Mon, Apr 20, 2015 at 02:45:02AM +0200, Stephan Mueller wrote: I do not want to deviate from the kernel crypto API by adding some additional wrapper. But what we can do is to leave the DRBG unseeded during alloc time. As long as the DRBG is unseeded, it will return EAGAIN to any request for random numbers, forcing the caller to use crypto_rng_reset to activate the DRBG. When the DRBG receives a reset, it will always obtain the seed and treat any user-provided data as personalization string / additional data. That's exactly what I was suggesting. I already have two patches that I will post once I finish testing. Ok, I will wait then for your patches before I send out my patch set for the seeding revamp. Cheers, Ciao Stephan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] crypto: drbg - replace spinlock with mutex
Am Samstag, 18. April 2015, 18:55:23 schrieb Herbert Xu: Hi Herbert, Incidentally the whole reset concept seems redundant as you could always free and allocate a new RNG instead. So I'm planning on replacing it with a seed/reseed function instead. IOW it will keep the existing state and just add in new data if it's already seeded. Very good. When you do that, may I ask you to also update the crypto_alloc_rng. This function has one major drawback at the moment: we are wasting precious entropy. The testmgr must allocate the RNG for performing its testing using crypto_alloc_rng. As the RNG implementation has no knowledge at allocation time that it will be used for testing, it will seed itself for the real work. Then comes testing: the seeded RNG is now reset with the test entropy and thereby wasting the initial seed. IMHO such change in the allocation function must happen when you remove the reset function, because the reset function currently is the way to perform testing of the RNGs. -- Ciao Stephan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] crypto: drbg - replace spinlock with mutex
On Fri, Apr 17, 2015 at 02:55:28PM +0200, Stephan Mueller wrote:
@@ -1349,7 +1348,9 @@ static int drbg_generate_long(struct drbg_state *drbg,
unsigned int chunk = 0;
slice = ((buflen - len) / drbg_max_request_bytes(drbg));
chunk = slice ? drbg_max_request_bytes(drbg) : (buflen - len);
+ mutex_lock(drbg-drbg_mutex);
tmplen = drbg_generate(drbg, buf + len, chunk, addtl);
+ mutex_unlock(drbg-drbg_mutex);
if (0 = tmplen)
return tmplen;
BTW I just noticed that we always return 0 now so this is broken.
@@ -1377,10 +1378,12 @@ static int drbg_generate_long(struct drbg_state *drbg,
static int drbg_instantiate(struct drbg_state *drbg, struct drbg_string
*pers,
int coreref, bool pr)
{
- int ret = -ENOMEM;
+ int ret = -EOPNOTSUPP;
pr_devel(DRBG: Initializing DRBG core %d with prediction resistance
%s\n, coreref, pr ? enabled : disabled);
+ mutex_init(drbg-drbg_mutex);
This shouldn't be here because the reset function will call this
and break. This needs to be moved to cra_init.
Incidentally the whole reset concept seems redundant as you could
always free and allocate a new RNG instead. So I'm planning on
replacing it with a seed/reseed function instead. IOW it will
keep the existing state and just add in new data if it's already
seeded.
This will change our user-space API semantically but as you're the
only user I hope we won't need to add any backwards compatibility
cruft.
This also means that cra_init will no longer seed this and the RNG
won't be usable until seed is called.
Cheers,
-
Email: Herbert Xu herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] crypto: drbg - replace spinlock with mutex
On Sat, Apr 18, 2015 at 01:35:40PM +0200, Stephan Mueller wrote: When you do that, may I ask you to also update the crypto_alloc_rng. This function has one major drawback at the moment: we are wasting precious entropy. The testmgr must allocate the RNG for performing its testing using crypto_alloc_rng. As the RNG implementation has no knowledge at allocation time that it will be used for testing, it will seed itself for the real work. Then comes testing: the seeded RNG is now reset with the test entropy and thereby wasting the initial seed. Yes the DRBG will be unusable when first allocated and you must seed it to actually draw entropy and be able to use it. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] crypto: drbg - replace spinlock with mutex
The DRBG shall hold a long term lock. Therefore, the lock is changed to
a mutex which implies that the DRBG can only be used in process context.
The lock now guards the instantiation as well as the entire DRBG
generation operation. Therefore, multiple callers are fully serialized
when generating a random number.
Signed-off-by: Stephan Mueller smuel...@chronox.de
---
crypto/drbg.c | 22 ++
include/crypto/drbg.h | 4 ++--
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index c8a083c..19916ea 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1181,7 +1181,6 @@ static inline int drbg_alloc_state(struct drbg_state
*drbg)
if (!drbg-scratchpad)
goto err;
}
- spin_lock_init(drbg-drbg_lock);
return 0;
err:
@@ -1349,7 +1348,9 @@ static int drbg_generate_long(struct drbg_state *drbg,
unsigned int chunk = 0;
slice = ((buflen - len) / drbg_max_request_bytes(drbg));
chunk = slice ? drbg_max_request_bytes(drbg) : (buflen - len);
+ mutex_lock(drbg-drbg_mutex);
tmplen = drbg_generate(drbg, buf + len, chunk, addtl);
+ mutex_unlock(drbg-drbg_mutex);
if (0 = tmplen)
return tmplen;
len += tmplen;
@@ -1377,10 +1378,12 @@ static int drbg_generate_long(struct drbg_state *drbg,
static int drbg_instantiate(struct drbg_state *drbg, struct drbg_string *pers,
int coreref, bool pr)
{
- int ret = -ENOMEM;
+ int ret = -EOPNOTSUPP;
pr_devel(DRBG: Initializing DRBG core %d with prediction resistance
%s\n, coreref, pr ? enabled : disabled);
+ mutex_init(drbg-drbg_mutex);
+ mutex_lock(drbg-drbg_mutex);
drbg-core = drbg_cores[coreref];
drbg-pr = pr;
drbg-seeded = false;
@@ -1401,7 +1404,7 @@ static int drbg_instantiate(struct drbg_state *drbg,
struct drbg_string *pers,
break;
#endif /* CONFIG_CRYPTO_DRBG_CTR */
default:
- return -EOPNOTSUPP;
+ goto unlock;
}
/* 9.1 step 1 is implicit with the selected DRBG type */
@@ -1416,7 +1419,7 @@ static int drbg_instantiate(struct drbg_state *drbg,
struct drbg_string *pers,
ret = drbg_alloc_state(drbg);
if (ret)
- return ret;
+ goto unlock;
ret = -EFAULT;
if (drbg-d_ops-crypto_init(drbg))
@@ -1426,10 +1429,13 @@ static int drbg_instantiate(struct drbg_state *drbg,
struct drbg_string *pers,
if (ret)
goto err;
+ mutex_unlock(drbg-drbg_mutex);
return 0;
err:
drbg_dealloc_state(drbg);
+unlock:
+ mutex_unlock(drbg-drbg_mutex);
return ret;
}
@@ -1444,10 +1450,10 @@ err:
*/
static int drbg_uninstantiate(struct drbg_state *drbg)
{
- spin_lock_bh(drbg-drbg_lock);
+ mutex_lock(drbg-drbg_mutex);
drbg_dealloc_state(drbg);
/* no scrubbing of test_data -- this shall survive an uninstantiate */
- spin_unlock_bh(drbg-drbg_lock);
+ mutex_unlock(drbg-drbg_mutex);
return 0;
}
@@ -1462,9 +1468,9 @@ static inline void drbg_set_testdata(struct drbg_state
*drbg,
{
if (!test_data || !test_data-testentropy)
return;
- spin_lock_bh(drbg-drbg_lock);
+ mutex_lock(drbg-drbg_mutex);;
drbg-test_data = test_data;
- spin_unlock_bh(drbg-drbg_lock);
+ mutex_unlock(drbg-drbg_mutex);
}
/***
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 5186f75..a43a7ed 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -49,7 +49,7 @@
#include crypto/internal/rng.h
#include crypto/rng.h
#include linux/fips.h
-#include linux/spinlock.h
+#include linux/mutex.h
#include linux/list.h
/*
@@ -104,7 +104,7 @@ struct drbg_test_data {
};
struct drbg_state {
- spinlock_t drbg_lock; /* lock around DRBG */
+ struct mutex drbg_mutex;/* lock around DRBG */
unsigned char *V; /* internal state 10.1.1.1 1a) */
/* hash: static value 10.1.1.1 1b) hmac / ctr: key */
unsigned char *C;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
