Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TACACS: Don't send sshd's bad password to AAA #9310

Merged
merged 1 commit into from
Nov 20, 2021

Conversation

renukamanavalan
Copy link
Contributor

@renukamanavalan renukamanavalan commented Nov 18, 2021

[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 "<BS><LF><CR><DEL>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

  1. Make sure tacacs is properly configured.
  2. Try logging in as, say "user-A"; ensure it succeeds
  3. Pick another user, say user-B and ensure this user has not logged into this device before (look into /etc/passed & folders under /home)
  4. Disable monit service (as that could fix the issue using disk_check.py)
  5. Start TCP dump for all TACACS servers.
  6. Simulate Read-only disk
  7. Try logging in using user-B.
  8. Verify it fails, after 3 attempts
  9. Stop tcp dump.
  10. TCP dump should show "authentication" for user-A only

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

Don't send sshd created bad password to AAA
@lguohan
Copy link
Collaborator

lguohan commented Nov 18, 2021

better to keep the same description as the original pr

@renukamanavalan renukamanavalan changed the title Cherry pick of PR #9123 TACACS: Don't send sshd's bad password to AAA Nov 18, 2021
@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep it as 0010 as the master commit? Then later if we decide to backport more patches, others will be easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. As master has diverged well with 3 patches, this patch had to be manually re-created. The contents of 0010 & 0007 are different but for the same purpose. Similarly, in future, if we would need any patch from master it has to be re-created manually. The title of the patch would indicate the patch purpose, which is same in both master & here.

Copy link
Collaborator

@qiluo-msft qiluo-msft Nov 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for misleading. Is it possible to keep original file name ../0010-handle-bad-password-set-by-sshd.patch? I am okay if the file content changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got your point.
The number is just meant for sequence of application. So here, this patch has to be applied after 0006, hence 0007.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants