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

Vault panic in github.com/hashicorp/vault/vault.(*Router).MatchingSystemView #7969

Closed
bradbl opened this issue Dec 4, 2019 · 7 comments · Fixed by #7991
Closed

Vault panic in github.com/hashicorp/vault/vault.(*Router).MatchingSystemView #7969

bradbl opened this issue Dec 4, 2019 · 7 comments · Fixed by #7991
Assignees
Labels
bug Used to indicate a potential bug core Issues and Pull-Requests specific to Vault Core
Milestone

Comments

@bradbl
Copy link
Contributor

bradbl commented Dec 4, 2019

Describe the bug
Under some conditions, Vault can panic during the request path. The stack trace is

panic(0x30c2de0, 0x652e3d0)
#011/usr/local/go/src/runtime/panic.go:522 +0x1b5
github.com/hashicorp/vault/vault.(*Router).MatchingSystemView(0xc0007a4460, 0x3dc0800, 0xc0062b4150, 0xc00628ebe0, 0xc, 0xc00628ebe0, 0xc)
#011/go/src/github.com/hashicorp/vault/vault/router.go:422 +0x127
github.com/hashicorp/vault/vault.(*SystemBackend).handleTuneReadCommon(0xc0011636c0, 0x3dc0800, 0xc0062b4150, 0xc00628ebd0, 0xb, 0xc00628ebd0, 0xb, 0x4000000033040e0)
#011/go/src/github.com/hashicorp/vault/vault/logical_system.go:1064 +0xfd
github.com/hashicorp/vault/vault.(*SystemBackend).handleAuthTuneRead(0xc0011636c0, 0x3dc0800, 0xc0062b4150, 0xc0062aa140, 0xc006291140, 0x3010106, 0x1, 0xffffffffffffffff)
#011/go/src/github.com/hashicorp/vault/vault/logical_system.go:1042 +0x1d0
github.com/hashicorp/vault/vendor/github.com/hashicorp/vault/sdk/framework.(*Backend).HandleRequest(0xc002e93860, 0x3dc0800, 0xc0062b4150, 0xc0062aa140, 0x0, 0x0, 0x0)
#011/go/src/github.com/hashicorp/vault/vendor/github.com/hashicorp/vault/sdk/framework/backend.go:256 +0x492
github.com/hashicorp/vault/vault.(*Router).routeCommon(0xc0007a4460, 0x3dc0800, 0xc0062b4150, 0xc0062aa140, 0xc00374ed00, 0x0, 0x0, 0x0, 0x0)
#011/go/src/github.com/hashicorp/vault/vault/router.go:676 +0x919
github.com/hashicorp/vault/vault.(*Router).Route(...)
#011/go/src/github.com/hashicorp/vault/vault/router.go:476
github.com/hashicorp/vault/vault.(*Core).doRouting(0xc0001e0e00, 0x3dc0800, 0xc0062b4150, 0xc0062aa140, 0xc0037ea9f0, 0x0, 0x0)
#011/go/src/github.com/hashicorp/vault/vault/request_handling.go:562 +0x60
github.com/hashicorp/vault/vault.(*Core).handleRequest(0xc0001e0e00, 0x3dc0800, 0xc0062b4150, 0xc0062aa140, 0x0, 0x0, 0x0, 0x0)
#011/go/src/github.com/hashicorp/vault/vault/request_handling.go:703 +0x460
github.com/hashicorp/vault/vault.(*Core).handleCancelableRequest(0xc0001e0e00, 0x3dc0800, 0xc0062b4150, 0xc0008be1c0, 0xc0062aa140, 0xc0008be1c0, 0x3dc0800, 0xc0062b4150)
#011/go/src/github.com/hashicorp/vault/vault/request_handling.go:454 +0xd47
github.com/hashicorp/vault/vault.(*Core).switchedLockHandleRequest(0xc0001e0e00, 0x3dc0800, 0xc006289e90, 0xc0062aa140, 0x6585701, 0x0, 0x0, 0x0)
#011/go/src/github.com/hashicorp/vault/vault/request_handling.go:421 +0x288
github.com/hashicorp/vault/vault.(*Core).HandleRequest(...)
#011/go/src/github.com/hashicorp/vault/vault/request_handling.go:386

It appears the backend field of a routeEntry can be nil, but this isn't checked.

To Reproduce
We observed this by enabling a custom auth plugin and then making a change which removed the plugin binary.

Expected behavior
No panics

Environment:

  • Vault Server Version (retrieve with vault status): v1.3.0
@catsby catsby added the bug Used to indicate a potential bug label Dec 4, 2019
@catsby
Copy link
Contributor

catsby commented Dec 4, 2019

Hey @bradbl - thanks for opening this! In general, we should absolutely check for nil, but I can't speak to this case specifically yet until I dig in.

Before we do dig in, could you elaborate on this part:

and then making a change which removed the plugin binary

Removed it from where, and how did you do this? The details probably don't really impact the code change to fix this, but I'd like to reproduce and verify as well.

Thanks!

@catsby catsby added core Issues and Pull-Requests specific to Vault Core waiting-for-response labels Dec 4, 2019
@bradbl
Copy link
Contributor Author

bradbl commented Dec 4, 2019

The plugin binary is placed into the Vault plugin directory. The Vault binary and plugin binary are bundled into a single Docker image and deployed together. The change here was inadvertently removing the plugin binary from the container but leaving it registered in the plugin catalog.

@calvn calvn self-assigned this Dec 6, 2019
@calvn
Copy link
Contributor

calvn commented Dec 6, 2019

Hi @bradbl! Does the panic occur immediately/shortly after server startup, or does it get triggered when making certain requests (e.g. /sys/auth/:path/tune)?

@briankassouf briankassouf added this to the 1.3.1 milestone Dec 6, 2019
@bradbl
Copy link
Contributor Author

bradbl commented Dec 6, 2019

Hi @bradbl! Does the panic occur immediately/shortly after server startup, or does it get triggered when making certain requests (e.g. /sys/auth/:path/tune)?

I can't say definitively, but the crashes were occurring often enough that it would have been triggered by a common path, like login, instead of tune.

@calvn
Copy link
Contributor

calvn commented Dec 6, 2019

I see, thanks for the info! I brought up tune since that's where the panic stemmed from this in case (handleAuthTuneRead).

@calvn
Copy link
Contributor

calvn commented Dec 6, 2019

@bradbl sorry to keep bugging you, but I was not able to repro the panic. Would you be able to give me a bit more detail around this?

  • What's the setup that you have (single node, HA cluster, replication?)
  • Is the plugin binary removed between restarts, or removed when the cluster is still running and unsealed?

Edit: The Vault version was given in the first post.

@calvn
Copy link
Contributor

calvn commented Dec 9, 2019

I was able to reproduce the panic reported. Currently working on a fix, and will link back to this issue once the PR is out. Thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug core Issues and Pull-Requests specific to Vault Core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants