From 63d11e6989dd4be3539bf1dcf40db31981dbbdbe Mon Sep 17 00:00:00 2001 From: benrubson Date: Sat, 11 Mar 2017 19:38:17 +0100 Subject: [PATCH 01/22] Return errno in case of write failed so make ::pwrite return an int --- encfs/BlockFileIO.cpp | 24 ++++++++++++------------ encfs/BlockFileIO.h | 6 +++--- encfs/CipherFileIO.cpp | 13 +++++++------ encfs/CipherFileIO.h | 2 +- encfs/FileIO.h | 2 +- encfs/FileNode.cpp | 2 +- encfs/FileNode.h | 2 +- encfs/MACFileIO.cpp | 8 ++++---- encfs/MACFileIO.h | 2 +- encfs/RawFileIO.cpp | 13 ++++++++----- encfs/RawFileIO.h | 2 +- encfs/encfs.cpp | 5 +++-- 12 files changed, 43 insertions(+), 38 deletions(-) diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index 032217d..73333bc 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -94,15 +94,15 @@ ssize_t BlockFileIO::cacheReadOneBlock(const IORequest &req) const { } } -bool BlockFileIO::cacheWriteOneBlock(const IORequest &req) { +int BlockFileIO::cacheWriteOneBlock(const IORequest &req) { // cache results of write (before pass-thru, because it may be modified // in-place) memcpy(_cache.data, req.data, req.dataLen); _cache.offset = req.offset; _cache.dataLen = req.dataLen; - bool ok = writeOneBlock(req); - if (!ok) clearCache(_cache, _blockSize); - return ok; + int res = writeOneBlock(req); + if (res) clearCache(_cache, _blockSize); + return res; } /** @@ -171,11 +171,11 @@ ssize_t BlockFileIO::read(const IORequest &req) const { return result; } -bool BlockFileIO::write(const IORequest &req) { +int BlockFileIO::write(const IORequest &req) { CHECK(_blockSize != 0); off_t fileSize = getSize(); - if (fileSize < 0) return false; + if (fileSize < 0) return fileSize; // where write request begins off_t blockNum = req.offset / _blockSize; @@ -213,7 +213,7 @@ bool BlockFileIO::write(const IORequest &req) { blockReq.data = NULL; blockReq.dataLen = _blockSize; - bool ok = true; + int res = 0; size_t size = req.dataLen; unsigned char *inPtr = req.data; while (size) { @@ -251,8 +251,8 @@ bool BlockFileIO::write(const IORequest &req) { } // Finally, write the damn thing! - if (!cacheWriteOneBlock(blockReq)) { - ok = false; + res = cacheWriteOneBlock(blockReq); + if (res) { break; } @@ -265,7 +265,7 @@ bool BlockFileIO::write(const IORequest &req) { if (mb.data) MemoryPool::release(mb); - return ok; + return res; } int BlockFileIO::blockSize() const { return _blockSize; } @@ -378,9 +378,9 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { // write back out partial block req.dataLen = partialBlock; - bool wrRes = cacheWriteOneBlock(req); + int wrRes = cacheWriteOneBlock(req); - if ((rdSz < 0) || (!wrRes)) { + if ((rdSz < 0) || (wrRes)) { // rwarning - unlikely to ever occur.. RLOG(WARNING) << "truncate failure: read " << rdSz << " bytes, partial block of " << partialBlock; diff --git a/encfs/BlockFileIO.h b/encfs/BlockFileIO.h index 8eb4520..6260229 100644 --- a/encfs/BlockFileIO.h +++ b/encfs/BlockFileIO.h @@ -43,7 +43,7 @@ class BlockFileIO : public FileIO { // implemented in terms of blocks. virtual ssize_t read(const IORequest &req) const; - virtual bool write(const IORequest &req); + virtual int write(const IORequest &req); virtual int blockSize() const; @@ -54,10 +54,10 @@ class BlockFileIO : public FileIO { // same as read(), except that the request.offset field is guarenteed to be // block aligned, and the request size will not be larger then 1 block. virtual ssize_t readOneBlock(const IORequest &req) const = 0; - virtual bool writeOneBlock(const IORequest &req) = 0; + virtual int writeOneBlock(const IORequest &req) = 0; ssize_t cacheReadOneBlock(const IORequest &req) const; - bool cacheWriteOneBlock(const IORequest &req); + int cacheWriteOneBlock(const IORequest &req); int _blockSize; bool _allowHoles; diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index 1a16dac..ef099b7 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -346,12 +346,12 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { return readSize; } -bool CipherFileIO::writeOneBlock(const IORequest &req) { +int CipherFileIO::writeOneBlock(const IORequest &req) { if (haveHeader && fsConfig->reverseEncryption) { VLOG(1) << "writing to a reverse mount with per-file IVs is not implemented"; - return false; + return -EPERM; } int bs = blockSize(); @@ -366,19 +366,20 @@ bool CipherFileIO::writeOneBlock(const IORequest &req) { ok = blockWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); } + int res = 0; if (ok) { if (haveHeader) { IORequest tmpReq = req; tmpReq.offset += HEADER_SIZE; - ok = base->write(tmpReq); + res = base->write(tmpReq); } else - ok = base->write(req); + res = base->write(req); } else { VLOG(1) << "encodeBlock failed for block " << blockNum << ", size " << req.dataLen; - ok = false; + res = -EBADMSG; } - return ok; + return res; } bool CipherFileIO::blockWrite(unsigned char *buf, int size, diff --git a/encfs/CipherFileIO.h b/encfs/CipherFileIO.h index a1bfd84..f1e7b91 100644 --- a/encfs/CipherFileIO.h +++ b/encfs/CipherFileIO.h @@ -65,7 +65,7 @@ class CipherFileIO : public BlockFileIO { private: virtual ssize_t readOneBlock(const IORequest &req) const; - virtual bool writeOneBlock(const IORequest &req); + virtual int writeOneBlock(const IORequest &req); virtual void generateReverseHeader(unsigned char *data); void initHeader(); diff --git a/encfs/FileIO.h b/encfs/FileIO.h index 474ff20..73de8b7 100644 --- a/encfs/FileIO.h +++ b/encfs/FileIO.h @@ -68,7 +68,7 @@ class FileIO { virtual off_t getSize() const = 0; virtual ssize_t read(const IORequest &req) const = 0; - virtual bool write(const IORequest &req) = 0; + virtual int write(const IORequest &req) = 0; virtual int truncate(off_t size) = 0; diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp index dea5a1a..4b350ba 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -209,7 +209,7 @@ ssize_t FileNode::read(off_t offset, unsigned char *data, ssize_t size) const { return io->read(req); } -bool FileNode::write(off_t offset, unsigned char *data, ssize_t size) { +int FileNode::write(off_t offset, unsigned char *data, ssize_t size) { VLOG(1) << "FileNode::write offset " << offset << ", data size " << size; IORequest req; diff --git a/encfs/FileNode.h b/encfs/FileNode.h index 6662bf1..b9fb26b 100644 --- a/encfs/FileNode.h +++ b/encfs/FileNode.h @@ -68,7 +68,7 @@ class FileNode { off_t getSize() const; ssize_t read(off_t offset, unsigned char *data, ssize_t size) const; - bool write(off_t offset, unsigned char *data, ssize_t size); + int write(off_t offset, unsigned char *data, ssize_t size); // truncate the file to a particular size int truncate(off_t size); diff --git a/encfs/MACFileIO.cpp b/encfs/MACFileIO.cpp index cf2760c..b09ed40 100644 --- a/encfs/MACFileIO.cpp +++ b/encfs/MACFileIO.cpp @@ -208,7 +208,7 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const { return readSize; } -bool MACFileIO::writeOneBlock(const IORequest &req) { +int MACFileIO::writeOneBlock(const IORequest &req) { int headerSize = macBytes + randBytes; int bs = blockSize() + headerSize; @@ -225,7 +225,7 @@ bool MACFileIO::writeOneBlock(const IORequest &req) { memcpy(newReq.data + headerSize, req.data, req.dataLen); if (randBytes > 0) { if (!cipher->randomize(newReq.data + macBytes, randBytes, false)) - return false; + return -EBADMSG; } if (macBytes > 0) { @@ -240,11 +240,11 @@ bool MACFileIO::writeOneBlock(const IORequest &req) { } // now, we can let the next level have it.. - bool ok = base->write(newReq); + int res = base->write(newReq); MemoryPool::release(mb); - return ok; + return res; } int MACFileIO::truncate(off_t size) { diff --git a/encfs/MACFileIO.h b/encfs/MACFileIO.h index 6bf4a25..07f41b6 100644 --- a/encfs/MACFileIO.h +++ b/encfs/MACFileIO.h @@ -64,7 +64,7 @@ class MACFileIO : public BlockFileIO { private: virtual ssize_t readOneBlock(const IORequest &req) const; - virtual bool writeOneBlock(const IORequest &req); + virtual int writeOneBlock(const IORequest &req); std::shared_ptr base; std::shared_ptr cipher; diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index accb3fd..78d1373 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -206,7 +206,7 @@ ssize_t RawFileIO::read(const IORequest &req) const { return readSize; } -bool RawFileIO::write(const IORequest &req) { +int RawFileIO::write(const IORequest &req) { rAssert(fd >= 0); rAssert(true == canWrite); @@ -215,14 +215,17 @@ bool RawFileIO::write(const IORequest &req) { ssize_t bytes = req.dataLen; off_t offset = req.offset; + int eno = 0; while (bytes && retrys > 0) { + errno = 0; ssize_t writeSize = ::pwrite(fd, buf, bytes, offset); + eno = errno; if (writeSize < 0) { knownSize = false; RLOG(WARNING) << "write failed at offset " << offset << " for " << bytes - << " bytes: " << strerror(errno); - return false; + << " bytes: " << strerror(eno); + return -eno; } bytes -= writeSize; @@ -235,14 +238,14 @@ bool RawFileIO::write(const IORequest &req) { RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of " << req.dataLen << ", max retries reached"; knownSize = false; - return false; + return (eno) ? -eno : -EIO; } else { if (knownSize) { off_t last = req.offset + req.dataLen; if (last > fileSize) fileSize = last; } - return true; + return 0; } } diff --git a/encfs/RawFileIO.h b/encfs/RawFileIO.h index 1214a56..9539b01 100644 --- a/encfs/RawFileIO.h +++ b/encfs/RawFileIO.h @@ -46,7 +46,7 @@ class RawFileIO : public FileIO { virtual off_t getSize() const; virtual ssize_t read(const IORequest &req) const; - virtual bool write(const IORequest &req); + virtual int write(const IORequest &req); virtual int truncate(off_t size); diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 7a0a127..39de833 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -619,10 +619,11 @@ int encfs_fsync(const char *path, int dataSync, struct fuse_file_info *file) { } int _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) { - if (fnode->write(offset, ptr, size)) + int res = fnode->write(offset, ptr, size); + if (!res) return size; else - return -EIO; + return res; } int encfs_write(const char *path, const char *buf, size_t size, off_t offset, From 36f3505af03344b1ce38633d3e85f9d4675206d5 Mon Sep 17 00:00:00 2001 From: benrubson Date: Thu, 16 Mar 2017 15:48:19 +0100 Subject: [PATCH 02/22] Make readOneBlock return -EBADMSG when decode fails instead of throwing an exception, as with writeOneBlock --- encfs/BlockFileIO.cpp | 4 ++++ encfs/CipherFileIO.cpp | 2 +- encfs/MACFileIO.cpp | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index 73333bc..d440dac 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -147,6 +147,10 @@ ssize_t BlockFileIO::read(const IORequest &req) const { } ssize_t readSize = cacheReadOneBlock(blockReq); + if (readSize < 0) { + result = readSize; + break; + } if (readSize <= partialOffset) break; // didn't get enough bytes int cpySize = min((size_t)(readSize - partialOffset), size); diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index ef099b7..4d1b510 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -337,7 +337,7 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { if (!ok) { VLOG(1) << "decodeBlock failed for block " << blockNum << ", size " << readSize; - readSize = -1; + readSize = -EBADMSG; } } else { VLOG(1) << "readSize zero for offset " << req.offset; diff --git a/encfs/MACFileIO.cpp b/encfs/MACFileIO.cpp index b09ed40..3b503cb 100644 --- a/encfs/MACFileIO.cpp +++ b/encfs/MACFileIO.cpp @@ -190,7 +190,7 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const { RLOG(WARNING) << "MAC comparison failure in block " << blockNum; if (!warnOnly) { MemoryPool::release(mb); - throw Error(_("MAC comparison failure, refusing to read")); + return -EBADMSG; } } } From 6f4ff008bcec0e55bd4bd0315e4ecd82eabe5293 Mon Sep 17 00:00:00 2001 From: benrubson Date: Fri, 17 Mar 2017 09:07:14 +0100 Subject: [PATCH 03/22] Make initHeader return -EBADMSG when encode fails instead of throwing an exception, as with read/write --- encfs/CipherFileIO.cpp | 29 +++++++++++++++++++++-------- encfs/CipherFileIO.h | 2 +- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index 4d1b510..0556f9d 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -109,7 +109,8 @@ bool CipherFileIO::setIV(uint64_t iv) { return false; } } - initHeader(); + if (initHeader() < 0) + return false; } uint64_t oldIV = externalIV; @@ -170,7 +171,7 @@ off_t CipherFileIO::getSize() const { return size; } -void CipherFileIO::initHeader() { +int CipherFileIO::initHeader() { // check if the file has a header, and read it if it does.. Otherwise, // create one. off_t rawSize = base->getSize(); @@ -196,8 +197,10 @@ void CipherFileIO::initHeader() { unsigned char buf[8] = {0}; do { - if (!cipher->randomize(buf, 8, false)) - throw Error("Unable to generate a random file IV"); + if (!cipher->randomize(buf, 8, false)) { + RLOG(ERROR) << "Unable to generate a random file IV"; + return -EBADMSG; + } fileIV = 0; for (int i = 0; i < 8; ++i) fileIV = (fileIV << 8) | (uint64_t)buf[i]; @@ -220,6 +223,7 @@ void CipherFileIO::initHeader() { } } VLOG(1) << "initHeader finished, fileIV = " << fileIV; + return 0; } bool CipherFileIO::writeHeader() { @@ -324,8 +328,11 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { bool ok; if (readSize > 0) { - if (haveHeader && fileIV == 0) - const_cast(this)->initHeader(); + if (haveHeader && fileIV == 0) { + int res = const_cast(this)->initHeader(); + if (res < 0) + return res; + } if (readSize != bs) { VLOG(1) << "streamRead(data, " << readSize << ", IV)"; @@ -357,7 +364,11 @@ int CipherFileIO::writeOneBlock(const IORequest &req) { int bs = blockSize(); off_t blockNum = req.offset / bs; - if (haveHeader && fileIV == 0) initHeader(); + if (haveHeader && fileIV == 0) { + int res = initHeader(); + if (res < 0) + return res; + } bool ok; if (req.dataLen != bs) { @@ -437,7 +448,9 @@ int CipherFileIO::truncate(off_t size) { if (base->open(newFlags) < 0) VLOG(1) << "writeHeader failed to re-open for write"; } - initHeader(); + int res = initHeader(); + if (res < 0) + return res; } // can't let BlockFileIO call base->truncate(), since it would be using diff --git a/encfs/CipherFileIO.h b/encfs/CipherFileIO.h index f1e7b91..4d94a70 100644 --- a/encfs/CipherFileIO.h +++ b/encfs/CipherFileIO.h @@ -68,7 +68,7 @@ class CipherFileIO : public BlockFileIO { virtual int writeOneBlock(const IORequest &req); virtual void generateReverseHeader(unsigned char *data); - void initHeader(); + int initHeader(); bool writeHeader(); bool blockRead(unsigned char *buf, int size, uint64_t iv64) const; bool streamRead(unsigned char *buf, int size, uint64_t iv64) const; From b88da06a0855cae5fec9dc04f041ca793eea2f80 Mon Sep 17 00:00:00 2001 From: benrubson Date: Fri, 17 Mar 2017 23:40:50 +0100 Subject: [PATCH 04/22] Make (stream|block)(Encode|Decode) return false --- encfs/BlockNameIO.cpp | 14 ++++++++++---- encfs/CipherFileIO.cpp | 19 +++++++++++++------ encfs/CipherFileIO.h | 2 +- encfs/SSL_Cipher.cpp | 16 ++++++++++++---- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/encfs/BlockNameIO.cpp b/encfs/BlockNameIO.cpp index 0fdff9a..573c96b 100644 --- a/encfs/BlockNameIO.cpp +++ b/encfs/BlockNameIO.cpp @@ -155,8 +155,11 @@ int BlockNameIO::encodeName(const char *plaintextName, int length, uint64_t *iv, encodedName[0] = (mac >> 8) & 0xff; encodedName[1] = (mac)&0xff; - _cipher->blockEncode((unsigned char *)encodedName + 2, length + padding, - (uint64_t)mac ^ tmpIV, _key); + bool ok; + ok = _cipher->blockEncode((unsigned char *)encodedName + 2, length + padding, + (uint64_t)mac ^ tmpIV, _key); + if (!ok) + throw Error("block encode failed in filename encode"); // convert to base 64 ascii int encodedStreamLen = length + 2 + padding; @@ -209,8 +212,11 @@ int BlockNameIO::decodeName(const char *encodedName, int length, uint64_t *iv, uint64_t tmpIV = 0; if (iv && _interface >= 3) tmpIV = *iv; - _cipher->blockDecode((unsigned char *)tmpBuf + 2, decodedStreamLen, - (uint64_t)mac ^ tmpIV, _key); + bool ok; + ok = _cipher->blockDecode((unsigned char *)tmpBuf + 2, decodedStreamLen, + (uint64_t)mac ^ tmpIV, _key); + if (!ok) + throw Error("block decode failed in filename decode"); // find out true string length int padding = (unsigned char)tmpBuf[2 + decodedStreamLen - 1]; diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index 0556f9d..11da222 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -186,7 +186,8 @@ int CipherFileIO::initHeader() { req.dataLen = 8; base->read(req); - cipher->streamDecode(buf, sizeof(buf), externalIV, key); + if(!cipher->streamDecode(buf, sizeof(buf), externalIV, key)) + return -EBADMSG; fileIV = 0; for (int i = 0; i < 8; ++i) fileIV = (fileIV << 8) | (uint64_t)buf[i]; @@ -210,7 +211,8 @@ int CipherFileIO::initHeader() { } while (fileIV == 0); // don't accept 0 as an option.. if (base->isWritable()) { - cipher->streamEncode(buf, sizeof(buf), externalIV, key); + if(!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) + return -EBADMSG; IORequest req; req.offset = 0; @@ -247,7 +249,8 @@ bool CipherFileIO::writeHeader() { fileIV >>= 8; } - cipher->streamEncode(buf, sizeof(buf), externalIV, key); + if(!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) + return false; IORequest req; req.offset = 0; @@ -271,7 +274,7 @@ bool CipherFileIO::writeHeader() { * the IV. This guarantees unpredictability and prevents watermarking * attacks. */ -void CipherFileIO::generateReverseHeader(unsigned char *headerBuf) { +int CipherFileIO::generateReverseHeader(unsigned char *headerBuf) { struct stat stbuf; int res = getAttr(&stbuf); @@ -305,7 +308,9 @@ void CipherFileIO::generateReverseHeader(unsigned char *headerBuf) { VLOG(1) << "fileIV=" << fileIV; // Encrypt externally-visible header - cipher->streamEncode(headerBuf, HEADER_SIZE, externalIV, key); + if(!cipher->streamEncode(headerBuf, HEADER_SIZE, externalIV, key)) + return -EBADMSG; + return 0; } /** @@ -481,7 +486,9 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const { // generate the file IV header // this is needed in any case - without IV the file cannot be decoded unsigned char headerBuf[HEADER_SIZE]; - const_cast(this)->generateReverseHeader(headerBuf); + int res = const_cast(this)->generateReverseHeader(headerBuf); + if (res < 0) + return res; // Copy the request so we can modify it without affecting the caller IORequest req = origReq; diff --git a/encfs/CipherFileIO.h b/encfs/CipherFileIO.h index 4d94a70..d27444c 100644 --- a/encfs/CipherFileIO.h +++ b/encfs/CipherFileIO.h @@ -66,7 +66,7 @@ class CipherFileIO : public BlockFileIO { private: virtual ssize_t readOneBlock(const IORequest &req) const; virtual int writeOneBlock(const IORequest &req); - virtual void generateReverseHeader(unsigned char *data); + virtual int generateReverseHeader(unsigned char *data); int initHeader(); bool writeHeader(); diff --git a/encfs/SSL_Cipher.cpp b/encfs/SSL_Cipher.cpp index f2f8294..d5af102 100644 --- a/encfs/SSL_Cipher.cpp +++ b/encfs/SSL_Cipher.cpp @@ -748,6 +748,7 @@ bool SSL_Cipher::streamEncode(unsigned char *buf, int size, uint64_t iv64, if (dstLen != size) { RLOG(ERROR) << "encoding " << size << " bytes, got back " << dstLen << " (" << tmpLen << " in final_ex)"; + return false; } return true; @@ -784,6 +785,7 @@ bool SSL_Cipher::streamDecode(unsigned char *buf, int size, uint64_t iv64, if (dstLen != size) { RLOG(ERROR) << "decoding " << size << " bytes, got back " << dstLen << " (" << tmpLen << " in final_ex)"; + return false; } return true; @@ -798,8 +800,10 @@ bool SSL_Cipher::blockEncode(unsigned char *buf, int size, uint64_t iv64, // data must be integer number of blocks const int blockMod = size % EVP_CIPHER_CTX_block_size(key->block_enc); - if (blockMod != 0) - throw Error("Invalid data size, not multiple of block size"); + if (blockMod != 0) { + RLOG(ERROR) << "Invalid data size, not multiple of block size"; + return false; + } Lock lock(key->mutex); @@ -816,6 +820,7 @@ bool SSL_Cipher::blockEncode(unsigned char *buf, int size, uint64_t iv64, if (dstLen != size) { RLOG(ERROR) << "encoding " << size << " bytes, got back " << dstLen << " (" << tmpLen << " in final_ex)"; + return false; } return true; @@ -830,8 +835,10 @@ bool SSL_Cipher::blockDecode(unsigned char *buf, int size, uint64_t iv64, // data must be integer number of blocks const int blockMod = size % EVP_CIPHER_CTX_block_size(key->block_dec); - if (blockMod != 0) - throw Error("Invalid data size, not multiple of block size"); + if (blockMod != 0) { + RLOG(ERROR) << "Invalid data size, not multiple of block size"; + return false; + } Lock lock(key->mutex); @@ -848,6 +855,7 @@ bool SSL_Cipher::blockDecode(unsigned char *buf, int size, uint64_t iv64, if (dstLen != size) { RLOG(ERROR) << "decoding " << size << " bytes, got back " << dstLen << " (" << tmpLen << " in final_ex)"; + return false; } return true; From 13ae4de83063e57524209400a7b2598279f9fc2d Mon Sep 17 00:00:00 2001 From: benrubson Date: Mon, 20 Mar 2017 14:23:19 +0100 Subject: [PATCH 05/22] Verify open/read/write/truncate returned values --- encfs/BlockFileIO.cpp | 55 +++++++++++++++++++++++----------------- encfs/BlockFileIO.h | 2 +- encfs/CipherFileIO.cpp | 57 +++++++++++++++++++++--------------------- encfs/MACFileIO.cpp | 2 +- 4 files changed, 62 insertions(+), 54 deletions(-) diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index d440dac..e3c9f7b 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -195,7 +195,9 @@ int BlockFileIO::write(const IORequest &req) { if (req.offset > fileSize) { // extend file first to fill hole with 0's.. const bool forceWrite = false; - padFile(fileSize, req.offset, forceWrite); + int res = padFile(fileSize, req.offset, forceWrite); + if (res) + return res; } // check against edge cases where we can just let the base class handle the @@ -244,7 +246,10 @@ int BlockFileIO::write(const IORequest &req) { } else { // have to merge with existing block data.. blockReq.dataLen = _blockSize; - blockReq.dataLen = cacheReadOneBlock(blockReq); + if ((blockReq.dataLen = cacheReadOneBlock(blockReq)) < 0) { + res = blockReq.dataLen; + break; + } // extend data if necessary.. if (partialOffset + toCopy > blockReq.dataLen) @@ -274,10 +279,11 @@ int BlockFileIO::write(const IORequest &req) { int BlockFileIO::blockSize() const { return _blockSize; } -void BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { +int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { off_t oldLastBlock = oldSize / _blockSize; off_t newLastBlock = newSize / _blockSize; int newBlockSize = newSize % _blockSize; + int res = 0; IORequest req; MemBlock mb; @@ -296,9 +302,10 @@ void BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { if (outSize) { memset(mb.data, 0, outSize); - cacheReadOneBlock(req); - req.dataLen = outSize; - cacheWriteOneBlock(req); + if ((res = cacheReadOneBlock(req)) >= 0) { + req.dataLen = outSize; + res = cacheWriteOneBlock(req); + } } } else VLOG(1) << "optimization: not padding last block"; @@ -317,33 +324,36 @@ void BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { if (req.dataLen != 0) { VLOG(1) << "padding block " << oldLastBlock; memset(mb.data, 0, _blockSize); - cacheReadOneBlock(req); - req.dataLen = _blockSize; // expand to full block size - cacheWriteOneBlock(req); + if ((res = cacheReadOneBlock(req)) >= 0) { + req.dataLen = _blockSize; // expand to full block size + res = cacheWriteOneBlock(req); + } ++oldLastBlock; } // 2, pad zero blocks unless holes are allowed if (!_allowHoles) { - for (; oldLastBlock != newLastBlock; ++oldLastBlock) { + for (; !res && (oldLastBlock != newLastBlock); ++oldLastBlock) { VLOG(1) << "padding block " << oldLastBlock; req.offset = oldLastBlock * _blockSize; req.dataLen = _blockSize; memset(mb.data, 0, req.dataLen); - cacheWriteOneBlock(req); + res = cacheWriteOneBlock(req); } } // 3. only necessary if write is forced and block is non 0 length - if (forceWrite && newBlockSize) { + if (!res && forceWrite && newBlockSize) { req.offset = newLastBlock * _blockSize; req.dataLen = newBlockSize; memset(mb.data, 0, req.dataLen); - cacheWriteOneBlock(req); + res = cacheWriteOneBlock(req); } } if (mb.data) MemoryPool::release(mb); + + return res; } int BlockFileIO::truncateBase(off_t size, FileIO *base) { @@ -357,10 +367,11 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { // states that it will pad with 0's. // do the truncate so that the underlying filesystem can allocate // the space, and then we'll fill it in padFile.. - if (base) base->truncate(size); + if (base) res = base->truncate(size); const bool forceWrite = true; - padFile(oldSize, size, forceWrite); + if (!res) + res = padFile(oldSize, size, forceWrite); } else if (size == oldSize) { // the easiest case, but least likely.... } else if (partialBlock) { @@ -376,19 +387,17 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { req.data = mb.data; ssize_t rdSz = cacheReadOneBlock(req); + if (rdSz < 0) + res = rdSz; // do the truncate - if (base) res = base->truncate(size); + else if (base) + res = base->truncate(size); // write back out partial block req.dataLen = partialBlock; - int wrRes = cacheWriteOneBlock(req); - - if ((rdSz < 0) || (wrRes)) { - // rwarning - unlikely to ever occur.. - RLOG(WARNING) << "truncate failure: read " << rdSz - << " bytes, partial block of " << partialBlock; - } + if (!res) + res = cacheWriteOneBlock(req); MemoryPool::release(mb); } else { diff --git a/encfs/BlockFileIO.h b/encfs/BlockFileIO.h index 6260229..ca02883 100644 --- a/encfs/BlockFileIO.h +++ b/encfs/BlockFileIO.h @@ -49,7 +49,7 @@ class BlockFileIO : public FileIO { protected: int truncateBase(off_t size, FileIO *base); - void padFile(off_t oldSize, off_t newSize, bool forceWrite); + int padFile(off_t oldSize, off_t newSize, bool forceWrite); // same as read(), except that the request.offset field is guarenteed to be // block aligned, and the request size will not be larger then 1 block. diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index 11da222..fae102b 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -95,20 +95,20 @@ bool CipherFileIO::setIV(uint64_t iv) { } else if (haveHeader) { // we have an old IV, and now a new IV, so we need to update the fileIV // on disk. - if (fileIV == 0) { - // ensure the file is open for read/write.. - int newFlags = lastFlags | O_RDWR; - int res = base->open(newFlags); - if (res < 0) { - if (res == -EISDIR) { - // duh -- there are no file headers for directories! - externalIV = iv; - return base->setIV(iv); - } else { - VLOG(1) << "writeHeader failed to re-open for write"; - return false; - } + // ensure the file is open for read/write.. + int newFlags = lastFlags | O_RDWR; + int res = base->open(newFlags); + if (res < 0) { + if (res == -EISDIR) { + // duh -- there are no file headers for directories! + externalIV = iv; + return base->setIV(iv); + } else { + VLOG(1) << "setIV failed to re-open for write"; + return false; } + } + if (fileIV == 0) { if (initHeader() < 0) return false; } @@ -184,7 +184,9 @@ int CipherFileIO::initHeader() { req.offset = 0; req.data = buf; req.dataLen = 8; - base->read(req); + ssize_t readSize = base->read(req); + if(readSize < 0) + return readSize; if(!cipher->streamDecode(buf, sizeof(buf), externalIV, key)) return -EBADMSG; @@ -219,7 +221,9 @@ int CipherFileIO::initHeader() { req.data = buf; req.dataLen = 8; - base->write(req); + int res = base->write(req); + if (res) + return res; } else { VLOG(1) << "base not writable, IV not written.."; } @@ -229,15 +233,6 @@ int CipherFileIO::initHeader() { } bool CipherFileIO::writeHeader() { - if (!base->isWritable()) { - // open for write.. - int newFlags = lastFlags | O_RDWR; - if (base->open(newFlags) < 0) { - VLOG(1) << "writeHeader failed to re-open for write"; - return false; - } - } - if (fileIV == 0) { RLOG(ERROR) << "Internal error: fileIV == 0 in writeHeader!!!"; } @@ -257,7 +252,8 @@ bool CipherFileIO::writeHeader() { req.data = buf; req.dataLen = 8; - base->write(req); + if (base->write(req)) + return false; return true; } @@ -351,7 +347,7 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { << readSize; readSize = -EBADMSG; } - } else { + } else if (readSize == 0) { VLOG(1) << "readSize zero for offset " << req.offset; } @@ -450,8 +446,11 @@ int CipherFileIO::truncate(off_t size) { if (!base->isWritable()) { // open for write.. int newFlags = lastFlags | O_RDWR; - if (base->open(newFlags) < 0) - VLOG(1) << "writeHeader failed to re-open for write"; + int res = base->open(newFlags); + if (res < 0) { + VLOG(1) << "truncate failed to re-open for write"; + return res; + } } int res = initHeader(); if (res < 0) @@ -462,7 +461,7 @@ int CipherFileIO::truncate(off_t size) { // the wrong size.. res = BlockFileIO::truncateBase(size, 0); - if (res == 0) base->truncate(size + HEADER_SIZE); + if (res == 0) res = base->truncate(size + HEADER_SIZE); } return res; } diff --git a/encfs/MACFileIO.cpp b/encfs/MACFileIO.cpp index 3b503cb..15bb032 100644 --- a/encfs/MACFileIO.cpp +++ b/encfs/MACFileIO.cpp @@ -253,7 +253,7 @@ int MACFileIO::truncate(off_t size) { int res = BlockFileIO::truncateBase(size, 0); - if (res == 0) base->truncate(locWithHeader(size, bs, headerSize)); + if (res == 0) res = base->truncate(locWithHeader(size, bs, headerSize)); return res; } From f024ae86f82bfb3c78033dcfc4fc1f9eae682fc4 Mon Sep 17 00:00:00 2001 From: benrubson Date: Mon, 20 Mar 2017 15:11:17 +0100 Subject: [PATCH 06/22] Make sure errno is correctly used --- encfs/DirNode.cpp | 11 ++++++----- encfs/FileNode.cpp | 12 +++++++----- encfs/RawFileIO.cpp | 32 ++++++++++++++++++-------------- encfs/encfs.h | 6 ++++-- encfs/main.cpp | 3 ++- 5 files changed, 37 insertions(+), 27 deletions(-) diff --git a/encfs/DirNode.cpp b/encfs/DirNode.cpp index 3612d6d..2e65f23 100644 --- a/encfs/DirNode.cpp +++ b/encfs/DirNode.cpp @@ -185,8 +185,9 @@ bool RenameOp::apply() { // rename on disk.. if (::rename(last->oldCName.c_str(), last->newCName.c_str()) == -1) { + int eno = errno; RLOG(WARNING) << "Error renaming " << last->oldCName << ": " - << strerror(errno); + << strerror(eno); dn->renameNode(last->newPName.c_str(), last->oldPName.c_str(), false); return false; } @@ -355,7 +356,8 @@ DirTraverse DirNode::openDir(const char *plaintextPath) { DIR *dir = ::opendir(cyName.c_str()); if (dir == NULL) { - VLOG(1) << "opendir error " << strerror(errno); + int eno = errno; + VLOG(1) << "opendir error " << strerror(eno); return DirTraverse(shared_ptr(), 0, std::shared_ptr()); } else { std::shared_ptr dp(dir, DirDeleter()); @@ -569,8 +571,7 @@ int DirNode::rename(const char *fromPlaintext, const char *toPlaintext) { } if (res != 0) { - VLOG(1) << "rename failed: " << strerror(errno); - res = -errno; + VLOG(1) << "rename failed: " << strerror(-res); } return res; @@ -692,7 +693,7 @@ int DirNode::unlink(const char *plaintextName) { res = ::unlink(fullName.c_str()); if (res == -1) { res = -errno; - VLOG(1) << "unlink error: " << strerror(errno); + VLOG(1) << "unlink error: " << strerror(-res); } } diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp index 4b350ba..9d52763 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -140,14 +140,16 @@ int FileNode::mknod(mode_t mode, dev_t rdev, uid_t uid, gid_t gid) { if (uid != 0) { olduid = setfsuid(uid); if (olduid == -1) { - RLOG(DEBUG) << "setfsuid error: " << strerror(errno); + int eno = errno; + RLOG(DEBUG) << "setfsuid error: " << strerror(eno); return -EPERM; } } if (gid != 0) { oldgid = setfsgid(gid); if (oldgid == -1) { - RLOG(DEBUG) << "setfsgid error: " << strerror(errno); + int eno = errno; + RLOG(DEBUG) << "setfsgid error: " << strerror(eno); return -EPERM; } } @@ -165,15 +167,15 @@ int FileNode::mknod(mode_t mode, dev_t rdev, uid_t uid, gid_t gid) { else res = ::mknod(_cname.c_str(), mode, rdev); - if (olduid >= 0) setfsuid(olduid); - if (oldgid >= 0) setfsgid(oldgid); - if (res == -1) { int eno = errno; VLOG(1) << "mknod error: " << strerror(eno); res = -eno; } + if (olduid >= 0) setfsuid(olduid); + if (oldgid >= 0) setfsgid(oldgid); + return res; } diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index 78d1373..f28aa3f 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -92,13 +92,11 @@ static int open_readonly_workaround(const char *path, int flags) { memset(&stbuf, 0, sizeof(struct stat)); if (lstat(path, &stbuf) != -1) { // make sure user has read/write permission.. - chmod(path, stbuf.st_mode | 0600); - fd = ::open(path, flags); - chmod(path, stbuf.st_mode); - } else { - RLOG(INFO) << "can't stat file " << path; + if (chmod(path, stbuf.st_mode | 0600) != -1) { + fd = ::open(path, flags); + chmod(path, stbuf.st_mode); + } } - return fd; } @@ -131,10 +129,11 @@ int RawFileIO::open(int flags) { #endif int newFd = ::open(name.c_str(), finalFlags); + int eno = errno; VLOG(1) << "open file with flags " << finalFlags << ", result = " << newFd; - if ((newFd == -1) && (errno == EACCES)) { + if ((newFd == -1) && (eno == EACCES)) { VLOG(1) << "using readonly workaround for open"; newFd = open_readonly_workaround(name.c_str(), finalFlags); } @@ -152,7 +151,7 @@ int RawFileIO::open(int flags) { result = fd = newFd; } else { result = -errno; - RLOG(DEBUG) << "::open error: " << strerror(errno); + RLOG(DEBUG) << "::open error: " << strerror(-result); } } @@ -185,8 +184,9 @@ off_t RawFileIO::getSize() const { const_cast(this)->knownSize = true; return fileSize; } else { - RLOG(ERROR) << "getSize on " << name << " failed: " << strerror(errno); - return -1; + int eno = errno; + RLOG(ERROR) << "getSize on " << name << " failed: " << strerror(eno); + return -eno; } } else { return fileSize; @@ -199,8 +199,9 @@ ssize_t RawFileIO::read(const IORequest &req) const { ssize_t readSize = pread(fd, req.data, req.dataLen, req.offset); if (readSize < 0) { + readSize = -errno; RLOG(WARNING) << "read failed at offset " << req.offset << " for " - << req.dataLen << " bytes: " << strerror(errno); + << req.dataLen << " bytes: " << strerror(-readSize); } return readSize; @@ -254,9 +255,6 @@ int RawFileIO::truncate(off_t size) { if (fd >= 0 && canWrite) { res = ::ftruncate(fd, size); -#if !defined(__FreeBSD__) && !defined(__APPLE__) - ::fdatasync(fd); -#endif } else res = ::truncate(name.c_str(), size); @@ -272,6 +270,12 @@ int RawFileIO::truncate(off_t size) { knownSize = true; } +#if !defined(__FreeBSD__) && !defined(__APPLE__) + if (fd >= 0 && canWrite) { + ::fdatasync(fd); + } +#endif + return res; } diff --git a/encfs/encfs.h b/encfs/encfs.h index 2988af2..05c4fea 100644 --- a/encfs/encfs.h +++ b/encfs/encfs.h @@ -41,7 +41,8 @@ static __inline int setfsuid(uid_t uid) { uid_t olduid = geteuid(); if (seteuid(uid) != 0) { - VLOG(1) << "seteuid error: " << errno; + int eno = errno; + VLOG(1) << "seteuid error: " << strerror(eno); } return olduid; @@ -51,7 +52,8 @@ static __inline int setfsgid(gid_t gid) { gid_t oldgid = getegid(); if (setegid(gid) != 0) { - VLOG(1) << "setfsgid error: " << errno; + int eno = errno; + VLOG(1) << "setfsgid error: " << strerror(eno); } return oldgid; diff --git a/encfs/main.cpp b/encfs/main.cpp index e56b83a..ebb96bd 100644 --- a/encfs/main.cpp +++ b/encfs/main.cpp @@ -504,9 +504,10 @@ void *encfs_init(fuse_conn_info *conn) { int res = pthread_create(&ctx->monitorThread, 0, idleMonitor, (void *)ctx); if (res != 0) { + int eno = errno; RLOG(ERROR) << "error starting idle monitor thread, " "res = " - << res << ", errno = " << errno; + << res << ", " << strerror(eno); } } From 11a83b85babc7403e020f4eb85254f256c57e36e Mon Sep 17 00:00:00 2001 From: benrubson Date: Mon, 20 Mar 2017 15:57:37 +0100 Subject: [PATCH 07/22] Remove not needed try/catch block --- encfs/encfs.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 39de833..ad0acec 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -356,11 +356,7 @@ int _do_readlink(EncFS_Context *ctx, const string &cyName, char *buf, buf[res] = '\0'; // ensure null termination string decodedName; - try { - decodedName = FSRoot->plainPath(buf); - } catch (...) { - VLOG(1) << "caught error decoding path"; - } + decodedName = FSRoot->plainPath(buf); if (!decodedName.empty()) { strncpy(buf, decodedName.c_str(), size - 1); From af6c593b38976b78e20dc612e41fea704c53105a Mon Sep 17 00:00:00 2001 From: benrubson Date: Thu, 1 Jun 2017 15:41:37 +0200 Subject: [PATCH 08/22] Properly check for fdatasync() --- CMakeLists.txt | 6 ++++++ encfs/FileNode.cpp | 4 +--- encfs/RawFileIO.cpp | 6 ++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 62eab18..bfcf7d2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -93,6 +93,12 @@ check_cxx_source_compiles ("#include " XATTR_ADD_OPT) set (CMAKE_REQUIRED_QUIET ${CMAKE_REQUIRED_QUIET_SAVE}) +# Check if we have fdatasync. +include (CheckCXXSourceCompiles) +check_cxx_source_compiles ("#include + int main() { fdatasync(0); return 1; } + " FDATASYNC) + # Check if we have some standard functions. include (CheckFuncs) check_function_exists_glibc (lchmod HAVE_LCHMOD) diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp index 9d52763..02d5c11 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -236,15 +236,13 @@ int FileNode::sync(bool datasync) { int fh = io->open(O_RDONLY); if (fh >= 0) { int res = -EIO; -#ifdef linux +#ifdef FDATASYNC if (datasync) res = fdatasync(fh); else res = fsync(fh); #else (void)datasync; - // no fdatasync support - // TODO: use autoconfig to check for it.. res = fsync(fh); #endif diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index f28aa3f..b2cbb26 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -270,11 +270,13 @@ int RawFileIO::truncate(off_t size) { knownSize = true; } -#if !defined(__FreeBSD__) && !defined(__APPLE__) if (fd >= 0 && canWrite) { +#ifdef FDATASYNC ::fdatasync(fd); - } +#else + ::fsync(fd); #endif + } return res; } From a64c273d9e2d870a8d38924dd142e0f9d632005b Mon Sep 17 00:00:00 2001 From: benrubson Date: Fri, 4 Aug 2017 09:22:15 +0200 Subject: [PATCH 09/22] Document BlockFileIO::read return code --- encfs/BlockFileIO.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index e3c9f7b..efbb3fe 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -111,6 +111,7 @@ int BlockFileIO::cacheWriteOneBlock(const IORequest &req) { * data from the front of the first block if the request is not aligned. * Always requests aligned data of the size of one block or less from the * lower layer. + * Returns the number of bytes read, or -errno in case of failure. */ ssize_t BlockFileIO::read(const IORequest &req) const { CHECK(_blockSize != 0); From 062e3a5a98e5c8bf230a2eae6c64591021177c62 Mon Sep 17 00:00:00 2001 From: benrubson Date: Fri, 4 Aug 2017 09:52:33 +0200 Subject: [PATCH 10/22] Document BlockFileIO::write return code --- encfs/BlockFileIO.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index efbb3fe..adadaa3 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -176,6 +176,10 @@ ssize_t BlockFileIO::read(const IORequest &req) const { return result; } +/** + * Returns the very first result returned by RawFileIO::write so : + * 0 in case of success, or -errno in case of failure. + */ int BlockFileIO::write(const IORequest &req) { CHECK(_blockSize != 0); @@ -280,6 +284,10 @@ int BlockFileIO::write(const IORequest &req) { int BlockFileIO::blockSize() const { return _blockSize; } +/** + * Returns the very first result returned by RawFileIO::write so : + * 0 in case of success, or -errno in case of failure. + */ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { off_t oldLastBlock = oldSize / _blockSize; off_t newLastBlock = newSize / _blockSize; From 483fb7c6ccb84dc20ce74b91521f203ba2ad8d04 Mon Sep 17 00:00:00 2001 From: benrubson Date: Fri, 4 Aug 2017 10:58:44 +0200 Subject: [PATCH 11/22] Make some return codes easier to read / understand --- encfs/BlockFileIO.cpp | 12 ++++++------ encfs/CipherFileIO.cpp | 6 +++--- encfs/MACFileIO.cpp | 2 +- encfs/RawFileIO.cpp | 2 +- encfs/encfs.cpp | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index adadaa3..11c46de 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -101,7 +101,7 @@ int BlockFileIO::cacheWriteOneBlock(const IORequest &req) { _cache.offset = req.offset; _cache.dataLen = req.dataLen; int res = writeOneBlock(req); - if (res) clearCache(_cache, _blockSize); + if (res < 0) clearCache(_cache, _blockSize); return res; } @@ -201,7 +201,7 @@ int BlockFileIO::write(const IORequest &req) { // extend file first to fill hole with 0's.. const bool forceWrite = false; int res = padFile(fileSize, req.offset, forceWrite); - if (res) + if (res < 0) return res; } @@ -266,7 +266,7 @@ int BlockFileIO::write(const IORequest &req) { // Finally, write the damn thing! res = cacheWriteOneBlock(blockReq); - if (res) { + if (res < 0) { break; } @@ -342,7 +342,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { // 2, pad zero blocks unless holes are allowed if (!_allowHoles) { - for (; !res && (oldLastBlock != newLastBlock); ++oldLastBlock) { + for (; !(res < 0) && (oldLastBlock != newLastBlock); ++oldLastBlock) { VLOG(1) << "padding block " << oldLastBlock; req.offset = oldLastBlock * _blockSize; req.dataLen = _blockSize; @@ -352,7 +352,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { } // 3. only necessary if write is forced and block is non 0 length - if (!res && forceWrite && newBlockSize) { + if (!(res < 0) && forceWrite && newBlockSize) { req.offset = newLastBlock * _blockSize; req.dataLen = newBlockSize; memset(mb.data, 0, req.dataLen); @@ -379,7 +379,7 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { if (base) res = base->truncate(size); const bool forceWrite = true; - if (!res) + if (!(res < 0)) res = padFile(oldSize, size, forceWrite); } else if (size == oldSize) { // the easiest case, but least likely.... diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index fae102b..6cf9282 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -222,7 +222,7 @@ int CipherFileIO::initHeader() { req.dataLen = 8; int res = base->write(req); - if (res) + if (res < 0) return res; } else { VLOG(1) << "base not writable, IV not written.."; @@ -252,7 +252,7 @@ bool CipherFileIO::writeHeader() { req.data = buf; req.dataLen = 8; - if (base->write(req)) + if (base->write(req) < 0) return false; return true; @@ -461,7 +461,7 @@ int CipherFileIO::truncate(off_t size) { // the wrong size.. res = BlockFileIO::truncateBase(size, 0); - if (res == 0) res = base->truncate(size + HEADER_SIZE); + if (!(res < 0)) res = base->truncate(size + HEADER_SIZE); } return res; } diff --git a/encfs/MACFileIO.cpp b/encfs/MACFileIO.cpp index 15bb032..9a17c7b 100644 --- a/encfs/MACFileIO.cpp +++ b/encfs/MACFileIO.cpp @@ -253,7 +253,7 @@ int MACFileIO::truncate(off_t size) { int res = BlockFileIO::truncateBase(size, 0); - if (res == 0) res = base->truncate(locWithHeader(size, bs, headerSize)); + if (!(res < 0)) res = base->truncate(locWithHeader(size, bs, headerSize)); return res; } diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index b2cbb26..231970d 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -246,7 +246,7 @@ int RawFileIO::write(const IORequest &req) { if (last > fileSize) fileSize = last; } - return 0; + return 0; //No matter how many bytes we wrote, we of course already know this. } } diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index ad0acec..64a47b0 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -616,10 +616,10 @@ int encfs_fsync(const char *path, int dataSync, struct fuse_file_info *file) { int _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) { int res = fnode->write(offset, ptr, size); - if (!res) - return size; - else + if (res < 0) return res; + else + return size; } int encfs_write(const char *path, const char *buf, size_t size, off_t offset, From eb94f782450dde2062c2bf75870f146b8293cd35 Mon Sep 17 00:00:00 2001 From: benrubson Date: Mon, 7 Aug 2017 09:11:45 +0200 Subject: [PATCH 12/22] HAVE_FDATASYNC is not into master --- CMakeLists.txt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bfcf7d2..62eab18 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -93,12 +93,6 @@ check_cxx_source_compiles ("#include " XATTR_ADD_OPT) set (CMAKE_REQUIRED_QUIET ${CMAKE_REQUIRED_QUIET_SAVE}) -# Check if we have fdatasync. -include (CheckCXXSourceCompiles) -check_cxx_source_compiles ("#include - int main() { fdatasync(0); return 1; } - " FDATASYNC) - # Check if we have some standard functions. include (CheckFuncs) check_function_exists_glibc (lchmod HAVE_LCHMOD) From 5c90ad451e16e69f3a73ff1541788aeec7a7aa1e Mon Sep 17 00:00:00 2001 From: benrubson Date: Wed, 9 Aug 2017 17:36:20 +0200 Subject: [PATCH 13/22] Make write return the number of bytes written --- encfs/BlockFileIO.cpp | 70 +++++++++++++++++++++++++----------------- encfs/BlockFileIO.h | 6 ++-- encfs/CipherFileIO.cpp | 27 ++++++++-------- encfs/CipherFileIO.h | 2 +- encfs/FileIO.h | 4 +-- encfs/FileNode.cpp | 11 +++++-- encfs/FileNode.h | 4 +-- encfs/MACFileIO.cpp | 14 ++++----- encfs/MACFileIO.h | 2 +- encfs/RawFileIO.cpp | 4 +-- encfs/RawFileIO.h | 2 +- encfs/encfs.cpp | 21 ++++++++----- 12 files changed, 95 insertions(+), 72 deletions(-) diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index 399d35e..5cf323c 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -69,7 +69,7 @@ ssize_t BlockFileIO::cacheReadOneBlock(const IORequest &req) const { * the lower file may have changed behind our back. */ if ((!_noCache) && (req.offset == _cache.offset) && (_cache.dataLen != 0)) { // satisfy request from cache - int len = req.dataLen; + size_t len = req.dataLen; if (_cache.dataLen < len) { len = _cache.dataLen; // Don't read past EOF } @@ -97,13 +97,13 @@ ssize_t BlockFileIO::cacheReadOneBlock(const IORequest &req) const { return result; } -int BlockFileIO::cacheWriteOneBlock(const IORequest &req) { +ssize_t BlockFileIO::cacheWriteOneBlock(const IORequest &req) { // cache results of write (before pass-thru, because it may be modified // in-place) memcpy(_cache.data, req.data, req.dataLen); _cache.offset = req.offset; _cache.dataLen = req.dataLen; - int res = writeOneBlock(req); + ssize_t res = writeOneBlock(req); if (res < 0) { clearCache(_cache, _blockSize); } @@ -121,7 +121,7 @@ int BlockFileIO::cacheWriteOneBlock(const IORequest &req) { ssize_t BlockFileIO::read(const IORequest &req) const { CHECK(_blockSize != 0); - int partialOffset = req.offset % _blockSize; + int partialOffset = req.offset % _blockSize; //can be int as _blockSize is int off_t blockNum = req.offset / _blockSize; ssize_t result = 0; @@ -161,7 +161,7 @@ ssize_t BlockFileIO::read(const IORequest &req) const { } if (readSize <= partialOffset) break; // didn't get enough bytes - int cpySize = min((size_t)(readSize - partialOffset), size); + size_t cpySize = min((size_t)(readSize - partialOffset), size); CHECK(cpySize <= readSize); // if we read to a temporary buffer, then move the data @@ -188,10 +188,9 @@ ssize_t BlockFileIO::read(const IORequest &req) const { } /** - * Returns the very first result returned by RawFileIO::write so : - * 0 in case of success, or -errno in case of failure. + * Returns the number of bytes written, or -errno in case of failure. */ -int BlockFileIO::write(const IORequest &req) { +ssize_t BlockFileIO::write(const IORequest &req) { CHECK(_blockSize != 0); off_t fileSize = getSize(); @@ -201,11 +200,11 @@ int BlockFileIO::write(const IORequest &req) { // where write request begins off_t blockNum = req.offset / _blockSize; - int partialOffset = req.offset % _blockSize; + int partialOffset = req.offset % _blockSize; //can be int as _blockSize is int // last block of file (for testing write overlaps with file boundary) off_t lastFileBlock = fileSize / _blockSize; - ssize_t lastBlockSize = fileSize % _blockSize; + size_t lastBlockSize = fileSize % _blockSize; off_t lastNonEmptyBlock = lastFileBlock; if (lastBlockSize == 0) { @@ -243,7 +242,7 @@ int BlockFileIO::write(const IORequest &req) { blockReq.data = nullptr; blockReq.dataLen = _blockSize; - int res = 0; + ssize_t res = 0; size_t size = req.dataLen; unsigned char *inPtr = req.data; while (size != 0u) { @@ -272,10 +271,12 @@ int BlockFileIO::write(const IORequest &req) { } else { // have to merge with existing block data.. blockReq.dataLen = _blockSize; - if ((blockReq.dataLen = cacheReadOneBlock(blockReq)) < 0) { - res = blockReq.dataLen; + ssize_t readSize = cacheReadOneBlock(blockReq); + if (readSize < 0) { + res = readSize; break; } + blockReq.dataLen = readSize; // extend data if necessary.. if (partialOffset + toCopy > blockReq.dataLen) { @@ -303,20 +304,22 @@ int BlockFileIO::write(const IORequest &req) { MemoryPool::release(mb); } - return res; + if (res < 0) { + return res; + } + return req.dataLen; } int BlockFileIO::blockSize() const { return _blockSize; } /** - * Returns the very first result returned by RawFileIO::write so : - * 0 in case of success, or -errno in case of failure. + * Returns 0 in case of success, or -errno in case of failure. */ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { off_t oldLastBlock = oldSize / _blockSize; off_t newLastBlock = newSize / _blockSize; - int newBlockSize = newSize % _blockSize; - int res = 0; + int newBlockSize = newSize % _blockSize; //can be int as _blockSize is int + ssize_t res = 0; IORequest req; MemBlock mb; @@ -366,7 +369,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { // 2, pad zero blocks unless holes are allowed if (!_allowHoles) { - for (; !(res < 0) && (oldLastBlock != newLastBlock); ++oldLastBlock) { + for (; (res >= 0) && (oldLastBlock != newLastBlock); ++oldLastBlock) { VLOG(1) << "padding block " << oldLastBlock; req.offset = oldLastBlock * _blockSize; req.dataLen = _blockSize; @@ -376,7 +379,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { } // 3. only necessary if write is forced and block is non 0 length - if (!(res < 0) && forceWrite && newBlockSize) { + if ((res >= 0) && forceWrite && newBlockSize) { req.offset = newLastBlock * _blockSize; req.dataLen = newBlockSize; memset(mb.data, 0, req.dataLen); @@ -388,11 +391,17 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { MemoryPool::release(mb); } - return res; + if (res < 0) { + return res; + } + return 0; } +/** + * Returns 0 in case of success, or -errno in case of failure. + */ int BlockFileIO::truncateBase(off_t size, FileIO *base) { - int partialBlock = size % _blockSize; + int partialBlock = size % _blockSize; //can be int as _blockSize is int int res = 0; off_t oldSize = getSize(); @@ -407,7 +416,7 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { } const bool forceWrite = true; - if (!(res < 0)) { + if (res == 0) { res = padFile(oldSize, size, forceWrite); } } else if (size == oldSize) { @@ -424,20 +433,23 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { req.dataLen = _blockSize; req.data = mb.data; - ssize_t rdSz = cacheReadOneBlock(req); - if (rdSz < 0) { - res = rdSz; + ssize_t readSize = cacheReadOneBlock(req); + if (readSize < 0) { + res = readSize; } - // do the truncate else if (base != nullptr) { + // do the truncate res = base->truncate(size); } // write back out partial block req.dataLen = partialBlock; - if (!(res < 0)) { - res = cacheWriteOneBlock(req); + if (res == 0) { + ssize_t writeSize = cacheWriteOneBlock(req); + if (writeSize < 0) { + res = writeSize; + } } MemoryPool::release(mb); diff --git a/encfs/BlockFileIO.h b/encfs/BlockFileIO.h index ca02883..94dd71e 100644 --- a/encfs/BlockFileIO.h +++ b/encfs/BlockFileIO.h @@ -43,7 +43,7 @@ class BlockFileIO : public FileIO { // implemented in terms of blocks. virtual ssize_t read(const IORequest &req) const; - virtual int write(const IORequest &req); + virtual ssize_t write(const IORequest &req); virtual int blockSize() const; @@ -54,10 +54,10 @@ class BlockFileIO : public FileIO { // same as read(), except that the request.offset field is guarenteed to be // block aligned, and the request size will not be larger then 1 block. virtual ssize_t readOneBlock(const IORequest &req) const = 0; - virtual int writeOneBlock(const IORequest &req) = 0; + virtual ssize_t writeOneBlock(const IORequest &req) = 0; ssize_t cacheReadOneBlock(const IORequest &req) const; - int cacheWriteOneBlock(const IORequest &req); + ssize_t cacheWriteOneBlock(const IORequest &req); int _blockSize; bool _allowHoles; diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index a83d821..44b41aa 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -233,9 +233,9 @@ int CipherFileIO::initHeader() { req.data = buf; req.dataLen = 8; - int res = base->write(req); - if (res < 0) { - return res; + ssize_t writeSize = base->write(req); + if (writeSize < 0) { + return writeSize; } } else { VLOG(1) << "base not writable, IV not written.."; @@ -333,14 +333,13 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { int bs = blockSize(); off_t blockNum = req.offset / bs; - ssize_t readSize = 0; IORequest tmpReq = req; // adjust offset if we have a file header if (haveHeader && !fsConfig->reverseEncryption) { tmpReq.offset += HEADER_SIZE; } - readSize = base->read(tmpReq); + ssize_t readSize = base->read(tmpReq); bool ok; if (readSize > 0) { @@ -353,9 +352,9 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { if (readSize != bs) { VLOG(1) << "streamRead(data, " << readSize << ", IV)"; - ok = streamRead(tmpReq.data, (int)readSize, blockNum ^ fileIV); + ok = streamRead(tmpReq.data, (int)readSize, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int } else { - ok = blockRead(tmpReq.data, (int)readSize, blockNum ^ fileIV); + ok = blockRead(tmpReq.data, (int)readSize, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int } if (!ok) { @@ -370,7 +369,7 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { return readSize; } -int CipherFileIO::writeOneBlock(const IORequest &req) { +ssize_t CipherFileIO::writeOneBlock(const IORequest &req) { if (haveHeader && fsConfig->reverseEncryption) { VLOG(1) @@ -390,12 +389,12 @@ int CipherFileIO::writeOneBlock(const IORequest &req) { bool ok; if (req.dataLen != bs) { - ok = streamWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); + ok = streamWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int } else { - ok = blockWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); + ok = blockWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int } - int res = 0; + ssize_t res = 0; if (ok) { if (haveHeader) { IORequest tmpReq = req; @@ -481,7 +480,7 @@ int CipherFileIO::truncate(off_t size) { // the wrong size.. res = BlockFileIO::truncateBase(size, nullptr); - if (!(res < 0)) { + if (res == 0) { res = base->truncate(size + HEADER_SIZE); } } @@ -531,7 +530,7 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const { VLOG(1) << "Adding " << headerBytes << " header bytes"; // copy the header bytes into the data - int headerOffset = HEADER_SIZE - headerBytes; + int headerOffset = HEADER_SIZE - headerBytes; //can be int as HEADER_SIZE is int memcpy(req.data, &headerBuf[headerOffset], headerBytes); // the read does not want data beyond the header @@ -554,7 +553,7 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const { if (readBytes < 0) { return readBytes; // Return error code } - ssize_t sum = headerBytes + readBytes; + ssize_t sum = headerBytes + readBytes; //could be size_t, but as we return ssize_t... VLOG(1) << "returning sum=" << sum; return sum; } diff --git a/encfs/CipherFileIO.h b/encfs/CipherFileIO.h index 9378e88..b0ddb19 100644 --- a/encfs/CipherFileIO.h +++ b/encfs/CipherFileIO.h @@ -65,7 +65,7 @@ class CipherFileIO : public BlockFileIO { private: virtual ssize_t readOneBlock(const IORequest &req) const; - virtual int writeOneBlock(const IORequest &req); + virtual ssize_t writeOneBlock(const IORequest &req); virtual int generateReverseHeader(unsigned char *data); int initHeader(); diff --git a/encfs/FileIO.h b/encfs/FileIO.h index 73de8b7..c1060db 100644 --- a/encfs/FileIO.h +++ b/encfs/FileIO.h @@ -34,7 +34,7 @@ struct IORequest { off_t offset; // amount of bytes to read/write. - int dataLen; + size_t dataLen; unsigned char *data; IORequest(); @@ -68,7 +68,7 @@ class FileIO { virtual off_t getSize() const = 0; virtual ssize_t read(const IORequest &req) const = 0; - virtual int write(const IORequest &req) = 0; + virtual ssize_t write(const IORequest &req) = 0; virtual int truncate(off_t size) = 0; diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp index f23b3ae..de65d89 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -224,7 +224,7 @@ off_t FileNode::getSize() const { return res; } -ssize_t FileNode::read(off_t offset, unsigned char *data, ssize_t size) const { +ssize_t FileNode::read(off_t offset, unsigned char *data, size_t size) const { IORequest req; req.offset = offset; req.dataLen = size; @@ -235,7 +235,7 @@ ssize_t FileNode::read(off_t offset, unsigned char *data, ssize_t size) const { return io->read(req); } -int FileNode::write(off_t offset, unsigned char *data, ssize_t size) { +ssize_t FileNode::write(off_t offset, unsigned char *data, size_t size) { VLOG(1) << "FileNode::write offset " << offset << ", data size " << size; IORequest req; @@ -245,7 +245,12 @@ int FileNode::write(off_t offset, unsigned char *data, ssize_t size) { Lock _lock(mutex); - return io->write(req); + ssize_t res = io->write(req); + //Of course due to encryption we genrally write more than requested + if (res < 0) { + return res; + } + return size; } int FileNode::truncate(off_t size) { diff --git a/encfs/FileNode.h b/encfs/FileNode.h index 6279c33..0eff109 100644 --- a/encfs/FileNode.h +++ b/encfs/FileNode.h @@ -79,8 +79,8 @@ class FileNode { int getAttr(struct stat *stbuf) const; off_t getSize() const; - ssize_t read(off_t offset, unsigned char *data, ssize_t size) const; - int write(off_t offset, unsigned char *data, ssize_t size); + ssize_t read(off_t offset, unsigned char *data, size_t size) const; + ssize_t write(off_t offset, unsigned char *data, size_t size); // truncate the file to a particular size int truncate(off_t size); diff --git a/encfs/MACFileIO.cpp b/encfs/MACFileIO.cpp index df7a753..cf12560 100644 --- a/encfs/MACFileIO.cpp +++ b/encfs/MACFileIO.cpp @@ -147,7 +147,7 @@ off_t MACFileIO::getSize() const { ssize_t MACFileIO::readOneBlock(const IORequest &req) const { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; + int bs = blockSize() + headerSize; //ok, should clearly fit into an int MemBlock mb = MemoryPool::allocate(bs); @@ -214,10 +214,10 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const { return readSize; } -int MACFileIO::writeOneBlock(const IORequest &req) { +ssize_t MACFileIO::writeOneBlock(const IORequest &req) { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; + int bs = blockSize() + headerSize; //ok, should clearly fit into an int // we have the unencrypted data, so we need to attach a header to it. MemBlock mb = MemoryPool::allocate(bs); @@ -247,20 +247,20 @@ int MACFileIO::writeOneBlock(const IORequest &req) { } // now, we can let the next level have it.. - int res = base->write(newReq); + ssize_t writeSize = base->write(newReq); MemoryPool::release(mb); - return res; + return writeSize; } int MACFileIO::truncate(off_t size) { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; + int bs = blockSize() + headerSize; //ok, should clearly fit into an int int res = BlockFileIO::truncateBase(size, nullptr); - if (!(res < 0)) { + if (res == 0) { res = base->truncate(locWithHeader(size, bs, headerSize)); } diff --git a/encfs/MACFileIO.h b/encfs/MACFileIO.h index eb80013..026a4c8 100644 --- a/encfs/MACFileIO.h +++ b/encfs/MACFileIO.h @@ -64,7 +64,7 @@ class MACFileIO : public BlockFileIO { private: virtual ssize_t readOneBlock(const IORequest &req) const; - virtual int writeOneBlock(const IORequest &req); + virtual ssize_t writeOneBlock(const IORequest &req); std::shared_ptr base; std::shared_ptr cipher; diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index d7dbc03..27aa021 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -212,7 +212,7 @@ ssize_t RawFileIO::read(const IORequest &req) const { return readSize; } -int RawFileIO::write(const IORequest &req) { +ssize_t RawFileIO::write(const IORequest &req) { rAssert(fd >= 0); rAssert(canWrite); @@ -253,7 +253,7 @@ int RawFileIO::write(const IORequest &req) { } } - return 0; //No matter how many bytes we wrote, we of course already know this. + return req.dataLen; } return true; diff --git a/encfs/RawFileIO.h b/encfs/RawFileIO.h index be69313..370edd5 100644 --- a/encfs/RawFileIO.h +++ b/encfs/RawFileIO.h @@ -46,7 +46,7 @@ class RawFileIO : public FileIO { virtual off_t getSize() const; virtual ssize_t read(const IORequest &req) const; - virtual int write(const IORequest &req); + virtual ssize_t write(const IORequest &req); virtual int truncate(off_t size); diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 0afc117..1ddbf46 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #ifdef linux #include #endif @@ -686,12 +687,17 @@ int encfs_release(const char *path, struct fuse_file_info *finfo) { } } -int _do_read(FileNode *fnode, unsigned char *ptr, size_t size, off_t off) { +ssize_t _do_read(FileNode *fnode, unsigned char *ptr, size_t size, off_t off) { return fnode->read(off, ptr, size); } int encfs_read(const char *path, char *buf, size_t size, off_t offset, struct fuse_file_info *file) { + //Unfortunately we have to convert from ssize_t (pread) to int (fuse), so let's check this will be OK + if (size > std::numeric_limits::max()) { + RLOG(ERROR) << "tried to read too much data: " << size; + return -EIO; + } return withFileNode("read", path, file, bind(_do_read, _1, (unsigned char *)buf, size, offset)); } @@ -708,16 +714,17 @@ int encfs_fsync(const char *path, int dataSync, struct fuse_file_info *file) { return withFileNode("fsync", path, file, bind(_do_fsync, _1, dataSync)); } -int _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) { - int res = fnode->write(offset, ptr, size); - if (res < 0) { - return res; - } - return size; +ssize_t _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) { + return fnode->write(offset, ptr, size); } int encfs_write(const char *path, const char *buf, size_t size, off_t offset, struct fuse_file_info *file) { + //Unfortunately we have to convert from ssize_t (pwrite) to int (fuse), so let's check this will be OK + if (size > std::numeric_limits::max()) { + RLOG(ERROR) << "tried to write too much data: " << size; + return -EIO; + } EncFS_Context *ctx = context(); if (isReadOnly(ctx)) { return -EROFS; From 3cd97c356839bf462fd4585fc06e06b40c0ce0b9 Mon Sep 17 00:00:00 2001 From: benrubson Date: Thu, 10 Aug 2017 12:18:46 +0200 Subject: [PATCH 14/22] Check (not entirely yet) read and write return codes --- .travis.yml | 6 +++++- encfs/RawFileIO.cpp | 7 +++++-- integration/normal.t.pl | 39 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 5bd591f..f763982 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,11 @@ dist: trusty language: cpp -sudo: true +sudo: required + +env: + global: + - SUDO_MOUNT=sudo os: - linux diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index 27aa021..886ab87 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -202,11 +202,13 @@ ssize_t RawFileIO::read(const IORequest &req) const { rAssert(fd >= 0); ssize_t readSize = pread(fd, req.data, req.dataLen, req.offset); + int eno = errno; + errno = 0; //just to be sure error seen in integration tests really comes from the function rc. if (readSize < 0) { - readSize = -errno; RLOG(WARNING) << "read failed at offset " << req.offset << " for " - << req.dataLen << " bytes: " << strerror(-readSize); + << req.dataLen << " bytes: " << strerror(eno); + return -eno; } return readSize; @@ -226,6 +228,7 @@ ssize_t RawFileIO::write(const IORequest &req) { errno = 0; ssize_t writeSize = ::pwrite(fd, buf, bytes, offset); eno = errno; + errno = 0; //just to be sure error seen in integration tests really comes from the function rc. if (writeSize < 0) { knownSize = false; diff --git a/integration/normal.t.pl b/integration/normal.t.pl index 3d184ad..9da2d5a 100755 --- a/integration/normal.t.pl +++ b/integration/normal.t.pl @@ -2,7 +2,7 @@ # Test EncFS normal and paranoid mode -use Test::More tests => 122; +use Test::More tests => 132; use File::Path; use File::Copy; use File::Temp; @@ -78,6 +78,8 @@ sub runTests &grow; &umask0777; &create_unmount_remount; + &checkReadError; + &checkWriteError; &configFromPipe; &cleanup; @@ -430,3 +432,38 @@ sub create_unmount_remount portable_unmount($mnt); } + +# Test that read errors are correctly thrown up to us +sub checkReadError +{ + # Not sure how to implement this, so feel free ! + ok(1, "read error"); +} + +# Test that write errors are correctly thrown up to us +sub checkWriteError +{ + # Not sure how to implement this on Mac OS, so feel free ! + if($^O eq "darwin") { + ok(1, "write error"); + ok(1, "write error"); + ok(1, "write error"); + ok(1, "write error"); + } + else { + my $crypt = "$workingDir/checkWriteError.crypt"; + my $mnt = "$workingDir/checkWriteError.mnt"; + mkdir($crypt) || BAIL_OUT($!); + mkdir($mnt) || BAIL_OUT($!); + system(($ENV{'SUDO_MOUNT'}||"")." mount -t tmpfs -o size=1m tmpfs $crypt"); + ok( $? == 0, "mount command returns 0") || return; + system("./build/encfs --standard --extpass=\"echo test\" $crypt $mnt 2>&1"); + ok( $? == 0, "encfs command returns 0") || return; + ok(open(OUT , "> $mnt/file"), "write content"); + while(print OUT "0123456789") {} + ok ($!{ENOSPC}, "write returned $! instead of ENOSPC"); + close OUT; + portable_unmount($mnt); + system(($ENV{'SUDO_MOUNT'}||"")." umount $crypt"); + } +} From 59ba9de03f093ce052450ee1efdf458bb1d348a4 Mon Sep 17 00:00:00 2001 From: benrubson Date: Thu, 10 Aug 2017 12:23:28 +0200 Subject: [PATCH 15/22] Typo --- encfs/RawFileIO.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index 886ab87..0a92c12 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -143,6 +143,7 @@ int RawFileIO::open(int flags) { if ((newFd == -1) && (eno == EACCES)) { VLOG(1) << "using readonly workaround for open"; newFd = open_readonly_workaround(name.c_str(), finalFlags); + eno = errno; } if (newFd >= 0) { @@ -157,8 +158,8 @@ int RawFileIO::open(int flags) { oldfd = fd; result = fd = newFd; } else { - result = -errno; - RLOG(DEBUG) << "::open error: " << strerror(-result); + result = -eno; + RLOG(DEBUG) << "::open error: " << strerror(eno); } } @@ -248,18 +249,15 @@ ssize_t RawFileIO::write(const IORequest &req) { << req.dataLen << ", max retries reached"; knownSize = false; return (eno) ? -eno : -EIO; - } else { - if (knownSize) { - off_t last = req.offset + req.dataLen; - if (last > fileSize) { - fileSize = last; - } + } + if (knownSize) { + off_t last = req.offset + req.dataLen; + if (last > fileSize) { + fileSize = last; } - - return req.dataLen; } - return true; + return req.dataLen; } int RawFileIO::truncate(off_t size) { From 8e13120b18d71282b9e312adda259c4634f547f5 Mon Sep 17 00:00:00 2001 From: benrubson Date: Thu, 10 Aug 2017 13:04:24 +0200 Subject: [PATCH 16/22] Check BADMSG return codes --- integration/normal.t.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/normal.t.pl b/integration/normal.t.pl index 9da2d5a..f47a380 100755 --- a/integration/normal.t.pl +++ b/integration/normal.t.pl @@ -111,7 +111,7 @@ sub corruption ok( open(IN, "< $crypt/corrupt"), "open corrupted file"); my $content; $result = read(IN, $content, 20); - ok(! defined $result, "corrupted file with MAC returns read error: $!"); + ok($!{EBADMSG} && (! defined $result), "corrupted file with MAC returns read error: $!"); } # Test internal modification From acbeb647658a334ff00b2f2b1dac2addb2082bad Mon Sep 17 00:00:00 2001 From: benrubson Date: Fri, 11 Aug 2017 07:51:21 +0200 Subject: [PATCH 17/22] Make write() write all its data even if it takes time --- encfs/RawFileIO.cpp | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index 0a92c12..706574c 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -203,10 +203,10 @@ ssize_t RawFileIO::read(const IORequest &req) const { rAssert(fd >= 0); ssize_t readSize = pread(fd, req.data, req.dataLen, req.offset); - int eno = errno; - errno = 0; //just to be sure error seen in integration tests really comes from the function rc. if (readSize < 0) { + int eno = errno; + errno = 0; //just to be sure error seen in integration tests really comes from the function rc. RLOG(WARNING) << "read failed at offset " << req.offset << " for " << req.dataLen << " bytes: " << strerror(eno); return -eno; @@ -219,37 +219,43 @@ ssize_t RawFileIO::write(const IORequest &req) { rAssert(fd >= 0); rAssert(canWrite); - int retrys = 10; + // int retrys = 10; void *buf = req.data; ssize_t bytes = req.dataLen; off_t offset = req.offset; - int eno = 0; - while ((bytes != 0) && retrys > 0) { - errno = 0; + /* + * Let's write while pwrite() writes, to avoid writing only a part of the request, + * whereas it could have been fully written. This to avoid inconsistencies / corruption. + */ + // while ((bytes != 0) && retrys > 0) { + while (bytes != 0) { ssize_t writeSize = ::pwrite(fd, buf, bytes, offset); - eno = errno; - errno = 0; //just to be sure error seen in integration tests really comes from the function rc. - if (writeSize < 0) { + if (writeSize <= 0) { + int eno = errno; + errno = 0; //just to be sure error seen in integration tests really comes from the function rc. knownSize = false; RLOG(WARNING) << "write failed at offset " << offset << " for " << bytes << " bytes: " << strerror(eno); - return -eno; + // pwrite is not expected to return 0, so eno should always be set, but we never know... + if (eno) { + return -eno; + } + return -EIO; } bytes -= writeSize; offset += writeSize; buf = (void *)((char *)buf + writeSize); - --retrys; } - if (bytes != 0) { - RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of " - << req.dataLen << ", max retries reached"; - knownSize = false; - return (eno) ? -eno : -EIO; - } + // if (bytes != 0) { + // RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of " + // << req.dataLen << ", max retries reached"; + // knownSize = false; + // return (eno) ? -eno : -EIO; + // } if (knownSize) { off_t last = req.offset + req.dataLen; if (last > fileSize) { From 41d5ec4cba37a390ec5b5010269503072157bade Mon Sep 17 00:00:00 2001 From: benrubson Date: Tue, 15 Aug 2017 16:15:58 +0200 Subject: [PATCH 18/22] Open file for write for every truncate solves #382 --- encfs/CipherFileIO.cpp | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index 44b41aa..824356e 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -456,34 +456,40 @@ bool CipherFileIO::streamRead(unsigned char *buf, int size, int CipherFileIO::truncate(off_t size) { int res = 0; + int reopen = 0; + // well, we will truncate, so we need a write access to the file + if (!base->isWritable()) { + int newFlags = lastFlags | O_RDWR; + int res = base->open(newFlags); + if (res < 0) { + VLOG(1) << "truncate failed to re-open for write"; + base->open(lastFlags); + return res; + } + reopen = 1; + } if (!haveHeader) { res = BlockFileIO::truncateBase(size, base.get()); } else { if (0 == fileIV) { // empty file.. create the header.. - if (!base->isWritable()) { - // open for write.. - int newFlags = lastFlags | O_RDWR; - int res = base->open(newFlags); - if (res < 0) { - VLOG(1) << "truncate failed to re-open for write"; - return res; - } - } - int res = initHeader(); - if (res < 0) { - return res; - } + res = initHeader(); } - // can't let BlockFileIO call base->truncate(), since it would be using // the wrong size.. - res = BlockFileIO::truncateBase(size, nullptr); - + if (res == 0) { + res = BlockFileIO::truncateBase(size, nullptr); + } if (res == 0) { res = base->truncate(size + HEADER_SIZE); } } + if (reopen == 1) { + reopen = base->open(lastFlags); + if (res < 0) { + res = reopen; + } + } return res; } From e2e1fadfa9c571dc0c9f44d52c52a942d47cb5f5 Mon Sep 17 00:00:00 2001 From: Valient Gough Date: Fri, 25 Aug 2017 22:29:52 -0700 Subject: [PATCH 19/22] skip sudo tests by default unless in CI --- .travis.yml | 4 ++-- integration/normal.t.pl | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 955c119..8c587a3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,14 +37,14 @@ matrix: - clang-4.0 - clang-tidy-4.0 env: - - CC=clang-4.0 CXX=clang++-4.0 CHECK=true INTEGRATION=true SUDO_MOUNT=sudo + - CC=clang-4.0 CXX=clang++-4.0 CHECK=true INTEGRATION=true SUDO_TESTS=true - os: osx compiler: clang osx_image: xcode8.3 sudo: true env: - - INTEGRATION=true SUDO_MOUNT=sudo + - INTEGRATION=true SUDO_TESTS=true before_script: - ./ci/setup.sh diff --git a/integration/normal.t.pl b/integration/normal.t.pl index f47a380..840d32c 100755 --- a/integration/normal.t.pl +++ b/integration/normal.t.pl @@ -443,8 +443,8 @@ sub checkReadError # Test that write errors are correctly thrown up to us sub checkWriteError { - # Not sure how to implement this on Mac OS, so feel free ! - if($^O eq "darwin") { + # No OSX impl, and requires sudo which is inconvenient outside of CI. + if($^O eq "darwin" || !defined($ENV{'SUDO_TESTS'})) { ok(1, "write error"); ok(1, "write error"); ok(1, "write error"); @@ -455,7 +455,7 @@ sub checkWriteError my $mnt = "$workingDir/checkWriteError.mnt"; mkdir($crypt) || BAIL_OUT($!); mkdir($mnt) || BAIL_OUT($!); - system(($ENV{'SUDO_MOUNT'}||"")." mount -t tmpfs -o size=1m tmpfs $crypt"); + system("sudo mount -t tmpfs -o size=1m tmpfs $crypt"); ok( $? == 0, "mount command returns 0") || return; system("./build/encfs --standard --extpass=\"echo test\" $crypt $mnt 2>&1"); ok( $? == 0, "encfs command returns 0") || return; @@ -464,6 +464,6 @@ sub checkWriteError ok ($!{ENOSPC}, "write returned $! instead of ENOSPC"); close OUT; portable_unmount($mnt); - system(($ENV{'SUDO_MOUNT'}||"")." umount $crypt"); + system("sudo umount $crypt"); } } From 9089b8555f53448f444cb7366cb0cfa89e47b869 Mon Sep 17 00:00:00 2001 From: Valient Gough Date: Fri, 25 Aug 2017 22:34:33 -0700 Subject: [PATCH 20/22] cleanup lint warnings, run clang-format --- CMakeLists.txt | 16 +++++++++++++- encfs/BlockFileIO.cpp | 18 ++++++++++------ encfs/BlockNameIO.cpp | 6 ++++-- encfs/CipherFileIO.cpp | 49 ++++++++++++++++++++++++------------------ encfs/Context.cpp | 7 +++--- encfs/Context.h | 4 ++-- encfs/DirNode.cpp | 2 +- encfs/FileNode.cpp | 4 ++-- encfs/MACFileIO.cpp | 6 +++--- encfs/RawFileIO.cpp | 24 ++++++++++++--------- encfs/SSL_Cipher.cpp | 12 ++++++----- encfs/XmlReader.cpp | 2 +- encfs/encfs.cpp | 22 +++++++++++-------- 13 files changed, 105 insertions(+), 67 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 76e617a..f3cd724 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -168,7 +168,21 @@ if (${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION} GREATER 3.5) # Need 3.6 or abo message(STATUS "clang-tidy not found.") else() message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}") - set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-checks=*,-modernize-loop-convert,-cppcoreguidelines-pro-*,-readability-inconsistent-declaration-parameter-name,-google-readability-casting,-cert-err58-cpp,-google-runtime-int,-readability-named-parameter,-google-build-using-namespace,-misc-unused-parameters,-google-runtime-references") + string(CONCAT TIDY_OPTS "-checks=*" + ",-cert-err58-cpp" + ",-cppcoreguidelines-pro-*" + ",-google-build-using-namespace" + ",-google-readability-casting" + ",-google-readability-todo" + ",-google-runtime-int" + ",-google-runtime-references" + ",-misc-misplaced-widening-cast" + ",-misc-unused-parameters" + ",-modernize-loop-convert" + ",-readability-inconsistent-declaration-parameter-name" + ",-readability-named-parameter" + ) + set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" ${TIDY_OPTS}) #set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-checks=-*,google-readability-redundant-smartptr-get") endif() else() diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index 5cf323c..b0eede1 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -121,7 +121,8 @@ ssize_t BlockFileIO::cacheWriteOneBlock(const IORequest &req) { ssize_t BlockFileIO::read(const IORequest &req) const { CHECK(_blockSize != 0); - int partialOffset = req.offset % _blockSize; //can be int as _blockSize is int + int partialOffset = + req.offset % _blockSize; // can be int as _blockSize is int off_t blockNum = req.offset / _blockSize; ssize_t result = 0; @@ -159,7 +160,9 @@ ssize_t BlockFileIO::read(const IORequest &req) const { result = readSize; break; } - if (readSize <= partialOffset) break; // didn't get enough bytes + if (readSize <= partialOffset) { + break; // didn't get enough bytes + } size_t cpySize = min((size_t)(readSize - partialOffset), size); CHECK(cpySize <= readSize); @@ -200,7 +203,8 @@ ssize_t BlockFileIO::write(const IORequest &req) { // where write request begins off_t blockNum = req.offset / _blockSize; - int partialOffset = req.offset % _blockSize; //can be int as _blockSize is int + int partialOffset = + req.offset % _blockSize; // can be int as _blockSize is int // last block of file (for testing write overlaps with file boundary) off_t lastFileBlock = fileSize / _blockSize; @@ -267,7 +271,7 @@ ssize_t BlockFileIO::write(const IORequest &req) { if (blockNum > lastNonEmptyBlock) { // just pad.. - blockReq.dataLen = toCopy + partialOffset; + blockReq.dataLen = partialOffset + toCopy; } else { // have to merge with existing block data.. blockReq.dataLen = _blockSize; @@ -318,7 +322,7 @@ int BlockFileIO::blockSize() const { return _blockSize; } int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { off_t oldLastBlock = oldSize / _blockSize; off_t newLastBlock = newSize / _blockSize; - int newBlockSize = newSize % _blockSize; //can be int as _blockSize is int + int newBlockSize = newSize % _blockSize; // can be int as _blockSize is int ssize_t res = 0; IORequest req; @@ -379,7 +383,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { } // 3. only necessary if write is forced and block is non 0 length - if ((res >= 0) && forceWrite && newBlockSize) { + if ((res >= 0) && forceWrite && (newBlockSize != 0)) { req.offset = newLastBlock * _blockSize; req.dataLen = newBlockSize; memset(mb.data, 0, req.dataLen); @@ -401,7 +405,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { * Returns 0 in case of success, or -errno in case of failure. */ int BlockFileIO::truncateBase(off_t size, FileIO *base) { - int partialBlock = size % _blockSize; //can be int as _blockSize is int + int partialBlock = size % _blockSize; // can be int as _blockSize is int int res = 0; off_t oldSize = getSize(); diff --git a/encfs/BlockNameIO.cpp b/encfs/BlockNameIO.cpp index e6ebeb9..d8c1070 100644 --- a/encfs/BlockNameIO.cpp +++ b/encfs/BlockNameIO.cpp @@ -166,8 +166,9 @@ int BlockNameIO::encodeName(const char *plaintextName, int length, uint64_t *iv, bool ok; ok = _cipher->blockEncode((unsigned char *)encodedName + 2, length + padding, (uint64_t)mac ^ tmpIV, _key); - if (!ok) + if (!ok) { throw Error("block encode failed in filename encode"); + } // convert to base 64 ascii int encodedStreamLen = length + 2 + padding; @@ -225,8 +226,9 @@ int BlockNameIO::decodeName(const char *encodedName, int length, uint64_t *iv, bool ok; ok = _cipher->blockDecode((unsigned char *)tmpBuf + 2, decodedStreamLen, (uint64_t)mac ^ tmpIV, _key); - if (!ok) + if (!ok) { throw Error("block decode failed in filename decode"); + } // find out true string length int padding = (unsigned char)tmpBuf[2 + decodedStreamLen - 1]; diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index 824356e..ad5c42c 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -106,10 +106,9 @@ bool CipherFileIO::setIV(uint64_t iv) { // duh -- there are no file headers for directories! externalIV = iv; return base->setIV(iv); - } else { - VLOG(1) << "setIV failed to re-open for write"; - return false; } + VLOG(1) << "setIV failed to re-open for write"; + return false; } if (fileIV == 0) { if (initHeader() < 0) { @@ -189,11 +188,11 @@ int CipherFileIO::initHeader() { req.data = buf; req.dataLen = 8; ssize_t readSize = base->read(req); - if(readSize < 0) { + if (readSize < 0) { return readSize; } - if(!cipher->streamDecode(buf, sizeof(buf), externalIV, key)) { + if (!cipher->streamDecode(buf, sizeof(buf), externalIV, key)) { return -EBADMSG; } @@ -224,7 +223,7 @@ int CipherFileIO::initHeader() { } while (fileIV == 0); // don't accept 0 as an option.. if (base->isWritable()) { - if(!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) { + if (!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) { return -EBADMSG; } @@ -257,7 +256,7 @@ bool CipherFileIO::writeHeader() { fileIV >>= 8; } - if(!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) { + if (!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) { return false; } @@ -266,11 +265,7 @@ bool CipherFileIO::writeHeader() { req.data = buf; req.dataLen = 8; - if (base->write(req) < 0) { - return false; - } - - return true; + return (base->write(req) >= 0); } /** @@ -319,8 +314,9 @@ int CipherFileIO::generateReverseHeader(unsigned char *headerBuf) { VLOG(1) << "fileIV=" << fileIV; // Encrypt externally-visible header - if(!cipher->streamEncode(headerBuf, HEADER_SIZE, externalIV, key)) + if (!cipher->streamEncode(headerBuf, HEADER_SIZE, externalIV, key)) { return -EBADMSG; + } return 0; } @@ -352,9 +348,13 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { if (readSize != bs) { VLOG(1) << "streamRead(data, " << readSize << ", IV)"; - ok = streamRead(tmpReq.data, (int)readSize, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int + ok = streamRead(tmpReq.data, (int)readSize, + blockNum ^ fileIV); // cast works because we work on a + // block and blocksize fit an int } else { - ok = blockRead(tmpReq.data, (int)readSize, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int + ok = blockRead(tmpReq.data, (int)readSize, + blockNum ^ fileIV); // cast works because we work on a + // block and blocksize fit an int } if (!ok) { @@ -389,9 +389,13 @@ ssize_t CipherFileIO::writeOneBlock(const IORequest &req) { bool ok; if (req.dataLen != bs) { - ok = streamWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int + ok = streamWrite(req.data, (int)req.dataLen, + blockNum ^ fileIV); // cast works because we work on a + // block and blocksize fit an int } else { - ok = blockWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int + ok = blockWrite(req.data, (int)req.dataLen, + blockNum ^ fileIV); // cast works because we work on a + // block and blocksize fit an int } ssize_t res = 0; @@ -400,8 +404,9 @@ ssize_t CipherFileIO::writeOneBlock(const IORequest &req) { IORequest tmpReq = req; tmpReq.offset += HEADER_SIZE; res = base->write(tmpReq); - } else + } else { res = base->write(req); + } } else { VLOG(1) << "encodeBlock failed for block " << blockNum << ", size " << req.dataLen; @@ -487,7 +492,7 @@ int CipherFileIO::truncate(off_t size) { if (reopen == 1) { reopen = base->open(lastFlags); if (res < 0) { - res = reopen; + res = reopen; } } return res; @@ -536,7 +541,8 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const { VLOG(1) << "Adding " << headerBytes << " header bytes"; // copy the header bytes into the data - int headerOffset = HEADER_SIZE - headerBytes; //can be int as HEADER_SIZE is int + int headerOffset = + HEADER_SIZE - headerBytes; // can be int as HEADER_SIZE is int memcpy(req.data, &headerBuf[headerOffset], headerBytes); // the read does not want data beyond the header @@ -559,7 +565,8 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const { if (readBytes < 0) { return readBytes; // Return error code } - ssize_t sum = headerBytes + readBytes; //could be size_t, but as we return ssize_t... + ssize_t sum = + headerBytes + readBytes; // could be size_t, but as we return ssize_t... VLOG(1) << "returning sum=" << sum; return sum; } diff --git a/encfs/Context.cpp b/encfs/Context.cpp index bfbcbf3..781661d 100644 --- a/encfs/Context.cpp +++ b/encfs/Context.cpp @@ -111,7 +111,8 @@ void EncFS_Context::renameNode(const char *from, const char *to) { // putNode stores "node" under key "path" in the "openFiles" map. It // increments the reference count if the key already exists. -void EncFS_Context::putNode(const char *path, std::shared_ptr node) { +void EncFS_Context::putNode(const char *path, + const std::shared_ptr &node) { Lock lock(contextMutex); auto &list = openFiles[std::string(path)]; // The length of "list" serves as the reference count. @@ -122,7 +123,7 @@ void EncFS_Context::putNode(const char *path, std::shared_ptr node) { // eraseNode is called by encfs_release in response to the RELEASE // FUSE-command we get from the kernel. void EncFS_Context::eraseNode(const char *path, - std::shared_ptr fnode) { + const std::shared_ptr &fnode) { Lock lock(contextMutex); auto it = openFiles.find(std::string(path)); @@ -151,7 +152,7 @@ void EncFS_Context::eraseNode(const char *path, // nextFuseFh returns the next unused uint64 to serve as the FUSE file // handle for the kernel. -uint64_t EncFS_Context::nextFuseFh(void) { +uint64_t EncFS_Context::nextFuseFh() { // This is thread-safe because currentFuseFh is declared as std::atomic return currentFuseFh++; } diff --git a/encfs/Context.h b/encfs/Context.h index 56b0fc4..d77334f 100644 --- a/encfs/Context.h +++ b/encfs/Context.h @@ -48,9 +48,9 @@ class EncFS_Context { void getAndResetUsageCounter(int *usage, int *openCount); - void putNode(const char *path, std::shared_ptr node); + void putNode(const char *path, const std::shared_ptr &node); - void eraseNode(const char *path, std::shared_ptr fnode); + void eraseNode(const char *path, const std::shared_ptr &fnode); void renameNode(const char *oldName, const char *newName); diff --git a/encfs/DirNode.cpp b/encfs/DirNode.cpp index 17a30f4..8dc3335 100644 --- a/encfs/DirNode.cpp +++ b/encfs/DirNode.cpp @@ -149,7 +149,7 @@ class RenameOp { ~RenameOp(); - operator bool() const { return renameList != nullptr; } + explicit operator bool() const { return renameList != nullptr; } bool apply(); void undo(); diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp index de65d89..7486720 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -246,9 +246,9 @@ ssize_t FileNode::write(off_t offset, unsigned char *data, size_t size) { Lock _lock(mutex); ssize_t res = io->write(req); - //Of course due to encryption we genrally write more than requested + // Of course due to encryption we genrally write more than requested if (res < 0) { - return res; + return res; } return size; } diff --git a/encfs/MACFileIO.cpp b/encfs/MACFileIO.cpp index cf12560..072a7f7 100644 --- a/encfs/MACFileIO.cpp +++ b/encfs/MACFileIO.cpp @@ -147,7 +147,7 @@ off_t MACFileIO::getSize() const { ssize_t MACFileIO::readOneBlock(const IORequest &req) const { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; //ok, should clearly fit into an int + int bs = blockSize() + headerSize; // ok, should clearly fit into an int MemBlock mb = MemoryPool::allocate(bs); @@ -217,7 +217,7 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const { ssize_t MACFileIO::writeOneBlock(const IORequest &req) { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; //ok, should clearly fit into an int + int bs = blockSize() + headerSize; // ok, should clearly fit into an int // we have the unencrypted data, so we need to attach a header to it. MemBlock mb = MemoryPool::allocate(bs); @@ -256,7 +256,7 @@ ssize_t MACFileIO::writeOneBlock(const IORequest &req) { int MACFileIO::truncate(off_t size) { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; //ok, should clearly fit into an int + int bs = blockSize() + headerSize; // ok, should clearly fit into an int int res = BlockFileIO::truncateBase(size, nullptr); diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index 706574c..1b04ffb 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -206,7 +206,8 @@ ssize_t RawFileIO::read(const IORequest &req) const { if (readSize < 0) { int eno = errno; - errno = 0; //just to be sure error seen in integration tests really comes from the function rc. + errno = 0; // just to be sure error seen in integration tests really comes + // from the function rc. RLOG(WARNING) << "read failed at offset " << req.offset << " for " << req.dataLen << " bytes: " << strerror(eno); return -eno; @@ -225,23 +226,25 @@ ssize_t RawFileIO::write(const IORequest &req) { off_t offset = req.offset; /* - * Let's write while pwrite() writes, to avoid writing only a part of the request, - * whereas it could have been fully written. This to avoid inconsistencies / corruption. + * Let's write while pwrite() writes, to avoid writing only a part of the + * request, + * whereas it could have been fully written. This to avoid inconsistencies / + * corruption. */ // while ((bytes != 0) && retrys > 0) { while (bytes != 0) { ssize_t writeSize = ::pwrite(fd, buf, bytes, offset); - if (writeSize <= 0) { + if (writeSize < 0) { int eno = errno; - errno = 0; //just to be sure error seen in integration tests really comes from the function rc. knownSize = false; RLOG(WARNING) << "write failed at offset " << offset << " for " << bytes << " bytes: " << strerror(eno); - // pwrite is not expected to return 0, so eno should always be set, but we never know... - if (eno) { - return -eno; - } + // pwrite is not expected to return 0, so eno should always be set, but we + // never know... + return -eno; + } + if (writeSize == 0) { return -EIO; } @@ -251,7 +254,8 @@ ssize_t RawFileIO::write(const IORequest &req) { } // if (bytes != 0) { - // RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of " + // RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of + // " // << req.dataLen << ", max retries reached"; // knownSize = false; // return (eno) ? -eno : -EIO; diff --git a/encfs/SSL_Cipher.cpp b/encfs/SSL_Cipher.cpp index deb16d5..b530fdc 100644 --- a/encfs/SSL_Cipher.cpp +++ b/encfs/SSL_Cipher.cpp @@ -106,7 +106,7 @@ int BytesToKey(int keyLen, int ivLen, const EVP_MD *md, memcpy(iv, mdBuf + offset, toCopy); iv += toCopy; niv -= toCopy; - offset += toCopy; + // offset += toCopy; } if ((nkey == 0) && (niv == 0)) { break; @@ -170,12 +170,14 @@ static Range CAMELLIABlockRange(64, 4096, 16); static std::shared_ptr NewCAMELLIACipher(const Interface &iface, int keyLen) { - if (keyLen <= 0) keyLen = 192; + if (keyLen <= 0) { + keyLen = 192; + } keyLen = CAMELLIAKeyRange.closest(keyLen); - const EVP_CIPHER *blockCipher = 0; - const EVP_CIPHER *streamCipher = 0; + const EVP_CIPHER *blockCipher = nullptr; + const EVP_CIPHER *streamCipher = nullptr; switch (keyLen) { case 128: @@ -503,7 +505,7 @@ CipherKey SSL_Cipher::newRandomKey() { compute a 64-bit check value for the data using HMAC. */ static uint64_t _checksum_64(SSLKey *key, const unsigned char *data, - int dataLen, uint64_t *chainedIV) { + int dataLen, uint64_t *const chainedIV) { rAssert(dataLen > 0); Lock lock(key->mutex); diff --git a/encfs/XmlReader.cpp b/encfs/XmlReader.cpp index 1e4cd20..b28c264 100644 --- a/encfs/XmlReader.cpp +++ b/encfs/XmlReader.cpp @@ -103,7 +103,7 @@ bool XmlValue::readB64(const char *path, unsigned char *data, std::string s = value->text(); s.erase(std::remove_if(s.begin(), s.end(), ::isspace), s.end()); - s.erase(s.find_last_not_of("=") + 1); + s.erase(s.find_last_not_of('=') + 1); int decodedSize = B64ToB256Bytes(s.size()); if (decodedSize != length) { diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 1ddbf46..721c28a 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -25,13 +25,13 @@ #include #include #include +#include #include #include #include #include #include #include -#include #ifdef linux #include #endif @@ -65,7 +65,7 @@ using namespace std::placeholders; namespace encfs { -#define GET_FN(ctx, finfo) ctx->getNode((void *)(uintptr_t)finfo->fh) +#define GET_FN(ctx, finfo) (ctx)->getNode((void *)(uintptr_t)(finfo)->fh) static EncFS_Context *context() { return (EncFS_Context *)fuse_get_context()->private_data; @@ -79,9 +79,10 @@ static EncFS_Context *context() { static bool isReadOnly(EncFS_Context *ctx) { return ctx->opts->readOnly; } // helper function -- apply a functor to a cipher path, given the plain path -static int withCipherPath(const char *opName, const char *path, - function op, - bool passReturnCode = false) { +static int withCipherPath( + const char *opName, const char *path, + const function &op, + bool passReturnCode = false) { EncFS_Context *ctx = context(); int res = -EIO; @@ -110,7 +111,7 @@ static int withCipherPath(const char *opName, const char *path, return res; } -static void checkCanary(std::shared_ptr fnode) { +static void checkCanary(const std::shared_ptr &fnode) { if (fnode->canary == CANARY_OK) { return; } @@ -693,7 +694,8 @@ ssize_t _do_read(FileNode *fnode, unsigned char *ptr, size_t size, off_t off) { int encfs_read(const char *path, char *buf, size_t size, off_t offset, struct fuse_file_info *file) { - //Unfortunately we have to convert from ssize_t (pread) to int (fuse), so let's check this will be OK + // Unfortunately we have to convert from ssize_t (pread) to int (fuse), so + // let's check this will be OK if (size > std::numeric_limits::max()) { RLOG(ERROR) << "tried to read too much data: " << size; return -EIO; @@ -714,13 +716,15 @@ int encfs_fsync(const char *path, int dataSync, struct fuse_file_info *file) { return withFileNode("fsync", path, file, bind(_do_fsync, _1, dataSync)); } -ssize_t _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) { +ssize_t _do_write(FileNode *fnode, unsigned char *ptr, size_t size, + off_t offset) { return fnode->write(offset, ptr, size); } int encfs_write(const char *path, const char *buf, size_t size, off_t offset, struct fuse_file_info *file) { - //Unfortunately we have to convert from ssize_t (pwrite) to int (fuse), so let's check this will be OK + // Unfortunately we have to convert from ssize_t (pwrite) to int (fuse), so + // let's check this will be OK if (size > std::numeric_limits::max()) { RLOG(ERROR) << "tried to write too much data: " << size; return -EIO; From 64c3b4459ca79c76ff83e1681be07f6dc1e3c37a Mon Sep 17 00:00:00 2001 From: Valient Gough Date: Fri, 25 Aug 2017 22:52:59 -0700 Subject: [PATCH 21/22] cleanup code, check errno usage --- encfs/RawFileIO.cpp | 75 +++++++++++++++++++++++---------------------- encfs/main.cpp | 3 +- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index 1b04ffb..6f722c9 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -90,6 +90,8 @@ Interface RawFileIO::interface() const { return RawFileIO_iface; } This works around the problem described in https://github.com/vgough/encfs/issues/181 Without this, "umask 0777 ; echo foo > bar" fails. + + Sets errno when -1 is returned. */ static int open_readonly_workaround(const char *path, int flags) { int fd = -1; @@ -118,52 +120,53 @@ int RawFileIO::open(int flags) { bool requestWrite = (((flags & O_RDWR) != 0) || ((flags & O_WRONLY) != 0)); VLOG(1) << "open call, requestWrite = " << requestWrite; - int result = 0; - // if we have a descriptor and it is writable, or we don't need writable.. if ((fd >= 0) && (canWrite || !requestWrite)) { VLOG(1) << "using existing file descriptor"; - result = fd; // success - } else { - int finalFlags = requestWrite ? O_RDWR : O_RDONLY; + return fd; // success + } + + int finalFlags = requestWrite ? O_RDWR : O_RDONLY; #if defined(O_LARGEFILE) - if ((flags & O_LARGEFILE) != 0) { - finalFlags |= O_LARGEFILE; - } + if ((flags & O_LARGEFILE) != 0) { + finalFlags |= O_LARGEFILE; + } #else #warning O_LARGEFILE not supported #endif - int newFd = ::open(name.c_str(), finalFlags); - int eno = errno; - - VLOG(1) << "open file with flags " << finalFlags << ", result = " << newFd; - - if ((newFd == -1) && (eno == EACCES)) { - VLOG(1) << "using readonly workaround for open"; - newFd = open_readonly_workaround(name.c_str(), finalFlags); - eno = errno; - } - - if (newFd >= 0) { - if (oldfd >= 0) { - RLOG(ERROR) << "leaking FD?: oldfd = " << oldfd << ", fd = " << fd - << ", newfd = " << newFd; - } - - // the old fd might still be in use, so just keep it around for - // now. - canWrite = requestWrite; - oldfd = fd; - result = fd = newFd; - } else { - result = -eno; - RLOG(DEBUG) << "::open error: " << strerror(eno); - } + int eno; + int newFd = ::open(name.c_str(), finalFlags); + if (newFd < 0) { + eno = errno; } - return result; + VLOG(1) << "open file with flags " << finalFlags << ", result = " << newFd; + + if ((newFd == -1) && (eno == EACCES)) { + VLOG(1) << "using readonly workaround for open"; + newFd = open_readonly_workaround(name.c_str(), finalFlags); + eno = errno; + } + + if (newFd < 0) { + RLOG(DEBUG) << "::open error: " << strerror(eno); + return -eno; + } + + if (oldfd >= 0) { + RLOG(ERROR) << "leaking FD?: oldfd = " << oldfd << ", fd = " << fd + << ", newfd = " << newFd; + } + + // the old fd might still be in use, so just keep it around for + // now. + canWrite = requestWrite; + oldfd = fd; + fd = newFd; + + return fd; } int RawFileIO::getAttr(struct stat *stbuf) const { @@ -206,8 +209,6 @@ ssize_t RawFileIO::read(const IORequest &req) const { if (readSize < 0) { int eno = errno; - errno = 0; // just to be sure error seen in integration tests really comes - // from the function rc. RLOG(WARNING) << "read failed at offset " << req.offset << " for " << req.dataLen << " bytes: " << strerror(eno); return -eno; diff --git a/encfs/main.cpp b/encfs/main.cpp index 3ae55d6..345a853 100644 --- a/encfs/main.cpp +++ b/encfs/main.cpp @@ -532,10 +532,9 @@ void *encfs_init(fuse_conn_info *conn) { int res = pthread_create(&ctx->monitorThread, nullptr, idleMonitor, (void *)ctx); if (res != 0) { - int eno = errno; RLOG(ERROR) << "error starting idle monitor thread, " "res = " - << res << ", " << strerror(eno); + << res << ", " << strerror(res); } } From 1e43b05712d5b5eb026265812089b42b14de8d96 Mon Sep 17 00:00:00 2001 From: Valient Gough Date: Fri, 25 Aug 2017 23:00:11 -0700 Subject: [PATCH 22/22] spread CI work over targets --- .travis.yml | 12 ++++++------ build.sh | 5 +++-- ci/setup.sh | 6 +++--- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8c587a3..3e84dd1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,7 @@ matrix: - os: linux compiler: gcc dist: trusty - sudo: false + sudo: required addons: apt: sources: @@ -17,12 +17,12 @@ matrix: - gettext - cmake3 env: - - INTEGRATION=false + - SUDO_TESTS=true - os: linux compiler: clang dist: trusty - sudo: true + sudo: false addons: apt: sources: @@ -37,14 +37,14 @@ matrix: - clang-4.0 - clang-tidy-4.0 env: - - CC=clang-4.0 CXX=clang++-4.0 CHECK=true INTEGRATION=true SUDO_TESTS=true + - CC=clang-4.0 CXX=clang++-4.0 CHECK=true INTEGRATION=false CMAKE=/tmp/bin/cmake - os: osx compiler: clang osx_image: xcode8.3 - sudo: true + sudo: required env: - - INTEGRATION=true SUDO_TESTS=true + - SUDO_TESTS=true before_script: - ./ci/setup.sh diff --git a/build.sh b/build.sh index af1b323..1379f28 100755 --- a/build.sh +++ b/build.sh @@ -1,9 +1,10 @@ #!/bin/bash -eu +: ${CMAKE:=cmake} : ${CHECK:=false} : ${INTEGRATION:=true} -cmake --version +${CMAKE} --version CFG=$* if [[ "$CHECK" == "true" ]]; then @@ -20,7 +21,7 @@ then fi cd build -cmake .. ${CFG} +${CMAKE} .. ${CFG} make -j2 make test if [[ "$INTEGRATION" == "true" ]]; then diff --git a/ci/setup.sh b/ci/setup.sh index 81bc0ce..51e743c 100755 --- a/ci/setup.sh +++ b/ci/setup.sh @@ -1,6 +1,6 @@ #!/bin/bash -eu -: ${INTEGRATION:=false} +: ${INTEGRATION:=true} : ${CHECK:=false} if [[ "$INTEGRATION" == "true" ]]; then @@ -15,8 +15,8 @@ if [[ "$CHECK" == "true" ]]; then if uname -s | grep -q Linux; then wget https://cmake.org/files/v3.9/cmake-3.9.1-Linux-x86_64.tar.gz -O /tmp/cmake.tar.gz tar -C /tmp/ -xf /tmp/cmake.tar.gz - sudo rm -f $(which cmake) - sudo ln -s $(ls -1 /tmp/cmake*/bin/cmake) /bin/ + mkdir /tmp/bin + ln -s $(ls -1 /tmp/cmake*/bin/cmake) /tmp/bin/ fi fi