From 678100a7c46d9458297e082802d8ecfb40838150 Mon Sep 17 00:00:00 2001 From: Renuka Manavalan <47282725+renukamanavalan@users.noreply.github.com> Date: Fri, 19 Nov 2021 17:33:13 -0800 Subject: [PATCH] Cherry pick of PR #9123 (#9310) [cherry-pick PR #9123 ] Why I did it When sshd realizes that this login can't succeed due to internal device state or configuration, instead of failing right there, it proceeds to prompt for password, so as the user does not get any clue on where is the failure point. Yet to ensure that this login does not proceed, sshd replaces user provided password with a specific pattern of characters matching length of user provided password. This pattern is "INCORRECT", which is bound to fail. If user provided length is smaller/equal, the substring of pattern is overwritten. If user provided length is greater, the pattern is repeated until length is exhausted. But if the PAM-tacacs plugin would send this password to AAA, the user could get locked out by AAA, for providing incorrect value. How I did it Hence this fix, matches obtained password against the pattern. If match, fail just before reaching AAA server. How to verify it Make sure tacacs is properly configured. Try logging in as, say "user-A"; ensure it succeeds Pick another user, say user-B and ensure this user has not logged into this device before (look into /etc/passed & folders under /home) Disable monit service (as that could fix the issue using disk_check.py) Start TCP dump for all TACACS servers. Simulate Read-only disk Try logging in using user-B. Verify it fails, after 3 attempts Stop tcp dump. TCP dump should show "authentication" for user-A only --- ...0007-handle-bad-password-set-by-sshd.patch | 133 ++++++++++++++++++ src/tacacs/pam/Makefile | 1 + 2 files changed, 134 insertions(+) create mode 100644 src/tacacs/pam/0007-handle-bad-password-set-by-sshd.patch diff --git a/src/tacacs/pam/0007-handle-bad-password-set-by-sshd.patch b/src/tacacs/pam/0007-handle-bad-password-set-by-sshd.patch new file mode 100644 index 000000000000..36a96da77ac5 --- /dev/null +++ b/src/tacacs/pam/0007-handle-bad-password-set-by-sshd.patch @@ -0,0 +1,133 @@ +From f2687e7a442c83e19190695021fb9a60fe07ba60 Mon Sep 17 00:00:00 2001 +From: Renuka Manavalan +Date: Wed, 17 Nov 2021 02:31:45 +0000 +Subject: [PATCH] handle bad password set by sshd + +--- + pam_tacplus.c | 11 +++++++++-- + support.c | 37 +++++++++++++++++++++++++++++++++++++ + support.h | 1 + + tacc.c | 4 ++-- + 4 files changed, 49 insertions(+), 4 deletions(-) + +diff --git a/pam_tacplus.c b/pam_tacplus.c +index ec8ea27..014421b 100644 +--- a/pam_tacplus.c ++++ b/pam_tacplus.c +@@ -251,6 +251,13 @@ int pam_sm_authenticate (pam_handle_t * pamh, int flags, + return PAM_CRED_INSUFFICIENT; + } + ++ if (validate_not_sshd_bad_pass(pass) != PAM_SUCCESS) { ++ syslog(LOG_ERR, "auth fail: Password incorrect"); ++ memset(pass, 0, strlen (pass)); ++ free(pass); ++ return PAM_AUTH_ERR; ++ } ++ + retval = pam_set_item (pamh, PAM_AUTHTOK, pass); + if (retval != PAM_SUCCESS) { + _pam_log(LOG_ERR, "unable to set password"); +@@ -483,7 +490,7 @@ int pam_sm_authenticate (pam_handle_t * pamh, int flags, + syslog(LOG_DEBUG, "%s: exit with pam status: %d", __FUNCTION__, status); + + if (NULL != pass) { +- bzero(pass, strlen (pass)); ++ memset(pass, 0, strlen (pass)); + free(pass); + pass = NULL; + } +@@ -979,7 +986,7 @@ finish: + syslog(LOG_DEBUG, "%s: exit with pam status: %d", __FUNCTION__, status); + + if (NULL != pass) { +- bzero(pass, strlen(pass)); ++ memset(pass, 0, strlen (pass)); + free(pass); + pass = NULL; + } +diff --git a/support.c b/support.c +index 3e55e2f..09d09bf 100644 +--- a/support.c ++++ b/support.c +@@ -108,6 +108,43 @@ int converse(pam_handle_t * pamh, int nargs, const struct pam_message *message, + return retval; + } + ++/* ++ * Ref: From ++ * For future archive searchers: ++ * > Why does OpenSSH replaces the password entered by the user with the ++ * > bad password - "\b\n\r\177INCORRECT ++ * ++ * There are some situations where sshd determines a user can't log in. ++ * Typical samples of that are DenyUsers or PermitRootLogin. ++ * In those cases sshd *still* calls PAM, so that delays set by it are ++ * still performed to the user (without leaking info about accounts ++ * existing, disabled, etc.). But in order to ensure it can't succeed, ++ * replaces the password with that impossible one. ++ * ++ */ ++int validate_not_sshd_bad_pass(const char *pass) ++{ ++ const char *SSHD_BAD_PASS = "\010\012\015\177INCORRECT"; ++ const int SSHD_BAD_PASS_LEN = strlen(SSHD_BAD_PASS); ++ ++ int len = strlen(pass); ++ const char *p = pass; ++ ++ if (len == 0) ++ return PAM_SUCCESS; ++ ++ while (len > 0) { ++ int l = len < SSHD_BAD_PASS_LEN ? len : SSHD_BAD_PASS_LEN; ++ ++ if (strncmp(p, SSHD_BAD_PASS, l) != 0) ++ return PAM_SUCCESS; ++ ++ len -= l; ++ p += l; ++ } ++ return PAM_AUTH_ERR; ++} ++ + /* stolen from pam_stress */ + int tacacs_get_password (pam_handle_t * pamh, int flags + ,int ctrl, char **password) { +diff --git a/support.h b/support.h +index 09b8a85..cb04a4f 100644 +--- a/support.h ++++ b/support.h +@@ -42,6 +42,7 @@ extern struct addrinfo *tac_source_addr; + int _pam_parse (int, const char **); + unsigned long _resolve_name (char *); + unsigned long _getserveraddr (char *serv); ++int validate_not_sshd_bad_pass(const char *pass); + int tacacs_get_password (pam_handle_t *, int, int, char **); + int converse (pam_handle_t *, int, const struct pam_message *, struct pam_response **); + void _pam_log (int, const char *, ...); +diff --git a/tacc.c b/tacc.c +index fcc7d8c..bf0f2a3 100644 +--- a/tacc.c ++++ b/tacc.c +@@ -181,7 +181,7 @@ int main(int argc, char **argv) { + break; + case 'L': + // tac_login is a global variable initialized in libtac +- bzero(tac_login, sizeof(tac_login)); ++ memset(tac_login, 0, sizeof(tac_login)); + strncpy(tac_login, optarg, sizeof(tac_login) - 1); + break; + case 'p': +@@ -312,7 +312,7 @@ int main(int argc, char **argv) { + } + + /* we no longer need the password in our address space */ +- bzero(pass, strlen(pass)); ++ memset(pass, 0, strlen(pass)); + pass = NULL; + + if (do_account) { +-- +2.17.1 + diff --git a/src/tacacs/pam/Makefile b/src/tacacs/pam/Makefile index 503bdcb864f0..843b75cc2f3d 100644 --- a/src/tacacs/pam/Makefile +++ b/src/tacacs/pam/Makefile @@ -20,6 +20,7 @@ $(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% : git apply ../0004-management-vrf-support.patch git apply ../0005-pam-Modify-parsing-of-IP-address-and-port-number-to-.patch git apply ../0006-Add-support-for-source-ip-address.patch + git apply ../0007-handle-bad-password-set-by-sshd.patch dpkg-buildpackage -rfakeroot -b -us -uc -j$(SONIC_CONFIG_MAKE_JOBS) --admindir $(SONIC_DPKG_ADMINDIR) popd