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

Allow Use Of Non-ASCII Character In SSH Client Passwords #322

Merged
merged 3 commits into from
Dec 12, 2019

Conversation

NoMoreFood
Copy link

  • Modified readpassphrase() to read unicode characters and return utf8-encoded password strings to allow use of non-ascii characters.

Addresses: PowerShell/Win32-OpenSSH#1178
May be related to: PowerShell/Win32-OpenSSH#1084

- Modified readpassphrase() to read unicode characters and return utf8-encoded password strings to allow use of non-ascii characters.
Copy link

@manojampalam manojampalam left a comment

Choose a reason for hiding this comment

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

I've been longing to rewrite this logic for a while.
My proposal is to do the following:

  1. Save current console settings
  2. Modify console settings to echo off and line mode (SetConsoleMode(ENABLE_LINE_INPUT)
  3. ReadFile(CONIN)
  4. Revert console settings

Thoughts?

@NoMoreFood
Copy link
Author

NoMoreFood commented Jun 6, 2018

Topically, I think that could work although messing with console mode can sometimes have some unforeseen consequences. I haven't written many console programs that dynamically ask for input so I don't have a strong knowledge base about what those might be.

The only immediate questions/concerns that come to mind are:

  • Is there a possibility now or in the future that readpassphrase() could be asynchronously interrupted/ terminated thereby not allowing us to restore console settings? Update: Doesn't look like this would be a concern.
  • Do we care about maintaining parity with the current code base by offering the functionality to console console behavior by manipulating/recompiling with altered RPP* constants?
  • Does buffering a sensitive piece of data prior to transmission create or exacerbate any potential security concerns in terms of allowing other user mode program to capture the data?
  • How does ReadFile(CONIO) respond to console input redirection scenarios? Would it behave differently from the current character-by-character processing using _getch()/_getwch()? For example, right now I can do things like echo ls / | ssh host -l user and it will ask for keyboard input for password and subsequently run ls / on the target system.

@manojampalam
Copy link

Hmm. Right. My proposal would only support a specific use case of readpassphrase and has unintended consequences. Let me review your changes.

@NoMoreFood
Copy link
Author

@manojampalam I hadn't noticed this before, but apparently when compiling under Visual Studio 2017 vice Visual Studio 2015 (and/or against the latest Windows 10 SDK), the current readpassphrase() truncates its output to the first character. This change also happens to address that behavior and, other than setting ENABLE_PKCS11 as a processor definition, is literally the only other thing I had to do to get PKCS11 support working. Any chance to review these changes?

@manojampalam
Copy link

Apologies for the delay. I'm still looking into Unix implementation and checking how best we can support all the different modes.

@NoMoreFood
Copy link
Author

@manojampalam Thoughts on moving forward with this?

@yan4321 yan4321 mentioned this pull request Dec 18, 2018
@WSLUser
Copy link

WSLUser commented Nov 15, 2019

Can we merge this in with as part of the 8.1 release? (rebase needed first)

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

Successfully merging this pull request may close these issues.

4 participants