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

Jump to bottom

x/crypto/ssh/terminal: ReadPassword() ignores Return-Key #36609

Closed
johmue opened this issue Jan 16, 2020 · 4 comments
Closed

x/crypto/ssh/terminal: ReadPassword() ignores Return-Key #36609

johmue opened this issue Jan 16, 2020 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@johmue
Copy link

johmue commented Jan 16, 2020 •
edited

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

$ go version
go version go1.13.6 windows/amd64

Does this issue reproduce with the latest release?

no (see comment on latest commit)

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

go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\username\AppData\Local\go-build
set GOENV=C:\Users\username\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\username\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\Username~1\AppData\Local\Temp\go-build859645086=/tmp/go-build -gno-record-gcc-switches

What did you do?

package main

import (
    "log"
    "syscall"
    "golang.org/x/crypto/ssh/terminal"
)

func main() {
	log.Print("Your user password: ")
	bytePassword, err := terminal.ReadPassword(int(syscall.Stdin))
	if err != nil {
		log.Fatalf("unable to read password from cli")
	}
	password := string(bytePassword)
	log.Print(password)
}

What did you expect to see?

If you run it and write hello and press return key it should log hello

What did you see instead?

program ignores return key and will not continue to the next step

Note

Please see my comment on commit golang/crypto@6d4e4cb
@gopherbot gopherbot added this to the Unreleased milestone Jan 16, 2020
johmue referenced this issue in golang/crypto Jan 16, 2020
@mattn @alexbrainman
ssh/terminal: stop using ENABLE_LINE_INPUT
6d4e4cb 
ReadConsole does not read more than 254 bytes when ENABLE_LINE_INPUT is
enabled.

Fixes golang/go#36071

Change-Id: If5c160404b855387a80f1d57638aac3f2db1a097
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/212377
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 16, 2020
@toothrot
Copy link
Contributor

/cc @mattn @alexbrainman @hanwen @FiloSottile and #36071

@gopherbot
Copy link

Change https://golang.org/cl/215417 mentions this issue: ssh/terminal: adjust ReadConsole rules on windows

@alexbrainman
Copy link
Member

@johmue thank you for filling this issue.

This https://golang.org/cl/215417 should fix your problem. Can you try it, please?

Thank you.

Alex

@johmue
Copy link
Author

johmue commented Jan 19, 2020

@alexbrainman I tried with Windows 10 and it seems to work fine now. Thx for taking care of it.

@ncw ncw mentioned this issue Jan 20, 2020
Closed
ncw added a commit to rclone/rclone that referenced this issue Jan 20, 2020
@ncw
Revert "vendor: update x/crypto/ssh - to fix Windows password length …
18d26e2 
…issues fixes #3798"

This turned out to introduce a regression, not being able to press Enter.

See: #3888 and golang/go#36609

This reverts commit 251cfc1.
@ncw ncw mentioned this issue Jan 20, 2020
Closed
FiloSottile added a commit to FiloSottile/age that referenced this issue Feb 20, 2020
@FiloSottile
go.mod: update x/crypto to bring in fix for golang/go#36609
111d0fe 
Updates #78
@FiloSottile FiloSottile mentioned this issue Feb 20, 2020
Closed
sthagen added a commit to sthagen/FiloSottile-age that referenced this issue Feb 21, 2020
@sthagen
Merge pull request #9 from FiloSottile/master
364d46b 
go.mod: update x/crypto to bring in fix for golang/go#36609
This was referenced May 20, 2020
Closed
Merged
Merged
Merged
Merged
gopherbot pushed a commit to golang/term that referenced this issue Oct 16, 2020
@alexbrainman
ssh/terminal: adjust ReadConsole rules on windows
1a268e5 
CL 212377 changed end of input character on windows - from \n to \r.
But CL 212377 did not adjust ReadConsole accordingly. For example,
after CL 212377 \n was still used to end of password processing,
and \r was ignored.

This CL swaps these rules - \r is now used to end password processing,
and \n are ignored. The change only affects windows, all non windows
code should work as before.

This CL also adjusts TestReadPasswordLineEnd to fit new rules.

Fixes golang/go#36609

Change-Id: I027bf80d10e7d4d4b17ff0264935d14b8bea9097
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/215417
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
@golang golang locked and limited conversation to collaborators Feb 7, 2021
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
@alexbrainman
ssh/terminal: adjust ReadConsole rules on windows
5a18973 
CL 212377 changed end of input character on windows - from \n to \r.
But CL 212377 did not adjust ReadConsole accordingly. For example,
after CL 212377 \n was still used to end of password processing,
and \r was ignored.

This CL swaps these rules - \r is now used to end password processing,
and \n are ignored. The change only affects windows, all non windows
code should work as before.

This CL also adjusts TestReadPasswordLineEnd to fit new rules.

Fixes golang/go#36609

Change-Id: I027bf80d10e7d4d4b17ff0264935d14b8bea9097
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/215417
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@alexbrainman
ssh/terminal: adjust ReadConsole rules on windows
1ddb58b 
CL 212377 changed end of input character on windows - from \n to \r.
But CL 212377 did not adjust ReadConsole accordingly. For example,
after CL 212377 \n was still used to end of password processing,
and \r was ignored.

This CL swaps these rules - \r is now used to end password processing,
and \n are ignored. The change only affects windows, all non windows
code should work as before.

This CL also adjusts TestReadPasswordLineEnd to fit new rules.

Fixes golang/go#36609

Change-Id: I027bf80d10e7d4d4b17ff0264935d14b8bea9097
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/215417
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@alexbrainman
ssh/terminal: adjust ReadConsole rules on windows
305d4b8 
CL 212377 changed end of input character on windows - from \n to \r.
But CL 212377 did not adjust ReadConsole accordingly. For example,
after CL 212377 \n was still used to end of password processing,
and \r was ignored.

This CL swaps these rules - \r is now used to end password processing,
and \n are ignored. The change only affects windows, all non windows
code should work as before.

This CL also adjusts TestReadPasswordLineEnd to fit new rules.

Fixes golang/go#36609

Change-Id: I027bf80d10e7d4d4b17ff0264935d14b8bea9097
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/215417
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@alexbrainman
ssh/terminal: adjust ReadConsole rules on windows
cc25c71 
CL 212377 changed end of input character on windows - from \n to \r.
But CL 212377 did not adjust ReadConsole accordingly. For example,
after CL 212377 \n was still used to end of password processing,
and \r was ignored.

This CL swaps these rules - \r is now used to end password processing,
and \n are ignored. The change only affects windows, all non windows
code should work as before.

This CL also adjusts TestReadPasswordLineEnd to fit new rules.

Fixes golang/go#36609

Change-Id: I027bf80d10e7d4d4b17ff0264935d14b8bea9097
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/215417
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@alexbrainman
ssh/terminal: adjust ReadConsole rules on windows
bc87ff2 
CL 212377 changed end of input character on windows - from \n to \r.
But CL 212377 did not adjust ReadConsole accordingly. For example,
after CL 212377 \n was still used to end of password processing,
and \r was ignored.

This CL swaps these rules - \r is now used to end password processing,
and \n are ignored. The change only affects windows, all non windows
code should work as before.

This CL also adjusts TestReadPasswordLineEnd to fit new rules.

Fixes golang/go#36609

Change-Id: I027bf80d10e7d4d4b17ff0264935d14b8bea9097
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/215417
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@toothrot @johmue @gopherbot @alexbrainman