Fix range and bias of NextDouble

- see https://github.com/bcgit/bc-csharp/issues/253

3 files changed, 61 insertions, 13 deletions

diff --git a/crypto/Contributors.html b/crypto/Contributors.html
index 220b2f5a6..35cd92af7 100644
--- a/crypto/Contributors.html
+++ b/crypto/Contributors.html
@@ -147,7 +147,10 @@
                 <p>Nicolas Dorier (https://github.com/NicolasDorier) - patch to fix culture-dependent lookups in MacUtilities.</p>
             </li>
             <li>
-		<p>Artem Storozhuk &lt;storojs72&#064gmail.com&gt; initial implementation of DSTU7564 (digest) and DSTU7624 (cipher) and their associated modes.</p>
+                <p>Artem Storozhuk &lt;storojs72&#064gmail.com&gt; - initial implementation of DSTU7564 (digest) and DSTU7624 (cipher) and their associated modes.</p>
+            </li>
+            <li>
+                <p>bkalakrishnan (https://github.com/bkalakrishnan) - reported issue with SecureRandom.NextDouble and advised how to fix.</p>
             </li>
 		</ul>
 	</body>
diff --git a/crypto/src/security/SecureRandom.cs b/crypto/src/security/SecureRandom.cs
index bd639a336..8c6b74d5b 100644
--- a/crypto/src/security/SecureRandom.cs
+++ b/crypto/src/security/SecureRandom.cs
@@ -4,6 +4,7 @@ using System.Threading;
 using Org.BouncyCastle.Crypto;
 using Org.BouncyCastle.Crypto.Digests;
 using Org.BouncyCastle.Crypto.Prng;
+using Org.BouncyCastle.Crypto.Utilities;
 using Org.BouncyCastle.Utilities;
 
 namespace Org.BouncyCastle.Security
@@ -232,31 +233,27 @@ namespace Org.BouncyCastle.Security
             generator.NextBytes(buf, off, len);
         }
 
-        private static readonly double DoubleScale = System.Math.Pow(2.0, 64.0);
+        private static readonly double DoubleScale = 1.0 / Convert.ToDouble(1L << 53);
 
         public override double NextDouble()
         {
-            return Convert.ToDouble((ulong) NextLong()) / DoubleScale;
+            ulong x = (ulong)NextLong() >> 11;
+
+            return Convert.ToDouble(x) * DoubleScale;
         }
 
         public virtual int NextInt()
         {
             byte[] bytes = new byte[4];
             NextBytes(bytes);
-
-            uint result = bytes[0];
-            result <<= 8;
-            result |= bytes[1];
-            result <<= 8;
-            result |= bytes[2];
-            result <<= 8;
-            result |= bytes[3];
-            return (int)result;
+            return (int)Pack.BE_To_UInt32(bytes);
         }
 
         public virtual long NextLong()
         {
-            return ((long)(uint) NextInt() << 32) | (long)(uint) NextInt();
+            byte[] bytes = new byte[8];
+            NextBytes(bytes);
+            return (long)Pack.BE_To_UInt64(bytes);
         }
     }
 }
diff --git a/crypto/test/src/security/test/SecureRandomTest.cs b/crypto/test/src/security/test/SecureRandomTest.cs
index 98bf75508..a474b5339 100644
--- a/crypto/test/src/security/test/SecureRandomTest.cs
+++ b/crypto/test/src/security/test/SecureRandomTest.cs
@@ -10,6 +10,7 @@ using Org.BouncyCastle.Crypto.Macs;
 using Org.BouncyCastle.Crypto.Prng;
 using Org.BouncyCastle.Crypto.Parameters;
 using Org.BouncyCastle.Utilities;
+using Org.BouncyCastle.Utilities.Test;
 
 namespace Org.BouncyCastle.Security.Tests
 {
@@ -36,6 +37,18 @@ namespace Org.BouncyCastle.Security.Tests
         }
 
         [Test]
+        public void TestNextDouble()
+        {
+            double min = new SecureRandom(new FixedRandomGenerator(0x00)).NextDouble();
+            Assert.GreaterOrEqual(min, 0.0);
+            Assert.Less(min, 1.0);
+
+            double max = new SecureRandom(new FixedRandomGenerator(0xFF)).NextDouble();
+            Assert.GreaterOrEqual(max, 0.0);
+            Assert.Less(max, 1.0);
+        }
+
+        [Test]
         public void TestSha1Prng()
         {
             SecureRandom random = SecureRandom.GetInstance("SHA1PRNG");
@@ -198,5 +211,40 @@ namespace Org.BouncyCastle.Security.Tests
 
             return chi2;
         }
+
+        private abstract class TestRandomGenerator
+            : IRandomGenerator
+        {
+            public virtual void AddSeedMaterial(byte[] seed)
+            {
+            }
+
+            public virtual void AddSeedMaterial(long seed)
+            {
+            }
+
+            public virtual void NextBytes(byte[] bytes)
+            {
+                NextBytes(bytes, 0, bytes.Length);
+            }
+
+            public abstract void NextBytes(byte[] bytes, int start, int len);
+        }
+
+        private sealed class FixedRandomGenerator
+            : TestRandomGenerator
+        {
+            private readonly byte b;
+
+            internal FixedRandomGenerator(byte b)
+            {
+                this.b = b;
+            }
+
+            public override void NextBytes(byte[] bytes, int start, int len)
+            {
+                Arrays.Fill(bytes, start, start + len, b);
+            }
+        }
     }
 }