From: Razvan Becheriu Date: Wed, 7 May 2025 11:38:22 +0000 (+0300) Subject: [#3842] backport #3832 to 2_6 X-Git-Tag: Kea-2.6.3~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7b938fb375daaa29b5bc8c5855bba8bfcf8bfc9d;p=thirdparty%2Fkea.git [#3842] backport #3832 to 2_6 --- diff --git a/ChangeLog b/ChangeLog index 77f8b4e824..a69128a99b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2265. [sec]* fdupont + Change the umask to no group write and no other access + at the entry of Kea server/agent binaries. + CVE:2025-32803 + (Gitlab #3842, #3832) + 2264. [sec]* tmark kea-dhcp4, kea-dhcp6, kea-dhcp-ddns, and kea-ctrl-agent will now only load hook libraries from the default installation @@ -20,7 +26,6 @@ removed as of libc++ 19. (Gitlab #3823, #3532) - Kea 2.6.2 (stable) released on March 26, 2025 2261. [build] mgodzina diff --git a/src/bin/admin/kea-admin.in b/src/bin/admin/kea-admin.in index c87acb993e..9797359496 100644 --- a/src/bin/admin/kea-admin.in +++ b/src/bin/admin/kea-admin.in @@ -25,6 +25,10 @@ # used. set -eu +# Set no group write and no other access umask +orig_umask=$(umask) +umask $((orig_umask | 0027)) + # Shell ${variables} derived from autoconf @variables@. Some depend on others, so mind the order. prefix="@prefix@" export prefix diff --git a/src/bin/agent/main.cc b/src/bin/agent/main.cc index 68687083a6..0d3b3c6b77 100644 --- a/src/bin/agent/main.cc +++ b/src/bin/agent/main.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -14,6 +15,8 @@ using namespace isc::agent; using namespace isc::process; int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret = EXIT_SUCCESS; // Launch the controller passing in command line arguments. diff --git a/src/bin/d2/main.cc b/src/bin/d2/main.cc index 25975a77d3..7a06993e20 100644 --- a/src/bin/d2/main.cc +++ b/src/bin/d2/main.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -23,6 +24,8 @@ using namespace std; /// The exit value of the program will be EXIT_SUCCESS if there were no /// errors, EXIT_FAILURE otherwise. int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret = EXIT_SUCCESS; // Launch the controller passing in command line arguments. diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc index 7979a04666..7b61a67078 100644 --- a/src/bin/dhcp4/main.cc +++ b/src/bin/dhcp4/main.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -71,6 +72,8 @@ usage() { int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ch; // The default. Any other values are useful for testing only. int server_port_number = DHCP4_SERVER_PORT; diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index fcfff7df53..6d08711f10 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -71,6 +72,8 @@ usage() { int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ch; // The default. Any other values are useful for testing only. int server_port_number = DHCP6_SERVER_PORT; diff --git a/src/bin/keactrl/keactrl.in b/src/bin/keactrl/keactrl.in index cccfdac303..5077091246 100644 --- a/src/bin/keactrl/keactrl.in +++ b/src/bin/keactrl/keactrl.in @@ -21,6 +21,10 @@ # used. set -eu +# Set no group write and no other access umask +orig_umask=$(umask) +umask $((orig_umask | 0027)) + PACKAGE_VERSION="@PACKAGE_VERSION@" EXTENDED_VERSION="@EXTENDED_VERSION@" diff --git a/src/bin/lfc/main.cc b/src/bin/lfc/main.cc index 1b49e9625a..6b9430a67b 100644 --- a/src/bin/lfc/main.cc +++ b/src/bin/lfc/main.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,8 @@ using namespace isc::lfc; /// The exit value of the program will be EXIT_SUCCESS if there were no /// errors, EXIT_FAILURE otherwise. int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret = EXIT_SUCCESS; try { // Ask scheduling to not give too much resources to LFC. diff --git a/src/bin/netconf/main.cc b/src/bin/netconf/main.cc index a6ac2a14af..bc241ea51e 100644 --- a/src/bin/netconf/main.cc +++ b/src/bin/netconf/main.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ using namespace isc::process; using namespace std; int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret = EXIT_SUCCESS; // Launch the controller passing in command line arguments. diff --git a/src/bin/perfdhcp/main.cc b/src/bin/perfdhcp/main.cc index 5262ace5fa..1572d8d79f 100644 --- a/src/bin/perfdhcp/main.cc +++ b/src/bin/perfdhcp/main.cc @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -19,6 +20,8 @@ using namespace isc::perfdhcp; int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret_code = 0; std::string diags; bool parser_error = true; diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc index 46946a228c..f807db18e1 100644 --- a/src/lib/util/filesystem.cc +++ b/src/lib/util/filesystem.cc @@ -69,21 +69,15 @@ isFile(string const& path) { return ((statbuf.st_mode & S_IFMT) == S_IFREG); } -Umask::Umask(mode_t mask) : orig_umask_(umask(S_IWGRP | S_IWOTH)) { - umask(orig_umask_ | mask); -} - -Umask::~Umask() { - umask(orig_umask_); -} - -bool -isSocket(string const& path) { - struct stat statbuf; - if (::stat(path.c_str(), &statbuf) < 0) { - return (false); +void +setUmask() { + // No group write and no other access. + mode_t mask(S_IWGRP | S_IRWXO); + mode_t orig = umask(mask); + // Handle the case where the original umask was already more restrictive. + if ((orig | mask) != mask) { + static_cast(umask(orig | mask)); } - return ((statbuf.st_mode & S_IFMT) == S_IFSOCK); } Path::Path(string const& full_name) { diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h index d726c3178c..34e7aad24b 100644 --- a/src/lib/util/filesystem.h +++ b/src/lib/util/filesystem.h @@ -49,27 +49,11 @@ isDir(const std::string& path); bool isFile(const std::string& path); -/// @brief RAII device to limit access of created files. -struct Umask { - /// @brief Constructor - /// - /// Set wanted bits in umask. - Umask(mode_t mask); - - /// @brief Destructor. - /// - /// Restore umask. - ~Umask(); - -private: - /// @brief Original umask. - mode_t orig_umask_; -}; - -bool -isSocket(const std::string& path); +/// @brief Set umask (at least 0027 i.e. no group write and no other access). +void +setUmask(); -/// @brief Paths on a filesystem +/// \brief Paths on a filesystem struct Path { /// @brief Constructor /// diff --git a/src/lib/util/tests/filesystem_unittests.cc b/src/lib/util/tests/filesystem_unittests.cc index 1b6e792a15..bd309ca195 100644 --- a/src/lib/util/tests/filesystem_unittests.cc +++ b/src/lib/util/tests/filesystem_unittests.cc @@ -70,16 +70,37 @@ TEST_F(FileUtilTest, isFile) { EXPECT_FALSE(isFile(TEST_DATA_BUILDDIR)); } -/// @brief Check Umask. -TEST_F(FileUtilTest, umask) { - // Protect the test itself assuming that Umask does what we expect... - Umask m0(0); - mode_t orig = umask(0); - { - Umask m(S_IROTH); - EXPECT_EQ(S_IROTH, umask(S_IRWXO)); +/// @brief Test fixture class for testing operations on umask. +struct UMaskUtilTest : ::testing::Test { + /// @brief Constructor. + /// + /// Cache the original umask value. + UMaskUtilTest() : orig_umask_(umask(S_IWGRP | S_IWOTH)) { } + + /// @brief Destructor. + /// + /// Restore the original umask value. + virtual ~UMaskUtilTest() { + static_cast(umask(orig_umask_)); } - EXPECT_EQ(0, umask(orig)); + +private: + /// @brief Original umask. + mode_t orig_umask_; +}; + +/// @brief Check setUmask from 0000. +TEST_F(UMaskUtilTest, umask0) { + static_cast(umask(0)); + ASSERT_NO_THROW(setUmask()); + EXPECT_EQ(S_IWGRP | S_IRWXO, umask(0)); +} + +/// @brief Check setUmask from no group access. +TEST_F(UMaskUtilTest, umask077) { + static_cast(umask(S_IRWXG | S_IRWXO)); + ASSERT_NO_THROW(setUmask()); + EXPECT_EQ(S_IRWXG | S_IRWXO, umask(0)); } /// @brief Check that the components are split correctly.