diff --git a/.travis.yml b/.travis.yml index 61cf5e3..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 + - 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 before_script: - ./ci/setup.sh 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/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 diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index ee9a835..b0eede1 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,17 +97,17 @@ ssize_t BlockFileIO::cacheReadOneBlock(const IORequest &req) const { return result; } -bool 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; - bool ok = writeOneBlock(req); - if (!ok) { + ssize_t res = writeOneBlock(req); + if (res < 0) { clearCache(_cache, _blockSize); } - return ok; + return res; } /** @@ -116,11 +116,13 @@ bool 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); - 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; @@ -154,11 +156,15 @@ 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); + size_t cpySize = min((size_t)(readSize - partialOffset), size); CHECK(cpySize <= readSize); // if we read to a temporary buffer, then move the data @@ -184,21 +190,25 @@ ssize_t BlockFileIO::read(const IORequest &req) const { return result; } -bool BlockFileIO::write(const IORequest &req) { +/** + * Returns the number of bytes written, or -errno in case of failure. + */ +ssize_t BlockFileIO::write(const IORequest &req) { CHECK(_blockSize != 0); off_t fileSize = getSize(); if (fileSize < 0) { - return false; + return fileSize; } // 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) { @@ -208,7 +218,10 @@ bool 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 < 0) { + return res; + } } // check against edge cases where we can just let the base class handle the @@ -233,7 +246,7 @@ bool BlockFileIO::write(const IORequest &req) { blockReq.data = nullptr; blockReq.dataLen = _blockSize; - bool ok = true; + ssize_t res = 0; size_t size = req.dataLen; unsigned char *inPtr = req.data; while (size != 0u) { @@ -258,11 +271,16 @@ bool 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; - blockReq.dataLen = cacheReadOneBlock(blockReq); + ssize_t readSize = cacheReadOneBlock(blockReq); + if (readSize < 0) { + res = readSize; + break; + } + blockReq.dataLen = readSize; // extend data if necessary.. if (partialOffset + toCopy > blockReq.dataLen) { @@ -274,8 +292,8 @@ bool BlockFileIO::write(const IORequest &req) { } // Finally, write the damn thing! - if (!cacheWriteOneBlock(blockReq)) { - ok = false; + res = cacheWriteOneBlock(blockReq); + if (res < 0) { break; } @@ -290,15 +308,22 @@ bool BlockFileIO::write(const IORequest &req) { MemoryPool::release(mb); } - return ok; + if (res < 0) { + return res; + } + return req.dataLen; } int BlockFileIO::blockSize() const { return _blockSize; } -void BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { +/** + * 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 newBlockSize = newSize % _blockSize; // can be int as _blockSize is int + ssize_t res = 0; IORequest req; MemBlock mb; @@ -317,9 +342,10 @@ void BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { if (outSize != 0) { 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"; @@ -338,39 +364,48 @@ 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 >= 0) && (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 != 0)) { + if ((res >= 0) && forceWrite && (newBlockSize != 0)) { req.offset = newLastBlock * _blockSize; req.dataLen = newBlockSize; memset(mb.data, 0, req.dataLen); - cacheWriteOneBlock(req); + res = cacheWriteOneBlock(req); } } if (mb.data != nullptr) { MemoryPool::release(mb); } + + 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(); @@ -381,11 +416,13 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { // do the truncate so that the underlying filesystem can allocate // the space, and then we'll fill it in padFile.. if (base != nullptr) { - base->truncate(size); + res = base->truncate(size); } const bool forceWrite = true; - padFile(oldSize, size, forceWrite); + if (res == 0) { + res = padFile(oldSize, size, forceWrite); + } } else if (size == oldSize) { // the easiest case, but least likely.... } else if (partialBlock != 0) { @@ -400,21 +437,23 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { req.dataLen = _blockSize; req.data = mb.data; - ssize_t rdSz = cacheReadOneBlock(req); + ssize_t readSize = cacheReadOneBlock(req); + if (readSize < 0) { + res = readSize; + } - // do the truncate - if (base != nullptr) { + else if (base != nullptr) { + // do the truncate res = base->truncate(size); } // write back out partial block req.dataLen = partialBlock; - bool 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 == 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 8eb4520..94dd71e 100644 --- a/encfs/BlockFileIO.h +++ b/encfs/BlockFileIO.h @@ -43,21 +43,21 @@ class BlockFileIO : public FileIO { // implemented in terms of blocks. virtual ssize_t read(const IORequest &req) const; - virtual bool write(const IORequest &req); + virtual ssize_t write(const IORequest &req); virtual int blockSize() const; 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. virtual ssize_t readOneBlock(const IORequest &req) const = 0; - virtual bool writeOneBlock(const IORequest &req) = 0; + virtual ssize_t writeOneBlock(const IORequest &req) = 0; ssize_t cacheReadOneBlock(const IORequest &req) const; - bool cacheWriteOneBlock(const IORequest &req); + ssize_t cacheWriteOneBlock(const IORequest &req); int _blockSize; bool _allowHoles; diff --git a/encfs/BlockNameIO.cpp b/encfs/BlockNameIO.cpp index 3389e91..d8c1070 100644 --- a/encfs/BlockNameIO.cpp +++ b/encfs/BlockNameIO.cpp @@ -163,8 +163,12 @@ 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; @@ -219,8 +223,12 @@ int BlockNameIO::decodeName(const char *encodedName, int length, uint64_t *iv, 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 e67095a..ad5c42c 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -98,20 +98,22 @@ 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. + // 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); + } + VLOG(1) << "setIV failed to re-open for write"; + return false; + } 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); - } - VLOG(1) << "writeHeader failed to re-open for write"; + if (initHeader() < 0) { return false; } - initHeader(); } uint64_t oldIV = externalIV; @@ -172,7 +174,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(); @@ -185,9 +187,14 @@ void 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; + } - 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) { @@ -201,7 +208,8 @@ void CipherFileIO::initHeader() { unsigned char buf[8] = {0}; do { if (!cipher->randomize(buf, 8, false)) { - throw Error("Unable to generate a random file IV"); + RLOG(ERROR) << "Unable to generate a random file IV"; + return -EBADMSG; } fileIV = 0; @@ -215,31 +223,28 @@ void 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; req.data = buf; req.dataLen = 8; - base->write(req); + ssize_t writeSize = base->write(req); + if (writeSize < 0) { + return writeSize; + } } else { VLOG(1) << "base not writable, IV not written.."; } } VLOG(1) << "initHeader finished, fileIV = " << fileIV; + return 0; } 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!!!"; } @@ -251,16 +256,16 @@ 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; req.data = buf; req.dataLen = 8; - base->write(req); - - return true; + return (base->write(req) >= 0); } /** @@ -275,7 +280,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); @@ -309,7 +314,10 @@ 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; } /** @@ -321,76 +329,90 @@ 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) { if (haveHeader && fileIV == 0) { - const_cast(this)->initHeader(); + int res = const_cast(this)->initHeader(); + if (res < 0) { + return res; + } } 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) { VLOG(1) << "decodeBlock failed for block " << blockNum << ", size " << readSize; - readSize = -1; + readSize = -EBADMSG; } - } else { + } else if (readSize == 0) { VLOG(1) << "readSize zero for offset " << req.offset; } return readSize; } -bool CipherFileIO::writeOneBlock(const IORequest &req) { +ssize_t 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(); off_t blockNum = req.offset / bs; if (haveHeader && fileIV == 0) { - initHeader(); + int res = initHeader(); + if (res < 0) { + return res; + } } 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 } + ssize_t 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, @@ -439,27 +461,38 @@ 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; - if (base->open(newFlags) < 0) { - VLOG(1) << "writeHeader failed to re-open for write"; - } - } - initHeader(); + 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) { - base->truncate(size + HEADER_SIZE); + 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; @@ -484,7 +517,10 @@ 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; @@ -505,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; + 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 @@ -528,7 +565,8 @@ 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 de88d85..b0ddb19 100644 --- a/encfs/CipherFileIO.h +++ b/encfs/CipherFileIO.h @@ -65,10 +65,10 @@ class CipherFileIO : public BlockFileIO { private: virtual ssize_t readOneBlock(const IORequest &req) const; - virtual bool writeOneBlock(const IORequest &req); - virtual void generateReverseHeader(unsigned char *data); + virtual ssize_t writeOneBlock(const IORequest &req); + virtual int 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; 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 9a3c847..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(); @@ -181,8 +181,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; } @@ -351,7 +352,8 @@ DirTraverse DirNode::openDir(const char *plaintextPath) { DIR *dir = ::opendir(cyName.c_str()); if (dir == nullptr) { - VLOG(1) << "opendir error " << strerror(errno); + int eno = errno; + VLOG(1) << "opendir error " << strerror(eno); return DirTraverse(shared_ptr(), 0, std::shared_ptr()); } std::shared_ptr dp(dir, DirDeleter()); @@ -584,8 +586,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; @@ -726,7 +727,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/FileIO.h b/encfs/FileIO.h index 474ff20..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 bool 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 33fb132..7486720 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -157,14 +157,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; } } @@ -185,6 +187,12 @@ int FileNode::mknod(mode_t mode, dev_t rdev, uid_t uid, gid_t gid) { res = ::mknod(_cname.c_str(), mode, rdev); } + if (res == -1) { + int eno = errno; + VLOG(1) << "mknod error: " << strerror(eno); + res = -eno; + } + if (olduid >= 0) { setfsuid(olduid); } @@ -192,12 +200,6 @@ int FileNode::mknod(mode_t mode, dev_t rdev, uid_t uid, gid_t gid) { setfsgid(oldgid); } - if (res == -1) { - int eno = errno; - VLOG(1) << "mknod error: " << strerror(eno); - res = -eno; - } - return res; } @@ -222,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; @@ -233,7 +235,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) { +ssize_t FileNode::write(off_t offset, unsigned char *data, size_t size) { VLOG(1) << "FileNode::write offset " << offset << ", data size " << size; IORequest req; @@ -243,7 +245,12 @@ bool 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) { @@ -266,8 +273,6 @@ int FileNode::sync(bool datasync) { } #else (void)datasync; - // no fdatasync support - // TODO: use autoconfig to check for it.. res = fsync(fh); #endif diff --git a/encfs/FileNode.h b/encfs/FileNode.h index 22accf4..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; - bool 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 b606c2a..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; + int bs = blockSize() + headerSize; // ok, should clearly fit into an int MemBlock mb = MemoryPool::allocate(bs); @@ -194,7 +194,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; } } } @@ -214,10 +214,10 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const { return readSize; } -bool 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); @@ -231,7 +231,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; } } @@ -247,21 +247,21 @@ bool MACFileIO::writeOneBlock(const IORequest &req) { } // now, we can let the next level have it.. - bool ok = base->write(newReq); + ssize_t writeSize = base->write(newReq); MemoryPool::release(mb); - return ok; + 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) { - base->truncate(locWithHeader(size, bs, headerSize)); + res = base->truncate(locWithHeader(size, bs, headerSize)); } return res; diff --git a/encfs/MACFileIO.h b/encfs/MACFileIO.h index fe39bf7..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 bool 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 624d4ec..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; @@ -97,13 +99,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; } @@ -120,50 +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); - - VLOG(1) << "open file with flags " << finalFlags << ", result = " << newFd; - - if ((newFd == -1) && (errno == EACCES)) { - VLOG(1) << "using readonly workaround for open"; - newFd = open_readonly_workaround(name.c_str(), finalFlags); - } - - 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 = -errno; - RLOG(DEBUG) << "::open error: " << strerror(errno); - } + 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 { @@ -192,8 +195,9 @@ off_t RawFileIO::getSize() const { const_cast(this)->knownSize = true; return fileSize; } - RLOG(ERROR) << "getSize on " << name << " failed: " << strerror(errno); - return -1; + int eno = errno; + RLOG(ERROR) << "getSize on " << name << " failed: " << strerror(eno); + return -eno; } return fileSize; } @@ -204,44 +208,59 @@ ssize_t RawFileIO::read(const IORequest &req) const { ssize_t readSize = pread(fd, req.data, req.dataLen, req.offset); if (readSize < 0) { + int eno = errno; RLOG(WARNING) << "read failed at offset " << req.offset << " for " - << req.dataLen << " bytes: " << strerror(errno); + << req.dataLen << " bytes: " << strerror(eno); + return -eno; } return readSize; } -bool RawFileIO::write(const IORequest &req) { +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; - while ((bytes != 0) && retrys > 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); if (writeSize < 0) { + int eno = errno; knownSize = false; RLOG(WARNING) << "write failed at offset " << offset << " for " << bytes - << " bytes: " << strerror(errno); - return false; + << " bytes: " << strerror(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; } 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 false; - } + // 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) { @@ -249,7 +268,7 @@ bool RawFileIO::write(const IORequest &req) { } } - return true; + return req.dataLen; } int RawFileIO::truncate(off_t size) { diff --git a/encfs/RawFileIO.h b/encfs/RawFileIO.h index bd74fe6..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 bool write(const IORequest &req); + virtual ssize_t write(const IORequest &req); virtual int truncate(off_t size); diff --git a/encfs/SSL_Cipher.cpp b/encfs/SSL_Cipher.cpp index 657b98f..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); @@ -810,6 +812,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; @@ -846,6 +849,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; @@ -861,7 +865,8 @@ 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"); + RLOG(ERROR) << "Invalid data size, not multiple of block size"; + return false; } Lock lock(key->mutex); @@ -879,6 +884,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; @@ -894,7 +900,8 @@ 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"); + RLOG(ERROR) << "Invalid data size, not multiple of block size"; + return false; } Lock lock(key->mutex); @@ -912,6 +919,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; 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 b70083a..721c28a 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -64,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; @@ -78,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; @@ -109,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; } @@ -408,11 +410,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); @@ -690,12 +688,18 @@ 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)); } @@ -712,15 +716,19 @@ 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) { - if (fnode->write(offset, ptr, size)) { - return size; - } - return -EIO; +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; diff --git a/encfs/encfs.h b/encfs/encfs.h index eada993..0d6b7d1 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 48e432f..345a853 100644 --- a/encfs/main.cpp +++ b/encfs/main.cpp @@ -534,7 +534,7 @@ void *encfs_init(fuse_conn_info *conn) { if (res != 0) { RLOG(ERROR) << "error starting idle monitor thread, " "res = " - << res << ", errno = " << errno; + << res << ", " << strerror(res); } } diff --git a/integration/normal.t.pl b/integration/normal.t.pl index 3d184ad..840d32c 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; @@ -109,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 @@ -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 +{ + # 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"); + ok(1, "write error"); + } + else { + my $crypt = "$workingDir/checkWriteError.crypt"; + my $mnt = "$workingDir/checkWriteError.mnt"; + mkdir($crypt) || BAIL_OUT($!); + mkdir($mnt) || BAIL_OUT($!); + 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; + ok(open(OUT , "> $mnt/file"), "write content"); + while(print OUT "0123456789") {} + ok ($!{ENOSPC}, "write returned $! instead of ENOSPC"); + close OUT; + portable_unmount($mnt); + system("sudo umount $crypt"); + } +}