Skip to content

Coverity fixes #125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
metsma wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Coverity fixes #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cdoc/CDoc2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ static constexpr std::string_view CEK = "CDOC20cek";
static constexpr std::string_view HMAC = "CDOC20hmac";
static constexpr std::string_view KEK = "CDOC20kek";
static constexpr std::string_view KEKPREMASTER = "CDOC20kekpremaster";
static constexpr std::string_view PAYLOAD = "CDOC20payload";
static const std::vector<uint8_t> PAYLOAD = {'C', 'D', 'O', 'C', '2', '0', 'p', 'a', 'y', 'l', 'o', 'a', 'd'};
static constexpr std::string_view SALT = "CDOC20salt";

static constexpr int KEY_LEN = 32;
Expand Down
21 changes: 10 additions & 11 deletions cdoc/CDoc2Reader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,10 @@ CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
LOG_ERROR("Cannot fetch share {}", i);
return result;
}
if (Crypto::xor_data(kek, kek, share.share) != libcdoc::OK) {
if (auto err = libcdoc::Crypto::xor_data(kek, kek, share.share); err != libcdoc::OK) {
setLastError("Failed to derive kek");
LOG_ERROR("Failed to derive kek");
return libcdoc::CRYPTO_ERROR;
return err;
}
}
LOG_INFO("Fetched all shares");
Expand All @@ -338,10 +338,10 @@ CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
LOG_ERROR("{}", last_error);
return CRYPTO_ERROR;
}
if (libcdoc::Crypto::xor_data(fmk, lock.encrypted_fmk, kek) != libcdoc::OK) {
if (auto err = libcdoc::Crypto::xor_data(fmk, lock.encrypted_fmk, kek); err != libcdoc::OK) {
setLastError(t_("Failed to decrypt/derive fmk"));
LOG_ERROR("{}", last_error);
return libcdoc::CRYPTO_ERROR;
return err;
}
std::vector<uint8_t> hhk = libcdoc::Crypto::expand(fmk, libcdoc::CDoc2::HMAC);

Expand Down Expand Up @@ -410,13 +410,12 @@ CDoc2Reader::beginDecryption(const std::vector<uint8_t>& fmk)
LOG_TRACE_KEY("cek: {}", cek);

priv->dec = std::make_unique<libcdoc::DecryptionSource>(*priv->_src, EVP_chacha20_poly1305(), cek, libcdoc::CDoc2::NONCE_LEN);
std::vector<uint8_t> 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) {
setLastError(priv->dec->getLastErrorStr(rv));
LOG_ERROR("{}", last_error);
return rv;
for(const auto &aad: {libcdoc::CDoc2::PAYLOAD, priv->header_data, priv->headerHMAC}) {
if(auto rv = priv->dec->updateAAD(aad); rv != OK) {
setLastError(priv->dec->getLastErrorStr(rv));
LOG_ERROR("{}", last_error);
return rv;
}
}

priv->zsrc = std::make_unique<libcdoc::ZSource>(priv->dec.get(), false);
Expand Down
65 changes: 32 additions & 33 deletions cdoc/CDoc2Writer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,11 @@ CDoc2Writer::writeHeader(const std::vector<libcdoc::Recipient> &recipients)
std::vector<uint8_t> nonce;
crypto->random(nonce, libcdoc::CDoc2::NONCE_LEN);
LOG_TRACE_KEY("nonce: {}", nonce);
auto cipher = std::make_unique<EncryptionConsumer>(*dst, EVP_chacha20_poly1305(), Crypto::Key(std::move(cek), nonce));
std::vector<uint8_t> aad(libcdoc::CDoc2::PAYLOAD.cbegin(), libcdoc::CDoc2::PAYLOAD.cend());
aad.insert(aad.end(), header.cbegin(), header.cend());
aad.insert(aad.end(), headerHMAC.cbegin(), headerHMAC.cend());
if(auto rv = cipher->writeAAD(aad); rv < 0)
return rv;
auto cipher = std::make_unique<EncryptionConsumer>(*dst, EVP_chacha20_poly1305(), Crypto::Key(std::move(cek), std::move(nonce)));
for(const auto &aad: {libcdoc::CDoc2::PAYLOAD, std::move(header), std::move(headerHMAC)}) {
if(auto rv = cipher->writeAAD(aad); rv < 0)
return rv;
}

auto *zcons = new ZConsumer(cipher.release(), true);
tar = std::make_unique<TarConsumer>(zcons, true);
Expand Down Expand Up @@ -197,7 +196,7 @@ CDoc2Writer::buildHeader(std::vector<uint8_t>& header, const std::vector<libcdoc
flatbuffers::FlatBufferBuilder builder;
std::vector<flatbuffers::Offset<cdoc20::header::RecipientRecord>> fb_rcpts;

std::vector<uint8_t> xor_key(libcdoc::CDoc2::KEY_LEN);
std::vector<uint8_t> xor_key;
for (unsigned int rcpt_idx = 0; rcpt_idx < recipients.size(); rcpt_idx++) {
const libcdoc::Recipient& rcpt = recipients.at(rcpt_idx);
if (rcpt.isPKI()) {
Expand All @@ -214,8 +213,8 @@ CDoc2Writer::buildHeader(std::vector<uint8_t>& header, const std::vector<libcdoc
}
if(rcpt.pk_type == libcdoc::PKType::RSA) {
crypto->random(kek, libcdoc::CDoc2::KEY_LEN);
if (libcdoc::Crypto::xor_data(xor_key, fmk, kek) != libcdoc::OK)
FAIL("Internal error", libcdoc::CRYPTO_ERROR);
if (auto err = libcdoc::Crypto::xor_data(xor_key, fmk, kek); err != libcdoc::OK)
FAIL("Internal error", err);
auto publicKey = libcdoc::Crypto::fromRSAPublicKeyDer(rcpt.rcpt_key);
if(!publicKey)
FAIL("Invalid RSA key", libcdoc::CRYPTO_ERROR);
Expand Down Expand Up @@ -249,8 +248,8 @@ CDoc2Writer::buildHeader(std::vector<uint8_t>& header, const std::vector<libcdoc
std::string info_str = libcdoc::CDoc2::getSaltForExpand(key_material, rcpt.rcpt_key);

kek = libcdoc::Crypto::expand(kekPm, info_str, fmk.size());
if (libcdoc::Crypto::xor_data(xor_key, fmk, kek) != libcdoc::OK)
FAIL("Internal error", libcdoc::CRYPTO_ERROR);
if (auto err = libcdoc::Crypto::xor_data(xor_key, fmk, kek); err != libcdoc::OK)
FAIL("Internal error", err);

LOG_DBG("info: {}", toHex(info_str));
LOG_TRACE_KEY("publicKeyDer: {}", rcpt.rcpt_key);
Expand Down Expand Up @@ -298,8 +297,8 @@ CDoc2Writer::buildHeader(std::vector<uint8_t>& header, const std::vector<libcdoc
LOG_TRACE_KEY("kek: {}", kek);

if (kek.empty()) return libcdoc::CRYPTO_ERROR;
if (libcdoc::Crypto::xor_data(xor_key, fmk, kek) != libcdoc::OK)
FAIL("Internal error", libcdoc::CRYPTO_ERROR);
if (auto err = libcdoc::Crypto::xor_data(xor_key, fmk, kek); err != libcdoc::OK)
FAIL("Internal error", err);
if (rcpt.kdf_iter > 0) {
fb_rcpts.push_back(createPasswordCapsule(builder, rcpt, salt, pw_salt, xor_key));
} else {
Expand Down Expand Up @@ -341,8 +340,8 @@ CDoc2Writer::buildHeader(std::vector<uint8_t>& header, const std::vector<libcdoc
std::vector<uint8_t> kek = libcdoc::Crypto::expand(kek_pm, info_str);
LOG_TRACE_KEY("kek: {}", kek);
if (kek.empty()) return libcdoc::CRYPTO_ERROR;
if (libcdoc::Crypto::xor_data(xor_key, fmk, kek) != libcdoc::OK)
FAIL("Internal error", libcdoc::CRYPTO_ERROR);
if (auto err = libcdoc::Crypto::xor_data(xor_key, fmk, kek); err != libcdoc::OK)
FAIL("Internal error", err);

// # Splitting KEK_i into shares
// for j in (2, 3, ..., n):
Expand All @@ -354,8 +353,8 @@ CDoc2Writer::buildHeader(std::vector<uint8_t>& header, const std::vector<libcdoc
// KEK_i_share_1 = XOR(KEK_i, KEK_i_share_2, KEK_i_share_3,..., KEK_i_share_n)
kek_shares[0] = std::move(kek);
for (int i = 1; i < N_SHARES; i++) {
if (libcdoc::Crypto::xor_data(kek_shares[0], kek_shares[0], kek_shares[i]) != libcdoc::OK)
FAIL("Internal error", libcdoc::CRYPTO_ERROR);
if (auto err = libcdoc::Crypto::xor_data(kek_shares[0], kek_shares[0], kek_shares[i]); err != libcdoc::OK)
FAIL("Internal error", err);
}
// # Client uploads all shares of KEK_i to CSS servers and
// # gets corresponding Capsule_i_Share_j_ID for each KEK_i_share_j
Expand Down Expand Up @@ -412,20 +411,20 @@ CDoc2Writer::addRecipient(const libcdoc::Recipient& rcpt)
FAIL("Cannot add Recipient when files are added", libcdoc::WORKFLOW_ERROR);
// Validate
switch(rcpt.type) {
case Recipient::SYMMETRIC_KEY:
if (!crypto)
FAIL("Symmetric key requires CryptoBackend", libcdoc::WORKFLOW_ERROR);
if(!rcpt.validate())
FAIL("Invalid recipient parameters", libcdoc::WRONG_ARGUMENTS);
break;
case Recipient::PUBLIC_KEY:
if(!rcpt.server_id.empty() && !network)
FAIL("Server capsule requires NetworkBackend", libcdoc::WORKFLOW_ERROR);
if(!rcpt.validate())
FAIL("Invalid recipient parameters", libcdoc::WRONG_ARGUMENTS);
break;
default:
FAIL("Invalid recipient type", WRONG_ARGUMENTS);
case Recipient::SYMMETRIC_KEY:
if (!crypto)
FAIL("Symmetric key requires CryptoBackend", libcdoc::WORKFLOW_ERROR);
if(!rcpt.validate())
FAIL("Invalid recipient parameters", libcdoc::WRONG_ARGUMENTS);
break;
case Recipient::PUBLIC_KEY:
if(!rcpt.server_id.empty() && !network)
FAIL("Server capsule requires NetworkBackend", libcdoc::WORKFLOW_ERROR);
if(!rcpt.validate())
FAIL("Invalid recipient parameters", libcdoc::WRONG_ARGUMENTS);
break;
default:
FAIL("Invalid recipient type", WRONG_ARGUMENTS);
}
recipients.push_back(rcpt);
return libcdoc::OK;
Expand Down Expand Up @@ -455,7 +454,7 @@ CDoc2Writer::addFile(const std::string& name, size_t size)
FAIL("Encryption not started", libcdoc::WORKFLOW_ERROR);
if (name.empty() || !libcdoc::isValidUtf8(name))
FAIL("Invalid file name", libcdoc::DATA_FORMAT_ERROR);
if (size > 8ULL * 1024 * 1024 * 1024)
if (constexpr size_t MAX_SIZE = 8ULL * 1024 * 1024 * 1024; size > MAX_SIZE)
FAIL("Invalid file size", libcdoc::WRONG_ARGUMENTS);
if(auto rv = tar->open(name, size); rv < 0)
FAIL(tar->getLastErrorStr(rv), rv);
Expand Down Expand Up @@ -496,7 +495,7 @@ CDoc2Writer::encrypt(libcdoc::MultiDataSource& src, const std::vector<libcdoc::R
{
if (finished)
FAIL("Encryption finished", libcdoc::WORKFLOW_ERROR);
for (auto rcpt : keys) {
for (const auto &rcpt : keys) {
if(auto rv = addRecipient(rcpt); rv != libcdoc::OK)
return rv;
}
Expand Down
58 changes: 12 additions & 46 deletions cdoc/CDocCipher.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,13 @@

#include <sstream>
#include <map>
#include <openssl/rand.h>
#include <openssl/x509.h>
#include <openssl/evp.h>
#include <openssl/rsa.h>

using namespace std;
using namespace libcdoc;

static string GenerateRandomSequence();

struct ToolPKCS11 : public libcdoc::PKCS11Backend {
const std::map<unsigned int, libcdoc::RcptInfo>& rcpts;

Expand Down Expand Up @@ -271,18 +268,18 @@ fill_recipients_from_rcpt_info(ToolConf& conf, ToolCrypto& crypto, std::vector<l
Recipient key;
if (rcpt.type == RcptInfo::Type::CERT) {
if (!conf.servers.empty()) {
key = libcdoc::Recipient::makeServer(label, rcpt.cert, conf.servers[0].ID);
key = libcdoc::Recipient::makeServer(std::move(label), rcpt.cert, conf.servers[0].ID);
} else {
key = libcdoc::Recipient::makeCertificate(label, rcpt.cert);
key = libcdoc::Recipient::makeCertificate(std::move(label), rcpt.cert);
}
} else if (rcpt.type == RcptInfo::Type::SKEY) {
key = libcdoc::Recipient::makeSymmetric(label, 0);
key = libcdoc::Recipient::makeSymmetric(std::move(label), 0);
if (conf.gen_label)
key.setLabelValue(CDoc2::Label::LABEL, rcpt.label);
LOG_DBG("Creating symmetric key:");
} else if (rcpt.type == RcptInfo::Type::PKEY) {
if (!conf.servers.empty()) {
key = libcdoc::Recipient::makeServer(label, rcpt.secret, libcdoc::PKType::ECC, conf.servers[0].ID);
key = libcdoc::Recipient::makeServer(std::move(label), rcpt.secret, libcdoc::PKType::ECC, conf.servers[0].ID);
} else {
const uint8_t *der = rcpt.secret.data();
EVP_PKEY *pkey = d2i_PUBKEY(nullptr, &der, rcpt.secret.size());
Expand All @@ -291,14 +288,14 @@ fill_recipients_from_rcpt_info(ToolConf& conf, ToolCrypto& crypto, std::vector<l
uint8_t *p = d.data();
i2d_PublicKey(pkey, &p);
if (id == EVP_PKEY_EC) {
key = libcdoc::Recipient::makePublicKey(label, rcpt.secret, libcdoc::PKType::ECC);
key = libcdoc::Recipient::makePublicKey(std::move(label), rcpt.secret, libcdoc::PKType::ECC);
} else if (id == EVP_PKEY_RSA) {
key = libcdoc::Recipient::makePublicKey(label, rcpt.secret, libcdoc::PKType::RSA);
key = libcdoc::Recipient::makePublicKey(std::move(label), rcpt.secret, libcdoc::PKType::RSA);
}
}
LOG_DBG("Creating public key:");
} else if (rcpt.type == RcptInfo::Type::P11_SYMMETRIC) {
key = libcdoc::Recipient::makeSymmetric(label, 0);
key = libcdoc::Recipient::makeSymmetric(std::move(label), 0);
if (conf.gen_label)
key.setLabelValue(CDoc2::Label::LABEL, rcpt.label);
} else if (rcpt.type == RcptInfo::Type::P11_PKI) {
Expand All @@ -312,19 +309,19 @@ fill_recipients_from_rcpt_info(ToolConf& conf, ToolCrypto& crypto, std::vector<l
}
LOG_DBG("Public key ({}): {}", rsa ? "rsa" : "ecc", toHex(val));
if (!conf.servers.empty()) {
key = libcdoc::Recipient::makeServer(label, val, rsa ? libcdoc::PKType::RSA : libcdoc::PKType::ECC, conf.servers[0].ID);
key = libcdoc::Recipient::makeServer(std::move(label), std::move(val), rsa ? libcdoc::PKType::RSA : libcdoc::PKType::ECC, conf.servers[0].ID);
} else {
key = libcdoc::Recipient::makePublicKey(label, val, rsa ? libcdoc::PKType::RSA : libcdoc::PKType::ECC);
key = libcdoc::Recipient::makePublicKey(std::move(label), std::move(val), rsa ? libcdoc::PKType::RSA : libcdoc::PKType::ECC);
}
} else if (rcpt.type == RcptInfo::Type::PASSWORD) {
LOG_DBG("Creating password key:");
key = libcdoc::Recipient::makeSymmetric(label, 65535);
key = libcdoc::Recipient::makeSymmetric(std::move(label), 65535);
if (conf.gen_label)
key.setLabelValue(CDoc2::Label::LABEL, rcpt.label);
#ifdef HAS_KEYSHARES
} else if (rcpt.type == RcptInfo::Type::SHARE) {
LOG_DBG("Creating keyshare recipient:");
key = libcdoc::Recipient::makeShare(label, conf.servers[0].ID, "PNOEE-" + rcpt.id);
key = libcdoc::Recipient::makeShare(std::move(label), conf.servers[0].ID, "PNOEE-" + rcpt.id);
#endif
}

Expand Down Expand Up @@ -679,6 +676,7 @@ void CDocCipher::Locks(const char* file) const
return;
}

restoreFlags rf(cout);
int lock_id = 1;
for (const Lock& lock : rdr->getLocks()) {
map<string, string> parsed_label(Lock::parseLabel(lock.label));
Expand Down Expand Up @@ -707,35 +705,3 @@ void CDocCipher::Locks(const char* file) const
lock_id++;
}
}

static string GenerateRandomSequence()
{
constexpr uint32_t upperbound = 'z' - '0' + 1;
constexpr int MaxSequenceLength = 11;

uint32_t rnd;
uint8_t rndByte;
ostringstream sequence;
for (int cnt = 0; cnt < MaxSequenceLength;)
{
if (RAND_bytes(&rndByte, 1) < 1)
{
rnd = rand() % upperbound + '0';
}
else
{
rnd = rndByte % upperbound + '0';
}

// arc4random_uniform tends to be not available on all platforms.
// rnd = arc4random_uniform(upperbound) + '0';

if (isalnum(rnd))
{
sequence << static_cast<char>(rnd);
cnt++;
}
}

return sequence.str();
}
6 changes: 5 additions & 1 deletion cdoc/DDocWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ constexpr XMLWriter::NS DDOC{ nullptr, "http://www.sk.ee/DigiDoc/v1.3.0#" };

DDOCWriter::DDOCWriter(DataConsumer &dst)
: XMLWriter(dst)
, state(writeStartElement(DDOC, "SignedDoc", {{"format", "DIGIDOC-XML"}, {"version", "1.3"}}))
{
writeStartElement(DDOC, "SignedDoc", {{"format", "DIGIDOC-XML"}, {"version", "1.3"}});
}

DDOCWriter::~DDOCWriter() noexcept
Expand All @@ -49,6 +49,8 @@ DDOCWriter::~DDOCWriter() noexcept
*/
int64_t DDOCWriter::addFile(const std::string &file, const std::string &mime, size_t size, libcdoc::DataSource& src)
{
if (state != OK)
return state;
return writeBase64Element(DDOC, "DataFile", [&src](DataConsumer &dst){ return src.readAll(dst); }, {
{"ContentType", "EMBEDDED_BASE64"},
{"Filename", file},
Expand All @@ -66,6 +68,8 @@ int64_t DDOCWriter::addFile(const std::string &file, const std::string &mime, si
*/
int64_t DDOCWriter::addFile(const std::string &file, const std::string &mime, const std::vector<unsigned char> &data)
{
if (state != OK)
return state;
return writeBase64Element(DDOC, "DataFile", data, {
{"ContentType", "EMBEDDED_BASE64"},
{"Filename", file},
Expand Down
1 change: 1 addition & 0 deletions cdoc/DDocWriter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class DDOCWriter final: public XMLWriter
private:
DDOCWriter(const DDOCWriter &) = delete;
DDOCWriter &operator=(const DDOCWriter &) = delete;
int64_t state;
int fileCount = 0;
};

Expand Down
5 changes: 0 additions & 5 deletions cdoc/Lock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,6 @@ Lock::parseLabel(const std::string& label)
label_to_prcss = label_wo_prefix;
}

auto range_to_sv = [](auto range) constexpr {
if (range.empty())
return std::string_view();
return std::string_view(&*range.begin(), std::ranges::distance(range));
};
for (const auto &part : std::ranges::split_view(label_to_prcss, '&'))
{
auto label_data_parts = std::ranges::split_view(part, '=');
Expand Down
Loading
Loading