panic in windowsKeychain.Get

[0x53A]

ref https://github.com/kopia/kopia/issues/732

I got a panic in Kopia, which uses go-keyring:

> "kopia.exe" snapshot create --description "abcdefg" "C:\some\path"
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x38 pc=0x136cc7a]
goroutine 1 [running]:
github.com/zalando/go-keyring.windowsKeychain.Get(0xc000167860, 0x22, 0xc00042f2ee, 0x8, 0x1a4eba0, 0xc00042f2e0, 0x1a3b740, 0xc00050f880)
    /home/travis/gopath/pkg/mod/github.com/zalando/go-keyring@v0.1.0/keyring_windows.go:20 +0x11a
github.com/zalando/go-keyring.Get(...)
    /home/travis/gopath/pkg/mod/github.com/zalando/go-keyring@v0.1.0/keyring.go:32
github.com/kopia/kopia/repo.GetPersistedPassword(0x1d8fbc0, 0xc000534120, 0xc0004be200, 0x39, 0xc00050f7e0, 0x0, 0xc000534150)
    /home/travis/gopath/src/github.com/kopia/kopia/repo/password.go:26 +0x375
github.com/kopia/kopia/cli.getPasswordFromFlags(0x1d8fbc0, 0xc000534120, 0x1d70100, 0xc0000720d0, 0x0, 0x0, 0x1a4cd60)
    /home/travis/gopath/src/github.com/kopia/kopia/cli/password.go:56 +0x145
github.com/kopia/kopia/cli.openRepository(0x1d8fbc0, 0xc000534120, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0)
    /home/travis/gopath/src/github.com/kopia/kopia/cli/config.go:60 +0x165
github.com/kopia/kopia/cli.maybeRepositoryAction.func1.1(0x0, 0x0)
    /home/travis/gopath/src/github.com/kopia/kopia/cli/app.go:141 +0xf0
github.com/kopia/kopia/cli.withProfiling(...)
    /home/travis/gopath/src/github.com/kopia/kopia/cli/profile_disabled.go:7
github.com/kopia/kopia/cli.maybeRepositoryAction.func1(0xc0001a2d80, 0xe8bcaa, 0x1ac45e0)
    /home/travis/gopath/src/github.com/kopia/kopia/cli/app.go:125 +0x64
gopkg.in/alecthomas/kingpin%2ev2.(*actionMixin).applyActions(0xc0004f9518, 0xc0001a2d80, 0x0, 0x0)
    /home/travis/gopath/pkg/mod/gopkg.in/alecthomas/kingpin.v2@v2.2.6/actions.go:28 +0x74
gopkg.in/alecthomas/kingpin%2ev2.(*Application).applyActions(0xc000152870, 0xc0001a2d80, 0x0, 0x0)
    /home/travis/gopath/pkg/mod/gopkg.in/alecthomas/kingpin.v2@v2.2.6/app.go:557 +0xe3
gopkg.in/alecthomas/kingpin%2ev2.(*Application).execute(0xc000152870, 0xc0001a2d80, 0xc00050f620, 0x2, 0x2, 0x0, 0x0, 0x0, 0xc000007dd8)
    /home/travis/gopath/pkg/mod/gopkg.in/alecthomas/kingpin.v2@v2.2.6/app.go:390 +0xa5
gopkg.in/alecthomas/kingpin%2ev2.(*Application).Parse(0xc000152870, 0xc000130010, 0x5, 0x7, 0x1, 0xc000007de0, 0x0, 0x1)
    /home/travis/gopath/pkg/mod/gopkg.in/alecthomas/kingpin.v2@v2.2.6/app.go:222 +0x213
main.main()
    /home/travis/gopath/src/github.com/kopia/kopia/main.go:77 +0x188

It crashes here in line 20: https://github.com/zalando/go-keyring/blob/6905df4fa6203334fbd683b7845b97ab68dd8e29/keyring_windows.go#L11-L20

Now, I don't know much about golang, but one thing that might be related is that go-keyring checks whether err != nil, whereas wincred checks if cred != nil.

In case err was nil and ret was 0 here https://github.com/danieljoos/wincred/blob/78f93c1f8b99b0c2f6e7f3d2bdc4993cf87bddff/sys.go#L70, I believe that it would cause nil, nil to be returned.

Another way nil, nil might be returned was if cred is null: https://github.com/danieljoos/wincred/blob/78f93c1f8b99b0c2f6e7f3d2bdc4993cf87bddff/conversion.go#L91-L94

See https://github.com/danieljoos/wincred/issues/5#issuecomment-238009787.

So I'd change the check from if err != nil to if cred == nil. That wouldn't solve the underlying issue, but might convert the panic into a normal, handleable error.

---

Update:

I took a look at the logs, and, running on two different computers, I had two errors out of 290 runs (0.69%). Each time it was executing a bunch of times successfully in sequence and then failed.

Ref https://github.com/danieljoos/wincred/issues/5#issuecomment-238009787, the CreadRead API seems to be a bit flakey, do you think it might make sense to check if cred, err are nil, nil, and then try a second time, or would that be better handled at the calling application (Kopia in this case)?

---

Update 2:

4 failures in 439 runs (0.91%)

[mislav]

This is being investigated in https://github.com/danieljoos/wincred/issues/32

[williambrode]

Thanks @mislav for the update, I just hit this issue too!