From ae13e7e4bfd58aa41d7f05c25294381d051f0c6f Mon Sep 17 00:00:00 2001 From: Cyril Soler Date: Mon, 25 May 2026 16:37:07 +0200 Subject: [PATCH] hardenned the security in saving config files, avoiding corruption in some rare situations (disk full + large config file) --- src/pqi/p3cfgmgr.cc | 82 +++++++++++++++++++++------------------------ src/pqi/pqistore.cc | 25 ++++++++++---- 2 files changed, 57 insertions(+), 50 deletions(-) diff --git a/src/pqi/p3cfgmgr.cc b/src/pqi/p3cfgmgr.cc index 2b49623ba..56bb01cd9 100644 --- a/src/pqi/p3cfgmgr.cc +++ b/src/pqi/p3cfgmgr.cc @@ -337,62 +337,56 @@ bool p3Config::saveConfig() BinEncryptedFileInterface *cfg_bio = new BinEncryptedFileInterface(newCfgFname.c_str(), bioflags); pqiSSLstore *stream = new pqiSSLstore(setupSerialiser(), RsPeerId(), cfg_bio, stream_flags); + bool ok = false; - written = written && stream->encryptedSendItems(toSave); + try + { + if(!stream->encryptedSendItems(toSave)) + throw std::runtime_error("(EE) Error while writing config file " + Filename() + ": file dropped!!"); - if(!written) - std::cerr << "(EE) Error while writing config file " << Filename() << ": file dropped!!" << std::endl; + /* store the hash */ + RsFileHash strHash(cfg_bio->gethash()); - /* store the hash */ - setHash(cfg_bio->gethash()); + /* sign data */ + std::string signature; - // bio is taken care of in stream's destructor, also forces file to close - delete stream; + if(!AuthSSL::getAuthSSL()->SignData(strHash.toByteArray(),strHash.SIZE_IN_BYTES, signature)) + throw std::runtime_error("(EE) Error while signing config file " + Filename() + ": file dropped!!"); - /* sign data */ - std::string signature; - RsFileHash strHash(Hash()); - AuthSSL::getAuthSSL()->SignData(strHash.toByteArray(),strHash.SIZE_IN_BYTES, signature); + /* write signature to configuration */ + BinMemInterface *signbio = new BinMemInterface(signature.c_str(), signature.length(), BIN_FLAGS_READABLE); - /* write signature to configuration */ - BinMemInterface *signbio = new BinMemInterface(signature.c_str(), - signature.length(), BIN_FLAGS_READABLE); + if(!signbio->writetofile(newSignFname.c_str())) + throw std::runtime_error("(EE) Error while writing to signature file " + newSignFname + ": file dropped!!"); - signbio->writetofile(newSignFname.c_str()); + delete signbio; - delete signbio; + // now rewrite current files to temp files + // rename back-up to current file + if(!RsDirUtil::renameFile(cfgFname, tmpCfgFname)) + throw std::runtime_error("p3Config::backedUpFileSave() Failed to rename backup meta files: " + cfgFname + " to " + tmpCfgFname); + if(!RsDirUtil::renameFile(signFname, tmpSignFname)) + throw std::runtime_error("p3Config::backedUpFileSave() Failed to rename backup meta files: " + signFname + " to " + tmpSignFname); - // now rewrite current files to temp files - // rename back-up to current file - if(!RsDirUtil::renameFile(cfgFname, tmpCfgFname) || !RsDirUtil::renameFile(signFname, tmpSignFname)){ -#ifdef CONFIG_DEBUG - std::cerr << "p3Config::backedUpFileSave() Failed to rename backup meta files: " << std::endl - << cfgFname << " to " << tmpCfgFname << std::endl - << signFname << " to " << tmpSignFname << std::endl; -#endif - written = false; - } + // now rewrite current files to temp files; rename back-up to current file + if(!RsDirUtil::renameFile(newCfgFname, cfgFname)) + throw std::runtime_error("p3Config::backedUpFileSave() Failed to rename backup meta files: " + newCfgFname + " to " + cfgFname); + if(!RsDirUtil::renameFile(newSignFname, signFname)) + throw std::runtime_error("p3Config::backedUpFileSave() Failed to rename backup meta files: " + newSignFname + " to " + signFname); + setHash(strHash); + ok = true; + } + catch (std::runtime_error& e) + { + RsErr() << e.what(); + ok = false; + } + delete stream; // bio is taken care of in stream's destructor, also forces file to close - // now rewrite current files to temp files - // rename back-up to current file - if(!RsDirUtil::renameFile(newCfgFname, cfgFname) || !RsDirUtil::renameFile(newSignFname, signFname)){ - #ifdef CONFIG_DEBUG - std::cerr << "p3Config::() Failed to rename meta files: " << std::endl - << newCfgFname << " to " << cfgFname << std::endl - << newSignFname << " to " << signFname << std::endl; - #endif - - written = false; - } - - - - saveDone(); // callback to inherited class to unlock any Mutexes protecting saveList() data - - return written; - + saveDone(); // callback to inherited class to unlock any Mutexes protecting saveList() data + return ok; } diff --git a/src/pqi/pqistore.cc b/src/pqi/pqistore.cc index 090a6a794..36e0ec524 100644 --- a/src/pqi/pqistore.cc +++ b/src/pqi/pqistore.cc @@ -412,14 +412,27 @@ bool pqiSSLstore::encryptedSendItems(const std::list& rsItemList) delete *it; } - bool result = true; + if(sizeItems != offset) + { + RsErr() << "Serialization error in " << __PRETTY_FUNCTION__ << std::endl; + return false; + } - if(sizeItems == offset) - enc_bio->senddata(data, sizeItems); - else - result = false; + int written = enc_bio->senddata(data, sizeItems); - return result; + if(written < 0) + { + RsErr() << "Write error in " << __PRETTY_FUNCTION__ << ": check disk space and permissions." << std::endl; + return false; + } + + if(sizeItems != (uint32_t)written) + { + RsErr() << "Write error in " << __PRETTY_FUNCTION__ << ": only " << written << " bytes sent instead of " << sizeItems << ": check disk space and permissions." << std::endl; + return false; + } + else + return true; } bool pqiSSLstore::getEncryptedItems(std::list& rsItemList)