]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#3832] Added umask 0027 setting
authorFrancis Dupont <fdupont@isc.org>
Tue, 29 Apr 2025 09:36:52 +0000 (11:36 +0200)
committerAndrei Pavel <andrei@isc.org>
Fri, 16 May 2025 09:20:43 +0000 (12:20 +0300)
changelog_unreleased/3832-CVE-2025-32803-Files-should-not-be-created-as-world--readable [new file with mode: 0644]
src/bin/agent/main.cc
src/bin/d2/main.cc
src/bin/dhcp4/main.cc
src/bin/dhcp6/main.cc
src/bin/lfc/main.cc
src/bin/netconf/main.cc
src/bin/perfdhcp/main.cc
src/lib/util/filesystem.cc
src/lib/util/filesystem.h
src/lib/util/tests/filesystem_unittests.cc

diff --git a/changelog_unreleased/3832-CVE-2025-32803-Files-should-not-be-created-as-world--readable b/changelog_unreleased/3832-CVE-2025-32803-Files-should-not-be-created-as-world--readable
new file mode 100644 (file)
index 0000000..af617dc
--- /dev/null
@@ -0,0 +1,8 @@
+[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 #3832)
+
+
+
index 68687083a6775366e1675c7c5eb22a25c5ac2058..0d3b3c6b77f3b9ad5944e3a56cfbe885cede236c 100644 (file)
@@ -7,6 +7,7 @@
 #include <config.h>
 #include <agent/ca_controller.h>
 #include <exceptions/exceptions.h>
+#include <util/filesystem.h>
 #include <cstdlib>
 #include <iostream>
 
@@ -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.
index 25975a77d39ed6a864591c1f46f37758144d0535..7a06993e2002143b303375845200c1f7dd490cd6 100644 (file)
@@ -10,6 +10,7 @@
 #include <exceptions/exceptions.h>
 #include <log/logger_manager.h>
 #include <log/logger_support.h>
+#include <util/filesystem.h>
 
 #include <iostream>
 
@@ -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.
index af577ab71087707f0bccd1fc32bfb527231092c7..bdc3b7308bccd67bd89727dbbb1ba03d24d89ef7 100644 (file)
@@ -18,6 +18,7 @@
 #include <log/output_option.h>
 #include <process/cfgrpt/config_report.h>
 #include <process/daemon.h>
+#include <util/filesystem.h>
 
 #include <boost/lexical_cast.hpp>
 
@@ -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;
index 15529c9e6094df1a7eea998bba0dbb57c81f83b7..1ea3aea9f8bc276fb918ad54d997ac9682cf1a23 100644 (file)
@@ -18,6 +18,7 @@
 #include <log/output_option.h>
 #include <process/cfgrpt/config_report.h>
 #include <process/daemon.h>
+#include <util/filesystem.h>
 
 #include <boost/lexical_cast.hpp>
 
@@ -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;
index 1b49e9625a8c537d9ad2dee4d52657bb00a45177..6b9430a67b77f4234013103f07c110e494bac02f 100644 (file)
@@ -9,6 +9,7 @@
 #include <exceptions/exceptions.h>
 #include <log/logger_support.h>
 #include <log/logger_manager.h>
+#include <util/filesystem.h>
 #include <boost/exception/diagnostic_information.hpp>
 #include <boost/exception_ptr.hpp>
 #include <iostream>
@@ -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.
index a6ac2a14af436d54c83b053bde8fa227613644d3..bc241ea51e102e6530b7bddc6c45ec424196cd82 100644 (file)
@@ -7,6 +7,7 @@
 #include <config.h>
 
 #include <exceptions/exceptions.h>
+#include <util/filesystem.h>
 #include <netconf/netconf_controller.h>
 
 #include <cstdlib>
@@ -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.
index 5262ace5fac4ad62c681cfa328e5f08d23cf1549..1572d8d79f89ef9b63994075cc06c6ff35d0126f 100644 (file)
@@ -6,6 +6,7 @@
 
 #include <config.h>
 
+#include <util/filesystem.h>
 #include <perfdhcp/avalanche_scen.h>
 #include <perfdhcp/basic_scen.h>
 #include <perfdhcp/command_options.h>
@@ -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;
index 4ff057466f01409109919c3753c4544107fb8cde..d6ae8e9939401f46b2bbdd97af5ae89de1331ddf 100644 (file)
@@ -78,6 +78,17 @@ isSocket(string const& path) {
     return ((statbuf.st_mode & S_IFMT) == S_IFSOCK);
 }
 
+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<void>(umask(orig | mask));
+    }
+}
+
 Path::Path(string const& full_name) {
     if (!full_name.empty()) {
         bool dir_present = false;
@@ -225,14 +236,15 @@ FileManager::validatePath(const std::string supported_path_str, const std::strin
     if (filename.empty()) {
         isc_throw(BadValue, "path: '" << input_path.str() << "' has no filename");
      }
+
     auto parent_path = input_path.parentPath();
     if (!parent_path.empty()) {
          if (!enforce_path) {
              // Security set to lax, let it fly.
              return (input_path_str);
          }
+
+
          // We only allow absolute path equal to default. Catch an invalid path.
         if (parent_path != supported_path_copy) {
              isc_throw(BadValue, "invalid path specified: '"
@@ -240,7 +252,7 @@ FileManager::validatePath(const std::string supported_path_str, const std::strin
                       << supported_path_copy << "'");
          }
      }
+
     std::string valid_path(supported_path_copy + "/" +  filename);
     return (valid_path);
 }
index ce5a2761890c881a33eeb30ea4f9c1f0982f2ff7..0ab788c56667ebfc503f54a8a3fb417c3e7ecd93 100644 (file)
@@ -48,9 +48,19 @@ isDir(const std::string& path);
 bool
 isFile(const std::string& path);
 
+/// @brief Check if there is a socket at the given path.
+///
+/// @param path The path being checked.
+///
+/// @return True if the path points to a socket, false otherwise including
+/// if the pointed location does not exist.
 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
 struct Path {
     /// @brief Constructor
index 1b3e88168a74b214d6827b3dfca995c5caf13c10..3f8c6c2939b9516f84639139f637bd50d115739a 100644 (file)
@@ -71,6 +71,39 @@ TEST_F(FileUtilTest, isFile) {
     EXPECT_FALSE(isFile(TEST_DATA_BUILDDIR));
 }
 
+/// @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<void>(umask(orig_umask_));
+    }
+
+private:
+    /// @brief Original umask.
+    mode_t orig_umask_;
+};
+
+/// @brief Check setUmask from 0000.
+TEST_F(UMaskUtilTest, umask0) {
+    static_cast<void>(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<void>(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.
 TEST(PathTest, components) {
     // Complete name
@@ -126,8 +159,6 @@ TEST(PathTest, replaceParentPath) {
     EXPECT_EQ("/just/some/dir/a.b", fname.str());
 }
 
-
-
 // Verifies FileManager::validatePath() when enforce_path is true.
 TEST(FileManager, validatePathEnforcePath) {
     std::string def_path = std::string(TEST_DATA_BUILDDIR) + "/";
@@ -168,7 +199,7 @@ TEST(FileManager, validatePathEnforcePath) {
         ""
     },
     {
-        // White space for file name. 
+        // White space for file name.
         __LINE__,
         "      ",
         "",
@@ -233,7 +264,7 @@ TEST(FileManager, validatePathEnforcePathFalse) {
         ""
     },
     {
-        // White space for file name. 
+        // White space for file name.
         __LINE__,
         "      ",
         "",