Use real pickling key

CMakeLists.txt

@@ -334,9 +334,11 @@ target_include_directories(${QUOTIENT_LIB_NAME} PUBLIC
     $<BUILD_INTERFACE:$<$<NOT:$<BOOL:${QUOTIENT_FORCE_NAMESPACED_INCLUDES}>>:${CMAKE_CURRENT_SOURCE_DIR}/Quotient>>
 )
 
+find_package(OpenSSL REQUIRED)
+
 target_link_libraries(${QUOTIENT_LIB_NAME}
     PUBLIC ${Qt}::Core ${Qt}::Network ${Qt}::Gui qt${${Qt}Core_VERSION_MAJOR}keychain ${Qt}::Sql
-    PRIVATE ${Qt}::CorePrivate matrix_rust_sdk_crypto_cpp)
+    PRIVATE ${Qt}::CorePrivate matrix_rust_sdk_crypto_cpp OpenSSL::Crypto)
 
 if(NOT WIN32)
     target_link_libraries(${QUOTIENT_LIB_NAME} PRIVATE PkgConfig::SQLITE)

Quotient/connection.cpp

@@ -319,6 +319,73 @@ void Connection::Private::loginToServer(LoginArgTs&&... loginArgs)
         });
 }
 
+inline QFuture<QKeychain::Job*> runKeychainJob(QKeychain::Job* j, const QString& keychainId)
+{
+    j->setAutoDelete(true);
+    j->setKey(keychainId);
+    auto ft = QtFuture::connect(j, &QKeychain::Job::finished);
+    j->start();
+    return ft;
+}
+
+QFuture<void> Connection::Private::setupPicklingKey()
+{
+    using namespace QKeychain;
+    const auto keychainId = q->userId() + "-Pickle"_L1;
+    qCInfo(MAIN) << "Keychain request: app" << qAppName() << "id" << keychainId;
+
+    return runKeychainJob(new ReadPasswordJob(qAppName()), keychainId)
+        .then([keychainId, this](const Job* j) -> QFuture<Job*> {
+            // The future will hold nullptr if the existing pickling key was found and no write is
+            // pending; a pointer to the write job if if a new key was made and is being written;
+            // be cancelled in case of an error.
+            switch (const auto readJob = static_cast<const ReadPasswordJob*>(j); readJob->error()) {
+            case Error::NoError: {
+                auto&& data = readJob->binaryData();
+                qDebug(E2EE) << "Successfully loaded pickling key from keychain";
+
+                setupCryptoMachine(data);
+                return QtFuture::makeReadyFuture<Job*>(nullptr);
+                qCritical(E2EE)
+                    << "The pickling key loaded from" << keychainId << "has length"
+                    << data.size() << "but the library expected" << PicklingKey::extent;
+                return {};
+            }
+            case Error::EntryNotFound: {
+                auto&& picklingKey = PicklingKey::generate();
+                auto writeJob = new WritePasswordJob(qAppName());
+                setupCryptoMachine(picklingKey.viewAsByteArray().toBase64());
+                writeJob->setBinaryData(picklingKey.viewAsByteArray().toBase64());
+                qDebug(E2EE) << "Saving a new pickling key to the keychain";
+                return runKeychainJob(writeJob, keychainId);
+            }
+            default:
+                qWarning(E2EE) << "Error loading pickling key - please fix your keychain:"
+                               << readJob->errorString();
+                //TODO: We probably want to fail entirely here.
+            }
+            return {};
+        })
+        .unwrap()
+        .then([](QFuture<Job*> writeFuture) {
+            if (const Job* const writeJob = writeFuture.result();
+                writeJob && writeJob->error() != Error::NoError) //
+            {
+                qCritical(E2EE) << "Could not save pickling key to keychain: "
+                                << writeJob->errorString();
+                writeFuture.cancel();
+            }
+        });
+}
+
+void Connection::Private::setupCryptoMachine(const QByteArray& picklingKey)
+{
+    auto mxIdForDb = q->userId();
+    mxIdForDb.replace(u':', u'_');
+    const QString databasePath{ QStandardPaths::writableLocation(QStandardPaths::AppDataLocation) % u'/' % mxIdForDb % u'/' % q->deviceId() };
+    cryptoMachine = crypto::init(stringToRust(q->userId()), stringToRust(q->deviceId()), stringToRust(databasePath), bytesToRust(picklingKey));
+}
+
 void Connection::Private::completeSetup(const QString& mxId, bool newLogin,
                                         const std::optional<QString>& deviceId,
                                         const std::optional<QString>& accessToken)
@@ -329,10 +396,7 @@ void Connection::Private::completeSetup(const QString& mxId, bool newLogin,
                   << "by user" << data->userId()
                   << "from device" << data->deviceId();
     connect(qApp, &QCoreApplication::aboutToQuit, q, &Connection::saveState);
-    auto mxIdForDb = mxId;
-    mxIdForDb.replace(u':', u'_');
-    const QString databasePath{ QStandardPaths::writableLocation(QStandardPaths::AppDataLocation) % u'/' % mxIdForDb % u'/' % *deviceId };
-    cryptoMachine = crypto::init(stringToRust(mxId), stringToRust(*deviceId), stringToRust(databasePath));
+    setupPicklingKey();
 
     if (accessToken.has_value()) {
         q->loadVersions();
@@ -528,6 +592,10 @@ void Connection::onSyncSuccess(SyncData&& data, bool fromCache)
 
 void Connection::Private::processOutgoingRequests()
 {
+    if (!cryptoMachine) {
+        qWarning() << "Crypto machine not loaded yet";
+        return;
+    }
     auto requests = (*cryptoMachine)->outgoing_requests();
     for (const auto &request : requests) {
         auto id = stringFromRust(request.id());
@@ -622,7 +690,9 @@ void Connection::Private::consumeRoomData(SyncDataList&& roomDataList,
                         for (const auto &id : ids) {
                             userIds.push_back(stringToRust(id));
                         }
-                        (*cryptoMachine)->update_tracked_users(userIds);
+                        if (cryptoMachine) {
+                            (*cryptoMachine)->update_tracked_users(userIds);
+                        }
                     }
                 },
                 Qt::QueuedConnection);

Quotient/connection_p.h

@@ -9,6 +9,7 @@
 #include "connectiondata.h"
 #include "settings.h"
 #include "syncdata.h"
+#include "e2ee/e2ee_common.h"
 
 #include "csapi/account-data.h"
 #include "csapi/capabilities.h"
@@ -160,5 +161,8 @@ class Q_DECL_HIDDEN Quotient::Connection::Private {
 
     void requestDeviceVerification(KeyVerificationSession* session);
     void requestUserVerification(KeyVerificationSession* session);
+
+    QFuture<void> setupPicklingKey();
+    void setupCryptoMachine(const QByteArray& picklingKey);
 };
 } // namespace Quotient

Quotient/e2ee/e2ee_common.cpp

@@ -8,5 +8,176 @@
 
 #include <QtCore/QRandomGenerator>
 
+#include <openssl/crypto.h>
+
 using namespace Quotient;
 
+QByteArray Quotient::byteArrayForOlm(size_t bufferSize)
+{
+    if (std::in_range<QByteArray::size_type>(bufferSize)) [[likely]]
+        return { static_cast<QByteArray::size_type>(bufferSize), '\0' };
+
+    qCritical(E2EE) << "Buffer size out of QByteArray range:" << bufferSize;
+    // Zero-length QByteArray is an almost guaranteed way to cause
+    // an internal error in QOlm* classes, unless checked
+    return {};
+}
+
+void Quotient::_impl::checkForSpanShortfall(QByteArray::size_type inputSize, int neededSize)
+{
+    if (inputSize < neededSize) {
+        qCCritical(E2EE) << "Not enough bytes to create a valid span: "
+                         << inputSize << '<' << neededSize
+                         << "- undefined behaviour imminent";
+        Q_ASSERT(false);
+        // Can't help it in Release builds; a span of the given size has
+        // to be returned regardless, so UB
+    }
+}
+
+void Quotient::fillFromSecureRng(std::span<byte_t> bytes)
+{
+    // Discussion of QRandomGenerator::system() vs. OpenSSL's RAND_bytes
+    //
+    // It is a rather close call between the two; to be honest, TL;DR from the
+    // below is "it doesn't really matter". Going through the source code of
+    // both libraries, along with reading others' investigations like
+    // https://github.com/golang/go/issues/33542, left me (@kitsune) with
+    // the following:
+    // 1. Both ultimately use more or less the same stuff internally - which is
+    //    is not surprising and is reassuring in TL;DR. Contrary to occasional
+    //    claims on interwebs though, QRandomGenerator does not rely on OpenSSL,
+    //    calling system primitives directly instead.
+    // 2. Qt's code is massively simpler - again, effectively it's a rather
+    //    thin wrapper around what I know for a reasonably good practice across
+    //    the platforms supported by Quotient.
+    // 3. OpenSSL is more flexible in that you can specify which RNG engine
+    //    you actually want: you can, e.g., explicitly use the hardware-based
+    //    (aka "true") RNG on modern Intel CPUs. QRandomGenerator "simply"
+    //    offers the nice Qt interface to whatever best the operating system
+    //    provides (and at least Linux kernel happens to also use true RNGs
+    //    where it can, so ¯\(ツ)/¯)
+    // 3. QRandomGenerator produces stuff in uint32_t (or uint64_t in case of
+    //    QRandomGenerator64) chunks; OpenSSL generates bytes, which is sorta
+    //    more convenient. Quotient never needs quantities of randomness that
+    //    are not multiples of 16 _bytes_; the Q_UNLIKELY branch below is merely
+    //    to cover an edge case that should never happen in the first place.
+    //    So, ¯\(ツ)/¯ again.
+    //
+    // Based on this, QRandomGenerator::system() wins by a tiny non-functional
+    // margin; my somewhat educated opinion for now is that both APIs are
+    // equally good from the functionality and security point of view.
+
+    // QRandomGenerator::fillRange works in terms of 32-bit words,
+    // and FixedBuffer happens to deal with sizes that are multiples
+    // of those (16, 32, etc.)
+    const qsizetype wordsCount = bytes.size() / 4;
+    QRandomGenerator::system()->fillRange(std::bit_cast<uint32_t*>(bytes.data()), wordsCount);
+    if (const int remainder = bytes.size() % 4; Q_UNLIKELY(remainder != 0)) {
+        // Not normal; but if it happens, apply best effort
+        QRandomGenerator::system()->generate(bytes.end() - remainder,
+                                             bytes.end());
+    }
+}
+
+namespace {
+auto initializeSecureHeap()
+{
+#if !defined(LIBRESSL_VERSION_NUMBER)
+    const auto result =
+        CRYPTO_secure_malloc_init(FixedBufferBase::TotalSecureHeapSize, 16);
+    if (result > 0) {
+        qDebug(E2EE) << FixedBufferBase::TotalSecureHeapSize
+                     << "bytes of secure heap initialised";
+        if (std::atexit([] {
+                CRYPTO_secure_malloc_done();
+                qDebug(E2EE) << "Dismantled secure heap";
+            })
+            != 0)
+            qWarning(E2EE)
+                << "Could not register a cleanup function for secure heap!";
+    } else
+        qCritical(E2EE) << "Secure heap could not be initialised, sensitive "
+                           "data will remain in common dynamic memory";
+#else
+    const auto result = 0;
+    qCritical(E2EE) << "Secure heap is not available in LibreSSL";
+#endif
+    return result;
+}
+
+FixedBufferBase::value_type* allocate(size_t bytes, bool initWithZeros = false)
+{
+#if !defined(LIBRESSL_VERSION_NUMBER)
+    static auto secureHeapInitialised [[maybe_unused]] = initializeSecureHeap();
+
+    const auto p = static_cast<FixedBufferBase::value_type*>(
+        initWithZeros ? OPENSSL_secure_zalloc(bytes) : OPENSSL_secure_malloc(bytes));
+    Q_ASSERT(CRYPTO_secure_allocated(p));
+    qDebug(E2EE) << "Allocated" << CRYPTO_secure_actual_size(p)
+                 << "bytes of secure heap (requested" << bytes << "bytes),"
+                 << CRYPTO_secure_used()
+                 << "/ 65536 bytes of secure heap used in total";
+#else
+    const auto p =
+        static_cast<FixedBufferBase::value_type*>(initWithZeros ? calloc(bytes, 1) : malloc(bytes));
+#endif
+    return p;
+}
+
+} // namespace
+
+FixedBufferBase::FixedBufferBase(size_type bufferSize, InitOptions options)
+    : size_(bufferSize)
+{
+    if (bufferSize >= TotalSecureHeapSize) {
+        qCritical(E2EE) << "Too large buffer size:" << bufferSize;
+        return;
+    }
+    if (options == Uninitialized)
+        return;
+
+    data_ = allocate(bufferSize, options == FillWithZeros);
+    if (options == FillWithRandom)
+        fillFromSecureRng({ data_, bufferSize });
+}
+
+void FixedBufferBase::fillFrom(QByteArray&& source)
+{
+    if (unsignedSize(source) != size_) {
+        qCritical(E2EE) << "Can't load a fixed buffer of length" << size_
+                        << "from a string with length" << source.size();
+        Q_ASSERT(unsignedSize(source) == size_); // Always false
+        return;
+    }
+    if (data_ != nullptr) {
+        qWarning(E2EE) << "Overwriting the fixed buffer with another string";
+        clear();
+    }
+
+    data_ = allocate(size_);
+    std::copy(source.cbegin(), source.cend(), std::bit_cast<char*>(data_));
+    if (source.isDetached())
+        source.clear();
+    else
+        qWarning(E2EE)
+            << "The fixed buffer source is shared; assuming that the caller is "
+               "responsible for securely clearing other copies";
+}
+
+void FixedBufferBase::clear()
+{
+    if (empty())
+        return;
+
+#if !defined(LIBRESSL_VERSION_NUMBER)
+    Q_ASSERT(CRYPTO_secure_allocated(data_));
+    const auto actualSize = OPENSSL_secure_actual_size(data_);
+    OPENSSL_secure_clear_free(data_, size_);
+    qDebug(E2EE) << "Deallocated" << actualSize << "bytes,"
+                 << CRYPTO_secure_used() << "/ 65536 bytes of secure heap used";
+#else
+    free(data_);
+#endif
+    data_ = nullptr;
+}

matrix-rust-sdk-crypto/src/lib.rs

@@ -34,17 +34,15 @@ impl Drop for CryptoMachine {
     }
 }
 
-fn init(user_id: String, device_id: String, path: String) -> Box<CryptoMachine> {
+fn init(user_id: String, device_id: String, path: String, pickle_key: String) -> Box<CryptoMachine> {
     let rt = tokio::runtime::Runtime::new().unwrap();
     let _ = rt.enter();
 
     let machine = rt.block_on(async {
         let user_id = UserId::parse(user_id).unwrap();
         let device_id: Box<DeviceId> = device_id.into();
 
-        //TODO: use real pickle key
-        println!("path: {path}");
-        let store = SqliteCryptoStore::open(Path::new(&path), "123123123".into()).await.unwrap();
+        let store = SqliteCryptoStore::open(Path::new(&path), Some(&pickle_key)).await.unwrap();
 
         OlmMachine::with_store(&user_id, &device_id, store)
             .await
@@ -845,7 +843,7 @@ mod ffi {
         type EncryptionInfo;
 
         // General CryptoMachine functions
-        fn init(user_id: String, device_id: String, path: String) -> Box<CryptoMachine>;
+        fn init(user_id: String, device_id: String, path: String, pickle_key: String) -> Box<CryptoMachine>;
         fn outgoing_requests(self: &CryptoMachine) -> Vec<OutgoingRequest>;
         fn receive_sync_changes(self: &mut CryptoMachine, sync_json: String) -> Vec<KeyVerificationRequest>;
         fn receive_verification_event(self: &mut CryptoMachine, full_json: String);