From 25ab00796435f45c492d7883f0ccf56f2fea7879 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Fri, 23 Jan 2026 16:42:05 +0200 Subject: [PATCH 1/7] Tar and workflow fixes and tests Signed-off-by: Lauris Kaplinski --- cdoc/CDoc.h | 9 ++ cdoc/CDoc2Reader.cpp | 80 ++++++++---- cdoc/CDoc2Writer.cpp | 42 +++++- cdoc/CDoc2Writer.h | 1 + cdoc/CryptoBackend.cpp | 4 +- cdoc/Io.h | 2 +- cdoc/Tar.cpp | 40 ++++-- test/libcdoc_boost.cpp | 289 ++++++++++++++++++++++++++++++++--------- 8 files changed, 367 insertions(+), 100 deletions(-) diff --git a/cdoc/CDoc.h b/cdoc/CDoc.h index 3ebe4e2f..7bad17c7 100644 --- a/cdoc/CDoc.h +++ b/cdoc/CDoc.h @@ -57,10 +57,16 @@ enum { NOT_SUPPORTED = -101, /** * @brief Conflicting or invalid arguments for a method + * + * This does not set CDocReader/CDocWriter into error state - so invoking subsequent methods + * with correct arguments will succeed */ WRONG_ARGUMENTS = -102, /** * @brief Components of multi-method workflow are called in wrong order + * + * This does not set CDocReader/CDocWriter into error state - so invoking subsequent methods + * in correct order will succeed */ WORKFLOW_ERROR = -103, /** @@ -85,6 +91,9 @@ enum { INPUT_STREAM_ERROR = -108, /** * @brief The supplied decryption key is wrong + * + * This does not set CDocReader/CDocWriter into error state - so invoking subsequent methods + * with correct key will succeed */ WRONG_KEY = -109, /** diff --git a/cdoc/CDoc2Reader.cpp b/cdoc/CDoc2Reader.cpp index 0f301ce6..1389c749 100644 --- a/cdoc/CDoc2Reader.cpp +++ b/cdoc/CDoc2Reader.cpp @@ -87,6 +87,20 @@ struct CDoc2Reader::Private { std::unique_ptr dec; std::unique_ptr zsrc; std::unique_ptr tar; + + result_t decryptAllAndClose() { + std::array buf; + result_t rv = dec->read(buf.data(), buf.size()); + while (rv == buf.size()) { + rv = dec->read(buf.data(), buf.size()); + } + if (rv < 0) return rv; + zsrc.reset(); + tar.reset(); + rv = dec->close(); + dec.reset(); + return rv; + } }; CDoc2Reader::~CDoc2Reader() @@ -118,35 +132,44 @@ CDoc2Reader::getLockForCert(const std::vector& cert){ libcdoc::result_t CDoc2Reader::getFMK(std::vector& fmk, unsigned int lock_idx) { + if (lock_idx >= priv->locks.size()) { + setLastError(t_("Invalid lock index")); + LOG_ERROR("{}", last_error); + return libcdoc::WRONG_ARGUMENTS; + } LOG_DBG("CDoc2Reader::getFMK: {}", lock_idx); LOG_DBG("CDoc2Reader::num locks: {}", priv->locks.size()); const Lock& lock = priv->locks.at(lock_idx); + LOG_DBG("Label: {}", lock.label); std::vector kek; if (lock.type == Lock::Type::PASSWORD) { // Password LOG_DBG("password"); std::string info_str = libcdoc::CDoc2::getSaltForExpand(lock.label); + LOG_DBG("info: {}", toHex(info_str)); std::vector kek_pm; - crypto->extractHKDF(kek_pm, lock.getBytes(Lock::SALT), lock.getBytes(Lock::PW_SALT), lock.getInt(Lock::KDF_ITER), lock_idx); - LOG_DBG("password2"); + if (auto rv = crypto->extractHKDF(kek_pm, lock.getBytes(Lock::SALT), lock.getBytes(Lock::PW_SALT), lock.getInt(Lock::KDF_ITER), lock_idx); rv != libcdoc::OK) { + setLastError(crypto->getLastErrorStr(rv)); + LOG_ERROR("{}", last_error); + return rv; + } + LOG_TRACE_KEY("salt: {}", lock.getBytes(Lock::SALT)); + LOG_TRACE_KEY("kek_pm: {}", kek_pm); kek = libcdoc::Crypto::expand(kek_pm, info_str, 32); - if (kek.empty()) return libcdoc::CRYPTO_ERROR; - LOG_DBG("password3"); } else if (lock.type == Lock::Type::SYMMETRIC_KEY) { // Symmetric key LOG_DBG("symmetric"); std::string info_str = libcdoc::CDoc2::getSaltForExpand(lock.label); - std::vector kek_pm; - crypto->extractHKDF(kek_pm, lock.getBytes(Lock::SALT), {}, 0, lock_idx); - kek = libcdoc::Crypto::expand(kek_pm, info_str, 32); - - LOG_DBG("Label: {}", lock.label); LOG_DBG("info: {}", toHex(info_str)); + std::vector kek_pm; + if (auto rv = crypto->extractHKDF(kek_pm, lock.getBytes(Lock::SALT), {}, 0, lock_idx); rv != libcdoc::OK) { + setLastError(crypto->getLastErrorStr(rv)); + LOG_ERROR("{}", last_error); + return rv; + } LOG_TRACE_KEY("salt: {}", lock.getBytes(Lock::SALT)); LOG_TRACE_KEY("kek_pm: {}", kek_pm); - LOG_TRACE_KEY("kek: {}", kek); - - if (kek.empty()) return libcdoc::CRYPTO_ERROR; + kek = libcdoc::Crypto::expand(kek_pm, info_str, 32); } else if ((lock.type == Lock::Type::PUBLIC_KEY) || (lock.type == Lock::Type::SERVER)) { // Public/private key std::vector key_material; @@ -196,13 +219,9 @@ CDoc2Reader::getFMK(std::vector& fmk, unsigned int lock_idx) LOG_ERROR("{}", last_error); return result; } - LOG_TRACE_KEY("Key kekPm: {}", kek_pm); - std::string info_str = libcdoc::CDoc2::getSaltForExpand(key_material, lock.getBytes(Lock::Params::RCPT_KEY)); - LOG_DBG("info: {}", toHex(info_str)); - kek = libcdoc::Crypto::expand(kek_pm, info_str, libcdoc::CDoc2::KEY_LEN); } } else if (lock.type == Lock::Type::SHARE_SERVER) { @@ -312,7 +331,6 @@ CDoc2Reader::getFMK(std::vector& fmk, unsigned int lock_idx) LOG_TRACE_KEY("KEK: {}", kek); - if(kek.empty()) { setLastError(t_("Failed to derive KEK")); LOG_ERROR("{}", last_error); @@ -394,10 +412,10 @@ CDoc2Reader::beginDecryption(const std::vector& fmk) std::vector aad(libcdoc::CDoc2::PAYLOAD.cbegin(), libcdoc::CDoc2::PAYLOAD.cend()); aad.insert(aad.end(), priv->header_data.cbegin(), priv->header_data.cend()); aad.insert(aad.end(), priv->headerHMAC.cbegin(), priv->headerHMAC.cend()); - if(priv->dec->updateAAD(aad) != OK) { - setLastError("Wrong decryption key (FMK)"); + if(auto rv = priv->dec->updateAAD(aad); rv != OK) { + setLastError(priv->dec->getLastErrorStr(rv)); LOG_ERROR("{}", last_error); - return libcdoc::WRONG_KEY; + return rv; } priv->zsrc = std::make_unique(priv->dec.get(), false); @@ -414,8 +432,13 @@ CDoc2Reader::nextFile(std::string& name, int64_t& size) LOG_ERROR("{}", last_error); return libcdoc::WORKFLOW_ERROR; } - result_t result = priv->tar->next(name, size); - if (result != OK) { + result_t result = priv->tar->next(name, size); + if (result < 0) { + result_t sr = priv->decryptAllAndClose(); + if (sr != OK) { + setLastError("Crypto payload integrity check failed"); + return sr; + } setLastError(priv->tar->getLastErrorStr(result)); } return result; @@ -430,7 +453,12 @@ CDoc2Reader::readData(uint8_t *dst, size_t size) return libcdoc::WORKFLOW_ERROR; } result_t result = priv->tar->read(dst, size); - if (result != OK) { + if (result < 0) { + result_t sr = priv->decryptAllAndClose(); + if (sr != OK) { + setLastError("Crypto payload integrity check failed"); + return sr; + } setLastError(priv->tar->getLastErrorStr(result)); } return result; @@ -439,11 +467,15 @@ CDoc2Reader::readData(uint8_t *dst, size_t size) libcdoc::result_t CDoc2Reader::finishDecryption() { + if (!priv->tar) { + setLastError("finishDecryption() called before beginDecryption()"); + LOG_ERROR("{}", last_error); + return libcdoc::WORKFLOW_ERROR; + } if (!priv->zsrc->isEof()) { setLastError(t_("CDoc contains additional payload data that is not part of content")); LOG_WARN("{}", last_error); } - setLastError({}); priv->zsrc.reset(); priv->tar.reset(); diff --git a/cdoc/CDoc2Writer.cpp b/cdoc/CDoc2Writer.cpp index 66942284..ba7f886e 100644 --- a/cdoc/CDoc2Writer.cpp +++ b/cdoc/CDoc2Writer.cpp @@ -457,6 +457,11 @@ CDoc2Writer::buildHeader(std::vector& header, const std::vector 8ULL * 1024 * 1024 * 1024) { + setLastError("Invalid file size"); + LOG_ERROR("{}", last_error); + return libcdoc::WRONG_ARGUMENTS; + } if(auto rv = tar->open(name, size); rv < 0) { setLastError(tar->getLastErrorStr(rv)); LOG_ERROR("{}", last_error); @@ -505,6 +529,11 @@ CDoc2Writer::addFile(const std::string& name, size_t size) libcdoc::result_t CDoc2Writer::writeData(const uint8_t *src, size_t size) { + if (finished) { + setLastError("Encryption finished"); + LOG_ERROR("{}", last_error); + return libcdoc::WORKFLOW_ERROR; + } if(!tar) { setLastError("No file added"); LOG_ERROR("{}", last_error); @@ -520,6 +549,11 @@ CDoc2Writer::writeData(const uint8_t *src, size_t size) libcdoc::result_t CDoc2Writer::finishEncryption() { + if (finished) { + setLastError("Encryption finished"); + LOG_ERROR("{}", last_error); + return libcdoc::WORKFLOW_ERROR; + } if(!tar) { setLastError("No file added"); LOG_ERROR("{}", last_error); @@ -531,12 +565,18 @@ CDoc2Writer::finishEncryption() tar.reset(); recipients.clear(); if (owned) dst->close(); + finished = true; return rv; } libcdoc::result_t CDoc2Writer::encrypt(libcdoc::MultiDataSource& src, const std::vector& keys) { + if (finished) { + setLastError("Encryption finished"); + LOG_ERROR("{}", last_error); + return libcdoc::WORKFLOW_ERROR; + } for (auto rcpt : keys) { if(auto rv = addRecipient(rcpt); rv != libcdoc::OK) return rv; diff --git a/cdoc/CDoc2Writer.h b/cdoc/CDoc2Writer.h index f68acea2..3bb5800a 100644 --- a/cdoc/CDoc2Writer.h +++ b/cdoc/CDoc2Writer.h @@ -46,6 +46,7 @@ class CDoc2Writer final: public libcdoc::CDocWriter { std::unique_ptr tar; std::vector recipients; + bool finished = false; }; } diff --git a/cdoc/CryptoBackend.cpp b/cdoc/CryptoBackend.cpp index b0c10459..8c0f1652 100644 --- a/cdoc/CryptoBackend.cpp +++ b/cdoc/CryptoBackend.cpp @@ -82,7 +82,7 @@ CryptoBackend::getKeyMaterial(std::vector& key_material, const std::vec if (pw_salt.empty()) return INVALID_PARAMS; std::vector secret; int result = getSecret(secret, idx); - if (result < 0) return result; + if (result) return result; LOG_DBG("Secret: {}", toHex(secret)); @@ -91,7 +91,7 @@ CryptoBackend::getKeyMaterial(std::vector& key_material, const std::vec if (key_material.empty()) return OPENSSL_ERROR; } else { int result = getSecret(key_material, idx); - if (result < 0) return result; + if (result) return result; LOG_DBG("Secret: {}", toHex(key_material)); if (key_material.size() != 32) { return INVALID_PARAMS; diff --git a/cdoc/Io.h b/cdoc/Io.h index cdf9fd5c..45902101 100644 --- a/cdoc/Io.h +++ b/cdoc/Io.h @@ -255,7 +255,7 @@ struct CDOC_EXPORT IStreamSource : public DataSource { if (_owned) delete _ifs; } - result_t seek(size_t pos) { + result_t seek(size_t pos) override { if(_ifs->bad()) return INPUT_STREAM_ERROR; _ifs->clear(); _ifs->seekg(pos); diff --git a/cdoc/Tar.cpp b/cdoc/Tar.cpp index e1f79417..c4775fdb 100644 --- a/cdoc/Tar.cpp +++ b/cdoc/Tar.cpp @@ -130,6 +130,10 @@ libcdoc::TarConsumer::~TarConsumer() libcdoc::result_t libcdoc::TarConsumer::write(const uint8_t *src, size_t size) noexcept { + if ((_current_size >= 0) && ((_current_written + size) > _current_size)) { + return WORKFLOW_ERROR; + } + _current_written += size; return _dst->write(src, size); } @@ -160,19 +164,25 @@ libcdoc::TarConsumer::writePadding(int64_t size) noexcept { libcdoc::result_t libcdoc::TarConsumer::close() noexcept { - if (_current_size > 0) { - if(auto rv = writePadding(_current_size); rv != OK) - return rv; - } - Header empty = {}; - if(auto rv = writeHeader(empty); rv != OK) - return rv; - if(auto rv = writeHeader(empty); rv != OK) - return rv; + result_t result = OK; + if ((_current_size >= 0) && (_current_written < _current_size)) { + result = DATA_FORMAT_ERROR; + } else { + if (_current_written > 0) { + if(auto rv = writePadding(_current_written); rv != OK) + return rv; + } + Header empty = {}; + if(auto rv = writeHeader(empty); rv != OK) + return rv; + if(auto rv = writeHeader(empty); rv != OK) + return rv; + } if (_owned) { - return _dst->close(); + if (auto rv = _dst->close(); rv != OK) + return rv; } - return OK; + return result; } bool @@ -184,12 +194,16 @@ libcdoc::TarConsumer::isError() noexcept libcdoc::result_t libcdoc::TarConsumer::open(const std::string& name, int64_t size) { - if (_current_size > 0) { - if(auto rv = writePadding(_current_size); rv != OK) + if ((_current_size >= 0) && (_current_written < _current_size)) { + return WORKFLOW_ERROR; + } + if (_current_written > 0) { + if(auto rv = writePadding(_current_written); rv != OK) return rv; } _current_size = size; + _current_written = 0; Header h {}; size_t len = std::min(name.size(), h.name.size()); std::copy_n(name.cbegin(), len, h.name.begin()); diff --git a/test/libcdoc_boost.cpp b/test/libcdoc_boost.cpp index 39035652..3b48536e 100644 --- a/test/libcdoc_boost.cpp +++ b/test/libcdoc_boost.cpp @@ -344,74 +344,173 @@ gen_random_filename() return utf16_to_utf8(u16); } -BOOST_AUTO_TEST_SUITE(LargeFiles) - -BOOST_FIXTURE_TEST_CASE_WITH_DECOR(EncryptWithPasswordAndLabel, FixtureBase, * utf::description("Testing weird and large files")) -{ - std::srand(1); +// CDoc2 password and label - std::vector data; - bool eof = false; - PipeConsumer pipec(data, eof); - PipeSource pipes(data, eof); - PipeCrypto pcrypto("password"); +struct TestCrypto : public libcdoc::CryptoBackend { + std::string_view password; - // Create writer - libcdoc::CDocWriter *writer = libcdoc::CDocWriter::createWriter(2, &pipec, false, nullptr, &pcrypto, nullptr); - BOOST_TEST(writer != nullptr); - libcdoc::Recipient rcpt = libcdoc::Recipient::makeSymmetric("test", 65536); - BOOST_TEST(writer->addRecipient(rcpt) == libcdoc::OK); - BOOST_TEST(writer->beginEncryption() == libcdoc::OK); + libcdoc::result_t getSecret(std::vector& dst, unsigned int idx) override final { + // Mark empty password with bogus error to detect it + if(password.empty()) return libcdoc::WRONG_ARGUMENTS; + dst.assign(password.cbegin(), password.cend()); + return libcdoc::OK; + }; +}; - // List of files: 0, 0, max_size...0 - std::vector files; - files.emplace_back(gen_random_filename(), 0); - files.emplace_back(gen_random_filename(), 0); - for (size_t size = max_filesize; size != 0; size = size / 100) { - files.emplace_back(gen_random_filename(), size); - } - files.emplace_back(gen_random_filename(), 0); +BOOST_AUTO_TEST_SUITE(CDoc2Errors) +BOOST_FIXTURE_TEST_CASE_WITH_DECOR(CDoc2EncryptErrors, EncryptFixture, + * utf::description("Cause various encryption errors")) +{ + std::string container = formTargetFile("CDoc2Errors.cdoc"); + uint8_t test_data[256]; - PipeWriter wrt(writer, files); + libcdoc::ToolConf conf; + TestCrypto crypto; - // Create reader - libcdoc::CDocReader *reader = libcdoc::CDocReader::createReader(&pipes, false, nullptr, &pcrypto, nullptr); - BOOST_TEST(reader != nullptr); + srand(0); + // Create writer + libcdoc::CDocWriter *wrt = libcdoc::CDocWriter::createWriter(2, container, &conf, &crypto, nullptr); + BOOST_TEST(wrt != nullptr, "Cannot create writer"); + // Nothing can be done until at least one recipient is added + BOOST_TEST(wrt->beginEncryption() == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->addFile("testfile", 1024) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->finishEncryption() == libcdoc::WORKFLOW_ERROR); + + // Add recipient + libcdoc::Recipient rcpt = libcdoc::Recipient::makeSymmetric("test-recipient", 65536); + BOOST_TEST(wrt->addRecipient(rcpt) == libcdoc::OK); + // Encryption cannot proceed before beginEncryption is called + BOOST_TEST(wrt->addFile("testfile", 1024) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->finishEncryption() == libcdoc::WORKFLOW_ERROR); + + // Begin encryption + BOOST_TEST(wrt->beginEncryption() == libcdoc::WRONG_ARGUMENTS); + crypto.password = "test-password"; + BOOST_TEST(wrt->beginEncryption() == libcdoc::OK); + // Cannot do anything else than add files + BOOST_TEST(wrt->addRecipient(rcpt) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->beginEncryption() == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::WORKFLOW_ERROR); + // Finish encryption will succeed with empty tar + + // Add file + BOOST_TEST(wrt->addFile("testfile", 1024) == libcdoc::OK); + // Errors + BOOST_TEST(wrt->addRecipient(rcpt) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->beginEncryption() == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->addFile("testfile", 1024) == libcdoc::WORKFLOW_ERROR); + + // Write data + for (int i = 0; i < 256; i++) test_data[i] = uint8_t(rand() & 0xff); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::OK); + BOOST_TEST(wrt->addRecipient(rcpt) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->beginEncryption() == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->addFile("testfile", 1024) == libcdoc::WORKFLOW_ERROR); + for (int i = 0; i < 256; i++) test_data[i] = uint8_t(rand() & 0xff); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::OK); + for (int i = 0; i < 256; i++) test_data[i] = uint8_t(rand() & 0xff); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::OK); + for (int i = 0; i < 256; i++) test_data[i] = uint8_t(rand() & 0xff); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::OK); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->addRecipient(rcpt) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->beginEncryption() == libcdoc::WORKFLOW_ERROR); + // Add file with unknown size + BOOST_TEST(wrt->addFile("testfile2", 10000000000ULL) == libcdoc::WRONG_ARGUMENTS); + BOOST_TEST(wrt->addFile("testfile2", 255) == libcdoc::OK); + for (int i = 0; i < 256; i++) test_data[i] = uint8_t(rand() & 0xff); + BOOST_TEST(wrt->writeData(test_data, 255) == libcdoc::OK); + BOOST_TEST(wrt->addRecipient(rcpt) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->beginEncryption() == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->finishEncryption() == libcdoc::OK); + + BOOST_TEST(wrt->addRecipient(rcpt) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->beginEncryption() == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->addFile("testfile", 1024) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->writeData(test_data, 256) == libcdoc::WORKFLOW_ERROR); + BOOST_TEST(wrt->finishEncryption() == libcdoc::WORKFLOW_ERROR); + + delete wrt; +} - // Fill buffer - while((data.size() < 2 * wrt.BUFSIZE) && !wrt.isEof()) { - BOOST_TEST(wrt.writeMore() == libcdoc::OK); - } - std::vector fmk; - BOOST_TEST(reader->getFMK(fmk, 0) == libcdoc::OK); - BOOST_TEST(reader->beginDecryption(fmk) == libcdoc::OK); +BOOST_FIXTURE_TEST_CASE_WITH_DECOR(CDoc2DecryptErrors, DecryptFixture, + * utf::depends_on("CDoc2Errors/CDoc2EncryptErrors") + * utf::description("Cause various decryption errors")) +{ + std::string container = checkTargetFile("CDoc2Errors.cdoc"); + libcdoc::ToolConf conf; + TestCrypto crypto; + uint8_t buf[1024]; + + libcdoc::CDocReader *rdr = libcdoc::CDocReader::createReader(container, &conf, &crypto, nullptr); + BOOST_TEST(rdr != nullptr, "Cannot create reader"); + std::vector fmk(32); + BOOST_TEST(rdr->getFMK(fmk, 10) == libcdoc::WRONG_ARGUMENTS); + // Decryption should start with random key + BOOST_TEST(rdr->beginDecryption(fmk) == libcdoc::OK); libcdoc::FileInfo fi; - for (int cfile = 0; cfile < files.size(); cfile++) { - // Fill buffer - while((data.size() < 2 * wrt.BUFSIZE) && !wrt.isEof()) { - BOOST_TEST(wrt.writeMore() == libcdoc::OK); - } - // Get file - BOOST_TEST(reader->nextFile(fi) == libcdoc::OK); - BOOST_TEST(fi.name == files[cfile].name); - BOOST_TEST(fi.size == files[cfile].size); - for (size_t pos = 0; pos < files[cfile].size; pos += wrt.BUFSIZE) { - // Fill buffer - while((data.size() < 2 * wrt.BUFSIZE) && !wrt.isEof()) { - BOOST_TEST(wrt.writeMore() == libcdoc::OK); - } - size_t toread = files[cfile].size - pos; - if (toread > wrt.BUFSIZE) toread = wrt.BUFSIZE; - uint8_t buf[wrt.BUFSIZE], cbuf[wrt.BUFSIZE]; - BOOST_TEST(reader->readData(buf, toread) == toread); - for (size_t i = 0; i < toread; i++) cbuf[i] = wrt.getChar(cfile, pos + i); - BOOST_TEST(std::memcmp(buf, cbuf, toread) == 0); - } + // But the first file should file + BOOST_TEST(rdr->nextFile(fi) != libcdoc::OK); + delete rdr; + + rdr = libcdoc::CDocReader::createReader(container, &conf, &crypto, nullptr); + BOOST_TEST(rdr != nullptr, "Cannot create reader"); + BOOST_TEST(rdr->getFMK(fmk, 0) == libcdoc::WRONG_ARGUMENTS); + crypto.password = "wrong-password"; + BOOST_TEST(rdr->getFMK(fmk, 0) == libcdoc::WRONG_KEY); + crypto.password = "test-password"; + BOOST_TEST(rdr->getFMK(fmk, 0) == libcdoc::OK); + BOOST_TEST(rdr->beginDecryption(fmk) == libcdoc::OK); + BOOST_TEST(rdr->nextFile(fi) == libcdoc::OK); + BOOST_TEST(fi.size == 1024); + BOOST_TEST(rdr->readData(buf, 256) == 256); + BOOST_TEST(rdr->readData(buf, 256) == 256); + BOOST_TEST(rdr->readData(buf, 256) == 256); + BOOST_TEST(rdr->readData(buf, 1024) == 256); + BOOST_TEST(rdr->nextFile(fi) == libcdoc::OK); + BOOST_TEST(fi.size == 255); + BOOST_TEST(rdr->readData(buf, 1024) == 255); + BOOST_TEST(rdr->finishDecryption() == libcdoc::OK); + delete rdr; + + // Write over the end of file + size_t fsize = std::filesystem::file_size(container); + std::fstream file(container, std::ios::out | std::ios::in); + BOOST_TEST(!file.bad()); + file.seekp(fsize - 16, std::ios::beg); + file.write((char *) buf, 16); + file.close(); + + rdr = libcdoc::CDocReader::createReader(container, &conf, &crypto, nullptr); + BOOST_TEST(rdr != nullptr, "Cannot create reader"); + BOOST_TEST(rdr->getFMK(fmk, 0) == libcdoc::OK); + BOOST_TEST(rdr->beginDecryption(fmk) == libcdoc::OK); + BOOST_TEST(rdr->nextFile(fi) == libcdoc::OK); + BOOST_TEST(rdr->nextFile(fi) == libcdoc::OK); + BOOST_TEST(rdr->finishDecryption() == libcdoc::CRYPTO_ERROR); + delete rdr; + + // Truncate file, should result zlib error + std::filesystem::resize_file(container, fsize - 32); + rdr = libcdoc::CDocReader::createReader(container, &conf, &crypto, nullptr); + BOOST_TEST(rdr != nullptr, "Cannot create reader"); + BOOST_TEST(rdr->getFMK(fmk, 0) == libcdoc::OK); + BOOST_TEST(rdr->beginDecryption(fmk) == libcdoc::OK); + libcdoc::result_t rv = rdr->nextFile(fi); + BOOST_TEST(((rv == libcdoc::OK) || (rv == libcdoc::CRYPTO_ERROR))); + for (int i = 0; i < 4; i++) { + rv = rdr->readData(buf, 256); + BOOST_TEST(((rv == 256) || (rv == libcdoc::CRYPTO_ERROR))); } - BOOST_TEST(reader->nextFile(fi) == libcdoc::END_OF_STREAM); - BOOST_TEST(reader->finishDecryption() == libcdoc::OK); + rv = rdr->nextFile(fi); + BOOST_TEST(((rv == libcdoc::OK) || (rv == libcdoc::CRYPTO_ERROR))); + rv = rdr->readData(buf, 256); + BOOST_TEST(((rv == 255) || (rv == libcdoc::CRYPTO_ERROR))); + BOOST_TEST(rdr->finishDecryption() == libcdoc::WORKFLOW_ERROR); + delete rdr; } - BOOST_AUTO_TEST_SUITE_END() // CDoc2 password and label @@ -553,6 +652,78 @@ BOOST_FIXTURE_TEST_CASE_WITH_DECOR(DecryptWithRSAKeyV1, DecryptFixture, } BOOST_AUTO_TEST_SUITE_END() +// Stream encryption/decryption of large files + +BOOST_AUTO_TEST_SUITE(LargeFiles) + +BOOST_FIXTURE_TEST_CASE_WITH_DECOR(EncryptWithPasswordAndLabel, FixtureBase, * utf::description("Testing weird and large files")) +{ + std::srand(1); + + std::vector data; + bool eof = false; + PipeConsumer pipec(data, eof); + PipeSource pipes(data, eof); + PipeCrypto pcrypto("password"); + + // Create writer + libcdoc::CDocWriter *writer = libcdoc::CDocWriter::createWriter(2, &pipec, false, nullptr, &pcrypto, nullptr); + BOOST_TEST(writer != nullptr); + libcdoc::Recipient rcpt = libcdoc::Recipient::makeSymmetric("test", 65536); + BOOST_TEST(writer->addRecipient(rcpt) == libcdoc::OK); + BOOST_TEST(writer->beginEncryption() == libcdoc::OK); + + // List of files: 0, 0, max_size...0 + std::vector files; + files.emplace_back(gen_random_filename(), 0); + files.emplace_back(gen_random_filename(), 0); + for (size_t size = max_filesize; size != 0; size = size / 100) { + files.emplace_back(gen_random_filename(), size); + } + files.emplace_back(gen_random_filename(), 0); + + PipeWriter wrt(writer, files); + + // Create reader + libcdoc::CDocReader *reader = libcdoc::CDocReader::createReader(&pipes, false, nullptr, &pcrypto, nullptr); + BOOST_TEST(reader != nullptr); + + // Fill buffer + while((data.size() < 2 * wrt.BUFSIZE) && !wrt.isEof()) { + BOOST_TEST(wrt.writeMore() == libcdoc::OK); + } + std::vector fmk; + BOOST_TEST(reader->getFMK(fmk, 0) == libcdoc::OK); + BOOST_TEST(reader->beginDecryption(fmk) == libcdoc::OK); + libcdoc::FileInfo fi; + for (int cfile = 0; cfile < files.size(); cfile++) { + // Fill buffer + while((data.size() < 2 * wrt.BUFSIZE) && !wrt.isEof()) { + BOOST_TEST(wrt.writeMore() == libcdoc::OK); + } + // Get file + BOOST_TEST(reader->nextFile(fi) == libcdoc::OK); + BOOST_TEST(fi.name == files[cfile].name); + BOOST_TEST(fi.size == files[cfile].size); + for (size_t pos = 0; pos < files[cfile].size; pos += wrt.BUFSIZE) { + // Fill buffer + while((data.size() < 2 * wrt.BUFSIZE) && !wrt.isEof()) { + BOOST_TEST(wrt.writeMore() == libcdoc::OK); + } + size_t toread = files[cfile].size - pos; + if (toread > wrt.BUFSIZE) toread = wrt.BUFSIZE; + uint8_t buf[wrt.BUFSIZE], cbuf[wrt.BUFSIZE]; + BOOST_TEST(reader->readData(buf, toread) == toread); + for (size_t i = 0; i < toread; i++) cbuf[i] = wrt.getChar(cfile, pos + i); + BOOST_TEST(std::memcmp(buf, cbuf, toread) == 0); + } + } + BOOST_TEST(reader->nextFile(fi) == libcdoc::END_OF_STREAM); + BOOST_TEST(reader->finishDecryption() == libcdoc::OK); +} + +BOOST_AUTO_TEST_SUITE_END() + // Label parsing BOOST_AUTO_TEST_SUITE(MachineLabelParsing) From b4a173662af1bdc42060e61b66ef9f1dab004eb2 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Fri, 30 Jan 2026 14:57:46 +0200 Subject: [PATCH 2/7] Create locks even if capsule is not supported --- cdoc/CDoc2Reader.cpp | 272 ++++++++++++++++++++--------------------- cdoc/Crypto.cpp | 2 +- cdoc/Lock.h | 7 +- test/libcdoc_boost.cpp | 10 +- 4 files changed, 148 insertions(+), 143 deletions(-) diff --git a/cdoc/CDoc2Reader.cpp b/cdoc/CDoc2Reader.cpp index 1389c749..91de38e7 100644 --- a/cdoc/CDoc2Reader.cpp +++ b/cdoc/CDoc2Reader.cpp @@ -101,6 +101,8 @@ struct CDoc2Reader::Private { dec.reset(); return rv; } + + static void buildLock(Lock& lock, const cdoc20::header::RecipientRecord& recipient); }; CDoc2Reader::~CDoc2Reader() @@ -487,11 +489,140 @@ CDoc2Reader::finishDecryption() return rv; } +void +CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecord& recipient) +{ + using namespace cdoc20::recipients; + using namespace cdoc20::header; + + lock.label = recipient.key_label()->str(); + lock.encrypted_fmk.assign(recipient.encrypted_fmk()->cbegin(), recipient.encrypted_fmk()->cend()); + + if(recipient.fmk_encryption_method() != cdoc20::header::FMKEncryptionMethod::XOR) + { + LOG_WARN("Unsupported FMK encryption method"); + return; + } + switch(recipient.capsule_type()) + { + case Capsule::recipients_ECCPublicKeyCapsule: + if(const auto *key = recipient.capsule_as_recipients_ECCPublicKeyCapsule()) { + if(key->curve() == EllipticCurve::secp384r1) { + lock.type = Lock::Type::PUBLIC_KEY; + lock.pk_type = Lock::PKType::ECC; + lock.setBytes(Lock::Params::RCPT_KEY, std::vector(key->recipient_public_key()->cbegin(), key->recipient_public_key()->cend())); + lock.setBytes(Lock::Params::KEY_MATERIAL, std::vector(key->sender_public_key()->cbegin(), key->sender_public_key()->cend())); + LOG_DBG("Load PK: {}", toHex(lock.getBytes(Lock::Params::RCPT_KEY))); + } else { + LOG_ERROR("Unsupported ECC curve: skipping"); + } + } + return; + case Capsule::recipients_RSAPublicKeyCapsule: + if(const auto *key = recipient.capsule_as_recipients_RSAPublicKeyCapsule()) + { + lock.type = Lock::Type::PUBLIC_KEY; + lock.pk_type = Lock::PKType::RSA; + lock.setBytes(Lock::Params::RCPT_KEY, std::vector(key->recipient_public_key()->cbegin(), key->recipient_public_key()->cend())); + lock.setBytes(Lock::Params::KEY_MATERIAL, std::vector(key->encrypted_kek()->cbegin(), key->encrypted_kek()->cend())); + } + return; + case Capsule::recipients_KeyServerCapsule: + if (const KeyServerCapsule *server = recipient.capsule_as_recipients_KeyServerCapsule()) { + KeyDetailsUnion details = server->recipient_key_details_type(); + switch (details) { + case KeyDetailsUnion::EccKeyDetails: + if(const EccKeyDetails *eccDetails = server->recipient_key_details_as_EccKeyDetails()) { + if(eccDetails->curve() != EllipticCurve::secp384r1) { + LOG_ERROR("Unsupported elliptic curve key type"); + return; + } + lock.pk_type = Lock::PKType::ECC; + lock.setBytes(Lock::Params::RCPT_KEY, std::vector(eccDetails->recipient_public_key()->cbegin(), eccDetails->recipient_public_key()->cend())); + } else { + LOG_ERROR("Invalid file format"); + return; + } + break; + case KeyDetailsUnion::RsaKeyDetails: + if(const RsaKeyDetails *rsaDetails = server->recipient_key_details_as_RsaKeyDetails()) { + lock.pk_type = Lock::PKType::RSA; + lock.setBytes(Lock::Params::RCPT_KEY, std::vector(rsaDetails->recipient_public_key()->cbegin(), rsaDetails->recipient_public_key()->cend())); + } else { + LOG_ERROR("Invalid file format"); + return; + } + break; + default: + LOG_ERROR("Unsupported Key Server Details: skipping"); + return; + } + lock.type = Lock::Type::SERVER; + lock.setString(Lock::Params::KEYSERVER_ID, server->keyserver_id()->str()); + lock.setString(Lock::Params::TRANSACTION_ID, server->transaction_id()->str()); + } else { + LOG_ERROR("Invalid file format"); + } + return; + case Capsule::recipients_SymmetricKeyCapsule: + if(const auto *capsule = recipient.capsule_as_recipients_SymmetricKeyCapsule()) + { + lock.type = Lock::SYMMETRIC_KEY; + lock.setBytes(Lock::SALT, std::vector(capsule->salt()->cbegin(), capsule->salt()->cend())); + } + return; + case Capsule::recipients_PBKDF2Capsule: + if(const auto *capsule = recipient.capsule_as_recipients_PBKDF2Capsule()) { + KDFAlgorithmIdentifier kdf_id = capsule->kdf_algorithm_identifier(); + if (kdf_id != KDFAlgorithmIdentifier::PBKDF2WithHmacSHA256) { + LOG_ERROR("Unsupported KDF algorithm: skipping"); + return; + } + lock.type = Lock::PASSWORD; + lock.setBytes(Lock::SALT, std::vector(capsule->salt()->cbegin(), capsule->salt()->cend())); + lock.setBytes(Lock::PW_SALT, std::vector(capsule->password_salt()->cbegin(), capsule->password_salt()->cend())); + lock.setInt(Lock::KDF_ITER, capsule->kdf_iterations()); + } + return; + case Capsule::recipients_KeySharesCapsule: + if (const auto *capsule = recipient.capsule_as_recipients_KeySharesCapsule()) { + if (capsule->recipient_type() != cdoc20::recipients::KeyShareRecipientType::SID_MID) { + LOG_ERROR("Invalid keyshare recipient type: {}", (int) capsule->recipient_type()); + return; + } + if (capsule->shares_scheme() != cdoc20::recipients::SharesScheme::N_OF_N) { + LOG_ERROR("Invalid keyshare scheme type: {}", (int) capsule->shares_scheme()); + return; + } + /* url,share_id;url,share_id... */ + std::vector strs; + for (auto cshare = capsule->shares()->cbegin(); cshare != capsule->shares()->cend(); ++cshare) { + std::string id = cshare->share_id()->str(); + std::string url = cshare->server_base_url()->str(); + std::string str = url + "," + id; + LOG_DBG("Keyshare: {}", str); + strs.push_back(std::move(str)); + } + std::string urls = join(strs, ";"); + LOG_DBG("Keyshare urls: {}", urls); + std::vector salt(capsule->salt()->cbegin(), capsule->salt()->cend()); + LOG_DBG("Keyshare salt: {}", toHex(salt)); + std::string recipient_id = capsule->recipient_id()->str(); + LOG_DBG("Keyshare recipient id: {}", recipient_id); + lock.type = Lock::SHARE_SERVER; + lock.setString(Lock::SHARE_URLS, urls); + lock.setBytes(Lock::SALT, salt); + lock.setString(Lock::RECIPIENT_ID, recipient_id); + } + return; + default: + LOG_ERROR("Unsupported Key Details: skipping"); + } +} + CDoc2Reader::CDoc2Reader(libcdoc::DataSource *src, bool take_ownership) : CDocReader(2), priv(std::make_unique(src, take_ownership)) { - - using namespace cdoc20::recipients; using namespace cdoc20::header; setLastError(t_("Invalid CDoc 2.0 header")); @@ -555,140 +686,9 @@ CDoc2Reader::CDoc2Reader(libcdoc::DataSource *src, bool take_ownership) setLastError({}); for(const auto *recipient: *recipients){ - if(recipient->fmk_encryption_method() != FMKEncryptionMethod::XOR) - { - LOG_WARN("Unsupported FMK encryption method: skipping"); - continue; - } - auto fillRecipientPK = [&recipient,&locks = priv->locks] (Lock::PKType pk_type, auto key) -> Lock& { - Lock &k = locks.emplace_back(Lock::Type::PUBLIC_KEY); - k.pk_type = pk_type; - k.setBytes(Lock::Params::RCPT_KEY, std::vector(key->recipient_public_key()->cbegin(), key->recipient_public_key()->cend())); - k.label = recipient->key_label()->str(); - k.encrypted_fmk.assign(recipient->encrypted_fmk()->cbegin(), recipient->encrypted_fmk()->cend()); - return k; - }; - switch(recipient->capsule_type()) - { - case Capsule::recipients_ECCPublicKeyCapsule: - if(const auto *key = recipient->capsule_as_recipients_ECCPublicKeyCapsule()) { - if(key->curve() != EllipticCurve::secp384r1) { - LOG_ERROR("Unsupported ECC curve: skipping"); - continue; - } - Lock &k = fillRecipientPK(Lock::PKType::ECC, key); - k.setBytes(Lock::Params::KEY_MATERIAL, std::vector(key->sender_public_key()->cbegin(), key->sender_public_key()->cend())); - LOG_DBG("Load PK: {}", toHex(k.getBytes(Lock::Params::RCPT_KEY))); - } - break; - case Capsule::recipients_RSAPublicKeyCapsule: - if(const auto *key = recipient->capsule_as_recipients_RSAPublicKeyCapsule()) - { - Lock &k = fillRecipientPK(Lock::PKType::RSA, key); - k.setBytes(Lock::Params::KEY_MATERIAL, std::vector(key->encrypted_kek()->cbegin(), key->encrypted_kek()->cend())); - } - break; - case Capsule::recipients_KeyServerCapsule: - if (const KeyServerCapsule *server = recipient->capsule_as_recipients_KeyServerCapsule()) { - KeyDetailsUnion details = server->recipient_key_details_type(); - Lock ckey; - switch (details) { - case KeyDetailsUnion::EccKeyDetails: - if(const EccKeyDetails *eccDetails = server->recipient_key_details_as_EccKeyDetails()) { - if(eccDetails->curve() == EllipticCurve::secp384r1) { - ckey.type = Lock::Type::SERVER; - ckey.pk_type = Lock::PKType::ECC; - ckey.setBytes(Lock::Params::RCPT_KEY, std::vector(eccDetails->recipient_public_key()->cbegin(), eccDetails->recipient_public_key()->cend())); - } else { - LOG_ERROR("Unsupported elliptic curve key type"); - } - } else { - LOG_ERROR("Invalid file format"); - } - break; - case KeyDetailsUnion::RsaKeyDetails: - if(const RsaKeyDetails *rsaDetails = server->recipient_key_details_as_RsaKeyDetails()) { - ckey.type = Lock::Type::SERVER; - ckey.pk_type = Lock::PKType::RSA; - ckey.setBytes(Lock::Params::RCPT_KEY, std::vector(rsaDetails->recipient_public_key()->cbegin(), rsaDetails->recipient_public_key()->cend())); - } else { - LOG_ERROR("Invalid file format"); - } - break; - default: - LOG_ERROR("Unsupported Key Server Details: skipping"); - } - if (ckey.type != Lock::Type::INVALID) { - ckey.label = recipient->key_label()->c_str(); - ckey.encrypted_fmk.assign(recipient->encrypted_fmk()->cbegin(), recipient->encrypted_fmk()->cend()); - ckey.setString(Lock::Params::KEYSERVER_ID, server->keyserver_id()->str()); - ckey.setString(Lock::Params::TRANSACTION_ID, server->transaction_id()->str()); - priv->locks.push_back(std::move(ckey)); - } - } else { - LOG_ERROR("Invalid file format"); - } - break; - case Capsule::recipients_SymmetricKeyCapsule: - if(const auto *capsule = recipient->capsule_as_recipients_SymmetricKeyCapsule()) - { - Lock &key = priv->locks.emplace_back(Lock::SYMMETRIC_KEY); - key.label = recipient->key_label()->str(); - key.encrypted_fmk.assign(recipient->encrypted_fmk()->cbegin(), recipient->encrypted_fmk()->cend()); - key.setBytes(Lock::SALT, std::vector(capsule->salt()->cbegin(), capsule->salt()->cend())); - } - break; - case Capsule::recipients_PBKDF2Capsule: - if(const auto *capsule = recipient->capsule_as_recipients_PBKDF2Capsule()) { - KDFAlgorithmIdentifier kdf_id = capsule->kdf_algorithm_identifier(); - if (kdf_id != KDFAlgorithmIdentifier::PBKDF2WithHmacSHA256) { - LOG_ERROR("Unsupported KDF algorithm: skipping"); - continue; - } - Lock &key = priv->locks.emplace_back(Lock::PASSWORD); - key.label = recipient->key_label()->str(); - key.encrypted_fmk.assign(recipient->encrypted_fmk()->cbegin(), recipient->encrypted_fmk()->cend()); - key.setBytes(Lock::SALT, std::vector(capsule->salt()->cbegin(), capsule->salt()->cend())); - key.setBytes(Lock::PW_SALT, std::vector(capsule->password_salt()->cbegin(), capsule->password_salt()->cend())); - key.setInt(Lock::KDF_ITER, capsule->kdf_iterations()); - } - break; - case Capsule::recipients_KeySharesCapsule: - if (const auto *capsule = recipient->capsule_as_recipients_KeySharesCapsule()) { - if (capsule->recipient_type() != cdoc20::recipients::KeyShareRecipientType::SID_MID) { - LOG_ERROR("Invalid keyshare recipient type: {}", (int) capsule->recipient_type()); - continue; - } - if (capsule->shares_scheme() != cdoc20::recipients::SharesScheme::N_OF_N) { - LOG_ERROR("Invalid keyshare scheme type: {}", (int) capsule->shares_scheme()); - continue; - } - /* url,share_id;url,share_id... */ - std::vector strs; - for (auto cshare = capsule->shares()->cbegin(); cshare != capsule->shares()->cend(); ++cshare) { - std::string id = cshare->share_id()->str(); - std::string url = cshare->server_base_url()->str(); - std::string str = url + "," + id; - LOG_DBG("Keyshare: {}", str); - strs.push_back(std::move(str)); - } - std::string urls = join(strs, ";"); - LOG_DBG("Keyshare urls: {}", urls); - std::vector salt(capsule->salt()->cbegin(), capsule->salt()->cend()); - LOG_DBG("Keyshare salt: {}", toHex(salt)); - std::string recipient_id = capsule->recipient_id()->str(); - LOG_DBG("Keyshare recipient id: {}", recipient_id); - Lock &lock = priv->locks.emplace_back(Lock::SHARE_SERVER); - lock.label = recipient->key_label()->str(); - lock.encrypted_fmk.assign(recipient->encrypted_fmk()->cbegin(), recipient->encrypted_fmk()->cend()); - lock.setString(Lock::SHARE_URLS, urls); - lock.setBytes(Lock::SALT, salt); - lock.setString(Lock::RECIPIENT_ID, recipient_id); - } - break; - default: - LOG_ERROR("Unsupported Key Details: skipping"); - } + Lock lock(Lock::Type::UNSUPPORTED); + Private::buildLock(lock, *recipient); + priv->locks.push_back(std::move(lock)); } } diff --git a/cdoc/Crypto.cpp b/cdoc/Crypto.cpp index 81ac0c3c..9f902ba5 100644 --- a/cdoc/Crypto.cpp +++ b/cdoc/Crypto.cpp @@ -621,6 +621,6 @@ result_t DecryptionSource::close() int len = 0; std::vector buffer(EVP_CIPHER_CTX_block_size(ctx.get()), 0); if (SSL_FAILED(EVP_CipherFinal_ex(ctx.get(), buffer.data(), &len), "EVP_CipherFinal_ex")) - return error = CRYPTO_ERROR; + return error = HASH_MISMATCH; return OK; } diff --git a/cdoc/Lock.h b/cdoc/Lock.h index 99d5f720..6c91e546 100644 --- a/cdoc/Lock.h +++ b/cdoc/Lock.h @@ -47,6 +47,11 @@ struct CDOC_EXPORT Lock * @brief Invalid value */ INVALID, + /** + * @brief Valid capsule but not supported by this library version + * + */ + UNSUPPORTED, /** * @brief Symmetric AES key */ @@ -194,7 +199,7 @@ struct CDOC_EXPORT Lock * @brief check whether lock is valid * @return true if valid */ - bool isValid() const noexcept { return (type != Type::INVALID) && !label.empty() && !encrypted_fmk.empty(); } + bool isValid() const noexcept { return (type != Type::INVALID) && (type != Type::UNSUPPORTED) && !label.empty() && !encrypted_fmk.empty(); } /** * @brief check whether lock is based on symmetric key * @return true if type is SYMMETRIC_KEY or PASSWORD diff --git a/test/libcdoc_boost.cpp b/test/libcdoc_boost.cpp index 3b48536e..1401dc39 100644 --- a/test/libcdoc_boost.cpp +++ b/test/libcdoc_boost.cpp @@ -489,7 +489,7 @@ BOOST_FIXTURE_TEST_CASE_WITH_DECOR(CDoc2DecryptErrors, DecryptFixture, BOOST_TEST(rdr->beginDecryption(fmk) == libcdoc::OK); BOOST_TEST(rdr->nextFile(fi) == libcdoc::OK); BOOST_TEST(rdr->nextFile(fi) == libcdoc::OK); - BOOST_TEST(rdr->finishDecryption() == libcdoc::CRYPTO_ERROR); + BOOST_TEST(rdr->finishDecryption() == libcdoc::HASH_MISMATCH); delete rdr; // Truncate file, should result zlib error @@ -499,15 +499,15 @@ BOOST_FIXTURE_TEST_CASE_WITH_DECOR(CDoc2DecryptErrors, DecryptFixture, BOOST_TEST(rdr->getFMK(fmk, 0) == libcdoc::OK); BOOST_TEST(rdr->beginDecryption(fmk) == libcdoc::OK); libcdoc::result_t rv = rdr->nextFile(fi); - BOOST_TEST(((rv == libcdoc::OK) || (rv == libcdoc::CRYPTO_ERROR))); + BOOST_TEST(((rv == libcdoc::OK) || (rv == libcdoc::HASH_MISMATCH))); for (int i = 0; i < 4; i++) { rv = rdr->readData(buf, 256); - BOOST_TEST(((rv == 256) || (rv == libcdoc::CRYPTO_ERROR))); + BOOST_TEST(((rv == 256) || (rv == libcdoc::HASH_MISMATCH))); } rv = rdr->nextFile(fi); - BOOST_TEST(((rv == libcdoc::OK) || (rv == libcdoc::CRYPTO_ERROR))); + BOOST_TEST(((rv == libcdoc::OK) || (rv == libcdoc::HASH_MISMATCH))); rv = rdr->readData(buf, 256); - BOOST_TEST(((rv == 255) || (rv == libcdoc::CRYPTO_ERROR))); + BOOST_TEST(((rv == 255) || (rv == libcdoc::HASH_MISMATCH))); BOOST_TEST(rdr->finishDecryption() == libcdoc::WORKFLOW_ERROR); delete rdr; } From 414452e0c185903ec543fc82ee7c53b617175a25 Mon Sep 17 00:00:00 2001 From: lauris71 Date: Mon, 2 Feb 2026 12:25:56 +0200 Subject: [PATCH 3/7] Update cdoc/CDoc2Reader.cpp Co-authored-by: Raul Metsma --- cdoc/CDoc2Reader.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cdoc/CDoc2Reader.cpp b/cdoc/CDoc2Reader.cpp index 91de38e7..41b3c3a2 100644 --- a/cdoc/CDoc2Reader.cpp +++ b/cdoc/CDoc2Reader.cpp @@ -686,9 +686,7 @@ CDoc2Reader::CDoc2Reader(libcdoc::DataSource *src, bool take_ownership) setLastError({}); for(const auto *recipient: *recipients){ - Lock lock(Lock::Type::UNSUPPORTED); - Private::buildLock(lock, *recipient); - priv->locks.push_back(std::move(lock)); + Private::buildLock(priv->locks.emplace_back(Lock::Type::UNSUPPORTED), *recipient); } } From 0854344b75d815a300eb41fd0cfd0f4176b5bb50 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Mon, 2 Feb 2026 13:00:09 +0200 Subject: [PATCH 4/7] Use toUint8Vector helper --- cdoc/CDoc2Reader.cpp | 26 +++++++++++++------------- cdoc/CryptoBackend.h | 4 ++-- cdoc/NetworkBackend.cpp | 5 ++--- cdoc/Utils.h | 10 ++++++++++ 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/cdoc/CDoc2Reader.cpp b/cdoc/CDoc2Reader.cpp index 41b3c3a2..514ee251 100644 --- a/cdoc/CDoc2Reader.cpp +++ b/cdoc/CDoc2Reader.cpp @@ -215,7 +215,7 @@ CDoc2Reader::getFMK(std::vector& fmk, unsigned int lock_idx) } } else { std::vector kek_pm; - int result = crypto->deriveHMACExtract(kek_pm, key_material, std::vector(libcdoc::CDoc2::KEKPREMASTER.cbegin(), libcdoc::CDoc2::KEKPREMASTER.cend()), lock_idx); + int result = crypto->deriveHMACExtract(kek_pm, key_material, toUint8Vector(libcdoc::CDoc2::KEKPREMASTER), lock_idx); if (result < 0) { setLastError(crypto->getLastErrorStr(result)); LOG_ERROR("{}", last_error); @@ -411,7 +411,7 @@ CDoc2Reader::beginDecryption(const std::vector& fmk) LOG_TRACE_KEY("cek: {}", cek); priv->dec = std::make_unique(*priv->_src, EVP_chacha20_poly1305(), cek, libcdoc::CDoc2::NONCE_LEN); - std::vector aad(libcdoc::CDoc2::PAYLOAD.cbegin(), libcdoc::CDoc2::PAYLOAD.cend()); + std::vector aad = toUint8Vector(libcdoc::CDoc2::PAYLOAD); aad.insert(aad.end(), priv->header_data.cbegin(), priv->header_data.cend()); aad.insert(aad.end(), priv->headerHMAC.cbegin(), priv->headerHMAC.cend()); if(auto rv = priv->dec->updateAAD(aad); rv != OK) { @@ -496,7 +496,7 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor using namespace cdoc20::header; lock.label = recipient.key_label()->str(); - lock.encrypted_fmk.assign(recipient.encrypted_fmk()->cbegin(), recipient.encrypted_fmk()->cend()); + lock.encrypted_fmk = toUint8Vector(recipient.encrypted_fmk()); if(recipient.fmk_encryption_method() != cdoc20::header::FMKEncryptionMethod::XOR) { @@ -510,8 +510,8 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor if(key->curve() == EllipticCurve::secp384r1) { lock.type = Lock::Type::PUBLIC_KEY; lock.pk_type = Lock::PKType::ECC; - lock.setBytes(Lock::Params::RCPT_KEY, std::vector(key->recipient_public_key()->cbegin(), key->recipient_public_key()->cend())); - lock.setBytes(Lock::Params::KEY_MATERIAL, std::vector(key->sender_public_key()->cbegin(), key->sender_public_key()->cend())); + lock.setBytes(Lock::Params::RCPT_KEY, toUint8Vector(key->recipient_public_key())); + lock.setBytes(Lock::Params::KEY_MATERIAL, toUint8Vector(key->sender_public_key())); LOG_DBG("Load PK: {}", toHex(lock.getBytes(Lock::Params::RCPT_KEY))); } else { LOG_ERROR("Unsupported ECC curve: skipping"); @@ -523,8 +523,8 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor { lock.type = Lock::Type::PUBLIC_KEY; lock.pk_type = Lock::PKType::RSA; - lock.setBytes(Lock::Params::RCPT_KEY, std::vector(key->recipient_public_key()->cbegin(), key->recipient_public_key()->cend())); - lock.setBytes(Lock::Params::KEY_MATERIAL, std::vector(key->encrypted_kek()->cbegin(), key->encrypted_kek()->cend())); + lock.setBytes(Lock::Params::RCPT_KEY, toUint8Vector(key->recipient_public_key())); + lock.setBytes(Lock::Params::KEY_MATERIAL, toUint8Vector(key->encrypted_kek())); } return; case Capsule::recipients_KeyServerCapsule: @@ -538,7 +538,7 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor return; } lock.pk_type = Lock::PKType::ECC; - lock.setBytes(Lock::Params::RCPT_KEY, std::vector(eccDetails->recipient_public_key()->cbegin(), eccDetails->recipient_public_key()->cend())); + lock.setBytes(Lock::Params::RCPT_KEY, toUint8Vector(eccDetails->recipient_public_key())); } else { LOG_ERROR("Invalid file format"); return; @@ -547,7 +547,7 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor case KeyDetailsUnion::RsaKeyDetails: if(const RsaKeyDetails *rsaDetails = server->recipient_key_details_as_RsaKeyDetails()) { lock.pk_type = Lock::PKType::RSA; - lock.setBytes(Lock::Params::RCPT_KEY, std::vector(rsaDetails->recipient_public_key()->cbegin(), rsaDetails->recipient_public_key()->cend())); + lock.setBytes(Lock::Params::RCPT_KEY, toUint8Vector(rsaDetails->recipient_public_key())); } else { LOG_ERROR("Invalid file format"); return; @@ -568,7 +568,7 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor if(const auto *capsule = recipient.capsule_as_recipients_SymmetricKeyCapsule()) { lock.type = Lock::SYMMETRIC_KEY; - lock.setBytes(Lock::SALT, std::vector(capsule->salt()->cbegin(), capsule->salt()->cend())); + lock.setBytes(Lock::SALT, toUint8Vector(capsule->salt())); } return; case Capsule::recipients_PBKDF2Capsule: @@ -579,8 +579,8 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor return; } lock.type = Lock::PASSWORD; - lock.setBytes(Lock::SALT, std::vector(capsule->salt()->cbegin(), capsule->salt()->cend())); - lock.setBytes(Lock::PW_SALT, std::vector(capsule->password_salt()->cbegin(), capsule->password_salt()->cend())); + lock.setBytes(Lock::SALT, toUint8Vector(capsule->salt())); + lock.setBytes(Lock::PW_SALT, toUint8Vector(capsule->password_salt())); lock.setInt(Lock::KDF_ITER, capsule->kdf_iterations()); } return; @@ -605,7 +605,7 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor } std::string urls = join(strs, ";"); LOG_DBG("Keyshare urls: {}", urls); - std::vector salt(capsule->salt()->cbegin(), capsule->salt()->cend()); + std::vector salt = toUint8Vector(capsule->salt()); LOG_DBG("Keyshare salt: {}", toHex(salt)); std::string recipient_id = capsule->recipient_id()->str(); LOG_DBG("Keyshare recipient id: {}", recipient_id); diff --git a/cdoc/CryptoBackend.h b/cdoc/CryptoBackend.h index c1013c0e..56e64475 100644 --- a/cdoc/CryptoBackend.h +++ b/cdoc/CryptoBackend.h @@ -36,8 +36,8 @@ struct Lock; * - decryptRSA for RSA keys * - getSecret for symmetric keys. * - * ECC and symmetric keys have also frontend methods; implementing these allows the program to perform certain cryptographic procedures in controlled - * environment and (in case of symmetric keys) avoid exposing secret keys/passwords. + * ECC and symmetric keys have also frontend methods; implementing these allows the program to perform certain cryptographic procedures in secure + * environment and (in case of symmetric keys) avoid exposing secret keys/passwords to library code. */ struct CDOC_EXPORT CryptoBackend { static constexpr int INVALID_PARAMS = -201; diff --git a/cdoc/NetworkBackend.cpp b/cdoc/NetworkBackend.cpp index 40163de2..3fb52d06 100644 --- a/cdoc/NetworkBackend.cpp +++ b/cdoc/NetworkBackend.cpp @@ -397,8 +397,7 @@ libcdoc::NetworkBackend::fetchKey (std::vector& dst, const std::string& } error = {}; std::string ks = v.get(); - std::vector key_material = fromBase64(ks); - dst.assign(key_material.cbegin(), key_material.cend()); + dst = fromBase64(ks); return libcdoc::OK; } @@ -434,7 +433,7 @@ libcdoc::NetworkBackend::fetchNonce(std::vector& dst, const std::string return NETWORK_ERROR; } std::string nonce_str = v.get(); - dst.assign(nonce_str.cbegin(), nonce_str.cend()); + dst = toUint8Vector(nonce_str); return OK; } diff --git a/cdoc/Utils.h b/cdoc/Utils.h index a0283894..a98e4c83 100644 --- a/cdoc/Utils.h +++ b/cdoc/Utils.h @@ -125,6 +125,16 @@ struct urlEncode { friend std::ostream& operator<<(std::ostream& escaped, urlEncode src); }; +std::vector toUint8Vector(const auto* data) +{ + return {data->cbegin(), data->cend()}; +} + +std::vector toUint8Vector(const auto& data) +{ + return {data.cbegin(), data.cend()}; +} + std::string urlDecode(const std::string &src); } // namespace libcdoc From 777700cc6b799ca2daa90296ec7716f31b4c5d56 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Mon, 2 Feb 2026 17:15:55 +0200 Subject: [PATCH 5/7] Removed some extra check from parsing --- cdoc/CDoc2Reader.cpp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/cdoc/CDoc2Reader.cpp b/cdoc/CDoc2Reader.cpp index 514ee251..30cf6831 100644 --- a/cdoc/CDoc2Reader.cpp +++ b/cdoc/CDoc2Reader.cpp @@ -539,29 +539,21 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor } lock.pk_type = Lock::PKType::ECC; lock.setBytes(Lock::Params::RCPT_KEY, toUint8Vector(eccDetails->recipient_public_key())); - } else { - LOG_ERROR("Invalid file format"); - return; } break; case KeyDetailsUnion::RsaKeyDetails: if(const RsaKeyDetails *rsaDetails = server->recipient_key_details_as_RsaKeyDetails()) { lock.pk_type = Lock::PKType::RSA; lock.setBytes(Lock::Params::RCPT_KEY, toUint8Vector(rsaDetails->recipient_public_key())); - } else { - LOG_ERROR("Invalid file format"); - return; } break; default: - LOG_ERROR("Unsupported Key Server Details: skipping"); + LOG_ERROR("Unsupported Key Server Details"); return; } lock.type = Lock::Type::SERVER; lock.setString(Lock::Params::KEYSERVER_ID, server->keyserver_id()->str()); lock.setString(Lock::Params::TRANSACTION_ID, server->transaction_id()->str()); - } else { - LOG_ERROR("Invalid file format"); } return; case Capsule::recipients_SymmetricKeyCapsule: @@ -616,7 +608,7 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor } return; default: - LOG_ERROR("Unsupported Key Details: skipping"); + LOG_ERROR("Unsupported capsule type"); } } From 8cdc65201185d1aac1680f073fdd7be9202403d9 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Tue, 3 Feb 2026 15:01:48 +0200 Subject: [PATCH 6/7] Remove INVALID lock type and rename UNSUPPORTED to UNKNOWN --- cdoc/CDoc2Reader.cpp | 2 +- cdoc/Lock.h | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/cdoc/CDoc2Reader.cpp b/cdoc/CDoc2Reader.cpp index 30cf6831..913adca3 100644 --- a/cdoc/CDoc2Reader.cpp +++ b/cdoc/CDoc2Reader.cpp @@ -678,7 +678,7 @@ CDoc2Reader::CDoc2Reader(libcdoc::DataSource *src, bool take_ownership) setLastError({}); for(const auto *recipient: *recipients){ - Private::buildLock(priv->locks.emplace_back(Lock::Type::UNSUPPORTED), *recipient); + Private::buildLock(priv->locks.emplace_back(), *recipient); } } diff --git a/cdoc/Lock.h b/cdoc/Lock.h index 6c91e546..3011f871 100644 --- a/cdoc/Lock.h +++ b/cdoc/Lock.h @@ -43,15 +43,11 @@ struct CDOC_EXPORT Lock * @brief The lock type */ enum Type : unsigned char { - /** - * @brief Invalid value - */ - INVALID, /** * @brief Valid capsule but not supported by this library version * */ - UNSUPPORTED, + UNKNOWN, /** * @brief Symmetric AES key */ @@ -180,7 +176,7 @@ struct CDOC_EXPORT Lock /** * @brief The lock type */ - Type type = Type::INVALID; + Type type = Type::UNKNOWN; /** * @brief algorithm type for public key based locks */ @@ -199,7 +195,7 @@ struct CDOC_EXPORT Lock * @brief check whether lock is valid * @return true if valid */ - bool isValid() const noexcept { return (type != Type::INVALID) && (type != Type::UNSUPPORTED) && !label.empty() && !encrypted_fmk.empty(); } + bool isValid() const noexcept { return (type != Type::UNKNOWN) && !label.empty() && !encrypted_fmk.empty(); } /** * @brief check whether lock is based on symmetric key * @return true if type is SYMMETRIC_KEY or PASSWORD From 5ef36d7c11098c3cbeb4becbcc34c6360093e3f3 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Wed, 4 Feb 2026 13:11:42 +0200 Subject: [PATCH 7/7] Return NULL from frontend if CDoc constructor fails --- cdoc/CDoc.cpp | 51 +++++++++++--------------------------------- cdoc/CDoc2Reader.cpp | 2 +- cdoc/Io.h | 2 -- 3 files changed, 14 insertions(+), 41 deletions(-) diff --git a/cdoc/CDoc.cpp b/cdoc/CDoc.cpp index 97e393b0..0be43f8b 100644 --- a/cdoc/CDoc.cpp +++ b/cdoc/CDoc.cpp @@ -103,7 +103,7 @@ libcdoc::CDocReader::createReader(DataSource *src, bool take_ownership, Configur int version = getCDocFileVersion(src); LOG_DBG("CDocReader::createReader: version {}", version); if (src->seek(0) != libcdoc::OK) return nullptr; - CDocReader *reader; + CDocReader *reader = nullptr; if (version == 1) { reader = new CDoc1Reader(src, take_ownership); } else if (version == 2) { @@ -111,12 +111,16 @@ libcdoc::CDocReader::createReader(DataSource *src, bool take_ownership, Configur } else { if(take_ownership) delete src; - return nullptr; - } - reader->conf = conf; - reader->crypto = crypto; + return nullptr; + } + if (!reader->getLastErrorStr().empty()) { + delete reader; + return nullptr; + } + reader->conf = conf; + reader->crypto = crypto; reader->network = network; - return reader; + return reader; } libcdoc::CDocReader * @@ -125,43 +129,14 @@ libcdoc::CDocReader::createReader(const std::string& path, Configuration *conf, if(path.empty()) return nullptr; auto isrc = make_unique(path); - int version = getCDocFileVersion(isrc.get()); - LOG_DBG("CDocReader::createReader: version {}", version); - if (isrc->seek(0) != libcdoc::OK) - return nullptr; - CDocReader *reader; - if (version == 1) { - reader = new CDoc1Reader(isrc.release(), true); - } else if (version == 2) { - reader = new CDoc2Reader(isrc.release(), true); - } else { - return nullptr; - } - reader->conf = conf; - reader->crypto = crypto; - reader->network = network; - return reader; + return createReader(isrc.release(), true, conf, crypto, network); } libcdoc::CDocReader * libcdoc::CDocReader::createReader(std::istream& ifs, Configuration *conf, CryptoBackend *crypto, NetworkBackend *network) { - libcdoc::IStreamSource *isrc = new libcdoc::IStreamSource(&ifs, false); - int version = getCDocFileVersion(isrc); - LOG_DBG("CDocReader::createReader: version {}", version); - CDocReader *reader; - if (version == 1) { - reader = new CDoc1Reader(isrc, true); - } else if (version == 2) { - reader = new CDoc2Reader(isrc, true); - } else { - delete isrc; - return nullptr; - } - reader->conf = conf; - reader->crypto = crypto; - reader->network = network; - return reader; + auto isrc = make_unique(&ifs, false); + return createReader(isrc.release(), true, conf, crypto, network); } #if LIBCDOC_TESTING diff --git a/cdoc/CDoc2Reader.cpp b/cdoc/CDoc2Reader.cpp index 913adca3..6b83dbc9 100644 --- a/cdoc/CDoc2Reader.cpp +++ b/cdoc/CDoc2Reader.cpp @@ -588,7 +588,7 @@ CDoc2Reader::Private::buildLock(Lock& lock, const cdoc20::header::RecipientRecor } /* url,share_id;url,share_id... */ std::vector strs; - for (auto cshare = capsule->shares()->cbegin(); cshare != capsule->shares()->cend(); ++cshare) { + for (auto cshare : *capsule->shares()) { std::string id = cshare->share_id()->str(); std::string url = cshare->server_base_url()->str(); std::string str = url + "," + id; diff --git a/cdoc/Io.h b/cdoc/Io.h index 45902101..dfc04199 100644 --- a/cdoc/Io.h +++ b/cdoc/Io.h @@ -259,8 +259,6 @@ struct CDOC_EXPORT IStreamSource : public DataSource { if(_ifs->bad()) return INPUT_STREAM_ERROR; _ifs->clear(); _ifs->seekg(pos); - //std::cerr << "Stream bad:" << _ifs->bad() << " eof:" << _ifs->eof() << " fail:" << _ifs->fail() << std::endl; - //std::cerr << "tell:" << _ifs->tellg() << std::endl; return bool(_ifs->bad()) ? INPUT_STREAM_ERROR : OK; }