From c5fdf060c7840aeb1eb71b1d9ab66576ace27864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Blin?= Date: Fri, 2 Jul 2021 12:01:13 -0400 Subject: [PATCH] archiver: improve import from archive If the user decides to transmit its account archive via a webservice in some case the webserver can re-compress the file resulting to a gunzip file into a gunzip file. We can check the header of the file to be able to detect this case. Then, replace inflateInit by inflateInit2 with better header detection to avoid header errors. Cf http://www.zlib.net/manual.html#inflateInit2 Change-Id: I1421affa8815ab347b439ef2e505da10275c80ff GitLab: #575 --- src/archiver.cpp | 10 +- src/archiver.h | 1 + src/fileutils.cpp | 55 ++++-- test/unitTest/Makefile.am | 6 + .../account_archive/account_archive.cpp | 180 ++++++++++++++++++ 5 files changed, 238 insertions(+), 14 deletions(-) create mode 100644 test/unitTest/account_archive/account_archive.cpp diff --git a/src/archiver.cpp b/src/archiver.cpp index 20a24975a..a916c6381 100644 --- a/src/archiver.cpp +++ b/src/archiver.cpp @@ -144,6 +144,14 @@ compressGzip(const std::string& str, const std::string& path) gzclose(fi); } +void +compressGzip(const std::vector& dat, const std::string& path) +{ + auto fi = openGzip(path, "wb"); + gzwrite(fi, dat.data(), dat.size()); + gzclose(fi); +} + std::vector decompressGzip(const std::string& path) { @@ -169,7 +177,7 @@ decompress(const std::vector& str) z_stream zs; // z_stream is zlib's control structure memset(&zs, 0, sizeof(zs)); - if (inflateInit(&zs) != Z_OK) + if (inflateInit2(&zs, 32+MAX_WBITS) != Z_OK) throw std::runtime_error("inflateInit failed while decompressing."); zs.next_in = (Bytef*) str.data(); diff --git a/src/archiver.h b/src/archiver.h index 5e84d25b4..5fa78a2d0 100644 --- a/src/archiver.h +++ b/src/archiver.h @@ -57,6 +57,7 @@ std::vector decompress(const std::vector& dat); * Compress string to a Gzip file */ void compressGzip(const std::string& str, const std::string& path); +void compressGzip(const std::vector& dat, const std::string& path); /** * Decompress Gzip file to bytes diff --git a/src/fileutils.cpp b/src/fileutils.cpp index 7ea786a2e..aebe0d9d7 100644 --- a/src/fileutils.cpp +++ b/src/fileutils.cpp @@ -535,24 +535,53 @@ readArchive(const std::string& path, const std::string& pwd) { JAMI_DBG("Reading archive from %s", path.c_str()); - std::vector data; - if (pwd.empty()) { - data = archiver::decompressGzip(path); - } else { - // Read file + auto isUnencryptedGzip = [](const std::vector& data) { + // NOTE: some webserver modify gzip files and this can end with a gunzip in a gunzip + // file. So, to make the readArchive more robust, we can support this case by detecting + // gzip header via 1f8b 08 + // We don't need to support more than 2 level, else somebody may be able to send + // gunzip in loops and abuse. + return data.size() > 3 && data[0] == 0x1f && data[1] == 0x8b && data[2] == 0x08; + }; + + auto decompress = [](std::vector& data) { try { - data = loadFile(path); - } catch (const std::exception& e) { - JAMI_ERR("Error loading archive: %s", e.what()); - throw e; - } - // Decrypt - try { - data = archiver::decompress(dht::crypto::aesDecrypt(data, pwd)); + data = archiver::decompress(data); } catch (const std::exception& e) { JAMI_ERR("Error decrypting archive: %s", e.what()); throw e; } + }; + + std::vector data; + // Read file + try { + data = loadFile(path); + } catch (const std::exception& e) { + JAMI_ERR("Error loading archive: %s", e.what()); + throw e; + } + + + if (isUnencryptedGzip(data)) { + if (!pwd.empty()) + JAMI_WARN() << "A gunzip in a gunzip is detected. A webserver may have a bad config"; + + decompress(data); + } + + if (!pwd.empty()) { + // Decrypt + try { + data = dht::crypto::aesDecrypt(data, pwd); + } catch (const std::exception& e) { + JAMI_ERR("Error decrypting archive: %s", e.what()); + throw e; + } + decompress(data); + } else if (isUnencryptedGzip(data)) { + JAMI_WARN() << "A gunzip in a gunzip is detected. A webserver may have a bad config"; + decompress(data); } return data; } diff --git a/test/unitTest/Makefile.am b/test/unitTest/Makefile.am index 95940fdc0..d7e0a9fb7 100644 --- a/test/unitTest/Makefile.am +++ b/test/unitTest/Makefile.am @@ -14,6 +14,12 @@ check_PROGRAMS = check_PROGRAMS += ut_account_factory ut_account_factory_SOURCES = account_factory/testAccount_factory.cpp common.cpp +# +# account_archive +# +check_PROGRAMS += ut_account_archive +ut_account_archive_SOURCES = account_archive/account_archive.cpp common.cpp + # # certstore # diff --git a/test/unitTest/account_archive/account_archive.cpp b/test/unitTest/account_archive/account_archive.cpp new file mode 100644 index 000000000..845761e14 --- /dev/null +++ b/test/unitTest/account_archive/account_archive.cpp @@ -0,0 +1,180 @@ +/* + * Copyright (C) 2021 Savoir-faire Linux Inc. + * Author: Sébastien Blin + * + * 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 3 of the License, or + * (at your option) any later version. + * + * 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 . + */ + +#include +#include +#include + +#include +#include +#include +#include + +#include "manager.h" +#include "jamidht/conversationrepository.h" +#include "jamidht/connectionmanager.h" +#include "jamidht/gitserver.h" +#include "jamidht/jamiaccount.h" +#include "../../test_runner.h" +#include "archiver.h" +#include "base64.h" +#include "dring.h" +#include "fileutils.h" +#include "account_const.h" +#include "common.h" + +#include +#include + +using namespace std::string_literals; +using namespace DRing::Account; + +namespace jami { +namespace test { + +class AccountArchiveTest : public CppUnit::TestFixture +{ +public: + AccountArchiveTest() + { + // Init daemon + DRing::init(DRing::InitFlag(DRing::DRING_FLAG_DEBUG | DRing::DRING_FLAG_CONSOLE_LOG)); + if (not Manager::instance().initialized) + CPPUNIT_ASSERT(DRing::start("dring-sample.yml")); + } + ~AccountArchiveTest() { DRing::fini(); } + static std::string name() { return "AccountArchive"; } + void setUp(); + void tearDown(); + + std::string aliceId; + std::string bobId; + +private: + void testExportImportNoPassword(); + void testExportImportNoPasswordDoubleGunzip(); + void testExportImportPassword(); + void testExportImportPasswordDoubleGunzip(); + + CPPUNIT_TEST_SUITE(AccountArchiveTest); + CPPUNIT_TEST(testExportImportNoPassword); + CPPUNIT_TEST(testExportImportNoPasswordDoubleGunzip); + CPPUNIT_TEST(testExportImportPassword); + CPPUNIT_TEST(testExportImportPasswordDoubleGunzip); + CPPUNIT_TEST_SUITE_END(); +}; + +CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(AccountArchiveTest, + AccountArchiveTest::name()); + +void +AccountArchiveTest::setUp() +{ + auto actors = load_actors_and_wait_for_announcement("actors/alice-bob-password.yml"); + aliceId = actors["alice"]; + bobId = actors["bob"]; +} + +void +AccountArchiveTest::tearDown() +{ + wait_for_removal_of({aliceId, bobId}); +} + +void +AccountArchiveTest::testExportImportNoPassword() +{ + auto aliceAccount = Manager::instance().getAccount(aliceId); + + CPPUNIT_ASSERT(aliceAccount->exportArchive("test.gz")); + + std::map details = DRing::getAccountTemplate("RING"); + details[ConfProperties::ARCHIVE_PATH] = "test.gz"; + + auto accountId = jami::Manager::instance().addAccount(details); + wait_for_announcement_of(accountId); + auto alice2Account = Manager::instance().getAccount(accountId); + CPPUNIT_ASSERT(alice2Account->getUsername() == aliceAccount->getUsername()); + std::remove("test.gz"); + wait_for_removal_of(accountId); +} + +void +AccountArchiveTest::testExportImportNoPasswordDoubleGunzip() +{ + auto aliceAccount = Manager::instance().getAccount(aliceId); + + CPPUNIT_ASSERT(aliceAccount->exportArchive("test.gz")); + auto dat = fileutils::loadFile("test.gz"); + archiver::compressGzip(dat, "test.gz"); + + std::map details = DRing::getAccountTemplate("RING"); + details[ConfProperties::ARCHIVE_PATH] = "test.gz"; + + auto accountId = jami::Manager::instance().addAccount(details); + wait_for_announcement_of(accountId); + auto alice2Account = Manager::instance().getAccount(accountId); + CPPUNIT_ASSERT(alice2Account->getUsername() == aliceAccount->getUsername()); + std::remove("test.gz"); + wait_for_removal_of(accountId); +} + +void +AccountArchiveTest::testExportImportPassword() +{ + auto bobAccount = Manager::instance().getAccount(bobId); + + CPPUNIT_ASSERT(bobAccount->exportArchive("test.gz", "test")); + + std::map details = DRing::getAccountTemplate("RING"); + details[ConfProperties::ARCHIVE_PATH] = "test.gz"; + details[ConfProperties::ARCHIVE_PASSWORD] = "test"; + + auto accountId = jami::Manager::instance().addAccount(details); + wait_for_announcement_of(accountId); + auto bob2Account = Manager::instance().getAccount(accountId); + CPPUNIT_ASSERT(bob2Account->getUsername() == bobAccount->getUsername()); + std::remove("test.gz"); + wait_for_removal_of(accountId); +} + +void +AccountArchiveTest::testExportImportPasswordDoubleGunzip() +{ + auto bobAccount = Manager::instance().getAccount(bobId); + + CPPUNIT_ASSERT(bobAccount->exportArchive("test.gz", "test")); + auto dat = fileutils::loadFile("test.gz"); + archiver::compressGzip(dat, "test.gz"); + + std::map details = DRing::getAccountTemplate("RING"); + details[ConfProperties::ARCHIVE_PATH] = "test.gz"; + details[ConfProperties::ARCHIVE_PASSWORD] = "test"; + + auto accountId = jami::Manager::instance().addAccount(details); + wait_for_announcement_of(accountId); + auto bob2Account = Manager::instance().getAccount(accountId); + CPPUNIT_ASSERT(bob2Account->getUsername() == bobAccount->getUsername()); + std::remove("test.gz"); + wait_for_removal_of(accountId); +} + +} // namespace test +} // namespace jami + +RING_TEST_RUNNER(jami::test::AccountArchiveTest::name())