Custom Windows SSH-Agent Deadlock issue

[gazzenger]

Hi Nathan,

I'm so glad I came across this repo, it does exactly what I need.
I've come across an issue when running winssh-pageant with my own custom SSH-Agent (this is running using the crypto/ssh/agent library)

I run my custom SSH-Agent, which allows connections using named pipes, and when I run winssh-pageant, this allows connections using PuTTY just fine.
But if I'm doing other things on my computer, after a little while, winssh-pageant always seems to crash with a deadlock.

From what I can find out about the error, it appears to be related to a nil pointer in the message loop, in the main() function.

...
for win.GetMessage(&msg, 0, 0, 0) > 0 {
...

A really good explanation of I what I think the problem is, is detailed here

The full error stack for my issue is show below.

$ go run ./
Exception 0xc0000005 0x0 0xc0001d3f30 0x7ff945ce1b4e
PC=0x7ff945ce1b4e

syscall.Syscall6(0x7ff945ce1b10, 0x4, 0xc0001d3f28, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        C:/Program Files/Go/src/runtime/syscall_windows.go:343 +0xf2
github.com/lxn/win.GetMessage(0xc0001d3f28, 0x0, 0x0, 0x1)
        C:/Users/user/go/pkg/mod/github.com/lxn/win@v0.0.0-20210218163916-a377121e959e/user32.go:2642 +0xab
main.main()
        C:/Users/user/Repos/winssh-pageant/main.go:31 +0xf4

goroutine 6 [chan receive, 7 minutes]:
github.com/Microsoft/go-winio.(*win32PipeListener).Accept(0xc00009e0a0, 0x5c, 0xc000043ee8, 0x2c4088, 0xc00009e0a0)
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/pipe.go:483 +0x14c
main.pipeProxy()
        C:/Users/user/Repos/winssh-pageant/pageant.go:188 +0x282
created by main.main
        C:/Users/user/Repos/winssh-pageant/main.go:20 +0x1ff

goroutine 18 [select, 7 minutes]:
github.com/Microsoft/go-winio.(*win32PipeListener).makeConnectedServerPipe(0xc00009e0a0, 0xc00003df68, 0x0, 0x0)
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/pipe.go:365 +0x139
github.com/Microsoft/go-winio.(*win32PipeListener).listenerRoutine(0xc00009e0a0)
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/pipe.go:395 +0x132
created by github.com/Microsoft/go-winio.ListenPipe
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/pipe.go:460 +0x1e9

goroutine 7 [syscall, locked to thread]:
syscall.Syscall6(0x7ff947b45dd0, 0x5, 0x324, 0xc000123f9c, 0xc000123fa0, 0xc000123fa8, 0xffffffff, 0x0, 0x0, 0x0, ...)
        C:/Program Files/Go/src/runtime/syscall_windows.go:343 +0xf2
github.com/Microsoft/go-winio.getQueuedCompletionStatus(0x324, 0xc000123f9c, 0xc000123fa0, 0xc000123fa8, 0xffffffff, 0x0, 0x0)
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/zsyscall_windows.go:107 +0xd5
github.com/Microsoft/go-winio.ioCompletionProcessor(0x324)
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/file.go:167 +0xb6
created by github.com/Microsoft/go-winio.initIo
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/file.go:73 +0x85

goroutine 8 [select, 7 minutes]:
github.com/Microsoft/go-winio.(*win32File).asyncIo(0xc00011c000, 0xc000112c00, 0x0, 0x0, 0x2c33e0, 0x34e218, 0x0, 0x0, 0x0)
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/file.go:194 +0x14b
github.com/Microsoft/go-winio.connectPipe(0xc00011c000, 0x0, 0x0)
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/pipe.go:472 +0xe5
github.com/Microsoft/go-winio.(*win32PipeListener).makeConnectedServerPipe.func1(0xc00004a2a0, 0xc00011c000)
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/pipe.go:362 +0x32
created by github.com/Microsoft/go-winio.(*win32PipeListener).makeConnectedServerPipe
        C:/Users/user/go/pkg/mod/github.com/!microsoft/go-winio@v0.4.14/pipe.go:361 +0xc5
rax     0x0
rbx     0xc0001d3f28
rcx     0x7ff945c91104
rdi     0x0
rsi     0xc0001d3e80
rbp     0xc0001d3e20
rsp     0xfab77ffda0
r8      0xfab77ffd98
r9      0xc0001d3e20
r10     0x0
r11     0x246
r12     0x27cac0
r13     0x0
r14     0x0
r15     0x2030000
rip     0x7ff945ce1b4e
rflags  0x10293
cs      0x33
fs      0x53
gs      0x2b
exit status 2

A few possible solutions I've found are listed below (I'm not sure if they would be a permanent fix, but they at least worked for me)

• One Possible solution I found was from here
  Suggesting to define msg as this

	 msg := (*win.MSG)(unsafe.Pointer(win.GlobalAlloc(0, unsafe.Sizeof(win.MSG{}))))
	 defer win.GlobalFree(win.HGLOBAL(unsafe.Pointer(msg)))
	 for win.GetMessage(msg, 0, 0, 0) > 0 {
	 	win.TranslateMessage(msg)
	 	win.DispatchMessage(msg)
	 }

• Another solution was to place an runtime.LockOSThread() in the main() function, I found this discussed here

func main()
    runtime.LockOSThread()
...

Thanks so much for putting together this repo, it's been really really helpful.
And many thanks for any help.

-Gary

[ndbeals]

Hello Gary! thanks for the helpful bug report, and I'm glad you find this tool useful.

Ohh joy this is going to be a fun one to fix 😅, interfacing with win32 api in go is always a nightmare.

I'll look into it, can I get the code/binary for the custom SSH agent you're using, so that I may replicate this error and then test fixes.

[ndbeals]

Poking through the forks, you've forked this and seemingly implemented a fix already: https://github.com/gazzenger/winssh-pageant

Did you have plans to open a pull request?

[gazzenger]

Awesome, I've gone and created a pull request, #11
There is some other stuff in this, relating more to reading from the SSH list function with bufio - so feel free to have a look.

Unrelated, but if you're interested, I've made some other commits, and moved the pageant.go file into it's own folder. This allows the package to be imported into other peoples code.
gazzenger@b3cebc4

[ndbeals]

Thanks for opening the pull request, I plan to merge it today.

As for making pageant.go it's own pacakge, I'm cool with that, though I personally lack the imagination to see why others may want to use it, but that's on me. I'll end up pulling that one in a little later, feel free to open a PR for it too.

Further discussion relating to PR #11 should be had in that PR, as I've got a few questions for you in it already.

[ndbeals]

PR #11 merged, closing this.