[#3841] backport #3832 to 2_4

diff --git a/ChangeLog b/ChangeLog
index f49bd222adb1f60a65feb4295604a41f93b09ab8..3dbe84667bbeffaf12c9d9a41e016d3b9db79633 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2262.  [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 #3841, #3832)
+
 2170.  [sec]*          tmark
        kea-dhcp4, kea-dhcp6, kea-dhcp-ddns, and kea-ctrl-agent will
        now only load hook libraries from the default installation

diff --git a/src/bin/admin/admin-utils.sh.in b/src/bin/admin/admin-utils.sh.in
index 62fdc6fad4b81151cfec03bc88515e43d037b406..6d1a42282bc428f41daa054582621f3fe20c214e 100644 (file)
--- a/src/bin/admin/admin-utils.sh.in
+++ b/src/bin/admin/admin-utils.sh.in
@@ -22,6 +22,10 @@
 # used.
 set -eu
 
+# Set no group write and no other access umask
+orig_umask=$(umask)
+umask $((orig_umask | 0027))
+
 # These are the default parameters. They will likely not work in any
 # specific deployment. Also used in unit tests.
 db_host='localhost'

diff --git a/src/bin/agent/main.cc b/src/bin/agent/main.cc
index 68687083a6775366e1675c7c5eb22a25c5ac2058..0d3b3c6b77f3b9ad5944e3a56cfbe885cede236c 100644 (file)
--- a/src/bin/agent/main.cc
+++ b/src/bin/agent/main.cc
@@ -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.

diff --git a/src/bin/d2/d2_lexer.cc b/src/bin/d2/d2_lexer.cc
index df67bfd54a11b61f76353908653124dc9e199a2e..00765394d076930236ad267d5ec54631a92f675c 100644 (file)
--- a/src/bin/d2/d2_lexer.cc
+++ b/src/bin/d2/d2_lexer.cc
@@ -1,6 +1,6 @@
-#line 1 "d2_lexer.cc"
+#line 2 "d2_lexer.cc"
 
-#line 3 "d2_lexer.cc"
+#line 4 "d2_lexer.cc"
 
 #define  YY_INT_ALIGNED short int
 
@@ -1136,7 +1136,7 @@ unsigned int comment_start_line = 0;
 
 /* To avoid the call to exit... oops! */
 #define YY_FATAL_ERROR(msg) isc::d2::D2ParserContext::fatal(msg)
-#line 1139 "d2_lexer.cc"
+#line 1140 "d2_lexer.cc"
 /* noyywrap disables automatic rewinding for the next file to parse. Since we
    always parse only a single string, there's no need to do any wraps. And
    using yywrap requires linking with -lfl, which provides the default yywrap
@@ -1162,8 +1162,8 @@ unsigned int comment_start_line = 0;
    by moving it ahead by yyleng bytes. yyleng specifies the length of the
    currently matched token. */
 #define YY_USER_ACTION  driver.loc_.columns(yyleng);
-#line 1165 "d2_lexer.cc"
 #line 1166 "d2_lexer.cc"
+#line 1167 "d2_lexer.cc"
 
 #define INITIAL 0
 #define COMMENT 1
@@ -1483,7 +1483,7 @@ YY_DECL
     }
 
 
-#line 1486 "d2_lexer.cc"
+#line 1487 "d2_lexer.cc"
 
        while ( /*CONSTCOND*/1 )                /* loops until end-of-file is reached */
                {
@@ -2465,7 +2465,7 @@ YY_RULE_SETUP
 #line 816 "d2_lexer.ll"
 ECHO;
        YY_BREAK
-#line 2468 "d2_lexer.cc"
+#line 2469 "d2_lexer.cc"
 
        case YY_END_OF_BUFFER:
                {

diff --git a/src/bin/d2/main.cc b/src/bin/d2/main.cc
index 25975a77d39ed6a864591c1f46f37758144d0535..7a06993e2002143b303375845200c1f7dd490cd6 100644 (file)
--- a/src/bin/d2/main.cc
+++ b/src/bin/d2/main.cc
@@ -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.

diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc
index 7979a0466642a0dc407c21b64659e50ee8540e96..7b61a67078e244601277fc18bcbe6e57aa1fd251 100644 (file)
--- a/src/bin/dhcp4/main.cc
+++ b/src/bin/dhcp4/main.cc
@@ -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;

diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc
index fcfff7df53508669f7f3a87d1d5d0ab8e1106116..6d08711f109c2cef6ea494b512c3acc5acd61ecf 100644 (file)
--- a/src/bin/dhcp6/main.cc
+++ b/src/bin/dhcp6/main.cc
@@ -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;

diff --git a/src/bin/keactrl/keactrl.in b/src/bin/keactrl/keactrl.in
index 450e997201ebc6a9ddce86ff0d170df06bf636f9..cb91b9b524e1601d84f45a7eb0dbd319a76f9f37 100644 (file)
--- a/src/bin/keactrl/keactrl.in
+++ b/src/bin/keactrl/keactrl.in
@@ -24,6 +24,10 @@
 # used.
 set -eu
 
+# Set no group write and no other access umask
+orig_umask=$(umask)
+umask $((orig_umask | 0027))
+
 VERSION="@PACKAGE_VERSION@"
 
 # Set the have_netconf flag to know if netconf is available.

diff --git a/src/bin/lfc/main.cc b/src/bin/lfc/main.cc
index 1b49e9625a8c537d9ad2dee4d52657bb00a45177..6b9430a67b77f4234013103f07c110e494bac02f 100644 (file)
--- a/src/bin/lfc/main.cc
+++ b/src/bin/lfc/main.cc
@@ -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.

diff --git a/src/bin/netconf/main.cc b/src/bin/netconf/main.cc
index a6ac2a14af436d54c83b053bde8fa227613644d3..bc241ea51e102e6530b7bddc6c45ec424196cd82 100644 (file)
--- a/src/bin/netconf/main.cc
+++ b/src/bin/netconf/main.cc
@@ -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.

diff --git a/src/bin/perfdhcp/main.cc b/src/bin/perfdhcp/main.cc
index 5262ace5fac4ad62c681cfa328e5f08d23cf1549..1572d8d79f89ef9b63994075cc06c6ff35d0126f 100644 (file)
--- a/src/bin/perfdhcp/main.cc
+++ b/src/bin/perfdhcp/main.cc
@@ -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;

diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc
index c4c69878a81c8af06253a486107fb8da62d42cfe..1f5f6ccbcb32e725370ebbe653c0eab05a5c4d32 100644 (file)
--- a/src/lib/util/filesystem.cc
+++ b/src/lib/util/filesystem.cc
@@ -42,14 +42,6 @@ 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;
@@ -59,6 +51,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;

diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h
index c434423fb69d7991e70dd4465b3cdf4ca4b2f445..7f36b8a84a00fa070e2e771a13b6061725f631c2 100644 (file)
--- a/src/lib/util/filesystem.h
+++ b/src/lib/util/filesystem.h
@@ -32,26 +32,13 @@ exists(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
 struct Path {
     /// @brief Constructor

diff --git a/src/lib/util/tests/filesystem_unittests.cc b/src/lib/util/tests/filesystem_unittests.cc
index e17e59594eb14a98d2e466d451eb20cbcc5ca88d..60f18f5bd762f03bbe727ed8c0b011fdf89d9f6b 100644 (file)
--- a/src/lib/util/tests/filesystem_unittests.cc
+++ b/src/lib/util/tests/filesystem_unittests.cc
@@ -22,6 +22,39 @@ using namespace std;
 
 namespace {
 
+/// @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