-
Notifications
You must be signed in to change notification settings - Fork 37
Masked input instead Hidden & more smart_subs addition to Password-Checker.py #46
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
base: main
Are you sure you want to change the base?
Conversation
|
👋 @LUCKYS1NGHH |
| def passwd_enter(prompt_part, mask="x"): | ||
| prompt = f"\nEnter the Password to {prompt_part}: " | ||
| sys.stdout.write(prompt) | ||
| sys.stdout.flush() | ||
| password = "" | ||
| fd = sys.stdin.fileno() | ||
| old_settings = termios.tcgetattr(fd) | ||
| try: | ||
| tty.setraw(fd) | ||
| while True: | ||
| ch = sys.stdin.read(1) | ||
| if ch in ('\r', '\n'): | ||
| sys.stdout.write('\r\n') | ||
| sys.stdout.flush() | ||
| break | ||
| elif ch == '\x7f': # backspace | ||
| if password: | ||
| password = password[:-1] | ||
| sys.stdout.write('\b \b') | ||
| sys.stdout.flush() | ||
| else: | ||
| password += ch | ||
| sys.stdout.write(mask) | ||
| sys.stdout.flush() | ||
| finally: | ||
| termios.tcsetattr(fd, termios.TCSADRAIN, old_settings) | ||
| return password |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @LUCKYS1NGHH,
Just review your changes, and they're great. It has improved UX for the end user, but these changes do have some major concerns, like security, cross-platform compatibility and an edge case issue.
I understand you're recommending these changes to make this small project even better, but these changes can't be accepted at the cost of the core intention of this project.
Major Concerns:
- Security:
- While the idea of masking sounds more user-friendly, it gives off more details, like password length.
- Since you're storing the password, it's unsecured and may persist even after execution.
- Cross-Platform Compatibility:
- Since you're using
termios, it's best suited for *unix-based systems but not Windows.
- Since you're using
- Edge Case:
- While this logic looks great, this has a major flaw, and that is detecting keystrokes that prompt signals like
SIGTERM, i.e., ctrl + c or any other Keyboard Interrupt/EOF. - This Edge case goes the same for the systems while using delete on macOS systems when compared to backspace on Windows-based systems.
- While this logic looks great, this has a major flaw, and that is detecting keystrokes that prompt signals like
While some external packages and modules provide such masking while handling all the above-mentioned concerns, I believe keeping the getpass implementation would make sense based on this project's core intention.
This project was meant to be a simple script that uses native functions/methods/packages to address multiple concerns, thus providing high portability where the script executes without any additional requirements.
Also getpass function is battle-tested and takes care of all the above concerns while also addressing many other security concerns that are not mentioned here.
Added a function called passwd_enter to the Password-Checker/check-password.py to Mask the user input instead Hidden. Also added more smart_subs to strong the password suggestions.