keybase / keybase/keybase-issues

Keybase service panics on bad key import

Open
#2,301 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
No language data
Stars
899
Forks
40
PR merge metrics
No merged PRs in 30d

Description

Tested on the latest .deb and on HEAD with Ubuntu 16.04 built with the production build tag.

When trying to run `keybase pgp import` with a key coming in from stdin, the keybase client just returns an EOF error message. I decided to try building the client from source myself and ran the following to start the service:

`$ keybase -H ~/projects/keybase/dev -d service`

After running `keybase -d pgp import` with a key from stdin, I got an error message from the client simply saying "[ERRO keybase standard.go:197] 022 EOF". Looking at the service logs shows that this is due to a nil pointer somewhere during the PGP key import engine initialization, specifically this function:

```
func (s StringWarning) Warn(g *GlobalContext) {
g.Log.Warning(string(s))
}
```

It looks like the GlobalContext isn't being set for some reason. Replacing `g.Log.Warning(string(s))` with `panic(string(s))` shows a more useful error message: `panic: Bad subkey: openpgp: invalid data: subkey signature invalid: openpgp: invalid signature: hash tag doesn't match`. This might be a bug in the Go OpenPGP library - I was using a key I generated and updated the expiry on, which seems to have caused problems for other people - but the service panic obscures the error output from that.

It seems like the correct behavior should be to at the very least return and log the underlying error rather than panic, though I don't know enough about the codebase to be able to patch it myself.

Service panic output:

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x661bf6]

goroutine 84 [running]:
panic(0x115aa00, 0xc82000e130)
/home/john/go/src/runtime/panic.go:481 +0x3e6
github.com/keybase/client/go/libkb.StringWarning.Warn(0xc8202b27e0, 0x6f, 0x0)
/home/john/devel/go/src/github.com/keybase/client/go/libkb/warning.go:26 +0x66
github.com/keybase/client/go/libkb.(*StringWarning).Warn(0xc82024e7d0, 0x0)
:3406 +0xa2
github.com/keybase/client/go/libkb.Warnings.Warn(0xc82024e7e0, 0x1, 0x1, 0x0)
/home/john/devel/go/src/github.com/keybase/client/go/libkb/warning.go:54 +0x8b
github.com/keybase/client/go/engine.NewPGPKeyImportEngineFromBytes(0xc820168000, 0x1621, 0x1621, 0x7f02b1ecb300, 0x0, 0x0, 0x0, 0x0)
/home/john/devel/go/src/github.com/keybase/client/go/engine/pgp_import_key.go:58 +0x12b
github.com/keybase/client/go/service.(*PGPHandler).PGPImport(0xc820290870, 0x7f02b1f90d50, 0xc82015a680, 0x0, 0xc820168000, 0x1621, 0x1621, 0x0, 0x0, 0x0)
/home/john/devel/go/src/github.com/keybase/client/go/service/pgp.go:165 +0x2a2
github.com/keybase/client/go/protocol.PGPProtocol.func12(0x7f02b1f90d50, 0xc82015a680, 0xe5f1e0, 0xc8200e7440, 0x0, 0x0, 0x0, 0x0)
/home/john/devel/go/src/github.com/keybase/client/go/protocol/pgp.go:268 +0x189
github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc.(*callRequest).Serve(0xc8203985a0, 0x7f02b1f096d8, 0xc82038c900, 0xc8200e74c0, 0x161fe00)
/home/john/devel/go/src/github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc/request.go:76 +0x24a
github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc.(*receiveHandler).handleReceiveDispatch.func1(0x7f02b1f91638, 0xc8203985a0, 0xc8201f47e0, 0xc8200e74c0, 0x161fe00)
/home/john/devel/go/src/github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc/receiver.go:121 +0x57
created by github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc.(*receiveHandler).handleReceiveDispatch
/home/john/devel/go/src/github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc/receiver.go:123 +0x3cf
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with go/libkb/warning.go and go/engine/pgp_import_key.go, then trace the call from go/service/pgp.go. Reproduce the failure with `keybase -d pgp import` using the reported invalid key and inspect how the nil GlobalContext is handled. Done means the service no longer panics and the underlying import error is returned or logged.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, cryptography
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.