Readpassphrase #692 #156
Readpassphrase #692 #156
Conversation
contrib/win32/win32compat/misc.c
Outdated
|
|
||
| /* prompt user */ | ||
| wchar_t* wtmp = utf8_to_utf16(prompt); |
There was a problem hiding this comment.
the general guidance of openssh code suggest declare variables at the beginning.
contrib/win32/win32compat/misc.c
Outdated
|
|
||
| if (outBufLen == 0) { | ||
| errno = EINVAL; | ||
| return(NULL); |
contrib/win32/win32compat/misc.c
Outdated
| int retr = 0; | ||
| char * | ||
| readpassphrase(const char *prompt, char *outBuf, size_t outBufLen, int flags) { | ||
| int currentIndex = 0; |
There was a problem hiding this comment.
name the variable like current_index for consistency with bsd code
| size_t len = 0; | ||
| int retr = 0; | ||
| char * | ||
| readpassphrase(const char *prompt, char *outBuf, size_t outBufLen, int flags) { |
There was a problem hiding this comment.
I saw RPP_REQUIRE_TTY-- fail if there is no tty at: https://linux.die.net/man/3/readpassphras
is there case where this function is called from virtual tty?
There was a problem hiding this comment.
Unix has the concept of reading from /dev/tty..
if we fail to open /dev/tty and RPP_REQUIRE_TTY is set to false then unix reads the input from STDIN..
since in windows there is no concept of /dev/tty this flag and the corresponding logic is ignored..
There was a problem hiding this comment.
@bagajjal the equivalent of /dev/tty in Windows is the console handle (CreateFile("CONIN")).
We need to map open(/dev/tty) to the above.
contrib/win32/win32compat/misc.c
Outdated
| return(NULL); | ||
| } | ||
|
|
||
| while (_kbhit()) _getch(); |
There was a problem hiding this comment.
What does this line do? flush out previous console input?
There was a problem hiding this comment.
_kbhit() returns a nonzero value, a keystroke is waiting in the buffer..
before we read from user.. we are clearing the keystroke in the waiting buffer.. Ideally this should never happen but the existing code has it and it doesn't harm so I kept it..
There was a problem hiding this comment.
Why should we read on a character basis? Why not call an API that reads an entire line (ReadFile on a console handle with console )?
Checkout console APIs on msdn, there are way to control a console handle for echoing, raw mode, etc that could be used for this purpose. We also use these APIs in enter_raw_mode, exit_raw_mode. check them out.
contrib/win32/win32compat/misc.c
Outdated
|
|
||
| currentIndex--; /* overwrite last character */ | ||
| } | ||
| } else if (ch == '\003') { /* exit on Ctrl+C */ |
There was a problem hiding this comment.
Unix doesn't have special treatment for ESC.. so it will be read as any other input character..
There was a problem hiding this comment.
If we use console APIs, as I described above, there is no need to handle and treat individual key strokes
There was a problem hiding this comment.
as we discussed, we will read character by character to align with the unix code and to treat the control+c for stopping the process.
There was a problem hiding this comment.
I am not familar with key stroke. but some characters like ESC, the logic will fall to this line printf("%c", ch); what will be displayed?
There was a problem hiding this comment.
It doesn't fail.. ESC is printed as [?].. here is the screenshot
|
Can you add a unit test case for open(tty)? |
Fix for 1. fix /. to root 2. remove the leader chars from path "//", "/./" #692
Response to fingerpring verification prompt does not echo Win32-OpenSSH#692
Implement readpassphrase to align with unix implementation.