From: Razvan Becheriu <razvan@isc.org>
Date: Wed, 7 May 2025 13:07:12 +0000 (+0300)
Subject: [#3841] backport #3832 to 2_4
X-Git-Tag: Kea-2.4.2~14
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0a535bbc423051664c3917640043e6525a5d25d8;p=thirdparty%2Fkea.git

[#3841] backport #3832 to 2_4
---

diff --git a/ChangeLog b/ChangeLog
index f49bd222ad..3dbe84667b 100644
--- 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 62fdc6fad4..6d1a42282b 100644
--- 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 68687083a6..0d3b3c6b77 100644
--- 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 df67bfd54a..00765394d0 100644
--- 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 25975a77d3..7a06993e20 100644
--- 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 7979a04666..7b61a67078 100644
--- 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 fcfff7df53..6d08711f10 100644
--- 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 450e997201..cb91b9b524 100644
--- 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 1b49e9625a..6b9430a67b 100644
--- 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 a6ac2a14af..bc241ea51e 100644
--- 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 5262ace5fa..1572d8d79f 100644
--- 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 c4c69878a8..1f5f6ccbcb 100644
--- 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 c434423fb6..7f36b8a84a 100644
--- 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 e17e59594e..60f18f5bd7 100644
--- 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