From cd7f3b8e8850cd80a4f6899cedc726e576c51abe Mon Sep 17 00:00:00 2001 From: muthu90tech Date: Sun, 30 Jun 2024 16:08:18 -0700 Subject: [PATCH] Add APIs to get group details and to change ownership of file. There are a few calls in scylladb that bypass Seastar APIs when making libc and syscall (getgrnam, chown), see issue scylladb/scylladb#17443. This commit implements those APIs, which can be used to address the issue scylladb/scylladb#17443 --- include/seastar/core/file.hh | 8 ++++++ include/seastar/core/reactor.hh | 2 ++ include/seastar/core/seastar.hh | 15 +++++++++++ src/core/reactor.cc | 43 ++++++++++++++++++++++++++++++++ src/core/syscall_result.hh | 2 +- src/util/file.cc | 8 ++++++ tests/unit/CMakeLists.txt | 3 +++ tests/unit/libc_wrapper_test.cc | 44 +++++++++++++++++++++++++++++++++ 8 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 tests/unit/libc_wrapper_test.cc diff --git a/include/seastar/core/file.hh b/include/seastar/core/file.hh index 0e1b78615d8..1c9b0b4651f 100644 --- a/include/seastar/core/file.hh +++ b/include/seastar/core/file.hh @@ -61,6 +61,14 @@ struct directory_entry { std::optional type; }; +/// Group details from the system group database +struct group_details { + sstring group_name; + sstring group_passwd; + __gid_t group_id; + std::vector group_members; +}; + /// Filesystem object stat information struct stat_data { uint64_t device_id; // ID of device containing file diff --git a/include/seastar/core/reactor.hh b/include/seastar/core/reactor.hh index 862e028c2e5..21a4b67e83b 100644 --- a/include/seastar/core/reactor.hh +++ b/include/seastar/core/reactor.hh @@ -481,6 +481,8 @@ public: future<> touch_directory(std::string_view name, file_permissions permissions = file_permissions::default_dir_permissions) noexcept; future> file_type(std::string_view name, follow_symlink = follow_symlink::yes) noexcept; future file_stat(std::string_view pathname, follow_symlink) noexcept; + future<> chown(std::string_view filepath, uid_t owner, gid_t group); + future> getgrnam(std::string_view name); future file_size(std::string_view pathname) noexcept; future file_accessible(std::string_view pathname, access_flags flags) noexcept; future file_exists(std::string_view pathname) noexcept { diff --git a/include/seastar/core/seastar.hh b/include/seastar/core/seastar.hh index 1ea67381a88..03d32dcdbe2 100644 --- a/include/seastar/core/seastar.hh +++ b/include/seastar/core/seastar.hh @@ -359,6 +359,21 @@ using follow_symlink = bool_class; /// with follow_symlink::yes, or for the link itself, with follow_symlink::no. future file_stat(std::string_view name, follow_symlink fs = follow_symlink::yes) noexcept; +/// Wrapper around getgrnam_r. +/// If the provided group name does not exist in the group database, this call will return an empty optional. +/// If the provided group name exists in the group database, the optional returned will contain the struct group_details information. +/// When an unexpected error is thrown by the getgrnam_r libc call, this function throws std::system_error with std::error_code. +/// \param groupname name of the group +/// +/// \return optional struct group_details of the group identified by name. struct group_details has details of the group from the group database. +future> getgrnam(std::string_view name); + +/// Change the owner and group of file. This is a wrapper around chown syscall. +/// The function throws std::system_error, when the chown syscall fails. +/// \param filepath +/// \param owner +/// \param group +future<> chown(std::string_view filepath, uid_t owner, gid_t group); /// Return the size of a file. /// /// \param name name of the file to return the size diff --git a/src/core/reactor.cc b/src/core/reactor.cc index 1ddda9e6ed0..2357c0360df 100644 --- a/src/core/reactor.cc +++ b/src/core/reactor.cc @@ -27,12 +27,14 @@ module; #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -2153,6 +2155,47 @@ void reactor::kill(pid_t pid, int sig) { ret.throw_if_error(); } +future> reactor::getgrnam(std::string_view name) { + syscall_result_extra> sr = co_await _thread_pool->submit>>( + [name = sstring(name)] { + struct group grp; + struct group *result; + memset(&grp, 0, sizeof(struct group)); + char buf[1024]; + errno = 0; + int ret = ::getgrnam_r(name.c_str(), &grp, buf, sizeof(buf), &result); + if (!result) { + return wrap_syscall(ret, std::optional(std::nullopt)); + } + + group_details gd; + gd.group_name = sstring(grp.gr_name); + gd.group_passwd = sstring(grp.gr_passwd); + gd.group_id = grp.gr_gid; + for (char **members = grp.gr_mem; *members != nullptr; ++members) { + gd.group_members.emplace_back(sstring(*members)); + } + return wrap_syscall(ret, std::optional(gd)); + }); + + if (sr.result != 0) { + throw std::system_error(sr.ec()); + } + + co_return sr.extra; +} + +future<> reactor::chown(std::string_view filepath, uid_t owner, gid_t group) { + syscall_result sr = co_await _thread_pool->submit>( + [filepath = sstring(filepath), owner, group] { + int ret = ::chown(filepath.c_str(), owner, group); + return wrap_syscall(ret); + }); + + sr.throw_if_error(); + co_return; +} + future reactor::file_stat(std::string_view pathname, follow_symlink follow) noexcept { // Allocating memory for a sstring can throw, hence the futurize_invoke diff --git a/src/core/syscall_result.hh b/src/core/syscall_result.hh index 493eb5c261d..06f6c737616 100644 --- a/src/core/syscall_result.hh +++ b/src/core/syscall_result.hh @@ -55,7 +55,7 @@ struct syscall_result { throw_fs_exception(reason, fs::path(path1), fs::path(path2)); } } -protected: + std::error_code ec() const { return std::error_code(error, std::system_category()); } diff --git a/src/util/file.cc b/src/util/file.cc index 99dacda6e84..58d62428aa1 100644 --- a/src/util/file.cc +++ b/src/util/file.cc @@ -132,6 +132,14 @@ future file_stat(std::string_view name, follow_symlink follow) noexce return engine().file_stat(name, follow); } +future> getgrnam(std::string_view name) { + return engine().getgrnam(name); +} + +future<> chown(std::string_view filepath, uid_t owner, gid_t group) { + return engine().chown(filepath, owner, group); +} + future file_size(std::string_view name) noexcept { return engine().file_size(name); } diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 527e2ed1020..65a274c7acf 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -354,6 +354,9 @@ seastar_add_test (network_interface seastar_add_test (json_formatter SOURCES json_formatter_test.cc) +seastar_add_test (libc_wrapper + SOURCES libc_wrapper_test.cc) + seastar_add_test (locking SOURCES locking_test.cc) diff --git a/tests/unit/libc_wrapper_test.cc b/tests/unit/libc_wrapper_test.cc new file mode 100644 index 00000000000..ac2fadad2ee --- /dev/null +++ b/tests/unit/libc_wrapper_test.cc @@ -0,0 +1,44 @@ +/* + * This file is open source software, licensed to you under the terms + * of the Apache License, Version 2.0 (the "License"). See the NOTICE file + * distributed with this work for additional information regarding copyright + * ownership. You may not use this file except in compliance with the License. + * + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * Copyright (C) 2024 ScyllaDB. + */ +#include +#include + +#include +#include +#include + +#include + +using namespace seastar; + +SEASTAR_TEST_CASE(getgrnam_group_name_does_not_exist_test) { + // A better approach would be to use a test setup and teardown to create a fake group + // with members in it and in the teardown delete the fake group. + std::optional grp = co_await getgrnam("roo"); + BOOST_REQUIRE(!grp.has_value()); +} + +SEASTAR_TEST_CASE(getgrnam_group_name_exists_test) { + std::optional grp = co_await getgrnam("root"); + BOOST_REQUIRE(grp.has_value()); + BOOST_REQUIRE_EQUAL(grp.value().group_name.c_str(), "root"); +} \ No newline at end of file