Skip to content

Commit fc41f1d

Browse files
Lauris Kaplinskimetsma
Lauris Kaplinski
authored and
metsma
committed
Tar and workflow fixes and tests
Signed-off-by: Lauris Kaplinski <lauris@raulwalter.com>
1 parent 7f658d2 commit fc41f1d

8 files changed

Lines changed: 367 additions & 100 deletions

File tree

‎cdoc/CDoc.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,16 @@ enum {
5757
NOT_SUPPORTED = -101,
5858
/**
5959
* @brief Conflicting or invalid arguments for a method
60+
*
61+
* This does not set CDocReader/CDocWriter into error state - so invoking subsequent methods
62+
* with correct arguments will succeed
6063
*/
6164
WRONG_ARGUMENTS = -102,
6265
/**
6366
* @brief Components of multi-method workflow are called in wrong order
67+
*
68+
* This does not set CDocReader/CDocWriter into error state - so invoking subsequent methods
69+
* in correct order will succeed
6470
*/
6571
WORKFLOW_ERROR = -103,
6672
/**
@@ -85,6 +91,9 @@ enum {
8591
INPUT_STREAM_ERROR = -108,
8692
/**
8793
* @brief The supplied decryption key is wrong
94+
*
95+
* This does not set CDocReader/CDocWriter into error state - so invoking subsequent methods
96+
* with correct key will succeed
8897
*/
8998
WRONG_KEY = -109,
9099
/**

‎cdoc/CDoc2Reader.cpp‎

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,20 @@ struct CDoc2Reader::Private {
8787
std::unique_ptr<libcdoc::DecryptionSource> dec;
8888
std::unique_ptr<libcdoc::ZSource> zsrc;
8989
std::unique_ptr<libcdoc::TarSource> tar;
90+
91+
result_t decryptAllAndClose() {
92+
std::array<uint8_t, 1024> buf;
93+
result_t rv = dec->read(buf.data(), buf.size());
94+
while (rv == buf.size()) {
95+
rv = dec->read(buf.data(), buf.size());
96+
}
97+
if (rv < 0) return rv;
98+
zsrc.reset();
99+
tar.reset();
100+
rv = dec->close();
101+
dec.reset();
102+
return rv;
103+
}
90104
};
91105

92106
CDoc2Reader::~CDoc2Reader()
@@ -118,35 +132,44 @@ CDoc2Reader::getLockForCert(const std::vector<uint8_t>& cert){
118132
libcdoc::result_t
119133
CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
120134
{
135+
if (lock_idx >= priv->locks.size()) {
136+
setLastError(t_("Invalid lock index"));
137+
LOG_ERROR("{}", last_error);
138+
return libcdoc::WRONG_ARGUMENTS;
139+
}
121140
LOG_DBG("CDoc2Reader::getFMK: {}", lock_idx);
122141
LOG_DBG("CDoc2Reader::num locks: {}", priv->locks.size());
123142
const Lock& lock = priv->locks.at(lock_idx);
143+
LOG_DBG("Label: {}", lock.label);
124144
std::vector<uint8_t> kek;
125145
if (lock.type == Lock::Type::PASSWORD) {
126146
// Password
127147
LOG_DBG("password");
128148
std::string info_str = libcdoc::CDoc2::getSaltForExpand(lock.label);
149+
LOG_DBG("info: {}", toHex(info_str));
129150
std::vector<uint8_t> kek_pm;
130-
crypto->extractHKDF(kek_pm, lock.getBytes(Lock::SALT), lock.getBytes(Lock::PW_SALT), lock.getInt(Lock::KDF_ITER), lock_idx);
131-
LOG_DBG("password2");
151+
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) {
152+
setLastError(crypto->getLastErrorStr(rv));
153+
LOG_ERROR("{}", last_error);
154+
return rv;
155+
}
156+
LOG_TRACE_KEY("salt: {}", lock.getBytes(Lock::SALT));
157+
LOG_TRACE_KEY("kek_pm: {}", kek_pm);
132158
kek = libcdoc::Crypto::expand(kek_pm, info_str, 32);
133-
if (kek.empty()) return libcdoc::CRYPTO_ERROR;
134-
LOG_DBG("password3");
135159
} else if (lock.type == Lock::Type::SYMMETRIC_KEY) {
136160
// Symmetric key
137161
LOG_DBG("symmetric");
138162
std::string info_str = libcdoc::CDoc2::getSaltForExpand(lock.label);
139-
std::vector<uint8_t> kek_pm;
140-
crypto->extractHKDF(kek_pm, lock.getBytes(Lock::SALT), {}, 0, lock_idx);
141-
kek = libcdoc::Crypto::expand(kek_pm, info_str, 32);
142-
143-
LOG_DBG("Label: {}", lock.label);
144163
LOG_DBG("info: {}", toHex(info_str));
164+
std::vector<uint8_t> kek_pm;
165+
if (auto rv = crypto->extractHKDF(kek_pm, lock.getBytes(Lock::SALT), {}, 0, lock_idx); rv != libcdoc::OK) {
166+
setLastError(crypto->getLastErrorStr(rv));
167+
LOG_ERROR("{}", last_error);
168+
return rv;
169+
}
145170
LOG_TRACE_KEY("salt: {}", lock.getBytes(Lock::SALT));
146171
LOG_TRACE_KEY("kek_pm: {}", kek_pm);
147-
LOG_TRACE_KEY("kek: {}", kek);
148-
149-
if (kek.empty()) return libcdoc::CRYPTO_ERROR;
172+
kek = libcdoc::Crypto::expand(kek_pm, info_str, 32);
150173
} else if ((lock.type == Lock::Type::PUBLIC_KEY) || (lock.type == Lock::Type::SERVER)) {
151174
// Public/private key
152175
std::vector<uint8_t> key_material;
@@ -196,13 +219,9 @@ CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
196219
LOG_ERROR("{}", last_error);
197220
return result;
198221
}
199-
200222
LOG_TRACE_KEY("Key kekPm: {}", kek_pm);
201-
202223
std::string info_str = libcdoc::CDoc2::getSaltForExpand(key_material, lock.getBytes(Lock::Params::RCPT_KEY));
203-
204224
LOG_DBG("info: {}", toHex(info_str));
205-
206225
kek = libcdoc::Crypto::expand(kek_pm, info_str, libcdoc::CDoc2::KEY_LEN);
207226
}
208227
} else if (lock.type == Lock::Type::SHARE_SERVER) {
@@ -312,7 +331,6 @@ CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
312331

313332
LOG_TRACE_KEY("KEK: {}", kek);
314333

315-
316334
if(kek.empty()) {
317335
setLastError(t_("Failed to derive KEK"));
318336
LOG_ERROR("{}", last_error);
@@ -394,10 +412,10 @@ CDoc2Reader::beginDecryption(const std::vector<uint8_t>& fmk)
394412
std::vector<uint8_t> aad(libcdoc::CDoc2::PAYLOAD.cbegin(), libcdoc::CDoc2::PAYLOAD.cend());
395413
aad.insert(aad.end(), priv->header_data.cbegin(), priv->header_data.cend());
396414
aad.insert(aad.end(), priv->headerHMAC.cbegin(), priv->headerHMAC.cend());
397-
if(priv->dec->updateAAD(aad) != OK) {
398-
setLastError("Wrong decryption key (FMK)");
415+
if(auto rv = priv->dec->updateAAD(aad); rv != OK) {
416+
setLastError(priv->dec->getLastErrorStr(rv));
399417
LOG_ERROR("{}", last_error);
400-
return libcdoc::WRONG_KEY;
418+
return rv;
401419
}
402420

403421
priv->zsrc = std::make_unique<libcdoc::ZSource>(priv->dec.get(), false);
@@ -414,8 +432,13 @@ CDoc2Reader::nextFile(std::string& name, int64_t& size)
414432
LOG_ERROR("{}", last_error);
415433
return libcdoc::WORKFLOW_ERROR;
416434
}
417-
result_t result = priv->tar->next(name, size);
418-
if (result != OK) {
435+
result_t result = priv->tar->next(name, size);
436+
if (result < 0) {
437+
result_t sr = priv->decryptAllAndClose();
438+
if (sr != OK) {
439+
setLastError("Crypto payload integrity check failed");
440+
return sr;
441+
}
419442
setLastError(priv->tar->getLastErrorStr(result));
420443
}
421444
return result;
@@ -430,7 +453,12 @@ CDoc2Reader::readData(uint8_t *dst, size_t size)
430453
return libcdoc::WORKFLOW_ERROR;
431454
}
432455
result_t result = priv->tar->read(dst, size);
433-
if (result != OK) {
456+
if (result < 0) {
457+
result_t sr = priv->decryptAllAndClose();
458+
if (sr != OK) {
459+
setLastError("Crypto payload integrity check failed");
460+
return sr;
461+
}
434462
setLastError(priv->tar->getLastErrorStr(result));
435463
}
436464
return result;
@@ -439,11 +467,15 @@ CDoc2Reader::readData(uint8_t *dst, size_t size)
439467
libcdoc::result_t
440468
CDoc2Reader::finishDecryption()
441469
{
470+
if (!priv->tar) {
471+
setLastError("finishDecryption() called before beginDecryption()");
472+
LOG_ERROR("{}", last_error);
473+
return libcdoc::WORKFLOW_ERROR;
474+
}
442475
if (!priv->zsrc->isEof()) {
443476
setLastError(t_("CDoc contains additional payload data that is not part of content"));
444477
LOG_WARN("{}", last_error);
445478
}
446-
447479
setLastError({});
448480
priv->zsrc.reset();
449481
priv->tar.reset();

‎cdoc/CDoc2Writer.cpp‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,11 @@ CDoc2Writer::buildHeader(std::vector<uint8_t>& header, const std::vector<libcdoc
457457
libcdoc::result_t
458458
CDoc2Writer::addRecipient(const libcdoc::Recipient& rcpt)
459459
{
460+
if (finished) {
461+
setLastError("Encryption finished");
462+
LOG_ERROR("{}", last_error);
463+
return libcdoc::WORKFLOW_ERROR;
464+
}
460465
if(tar) {
461466
setLastError("Cannot add Recipient when files are added");
462467
LOG_ERROR("{}", last_error);
@@ -469,6 +474,11 @@ CDoc2Writer::addRecipient(const libcdoc::Recipient& rcpt)
469474
libcdoc::result_t
470475
CDoc2Writer::beginEncryption()
471476
{
477+
if (finished) {
478+
setLastError("Encryption finished");
479+
LOG_ERROR("{}", last_error);
480+
return libcdoc::WORKFLOW_ERROR;
481+
}
472482
last_error.clear();
473483
if(recipients.empty()) {
474484
setLastError("No recipients added");
@@ -488,12 +498,26 @@ CDoc2Writer::beginEncryption()
488498
libcdoc::result_t
489499
CDoc2Writer::addFile(const std::string& name, size_t size)
490500
{
501+
if (finished) {
502+
setLastError("Encryption finished");
503+
LOG_ERROR("{}", last_error);
504+
return libcdoc::WORKFLOW_ERROR;
505+
}
491506
if(!tar) {
492507
setLastError("Encryption not started");
493508
LOG_ERROR("{}", last_error);
494509
return libcdoc::WORKFLOW_ERROR;
495510
}
496-
if (name.empty() || !libcdoc::isValidUtf8(name)) return libcdoc::DATA_FORMAT_ERROR;
511+
if (name.empty() || !libcdoc::isValidUtf8(name)) {
512+
setLastError("Invalid file name");
513+
LOG_ERROR("{}", last_error);
514+
return libcdoc::DATA_FORMAT_ERROR;
515+
}
516+
if (size > 8ULL * 1024 * 1024 * 1024) {
517+
setLastError("Invalid file size");
518+
LOG_ERROR("{}", last_error);
519+
return libcdoc::WRONG_ARGUMENTS;
520+
}
497521
if(auto rv = tar->open(name, size); rv < 0) {
498522
setLastError(tar->getLastErrorStr(rv));
499523
LOG_ERROR("{}", last_error);
@@ -505,6 +529,11 @@ CDoc2Writer::addFile(const std::string& name, size_t size)
505529
libcdoc::result_t
506530
CDoc2Writer::writeData(const uint8_t *src, size_t size)
507531
{
532+
if (finished) {
533+
setLastError("Encryption finished");
534+
LOG_ERROR("{}", last_error);
535+
return libcdoc::WORKFLOW_ERROR;
536+
}
508537
if(!tar) {
509538
setLastError("No file added");
510539
LOG_ERROR("{}", last_error);
@@ -520,6 +549,11 @@ CDoc2Writer::writeData(const uint8_t *src, size_t size)
520549
libcdoc::result_t
521550
CDoc2Writer::finishEncryption()
522551
{
552+
if (finished) {
553+
setLastError("Encryption finished");
554+
LOG_ERROR("{}", last_error);
555+
return libcdoc::WORKFLOW_ERROR;
556+
}
523557
if(!tar) {
524558
setLastError("No file added");
525559
LOG_ERROR("{}", last_error);
@@ -531,12 +565,18 @@ CDoc2Writer::finishEncryption()
531565
tar.reset();
532566
recipients.clear();
533567
if (owned) dst->close();
568+
finished = true;
534569
return rv;
535570
}
536571

537572
libcdoc::result_t
538573
CDoc2Writer::encrypt(libcdoc::MultiDataSource& src, const std::vector<libcdoc::Recipient>& keys)
539574
{
575+
if (finished) {
576+
setLastError("Encryption finished");
577+
LOG_ERROR("{}", last_error);
578+
return libcdoc::WORKFLOW_ERROR;
579+
}
540580
for (auto rcpt : keys) {
541581
if(auto rv = addRecipient(rcpt); rv != libcdoc::OK)
542582
return rv;

‎cdoc/CDoc2Writer.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class CDoc2Writer final: public libcdoc::CDocWriter {
4646

4747
std::unique_ptr<libcdoc::TarConsumer> tar;
4848
std::vector<libcdoc::Recipient> recipients;
49+
bool finished = false;
4950
};
5051

5152
}

‎cdoc/CryptoBackend.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ CryptoBackend::getKeyMaterial(std::vector<uint8_t>& key_material, const std::vec
8282
if (pw_salt.empty()) return INVALID_PARAMS;
8383
std::vector<uint8_t> secret;
8484
int result = getSecret(secret, idx);
85-
if (result < 0) return result;
85+
if (result) return result;
8686

8787
LOG_DBG("Secret: {}", toHex(secret));
8888

@@ -91,7 +91,7 @@ CryptoBackend::getKeyMaterial(std::vector<uint8_t>& key_material, const std::vec
9191
if (key_material.empty()) return OPENSSL_ERROR;
9292
} else {
9393
int result = getSecret(key_material, idx);
94-
if (result < 0) return result;
94+
if (result) return result;
9595
LOG_DBG("Secret: {}", toHex(key_material));
9696
if (key_material.size() != 32) {
9797
return INVALID_PARAMS;

‎cdoc/Io.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ struct CDOC_EXPORT IStreamSource : public DataSource {
255255
if (_owned) delete _ifs;
256256
}
257257

258-
result_t seek(size_t pos) {
258+
result_t seek(size_t pos) override {
259259
if(_ifs->bad()) return INPUT_STREAM_ERROR;
260260
_ifs->clear();
261261
_ifs->seekg(pos);

‎cdoc/Tar.cpp‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ libcdoc::TarConsumer::~TarConsumer()
130130
libcdoc::result_t
131131
libcdoc::TarConsumer::write(const uint8_t *src, size_t size) noexcept
132132
{
133+
if ((_current_size >= 0) && ((_current_written + size) > _current_size)) {
134+
return WORKFLOW_ERROR;
135+
}
136+
_current_written += size;
133137
return _dst->write(src, size);
134138
}
135139

@@ -160,19 +164,25 @@ libcdoc::TarConsumer::writePadding(int64_t size) noexcept {
160164
libcdoc::result_t
161165
libcdoc::TarConsumer::close() noexcept
162166
{
163-
if (_current_size > 0) {
164-
if(auto rv = writePadding(_current_size); rv != OK)
165-
return rv;
166-
}
167-
Header empty = {};
168-
if(auto rv = writeHeader(empty); rv != OK)
169-
return rv;
170-
if(auto rv = writeHeader(empty); rv != OK)
171-
return rv;
167+
result_t result = OK;
168+
if ((_current_size >= 0) && (_current_written < _current_size)) {
169+
result = DATA_FORMAT_ERROR;
170+
} else {
171+
if (_current_written > 0) {
172+
if(auto rv = writePadding(_current_written); rv != OK)
173+
return rv;
174+
}
175+
Header empty = {};
176+
if(auto rv = writeHeader(empty); rv != OK)
177+
return rv;
178+
if(auto rv = writeHeader(empty); rv != OK)
179+
return rv;
180+
}
172181
if (_owned) {
173-
return _dst->close();
182+
if (auto rv = _dst->close(); rv != OK)
183+
return rv;
174184
}
175-
return OK;
185+
return result;
176186
}
177187

178188
bool
@@ -184,12 +194,16 @@ libcdoc::TarConsumer::isError() noexcept
184194
libcdoc::result_t
185195
libcdoc::TarConsumer::open(const std::string& name, int64_t size)
186196
{
187-
if (_current_size > 0) {
188-
if(auto rv = writePadding(_current_size); rv != OK)
197+
if ((_current_size >= 0) && (_current_written < _current_size)) {
198+
return WORKFLOW_ERROR;
199+
}
200+
if (_current_written > 0) {
201+
if(auto rv = writePadding(_current_written); rv != OK)
189202
return rv;
190203
}
191204

192205
_current_size = size;
206+
_current_written = 0;
193207
Header h {};
194208
size_t len = std::min(name.size(), h.name.size());
195209
std::copy_n(name.cbegin(), len, h.name.begin());

0 commit comments

Comments
 (0)