From 632fbd33782973d5d1e745d6170b19416ef4cc5c Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Fri, 10 Nov 2023 00:58:52 -0500 Subject: [PATCH 01/14] [3rd-party] Update to use libssh v 0.10.5 Fixes #3296 --- 3rd-party/libssh/libssh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rd-party/libssh/libssh b/3rd-party/libssh/libssh index 3413f2fe79..88bca8225e 160000 --- a/3rd-party/libssh/libssh +++ b/3rd-party/libssh/libssh @@ -1 +1 @@ -Subproject commit 3413f2fe7940aae7be1d47665e3da8eb0d9e408b +Subproject commit 88bca8225e8c52a4db986035deeed3a22919850c From f4b375d88301ebeb3c4220353b0a7d6e85e515ca Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Fri, 10 Nov 2023 01:01:31 -0500 Subject: [PATCH 02/14] [3rd-party] Define CHUNKSIZE for libssh This allows us to drop a patch in our libssh fork. --- 3rd-party/libssh/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/3rd-party/libssh/CMakeLists.txt b/3rd-party/libssh/CMakeLists.txt index dbfb1232ac..e7a88768aa 100644 --- a/3rd-party/libssh/CMakeLists.txt +++ b/3rd-party/libssh/CMakeLists.txt @@ -61,6 +61,8 @@ set(libssh_VERSION_MAJOR "${CMAKE_MATCH_1}") set(libssh_VERSION_MINOR "${CMAKE_MATCH_2}") set(libssh_VERSION_PATCH "${CMAKE_MATCH_3}") +add_compile_options(-DCHUNKSIZE=65536) + string(REGEX REPLACE "^set\\(LIBRARY_VERSION \"(.*)\"\\)$" "\\1" LIBRARY_VERSION "${LIBRARY_VERSION}") if (NOT LIBRARY_VERSION) From d04c44ec13e79a3f5d6a1524a8a75418b894c17f Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Fri, 10 Nov 2023 01:02:36 -0500 Subject: [PATCH 03/14] [3rd-party] Update the submodule.md info file for libssh --- 3rd-party/submodule_info.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/3rd-party/submodule_info.md b/3rd-party/submodule_info.md index b43e3fb2ea..5ff54cfe53 100644 --- a/3rd-party/submodule_info.md +++ b/3rd-party/submodule_info.md @@ -12,8 +12,8 @@ Version: 1.52.1 (+[our patches](https://github.com/CanonicalLtd/grpc/compare/v1. ### libssh -Version: 0.10.4 (+[our patches](https://github.com/CanonicalLtd/libssh/compare/libssh-0.10.4..3413f2fe)) | - | +Version: 0.10.5 (+[our patches](https://github.com/canonical/libssh/compare/libssh-0.10.5..843b97db)) | + | ### fmt From bbb1907f92712b253787f7e6984d380a5201cdff Mon Sep 17 00:00:00 2001 From: sharder996 Date: Thu, 12 Oct 2023 10:57:34 -0500 Subject: [PATCH 04/14] [sftp] reverse map default id if pairing found --- src/sshfs_mount/sftp_server.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index f13b44d797..3cbe2f63b7 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -273,8 +273,12 @@ int mapped_id_for(const mp::id_mappings& id_maps, const int id, const int id_if_ int reverse_id_for(const mp::id_mappings& id_maps, const int id, const int rev_id_if_not_found) { auto found = std::find_if(id_maps.cbegin(), id_maps.cend(), [id](std::pair p) { return id == p.second; }); + auto default_found = std::find_if(id_maps.cbegin(), id_maps.cend(), [rev_id_if_not_found](std::pair p) { + return rev_id_if_not_found == p.second; + }); - return found == id_maps.cend() ? rev_id_if_not_found : found->first; + return found == id_maps.cend() ? (default_found == id_maps.cend() ? rev_id_if_not_found : default_found->first) + : found->first; } } // namespace From 8d5ba4a51476749d4b334f920c73f13b8634e61b Mon Sep 17 00:00:00 2001 From: sharder996 Date: Thu, 12 Oct 2023 10:59:06 -0500 Subject: [PATCH 05/14] [sftp] rename parameter to more accurately represent usage --- src/sshfs_mount/sftp_server.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index 3cbe2f63b7..8b7468ef3d 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -270,14 +270,14 @@ int mapped_id_for(const mp::id_mappings& id_maps, const int id, const int id_if_ return id; } -int reverse_id_for(const mp::id_mappings& id_maps, const int id, const int rev_id_if_not_found) +int reverse_id_for(const mp::id_mappings& id_maps, const int id, const int default_id) { auto found = std::find_if(id_maps.cbegin(), id_maps.cend(), [id](std::pair p) { return id == p.second; }); - auto default_found = std::find_if(id_maps.cbegin(), id_maps.cend(), [rev_id_if_not_found](std::pair p) { - return rev_id_if_not_found == p.second; + auto default_found = std::find_if(id_maps.cbegin(), id_maps.cend(), [default_id](std::pair p) { + return default_id == p.second; }); - return found == id_maps.cend() ? (default_found == id_maps.cend() ? rev_id_if_not_found : default_found->first) + return found == id_maps.cend() ? (default_found == id_maps.cend() ? default_id : default_found->first) : found->first; } } // namespace @@ -339,14 +339,14 @@ inline int mp::SftpServer::mapped_gid_for(const int gid) return mapped_id_for(gid_mappings, gid, default_gid); } -inline int mp::SftpServer::reverse_uid_for(const int uid, const int rev_uid_if_not_found) +inline int mp::SftpServer::reverse_uid_for(const int uid, const int default_id) { - return reverse_id_for(uid_mappings, uid, rev_uid_if_not_found); + return reverse_id_for(uid_mappings, uid, default_id); } -inline int mp::SftpServer::reverse_gid_for(const int gid, const int rev_gid_if_not_found) +inline int mp::SftpServer::reverse_gid_for(const int gid, const int default_id) { - return reverse_id_for(gid_mappings, gid, rev_gid_if_not_found); + return reverse_id_for(gid_mappings, gid, default_id); } void mp::SftpServer::process_message(sftp_client_message msg) From 12eaf5a2706c05db03e6d64e21a29e19a5e7e991 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Thu, 2 Nov 2023 08:35:54 -0500 Subject: [PATCH 06/14] [sftp server] block access to files owned by root with mapping --- src/sshfs_mount/sftp_server.cpp | 46 ++++++++++++++++++++++++++++++++- src/sshfs_mount/sftp_server.h | 2 ++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index 8b7468ef3d..d65f6771ec 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -270,6 +270,13 @@ int mapped_id_for(const mp::id_mappings& id_maps, const int id, const int id_if_ return id; } +int reverse_id_for(const mp::id_mappings& id_maps, const int id) +{ + auto found = std::find_if(id_maps.cbegin(), id_maps.cend(), [id](std::pair p) { return id == p.second; }); + + return found == id_maps.cend() ? -1 : found->first; +} + int reverse_id_for(const mp::id_mappings& id_maps, const int id, const int default_id) { auto found = std::find_if(id_maps.cbegin(), id_maps.cend(), [id](std::pair p) { return id == p.second; }); @@ -349,6 +356,16 @@ inline int mp::SftpServer::reverse_gid_for(const int gid, const int default_id) return reverse_id_for(gid_mappings, gid, default_id); } +inline int mp::SftpServer::reverse_uid_for(const int uid) +{ + return reverse_id_for(uid_mappings, uid); +} + +inline int mp::SftpServer::reverse_gid_for(const int gid) +{ + return reverse_id_for(uid_mappings, gid); +} + void mp::SftpServer::process_message(sftp_client_message msg) { int ret = 0; @@ -594,8 +611,35 @@ int mp::SftpServer::handle_open(sftp_client_message msg) mode |= QIODevice::Truncate; auto file = std::make_unique(filename); + QFileInfo file_info(filename); - auto exists = QFileInfo(filename).isSymLink() || file->exists(); + auto exists = file_info.isSymLink() || file->exists(); + + if (exists) + { + sftp_attributes_struct attr{}; + + if (file_info.isSymLink()) + { + mp::platform::symlink_attr_from(filename, &attr); + attr.uid = mapped_uid_for(attr.uid); + attr.gid = mapped_gid_for(attr.gid); + } + else + { + attr = attr_from(file_info); + } + + if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: permission denied: cannot access path \'{}\' without mapping for root", + __FUNCTION__, + filename)); + return reply_perm_denied(msg); + } + } if (!MP_FILEOPS.open(*file, mode)) { diff --git a/src/sshfs_mount/sftp_server.h b/src/sshfs_mount/sftp_server.h index ba029a9ef6..cef6e76452 100644 --- a/src/sshfs_mount/sftp_server.h +++ b/src/sshfs_mount/sftp_server.h @@ -57,6 +57,8 @@ class SftpServer int mapped_gid_for(const int gid); int reverse_uid_for(const int uid, const int rev_uid_if_not_found); int reverse_gid_for(const int gid, const int rev_gid_if_not_found); + int reverse_uid_for(const int uid); + int reverse_gid_for(const int gid); int handle_close(sftp_client_message msg); int handle_fstat(sftp_client_message msg); From 53548f585ac761cf6014da7d6096d85b5bc04eac Mon Sep 17 00:00:00 2001 From: sharder996 Date: Thu, 2 Nov 2023 09:06:35 -0500 Subject: [PATCH 07/14] [sftp server] rename parameters --- src/sshfs_mount/sftp_server.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index d65f6771ec..3c3c2b44f3 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -346,14 +346,14 @@ inline int mp::SftpServer::mapped_gid_for(const int gid) return mapped_id_for(gid_mappings, gid, default_gid); } -inline int mp::SftpServer::reverse_uid_for(const int uid, const int default_id) +inline int mp::SftpServer::reverse_uid_for(const int uid, const int rev_uid_if_not_found) { - return reverse_id_for(uid_mappings, uid, default_id); + return reverse_id_for(uid_mappings, uid, rev_uid_if_not_found); } -inline int mp::SftpServer::reverse_gid_for(const int gid, const int default_id) +inline int mp::SftpServer::reverse_gid_for(const int gid, const int rev_gid_if_not_found) { - return reverse_id_for(gid_mappings, gid, default_id); + return reverse_id_for(gid_mappings, gid, rev_gid_if_not_found); } inline int mp::SftpServer::reverse_uid_for(const int uid) From 396d2170b9075bd721fc4a506dafa5b6daad9211 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Thu, 2 Nov 2023 09:40:02 -0500 Subject: [PATCH 08/14] [sftp server] restrict root in the instance from changing ownership --- src/sshfs_mount/sftp_server.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index 3c3c2b44f3..5d6868e9b3 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -935,6 +935,17 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) } } + if ((msg->attr->uid == 0 && reverse_uid_for(msg->attr->uid) == -1) || + (msg->attr->gid == 0 && reverse_gid_for(msg->attr->gid) == -1)) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: permission denied: cannot modify path \'{}\' without reverse mapping for root", + __FUNCTION__, + filename)); + return reply_perm_denied(msg); + } + QFile file{filename}; if (msg->attr->flags & SSH_FILEXFER_ATTR_SIZE) From bab17f8e0d5dabe8734257bb2d1de6c41946c778 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Thu, 2 Nov 2023 19:40:06 -0500 Subject: [PATCH 09/14] [sftp server] restrict guest from chowning to root --- src/sshfs_mount/sftp_server.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index 5d6868e9b3..ff09f656ea 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -935,8 +935,22 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) } } - if ((msg->attr->uid == 0 && reverse_uid_for(msg->attr->uid) == -1) || - (msg->attr->gid == 0 && reverse_gid_for(msg->attr->gid) == -1)) + QFile file{filename}; + QFileInfo file_info(filename); + sftp_attributes_struct attr{}; + + if (file_info.isSymLink()) + { + mp::platform::symlink_attr_from(filename.toStdString().c_str(), &attr); + attr.uid = mapped_uid_for(attr.uid); + attr.gid = mapped_gid_for(attr.gid); + } + else + { + attr = attr_from(file_info); + } + + if ((attr.uid != 0 && msg->attr->uid == 0) || (attr.gid != 0 && msg->attr->gid == 0)) { mpl::log(mpl::Level::trace, category, @@ -946,8 +960,6 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) return reply_perm_denied(msg); } - QFile file{filename}; - if (msg->attr->flags & SSH_FILEXFER_ATTR_SIZE) { if (!MP_FILEOPS.resize(file, msg->attr->size)) From 70f28b497e66e57897287e433ee4d5a6e81c7051 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Thu, 2 Nov 2023 19:41:11 -0500 Subject: [PATCH 10/14] [sftp server] restrict access to host files owned by root --- src/sshfs_mount/sftp_server.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index ff09f656ea..e1a3c75ac6 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -1036,6 +1036,16 @@ int mp::SftpServer::handle_stat(sftp_client_message msg, const bool follow) attr = attr_from(file_info); } + if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: permission denied: cannot access path \'{}\' without mapping for root", + __FUNCTION__, + filename)); + return reply_perm_denied(msg); + } + return sftp_reply_attr(msg, &attr); } From eb08c90bf37a5ee7bd1c20eb4929ea2b710e0fd6 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Tue, 7 Nov 2023 11:03:00 +0200 Subject: [PATCH 11/14] [sftp server] block instance from modifying root-owned file attributes without reverse mapping --- src/sshfs_mount/sftp_server.cpp | 71 +++++++++++++++++++++++++-------- 1 file changed, 55 insertions(+), 16 deletions(-) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index e1a3c75ac6..d12d307870 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -950,18 +950,18 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) attr = attr_from(file_info); } - if ((attr.uid != 0 && msg->attr->uid == 0) || (attr.gid != 0 && msg->attr->gid == 0)) - { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: permission denied: cannot modify path \'{}\' without reverse mapping for root", - __FUNCTION__, - filename)); - return reply_perm_denied(msg); - } - if (msg->attr->flags & SSH_FILEXFER_ATTR_SIZE) { + if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot resize \'{}\' without reverse mapping for root: permission denied", + __FUNCTION__, + filename)); + return reply_perm_denied(msg); + } + if (!MP_FILEOPS.resize(file, msg->attr->size)) { mpl::log(mpl::Level::trace, category, fmt::format("{}: cannot resize \'{}\'", __FUNCTION__, filename)); @@ -971,6 +971,17 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) if (msg->attr->flags & SSH_FILEXFER_ATTR_PERMISSIONS) { + if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot set permissions for \'{}\' without reverse mapping for root: permission denied", + __FUNCTION__, + filename)); + return reply_perm_denied(msg); + } + if (!MP_FILEOPS.setPermissions(file, to_qt_permissions(msg->attr->permissions))) { mpl::log(mpl::Level::trace, category, @@ -981,6 +992,18 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) if (msg->attr->flags & SSH_FILEXFER_ATTR_ACMODTIME) { + if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format( + "{}: cannot set modification date for \'{}\' without reverse mapping for root: permission denied", + __FUNCTION__, + filename)); + return reply_perm_denied(msg); + } + if (MP_PLATFORM.utime(filename.toStdString().c_str(), msg->attr->atime, msg->attr->mtime) < 0) { mpl::log(mpl::Level::trace, category, @@ -989,13 +1012,29 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) } } - if ((msg->attr->flags & SSH_FILEXFER_ATTR_UIDGID) && - (MP_PLATFORM.chown(filename.toStdString().c_str(), reverse_uid_for(msg->attr->uid, msg->attr->uid), - reverse_gid_for(msg->attr->gid, msg->attr->gid)) < 0)) + if (msg->attr->flags & SSH_FILEXFER_ATTR_UIDGID) { - mpl::log(mpl::Level::trace, category, - fmt::format("{}: cannot set ownership for \'{}\'", __FUNCTION__, filename)); - return reply_failure(msg); + if ((msg->attr->uid == 0 && reverse_uid_for(attr.uid) == -1) || + (msg->attr->gid == 0 && reverse_gid_for(attr.gid) == -1)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: permission denied: cannot set ownership for \'{}\' without reverse mapping for root", + __FUNCTION__, + filename)); + return reply_perm_denied(msg); + } + + if (MP_PLATFORM.chown(filename.toStdString().c_str(), + reverse_uid_for(msg->attr->uid, msg->attr->uid), + reverse_gid_for(msg->attr->gid, msg->attr->gid)) < 0) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot set ownership for \'{}\'", __FUNCTION__, filename)); + return reply_failure(msg); + } } return reply_ok(msg); From a58a9c831d847a0b1cf85b909b7b5bde055d34c1 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Thu, 9 Nov 2023 16:09:48 +0200 Subject: [PATCH 12/14] [sftp server] check for valid permissions on all sftp messages - factor out common condition checks --- src/sshfs_mount/sftp_server.cpp | 322 +++++++++++--------------------- src/sshfs_mount/sftp_server.h | 7 +- 2 files changed, 117 insertions(+), 212 deletions(-) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index d12d307870..314e66a58e 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -211,14 +211,6 @@ auto to_unix_permissions(QFile::Permissions perms) return out; } -auto validate_path(const std::string& source_path, const std::string& current_path) -{ - if (source_path.empty()) - return false; - - return current_path.compare(0, source_path.length(), source_path) == 0; -} - template auto handle_from(sftp_client_message msg, const std::unordered_map>& handles) -> T* { @@ -270,13 +262,6 @@ int mapped_id_for(const mp::id_mappings& id_maps, const int id, const int id_if_ return id; } -int reverse_id_for(const mp::id_mappings& id_maps, const int id) -{ - auto found = std::find_if(id_maps.cbegin(), id_maps.cend(), [id](std::pair p) { return id == p.second; }); - - return found == id_maps.cend() ? -1 : found->first; -} - int reverse_id_for(const mp::id_mappings& id_maps, const int id, const int default_id) { auto found = std::find_if(id_maps.cbegin(), id_maps.cend(), [id](std::pair p) { return id == p.second; }); @@ -311,6 +296,23 @@ mp::SftpServer::~SftpServer() stop_invoked = true; } +sftp_attributes_struct mp::SftpServer::get_uid_gid_from(const QFileInfo& file_info) +{ + sftp_attributes_struct attr{}; + + if (file_info.isSymLink()) + { + mp::platform::symlink_attr_from(file_info.fileName().toStdString().c_str(), &attr); + } + else + { + attr.uid = file_info.ownerId(); + attr.gid = file_info.groupId(); + } + + return attr; +} + sftp_attributes_struct mp::SftpServer::attr_from(const QFileInfo& file_info) { sftp_attributes_struct attr{}; @@ -356,14 +358,63 @@ inline int mp::SftpServer::reverse_gid_for(const int gid, const int rev_gid_if_n return reverse_id_for(gid_mappings, gid, rev_gid_if_not_found); } -inline int mp::SftpServer::reverse_uid_for(const int uid) +inline bool mp::SftpServer::has_uid_mapping_for(const int uid) +{ + return std::find_if(uid_mappings.begin(), uid_mappings.end(), [uid](std::pair p) { + return uid == p.first; + }) != uid_mappings.end(); +} + +inline bool mp::SftpServer::has_gid_mapping_for(const int gid) { - return reverse_id_for(uid_mappings, uid); + return std::find_if(gid_mappings.begin(), gid_mappings.end(), [gid](std::pair p) { + return gid == p.first; + }) != gid_mappings.end(); } -inline int mp::SftpServer::reverse_gid_for(const int gid) +bool mp::SftpServer::validate_path(const std::string& source_path, const std::string& current_path) { - return reverse_id_for(uid_mappings, gid); + auto valid = !source_path.empty() && current_path.compare(0, source_path.length(), source_path) == 0; + + if (!valid) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", + __FUNCTION__, + current_path, + source_path)); + } + + return valid; +} + +bool mp::SftpServer::validate_permissions(const QString& filename) +{ + QFileInfo file_info(filename); + auto exists = file_info.isSymLink() || file_info.exists(); + sftp_attributes_struct attr{}; + + if (file_info.isSymLink()) + { + mp::platform::symlink_attr_from(filename.toStdString().c_str(), &attr); + } + else + { + attr.uid = file_info.ownerId(); + attr.gid = file_info.groupId(); + } + + auto can_access = exists && has_uid_mapping_for(attr.uid) && has_gid_mapping_for(attr.gid); + if (!can_access) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + } + + return can_access; } void mp::SftpServer::process_message(sftp_client_message msg) @@ -527,13 +578,8 @@ int mp::SftpServer::handle_fstat(sftp_client_message msg) int mp::SftpServer::handle_mkdir(sftp_client_message msg) { const auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + if (!validate_path(source_path, filename) || !validate_permissions(filename)) return reply_perm_denied(msg); - } QDir dir(filename); if (!dir.mkdir(filename)) @@ -567,13 +613,8 @@ int mp::SftpServer::handle_mkdir(sftp_client_message msg) int mp::SftpServer::handle_rmdir(sftp_client_message msg) { const auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + if (!validate_path(source_path, filename) || !validate_permissions(filename)) return reply_perm_denied(msg); - } QDir dir(filename); if (!MP_FILEOPS.rmdir(dir, filename)) @@ -588,13 +629,9 @@ int mp::SftpServer::handle_rmdir(sftp_client_message msg) int mp::SftpServer::handle_open(sftp_client_message msg) { const auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + + if (!validate_path(source_path, filename) || !validate_permissions(filename)) return reply_perm_denied(msg); - } QIODevice::OpenMode mode{QIODevice::NotOpen}; const auto flags = sftp_client_message_get_flags(msg); @@ -612,35 +649,8 @@ int mp::SftpServer::handle_open(sftp_client_message msg) auto file = std::make_unique(filename); QFileInfo file_info(filename); - auto exists = file_info.isSymLink() || file->exists(); - if (exists) - { - sftp_attributes_struct attr{}; - - if (file_info.isSymLink()) - { - mp::platform::symlink_attr_from(filename, &attr); - attr.uid = mapped_uid_for(attr.uid); - attr.gid = mapped_gid_for(attr.gid); - } - else - { - attr = attr_from(file_info); - } - - if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) - { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: permission denied: cannot access path \'{}\' without mapping for root", - __FUNCTION__, - filename)); - return reply_perm_denied(msg); - } - } - if (!MP_FILEOPS.open(*file, mode)) { mpl::log(mpl::Level::trace, category, fmt::format("Cannot open \'{}\': {}", filename, file->errorString())); @@ -685,13 +695,8 @@ int mp::SftpServer::handle_open(sftp_client_message msg) int mp::SftpServer::handle_opendir(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + if (!validate_path(source_path, filename) || !validate_permissions(filename)) return reply_perm_denied(msg); - } QDir dir(filename); if (!dir.exists()) @@ -796,13 +801,8 @@ int mp::SftpServer::handle_readdir(sftp_client_message msg) int mp::SftpServer::handle_readlink(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + if (!validate_path(source_path, filename) || !validate_permissions(filename)) return reply_perm_denied(msg); - } auto link = QFile::symLinkTarget(filename); if (link.isEmpty()) @@ -819,13 +819,8 @@ int mp::SftpServer::handle_readlink(sftp_client_message msg) int mp::SftpServer::handle_realpath(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + if (!validate_path(source_path, filename) || !validate_permissions(filename)) return reply_perm_denied(msg); - } auto realpath = QFileInfo(filename).absoluteFilePath(); return sftp_reply_name(msg, realpath.toStdString().c_str(), nullptr); @@ -834,13 +829,8 @@ int mp::SftpServer::handle_realpath(sftp_client_message msg) int mp::SftpServer::handle_remove(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + if (!validate_path(source_path, filename) || !validate_permissions(filename)) return reply_perm_denied(msg); - } QFile file{filename}; if (!MP_FILEOPS.remove(file)) @@ -855,13 +845,8 @@ int mp::SftpServer::handle_remove(sftp_client_message msg) int mp::SftpServer::handle_rename(sftp_client_message msg) { const auto source = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, source)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, source, source_path)); + if (!validate_path(source_path, source) || !validate_permissions(source)) return reply_perm_denied(msg); - } if (!QFileInfo(source).isSymLink() && !QFile::exists(source)) { @@ -871,13 +856,8 @@ int mp::SftpServer::handle_rename(sftp_client_message msg) } const auto target = sftp_client_message_get_data(msg); - if (!validate_path(source_path, target)) - { - mpl::log(mpl::Level::trace, category, - fmt::format("{}: cannot validate target path \'{}\' against source \'{}\'", __FUNCTION__, target, - source_path)); + if (!validate_path(source_path, target) || !validate_permissions(target)) return reply_perm_denied(msg); - } QFile target_file{target}; if (target_file.exists()) @@ -920,12 +900,7 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) { filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename.toStdString())) - { - mpl::log(mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, - source_path)); return reply_perm_denied(msg); - } if (!QFileInfo(filename).isSymLink() && !QFile::exists(filename)) { @@ -935,6 +910,9 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) } } + if (!validate_permissions(filename)) + return reply_perm_denied(msg); + QFile file{filename}; QFileInfo file_info(filename); sftp_attributes_struct attr{}; @@ -950,91 +928,39 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) attr = attr_from(file_info); } - if (msg->attr->flags & SSH_FILEXFER_ATTR_SIZE) + if (msg->attr->flags & SSH_FILEXFER_ATTR_SIZE && !MP_FILEOPS.resize(file, msg->attr->size)) { - if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) - { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: cannot resize \'{}\' without reverse mapping for root: permission denied", - __FUNCTION__, - filename)); - return reply_perm_denied(msg); - } - - if (!MP_FILEOPS.resize(file, msg->attr->size)) - { - mpl::log(mpl::Level::trace, category, fmt::format("{}: cannot resize \'{}\'", __FUNCTION__, filename)); - return reply_failure(msg); - } + mpl::log(mpl::Level::trace, category, fmt::format("{}: cannot resize \'{}\'", __FUNCTION__, filename)); + return reply_failure(msg); } - if (msg->attr->flags & SSH_FILEXFER_ATTR_PERMISSIONS) + if (msg->attr->flags & SSH_FILEXFER_ATTR_PERMISSIONS && + !MP_FILEOPS.setPermissions(file, to_qt_permissions(msg->attr->permissions))) { - if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) - { - mpl::log( - mpl::Level::trace, - category, - fmt::format("{}: cannot set permissions for \'{}\' without reverse mapping for root: permission denied", - __FUNCTION__, - filename)); - return reply_perm_denied(msg); - } - - if (!MP_FILEOPS.setPermissions(file, to_qt_permissions(msg->attr->permissions))) - { - mpl::log(mpl::Level::trace, category, - fmt::format("{}: set permissions failed for \'{}\'", __FUNCTION__, filename)); - return reply_failure(msg); - } + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: set permissions failed for \'{}\'", __FUNCTION__, filename)); + return reply_failure(msg); } - if (msg->attr->flags & SSH_FILEXFER_ATTR_ACMODTIME) + if (msg->attr->flags & SSH_FILEXFER_ATTR_ACMODTIME && + MP_PLATFORM.utime(filename.toStdString().c_str(), msg->attr->atime, msg->attr->mtime) < 0) { - if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) - { - mpl::log( - mpl::Level::trace, - category, - fmt::format( - "{}: cannot set modification date for \'{}\' without reverse mapping for root: permission denied", - __FUNCTION__, - filename)); - return reply_perm_denied(msg); - } - - if (MP_PLATFORM.utime(filename.toStdString().c_str(), msg->attr->atime, msg->attr->mtime) < 0) - { - mpl::log(mpl::Level::trace, category, - fmt::format("{}: cannot set modification date for \'{}\'", __FUNCTION__, filename)); - return reply_failure(msg); - } + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot set modification date for \'{}\'", __FUNCTION__, filename)); + return reply_failure(msg); } - if (msg->attr->flags & SSH_FILEXFER_ATTR_UIDGID) + if (msg->attr->flags & SSH_FILEXFER_ATTR_UIDGID && + MP_PLATFORM.chown(filename.toStdString().c_str(), + reverse_uid_for(msg->attr->uid, msg->attr->uid), + reverse_gid_for(msg->attr->gid, msg->attr->gid)) < 0) { - if ((msg->attr->uid == 0 && reverse_uid_for(attr.uid) == -1) || - (msg->attr->gid == 0 && reverse_gid_for(attr.gid) == -1)) - { - mpl::log( - mpl::Level::trace, - category, - fmt::format("{}: permission denied: cannot set ownership for \'{}\' without reverse mapping for root", - __FUNCTION__, - filename)); - return reply_perm_denied(msg); - } - - if (MP_PLATFORM.chown(filename.toStdString().c_str(), - reverse_uid_for(msg->attr->uid, msg->attr->uid), - reverse_gid_for(msg->attr->gid, msg->attr->gid)) < 0) - { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: cannot set ownership for \'{}\'", __FUNCTION__, filename)); - return reply_failure(msg); - } + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot set ownership for \'{}\'", __FUNCTION__, filename)); + return reply_failure(msg); } return reply_ok(msg); @@ -1043,13 +969,9 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) int mp::SftpServer::handle_stat(sftp_client_message msg, const bool follow) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + + if (!validate_path(source_path, filename) || !validate_permissions(filename)) return reply_perm_denied(msg); - } QFileInfo file_info(filename); if (!file_info.isSymLink() && !file_info.exists()) @@ -1075,31 +997,16 @@ int mp::SftpServer::handle_stat(sftp_client_message msg, const bool follow) attr = attr_from(file_info); } - if ((attr.uid == 0 && reverse_uid_for(attr.uid) == -1) || (attr.gid == 0 && reverse_gid_for(attr.gid) == -1)) - { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: permission denied: cannot access path \'{}\' without mapping for root", - __FUNCTION__, - filename)); - return reply_perm_denied(msg); - } - return sftp_reply_attr(msg, &attr); } int mp::SftpServer::handle_symlink(sftp_client_message msg) { const auto old_name = sftp_client_message_get_filename(msg); - const auto new_name = sftp_client_message_get_data(msg); - if (!validate_path(source_path, new_name)) - { - mpl::log( - mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, new_name, source_path)); + + if (!validate_path(source_path, new_name) || !validate_permissions(new_name)) return reply_perm_denied(msg); - } if (!MP_PLATFORM.symlink(old_name, new_name, QFileInfo(old_name).isDir())) { @@ -1162,15 +1069,10 @@ int mp::SftpServer::handle_extended(sftp_client_message msg) if (method == "hardlink@openssh.com") { const auto old_name = sftp_client_message_get_filename(msg); - const auto new_name = sftp_client_message_get_data(msg); - if (!validate_path(source_path, new_name)) - { - mpl::log(mpl::Level::trace, category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, new_name, - source_path)); + + if (!validate_path(source_path, new_name) || !validate_permissions(new_name)) return reply_perm_denied(msg); - } if (!MP_PLATFORM.link(old_name, new_name)) { diff --git a/src/sshfs_mount/sftp_server.h b/src/sshfs_mount/sftp_server.h index cef6e76452..0c00ad94da 100644 --- a/src/sshfs_mount/sftp_server.h +++ b/src/sshfs_mount/sftp_server.h @@ -52,13 +52,16 @@ class SftpServer private: void process_message(sftp_client_message msg); + sftp_attributes_struct get_uid_gid_from(const QFileInfo& file_info); sftp_attributes_struct attr_from(const QFileInfo& file_info); int mapped_uid_for(const int uid); int mapped_gid_for(const int gid); int reverse_uid_for(const int uid, const int rev_uid_if_not_found); int reverse_gid_for(const int gid, const int rev_gid_if_not_found); - int reverse_uid_for(const int uid); - int reverse_gid_for(const int gid); + bool has_uid_mapping_for(const int uid); + bool has_gid_mapping_for(const int gid); + bool validate_permissions(const QString& filename); + bool validate_path(const std::string& source_path, const std::string& current_path); int handle_close(sftp_client_message msg); int handle_fstat(sftp_client_message msg); From 6ba7e42103f5f357a297221a70ae9072939c111b Mon Sep 17 00:00:00 2001 From: sharder996 Date: Fri, 10 Nov 2023 13:35:18 +0200 Subject: [PATCH 13/14] [sftp server] working changes --- src/sshfs_mount/sftp_server.cpp | 99 ++++++++++++++++++++++++++++++--- 1 file changed, 91 insertions(+), 8 deletions(-) diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index 314e66a58e..ca875b9760 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -406,13 +406,13 @@ bool mp::SftpServer::validate_permissions(const QString& filename) } auto can_access = exists && has_uid_mapping_for(attr.uid) && has_gid_mapping_for(attr.gid); - if (!can_access) - { - mpl::log( - mpl::Level::trace, - category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); - } + // if (!can_access) + // { + // mpl::log( + // mpl::Level::trace, + // category, + // fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + // } return can_access; } @@ -559,6 +559,7 @@ int mp::SftpServer::handle_close(sftp_client_message msg) int mp::SftpServer::handle_fstat(sftp_client_message msg) { + auto file = handle_from(msg, open_file_handles); if (file == nullptr) { @@ -566,6 +567,10 @@ int mp::SftpServer::handle_fstat(sftp_client_message msg) return reply_bad_handle(msg, "fstat"); } + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: attempting to access path \'{}\'", __FUNCTION__, file->fileName())); + QFileInfo file_info(*file); if (file_info.isSymLink()) @@ -579,7 +584,13 @@ int mp::SftpServer::handle_mkdir(sftp_client_message msg) { const auto filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename) || !validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); + } QDir dir(filename); if (!dir.mkdir(filename)) @@ -614,7 +625,13 @@ int mp::SftpServer::handle_rmdir(sftp_client_message msg) { const auto filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename) || !validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); + } QDir dir(filename); if (!MP_FILEOPS.rmdir(dir, filename)) @@ -631,7 +648,13 @@ int mp::SftpServer::handle_open(sftp_client_message msg) const auto filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename) || !validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); + } QIODevice::OpenMode mode{QIODevice::NotOpen}; const auto flags = sftp_client_message_get_flags(msg); @@ -696,7 +719,13 @@ int mp::SftpServer::handle_opendir(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename) || !validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); + } QDir dir(filename); if (!dir.exists()) @@ -802,7 +831,13 @@ int mp::SftpServer::handle_readlink(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename) || !validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); + } auto link = QFile::symLinkTarget(filename); if (link.isEmpty()) @@ -820,7 +855,13 @@ int mp::SftpServer::handle_realpath(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename) || !validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); + } auto realpath = QFileInfo(filename).absoluteFilePath(); return sftp_reply_name(msg, realpath.toStdString().c_str(), nullptr); @@ -830,7 +871,13 @@ int mp::SftpServer::handle_remove(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename) || !validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); + } QFile file{filename}; if (!MP_FILEOPS.remove(file)) @@ -846,7 +893,12 @@ int mp::SftpServer::handle_rename(sftp_client_message msg) { const auto source = sftp_client_message_get_filename(msg); if (!validate_path(source_path, source) || !validate_permissions(source)) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, source)); return reply_perm_denied(msg); + } if (!QFileInfo(source).isSymLink() && !QFile::exists(source)) { @@ -857,7 +909,12 @@ int mp::SftpServer::handle_rename(sftp_client_message msg) const auto target = sftp_client_message_get_data(msg); if (!validate_path(source_path, target) || !validate_permissions(target)) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, target)); return reply_perm_denied(msg); + } QFile target_file{target}; if (target_file.exists()) @@ -911,8 +968,13 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) } if (!validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); - + } QFile file{filename}; QFileInfo file_info(filename); sftp_attributes_struct attr{}; @@ -970,8 +1032,16 @@ int mp::SftpServer::handle_stat(sftp_client_message msg, const bool follow) { auto filename = sftp_client_message_get_filename(msg); + mpl::log(mpl::Level::trace, category, fmt::format("{}: attempting to access path \'{}\'", __FUNCTION__, filename)); + if (!validate_path(source_path, filename) || !validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); + } QFileInfo file_info(filename); if (!file_info.isSymLink() && !file_info.exists()) @@ -1006,7 +1076,13 @@ int mp::SftpServer::handle_symlink(sftp_client_message msg) const auto new_name = sftp_client_message_get_data(msg); if (!validate_path(source_path, new_name) || !validate_permissions(new_name)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, new_name)); return reply_perm_denied(msg); + } if (!MP_PLATFORM.symlink(old_name, new_name, QFileInfo(old_name).isDir())) { @@ -1072,7 +1148,14 @@ int mp::SftpServer::handle_extended(sftp_client_message msg) const auto new_name = sftp_client_message_get_data(msg); if (!validate_path(source_path, new_name) || !validate_permissions(new_name)) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", + __FUNCTION__, + new_name)); return reply_perm_denied(msg); + } if (!MP_PLATFORM.link(old_name, new_name)) { From 0f478b83dc7e39e3062c6fe8cbebadaf1a4154f8 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Tue, 28 Nov 2023 09:23:48 -0600 Subject: [PATCH 14/14] working changes --- include/multipass/file_ops.h | 2 + src/sshfs_mount/sftp_server.cpp | 295 ++++++++++++------ src/sshfs_mount/sftp_server.h | 3 +- src/sshfs_mount/sshfs_mount.cpp | 2 +- src/utils/file_ops.cpp | 10 + tests/mock_file_ops.h | 2 + tests/test_sftpserver.cpp | 512 +++++++++++++++++++++++++++++--- tests/test_sshfsmount.cpp | 2 +- 8 files changed, 694 insertions(+), 134 deletions(-) diff --git a/include/multipass/file_ops.h b/include/multipass/file_ops.h index ced9e1f5f1..3e852245b5 100644 --- a/include/multipass/file_ops.h +++ b/include/multipass/file_ops.h @@ -57,6 +57,8 @@ class FileOps : public Singleton virtual bool exists(const QFileInfo& file) const; virtual bool isDir(const QFileInfo& file) const; virtual bool isReadable(const QFileInfo& file) const; + virtual uint ownerId(const QFileInfo& file) const; + virtual uint groupId(const QFileInfo& file) const; // QFile operations virtual bool exists(const QFile& file) const; diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index ca875b9760..d24222677a 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -296,23 +296,6 @@ mp::SftpServer::~SftpServer() stop_invoked = true; } -sftp_attributes_struct mp::SftpServer::get_uid_gid_from(const QFileInfo& file_info) -{ - sftp_attributes_struct attr{}; - - if (file_info.isSymLink()) - { - mp::platform::symlink_attr_from(file_info.fileName().toStdString().c_str(), &attr); - } - else - { - attr.uid = file_info.ownerId(); - attr.gid = file_info.groupId(); - } - - return attr; -} - sftp_attributes_struct mp::SftpServer::attr_from(const QFileInfo& file_info) { sftp_attributes_struct attr{}; @@ -372,27 +355,28 @@ inline bool mp::SftpServer::has_gid_mapping_for(const int gid) }) != gid_mappings.end(); } -bool mp::SftpServer::validate_path(const std::string& source_path, const std::string& current_path) +inline bool mp::SftpServer::has_reverse_uid_mapping_for(const int uid) { - auto valid = !source_path.empty() && current_path.compare(0, source_path.length(), source_path) == 0; + return std::find_if(uid_mappings.cbegin(), uid_mappings.cend(), [uid](std::pair p) { + return uid == p.second; + }) != uid_mappings.end(); +} - if (!valid) - { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", - __FUNCTION__, - current_path, - source_path)); - } +inline bool mp::SftpServer::has_reverse_gid_mapping_for(const int gid) +{ + return std::find_if(gid_mappings.cbegin(), gid_mappings.cend(), [gid](std::pair p) { + return gid == p.second; + }) != gid_mappings.end(); +} - return valid; +bool mp::SftpServer::validate_path(const std::string& source_path, const std::string& current_path) +{ + return !source_path.empty() && current_path.compare(0, source_path.length(), source_path) == 0; } bool mp::SftpServer::validate_permissions(const QString& filename) { QFileInfo file_info(filename); - auto exists = file_info.isSymLink() || file_info.exists(); sftp_attributes_struct attr{}; if (file_info.isSymLink()) @@ -401,20 +385,11 @@ bool mp::SftpServer::validate_permissions(const QString& filename) } else { - attr.uid = file_info.ownerId(); - attr.gid = file_info.groupId(); + attr.uid = MP_FILEOPS.ownerId(file_info); + attr.gid = MP_FILEOPS.groupId(file_info); } - auto can_access = exists && has_uid_mapping_for(attr.uid) && has_gid_mapping_for(attr.gid); - // if (!can_access) - // { - // mpl::log( - // mpl::Level::trace, - // category, - // fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); - // } - - return can_access; + return has_uid_mapping_for(attr.uid) && has_gid_mapping_for(attr.gid); } void mp::SftpServer::process_message(sftp_client_message msg) @@ -583,16 +558,33 @@ int mp::SftpServer::handle_fstat(sftp_client_message msg) int mp::SftpServer::handle_mkdir(sftp_client_message msg) { const auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename) || !validate_permissions(filename)) + if (!validate_path(source_path, filename)) { mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); return reply_perm_denied(msg); } QDir dir(filename); + QDir parent_dir{dir}; + parent_dir.cdUp(); + QFileInfo parent_file_info{parent_dir.path()}; + + if (!has_uid_mapping_for(MP_FILEOPS.ownerId(parent_file_info)) && + !has_gid_mapping_for(MP_FILEOPS.groupId(parent_file_info))) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot create path \'{}\' with permissions \'{}:{}\': permission denied", + __FUNCTION__, + parent_file_info.ownerId(), + parent_file_info.groupId(), + filename)); + return reply_perm_denied(msg); + } + if (!dir.mkdir(filename)) { mpl::log(mpl::Level::trace, category, fmt::format("{}: mkdir failed for \'{}\'", __FUNCTION__, filename)); @@ -604,18 +596,19 @@ int mp::SftpServer::handle_mkdir(sftp_client_message msg) { mpl::log(mpl::Level::trace, category, fmt::format("{}: set permissions failed for \'{}\'", __FUNCTION__, filename)); + dir.rmpath(filename); return reply_failure(msg); } - QFileInfo current_dir(filename); - QFileInfo parent_dir(current_dir.path()); - int rev_uid = reverse_uid_for(msg->attr->uid, parent_dir.ownerId()); - int rev_gid = reverse_gid_for(msg->attr->gid, parent_dir.groupId()); - - if (MP_PLATFORM.chown(filename, rev_uid, rev_gid) < 0) + if (MP_PLATFORM.chown(filename, parent_file_info.ownerId(), parent_file_info.groupId()) < 0) { - mpl::log(mpl::Level::trace, category, - fmt::format("failed to chown '{}' to owner:{} and group:{}", filename, rev_uid, rev_gid)); + mpl::log(mpl::Level::trace, + category, + fmt::format("failed to chown '{}' to owner:{} and group:{}", + filename, + parent_file_info.ownerId(), + parent_file_info.groupId())); + dir.rmpath(filename); return reply_failure(msg); } return reply_ok(msg); @@ -624,16 +617,28 @@ int mp::SftpServer::handle_mkdir(sftp_client_message msg) int mp::SftpServer::handle_rmdir(sftp_client_message msg) { const auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename) || !validate_permissions(filename)) + if (!validate_path(source_path, filename)) { mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); return reply_perm_denied(msg); } QDir dir(filename); + QFileInfo file_info(filename); + auto exists = file_info.isSymLink() || dir.exists(); + + if ((exists && !validate_permissions(filename)) || !validate_permissions(file_info.dir().path())) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, filename)); + return reply_perm_denied(msg); + } + if (!MP_FILEOPS.rmdir(dir, filename)) { mpl::log(mpl::Level::trace, category, fmt::format("{}: rmdir failed for \'{}\'", __FUNCTION__, filename)); @@ -647,12 +652,25 @@ int mp::SftpServer::handle_open(sftp_client_message msg) { const auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename) || !validate_permissions(filename)) + if (!validate_path(source_path, filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + return reply_perm_denied(msg); + } + + auto file = std::make_unique(filename); + QFileInfo file_info(filename); + auto exists = file_info.isSymLink() || file->exists(); + + if ((exists && !validate_permissions(filename)) || !validate_permissions(file_info.dir().path())) { mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); } @@ -670,10 +688,6 @@ int mp::SftpServer::handle_open(sftp_client_message msg) if (flags & SSH_FXF_TRUNC) mode |= QIODevice::Truncate; - auto file = std::make_unique(filename); - QFileInfo file_info(filename); - auto exists = file_info.isSymLink() || file->exists(); - if (!MP_FILEOPS.open(*file, mode)) { mpl::log(mpl::Level::trace, category, fmt::format("Cannot open \'{}\': {}", filename, file->errorString())); @@ -718,12 +732,21 @@ int mp::SftpServer::handle_open(sftp_client_message msg) int mp::SftpServer::handle_opendir(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename) || !validate_permissions(filename)) + if (!validate_path(source_path, filename)) { mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + return reply_perm_denied(msg); + } + + if (!validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); } @@ -830,12 +853,21 @@ int mp::SftpServer::handle_readdir(sftp_client_message msg) int mp::SftpServer::handle_readlink(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename) || !validate_permissions(filename)) + if (!validate_path(source_path, filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + return reply_perm_denied(msg); + } + + if (!validate_permissions(filename)) { mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); } @@ -854,12 +886,21 @@ int mp::SftpServer::handle_readlink(sftp_client_message msg) int mp::SftpServer::handle_realpath(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename) || !validate_permissions(filename)) + if (!validate_path(source_path, filename)) { mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + return reply_perm_denied(msg); + } + + if (!validate_permissions(filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); } @@ -870,12 +911,21 @@ int mp::SftpServer::handle_realpath(sftp_client_message msg) int mp::SftpServer::handle_remove(sftp_client_message msg) { auto filename = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, filename) || !validate_permissions(filename)) + if (!validate_path(source_path, filename)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); + return reply_perm_denied(msg); + } + + if (!validate_permissions(filename)) { mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); } @@ -892,11 +942,21 @@ int mp::SftpServer::handle_remove(sftp_client_message msg) int mp::SftpServer::handle_rename(sftp_client_message msg) { const auto source = sftp_client_message_get_filename(msg); - if (!validate_path(source_path, source) || !validate_permissions(source)) + if (!validate_path(source_path, source)) { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, source)); + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, source, source_path)); + return reply_perm_denied(msg); + } + + if (!validate_permissions(source)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, source)); return reply_perm_denied(msg); } @@ -908,15 +968,27 @@ int mp::SftpServer::handle_rename(sftp_client_message msg) } const auto target = sftp_client_message_get_data(msg); - if (!validate_path(source_path, target) || !validate_permissions(target)) + if (!validate_path(source_path, target)) { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, target)); + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, target, source_path)); return reply_perm_denied(msg); } QFile target_file{target}; + + if ((target_file.exists() && !validate_permissions(target)) || + !validate_permissions(QFileInfo(target).dir().path())) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, target)); + return reply_perm_denied(msg); + } + if (target_file.exists()) { if (!MP_FILEOPS.remove(target_file)) @@ -957,7 +1029,15 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) { filename = sftp_client_message_get_filename(msg); if (!validate_path(source_path, filename.toStdString())) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", + __FUNCTION__, + filename, + source_path)); return reply_perm_denied(msg); + } if (!QFileInfo(filename).isSymLink() && !QFile::exists(filename)) { @@ -972,9 +1052,10 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot access path \'{}\' without id mapping: permission denied", __FUNCTION__, filename)); return reply_perm_denied(msg); } + QFile file{filename}; QFileInfo file_info(filename); sftp_attributes_struct attr{}; @@ -1014,15 +1095,25 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) return reply_failure(msg); } - if (msg->attr->flags & SSH_FILEXFER_ATTR_UIDGID && - MP_PLATFORM.chown(filename.toStdString().c_str(), - reverse_uid_for(msg->attr->uid, msg->attr->uid), - reverse_gid_for(msg->attr->gid, msg->attr->gid)) < 0) + if (msg->attr->flags & SSH_FILEXFER_ATTR_UIDGID) { - mpl::log(mpl::Level::trace, - category, - fmt::format("{}: cannot set ownership for \'{}\'", __FUNCTION__, filename)); - return reply_failure(msg); + if (!has_reverse_uid_mapping_for(msg->attr->uid) && !has_reverse_gid_mapping_for(msg->attr->gid)) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot set ownership for \'{}\' without id mapping", __FUNCTION__, filename)); + return reply_perm_denied(msg); + } + + if (MP_PLATFORM.chown(filename.toStdString().c_str(), + reverse_uid_for(msg->attr->uid, msg->attr->uid), + reverse_gid_for(msg->attr->gid, msg->attr->gid)) < 0) + { + mpl::log(mpl::Level::trace, + category, + fmt::format("{}: cannot set ownership for \'{}\'", __FUNCTION__, filename)); + return reply_failure(msg); + } } return reply_ok(msg); @@ -1034,12 +1125,12 @@ int mp::SftpServer::handle_stat(sftp_client_message msg, const bool follow) mpl::log(mpl::Level::trace, category, fmt::format("{}: attempting to access path \'{}\'", __FUNCTION__, filename)); - if (!validate_path(source_path, filename) || !validate_permissions(filename)) + if (!validate_path(source_path, filename)) { mpl::log( mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", __FUNCTION__, filename)); + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, filename, source_path)); return reply_perm_denied(msg); } @@ -1075,7 +1166,16 @@ int mp::SftpServer::handle_symlink(sftp_client_message msg) const auto old_name = sftp_client_message_get_filename(msg); const auto new_name = sftp_client_message_get_data(msg); - if (!validate_path(source_path, new_name) || !validate_permissions(new_name)) + if (!validate_path(source_path, new_name)) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, new_name, source_path)); + return reply_perm_denied(msg); + } + + if (!validate_permissions(new_name)) { mpl::log( mpl::Level::trace, @@ -1147,13 +1247,26 @@ int mp::SftpServer::handle_extended(sftp_client_message msg) const auto old_name = sftp_client_message_get_filename(msg); const auto new_name = sftp_client_message_get_data(msg); - if (!validate_path(source_path, new_name) || !validate_permissions(new_name)) + if (!validate_path(source_path, new_name)) { mpl::log(mpl::Level::trace, category, - fmt::format("{}: cannot access path \'{}\' without mapping: permission denied", + fmt::format("{}: cannot validate path \'{}\' against source \'{}\'", __FUNCTION__, - new_name)); + new_name, + source_path)); + return reply_perm_denied(msg); + } + + if (!validate_permissions(old_name) || !validate_permissions(QFileInfo(new_name).dir().path())) + { + mpl::log( + mpl::Level::trace, + category, + fmt::format("{}: cannot access path \'{}\' against source \'{}\' without mapping: permission denied", + __FUNCTION__, + new_name, + old_name)); return reply_perm_denied(msg); } diff --git a/src/sshfs_mount/sftp_server.h b/src/sshfs_mount/sftp_server.h index 0c00ad94da..c0798a1031 100644 --- a/src/sshfs_mount/sftp_server.h +++ b/src/sshfs_mount/sftp_server.h @@ -52,7 +52,6 @@ class SftpServer private: void process_message(sftp_client_message msg); - sftp_attributes_struct get_uid_gid_from(const QFileInfo& file_info); sftp_attributes_struct attr_from(const QFileInfo& file_info); int mapped_uid_for(const int uid); int mapped_gid_for(const int gid); @@ -60,6 +59,8 @@ class SftpServer int reverse_gid_for(const int gid, const int rev_gid_if_not_found); bool has_uid_mapping_for(const int uid); bool has_gid_mapping_for(const int gid); + bool has_reverse_uid_mapping_for(const int uid); + bool has_reverse_gid_mapping_for(const int gid); bool validate_permissions(const QString& filename); bool validate_path(const std::string& source_path, const std::string& current_path); diff --git a/src/sshfs_mount/sshfs_mount.cpp b/src/sshfs_mount/sshfs_mount.cpp index 59f8d6605d..f3db000862 100644 --- a/src/sshfs_mount/sshfs_mount.cpp +++ b/src/sshfs_mount/sshfs_mount.cpp @@ -105,7 +105,7 @@ auto get_sshfs_exec_and_options(mp::SSHSession& session) } else { - sshfs_exec += " -o dcache_timeout=3"; + sshfs_exec += " -o dcache_timeout=3 -o dir_cache=no"; } } else diff --git a/src/utils/file_ops.cpp b/src/utils/file_ops.cpp index d97f881e1b..449a253af7 100644 --- a/src/utils/file_ops.cpp +++ b/src/utils/file_ops.cpp @@ -72,6 +72,16 @@ bool mp::FileOps::isReadable(const QFileInfo& file) const return file.isReadable(); } +uint mp::FileOps::ownerId(const QFileInfo& file) const +{ + return file.ownerId(); +} + +uint mp::FileOps::groupId(const QFileInfo& file) const +{ + return file.groupId(); +} + bool mp::FileOps::is_open(const QFile& file) const { return file.isOpen(); diff --git a/tests/mock_file_ops.h b/tests/mock_file_ops.h index 1bc5772656..f956247ceb 100644 --- a/tests/mock_file_ops.h +++ b/tests/mock_file_ops.h @@ -41,6 +41,8 @@ class MockFileOps : public FileOps MOCK_METHOD(bool, exists, (const QFileInfo&), (const, override)); MOCK_METHOD(bool, isDir, (const QFileInfo&), (const, override)); MOCK_METHOD(bool, isReadable, (const QFileInfo&), (const, override)); + MOCK_METHOD(uint, ownerId, (const QFileInfo&), (const, override)); + MOCK_METHOD(uint, groupId, (const QFileInfo&), (const, override)); // QFile mock methods MOCK_METHOD(bool, exists, (const QFile&), (const, override)); diff --git a/tests/test_sftpserver.cpp b/tests/test_sftpserver.cpp index 49cfea935a..bd6a83c25a 100644 --- a/tests/test_sftpserver.cpp +++ b/tests/test_sftpserver.cpp @@ -49,15 +49,27 @@ namespace constexpr uint8_t SFTP_BAD_MESSAGE{255u}; struct SftpServer : public mp::test::SftpServerTest { + SftpServer() + { + ON_CALL(*mock_file_ops, ownerId(_)).WillByDefault([](const QFileInfo& file) { return file.ownerId(); }); + ON_CALL(*mock_file_ops, groupId(_)).WillByDefault([](const QFileInfo& file) { return file.groupId(); }); + } + mp::SftpServer make_sftpserver() { return make_sftpserver(""); } - mp::SftpServer make_sftpserver(const std::string& path, const mp::id_mappings& gid_mappings = {}, - const mp::id_mappings& uid_mappings = {}) + mp::SftpServer make_sftpserver(const std::string& path, + mp::id_mappings gid_mappings = {}, + mp::id_mappings uid_mappings = {}) { mp::SSHSession session{"a", 42, "ubuntu", key_provider}; + + QFileInfo file_info{QString::fromStdString(path)}; + uid_mappings.push_back({file_info.ownerId(), 1000}); + gid_mappings.push_back({file_info.groupId(), 1000}); + return {std::move(session), path, path, gid_mappings, uid_mappings, default_id, default_id, "sshfs"}; } @@ -98,6 +110,9 @@ struct SftpServer : public mp::test::SftpServerTest std::queue messages; int default_id{1000}; mpt::MockLogger::Scope logger_scope = mpt::MockLogger::inject(); + + mpt::MockFileOps::GuardedMock mock_file_ops_guard{mpt::MockFileOps::inject()}; + mpt::MockFileOps* mock_file_ops{mock_file_ops_guard.first}; }; struct MessageAndReply @@ -390,9 +405,11 @@ TEST_F(SftpServer, opendir_not_readable_fails) { auto dir_name = name_as_char_array(mpt::test_data_path().toStdString()); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, isReadable(A())).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); auto sftp = make_sftpserver(mpt::test_data_path().toStdString()); auto msg = make_msg(SFTP_OPENDIR); @@ -505,9 +522,11 @@ TEST_F(SftpServer, mkdir_set_permissions_fails) auto new_dir = fmt::format("{}/mkdir-test", temp_dir.path().toStdString()); auto new_dir_name = name_as_char_array(new_dir); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_MKDIR); @@ -623,9 +642,11 @@ TEST_F(SftpServer, rmdir_unable_to_remove_fails) auto new_dir = fmt::format("{}/mkdir-test", temp_dir.path().toStdString()); auto new_dir_name = name_as_char_array(new_dir); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, rmdir(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_RMDIR); @@ -647,7 +668,36 @@ TEST_F(SftpServer, rmdir_unable_to_remove_fails) EXPECT_EQ(failure_num_calls, 1); } -TEST_F(SftpServer, handles_readlink) +TEST_F(SftpServer, handlesRmdirFailsWhenIdsAreNotMapped) +{ + mpt::TempDir temp_dir; + auto new_dir = fmt::format("{}/mkdir-test", temp_dir.path().toStdString()); + auto new_dir_name = name_as_char_array(new_dir); + + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); + + QDir dir(new_dir_name.data()); + ASSERT_TRUE(dir.mkdir(new_dir_name.data())); + ASSERT_TRUE(dir.exists()); + + auto sftp = make_sftpserver(temp_dir.path().toStdString()); + auto msg = make_msg(SFTP_RMDIR); + msg->filename = new_dir_name.data(); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + REPLACE(sftp_reply_status, reply_status); + REPLACE(sftp_get_client_message, make_msg_handler()); + + sftp.run(); + + EXPECT_TRUE(dir.exists()); + EXPECT_THAT(perm_denied_num_calls, Eq(1)); +} + +TEST_F(SftpServer, handlesReadlink) { mpt::TempDir temp_dir; auto file_name = temp_dir.path() + "/test-file"; @@ -853,9 +903,11 @@ TEST_F(SftpServer, rename_cannot_remove_target_fails) auto target_name = name_as_char_array(new_name.toStdString()); REPLACE(sftp_client_message_get_data, [&target_name](auto...) { return target_name.data(); }); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, remove(_)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int failure_num_calls{0}; auto reply_status = make_reply_status(msg.get(), SSH_FX_FAILURE, failure_num_calls); @@ -889,9 +941,11 @@ TEST_F(SftpServer, rename_failure_fails) auto target_name = name_as_char_array(new_name.toStdString()); REPLACE(sftp_client_message_get_data, [&target_name](auto...) { return target_name.data(); }); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, rename(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int failure_num_calls{0}; auto reply_status = make_reply_status(msg.get(), SSH_FX_FAILURE, failure_num_calls); @@ -989,14 +1043,47 @@ TEST_F(SftpServer, remove_non_existing_fails) EXPECT_EQ(failure_num_calls, 1); } -TEST_F(SftpServer, open_in_write_mode_creates_file) +TEST_F(SftpServer, removeFailsWhenIdsAreNotMapped) { mpt::TempDir temp_dir; auto file_name = temp_dir.path() + "/test-file"; + mpt::make_file_with_content(file_name); - ASSERT_FALSE(QFile::exists(file_name)); + ASSERT_TRUE(QFile::exists(file_name)); auto sftp = make_sftpserver(temp_dir.path().toStdString()); + auto msg = make_msg(SFTP_REMOVE); + auto name = name_as_char_array(file_name.toStdString()); + msg->filename = name.data(); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); + + logger_scope.mock_logger->screen_logs(mpl::Level::trace); + EXPECT_CALL(*logger_scope.mock_logger, + log(Eq(mpl::Level::trace), + mpt::MockLogger::make_cstring_matcher(StrEq("sftp server")), + mpt::MockLogger::make_cstring_matcher( + AllOf(HasSubstr("cannot access path"), HasSubstr(file_name.toStdString()))))); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); +} + +TEST_F(SftpServer, openInWriteModeCreatesFile) +{ + mpt::TempDir temp_dir; + auto file_name = temp_dir.path() + "/test-file"; + + ASSERT_FALSE(QFile::exists(file_name)); + + auto sftp = make_sftpserver(temp_dir.path().toStdString(), + {{QFileInfo(temp_dir.path()).ownerId(), 1000}}, + {{QFileInfo(temp_dir.path()).groupId(), 1000}}); auto msg = make_msg(SFTP_OPEN); msg->flags |= SSH_FXF_WRITE; sftp_attributes_struct attr{}; @@ -1059,9 +1146,11 @@ TEST_F(SftpServer, open_unable_to_open_fails) ASSERT_TRUE(QFile::exists(file_name)); ASSERT_THAT(size, Gt(0)); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_OPEN); @@ -1094,10 +1183,12 @@ TEST_F(SftpServer, open_unable_to_set_permissions_fails) mpt::TempDir temp_dir; auto file_name = temp_dir.path() + "/test-file"; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_OPEN); @@ -1310,6 +1401,30 @@ TEST_F(SftpServer, handles_readdir_attributes_preserved) EXPECT_TRUE(compare_permission(test_file_attrs->permissions, test_file_info, Permission::Other)); } +TEST_F(SftpServer, opendirFailsWhenIdsAreNotMapped) +{ + mpt::TempDir temp_dir; + + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly(Return(1001)); + EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly(Return(1001)); + + auto sftp = make_sftpserver(temp_dir.path().toStdString()); + auto open_dir_msg = make_msg(SFTP_OPENDIR); + auto dir_name = name_as_char_array(temp_dir.path().toStdString()); + open_dir_msg->filename = dir_name.data(); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(open_dir_msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); +} + TEST_F(SftpServer, handles_close) { mpt::TempDir temp_dir; @@ -1511,9 +1626,11 @@ TEST_F(SftpServer, setstat_resize_failure_fails) msg->attr = &attr; msg->flags = SSH_FXF_WRITE; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, resize(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int failure_num_calls{0}; auto reply_status = make_reply_status(msg.get(), SSH_FX_FAILURE, failure_num_calls); @@ -1551,10 +1668,12 @@ TEST_F(SftpServer, setstat_set_permissions_failure_fails) msg->attr = &attr; msg->flags = SSH_FXF_WRITE; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, resize(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int failure_num_calls{0}; auto reply_status = make_reply_status(msg.get(), SSH_FX_FAILURE, failure_num_calls); @@ -1573,13 +1692,13 @@ TEST_F(SftpServer, setstat_set_permissions_failure_fails) EXPECT_EQ(failure_num_calls, 1); } -TEST_F(SftpServer, setstat_chown_failure_fails) +TEST_F(SftpServer, setstatChownFailureFails) { mpt::TempDir temp_dir; auto file_name = temp_dir.path() + "/test-file"; mpt::make_file_with_content(file_name); - auto sftp = make_sftpserver(temp_dir.path().toStdString()); + auto sftp = make_sftpserver(temp_dir.path().toStdString(), {{0, 0}}, {{0, 0}}); auto msg = make_msg(SFTP_SETSTAT); auto name = name_as_char_array(file_name.toStdString()); sftp_attributes_struct attr{}; @@ -1587,6 +1706,8 @@ TEST_F(SftpServer, setstat_chown_failure_fails) attr.size = expected_size; attr.flags = SSH_FILEXFER_ATTR_UIDGID; attr.permissions = 0777; + attr.uid = 0; + attr.gid = 0; msg->filename = name.data(); msg->attr = &attr; @@ -1613,15 +1734,92 @@ TEST_F(SftpServer, setstat_chown_failure_fails) EXPECT_EQ(failure_num_calls, 1); } -TEST_F(SftpServer, setstat_utime_failure_fails) +TEST_F(SftpServer, setstatChownFailsWhenNewIdsAreNotMapped) { mpt::TempDir temp_dir; auto file_name = temp_dir.path() + "/test-file"; mpt::make_file_with_content(file_name); + auto sftp = make_sftpserver(temp_dir.path().toStdString(), + {{QFileInfo(temp_dir.path()).ownerId(), 1000}}, + {{QFileInfo(temp_dir.path()).groupId(), 1000}}); + auto msg = make_msg(SFTP_SETSTAT); + auto name = name_as_char_array(file_name.toStdString()); + sftp_attributes_struct attr{}; + const int expected_size = 7777; + attr.size = expected_size; + attr.flags = SSH_FILEXFER_ATTR_UIDGID; + attr.permissions = 0777; + attr.uid = 0; + attr.gid = 0; + + msg->filename = name.data(); + msg->attr = &attr; + msg->flags = SSH_FXF_WRITE; + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); + + logger_scope.mock_logger->screen_logs(mpl::Level::trace); + EXPECT_CALL(*logger_scope.mock_logger, + log(Eq(mpl::Level::trace), + mpt::MockLogger::make_cstring_matcher(StrEq("sftp server")), + mpt::MockLogger::make_cstring_matcher( + AllOf(HasSubstr("without id mapping"), HasSubstr(file_name.toStdString()))))); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); +} + +TEST_F(SftpServer, setstatFailsWhenIdsAreNotMapped) +{ + mpt::TempDir temp_dir; + auto file_name = temp_dir.path() + "/test-file"; + mpt::make_file_with_content(file_name); + + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly(Return(1001)); + EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly(Return(1001)); + auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_SETSTAT); auto name = name_as_char_array(file_name.toStdString()); + + msg->filename = name.data(); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); + + logger_scope.mock_logger->screen_logs(mpl::Level::trace); + EXPECT_CALL(*logger_scope.mock_logger, + log(Eq(mpl::Level::trace), + mpt::MockLogger::make_cstring_matcher(StrEq("sftp server")), + mpt::MockLogger::make_cstring_matcher( + AllOf(HasSubstr("cannot access path"), HasSubstr(file_name.toStdString()))))); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); +} + +TEST_F(SftpServer, setstatUtimeFailureFails) +{ + mpt::TempDir temp_dir; + auto file_name = temp_dir.path() + "/test-file"; + mpt::make_file_with_content(file_name); + + auto sftp = make_sftpserver(temp_dir.path().toStdString(), + {{QFileInfo(temp_dir.path()).ownerId(), 1000}}, + {{QFileInfo(temp_dir.path()).groupId(), 1000}}); + auto msg = make_msg(SFTP_SETSTAT); + auto name = name_as_char_array(file_name.toStdString()); sftp_attributes_struct attr{}; const int expected_size = 7777; attr.size = expected_size; @@ -1644,7 +1842,8 @@ TEST_F(SftpServer, setstat_utime_failure_fails) logger_scope.mock_logger->screen_logs(mpl::Level::trace); EXPECT_CALL(*logger_scope.mock_logger, - log(Eq(mpl::Level::trace), mpt::MockLogger::make_cstring_matcher(StrEq("sftp server")), + log(Eq(mpl::Level::trace), + mpt::MockLogger::make_cstring_matcher(StrEq("sftp server")), mpt::MockLogger::make_cstring_matcher( AllOf(HasSubstr("cannot set modification date for"), HasSubstr(file_name.toStdString()))))); @@ -1730,12 +1929,14 @@ TEST_F(SftpServer, write_cannot_seek_fails) return ssh_string_new(4); }; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce([](QFileDevice& file, QIODevice::OpenMode mode) { return file.open(mode); }); EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, seek(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int failure_num_calls{0}; auto reply_status = make_reply_status(write_msg.get(), SSH_FX_FAILURE, failure_num_calls); @@ -1784,13 +1985,15 @@ TEST_F(SftpServer, write_failure_fails) return ssh_string_new(4); }; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce([](QFileDevice& file, QIODevice::OpenMode mode) { return file.open(mode); }); EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, seek(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, write(_, _, _)).WillOnce(Return(-1)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int failure_num_calls{0}; auto reply_status = make_reply_status(write_msg.get(), SSH_FX_FAILURE, failure_num_calls); @@ -1881,9 +2084,11 @@ TEST_F(SftpServer, read_cannot_seek_fails) return ssh_string_new(4); }; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, seek(_, _)).WillOnce(Return(false)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int failure_num_calls{0}; auto reply_status = make_reply_status(read_msg.get(), SSH_FX_FAILURE, failure_num_calls); @@ -1929,10 +2134,12 @@ TEST_F(SftpServer, read_returns_failure_fails) return ssh_string_new(4); }; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, seek(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, read(_, _, _)).WillOnce(Return(-1)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int failure_num_calls{0}; auto reply_status = make_reply_status(read_msg.get(), SSH_FX_FAILURE, failure_num_calls); @@ -1977,10 +2184,12 @@ TEST_F(SftpServer, read_returns_zero_end_of_file) return ssh_string_new(4); }; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, seek(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, read(_, _, _)).WillOnce(Return(0)); + // EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); + // EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); int eof_num_calls{0}; auto reply_status = make_reply_status(read_msg.get(), SSH_FX_EOF, eof_num_calls); @@ -2091,12 +2300,76 @@ TEST_F(SftpServer, extended_link_failure_fails) EXPECT_EQ(failure_num_calls, 1); } -TEST_F(SftpServer, handle_extended_rename) +TEST_F(SftpServer, extendedLinkFailureFailsWhenSourceFileIdsAreNotMapped) +{ + mpt::TempDir temp_dir; + auto file_name = temp_dir.path() + "/test-file"; + auto link_name = temp_dir.path() + "/test-link"; + mpt::make_file_with_content(file_name); + + auto sftp = make_sftpserver(temp_dir.path().toStdString()); + auto msg = make_msg(SFTP_EXTENDED); + auto submessage = name_as_char_array("hardlink@openssh.com"); + msg->submessage = submessage.data(); + auto name = name_as_char_array(file_name.toStdString()); + msg->filename = name.data(); + + auto target_name = name_as_char_array(link_name.toStdString()); + REPLACE(sftp_client_message_get_data, [&target_name](auto...) { return target_name.data(); }); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); +} + +TEST_F(SftpServer, extendedLinkFailureFailsWhenLinkDirIdsAreNotMapped) +{ + mpt::TempDir temp_dir; + auto file_name = temp_dir.path() + "/test-file"; + auto link_name = temp_dir.path() + "/test-link"; + mpt::make_file_with_content(file_name); + + int host_uid = QFileInfo(temp_dir.path()).ownerId(); + int host_gid = QFileInfo(temp_dir.path()).groupId(); + + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + EXPECT_CALL(*mock_file_ops, ownerId(_)).WillOnce(Return(host_uid)).WillOnce(Return(1001)); + EXPECT_CALL(*mock_file_ops, groupId(_)).WillOnce(Return(host_gid)).WillOnce(Return(1001)); + + auto sftp = make_sftpserver(temp_dir.path().toStdString()); + auto msg = make_msg(SFTP_EXTENDED); + auto submessage = name_as_char_array("hardlink@openssh.com"); + msg->submessage = submessage.data(); + auto name = name_as_char_array(file_name.toStdString()); + msg->filename = name.data(); + + auto target_name = name_as_char_array(link_name.toStdString()); + REPLACE(sftp_client_message_get_data, [&target_name](auto...) { return target_name.data(); }); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); +} + +TEST_F(SftpServer, extendedRenameReplacesTargetFile) { mpt::TempDir temp_dir; auto old_name = temp_dir.path() + "/test-file"; auto new_name = temp_dir.path() + "/test-renamed"; mpt::make_file_with_content(old_name); + mpt::make_file_with_content(new_name); auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_EXTENDED); @@ -2120,6 +2393,111 @@ TEST_F(SftpServer, handle_extended_rename) EXPECT_FALSE(QFile::exists(old_name)); } +TEST_F(SftpServer, extendedRenameFailsWhenSourceFileIdsAreNotMapped) +{ + mpt::TempDir temp_dir; + auto old_name = temp_dir.path() + "/test-file"; + auto new_name = temp_dir.path() + "/test-renamed"; + mpt::make_file_with_content(old_name); + + auto sftp = make_sftpserver(temp_dir.path().toStdString()); + + auto msg = make_msg(SFTP_EXTENDED); + auto submessage = name_as_char_array("posix-rename@openssh.com"); + msg->submessage = submessage.data(); + auto name = name_as_char_array(old_name.toStdString()); + msg->filename = name.data(); + + auto target_name = name_as_char_array(new_name.toStdString()); + REPLACE(sftp_client_message_get_data, [&target_name](auto...) { return target_name.data(); }); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + REPLACE(sftp_reply_status, reply_status); + REPLACE(sftp_get_client_message, make_msg_handler()); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); + EXPECT_TRUE(QFile::exists(old_name)); + EXPECT_FALSE(QFile::exists(new_name)); +} + +TEST_F(SftpServer, extendedRenameFailsWhenTargetFileIdsAreNotMapped) +{ + mpt::TempDir temp_dir; + auto old_name = temp_dir.path() + "/test-file"; + auto new_name = temp_dir.path() + "/test-renamed"; + mpt::make_file_with_content(old_name); + mpt::make_file_with_content(new_name); + + int host_uid = QFileInfo(temp_dir.path()).ownerId(); + int host_gid = QFileInfo(temp_dir.path()).groupId(); + + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + EXPECT_CALL(*mock_file_ops, ownerId(_)).WillOnce(Return(host_uid)).WillOnce(Return(1001)); + EXPECT_CALL(*mock_file_ops, groupId(_)).WillOnce(Return(host_gid)).WillOnce(Return(1001)); + + auto sftp = make_sftpserver(temp_dir.path().toStdString(), {{host_uid, 1000}}, {{host_gid, 1000}}); + + auto msg = make_msg(SFTP_EXTENDED); + auto submessage = name_as_char_array("posix-rename@openssh.com"); + msg->submessage = submessage.data(); + auto name = name_as_char_array(old_name.toStdString()); + msg->filename = name.data(); + + auto target_name = name_as_char_array(new_name.toStdString()); + REPLACE(sftp_client_message_get_data, [&target_name](auto...) { return target_name.data(); }); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + REPLACE(sftp_reply_status, reply_status); + REPLACE(sftp_get_client_message, make_msg_handler()); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); + EXPECT_TRUE(QFile::exists(old_name)); + EXPECT_TRUE(QFile::exists(new_name)); +} + +TEST_F(SftpServer, extendedRenameFailsWhenTargetDirIdsAreNotMapped) +{ + mpt::TempDir temp_dir; + auto old_name = temp_dir.path() + "/test-file"; + auto new_name = temp_dir.path() + "/root-dir/test-renamed"; + mpt::make_file_with_content(old_name); + + int host_uid = QFileInfo(temp_dir.path()).ownerId(); + int host_gid = QFileInfo(temp_dir.path()).groupId(); + + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + EXPECT_CALL(*mock_file_ops, ownerId(_)).WillOnce(Return(host_uid)).WillOnce(Return(1001)); + EXPECT_CALL(*mock_file_ops, groupId(_)).WillOnce(Return(host_gid)).WillOnce(Return(1001)); + + auto sftp = make_sftpserver(temp_dir.path().toStdString(), {{host_uid, 1000}}, {{host_gid, 1000}}); + + auto msg = make_msg(SFTP_EXTENDED); + auto submessage = name_as_char_array("posix-rename@openssh.com"); + msg->submessage = submessage.data(); + auto name = name_as_char_array(old_name.toStdString()); + msg->filename = name.data(); + + auto target_name = name_as_char_array(new_name.toStdString()); + REPLACE(sftp_client_message_get_data, [&target_name](auto...) { return target_name.data(); }); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + REPLACE(sftp_reply_status, reply_status); + REPLACE(sftp_get_client_message, make_msg_handler()); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); + EXPECT_TRUE(QFile::exists(old_name)); + EXPECT_FALSE(QFile::exists(new_name)); +} + TEST_F(SftpServer, extended_rename_in_invalid_dir_fails) { mpt::TempDir temp_dir; @@ -2302,33 +2680,33 @@ TEST_F(SftpServer, DISABLE_ON_WINDOWS(mkdir_chown_honors_maps_in_the_host)) sftp.run(); } -TEST_F(SftpServer, DISABLE_ON_WINDOWS(mkdir_chown_works_when_ids_are_not_mapped)) +TEST_F(SftpServer, DISABLE_ON_WINDOWS(mkdirFailsWhenIdsAreNotMapped)) { mpt::TempDir temp_dir; auto new_dir = fmt::format("{}/mkdir-test", temp_dir.path().toStdString()); auto new_dir_name = name_as_char_array(new_dir); - auto [mock_platform, guard] = mpt::MockPlatform::inject(); + // auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + EXPECT_CALL(*mock_file_ops, ownerId(_)).WillRepeatedly(Return(1001)); + EXPECT_CALL(*mock_file_ops, groupId(_)).WillRepeatedly(Return(1001)); auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_MKDIR); msg->filename = new_dir_name.data(); - sftp_attributes_struct attr{}; - msg->attr = &attr; - msg->attr->uid = 1003; - msg->attr->gid = 1004; - - REPLACE(sftp_get_client_message, make_msg_handler()); - QFileInfo parent_dir(temp_dir.path()); + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); - EXPECT_CALL(*mock_platform, chown(_, parent_dir.ownerId(), parent_dir.groupId())).Times(1); + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); } -TEST_F(SftpServer, DISABLE_ON_WINDOWS(open_chown_honors_maps_in_the_host)) +TEST_F(SftpServer, DISABLE_ON_WINDOWS(openChownHonorsMapsInTheHost)) { mpt::TempDir temp_dir; auto file_name = temp_dir.path() + "/test-file"; @@ -2353,12 +2731,64 @@ TEST_F(SftpServer, DISABLE_ON_WINDOWS(open_chown_honors_maps_in_the_host)) msg->attr->uid = sftp_uid; msg->attr->gid = sftp_gid; + bool reply_handle_invoked{false}; + auto reply_handle = [&reply_handle_invoked](auto...) { + reply_handle_invoked = true; + return SSH_OK; + }; + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_handle, reply_handle); - EXPECT_CALL(*mock_platform, chown(_, host_uid, host_gid)).WillOnce(Return(-1)); + EXPECT_CALL(*mock_platform, chown(_, host_uid, host_gid)).WillOnce(Return(0)); EXPECT_CALL(*mock_platform, chown(_, sftp_uid, sftp_gid)).Times(0); sftp.run(); + + EXPECT_TRUE(reply_handle_invoked); +} + +TEST_F(SftpServer, DISABLE_ON_WINDOWS(openFailsInDirWhenDirIdsAreNotMapped)) +{ + mpt::TempDir temp_dir; + auto file_name = temp_dir.path() + "/test-file"; + + auto sftp = make_sftpserver(temp_dir.path().toStdString()); + auto msg = make_msg(SFTP_OPEN); + auto name = name_as_char_array(file_name.toStdString()); + msg->filename = name.data(); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); +} + +TEST_F(SftpServer, DISABLE_ON_WINDOWS(openFailsWhenIdsAreNotMapped)) +{ + mpt::TempDir temp_dir; + auto file_name = temp_dir.path() + "/test-file"; + mpt::make_file_with_content(file_name); + + auto sftp = make_sftpserver(temp_dir.path().toStdString()); + auto msg = make_msg(SFTP_OPEN); + auto name = name_as_char_array(file_name.toStdString()); + msg->filename = name.data(); + + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); + + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); + + sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); } TEST_F(SftpServer, DISABLE_ON_WINDOWS(setstat_chown_honors_maps_in_the_host)) @@ -2424,12 +2854,14 @@ TEST_F(SftpServer, DISABLE_ON_WINDOWS(setstat_chown_works_when_ids_are_not_mappe msg->attr->uid = sftp_uid; msg->attr->gid = sftp_gid; - REPLACE(sftp_get_client_message, make_msg_handler()); - - auto [mock_platform, guard] = mpt::MockPlatform::inject(); + int perm_denied_num_calls{0}; + auto reply_status = make_reply_status(msg.get(), SSH_FX_PERMISSION_DENIED, perm_denied_num_calls); - EXPECT_CALL(*mock_platform, chown(_, sftp_uid, sftp_gid)).Times(1); + REPLACE(sftp_get_client_message, make_msg_handler()); + REPLACE(sftp_reply_status, reply_status); sftp.run(); + + EXPECT_EQ(perm_denied_num_calls, 1); } } // namespace diff --git a/tests/test_sshfsmount.cpp b/tests/test_sshfsmount.cpp index 36d9c355f8..9c322e8a1e 100644 --- a/tests/test_sshfsmount.cpp +++ b/tests/test_sshfsmount.cpp @@ -193,7 +193,7 @@ struct SshfsMount : public mp::test::SftpServerTest {"id -u", "1000\n"}, {"id -g", "1000\n"}, {"sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -o slave -o transform_symlinks -o allow_other -o " - "Compression=no -o dcache_timeout=3 :\"source\" " + "Compression=no -o dcache_timeout=3 -o dir_cache=no:\"source\" " "\"target\"", "don't care\n"}}; };