go: x/crypto/ssh/terminal: ReadPassword doesn't work on redirected stdin giving inappropriate ioctl for device

@ncw the only solution I know is to open the tty and read from it, something like:

func readPassword(prompt string) ([]byte, error) {
    fmt.Fprint(os.Stderr, prompt)
    var fd int
    if terminal.IsTerminal(syscall.Stdin) {
        fd = syscall.Stdin
    } else {
        tty, err := os.Open("/dev/tty")
        if err != nil {
            return nil, errors.Wrap(err, "error allocating terminal")
        }
        defer tty.Close()
        fd = int(tty.Fd())
    }

    pass, err := terminal.ReadPassword(fd)
    fmt.Fprintln(os.Stderr)
    return pass, err
}

I end up to this situation too. My specific case is exactly like #2. Here is what I came up with:

func readPassword(prompt string) ([]byte, error) {
	fmt.Fprint(os.Stderr, prompt)
	var fd int
	var pass []byte
	if terminal.IsTerminal(syscall.Stdin) {
		fd = syscall.Stdin
		inputPass, err := terminal.ReadPassword(fd)
		if err != nil {
			return nil, err
		}
		pass = inputPass
	} else {
		reader := bufio.NewReader(os.Stdin)
		s, err := reader.ReadString('\n')
		if err != nil {
			return nil, err
		}
		pass = []byte(s)
	}

	return pass, nil
}

@ncw, at first I didn’t like the idea of changing terminal.ReadPassword, because users have different needs and I like the standard function to be as simple as possible, but now I realize that the code to handle use case #2 is already here, and the change would be trivial.

Ok, I would expect a function called terminal.ReadPassword to only work on a terminal, but the bad thing is that currently, use case #2 requires users to essentially duplicate the readPasswordLine function (possibly in an unsafe way with bufio).

The code could be changed from this:

termios, err := unix.IoctlGetTermios(fd, ioctlReadTermios)
if err != nil {
	return nil, err
}
// set up the terminal
// do the read

to this:

termios, err := unix.IoctlGetTermios(fd, ioctlReadTermios)
if err == nil {
	// set up the terminal
}
// do the read

Users that only want a terminal could still use the IsTerminal check or open /dev/tty directly.

Possible problems:

• it would be nice to have a (generous) limit on password length, to avoid filling up the memory by accident when reading from non-terminals;
• it may be safer to distinguish between the ENOTTY error and other errors (in the latter case it should fail, to avoid the risk of accidentally displaying the password).

@truongnmt The bufio.Reader tries to fill its own buffer but doesn’t wait for it to be full, so if it works for you it is probably because the timing is right (no more than the first password is available at the time buffering takes place).

For example:

$ echo -en 'password1\npassword2\n' | yourprogram # probably fails
$ cat passwordfile
password1
password2
$ cat passwordfile | yourprogram # probably fails
$ { echo password1; sleep 1; echo password2; } | yourprogram # probably works

Here is a playground demonstration: https://play.golang.org/p/uxSMCw7anP4

Notice that terminal.ReadPassword itself also avoids buffering. In fact its implementation is similar to mine, see here.

@truongnmt your solution may be fine for your use case, but be aware that the buffering will generally prevent further use of standard input.

For example suppose that you need to read two passwords. Calling your readPassword function twice won’t work, because the bufio.Reader of the first call will probably buffer more than the first password, and then such buffer is discarded and never seen by the second call. If it’s not a second password, it may be some other kind of input.

Here’s a variation that doesn’t have the buffering problem:

func readPassword(prompt string) (pw []byte, err error) {
	fd := int(os.Stdin.Fd())
	if terminal.IsTerminal(fd) {
		fmt.Fprint(os.Stderr, prompt)
		pw, err = terminal.ReadPassword(fd)
		fmt.Fprintln(os.Stderr)
		return
	}

	var b [1]byte
	for {
		n, err := os.Stdin.Read(b[:])
		// terminal.ReadPassword discards any '\r', so we do the same
		if n > 0 && b[0] != '\r' {
			if b[0] == '\n' {
				return pw, nil
			}
			pw = append(pw, b[0])
			// limit size, so that a wrong input won't fill up the memory
			if len(pw) > 1024 {
				err = errors.New("password too long")
			}
		}
		if err != nil {
			// terminal.ReadPassword accepts EOF-terminated passwords
			// if non-empty, so we do the same
			if err == io.EOF && len(pw) > 0 {
				err = nil
			}
			return pw, err
		}
	}
}

@truongnmt you also forgot to trim the final \n from the reader.ReadString output (notice that terminal.ReadPassword does not include the \n in its output).