Skip to content

Commit c9beac3

Browse files
author
Lauris Kaplinski
committed
Added tests for basic encryption/decryption workflow errors
Signed-off-by: Lauris Kaplinski <lauris@raulwalter.com>
1 parent 8309182 commit c9beac3

File tree

8 files changed

+274
-96
lines changed

8 files changed

+274
-96
lines changed

‎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: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -118,35 +118,44 @@ CDoc2Reader::getLockForCert(const std::vector<uint8_t>& cert){
118118
libcdoc::result_t
119119
CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
120120
{
121+
if (lock_idx >= priv->locks.size()) {
122+
setLastError(t_("Invalid lock index"));
123+
LOG_ERROR("{}", last_error);
124+
return libcdoc::WRONG_ARGUMENTS;
125+
}
121126
LOG_DBG("CDoc2Reader::getFMK: {}", lock_idx);
122127
LOG_DBG("CDoc2Reader::num locks: {}", priv->locks.size());
123128
const Lock& lock = priv->locks.at(lock_idx);
129+
LOG_DBG("Label: {}", lock.label);
124130
std::vector<uint8_t> kek;
125131
if (lock.type == Lock::Type::PASSWORD) {
126132
// Password
127133
LOG_DBG("password");
128134
std::string info_str = libcdoc::CDoc2::getSaltForExpand(lock.label);
135+
LOG_DBG("info: {}", toHex(info_str));
129136
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");
137+
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) {
138+
setLastError(crypto->getLastErrorStr(rv));
139+
LOG_ERROR("{}", last_error);
140+
return rv;
141+
}
142+
LOG_TRACE_KEY("salt: {}", lock.getBytes(Lock::SALT));
143+
LOG_TRACE_KEY("kek_pm: {}", kek_pm);
132144
kek = libcdoc::Crypto::expand(kek_pm, info_str, 32);
133-
if (kek.empty()) return libcdoc::CRYPTO_ERROR;
134-
LOG_DBG("password3");
135145
} else if (lock.type == Lock::Type::SYMMETRIC_KEY) {
136146
// Symmetric key
137147
LOG_DBG("symmetric");
138148
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);
144149
LOG_DBG("info: {}", toHex(info_str));
150+
std::vector<uint8_t> kek_pm;
151+
if (auto rv = crypto->extractHKDF(kek_pm, lock.getBytes(Lock::SALT), {}, 0, lock_idx); rv != libcdoc::OK) {
152+
setLastError(crypto->getLastErrorStr(rv));
153+
LOG_ERROR("{}", last_error);
154+
return rv;
155+
}
145156
LOG_TRACE_KEY("salt: {}", lock.getBytes(Lock::SALT));
146157
LOG_TRACE_KEY("kek_pm: {}", kek_pm);
147-
LOG_TRACE_KEY("kek: {}", kek);
148-
149-
if (kek.empty()) return libcdoc::CRYPTO_ERROR;
158+
kek = libcdoc::Crypto::expand(kek_pm, info_str, 32);
150159
} else if ((lock.type == Lock::Type::PUBLIC_KEY) || (lock.type == Lock::Type::SERVER)) {
151160
// Public/private key
152161
std::vector<uint8_t> key_material;
@@ -196,13 +205,9 @@ CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
196205
LOG_ERROR("{}", last_error);
197206
return result;
198207
}
199-
200208
LOG_TRACE_KEY("Key kekPm: {}", kek_pm);
201-
202209
std::string info_str = libcdoc::CDoc2::getSaltForExpand(key_material, lock.getBytes(Lock::Params::RCPT_KEY));
203-
204210
LOG_DBG("info: {}", toHex(info_str));
205-
206211
kek = libcdoc::Crypto::expand(kek_pm, info_str, libcdoc::CDoc2::KEY_LEN);
207212
}
208213
} else if (lock.type == Lock::Type::SHARE_SERVER) {
@@ -312,7 +317,6 @@ CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
312317

313318
LOG_TRACE_KEY("KEK: {}", kek);
314319

315-
316320
if(kek.empty()) {
317321
setLastError(t_("Failed to derive KEK"));
318322
LOG_ERROR("{}", last_error);
@@ -394,10 +398,10 @@ CDoc2Reader::beginDecryption(const std::vector<uint8_t>& fmk)
394398
std::vector<uint8_t> aad(libcdoc::CDoc2::PAYLOAD.cbegin(), libcdoc::CDoc2::PAYLOAD.cend());
395399
aad.insert(aad.end(), priv->header_data.cbegin(), priv->header_data.cend());
396400
aad.insert(aad.end(), priv->headerHMAC.cbegin(), priv->headerHMAC.cend());
397-
if(priv->dec->updateAAD(aad) != OK) {
398-
setLastError("Wrong decryption key (FMK)");
401+
if(auto rv = priv->dec->updateAAD(aad); rv != OK) {
402+
setLastError(priv->dec->getLastErrorStr(rv));
399403
LOG_ERROR("{}", last_error);
400-
return libcdoc::WRONG_KEY;
404+
return rv;
401405
}
402406

403407
priv->zsrc = std::make_unique<libcdoc::ZSource>(priv->dec.get(), false);

‎cdoc/CDoc2Writer.cpp‎

Lines changed: 31 additions & 0 deletions
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,6 +498,11 @@ 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);
@@ -505,6 +520,11 @@ CDoc2Writer::addFile(const std::string& name, size_t size)
505520
libcdoc::result_t
506521
CDoc2Writer::writeData(const uint8_t *src, size_t size)
507522
{
523+
if (finished) {
524+
setLastError("Encryption finished");
525+
LOG_ERROR("{}", last_error);
526+
return libcdoc::WORKFLOW_ERROR;
527+
}
508528
if(!tar) {
509529
setLastError("No file added");
510530
LOG_ERROR("{}", last_error);
@@ -520,6 +540,11 @@ CDoc2Writer::writeData(const uint8_t *src, size_t size)
520540
libcdoc::result_t
521541
CDoc2Writer::finishEncryption()
522542
{
543+
if (finished) {
544+
setLastError("Encryption finished");
545+
LOG_ERROR("{}", last_error);
546+
return libcdoc::WORKFLOW_ERROR;
547+
}
523548
if(!tar) {
524549
setLastError("No file added");
525550
LOG_ERROR("{}", last_error);
@@ -531,12 +556,18 @@ CDoc2Writer::finishEncryption()
531556
tar.reset();
532557
recipients.clear();
533558
if (owned) dst->close();
559+
finished = true;
534560
return rv;
535561
}
536562

537563
libcdoc::result_t
538564
CDoc2Writer::encrypt(libcdoc::MultiDataSource& src, const std::vector<libcdoc::Recipient>& keys)
539565
{
566+
if (finished) {
567+
setLastError("Encryption finished");
568+
LOG_ERROR("{}", last_error);
569+
return libcdoc::WORKFLOW_ERROR;
570+
}
540571
for (auto rcpt : keys) {
541572
if(auto rv = addRecipient(rcpt); rv != libcdoc::OK)
542573
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)