devbean / QtCipherSqlitePlugin

A Qt plugin for cipher SQLite.
http://qtciphersqliteplugin.galaxyworld.org
GNU Lesser General Public License v2.1
380 stars 155 forks source link

QSqlDatabase::isOpen returns incorrect data #18

Open mikhaylovv opened 6 years ago

mikhaylovv commented 6 years ago
if (_db.open()) {
    qDebug() << "db is OPEN: " << _db.isOpen();
}
else {
    qDebug() << "db is NOT open: " << _db.isOpen();
}

And the result is: db is NOT open: true

Ubuntu 18.04 gcc version 7.3.0 (Ubuntu 7.3.0-16ubuntu3) Qt 5.10.1 64bit

mikhaylovv commented 6 years ago

This error appears when you open an already created and encrypted database with QSQLITE_CREATE_KEY

mikhaylovv commented 6 years ago

My test using Catch2 for testing:

TEST_CASE("Database cipher tests", "[DatabaseCipher]") {
    REQUIRE(QSqlDatabase::isDriverAvailable("SQLITECIPHER"));

    QVector<QString> copies(3);
    for (int i = 0; i < 3; ++i) {
        copies[i] = QString::number(i) + "_storageCopy.sqlite";

        auto conn = QSqlDatabase::addDatabase("SQLITECIPHER", "DatabaseCipherTest_" + QString::number(i));
        conn.setDatabaseName(copies[i]);
        conn.setPassword(QString::number((i + 1) * 123));// 123 246 369
        conn.setConnectOptions("QSQLITE_CREATE_KEY");

        REQUIRE(conn.open());
        conn.close();
    }

    SECTION("Database::open() returns false with incorrect password") {
        bool dbFound = false;
        for (int i = 0; i < 3; ++i) {
            auto conn = QSqlDatabase::database("DatabaseCipherTest_" + QString::number(i), false);
            REQUIRE(conn.isValid());
            conn.setDatabaseName(copies.at(i));
            conn.setPassword("369");
            if (conn.open()) {
                CHECK(i == 2);
                CHECK(conn.isOpen());
                dbFound = true;
            }
            else {
                qDebug() << "connection error: " << conn.lastError().text();
                CHECK(!conn.isOpen()); // this is assert(false) x 3
            }
            conn.close();
        }

        CHECK(dbFound); // assert(false)
    }

    SECTION("Database::isOpen() NOT equal Database::open()") {
        for (int i = 0; i < 3; ++i) {
            auto conn = QSqlDatabase::database("DatabaseCipherTest_" + QString::number(i), false);
            REQUIRE(conn.isValid());
            conn.setDatabaseName(copies[i]);
            conn.setPassword(QString::number((i + 1) * 123));// 123 246 369

            CHECK(conn.open()); // this is assert(false) x 3
            CHECK(conn.isOpen()); // previous line false, but this TRUE
        }
    }

    for (auto& copy : copies)
        QFile::remove(copy);
}

connection error: "not an error Cannot create password. Maybe it is encrypted?" false asserts marks as // assert(false)

devbean commented 6 years ago

Thank you. And yes I know this bug.

In fact, QSQLITE_CREATE_KEY has two phases: first it open database then create password to this databse file. Some pseudo-code as following:

bool SQLiteCipherDriver::open()
{
...
if (sqlite3_open_v2(db, &d->access, openMode, nullptr) == SQLITE_OK) {
    setOpen(true);
    wxsqlite3_config(d->access, "cipher", CODEC_TYPE_SQLCIPHER);
    const int result = sqlite3_rekey(d->access, "pass", 4);
    if (SQLITE_OK != result) {
        return false;
    }
} else {
  setOpen(false);
}
...
}

As you can see, first the plugin need open SQLite then rekey (create password). In Qt SQL plugin, QSqlDriver::open() is used for opening a database. If we open database successfully, we should set isOpen() to true but because rekey is only an action after open and it fails so open() returns false. (But I'm not sure if this is correct for open(). That's all because rekey is actually two actions. Maybe this should be recode.

mikhaylovv commented 6 years ago

if you create a database without a QSQLITE_CREATE_KEY flag, it is not encrypted. it would be good if the database to be encrypted if the password is not an empty field. or at least the base will close and setOpen(false);, if it could not do rekey