This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: getpass.getpass on Windows fallback detection is incomplete
Type: behavior Stage: needs patch
Components: Library (Lib), Windows Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: eryksun, matejcik, paul.moore, steve.dower, terry.reedy, tim.golden, zach.ware
Priority: normal Keywords:

Created on 2021-07-28 13:08 by matejcik, last changed 2022-04-11 14:59 by admin.

Messages (11)
msg398370 - (view) Author: jan matejek (matejcik) * Date: 2021-07-28 13:08
The fallback detection for `win_getpass` checks that `sys.stdin` is different from `sys.__stdin__`. If yes, it assumes that it's incapable of disabling echo, and calls `default_getpass` which reads from stdin.

If they are the same object, it assumes it's in a terminal and uses `msvcrt` to read characters.

I was not able to find any rationale for this check, it seems to have been introduced, oh wow, in 2001, to fix something IDLE-related.

Anyway, the check is trivially incorrect. It fails when such script is launched from `subprocess.Popen(stdin=PIPE)`, where both `sys.stdin` and `sys.__stdin__` are the same instance of `TextIOWrapper`. Same actually happens in MinTTY (Git Bash etc.) which is not a true terminal as far as I was able to find out?

It seems that a better check would be, e.g., `sys.stdin.isatty()`, which correctly returns `False` both in subprocesses and in MinTTY.
msg398372 - (view) Author: jan matejek (matejcik) * Date: 2021-07-28 13:12
...this is a problem because:

When the check incorrectly infers that it can use `msvcrt` while its stdin is a pipe, the calls to `putwch` and `getwch` are going into the void and the program effectively freezes waiting for input that never comes.

See also:
https://stackoverflow.com/questions/49858821/python-getpass-doesnt-work-on-windows-git-bash-mingw64/54046572
https://github.com/ipython/ipython/issues/854
msg398374 - (view) Author: jan matejek (matejcik) * Date: 2021-07-28 13:14
For that matter, in standard Windows Command Prompt `sys.stdin` and `sys.__stdin__` are also identical, but `isatty()` reports True.

I suspect is that the code has drifted and `sys.stdin` is _always_ identical to `sys.__stdin__` ?
msg398399 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-07-28 15:52
> When the check incorrectly infers that it can use `msvcrt` while 
> its stdin is a pipe, the calls to `putwch` and `getwch` are going 
> into the void and the program effectively freezes waiting for 
> input that never comes.

The C runtime's getwch() and putwch() functions use the console I/O files "CONIN$" and "CONOUT$". If "CONIN$" can't be opened because the process has no console, then getwch() fails and returns U+FFFF. But mintty does have a console, albeit with a hidden window, which gets inherited by bash and child processes. So getwch() ends up waiting for a key to be pressed in an inaccessible, hidden window.

It should suffice to check that sys.stdin isn't None and that sys.stdin.isatty() is True, since it's reasonable to assume that the console has a visible window in this case. Currently this isn't a bulletproof check, however, because the Windows C runtime doesn't implement isatty() correctly. It returns true for any character device, which includes the common case of redirecting standard I/O to the "NUL" device. io.FileIO.isatty() could be updated to return True only if both C isatty() is true and GetConsoleMode() succeeds. This would filter out false positives for non-console character devices, particularly "NUL".
msg398431 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-07-28 21:51
We could also provide a better check in WindowsConsoleIO.isatty, since that should have already handled most of the previous checks (I wouldn't want to do a typecheck in getpass, though). But yeah, this seems like "sys.stdin and sys.stdin.isatty()" is the right condition.
msg398509 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-07-29 19:14
It should be noted that changing win_getpass() to check sys.stdin.isatty() makes it inconsistent with unix_getpass(). The latter tries to open "/dev/tty" regardless of sys.stdin. To be consistent, win_getpass() would be implemented to call fallback_getpass() only if "CONIN$" is inaccessible. 

We'd still have the problem that's reported here. A classic console session (not ConPTY) can have no window, or an invisible window, and thus no way for a user to enter text. I don't see any reports that reading from "/dev/tty" makes getpass() wait forever in POSIX, so this situation is probably unique to Windows, or at least it's far more likely in Windows, which may justify introducing an inconsistency.
msg398510 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-07-29 19:15
> We could also provide a better check in WindowsConsoleIO.isatty, 

For isatty(), my concern is a false positive if the file is the "NUL" device, which should be an io.FileIO object. I suppose checking the file type in io._WindowsConsoleIO.isatty() could be useful in case the file descriptor gets reassigned to a non-console file (e.g. via os.close or os.dup2). 

I'd prefer to implement the check in a common _Py_isatty() function that supports the optional errno values EBADF and ENOTTY [1]. For example:

    int
    _Py_isatty(int fd)
    {
        DWORD mode;
        HANDLE handle = _Py_get_osfhandle_noraise(fd);

        if (handle == INVALID_HANDLE_VALUE) {
            errno = EBADF;
            return 0;
        }

        switch (GetFileType(handle)) {
        case FILE_TYPE_CHAR:
            break;
        case FILE_TYPE_DISK:
        case FILE_TYPE_PIPE:
            errno = ENOTTY;
            return 0;
        default:
            errno = EBADF;
            return 0;
        }
        
        if (!GetConsoleMode(handle, &mode) && 
              GetLastError() != ERROR_ACCESS_DENIED) {
            errno = ENOTTY;
            return 0;
        }
        
        return 1;
    }

---

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/isatty.html
msg398511 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-07-29 19:32
> I suppose checking the file type in io._WindowsConsoleIO.isatty() could be useful in case the file descriptor gets reassigned to a non-console file (e.g. via os.close or os.dup2).

Pretty sure we currently don't support these anyway. WindowsConsoleIO doesn't actually use the standard file descriptors for stdin/out/err - it creates new ones based on the real handles just in case a caller requests them. In my opinion, correctness of "write to console" outweighs correctness of "manually close the console FD and wait for a new one with the same number to be opened" ;)

If we have WindowsConsoleIO as the type, it shouldn't be possible for it to refer to any kind of file other than the console or it would be a FileIO. If that's no longer true, we should fix that first. WindowsConsoleIO.isatty ought to be "return True".
msg398519 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-07-29 21:16
> WindowsConsoleIO doesn't actually use the standard file descriptors 
> for stdin/out/err

To resolve bpo-30555, _WindowsConsoleIO was changed to eliminate self->handle and get the underlying handle dynamically via _get_osfhandle(). It's thus susceptible to close() and dup2() calls in Python or a C library. This consequence was noted. The alternative was to duplicate the OS handle that self->fd wraps, isolating it from self->handle. But then self->fd could get closed, or duped to another file. Either way there are file and file-type sync problems. It's not going to work in the seamless way that people are used to in POSIX, which only uses a single io.FileIO type. I think a better resolution will be possible in the context of your side project, if you're still working on it...
msg398962 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-08-05 04:12
The rationale for the __stdin__ and stdin check is this:  When python starts, both are usually set an io.TextIOWrapper wrapping a system console. (At least on Windows, both may instead be None instead.)  __stdin__ should never be altered.  Getpass.getpass knows how to turn off echo on a system terminal.  So if stdin is not None and is not __stdin__, stdin is very likely not a system terminal, and if so, getpass does not know to turn off echo, if indeed this is possible.

As far as I know, the tk and hence tkinter text widget do not come with a option to not display key presses that are stored in the widget.  An application would have to intercept keypresses and store them elsewhere.
But this is quite different issue.

This issue is not about echo, but about interactivity.  Testing that with isatty is *NOT* a substitute for testing whether an interactive device is the system terminal or not.  (IDLE's stdio connected to Shell passes isatty.)  Any new test would have to added without deleting the current test.
msg399004 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-08-05 13:22
The `sys.stdin is not sys.__stdin__` check is not relevant information. We need to know whether msvcrt.getwch() works and that the user should be able to type the password in the console window. sys.__stdin__ could be a file object for the NUL device, a pipe, or a file.

Also, this check has never been implemented in POSIX. If I run `python -m idlelib` in Linux, getpass() still reads the password from the terminal instead of sys.stdin.

> IDLE's stdio connected to Shell passes isatty

IOBase.isatty() is more abstract than I expected, since it can be true for any stream that's "interactive". While FileIO.isatty() and os.isatty() are definitely wrong in Windows (i.e. they should not be true for NUL or a serial/parallel port), I see now that fixing isatty() doesn't solve the problem. 

We need to directly check whether sys.stdin is a console. If so, we can reasonably assume that there's a visible, accessible console window. The check would be something like:

    try:
        _winapi.GetConsoleMode(msvcrt.get_osfhandle(sys.stdin.fileno()))
    except (AttributeError, OSError):
        return fallback_getpass(prompt, stream)

_winapi.GetConsoleMode() would need to be implemented.

This is inconsistent with Unix since it's not trying to open "CONIN$". But relying on the latter is problematic in Windows since it succeeds in any process that's attached to a console session, even if the console window is hidden or never created (i.e. CREATE_NO_WINDOW).
History
Date User Action Args
2022-04-11 14:59:48adminsetgithub: 88925
2021-08-05 13:22:40eryksunsetmessages: + msg399004
2021-08-05 04:13:06terry.reedysettitle: getpass.getpass on Windows fallback detection is bad -> getpass.getpass on Windows fallback detection is incomplete
2021-08-05 04:12:44terry.reedysetnosy: + terry.reedy
messages: + msg398962
2021-07-29 21:16:25eryksunsetmessages: + msg398519
2021-07-29 19:32:04steve.dowersetmessages: + msg398511
2021-07-29 19:15:41eryksunsetmessages: + msg398510
2021-07-29 19:14:24eryksunsetmessages: + msg398509
2021-07-28 21:51:41steve.dowersetmessages: + msg398431
2021-07-28 19:16:39zach.waresetnosy: + paul.moore, tim.golden, zach.ware, steve.dower
stage: needs patch

components: + Windows
versions: + Python 3.10, Python 3.11
2021-07-28 15:52:59eryksunsetnosy: + eryksun
messages: + msg398399
2021-07-28 13:23:04matejciksetversions: + Python 3.9
2021-07-28 13:22:45matejciksetversions: - Python 3.6, Python 3.7, Python 3.8, Python 3.9, Python 3.10, Python 3.11
2021-07-28 13:14:28matejciksetmessages: + msg398374
2021-07-28 13:12:11matejciksetmessages: + msg398372
2021-07-28 13:08:08matejcikcreate