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

x/crypto/ssh/terminal: ReadPassword keeps echo disabled when stopped with Ctrl+C #31180

Open
giner opened this issue Apr 1, 2019 · 12 comments
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@giner
Copy link

giner commented Apr 1, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Linux and macOS

What did you do?

Tried to use the following code to read passwords (from https://qiita.com/moutend/items/12d53750363edbbc3d6b):

package main
  
import (
    "fmt"
    "log"
    "syscall"

    "golang.org/x/crypto/ssh/terminal"
)

func main() {
    fmt.Print("Password: ")
    password, err := terminal.ReadPassword(int(syscall.Stdin))
    if err != nil {
        log.Fatal(err)
    } else {
        fmt.Printf("\nYour password is %v\n", string(password))
    }
}

What did you expect to see?

If I press Ctrl+C at the point when I'm supposed to enter password I'm expecting my terminal back working as before running the code.

What did you see instead?

I'm unable to see what I'm typing anymore as echo is kept disabled

@gopherbot gopherbot added this to the Unreleased milestone Apr 1, 2019
@andybons andybons changed the title x/crypto: teminal.ReadPassword keeps echo disabled when stopped with Ctrl+C x/crypto/ssh/terminal: ReadPassword keeps echo disabled when stopped with Ctrl+C Apr 1, 2019
@andybons
Copy link
Member

andybons commented Apr 1, 2019

@FiloSottile @hanwen

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 1, 2019
@hanwen
Copy link
Contributor

hanwen commented Apr 2, 2019

I thought this is normal behavior with other similar programs too.

@agnivade

This comment has been minimized.

@agnivade
Copy link
Contributor

Doesn't seem to be the behavior for sudo su -

@giner
Copy link
Author

giner commented Apr 18, 2019

Removed Linux from the description as I cannot reproduce it on Linux with go1.12.1.

@agnivade
Copy link
Contributor

Actually, I can reproduce on Linux.

@giner
Copy link
Author

giner commented Apr 18, 2019

It seems to depend on the shell. Cannot reproduce with bash (Ubuntu 18.04) but reproduced with dash.

@zladovan
Copy link

I was able to reproduce with bash 5.0.11 on linux 4.19.99-1-MANJARO.

go version go1.13.7 linux/amd64

@norbjd
Copy link

norbjd commented May 22, 2022

I got the exact same issue and found a way to fix it, based on this thread: https://groups.google.com/g/golang-nuts/c/DCl8xUJMJJ0.

import (
	"os"
	"os/signal"
	"syscall"

	"golang.org/x/term"
)

func ReadPassword() (string, error) {
	stdin := int(syscall.Stdin)
	oldState, err := term.GetState(stdin)
	if err != nil {
		return "", err
	}
	defer term.Restore(stdin, oldState)

	sigch := make(chan os.Signal, 1)
	signal.Notify(sigch, os.Interrupt)
	go func() {
		for _ = range sigch {
			term.Restore(stdin, oldState)
			os.Exit(1)
		}
	}()

	password, err := term.ReadPassword(stdin)
	if err != nil {
		return "", err
	}
	return string(password), nil
}

Calling that ReadPassword method and doing a CTRL+C when the prompt appears does restore the old terminal state, meaning echo is not disabled anymore.

Also, based on that PR: https://go-review.googlesource.com/c/crypto/+/258003/, I believe we should use golang.org/x/term instead of golang.org/x/crypto/ssh/terminal.

Hope that helps.

@SadPencil
Copy link

SadPencil commented Aug 10, 2022

I reproduced this bug with ash on OpenWRT.
BusyBox v1.35.0 (2022-06-23 16:55:05 UTC) built-in shell (ash)

@SadPencil
Copy link

SadPencil commented Aug 10, 2022

The issue should be renamed to x/term from x/crypto/ssh/terminal.

@norbjd Personally I think your code is exactly the solution to this issue. How about making a pull request? So that the implementation of readPassword() gets fixed directly.

@norbjd
Copy link

norbjd commented Aug 10, 2022

@SadPencil I'm not sure but I think my code does not work on Windows. I'll take a look 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

8 participants