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 does not support umlauts on Windows #22828

Closed
levrik opened this Issue Nov 21, 2017 · 22 comments

Comments

Projects
None yet
9 participants
@levrik

levrik commented Nov 21, 2017

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

go version go1.9.2 windows/amd64

Does this issue reproduce with the latest release?

yes

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

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\development\go
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0
set CXX=g++
set CGO_ENABLED=1
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

What did you do?

Execute this on Gist on Windows (https://gist.github.com/levrik/99210259065679da20c3c66ef383eea9) and enter some umlauts in the password prompt (e.g. ö, ü or ä).

What did you expect to see?

image
(Screenshot taken on go version go1.9.2 darwin/amd64)

What did you see instead?

pasted image at 2017_11_21 10_39 am

This was discovered when a colleague tried to use a new version of an internal command line tool and it didn't accept the correct password anymore (password was supplied via command line flag before).

@gopherbot gopherbot added this to the Unreleased milestone Nov 21, 2017

@bradfitz

This comment has been minimized.

Member

bradfitz commented Nov 21, 2017

@levrik

This comment has been minimized.

levrik commented Nov 22, 2017

Just looked a bit more into this issue. Basically the problem is that Go expects UTF-8 (or some different encoding) but the Windows console uses Code Page xxx. Which code page exactly seems to depend on the configured language (for example US English uses 437, German uses 850).

Using the following workaround for now. Seems to work fine on German and English Windows systems. But I'm not sure if it's a good idea to always use the US English Code Page 437. It should respect the current code page of the system.

// ReadPassword reads a line of input from a terminal without local echo.  This
// is commonly used for inputting passwords and other sensitive data. The slice
// returned does not include the \n.
func ReadPassword(fd int) ([]byte, error) {
	var st uint32
	if err := windows.GetConsoleMode(windows.Handle(fd), &st); err != nil {
		return nil, err
	}
	old := st

	st &^= (windows.ENABLE_ECHO_INPUT)
	st |= (windows.ENABLE_PROCESSED_INPUT | windows.ENABLE_LINE_INPUT | windows.ENABLE_PROCESSED_OUTPUT)
	if err := windows.SetConsoleMode(windows.Handle(fd), st); err != nil {
		return nil, err
	}

	defer func() {
		windows.SetConsoleMode(windows.Handle(fd), old)
	}()

+	decodedFd := transform.NewReader(passwordReader(fd), charmap.CodePage437.NewDecoder())
-	return readPasswordLine(passwordReader(fd))
+	return readPasswordLine(decodedFd)
}
@mattn

This comment has been minimized.

Member

mattn commented Nov 22, 2017

As far as I can see, x/crypto/ssh/terminal#ReadPassword take a string argument not int.

@mattn

This comment has been minimized.

Member

mattn commented Nov 22, 2017

Oh, sorry. My bad. it is implemented in util_windows.go

@mattn

This comment has been minimized.

Member

mattn commented Nov 22, 2017

This should fix the issue.

diff --git a/ssh/terminal/util_windows.go b/ssh/terminal/util_windows.go
index 60979cc..e7dc952 100644
--- a/ssh/terminal/util_windows.go
+++ b/ssh/terminal/util_windows.go
@@ -17,6 +17,8 @@
 package terminal
 
 import (
+	"os"
+
 	"golang.org/x/sys/windows"
 )
 
@@ -98,5 +100,5 @@ func ReadPassword(fd int) ([]byte, error) {
 		windows.SetConsoleMode(windows.Handle(fd), old)
 	}()
 
-	return readPasswordLine(passwordReader(fd))
+	return readPasswordLine(os.NewFile(uintptr(fd), "sysfile"))
 }
@gopherbot

This comment has been minimized.

gopherbot commented Nov 22, 2017

Change https://golang.org/cl/79335 mentions this issue: ssh/terminal: read password from os.File

@egonelbre

This comment has been minimized.

Contributor

egonelbre commented Nov 22, 2017

EDIT: nevermind didn't see the previous updates.

When you read from stdin it uses default codepage. Printing the byte-sequence verifies this.

GetConsoleCP (https://docs.microsoft.com/en-us/windows/console/getconsolecp) can be used to get the current code page. SetConsoleCP can be used to switch to code page 65001 (utf8) -- but, initial testing only resulted getting 0 for characters. One possibility is to lookup the active code-page and translate after reading the buffer. However, I suspect that Code Page can change during typing.

readPasswordLine seems to need adjusting for multi-byte characters as well.

@forskning

This comment has been minimized.

forskning commented Nov 22, 2017

On a laptop's Nordic keyboard when set on Windows XP to a Danish layout typing in cmd.exe set to Code Page 850 the umlaut "¨" followed by "a" or "o", will render respectively "ä" and "ö", the characters "æ", "ø" , and "å" will also render utilising those settings, but none of the above render using levrik 's code sample.

The code sample doesn't seem to run properly utilising mintty.exe.

@levrik

This comment has been minimized.

levrik commented Nov 22, 2017

EDIT: Nevermind, @forskning was referring to the initial bug report

@forskning I think the German umlauts are included in the English code page so it works fine. As I said, it was just a quick workaround for us (we only have German and English Windows systems).

@bronze1man

This comment has been minimized.

bronze1man commented Nov 23, 2017

I think you guys need a chinese/japanese version win7 to test the real console code page problem.
I think you should use windows api MultiByteToWideChar and GetConsoleCP to solve your problem.
Implement another MultiByteToWideChar in golang will make the binary very large if you include gbk support.

By the way code page 65001 (utf8) on chinese win7 is an useless stuff. It looks like code page 437. But it will crash the process(include ipconfig.exe) if you want to output some chinese char.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Nov 23, 2017

@levrik did you try https://golang.org/cl/79335 ? Does it fixes your problem?
I think it should work for you.

Thank you.

Alex

@forskning

This comment has been minimized.

forskning commented Nov 23, 2017

@alexbrainman I confirm after applying CL 79335 and running cmd.exe set to Code Page 850

that with the keyboard layout set to DE that ö, ü, ä, and ß, all render

and with the keyboard layout set to DA that ä, ö, æ, ø, and å, all render

@levrik

This comment has been minimized.

levrik commented Nov 23, 2017

@alexbrainman sorry, didn't have the time to test yet. Will try it out this evening after work.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Nov 23, 2017

I confirm after applying CL 79335 and running cmd.exe set to Code Page 850

that with the keyboard layout set to DE that ö, ü, ä, and ß, all render

and with the keyboard layout set to DA that ä, ö, æ, ø, and å, all render

Thanks for checking.

Will try it out this evening after work.

Sounds good. I will wait.

Alex

@forskning

This comment has been minimized.

forskning commented Nov 23, 2017

My circa 2009 Danish Windows XP defaults to cp850.

But I think cp850 has since been superseded by cp1252.

The below link is regarding Tcl. (one can search therein for 1252)

http://nurmi-labs.blogspot.com/2017/05/9pvfs.html

FWIW, a more specific title might be the following.

ReadPassword does not support Western European Code Page

@cznic

This comment has been minimized.

Contributor

cznic commented Nov 23, 2017

Genuine question(s): What's the advantage of using code pages on Windows? Why do not people switch to UTF-8? Is that possible on Windows?

@forskning

This comment has been minimized.

forskning commented Nov 23, 2017

aside from some users' preference for cmd.exe, possibly with
variables being SET on the command-line or via a batch file

http://nurmi-labs.blogspot.com/2016/11/git.html

for git-bash.exe I have LANG=da_DK.UTF-8

but as it utilises the Windows console $LANG has no effect on the current code in question

for tcsh.exe, which I added to my system and I run in mintty.exe, I have LANG=en_US

https://en.wikipedia.org/wiki/Code_page_850

above is a wikipedia page including information as

"code page 850 is the primary code page and default OEM code page in many countries, including various English-speaking locales (e.g. in the United Kingdom, Ireland, and Canada)"

and,

"Systems largely replaced code page 850 with, first, Windows-1252 (often mislabeled as ISO-8859-1), and later with UCS-2, and finally with UTF-16."

http://edb-terminaler.blogspot.com/#box-drawing

the above has an example of using box-drawing characters from cp437 or cp850

@bronze1man

This comment has been minimized.

bronze1man commented Nov 23, 2017

here is why using code pages while working with cmd.exe and ipconfig.exe

What's the advantage of using code pages on Windows?

It works.If you do not using code pages in cmd.exe , you can not see chinese char.

Why do not people switch to UTF-8?

code page 65001 (utf8) with cmd.exe on chinese win7 is an useless stuff. It looks like code page 437. But it will crash the process(include ipconfig.exe) if you want to output some chinese char.

Is that possible on Windows?

No. At least I can not find a way to solve this problem by setting some stuff with cmd.exe. It has a stuff called code page 65001 (utf8), but code page 65001 (utf8) is not useful with chinese win7.(it may work with english win7)

@forskning

This comment has been minimized.

forskning commented Nov 23, 2017

There is S. Quinlan's 9pm which contains the 9term.exe dumb terminal.

NB: Some golang code is expected to run in an intelligent terminal, and
this being dated software some Chinese characters would not be expected
to render.

https://9p.io/magic/webls?dir=/sources/extra
9pm051031.zip

https://github.com/nurmi-labs/9pm (copied to GitHub)

http://nurmi-labs.blogspot.com/2015/10/regexp.html (font settings)

And cognisant of different file permissions in a separate environment
subsystem, one could run an xterminal under WSL with VcXsrv.

http://nurmi-labs.blogspot.com/2017/03/p9p.html

NB: I haven't resolved some issues with the German keyboard layout for WeirdX.

@levrik

This comment has been minimized.

levrik commented Nov 23, 2017

@alexbrainman need to move my test to tomorrow. got a bit late today.

@levrik

This comment has been minimized.

levrik commented Nov 24, 2017

@alexbrainman Fix is working for me. Thanks!

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Nov 24, 2017

@alexbrainman Fix is working for me. Thanks!

Thank you for confirming it.

Alex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment