From 663cace258c429c13d980aec5c8f40db50ba580b Mon Sep 17 00:00:00 2001 From: Edward Ned Harvey Date: Tue, 5 Aug 2014 11:02:40 -0400 Subject: use 32 bytes instead of 24 for seed material from ThreadedSeedGenerator --- crypto/src/security/SecureRandom.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'crypto/src') diff --git a/crypto/src/security/SecureRandom.cs b/crypto/src/security/SecureRandom.cs index ac9d98158..9fd7e9e65 100644 --- a/crypto/src/security/SecureRandom.cs +++ b/crypto/src/security/SecureRandom.cs @@ -26,8 +26,12 @@ namespace Org.BouncyCastle.Security gen = new ReversedWindowGenerator(gen, 32); SecureRandom sr = master[0] = new SecureRandom(gen); + // Even though Ticks has at most 8 or 14 bits of entropy, there's no harm in adding it. sr.SetSeed(DateTime.Now.Ticks); - sr.SetSeed(new ThreadedSeedGenerator().GenerateSeed(24, true)); + + // 32 will be enough when ThreadedSeedGenerator is fixed. Until then, ThreadedSeedGenerator returns low + // entropy, and this is not sufficient to be secure. http://www.bouncycastle.org/csharpdevmailarchive/msg00814.html + sr.SetSeed(new ThreadedSeedGenerator().GenerateSeed(32, true)); sr.GenerateSeed(1 + sr.Next(32)); } -- cgit 1.5.1 From e64eee6f58c70ad9c1dd02658461014599369319 Mon Sep 17 00:00:00 2001 From: Edward Ned Harvey Date: Tue, 5 Aug 2014 11:03:28 -0400 Subject: after seeding, pointlessly threw away the first few bytes. Removed. --- crypto/src/security/SecureRandom.cs | 1 - 1 file changed, 1 deletion(-) (limited to 'crypto/src') diff --git a/crypto/src/security/SecureRandom.cs b/crypto/src/security/SecureRandom.cs index 9fd7e9e65..6bc019481 100644 --- a/crypto/src/security/SecureRandom.cs +++ b/crypto/src/security/SecureRandom.cs @@ -32,7 +32,6 @@ namespace Org.BouncyCastle.Security // 32 will be enough when ThreadedSeedGenerator is fixed. Until then, ThreadedSeedGenerator returns low // entropy, and this is not sufficient to be secure. http://www.bouncycastle.org/csharpdevmailarchive/msg00814.html sr.SetSeed(new ThreadedSeedGenerator().GenerateSeed(32, true)); - sr.GenerateSeed(1 + sr.Next(32)); } return master[0]; -- cgit 1.5.1 From 8accb371a1855b54d5da6d05e6d2e26fe86e739d Mon Sep 17 00:00:00 2001 From: Edward Ned Harvey Date: Tue, 5 Aug 2014 11:05:30 -0400 Subject: use CryptoApiRandomGenerator in addition to other entropy sources --- crypto/src/security/SecureRandom.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'crypto/src') diff --git a/crypto/src/security/SecureRandom.cs b/crypto/src/security/SecureRandom.cs index 6bc019481..ed0193e8d 100644 --- a/crypto/src/security/SecureRandom.cs +++ b/crypto/src/security/SecureRandom.cs @@ -28,7 +28,12 @@ namespace Org.BouncyCastle.Security // Even though Ticks has at most 8 or 14 bits of entropy, there's no harm in adding it. sr.SetSeed(DateTime.Now.Ticks); - + // In addition to Ticks and ThreadedSeedGenerator, also seed from CryptoApiRandomGenerator + CryptoApiRandomGenerator systemRNG = new CryptoApiRandomGenerator(); + byte[] systemSeed = new byte[32]; + systemRNG.NextBytes(systemSeed); + sr.SetSeed(systemSeed); + Array.Clear(systemSeed,0,systemSeed.Length); // 32 will be enough when ThreadedSeedGenerator is fixed. Until then, ThreadedSeedGenerator returns low // entropy, and this is not sufficient to be secure. http://www.bouncycastle.org/csharpdevmailarchive/msg00814.html sr.SetSeed(new ThreadedSeedGenerator().GenerateSeed(32, true)); -- cgit 1.5.1 From 66cfe2970d6cc02f10fce030fbc0b6f8cb8ece5a Mon Sep 17 00:00:00 2001 From: Edward Ned Harvey Date: Tue, 5 Aug 2014 11:07:33 -0400 Subject: SecureRandom ctor: given this is a sha1Generator, seed with 20 bytes instead of 8 --- crypto/src/security/SecureRandom.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crypto/src') diff --git a/crypto/src/security/SecureRandom.cs b/crypto/src/security/SecureRandom.cs index ed0193e8d..055162f1f 100644 --- a/crypto/src/security/SecureRandom.cs +++ b/crypto/src/security/SecureRandom.cs @@ -80,7 +80,7 @@ namespace Org.BouncyCastle.Security public SecureRandom() : this(sha1Generator) { - SetSeed(GetSeed(8)); + SetSeed(GetSeed(20)); // 20 bytes because this is sha1Generator } public SecureRandom( -- cgit 1.5.1 From a0d33a340a7445f466aedcaafdbf7f2f7e738d9a Mon Sep 17 00:00:00 2001 From: Edward Ned Harvey Date: Tue, 5 Aug 2014 11:31:07 -0400 Subject: GetInstance() returns a seeded instance. If you want an unseeded instance, you must use the unseeded ctor in which you supply your own generator --- crypto/src/security/SecureRandom.cs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'crypto/src') diff --git a/crypto/src/security/SecureRandom.cs b/crypto/src/security/SecureRandom.cs index 055162f1f..ef932ee8c 100644 --- a/crypto/src/security/SecureRandom.cs +++ b/crypto/src/security/SecureRandom.cs @@ -46,24 +46,20 @@ namespace Org.BouncyCastle.Security public static SecureRandom GetInstance( string algorithm) { - // TODO Compared to JDK, we don't auto-seed if the client forgets - problem? - // TODO Support all digests more generally, by stripping PRNG and calling DigestUtilities? string drgName = Platform.ToUpperInvariant(algorithm); - IRandomGenerator drg = null; if (drgName == "SHA1PRNG") { - drg = sha1Generator; + SecureRandom newPrng = new SecureRandom(sha1Generator); + newPrng.SetSeed(GetSeed(20)); + return newPrng; } else if (drgName == "SHA256PRNG") { - drg = sha256Generator; - } - - if (drg != null) - { - return new SecureRandom(drg); + SecureRandom newPrng = new SecureRandom(sha256Generator); + newPrng.SetSeed(GetSeed(32)); + return newPrng; } throw new ArgumentException("Unrecognised PRNG algorithm: " + algorithm, "algorithm"); -- cgit 1.5.1 From 026e0efbf6a1aa38c868c442b6813ecfac51cfd3 Mon Sep 17 00:00:00 2001 From: Edward Ned Harvey Date: Tue, 5 Aug 2014 11:32:29 -0400 Subject: In SecureRandom, the usage of ReversedWindowGenerator only reordered the output of sha256Generator. It added computation overhead and zero cryptographic value. Removed. --- crypto/src/security/SecureRandom.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'crypto/src') diff --git a/crypto/src/security/SecureRandom.cs b/crypto/src/security/SecureRandom.cs index ef932ee8c..c91b5ee91 100644 --- a/crypto/src/security/SecureRandom.cs +++ b/crypto/src/security/SecureRandom.cs @@ -22,9 +22,7 @@ namespace Org.BouncyCastle.Security { if (master[0] == null) { - IRandomGenerator gen = sha256Generator; - gen = new ReversedWindowGenerator(gen, 32); - SecureRandom sr = master[0] = new SecureRandom(gen); + SecureRandom sr = master[0] = new SecureRandom(sha256Generator); // Even though Ticks has at most 8 or 14 bits of entropy, there's no harm in adding it. sr.SetSeed(DateTime.Now.Ticks); -- cgit 1.5.1