From b6b9988e58778038424d6697caf31db1b93eea25 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Franti=C5=A1ek=20Dvo=C5=99=C3=A1k?= Date: Thu, 10 Oct 2013 14:44:14 +0200 Subject: [PATCH] Add permissions check to catalog calls (but always returning OK for now, only print ugly debug message). --- src/VfsNs.cpp | 317 ++++++++++++++++++++++++++++++++++++++++++++++++++++------ src/VfsNs.h | 16 ++- 2 files changed, 298 insertions(+), 35 deletions(-) diff --git a/src/VfsNs.cpp b/src/VfsNs.cpp index d483dcb..5d0f3bc 100644 --- a/src/VfsNs.cpp +++ b/src/VfsNs.cpp @@ -19,6 +19,17 @@ using namespace dmlite; +static uid_t getUid(const SecurityContext* ctx) { + return ctx->user.getUnsigned("uid"); +} + + + +static gid_t getGid(const SecurityContext* ctx) { + return ctx->groups[0].getUnsigned("gid"); +} + + VfsCatalog::VfsCatalog(const std::string& host) throw (DmException): Catalog(), hostName_(host) { @@ -50,7 +61,7 @@ void VfsCatalog::setStackInstance(StackInstance* si) throw (DmException) void VfsCatalog::setSecurityContext(const SecurityContext* ctx) throw (DmException) { - // TODO + secCtx_ = ctx; } @@ -71,18 +82,57 @@ std::string VfsCatalog::getWorkingDir(void) throw (DmException) -ExtendedStat VfsCatalog::extendedStat(const std::string& path, bool follow) throw (DmException) -{ +// +// helper function +// (without setting st_nlink and checking permissions) +// +ExtendedStat VfsCatalog::vfsExtendedStat(const std::string& name, const std::string& path, bool follow) throw (DmException) { ExtendedStat xStat; struct stat fstat; if (follow) - wrapCall(stat(path.c_str(), &fstat)); + wrapCall(stat(path.c_str(), &fstat), "could not stat '%s'", path.c_str()); else - wrapCall(lstat(path.c_str(), &fstat)); + wrapCall(lstat(path.c_str(), &fstat), "could not lstat '%s'", path.c_str()); + + xStat.acl = Acl(); + xStat.name = name; + xStat.stat = fstat; + xStat.parent = 0; + xStat.status = ExtendedStat::kOnline; + xStat["pool"] = std::string("vfs"); + + return xStat; +} + + + +ExtendedStat VfsCatalog::extendedStat(const std::string& path, bool follow) throw (DmException) +{ + ExtendedStat meta; + std::string dir, c; + std::vector components; + + // check permissions from the top + if (path[0] == '/') dir = path; + else dir = this->getWorkingDir() + "/" + path; + components = Url::splitPath(dir); + + dir = ""; + for (unsigned int i = 0; i < components.size() - 1; i++) { + if (i < 2) dir += components[i]; + else dir = dir + "/" + components[i]; + + meta = vfsExtendedStat(components[i], dir, true); + + if (checkPermissions(this->secCtx_, meta.acl, meta.stat, S_IEXEC) != 0) + vfsThrow(EACCES, "not enough permissions to list '%s'", meta.name.c_str()); + } + + meta = vfsExtendedStat(components.back(), path, follow); // black magic - dmlite tests require proper count in st_nlink - if (S_ISDIR(fstat.st_mode)) { + if (S_ISDIR(meta.stat.st_mode)) { Directory *dir; struct dirent *entry; int count = 0; @@ -94,35 +144,38 @@ ExtendedStat VfsCatalog::extendedStat(const std::string& path, bool follow) thro count++; } closeDir(dir); - fstat.st_nlink = count; + meta.stat.st_nlink = count; } - std::vector components = Url::splitPath(path); - - xStat.stat = fstat; - xStat.parent = 0; - xStat.name = components.back(); - xStat.status = ExtendedStat::kOnline; - xStat["pool"] = std::string("vfs"); - - return xStat; + return meta; } bool VfsCatalog::access(const std::string& path, int mode) throw (DmException) { - struct stat fstat; + ExtendedStat meta; + mode_t perm; - if (stat(path.c_str(), &fstat) == -1) { - if (errno == ENOENT) { + try { + meta = this->extendedStat(path); + + perm = 0; + if (mode & R_OK) perm = S_IREAD; + if (mode & W_OK) perm |= S_IWRITE; + if (mode & X_OK) perm |= S_IEXEC; + } catch (DmException& e) { + if (e.code() == ENOENT) { // throw exeption up only if there was not requested test for existence (F_OK) if (mode & F_OK) return false; - else vfsThrow(DMLITE_NO_SUCH_REPLICA, "stat() failed on '" + path + "': " + strerror(errno)); - } else - vfsThrowErrno("stat() failed on '" + path + "'"); + else throw DmException(DMLITE_NO_SUCH_REPLICA, e.what()); + } else { + throw; + } } + if (checkPermissions(this->secCtx_, meta.acl, meta.stat, perm) != 0) return false; + if (::access(path.c_str(), mode) == -1) { if (errno == ENOENT) { // stat() performed OK, but access() failed with ENOENT @@ -159,6 +212,12 @@ std::vector VfsCatalog::getReplicas(const std::string& path) throw (DmE { Replica replica; ExtendedStat xStat = this->extendedStat(path, true); + + if (checkPermissions(this->secCtx_, xStat.acl, xStat.stat, S_IREAD) != 0) + vfsThrow(EACCES, "not enough permissions to read '%s'", path.c_str()); + + if (S_ISDIR(xStat.stat.st_mode)) + vfsThrow(EISDIR, "directories do not have replicas"); replica.replicaid = 0; replica.atime = xStat.stat.st_atime; @@ -182,9 +241,16 @@ std::vector VfsCatalog::getReplicas(const std::string& path) throw (DmE -void VfsCatalog::symlink(const std::string& oldpath, const std::string& newpath) throw (DmException) +void VfsCatalog::symlink(const std::string& oldPath, const std::string& newPath) throw (DmException) { - wrapCall(::symlink(oldpath.c_str(), newpath.c_str())); + std::string parentPath, symName; + + ExtendedStat parent = this->getParent(newPath, &parentPath, &symName); + + if (checkPermissions(this->secCtx_, parent.acl, parent.stat, S_IWRITE | S_IEXEC) != 0) + vfsThrow(EACCES, "not enough permissions on '%s'", parentPath.c_str()); + + wrapCall(::symlink(oldPath.c_str(), newPath.c_str())); } @@ -192,6 +258,10 @@ void VfsCatalog::symlink(const std::string& oldpath, const std::string& newpath) std::string VfsCatalog::readLink(const std::string& path) throw (DmException) { char buf[PATH_MAX]; + ExtendedStat meta = this->extendedStat(path, false); + + if (checkPermissions(this->secCtx_, meta.acl, meta.stat, S_IREAD) != 0) + vfsThrow(EACCES, "not enough permissions on '%s'", path.c_str()); memset(buf, 0, sizeof(buf)); wrapCall(readlink(path.c_str(), buf, sizeof(buf) - 1)); @@ -202,6 +272,24 @@ std::string VfsCatalog::readLink(const std::string& path) throw (DmException) void VfsCatalog::unlink(const std::string& path) throw (DmException) { + std::string parentPath, name; + + ExtendedStat parent = this->getParent(path, &parentPath, &name); + + if (checkPermissions(this->secCtx_, parent.acl, parent.stat, S_IEXEC) != 0) + vfsThrow(EACCES, "not enough permissions on '%s' to list", parentPath.c_str()); + if (checkPermissions(this->secCtx_, parent.acl, parent.stat, S_IWRITE) != 0) + vfsThrow(EACCES, "not enough permissions on '%s' to unlink '%s'", parentPath.c_str(), path.c_str()); + + // Sticky bit set ==> only directory or file owner can delete + if ((parent.stat.st_mode & S_ISVTX) == S_ISVTX) { + ExtendedStat file = this->extendedStat(path); + if (getUid(this->secCtx_) != file.stat.st_uid && + getUid(this->secCtx_) != parent.stat.st_uid) { + vfsThrow(EACCES, "not enough permissions to unlink '%s' (sticky bit set)", path.c_str()); + } + } + wrapCall(::unlink(path.c_str())); } @@ -210,9 +298,53 @@ void VfsCatalog::unlink(const std::string& path) throw (DmException) void VfsCatalog::create(const std::string& path, mode_t mode) throw (DmException) { FILE *f; + std::string parentPath, name; + int code = DMLITE_SUCCESS; + ExtendedStat file; + + ExtendedStat parent = this->getParent(path, &parentPath, &name); - wrapCall(f = fopen(path.c_str(), "w")); - wrapCall(fclose(f)); + if (checkPermissions(this->secCtx_, parent.acl, parent.stat, S_IEXEC) != 0) + vfsThrow(EACCES, "not enough permissions on '%s' to list", parentPath.c_str()); + if (checkPermissions(this->secCtx_, parent.acl, parent.stat, S_IWRITE) != 0) + vfsThrow(EACCES, "need write access on '%s' to create '%s'", parentPath.c_str(), path.c_str()); + + try { + file = this->extendedStat(path); + } catch (DmException e) { + code = DMLITE_ERRNO(e.code()); + if (code != ENOENT) throw; + } + +#if 0 + // Effective gid + gid_t egid; + if (parent.stat.st_mode & S_ISGID) { + egid = parent.stat.st_gid; + mode |= S_ISGID; + } else { + egid = getGid(this->secCtx_); + } + // Generate inherited ACL's if there are defaults + if (parent.acl.has(AclEntry::kDefault | AclEntry::kUserObj)) { + newFile.acl = Acl(parent.acl, getUid(this->secCtx_), egid, mode, &newFile.stat.st_mode); + } +#endif + + if (code == ENOENT) { + // Create new + wrapCall(f = fopen(path.c_str(), "w")); + wrapCall(fclose(f)); + } else { + // Truncate + if (S_ISDIR(file.stat.st_mode)) + vfsThrow(EISDIR, "'%s' is directory, can not truncate", path.c_str()); + if (getUid(this->secCtx_) != file.stat.st_uid && + checkPermissions(this->secCtx_, file.acl, file.stat, S_IWRITE) != 0) { + vfsThrow(EACCES, "not enough permissions to truncate '%s'", path.c_str()); + } + wrapCall(truncate(path.c_str(), 0)); + } setMode(path, mode); } @@ -228,6 +360,20 @@ mode_t VfsCatalog::umask(mode_t mask) throw () void VfsCatalog::setMode(const std::string& path, mode_t mode) throw (DmException) { + ExtendedStat meta = this->extendedStat(path); + + // User has to be the owner, or root + if (getUid(this->secCtx_) != meta.stat.st_uid && + getUid(this->secCtx_) != 0) { + vfsThrow(EACCES, "only the owner or root can set the mode of '%s'", path.c_str()); + } + // Clean up unwanted bits + mode &= ~S_IFMT; + // Keep type bits + mode |= (meta.stat.st_mode & S_IFMT); + // TODO: buildin resets S_ISGID if not in the group + // TODO: update ACL (kUserObj, kGroupObj/kMask, kOther) + wrapCall(chmod(path.c_str(), mode)); } @@ -242,6 +388,14 @@ void VfsCatalog::setOwner(const std::string& path, uid_t newUid, gid_t newGid, b void VfsCatalog::setSize(const std::string& path, size_t newSize) throw (DmException) { + ExtendedStat file = this->extendedStat(path); + + if (S_ISDIR(file.stat.st_mode)) + vfsThrow(EISDIR, "'%s' is directory, can not truncate", path.c_str()); + if (getUid(this->secCtx_) != file.stat.st_uid && + checkPermissions(this->secCtx_, file.acl, file.stat, S_IWRITE) != 0) { + vfsThrow(EACCES, "not enough permissions to truncate '%s'", path.c_str()); + } wrapCall(truncate(path.c_str(), newSize)); } @@ -265,7 +419,14 @@ void VfsCatalog::setAcl(const std::string& path, const Acl& acl) throw (DmExcept void VfsCatalog::utime(const std::string& path, const struct utimbuf* buf) throw (DmException) { - ::utime(path.c_str(), buf); + ExtendedStat meta = this->extendedStat(path); + + if (getUid(this->secCtx_) != meta.stat.st_uid && + checkPermissions(this->secCtx_, meta.acl, meta.stat, S_IWRITE) != 0) { + vfsThrow(EACCES, "not enough permissions to modify the time of '%s'", path.c_str()); + } + + wrapCall(::utime(path.c_str(), buf)); } @@ -306,14 +467,21 @@ void VfsCatalog::updateExtendedAttributes(const std::string& path, Directory* VfsCatalog::openDir(const std::string& path) throw (DmException) { PrivateDir *privateDir; + ExtendedStat meta; privateDir = new PrivateDir(); - privateDir->path = path; - privateDir->dir = opendir(path.c_str()); - if (privateDir->dir == NULL) { + try { + meta = this->extendedStat(path); + if (checkPermissions(this->secCtx_, meta.acl, meta.stat, S_IREAD) != 0) + vfsThrow(EACCES, "not enough permissions to read '%s'", path.c_str()); + + privateDir->path = path; + privateDir->dir = opendir(path.c_str()); + if (privateDir->dir == NULL) + vfsThrowErrno("error opening directory '%s'", path.c_str()); + } catch (...) { delete privateDir; - vfsThrowErrno("error opening directory '" + path + "'"); - return NULL; + throw; } return privateDir; @@ -375,6 +543,19 @@ ExtendedStat* VfsCatalog::readDirx(Directory* dir) throw (DmException) void VfsCatalog::makeDir(const std::string& path, mode_t mode) throw (DmException) { + std::string parentPath, name; + + ExtendedStat parent = this->getParent(path, &parentPath, &name); + + if (checkPermissions(this->secCtx_, parent.acl, parent.stat, S_IEXEC) != 0) + vfsThrow(EACCES, "not enough permissions on '%s' to list", parentPath.c_str()); + if (checkPermissions(this->secCtx_, parent.acl, parent.stat, S_IWRITE) != 0) + vfsThrow(EACCES, "need write access on '%s' to create directory '%s'", parentPath.c_str(), name.c_str()); + + // Clean up unwanted bits + mode &= ~S_IFMT; + // TODO: S_ISGID + // TODO: inherit default ACL wrapCall(mkdir(path.c_str(), mode)); } @@ -382,6 +563,27 @@ void VfsCatalog::makeDir(const std::string& path, mode_t mode) throw (DmExceptio void VfsCatalog::rename(const std::string& oldPath, const std::string& newPath) throw (DmException) { + std::string oldParentPath, newParentPath, oldName, newName; + + if (oldPath == "/" || newPath == "/") + vfsThrow(EINVAL, "not the source, neither the destination, can be '/'"); + + ExtendedStat oldParent = this->getParent(oldPath, &oldParentPath, &oldName); + ExtendedStat newParent = this->getParent(newPath, &oldParentPath, &newName); + if (checkPermissions(this->secCtx_, oldParent.acl, oldParent.stat, S_IWRITE) != 0) + vfsThrow(EACCES, "Not enough permissions on origin '%s'", oldPath.c_str()); + if (checkPermissions(this->secCtx_, newParent.acl, newParent.stat, S_IWRITE) != 0) + vfsThrow(EACCES, "Not enough permissions on destination '%s'", newPath.c_str()); + + // Check sticky + if (oldParent.stat.st_mode & S_ISVTX) { + ExtendedStat old = this->extendedStat(oldPath); + if (getUid(this->secCtx_) != oldParent.stat.st_uid && + getUid(this->secCtx_) != old.stat.st_uid && + checkPermissions(this->secCtx_, old.acl, old.stat, S_IWRITE) != 0) + vfsThrow(EACCES, "not enough permissions (sticky bit set on parent '%s')", oldParentPath.c_str()); + } + // XXX: test-rename should probably accept both ENOTEMPTY and EEXIST instead // of changing it here? try { @@ -397,6 +599,24 @@ void VfsCatalog::rename(const std::string& oldPath, const std::string& newPath) void VfsCatalog::removeDir(const std::string& path) throw (DmException) { char *cwd = NULL, *rmd = NULL; + std::string parentPath, name; + + if (path == "/") + vfsThrow(EINVAL, "can not remove '/'"); + + ExtendedStat parent = this->getParent(path, &parentPath, &name); + ExtendedStat entry = this->extendedStat(path); + if ((parent.stat.st_mode & S_ISVTX) == S_ISVTX) { + // Sticky bit set + if (getUid(this->secCtx_) != entry.stat.st_uid && + getUid(this->secCtx_) != parent.stat.st_uid && + checkPermissions(this->secCtx_, entry.acl, entry.stat, S_IWRITE) != 0) + vfsThrow(EACCES, "not enough permissions to remove '%s' (sticky bit set)", path.c_str()); + } else { + // No sticky bit + if (checkPermissions(this->secCtx_, parent.acl, parent.stat, S_IWRITE) != 0) + vfsThrow(EACCES, "not enough permissions to remove '%s'", path.c_str()); + } // check if we are not removing current working directory cwd = get_current_dir_name(); @@ -404,7 +624,7 @@ void VfsCatalog::removeDir(const std::string& path) throw (DmException) changeDir(path); rmd = get_current_dir_name(); if (strcmp(cwd, rmd) == 0) { - vfsThrow(EINVAL, "removing current working directory"); + vfsThrow(EINVAL, "can not remove current working directory"); } ::chdir(cwd); // OK, we can try to remove @@ -449,3 +669,34 @@ void VfsCatalog::updateReplica(const Replica& replica) throw (DmException) { // Nothing } + + + +ExtendedStat VfsCatalog::getParent(const std::string& path, + std::string* parentPath, + std::string* name) throw (DmException) +{ + if (path.empty()) + vfsThrow(EINVAL, "empty path"); + + std::vector components = Url::splitPath(path); + + *name = components.back(); + components.pop_back(); + + *parentPath = Url::joinPath(components); + + // Get the files now + if (!parentPath->empty()) { + return this->extendedStat(*parentPath); + } else { + return this->extendedStat(this->getWorkingDir()); + } +} + + + +int VfsCatalog::checkPermissions(const SecurityContext *context, const Acl &acl, const struct stat &stat, mode_t mode) { + fprintf(stderr, "VfsCatalog::checkPermissions(inode %lu, %04o)\n", stat.st_ino, mode); + return 0; +} diff --git a/src/VfsNs.h b/src/VfsNs.h index 54ff1ec..2e5904a 100644 --- a/src/VfsNs.h +++ b/src/VfsNs.h @@ -39,7 +39,7 @@ namespace dmlite { void changeDir (const std::string&) throw (DmException); std::string getWorkingDir (void) throw (DmException); - ExtendedStat extendedStat(const std::string&, bool) throw (DmException); + ExtendedStat extendedStat(const std::string&, bool = true) throw (DmException); bool access(const std::string& path, int mode) throw (DmException); bool accessReplica(const std::string& replica, int mode) throw (DmException); @@ -92,9 +92,21 @@ namespace dmlite { void updateReplica(const Replica& replica) throw (DmException); protected: + /// Get the parent of a directory. + /// @param path The path to split. + /// @param parentPath Where to put the parent path. + /// @param name Where to put the file name (stripping last /). + /// @return The parent metadata. + ExtendedStat getParent(const std::string& path, std::string* parentPath, + std::string* name) throw (DmException); + ExtendedStat vfsExtendedStat(const std::string& name, const std::string& path, bool follow) throw (DmException); + StackInstance* si_; - + const SecurityContext* secCtx_; std::string hostName_; + + private: + int checkPermissions(const SecurityContext *context, const Acl &acl, const struct stat &stat, mode_t mode); }; }; -- 1.8.2.3