From 221d00c4945c027b83d2ea72bc213e465d16884d Mon Sep 17 00:00:00 2001 From: Aqua-sama Date: Wed, 29 Apr 2020 18:45:17 +0300 Subject: Expand pluginloader test coverage - add poi-plugin-load to test compatibility of plugins - rewrite tests to use catch2 - use cpp stdlib to read files - clang-tidy and clang-format pass --- .clang-tidy | 2 +- lib/pluginloader/meson.build | 38 +++++---- lib/pluginloader/pluginloader.cpp | 107 +++++++++++++----------- lib/pluginloader/pluginloader.h | 49 ++++++----- lib/pluginloader/test/pluginloader-load.cpp | 22 +++++ lib/pluginloader/test/pluginloader-sigmatch.cpp | 98 ++++++++++++++++++++-- plugins/ProfileEditor/meson.build | 11 +++ src/browser.cpp | 2 +- 8 files changed, 229 insertions(+), 100 deletions(-) create mode 100644 lib/pluginloader/test/pluginloader-load.cpp diff --git a/.clang-tidy b/.clang-tidy index 4963ca4..fda977d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: 'clang-diagnostic-*,clang-analyzer-*,bugprone-*,cert-*,cppcoreguidelines-*,hicpp-*,misc-*,modernize-*,performance-*,portability-*,readability-*, -modernize-use-trailing-return-type' +Checks: 'clang-diagnostic-*,clang-analyzer-*,bugprone-*,cert-*,cppcoreguidelines-*,hicpp-*,misc-*,modernize-*,performance-*,portability-*,readability-*,-hicpp-signed-bitwise,-modernize-use-trailing-return-type' WarningsAsErrors: '' HeaderFilterRegex: '' AnalyzeTemporaryDtors: false diff --git a/lib/pluginloader/meson.build b/lib/pluginloader/meson.build index acdd47e..5e7c39c 100644 --- a/lib/pluginloader/meson.build +++ b/lib/pluginloader/meson.build @@ -22,25 +22,22 @@ publicKey_h = custom_target('publicKey_h', '--output=@OUTPUT@', '--array-name=publicKey_pem'] ) -pluginloader_moc = mod_qt5.preprocess( - moc_headers: ['pluginloader.h'], - dependencies: dep_qt5 -) - dep_pluginloader = declare_dependency( include_directories: include_directories('.'), link_with: static_library('plugin', - ['pluginloader.cpp', pluginloader_moc, publicKey_h], + ['pluginloader.cpp', publicKey_h], include_directories: include_directories('.'), dependencies: [dep_qt5, dependency('openssl', required: true)]) ) # generate a test file that would be signed -signedfile_dat = custom_target('signedfile.dat', - input: 'write-random.py', - output: 'signedfile.dat', - command: [ python3, '@INPUT@', '--output=@OUTPUT@' ] -) +unsignedfile_dat = custom_target('unsignedfile.dat', input: 'write-random.py', output: 'unsignedfile.dat', command: [ python3, '@INPUT@', '--output=@OUTPUT@' ]) + +signedfile_dat = custom_target('signedfile.dat', input: 'write-random.py', output: 'signedfile.dat', command: [ python3, '@INPUT@', '--output=@OUTPUT@' ]) + +badsignedfile_dat = custom_target('badsignedfile.dat', input: 'write-random.py', output: 'badsignedfile.dat', command: [ python3, '@INPUT@', '--output=@OUTPUT@' ]) +badsignedfile_sig = custom_target('badsignedfile.dat.sig', input: 'write-random.py', output: 'badsignedfile.dat.sig', command: [ python3, '@INPUT@', '--output=@OUTPUT@' ]) + # sign test file signedfile_sig = custom_target('signedfile.dat.sig', input: signedfile_dat, @@ -48,15 +45,24 @@ signedfile_sig = custom_target('signedfile.dat.sig', command: [ openssl, 'dgst', '-sha256', '-sign', private_pem, '-out', '@OUTPUT@', '@INPUT@' ] ) -signedfile_idep = declare_dependency(sources: [ signedfile_dat, signedfile_sig ]) +signedfile_idep = declare_dependency(sources: [ unsignedfile_dat, signedfile_dat, signedfile_sig, badsignedfile_dat, badsignedfile_sig ]) pluginloader_sigmatch = executable('pluginloader-sigmatch', sources: [ 'test/pluginloader-sigmatch.cpp' ], - dependencies: [ dep_qt5, dep_gtest, dep_pluginloader, signedfile_idep ] + dependencies: [ dep_qt5, dep_catch, dep_pluginloader, signedfile_idep ] ) -test('pluginloader: signature matching', pluginloader_sigmatch, - env: environment({ 'SIGNEDFILE' : 'signedfile.dat' }), - workdir: meson.current_build_dir() +test('signature matching', pluginloader_sigmatch, suite: 'pluginloader', + env: { + 'SIGNEDFILE' : signedfile_dat.full_path(), + 'UNSIGNEDFILE': unsignedfile_dat.full_path(), + 'BADSIGNEDFILE': badsignedfile_dat.full_path() + }, ) +poi_plugin_loader = executable('poi-plugin-load', dependencies: [ dep_qt5, dep_spdlog, dep_pluginloader ], sources: 'test/pluginloader-load.cpp') + +# make sure this fails when no plugin or an invalid file is passed +test('load', poi_plugin_loader, suite: 'pluginloader', should_fail: true) +test('load', poi_plugin_loader, suite: 'pluginloader', args: files('meson.build'), should_fail: true) + diff --git a/lib/pluginloader/pluginloader.cpp b/lib/pluginloader/pluginloader.cpp index e5c4b89..082a449 100644 --- a/lib/pluginloader/pluginloader.cpp +++ b/lib/pluginloader/pluginloader.cpp @@ -1,38 +1,45 @@ /* * This file is part of smolbote. It's copyrighted by the contributors recorded * in the version control history of the file, available from its original - * location: https://neueland.iserlohn-fortress.net/gitea/aqua/smolbote + * location: https://library.iserlohn-fortress.net/aqua/smolbote.git * * SPDX-License-Identifier: GPL-3.0 */ #include "pluginloader.h" +#include "publicKey.h" #include +#include +#include #include #include -#include "publicKey.h" - -PluginLoader::PluginLoader(const QString &fileName, PluginLoader::SignatureState state, QObject *parent) - : QPluginLoader(fileName, parent) - , m_state(state) -{ -} bool PluginLoader::verify(const char *hashName) { - const QString sigName = this->fileName() + ".sig"; - if(!QFile::exists(sigName)) { - if(m_state.ignored || m_state.checked) - return true; - - m_sigError = tr("A signature is required, but none was found."); + const std::filesystem::path plugin_path(fileName().toStdString()); + if(!std::filesystem::is_regular_file(plugin_path)) { + m_sigError = tr("A plugin is required, but none was found."); return false; } + if(m_state <= SigIgnored) { + return true; + } + + const std::filesystem::path signature_path(fileName().toStdString() + ".sig"); + if(!std::filesystem::is_regular_file(signature_path)) { + if(m_state >= SigEnforced) { + m_sigError = tr("A signature is required, but none was found."); + return false; + } + + return true; + } + auto *bio = BIO_new_mem_buf(publicKey_pem, publicKey_pem_len); Q_CHECK_PTR(bio); - auto *key = PEM_read_bio_PUBKEY(bio, NULL, NULL, NULL); + auto *key = PEM_read_bio_PUBKEY(bio, nullptr, nullptr, nullptr); Q_CHECK_PTR(key); auto *ctx = EVP_MD_CTX_new(); @@ -41,53 +48,53 @@ bool PluginLoader::verify(const char *hashName) const auto *md = EVP_get_digestbyname(hashName); Q_CHECK_PTR(md); - int rc = EVP_DigestVerifyInit(ctx, NULL, md, NULL, key); + int rc = EVP_DigestVerifyInit(ctx, nullptr, md, nullptr, key); if(rc != 1) { m_sigError = tr("Failed to compute signature (stage=init)"); return false; } // read plugin into DigestVerifyUpdate - QFile plugin(this->fileName()); - plugin.open(QIODevice::ReadOnly); - int len = plugin.size(); - int read = 0; - auto *buf = new unsigned char[1024]; - while(len > 0) { - read = plugin.read(reinterpret_cast(buf), 1024); - len -= read; - - rc = EVP_DigestVerifyUpdate(ctx, buf, read); - if(rc != 1) { - m_sigError = tr("Failed to compute signature (staga=update)"); - return false; + { + std::ifstream plugin(plugin_path, std::ios::binary); + plugin.unsetf(std::ios::skipws); + if(!plugin.is_open()) { + m_sigError = tr("Cannot read plugin during signature check"); + return true; + } + + const std::size_t buffer_size = 1024; + std::vector buffer(buffer_size); + while(const auto sz = plugin.readsome(&buffer.front(), buffer_size)) { + rc = EVP_DigestVerifyUpdate(ctx, reinterpret_cast(buffer.data()), sz); + if(rc != 1) { + m_sigError = tr("Failed to compute signature (stage=update)"); + return false; + } } } - delete[] buf; - plugin.close(); // read signature into DigestVerifyFinal - QFile sigFile(sigName); - sigFile.open(QIODevice::ReadOnly); - const int sig_len = sigFile.size(); - const auto* sig = [&sigFile, sig_len]() { - auto* buf = new unsigned char[sig_len]; - sigFile.read(reinterpret_cast(buf), sig_len); - return buf; - }(); - sigFile.close(); - - rc = EVP_DigestVerifyFinal(ctx, sig, sig_len); - delete sig; - - if(rc == 1) - return true; - else { - if(m_state.ignored) - return true; + { + std::ifstream signature(signature_path, std::ios::binary); + signature.unsetf(std::ios::skipws); + if(!signature.is_open()) { + m_sigError = tr("Cannot read signature during signature check"); + return false; + } + + const auto buffer_size = std::filesystem::file_size(signature_path); + std::vector buffer(buffer_size); + buffer.insert(buffer.begin(), std::istream_iterator(signature), std::istream_iterator()); + signature.close(); + + rc = EVP_DigestVerifyFinal(ctx, buffer.data(), buffer_size); + } + + if(rc != 1) { m_sigError = tr("Signature does not match"); return false; } + return true; } - diff --git a/lib/pluginloader/pluginloader.h b/lib/pluginloader/pluginloader.h index b602f5b..703c285 100644 --- a/lib/pluginloader/pluginloader.h +++ b/lib/pluginloader/pluginloader.h @@ -1,7 +1,7 @@ /* * This file is part of smolbote. It's copyrighted by the contributors recorded * in the version control history of the file, available from its original - * location: https://neueland.iserlohn-fortress.net/gitea/aqua/smolbote + * location: https://library.iserlohn-fortress.net/aqua/smolbote.git * * SPDX-License-Identifier: GPL-3.0 */ @@ -10,38 +10,41 @@ class PluginLoader : public QPluginLoader { - Q_OBJECT - public: - - struct SignatureState { - bool ignored; // always ignore signature - bool checked; // check if available - bool enforced; // always check - - SignatureState(bool ignore, bool check, bool enforce) { - ignored = ignore; - checked = check; - enforced = enforce; - } + enum signature_level { + SigIgnored = 1, + SigChecked = (1 << 1), + SigEnforced = (1 << 2), }; + typedef unsigned int signature_state_t; + static signature_state_t signature_state(bool ignore, bool check, bool enforce) + { + return (static_cast(enforce) << 2) | (static_cast(check) << 1) | static_cast(ignore); + } - PluginLoader(const QString &fileName, SignatureState state, QObject *parent = nullptr); - ~PluginLoader() = default; + PluginLoader(const QString &fileName, const signature_state_t state, QObject *parent = nullptr) + : QPluginLoader(fileName, parent) + , m_state(state) + { + } + ~PluginLoader() override = default; + + PluginLoader(const PluginLoader &) = delete; + PluginLoader &operator=(const PluginLoader &) = delete; + PluginLoader(PluginLoader &&) = delete; + PluginLoader &operator=(PluginLoader &&) = delete; - QString errorString() const + [[nodiscard]] QString errorString() const { - if(!m_sigError.isEmpty()) + if(!m_sigError.isEmpty()) { return m_sigError; - else - return QPluginLoader::errorString(); + } + return QPluginLoader::errorString(); } bool verify(const char *hashName = "SHA256"); private: - const SignatureState m_state; - + const int m_state; QString m_sigError; }; - diff --git a/lib/pluginloader/test/pluginloader-load.cpp b/lib/pluginloader/test/pluginloader-load.cpp new file mode 100644 index 0000000..9c329a3 --- /dev/null +++ b/lib/pluginloader/test/pluginloader-load.cpp @@ -0,0 +1,22 @@ +#include "pluginloader.h" +#include + +int main(int argc, char **argv) +{ + if(argc != 2) { + spdlog::error("usage: {} path/to/plugin.so", argv[0]); + return -1; + } + + const auto state = PluginLoader::signature_state(false, true, false); + PluginLoader loader(argv[1], state); + if(loader.load()) { + spdlog::info("Loaded plugin {}", argv[1]); + } else { + spdlog::error("Failed loading plugin {}", argv[1]); + spdlog::error(qUtf8Printable(loader.errorString())); + return -1; + } + + return 0; +} diff --git a/lib/pluginloader/test/pluginloader-sigmatch.cpp b/lib/pluginloader/test/pluginloader-sigmatch.cpp index 2e5a1ff..991d9bc 100644 --- a/lib/pluginloader/test/pluginloader-sigmatch.cpp +++ b/lib/pluginloader/test/pluginloader-sigmatch.cpp @@ -1,17 +1,97 @@ +#define CATCH_CONFIG_MAIN + #include "pluginloader.h" -#include +#include + +TEST_CASE("PluginLoader::signature_state") +{ + // ignore + REQUIRE(PluginLoader::signature_state(true, false, false) == PluginLoader::SigIgnored); + + // check + REQUIRE(PluginLoader::signature_state(false, true, false) >= PluginLoader::SigChecked); + REQUIRE(PluginLoader::signature_state(false, true, false) < PluginLoader::SigEnforced); + REQUIRE(PluginLoader::signature_state(true, true, false) >= PluginLoader::SigChecked); + REQUIRE(PluginLoader::signature_state(true, true, false) < PluginLoader::SigEnforced); + + // enfore + REQUIRE(PluginLoader::signature_state(false, false, true) >= PluginLoader::SigEnforced); + REQUIRE(PluginLoader::signature_state(true, false, true) >= PluginLoader::SigEnforced); + REQUIRE(PluginLoader::signature_state(false, true, true) >= PluginLoader::SigEnforced); + REQUIRE(PluginLoader::signature_state(true, true, true) >= PluginLoader::SigEnforced); +} + +TEST_CASE("files") +{ + REQUIRE(qEnvironmentVariableIsSet("UNSIGNEDFILE")); + REQUIRE(qEnvironmentVariableIsSet("SIGNEDFILE")); + REQUIRE(qEnvironmentVariableIsSet("BADSIGNEDFILE")); +} + +TEST_CASE("PluginLoader::verify missing plugin") +{ + const auto state = PluginLoader::signature_state(false, false, false); + PluginLoader loader("", state); + + REQUIRE_FALSE(loader.verify()); + REQUIRE_FALSE(loader.errorString().isEmpty()); +} + +TEST_CASE("PluginLoader::verify signature ignored") +{ + const auto state = PluginLoader::signature_state(true, false, false); + PluginLoader loader(qgetenv("UNSIGNEDFILE"), state); + + REQUIRE(loader.verify()); +} -PluginLoader *loader = nullptr; +TEST_CASE("PluginLoader::verify signature checked [avialable]") +{ + const auto state = PluginLoader::signature_state(false, true, false); + PluginLoader loader(qgetenv("SIGNEDFILE"), state); + + REQUIRE(loader.verify()); +} + +TEST_CASE("PluginLoader::verify signature checked [missing]") +{ + const auto state = PluginLoader::signature_state(false, true, false); + PluginLoader loader(qgetenv("UNSIGNEDFILE"), state); + + REQUIRE(loader.verify()); +} + +TEST_CASE("PluginLoader::verify signature checked [bad]") +{ + const auto state = PluginLoader::signature_state(false, true, false); + PluginLoader loader(qgetenv("BADSIGNEDFILE"), state); + + REQUIRE_FALSE(loader.verify()); + REQUIRE_FALSE(loader.errorString().isEmpty()); +} + +TEST_CASE("PluginLoader::verify signature enforced [avialable]") +{ + const auto state = PluginLoader::signature_state(false, false, true); + PluginLoader loader(qgetenv("SIGNEDFILE"), state); + + REQUIRE(loader.verify()); +} + +TEST_CASE("PluginLoader::verify signature enforced [missing]") +{ + const auto state = PluginLoader::signature_state(false, false, true); + PluginLoader loader(qgetenv("UNSIGNEDFILE"), state); -TEST(PluginLoader, SignatureMatcher) { - EXPECT_TRUE(loader->verify()); + REQUIRE_FALSE(loader.verify()); + REQUIRE_FALSE(loader.errorString().isEmpty()); } -int main(int argc, char **argv) +TEST_CASE("PluginLoader::verify signature enforced [bad]") { - const PluginLoader::SignatureState state(false, true, false); - loader = new PluginLoader(qgetenv("SIGNEDFILE"), state); + const auto state = PluginLoader::signature_state(false, false, true); + PluginLoader loader(qgetenv("BADSIGNEDFILE"), state); - testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); + REQUIRE_FALSE(loader.verify()); + REQUIRE_FALSE(loader.errorString().isEmpty()); } diff --git a/plugins/ProfileEditor/meson.build b/plugins/ProfileEditor/meson.build index c44f5c4..caa3a75 100644 --- a/plugins/ProfileEditor/meson.build +++ b/plugins/ProfileEditor/meson.build @@ -11,3 +11,14 @@ ProfileEditorPlugin_lib = shared_library('ProfileEditorPlugin', include_directories: include, install: true, install_dir: join_paths(get_option('libdir'), 'smolbote/plugins') ) + +test('run', executable('ProfileEditor', 'test/main.cpp', + link_with: ProfileEditorPlugin, + include_directories: plugininterfaces_include, + dependencies: [ dep_qt5 ]), + args: [ '-platform', 'offscreen' ], + env: 'autoclose=1', + suite: 'ProfileEdtitorPlugin' +) + +test('load', poi_plugin_loader, suite: 'ProfileEditorPlugin', args: ProfileEditorPlugin.full_path()) diff --git a/src/browser.cpp b/src/browser.cpp index c55120d..61be3b0 100644 --- a/src/browser.cpp +++ b/src/browser.cpp @@ -122,7 +122,7 @@ void Browser::loadPlugins(const QStringList &paths, const std::function("plugins.signature.ignored").value(), conf.value("plugins.signature.checked").value(), conf.value("plugins.signature.enforced").value()); -- cgit v1.2.1