Wupeka/aoc kmod and logfile fixes

src/modules/complianceengine/src/lib/KernelModuleTools.cpp

@@ -5,6 +5,7 @@
 #include <Telemetry.h>
 #include <algorithm>
 #include <dirent.h>
+#include <fstream>
 #include <fts.h>
 #include <iostream>
 #include <string>
@@ -137,7 +138,88 @@ Result<bool> IsKernelModuleLoaded(std::string moduleName, ContextInterface& cont
     return false;
 }
 
-Result<Status> IsKernelModuleBlocked(std::string moduleName, IndicatorsTree& indicators, ContextInterface& context)
+// Reads the currently-running kernel release (equivalent to `uname -r`) from
+// /proc/sys/kernel/osrelease. Returns an empty string if it cannot be read.
+static std::string GetRunningKernelRelease(ContextInterface& context)
+{
+    std::ifstream ifs(context.GetSpecialFilePath("/proc/sys/kernel/osrelease"));
+    std::string release;
+    std::getline(ifs, release);
+    while (!release.empty() && (release.back() == '\n' || release.back() == '\r' || release.back() == ' ' || release.back() == '\t'))
+    {
+        release.pop_back();
+    }
+    return release;
+}
+
+// Returns true if the module object file is present in the currently-running
+// kernel's module directory (/lib/modules/$(uname -r)/kernel).
+//
+// CIS only requires the "install <module> /bin/false" masking when the module is
+// loadable in the running kernel. When the module exists only in a non-running
+// installed kernel (or not at all in the running kernel), deny-listing alone is
+// sufficient, so the mask must not be required in that case.
+Result<bool> IsModuleAvailableInRunningKernel(const std::string& moduleName, ContextInterface& context)
+{
+    std::string release = GetRunningKernelRelease(context);
+    if (release.empty())
+    {
+        // Unable to determine the running kernel; be conservative and assume the module
+        // may be loadable so the stricter mask requirement still applies.
+        return true;
+    }
+
+    std::string kernelDirPath = context.GetSpecialFilePath("/lib/modules") + "/" + release + "/kernel";
+    struct stat st;
+    if (stat(kernelDirPath.c_str(), &st) != 0)
+    {
+        if (errno == ENOENT)
+        {
+            return false;
+        }
+        OsConfigLogError(context.GetLogHandle(), "Failed to stat %s - errno %d", kernelDirPath.c_str(), errno);
+        OSConfigTelemetryStatusTrace("stat", errno);
+        return true;
+    }
+    if (!S_ISDIR(st.st_mode))
+    {
+        return false;
+    }
+
+    char* paths[] = {const_cast<char*>(kernelDirPath.c_str()), nullptr};
+    FTS* fts = fts_open(paths, FTS_PHYSICAL, nullptr);
+    if (!fts)
+    {
+        OsConfigLogError(context.GetLogHandle(), "Failed to open %s - errno %d", kernelDirPath.c_str(), errno);
+        OSConfigTelemetryStatusTrace("fts_open", errno);
+        return false;
+    }
+    auto ftsDeleter = std::unique_ptr<FTS, int (*)(FTS*)>(fts, fts_close);
+
+    std::string target = moduleName + ".ko";
+    std::string overlayTarget = moduleName + "_overlay.ko";
+    std::string targetUnderscore = target;
+    std::replace(targetUnderscore.begin(), targetUnderscore.end(), '-', '_');
+    std::string overlayTargetUnderscore = overlayTarget;
+    std::replace(overlayTargetUnderscore.begin(), overlayTargetUnderscore.end(), '-', '_');
+
+    FTSENT* node = nullptr;
+    while ((node = fts_read(fts)) != nullptr)
+    {
+        if (node->fts_info != FTS_F)
+        {
+            continue;
+        }
+        std::string baseName = node->fts_name;
+        if (baseName.find(target) == 0 || baseName.find(targetUnderscore) == 0 || baseName.find(overlayTarget) == 0 || baseName.find(overlayTargetUnderscore) == 0)
+        {
+            return true;
+        }
+    }
+    return false;
+}
+
+Result<Status> IsKernelModuleBlocked(std::string moduleName, bool requireMask, IndicatorsTree& indicators, ContextInterface& context)
 {
     Result<std::string> modprobeOutput = context.ExecuteCommand("modprobe --showconfig");
     if (modprobeOutput.HasValue())
@@ -158,18 +240,21 @@ Result<Status> IsKernelModuleBlocked(std::string moduleName, IndicatorsTree& ind
             return indicators.NonCompliant("Module " + moduleName + " is not blacklisted in modprobe configuration");
         }
 
-        regex modprobeInstallRegex;
-        try
+        if (requireMask)
         {
-            modprobeInstallRegex = regex("^install\\s+" + UnderscoreForRegex(moduleName) + "\\s+(/usr)?/bin/(true|false)");
-        }
-        catch (std::exception& e)
-        {
-            return Error(e.what());
-        }
-        if (!MultilineRegexSearch(modprobeOutput.Value(), modprobeInstallRegex))
-        {
-            return indicators.NonCompliant("Module " + moduleName + " is not masked in modprobe configuration");
+            regex modprobeInstallRegex;
+            try
+            {
+                modprobeInstallRegex = regex("^install\\s+" + UnderscoreForRegex(moduleName) + "\\s+(/usr)?/bin/(true|false)");
+            }
+            catch (std::exception& e)
+            {
+                return Error(e.what());
+            }
+            if (!MultilineRegexSearch(modprobeOutput.Value(), modprobeInstallRegex))
+            {
+                return indicators.NonCompliant("Module " + moduleName + " is not masked in modprobe configuration");
+            }
         }
     }
     else

src/modules/complianceengine/src/lib/KernelModuleTools.h

@@ -13,7 +13,8 @@ namespace ComplianceEngine
 
 Result<bool> SearchFilesystemForModuleName(std::string& moduleName, ContextInterface& context);
 Result<bool> IsKernelModuleLoaded(std::string moduleName, ContextInterface& context);
-Result<Status> IsKernelModuleBlocked(std::string moduleName, IndicatorsTree& indicators, ContextInterface& context);
+Result<bool> IsModuleAvailableInRunningKernel(const std::string& moduleName, ContextInterface& context);
+Result<Status> IsKernelModuleBlocked(std::string moduleName, bool requireMask, IndicatorsTree& indicators, ContextInterface& context);
 
 } // namespace ComplianceEngine
 

src/modules/complianceengine/src/lib/procedures/KernelModule.cpp

@@ -32,7 +32,13 @@ Result<Status> AuditKernelModuleUnavailable(const KernelModuleUnavailableParams&
         return indicators.NonCompliant("Module " + moduleName + " is loaded");
     }
 
-    return IsKernelModuleBlocked(moduleName, indicators, context);
+    auto inRunningKernel = IsModuleAvailableInRunningKernel(moduleName, context);
+    if (!inRunningKernel.HasValue())
+    {
+        return Result<Status>(inRunningKernel.Error());
+    }
+
+    return IsKernelModuleBlocked(moduleName, inRunningKernel.Value(), indicators, context);
 }
 
 } // namespace ComplianceEngine

src/modules/complianceengine/src/lib/procedures/LogFilePermissions.cpp

@@ -5,11 +5,15 @@
 #include <Evaluator.h>
 #include <FilePermissions.h>
 #include <FileTreeWalk.h>
+#include <ListValidShells.h>
 #include <LogFilePermissions.h>
 #include <Result.h>
 #include <Telemetry.h>
+#include <UsersIterator.h>
 #include <fnmatch.h>
 #include <map>
+#include <pwd.h>
+#include <set>
 #include <string>
 #include <sys/stat.h>
 
@@ -45,10 +49,46 @@ const std::map<std::string, std::map<std::string, std::string>> g_logfilePattern
     {"*.journal~", {{"owner", "root"}, {"group", "root|systemd-journal"}, {"mask", "0137"}}},
 };
 
-// Default mask for files that don't match any pattern
-// TODO(wpk) add the magic related to daemon-owned files.
+// Default arguments for files that don't match any pattern.
 const std::map<std::string, std::string> g_defaultLogfileArgs = {{"owner", "root|syslog"}, {"group", "root|adm"}, {"mask", "0137"}};
 
+using DaemonUidSet = std::set<uid_t>;
+
+// Builds the set of uids that belong to root or a daemon/service account (one whose login shell is
+// not a valid interactive shell per /etc/shells). Computed once from /etc/passwd so we don't have to
+// look up each file's owner while walking the log directory.
+Result<DaemonUidSet> BuildDaemonUidSet(ContextInterface& context)
+{
+    auto validShells = ListValidShells(context);
+    if (!validShells.HasValue())
+    {
+        OsConfigLogError(context.GetLogHandle(), "Failed to list valid shells: %s", validShells.Error().message.c_str());
+        OSConfigTelemetryStatusTrace("ListValidShells", validShells.Error().code);
+        return validShells.Error();
+    }
+
+    auto users = UsersRange::Make(context.GetSpecialFilePath("/etc/passwd"), context.GetLogHandle());
+    if (!users.HasValue())
+    {
+        OsConfigLogError(context.GetLogHandle(), "Failed to enumerate users: %s", users.Error().message.c_str());
+        OSConfigTelemetryStatusTrace("UsersRange", users.Error().code);
+        return users.Error();
+    }
+
+    DaemonUidSet daemonUids;
+    for (const auto& user : users.Value())
+    {
+        const std::string name = (user.pw_name != nullptr) ? user.pw_name : std::string();
+        const std::string shell = (user.pw_shell != nullptr) ? user.pw_shell : std::string();
+        if (name == "root" || validShells.Value().find(shell) == validShells.Value().end())
+        {
+            daemonUids.insert(user.pw_uid);
+        }
+    }
+
+    return daemonUids;
+}
+
 FilePermissionsParams GetFilePermissionsParams(const std::string& filename, std::map<std::string, std::string> args)
 {
     args["path"] = filename;
@@ -60,7 +100,18 @@ FilePermissionsParams GetFilePermissionsParams(const std::string& filename, std:
     return result.Value();
 }
 
-FilePermissionsParams GetFilePermissionArgs(const std::string& filename, const std::string& fullPath)
+FilePermissionsParams GetDefaultFilePermissionArgs(const std::string& fullPath, const struct stat& statInfo, const DaemonUidSet& daemonUids, bool remediate)
+{
+    if (!remediate && daemonUids.find(statInfo.st_uid) != daemonUids.end())
+    {
+        return GetFilePermissionsParams(fullPath, {{"mask", "0137"}});
+    }
+
+    return GetFilePermissionsParams(fullPath, g_defaultLogfileArgs);
+}
+
+FilePermissionsParams GetFilePermissionArgs(const std::string& filename, const std::string& fullPath, const struct stat& statInfo,
+    const DaemonUidSet& daemonUids, bool remediate)
 {
     for (const auto& pattern : g_logfilePatterns)
     {
@@ -70,11 +121,11 @@ FilePermissionsParams GetFilePermissionArgs(const std::string& filename, const s
         }
     }
 
-    return GetFilePermissionsParams(fullPath, g_defaultLogfileArgs);
+    return GetDefaultFilePermissionArgs(fullPath, statInfo, daemonUids, remediate);
 }
 
-Result<Status> ProcessLogfile(const std::string& path, const std::string& filename, const struct stat& statInfo, IndicatorsTree& indicators,
-    ContextInterface& context, bool remediate)
+Result<Status> ProcessLogfile(const std::string& path, const std::string& filename, const struct stat& statInfo, const DaemonUidSet& daemonUids,
+    IndicatorsTree& indicators, ContextInterface& context, bool remediate)
 {
     if (S_ISDIR(statInfo.st_mode))
     {
@@ -93,7 +144,7 @@ Result<Status> ProcessLogfile(const std::string& path, const std::string& filena
     }
 
     const std::string fullPath = path + "/" + filename;
-    const auto params = GetFilePermissionArgs(filename, fullPath);
+    const auto params = GetFilePermissionArgs(filename, fullPath, statInfo, daemonUids, remediate);
 
     OsConfigLogDebug(context.GetLogHandle(), "Processing logfile: %s with pattern-matched permissions", fullPath.c_str());
 
@@ -126,8 +177,14 @@ Result<Status> AuditLogFilePermissions(const LogFilePermissionsParams& params, I
     assert(params.path.HasValue());
     OsConfigLogInfo(context.GetLogHandle(), "Auditing logfile access permissions in directory: %s", params.path->c_str());
 
-    auto callback = [&indicators, &context](const std::string& dirPath, const std::string& filename, const struct stat& statInfo) -> Result<Status> {
-        return ProcessLogfile(dirPath, filename, statInfo, indicators, context, false);
+    auto daemonUids = BuildDaemonUidSet(context);
+    if (!daemonUids.HasValue())
+    {
+        return daemonUids.Error();
+    }
+
+    auto callback = [&daemonUids, &indicators, &context](const std::string& dirPath, const std::string& filename, const struct stat& statInfo) -> Result<Status> {
+        return ProcessLogfile(dirPath, filename, statInfo, daemonUids.Value(), indicators, context, false);
     };
 
     auto result = FileTreeWalk(params.path.Value(), callback, BreakOnNonCompliant::False, context);
@@ -154,8 +211,14 @@ Result<Status> RemediateLogFilePermissions(const LogFilePermissionsParams& param
     assert(params.path.HasValue());
     OsConfigLogInfo(context.GetLogHandle(), "Remediating logfile access permissions in directory: %s", params.path->c_str());
 
-    auto callback = [&indicators, &context](const std::string& dirPath, const std::string& filename, const struct stat& statInfo) -> Result<Status> {
-        return ProcessLogfile(dirPath, filename, statInfo, indicators, context, true);
+    auto daemonUids = BuildDaemonUidSet(context);
+    if (!daemonUids.HasValue())
+    {
+        return daemonUids.Error();
+    }
+
+    auto callback = [&daemonUids, &indicators, &context](const std::string& dirPath, const std::string& filename, const struct stat& statInfo) -> Result<Status> {
+        return ProcessLogfile(dirPath, filename, statInfo, daemonUids.Value(), indicators, context, true);
     };
 
     auto result = FileTreeWalk(params.path.Value(), callback, BreakOnNonCompliant::False, context);

src/modules/complianceengine/src/lib/procedures/WirelessDisabled.cpp

@@ -114,7 +114,7 @@ Result<Status> AuditWirelessDisabled(IndicatorsTree& indicators, ContextInterfac
         {
             return indicators.NonCompliant("Kernel module loaded '" + module + "'");
         }
-        auto isModuleBlocked = IsKernelModuleBlocked(module, indicators, context);
+        auto isModuleBlocked = IsKernelModuleBlocked(module, true, indicators, context);
         if (!isModuleBlocked.HasValue() || isModuleBlocked.Value() == Status::NonCompliant)
         {
             return isModuleBlocked;

src/modules/complianceengine/tests/procedures/KernelModuleTest.cpp

@@ -58,15 +58,16 @@ class EnsureKernelModuleTest : public ::testing::Test
 
 // TODO(kkanas) remove
 // Helper to create a fake /lib/modules tree
-static std::string CreateModulesTree(MockContext& ctx, const std::vector<std::string>& files)
+static std::string CreateModulesTree(MockContext& ctx, const std::vector<std::string>& files, const std::string& kernelVersion = "5.15.test",
+    const std::string& runningRelease = "5.15.test")
 {
     std::string root = ctx.GetTempdirPath() + "/modulesRoot";
     if (::mkdir(root.c_str(), 0755) != 0)
     {
         ADD_FAILURE() << "Failed to create root dir: " << strerror(errno);
         return "";
     }
-    std::string versionDir = root + "/5.15.test";
+    std::string versionDir = root + "/" + kernelVersion;
     if (::mkdir(versionDir.c_str(), 0755) != 0)
     {
         ADD_FAILURE() << "Failed to create version dir: " << strerror(errno);
@@ -86,6 +87,14 @@ static std::string CreateModulesTree(MockContext& ctx, const std::vector<std::st
         ofs.close();
     }
     ctx.SetSpecialFilePath("/lib/modules", root);
+
+    // Expose the running kernel release so IsModuleAvailableInRunningKernel can determine
+    // whether the module lives in the running kernel's module directory.
+    std::string osrelease = root + "/osrelease";
+    std::ofstream rel(osrelease);
+    rel << runningRelease << "\n";
+    rel.close();
+    ctx.SetSpecialFilePath("/proc/sys/kernel/osrelease", osrelease);
     return root;
 }
 
@@ -335,3 +344,21 @@ TEST_F(EnsureKernelModuleTest, ExactDashFilenameStillMatches)
     ASSERT_TRUE(result.HasValue());
     ASSERT_EQ(result.Value(), Status::NonCompliant);
 }
+
+TEST_F(EnsureKernelModuleTest, ModuleOnlyInNonRunningKernelNeedsOnlyBlocklist)
+{
+    // Module exists in an installed kernel that is NOT the running one. Per CIS, only
+    // deny-listing is required in that case; the install/mask line must not be required.
+    CreateModulesTree(mContext, {"usb-storage.ko"}, "5.15.test", "6.99.running");
+
+    EXPECT_CALL(mContext, GetFileContents(::testing::StrEq(procModulesPath))).WillRepeatedly(::testing::Return(Result<std::string>(procModulesNegativeOutput)));
+    // Deny-listed but no install/mask line present.
+    EXPECT_CALL(mContext, ExecuteCommand(::testing::HasSubstr(modprobeCommand))).WillRepeatedly(::testing::Return(Result<std::string>(modprobeBlocklistOutput)));
+
+    KernelModuleUnavailableParams params;
+    params.moduleName = "usb-storage";
+
+    auto result = AuditKernelModuleUnavailable(params, indicators, mContext);
+    ASSERT_TRUE(result.HasValue());
+    ASSERT_EQ(result.Value(), Status::Compliant);
+}