Merge branch 'modulo-bias'

https://github.com/keepassx/keepassx/pull/21

src/core/Database.cpp

@@ -212,7 +212,7 @@ void Database::setKey(const CompositeKey& key, const QByteArray& transformSeed,
 
 void Database::setKey(const CompositeKey& key)
 {
-    setKey(key, Random::randomArray(32));
+    setKey(key, randomGen()->randomArray(32));
 }
 
 bool Database::hasKey() const

src/core/Global.h

@@ -128,4 +128,8 @@ template <> class QStaticAssertFailure<true> {};
 #  define KEEPASSX_EXPORT Q_DECL_EXPORT
 #endif
 
+#ifndef QUINT32_MAX
+#define QUINT32_MAX 4294967295U
+#endif
+
 #endif // KEEPASSX_GLOBAL_H

src/core/PasswordGenerator.cpp

@@ -40,20 +40,20 @@ QString PasswordGenerator::generatePassword(int length,
 
     if (flags & CharFromEveryGroup) {
         for (int i = 0; i < groups.size(); i++) {
-            int pos = Random::randomUInt(groups[i].size());
+            int pos = randomGen()->randomUInt(groups[i].size());
 
             password.append(groups[i][pos]);
         }
 
         for (int i = groups.size(); i < length; i++) {
-            int pos = Random::randomUInt(passwordChars.size());
+            int pos = randomGen()->randomUInt(passwordChars.size());
 
             password.append(passwordChars[pos]);
         }
 
         // shuffle chars
         for (int i = (password.size() - 1); i >= 1; i--) {
-            int j = Random::randomUInt(i + 1);
+            int j = randomGen()->randomUInt(i + 1);
 
             QChar tmp = password[i];
             password[i] = password[j];
@@ -62,7 +62,7 @@ QString PasswordGenerator::generatePassword(int length,
     }
     else {
         for (int i = 0; i < length; i++) {
-            int pos = Random::randomUInt(passwordChars.size());
+            int pos = randomGen()->randomUInt(passwordChars.size());
 
             password.append(passwordChars[pos]);
         }

src/core/Uuid.cpp

@@ -37,7 +37,7 @@ Uuid::Uuid(const QByteArray& data)
 
 Uuid Uuid::random()
 {
-    return Uuid(Random::randomArray(Length));
+    return Uuid(randomGen()->randomArray(Length));
 }
 
 QString Uuid::toBase64() const

src/crypto/Random.cpp

@@ -21,9 +21,17 @@
 
 #include "crypto/Crypto.h"
 
+class RandomBackendGcrypt : public RandomBackend
+{
+public:
+    void randomize(void* data, int len) Q_DECL_OVERRIDE;
+};
+
+Random* Random::m_instance(Q_NULLPTR);
+
 void Random::randomize(QByteArray& ba)
 {
-    randomize(ba.data(), ba.size());
+    m_backend->randomize(ba.data(), ba.size());
 }
 
 QByteArray Random::randomArray(int len)
@@ -38,8 +46,18 @@ QByteArray Random::randomArray(int len)
 
 quint32 Random::randomUInt(quint32 limit)
 {
+    Q_ASSERT(limit != 0);
+    Q_ASSERT(limit <= QUINT32_MAX);
+
     quint32 rand;
-    randomize(&rand, 4);
+    const quint32 ceil = QUINT32_MAX - (QUINT32_MAX % limit) - 1;
+
+    // To avoid modulo bias:
+    // Make sure rand is below the largest number where rand%limit==0
+    do {
+        m_backend->randomize(&rand, 4);
+    } while (rand > ceil);
+
     return (rand % limit);
 }
 
@@ -48,13 +66,32 @@ quint32 Random::randomUIntRange(quint32 min, quint32 max)
     return min + randomUInt(max - min);
 }
 
-void Random::randomize(void* data, int len)
+Random* Random::instance()
+{
+    if (!m_instance) {
+        m_instance = new Random(new RandomBackendGcrypt());
+    }
+
+    return m_instance;
+}
+
+void Random::createWithBackend(RandomBackend* backend)
+{
+    Q_ASSERT(backend);
+    Q_ASSERT(!m_instance);
+
+    m_instance = new Random(backend);
+}
+
+Random::Random(RandomBackend* backend)
+    : m_backend(backend)
+{
+}
+
+
+void RandomBackendGcrypt::randomize(void* data, int len)
 {
     Q_ASSERT(Crypto::initalized());
 
     gcry_randomize(data, len, GCRY_STRONG_RANDOM);
 }
-
-Random::Random()
-{
-}

src/crypto/Random.h

@@ -19,26 +19,45 @@
 #define KEEPASSX_RANDOM_H
 
 #include <QByteArray>
+#include <QScopedPointer>
+
+class RandomBackend
+{
+public:
+    virtual void randomize(void* data, int len) = 0;
+    virtual ~RandomBackend() {}
+};
 
 class Random
 {
 public:
-    static void randomize(QByteArray& ba);
-    static QByteArray randomArray(int len);
+    void randomize(QByteArray& ba);
+    QByteArray randomArray(int len);
 
     /**
      * Generate a random quint32 in the range [0, @p limit)
      */
-    static quint32 randomUInt(quint32 limit);
+    quint32 randomUInt(quint32 limit);
 
     /**
      * Generate a random quint32 in the range [@p min, @p max)
      */
-    static quint32 randomUIntRange(quint32 min, quint32 max);
+    quint32 randomUIntRange(quint32 min, quint32 max);
+
+    static Random* instance();
+    static void createWithBackend(RandomBackend* backend);
 
 private:
-    static void randomize(void* data, int len);
-    Random();
+    Random(RandomBackend* backend);
+
+    QScopedPointer<RandomBackend> m_backend;
+    static Random* m_instance;
+
+    Q_DISABLE_COPY(Random)
 };
 
+inline Random* randomGen() {
+    return Random::instance();
+}
+
 #endif // KEEPASSX_RANDOM_H

src/format/KeePass2Writer.cpp

@@ -45,10 +45,10 @@ void KeePass2Writer::writeDatabase(QIODevice* device, Database* db)
     m_error = false;
     m_errorStr.clear();
 
-    QByteArray masterSeed = Random::randomArray(32);
-    QByteArray encryptionIV = Random::randomArray(16);
-    QByteArray protectedStreamKey = Random::randomArray(32);
-    QByteArray startBytes = Random::randomArray(32);
+    QByteArray masterSeed = randomGen()->randomArray(32);
+    QByteArray encryptionIV = randomGen()->randomArray(16);
+    QByteArray protectedStreamKey = randomGen()->randomArray(32);
+    QByteArray startBytes = randomGen()->randomArray(32);
     QByteArray endOfHeader = "\r\n\r\n";
 
     CryptoHash hash(CryptoHash::Sha256);

src/keys/FileKey.cpp

@@ -121,7 +121,7 @@ void FileKey::create(QIODevice* device)
 
     xmlWriter.writeStartElement("Key");
 
-    QByteArray data = Random::randomArray(32);
+    QByteArray data = randomGen()->randomArray(32);
     xmlWriter.writeTextElement("Data", QString::fromAscii(data.toBase64()));
 
     xmlWriter.writeEndElement();

tests/CMakeLists.txt

@@ -162,6 +162,9 @@ add_unit_test(NAME testentry SOURCES TestEntry.cpp MOCS TestEntry.h
 add_unit_test(NAME testargumentparser SOURCES TestArgumentParser.cpp MOCS TestArgumentParser.h
               LIBS ${TEST_LIBRARIES})
 
+add_unit_test(NAME testrandom SOURCES TestRandom.cpp MOCS TestRandom.h
+              LIBS ${TEST_LIBRARIES})
+
 if(WITH_GUI_TESTS)
   add_subdirectory(gui)
 endif(WITH_GUI_TESTS)

tests/TestRandom.cpp

@@ -0,0 +1,97 @@
+/*
+ *  Copyright (C) 2013 Felix Geyer <debfx@fobos.de>
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 2 or (at your option)
+ *  version 3 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "TestRandom.h"
+
+#include "tests.h"
+#include "core/Endian.h"
+
+#include <QTest>
+
+void TestRandom::initTestCase()
+{
+    m_backend = new RandomBackendTest();
+
+    Random::createWithBackend(m_backend);
+}
+
+void TestRandom::testUInt()
+{
+    QByteArray nextBytes;
+
+    nextBytes = Endian::int32ToBytes(42, QSysInfo::ByteOrder);
+    m_backend->setNextBytes(nextBytes);
+    QCOMPARE(randomGen()->randomUInt(100), 42U);
+
+    nextBytes = Endian::int32ToBytes(117, QSysInfo::ByteOrder);
+    m_backend->setNextBytes(nextBytes);
+    QCOMPARE(randomGen()->randomUInt(100), 17U);
+
+    nextBytes = Endian::int32ToBytes(1001, QSysInfo::ByteOrder);
+    m_backend->setNextBytes(nextBytes);
+    QCOMPARE(randomGen()->randomUInt(1), 0U);
+
+    nextBytes.clear();
+    nextBytes.append(Endian::int32ToBytes(QUINT32_MAX, QSysInfo::ByteOrder));
+    nextBytes.append(Endian::int32ToBytes(QUINT32_MAX - 70000U, QSysInfo::ByteOrder));
+    m_backend->setNextBytes(nextBytes);
+    QCOMPARE(randomGen()->randomUInt(100000U), (QUINT32_MAX - 70000U) % 100000U);
+
+    nextBytes.clear();
+    for (int i = 0; i < 10000; i++) {
+        nextBytes.append(Endian::int32ToBytes((QUINT32_MAX / 2U) + 1U + i, QSysInfo::ByteOrder));
+    }
+    nextBytes.append(Endian::int32ToBytes(QUINT32_MAX / 2U, QSysInfo::ByteOrder));
+    m_backend->setNextBytes(nextBytes);
+    QCOMPARE(randomGen()->randomUInt((QUINT32_MAX / 2U) + 1U), QUINT32_MAX / 2U);
+}
+
+void TestRandom::testUIntRange()
+{
+    QByteArray nextBytes;
+
+    nextBytes = Endian::int32ToBytes(42, QSysInfo::ByteOrder);
+    m_backend->setNextBytes(nextBytes);
+    QCOMPARE(randomGen()->randomUIntRange(100, 200), 142U);
+}
+
+
+RandomBackendTest::RandomBackendTest()
+    : m_bytesIndex(0)
+{
+}
+
+void RandomBackendTest::randomize(void* data, int len)
+{
+    QVERIFY(len <= (m_nextBytes.size() - m_bytesIndex));
+
+    char* charData = reinterpret_cast<char*>(data);
+
+    for (int i = 0; i < len; i++) {
+        charData[i] = m_nextBytes[m_bytesIndex + i];
+    }
+
+    m_bytesIndex += len;
+}
+
+void RandomBackendTest::setNextBytes(const QByteArray& nextBytes)
+{
+    m_nextBytes = nextBytes;
+    m_bytesIndex = 0;
+}
+
+QTEST_GUILESS_MAIN(TestRandom)

tests/TestRandom.h

@@ -0,0 +1,51 @@
+/*
+ *  Copyright (C) 2013 Felix Geyer <debfx@fobos.de>
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 2 or (at your option)
+ *  version 3 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef KEEPASSX_TESTRANDOM_H
+#define KEEPASSX_TESTRANDOM_H
+
+#include "core/Global.h"
+#include "crypto/Random.h"
+
+#include <QObject>
+
+class RandomBackendTest : public RandomBackend
+{
+public:
+    RandomBackendTest();
+    void randomize(void* data, int len) Q_DECL_OVERRIDE;
+    void setNextBytes(const QByteArray& nextBytes);
+
+private:
+    QByteArray m_nextBytes;
+    int m_bytesIndex;
+};
+
+class TestRandom : public QObject
+{
+    Q_OBJECT
+
+private Q_SLOTS:
+    void initTestCase();
+    void testUInt();
+    void testUIntRange();
+
+private:
+    RandomBackendTest* m_backend;
+};
+
+#endif // KEEPASSX_TESTRANDOM_H