From 8d256d1dbb1d666665ee30ba2f07468396e16c79 Mon Sep 17 00:00:00 2001 From: Marius Niculescu Date: Fri, 29 Mar 2024 09:32:42 -0700 Subject: [PATCH] Expanding the AuditEnsurePasswordReuseIsLimited and AuditEnsureLockoutForFailedPasswordAttempts Security Baseline audit checks for all supported distros (#668) --- src/common/commonutils/CommonUtils.h | 2 ++ src/common/commonutils/FileUtils.c | 19 ++++++++------- src/common/tests/CommonUtilsUT.cpp | 12 +++++++++- .../src/lib/SecurityBaseline.c | 24 +++++++++---------- 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/common/commonutils/CommonUtils.h b/src/common/commonutils/CommonUtils.h index 8e450358a..1a83e5868 100644 --- a/src/common/commonutils/CommonUtils.h +++ b/src/common/commonutils/CommonUtils.h @@ -10,6 +10,8 @@ #include #define PRETTY_NAME_AZURE_LINUX_2 "CBL-Mariner/Linux" +#define PRETTY_NAME_ALMA_LINUX_9 "AlmaLinux 9 (Beryllium)" +#define PRETTY_NAME_ALMA_LINUX_9_3 "AlmaLinux 9.3 (Shamrock Pampas Cat)" #define PRETTY_NAME_AMAZON_LINUX_2 "Amazon Linux 2" #define PRETTY_NAME_CENTOS_7 "CentOS Linux 7" #define PRETTY_NAME_CENTOS_8 "CentOS Stream 8" diff --git a/src/common/commonutils/FileUtils.c b/src/common/commonutils/FileUtils.c index 04ee32463..96e3e6b39 100644 --- a/src/common/commonutils/FileUtils.c +++ b/src/common/commonutils/FileUtils.c @@ -1041,7 +1041,7 @@ int CheckLockoutForFailedPasswordAttempts(const char* fileName, void* log) char* value = NULL; int option = 0; int status = ENOENT; - + if (0 == CheckFileExists(fileName, log)) { if (NULL == (contents = LoadStringFromFile(fileName, false, log))) @@ -1052,24 +1052,25 @@ int CheckLockoutForFailedPasswordAttempts(const char* fileName, void* log) { buffer = contents; - // Example of a valid line: + // Example of valid lines: // // auth required pam_tally2.so file=/var/log/tallylog deny=5 even_deny_root unlock_time=2000 + // auth required pam_faillock.so deny=1 even_deny_root unlock_time=300 // - // To pass, all attributes must be present, including pam_tally2.so, the deny value must be between - // 1 and 5 (inclusive), the unlock_time set to a positive value, with any number of spaces between. - // The even_deny_root and any other attribute like it are optional. + // To pass, all attributes must be present, including either pam_faillock.so or pam_tally2.so, + // the deny value must be between 1 and 5 (inclusive), the unlock_time set to a positive value, + // with any number of spaces between.The even_deny_root and any other attribute like it are optional. // // There can be multiple 'auth' lines in the file. Only the right one matters. while (NULL != (value = GetStringOptionFromBuffer(buffer, "auth", ' ', log))) { if (((0 == strcmp("required", value)) && FreeAndReturnTrue(value)) && - ((NULL != (value = GetStringOptionFromBuffer(buffer, "required", ' ', log))) && (0 == strcmp("pam_tally2.so", value)) && FreeAndReturnTrue(value)) && + (((NULL != (value = GetStringOptionFromBuffer(buffer, "required", ' ', log))) && (0 == strcmp("pam_faillock.so", value)) && FreeAndReturnTrue(value)) || + (((NULL != (value = GetStringOptionFromBuffer(buffer, "required", ' ', log))) && (0 == strcmp("pam_tally2.so", value)) && FreeAndReturnTrue(value)) && ((NULL != (value = GetStringOptionFromBuffer(buffer, "pam_tally2.so", ' ', log))) && (0 == strcmp("file=/var/log/tallylog", value)) && FreeAndReturnTrue(value)) && - ((NULL != (value = GetStringOptionFromBuffer(buffer, "file", '=', log))) && (0 == strcmp("/var/log/tallylog", value)) && FreeAndReturnTrue(value)) && - ((0 < (option = GetIntegerOptionFromBuffer(buffer, "deny", '=', log))) && (6 > option)) && - (0 < (option = GetIntegerOptionFromBuffer(buffer, "unlock_time", '=', log)))) + ((NULL != (value = GetStringOptionFromBuffer(buffer, "file", '=', log))) && (0 == strcmp("/var/log/tallylog", value)) && FreeAndReturnTrue(value)))) && + (0 < (option = GetIntegerOptionFromBuffer(buffer, "deny", '=', log))) && (6 > option) && (0 < (option = GetIntegerOptionFromBuffer(buffer, "unlock_time", '=', log)))) { status = 0; break; diff --git a/src/common/tests/CommonUtilsUT.cpp b/src/common/tests/CommonUtilsUT.cpp index 003980963..3113ad345 100755 --- a/src/common/tests/CommonUtilsUT.cpp +++ b/src/common/tests/CommonUtilsUT.cpp @@ -1576,6 +1576,8 @@ TEST_F(CommonUtilsTest, CheckLockoutForFailedPasswordAttempts) { const char* goodTestFileContents[] = { "auth required pam_tally2.so file=/var/log/tallylog deny=1 unlock_time=2000", + "auth required pam_faillock.so deny=3 unlock_time=600", + "auth required pam_faillock.so preauth silent audit deny=1 unlock_time=2000", "auth required pam_tally2.so file=/var/log/tallylog deny=1 even_deny_root unlock_time=2000", "auth required pam_tally2.so file=/var/log/tallylog deny=2 unlock_time=210", "auth required pam_tally2.so file=/var/log/tallylog deny=2 even_deny_root unlock_time=345", @@ -1586,7 +1588,9 @@ TEST_F(CommonUtilsTest, CheckLockoutForFailedPasswordAttempts) "auth required pam_tally2.so file=/var/log/tallylog deny=5 unlock_time=203", "auth required pam_tally2.so file=/var/log/tallylog deny=5 even_deny_root unlock_time=5001", "This is a positive test\nauth required pam_tally2.so file=/var/log/tallylog deny=3 unlock_time=123", - "This is a positive test\nAnother one with auth test\nauth required pam_tally2.so file=/var/log/tallylog deny=3 unlock_time=123" + "This is a positive test\nAnother one with auth test\nauth required pam_tally2.so file=/var/log/tallylog deny=3 unlock_time=123", + "This is a positive test\nauth required pam_faillock.so deny=3 unlock_time=543", + "This is a positive test\nAnother one with auth test\nauth required pam_faillock.so deny=5 unlock_time=647" }; const char* badTestFileContents[] = { @@ -1601,6 +1605,12 @@ TEST_F(CommonUtilsTest, CheckLockoutForFailedPasswordAttempts) "auth required pam_tally2.so file=/var/log/tallylog deny=0 unlock_time=0", "auth required pam_tally2.so file=/var/log/tallylog deny=2 unlock_time=", "auth required pam_tally2.so file=/var/log/tallylog", + "auth required pam_faillock.so", + "auth required pam_faillock.so deny=1", + "auth required pam_faillock.so unlock_time=500", + "auth required pam_faillock.so deny=6 unlock_time=100", + "auth required pam_faillock.so deny=4 unlock_time=-1", + "auth required pam_faillock.so deny=7 unlock_time=-100", "This is a negative auth test", "This is a negative test", "auth [success=1 default=ignore] pam_unix.so nullok\n" diff --git a/src/modules/securitybaseline/src/lib/SecurityBaseline.c b/src/modules/securitybaseline/src/lib/SecurityBaseline.c index e4ccf6ce7..57ac45e63 100644 --- a/src/modules/securitybaseline/src/lib/SecurityBaseline.c +++ b/src/modules/securitybaseline/src/lib/SecurityBaseline.c @@ -1199,11 +1199,13 @@ static char* AuditEnsurePermissionsOnBootloaderConfig(void) static char* AuditEnsurePasswordReuseIsLimited(void) { - //TBD: refine this and expand to other distros + const char* etcPamdSystemAuth = "/etc/pam.d/system-auth"; int option = 0; - return (5 >= (option = GetIntegerOptionFromFile(g_etcPamdCommonPassword, "remember", '=', SecurityBaselineGetLog()))) ? - ((-999 == option) ? FormatAllocateString("A 'remember' option is not found in %s", g_etcPamdCommonPassword) : - FormatAllocateString("A 'remember' option is set to '%d' in %s instead of expected '5' or greater", option, g_etcPamdCommonPassword)) : + + return ((5 >= (option = GetIntegerOptionFromFile(g_etcPamdCommonPassword, "remember", '=', SecurityBaselineGetLog()))) || + (5 >= (option = GetIntegerOptionFromFile(etcPamdSystemAuth, "remember", '=', SecurityBaselineGetLog())))) ? + ((-999 == option) ? FormatAllocateString("The 'remember' option is not found in '%s' or in '%s'", g_etcPamdCommonPassword, etcPamdSystemAuth) : + FormatAllocateString("The 'remember' option is set to '%d' in '%s' or '%s' instead of expected '5' or greater", option, g_etcPamdCommonPassword, etcPamdSystemAuth)) : DuplicateString(g_pass); } @@ -1262,17 +1264,13 @@ static char* AuditEnsurePasswordCreationRequirements(void) static char* AuditEnsureLockoutForFailedPasswordAttempts(void) { - //TBD: expand to other distros const char* passwordAuth = "/etc/pam.d/password-auth"; + const char* commonAuth = "/etc/pam.d/common-auth"; - return ((0 == CheckLockoutForFailedPasswordAttempts(passwordAuth, SecurityBaselineGetLog())) && - (EEXIST == CheckLineNotFoundOrCommentedOut(passwordAuth, '#', "auth", SecurityBaselineGetLog())) && - (EEXIST == CheckLineNotFoundOrCommentedOut(passwordAuth, '#', "pam_tally2.so", SecurityBaselineGetLog())) && - (EEXIST == CheckLineNotFoundOrCommentedOut(passwordAuth, '#', "file=/var/log/tallylog", SecurityBaselineGetLog())) && - (0 < GetIntegerOptionFromFile(passwordAuth, "deny", '=', SecurityBaselineGetLog())) && - (0 < GetIntegerOptionFromFile(passwordAuth, "unlock_time", '=', SecurityBaselineGetLog()))) ? DuplicateString(g_pass) : - FormatAllocateString("In %s: lockout for failed password attempts not set, 'auth', 'pam_tally2.so', 'file=/var/log/tallylog' " - "not found, 'deny' or 'unlock_time' is not found or not set to greater than 0", passwordAuth); + return ((0 == CheckLockoutForFailedPasswordAttempts(passwordAuth, SecurityBaselineGetLog())) || + (0 == CheckLockoutForFailedPasswordAttempts(commonAuth, SecurityBaselineGetLog()))) ? DuplicateString(g_pass) : + FormatAllocateString("In %s: lockout for failed password attempts not set, 'auth', 'pam_faillock.so' or 'pam_tally2.so' and 'file=/var/log/tallylog' " + "not found, or 'deny' or 'unlock_time' not found, or 'deny' not in between 1 and 5, or 'unlock_time' not set to greater than 0", passwordAuth); } static char* AuditEnsureDisabledInstallationOfCramfsFileSystem(void)