[Security] Use a CSPRNG for salts, tokens, and RNG seeding

Password salts and activation tokens were generated with the global SFMT
RNG, which was seeded from a 32-bit timestamp, making registration
salts and activation tokens predictable. The game RNG used the same
timestamp seed across restarts.

Add CryptoUtil backed by OpenSSL RAND_bytes and use it for salt/token
generation and to seed RNG_SFMT with a 64-bit CSPRNG value in both the
client and server. Link libcockatrice_utility against OpenSSL::Crypto.

Took 30 seconds

Took 25 minutes

cockatrice/src/main.cpp

@@ -53,6 +53,7 @@
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/settings/network_settings.h>
 #include <libcockatrice/settings/personal_settings.h>
+#include <libcockatrice/utility/cryptoutil.h>
 
 QTranslator *translator, *qtTranslator;
 RNG_Abstract *rng;
@@ -292,7 +293,7 @@ int main(int argc, char *argv[])
         }
     }
 
-    rng = new RNG_SFMT;
+    rng = new RNG_SFMT(CryptoUtil::randomUInt64());
     themeManager = new ThemeManager;
     soundEngine = new SoundEngine;
 

libcockatrice_rng/libcockatrice/rng/rng_sfmt.cpp

@@ -17,6 +17,13 @@ RNG_SFMT::RNG_SFMT(QObject *parent) : RNG_Abstract(parent)
     sfmt_init_gen_rand(&sfmt, QDateTime::currentDateTime().toSecsSinceEpoch());
 }
 
+RNG_SFMT::RNG_SFMT(uint64_t seed, QObject *parent) : RNG_Abstract(parent)
+{
+    // initialize the random number generator with a 64bit seed, e.g. from a CSPRNG
+    uint32_t seedArray[2] = {static_cast<uint32_t>(seed), static_cast<uint32_t>(seed >> 32)};
+    sfmt_init_by_array(&sfmt, seedArray, 2);
+}
+
 /**
  * This method is the rand() equivalent which calls the cdf with proper bounds.
  *

libcockatrice_rng/libcockatrice/rng/rng_sfmt.h

@@ -37,6 +37,7 @@ private:
 
 public:
     explicit RNG_SFMT(QObject *parent = nullptr);
+    explicit RNG_SFMT(uint64_t seed, QObject *parent = nullptr);
     unsigned int rand(int min, int max) override;
 };
 

libcockatrice_utility/CMakeLists.txt

@@ -6,13 +6,14 @@ set(CMAKE_AUTOUIC ON)
 set(CMAKE_AUTORCC ON)
 
 set(UTILITY_SOURCES
-    libcockatrice/utility/expression.cpp libcockatrice/utility/levenshtein.cpp libcockatrice/utility/passwordhasher.cpp
+    libcockatrice/utility/cryptoutil.cpp libcockatrice/utility/expression.cpp libcockatrice/utility/levenshtein.cpp
-    libcockatrice/utility/server_rate_limiter.cpp libcockatrice/utility/warning_categories.cpp
+    libcockatrice/utility/passwordhasher.cpp libcockatrice/utility/server_rate_limiter.cpp libcockatrice/utility/warning_categories.cpp
 )
 
 set(UTILITY_HEADERS
     libcockatrice/utility/card_ref.h
     libcockatrice/utility/color.h
+    libcockatrice/utility/cryptoutil.h
     libcockatrice/utility/expression.h
     libcockatrice/utility/levenshtein.h
     libcockatrice/utility/macros.h
@@ -32,7 +33,9 @@ add_library(libcockatrice_utility STATIC ${UTILITY_SOURCES} ${UTILITY_HEADERS})
 
 target_include_directories(libcockatrice_utility PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 
-target_link_libraries(libcockatrice_utility PUBLIC libcockatrice_rng ${QT_CORE_MODULE})
+find_package(OpenSSL REQUIRED)
+
+target_link_libraries(libcockatrice_utility PUBLIC libcockatrice_rng OpenSSL::Crypto ${QT_CORE_MODULE})
 
 set(ORACLE_LIBS)
 

libcockatrice_utility/libcockatrice/utility/cryptoutil.cpp

@@ -0,0 +1,25 @@
+#include "cryptoutil.h"
+
+#include <openssl/rand.h>
+
+namespace CryptoUtil
+{
+QByteArray randomBytes(int count)
+{
+    QByteArray bytes(count, '\0');
+    if (RAND_bytes(reinterpret_cast<unsigned char *>(bytes.data()), count) != 1) {
+        // Randomness failure is fatal: never fall back to a predictable source.
+        qFatal("CryptoUtil::randomBytes: RAND_bytes failed");
+    }
+    return bytes;
+}
+
+quint64 randomUInt64()
+{
+    quint64 value;
+    if (RAND_bytes(reinterpret_cast<unsigned char *>(&value), sizeof(value)) != 1) {
+        qFatal("CryptoUtil::randomUInt64: RAND_bytes failed");
+    }
+    return value;
+}
+} // namespace CryptoUtil

libcockatrice_utility/libcockatrice/utility/cryptoutil.h

@@ -0,0 +1,13 @@
+#ifndef CRYPTOUTIL_H
+#define CRYPTOUTIL_H
+
+#include <QByteArray>
+#include <QtGlobal>
+
+namespace CryptoUtil
+{
+QByteArray randomBytes(int count);
+quint64 randomUInt64();
+} // namespace CryptoUtil
+
+#endif

libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp

@@ -1,7 +1,7 @@
 #include "passwordhasher.h"
 
 #include <QCryptographicHash>
-#include <libcockatrice/rng/rng_sfmt.h>
+#include <libcockatrice/utility/cryptoutil.h>
 
 QString PasswordHasher::computeHash(const QString &password, const QString &salt)
 {
@@ -21,12 +21,28 @@ QString PasswordHasher::generateRandomSalt(const int len)
     static const char alphanum[] = "0123456789"
                                    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
                                    "abcdefghijklmnopqrstuvwxyz";
+    const int size = sizeof(alphanum) - 1;
+
+    // Two bytes per character, corrected for modulo bias via rejection sampling.
+    const int bucketSize = 65536 / size;
+    const int limit = bucketSize * size;
 
     QString ret;
-    int size = sizeof(alphanum) - 1;
+    ret.reserve(len);
-
+    QByteArray random = CryptoUtil::randomBytes(len * 2);
+    int bytesUsed = 0;
     for (int i = 0; i < len; ++i) {
-        ret.append(alphanum[rng->rand(0, size)]);
+        unsigned int value;
+        do {
+            if (bytesUsed >= random.size()) {
+                random = CryptoUtil::randomBytes(len * 2);
+                bytesUsed = 0;
+            }
+            value = static_cast<unsigned int>(static_cast<unsigned char>(random.at(bytesUsed))) << 8 |
+                    static_cast<unsigned int>(static_cast<unsigned char>(random.at(bytesUsed + 1)));
+            bytesUsed += 2;
+        } while (value >= limit);
+        ret.append(alphanum[value / bucketSize]);
     }
 
     return ret;
@@ -34,5 +50,5 @@ QString PasswordHasher::generateRandomSalt(const int len)
 
 QString PasswordHasher::generateActivationToken()
 {
-    return QCryptographicHash::hash(generateRandomSalt().toUtf8(), QCryptographicHash::Md5).toBase64().left(16);
+    return QString(CryptoUtil::randomBytes(16).toBase64().left(16));
 }

servatrice/src/main.cpp

@@ -33,6 +33,7 @@
 #include <QtGlobal>
 #include <iostream>
 #include <libcockatrice/rng/rng_sfmt.h>
+#include <libcockatrice/utility/cryptoutil.h>
 #include <libcockatrice/utility/passwordhasher.h>
 
 RNG_Abstract *rng;
@@ -169,7 +170,7 @@ int main(int argc, char *argv[])
 
     signalhandler = new SignalHandler();
 
-    rng = new RNG_SFMT;
+    rng = new RNG_SFMT(CryptoUtil::randomUInt64());
 
     std::cerr << "Servatrice " << VERSION_STRING << " starting." << std::endl;
     std::cerr << "-------------------------" << std::endl;

tests/password_hash_test.cpp

@@ -1,25 +1,9 @@
 #include "gtest/gtest.h"
-#include <libcockatrice/rng/rng_abstract.h>
+#include <cstring>
-#include <libcockatrice/rng/rng_sfmt.h>
 #include <libcockatrice/utility/passwordhasher.h>
 
-RNG_Abstract *rng;
-
 namespace
 {
-class PasswordHashTest : public ::testing::Test
-{
-protected:
-    void SetUp() override
-    {
-        rng = new RNG_SFMT;
-    }
-
-    void TearDown() override
-    {
-        delete rng;
-    }
-};
 
 TEST(PasswordHashTest, RegressionTest)
 {
@@ -29,6 +13,29 @@ TEST(PasswordHashTest, RegressionTest)
     QString hash = PasswordHasher::computeHash(password, salt);
     ASSERT_EQ(hash, salt + expected) << "The computed hash value remains the same";
 }
+
+TEST(PasswordHashTest, SaltUsesAlphanumericCharset)
+{
+    static const char alphanum[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
+    const QString salt = PasswordHasher::generateRandomSalt();
+    ASSERT_EQ(salt.size(), 16);
+    for (const QChar &c : salt) {
+        ASSERT_NE(strchr(alphanum, c.toLatin1()), nullptr);
+    }
+}
+
+TEST(PasswordHashTest, SaltsAreUnique)
+{
+    const QString salt1 = PasswordHasher::generateRandomSalt();
+    const QString salt2 = PasswordHasher::generateRandomSalt();
+    ASSERT_NE(salt1, salt2);
+}
+
+TEST(PasswordHashTest, TokenHasExpectedLength)
+{
+    const QString token = PasswordHasher::generateActivationToken();
+    ASSERT_EQ(token.size(), 16);
+}
 } // namespace
 
 int main(int argc, char **argv)