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

go panic when using JWT token and root role #9695

Closed
jxuan opened this issue May 4, 2018 · 12 comments
Closed

go panic when using JWT token and root role #9695

jxuan opened this issue May 4, 2018 · 12 comments
Assignees

Comments

@jxuan
Copy link

jxuan commented May 4, 2018

When I use the latest master code to test jwt token, sometimes ETCD server stops with the following go panic. Note that I am using a test account with root role.

`panic: interface conversion: auth.TokenProvider is *auth.tokenJWT, not *auth.tokenSimple

goroutine 610 [running]:
github.com/coreos/etcd/auth.(*authStore).WithRoot(0xc420373bc0, 0x4d5d760, 0xc42036e7c0, 0x4c505e4, 0xc420f64254)
/Users/jiangxuan/gopath/src/github.com/coreos/etcd/auth/store.go:1277 +0x82a
github.com/coreos/etcd/etcdserver.(*EtcdServer).run.func9.1()
/Users/jiangxuan/gopath/src/github.com/coreos/etcd/etcdserver/server.go:989 +0x8c
github.com/coreos/etcd/etcdserver.(*EtcdServer).goAttach.func1(0xc420112000, 0xc420bc9d10)
/Users/jiangxuan/gopath/src/github.com/coreos/etcd/etcdserver/server.go:2371 +0x55
created by github.com/coreos/etcd/etcdserver.(*EtcdServer).goAttach
/Users/jiangxuan/gopath/src/github.com/coreos/etcd/etcdserver/server.go:2369 +0x1a8`

It is caused by hardcoding tokenSimple in auth/store.go line 1277:
1277 if ts := as.tokenProvider.(*tokenSimple); ts != nil {
It should be written for simple token as well as jwt token, instead of being hardcoded for simpleToken only.

@jxuan jxuan changed the title go panic when using JWT token and root account go panic when using JWT token and root role May 4, 2018
@jxuan
Copy link
Author

jxuan commented May 4, 2018

etcd --version
etcd Version: 3.3.0+git
Git SHA: Not provided (use ./build instead of go build)
Go Version: go1.10.2
Go OS/Arch: darwin/amd64

@hexfusion
Copy link
Contributor

@jxuan going to take a stab at this unless you already have it knocked out.

@hexfusion
Copy link
Contributor

@jxuan could you please try #9698 I am curious if this helps your problem.

@hexfusion
Copy link
Contributor

@jxuan ping, would you mind sharing your etcd startup config so I can test against this PR, please. In general, can you outline steps to reproduce the panic? Thanks!

@hexfusion hexfusion self-assigned this May 9, 2018
@jxuan
Copy link
Author

jxuan commented May 9, 2018

Sorry for the late reply. Below is my setup:
./etcd_mac
--name etcd-a-0
--debug
--election-timeout 1500
--heartbeat-interval 300
--initial-advertise-peer-urls "http://127.0.0.1:2380"
--initial-cluster-token etcd-stg-a-cluster
--initial-cluster-state new
--advertise-client-urls "http://127.0.0.1:2379"
--listen-client-urls "http://127.0.0.1:2379"
--listen-peer-urls "http://127.0.0.1:2380"
--bcrypt-cost 4
--auth-token jwt,pub-key=mesh_test_pubkey.pem,priv-key=mesh_test.pem,sign-method=RS256

@jxuan
Copy link
Author

jxuan commented May 9, 2018

To reproduce this error, I have a client connecting to my etcd server host (one host), then I restart the etcd server host. The client will re-connect to the server again. This is when it triggers the go panic. Hope it helps.

@hexfusion
Copy link
Contributor

@jxuan thank you for the details!

@jxuan
Copy link
Author

jxuan commented May 15, 2018

Any update on the fix?

@hexfusion
Copy link
Contributor

@jxuan sorry for the delay didn't get much done this weekend. Will hop on this Tuesday should have an update soon.

@hexfusion
Copy link
Contributor

@jxuan please try my PR #9698? I can easily reproduce the panic with the following steps built against the master.

  • start etcd with flags above
  • enable auth
  • grant a lease a few times ie etcdctl --user="root:toor" lease grant 3

With the fix I can not make this happen, please try to break :).

@hexfusion
Copy link
Contributor

@jxuan have you had a chance to test this? Would like for you to sign off on this before we merge.

@hexfusion
Copy link
Contributor

Fixed by #9698

@jxuan please let us know if you have any issues.

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

No branches or pull requests

3 participants