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.

Sign up for GitHub

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

Jump to bottom

Fix a panic in MongoDB backend with concurrent create/revoke #5463

Merged
merged 1 commit into from
Oct 4, 2018
Merged

Fix a panic in MongoDB backend with concurrent create/revoke #5463

merged 1 commit into from
Oct 4, 2018

Conversation

KJTsanaktsidis
Copy link
Contributor

When Vault is concurrently creating and revoking leases for MongoDB
users as part of the database secrets engine, and then loses connection
to MongoDB, it can panic. This occurs because the RevokeUser path does
not lock the mutex, but the CreateUser path does. Both threads of
execution can concurrently decide to call c.session.Close() in
mongodb/connection_producer.go:119, and then mgo panics when the second
close attempt occurs.

The panic seen is as follows:

panic: Session already closed

goroutine 3407 [running]:
github.com/hashicorp/vault/vendor/gopkg.in/mgo%2ev2.(*Session).cluster(0xc420271860, 0x35c9120)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vendor/gopkg.in/mgo.v2/session.go:1624 +0x56
github.com/hashicorp/vault/vendor/gopkg.in/mgo%2ev2.(*Session).acquireSocket(0xc420271860, 0x1, 0x0, 0x0, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vendor/gopkg.in/mgo.v2/session.go:4440 +0x1da
github.com/hashicorp/vault/vendor/gopkg.in/mgo%2ev2.(*Database).Run(0xc420834f88, 0x2e3ca40, 0x376b620, 0x0, 0x0, 0x0, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vendor/gopkg.in/mgo.v2/session.go:649 +0x42
github.com/hashicorp/vault/vendor/gopkg.in/mgo%2ev2.(*Session).Run(0xc420271860, 0x2e3ca40, 0x376b620, 0x0, 0x0, 0x10da23b, 0x8)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vendor/gopkg.in/mgo.v2/session.go:2014 +0x82
github.com/hashicorp/vault/vendor/gopkg.in/mgo%2ev2.(*Session).Ping(0xc420271860, 0xc42050a000, 0xc420050570)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vendor/gopkg.in/mgo.v2/session.go:2043 +0x4b
github.com/hashicorp/vault/plugins/database/mongodb.(*mongoDBConnectionProducer).connectionWithoutLock(0xc4208af700, 0x37912a0, 0xc4207ec330, 0x10, 0x2e65620, 0xc420835001, 0xc4207f3060)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/plugins/database/mongodb/connection_producer.go:114 +0x192
github.com/hashicorp/vault/plugins/database/mongodb.(*mongoDBConnectionProducer).Connection(0xc4208af700, 0x37912a0, 0xc4207ec330, 0xc420050570, 0xc420050500, 0xc420634218, 0xc420634220)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/plugins/database/mongodb/connection_producer.go:146 +0x3f
github.com/hashicorp/vault/plugins/database/mongodb.(*MongoDB).getConnection(0xc4205e66c0, 0x37912a0, 0xc4207ec330, 0x0, 0x0, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/plugins/database/mongodb/mongodb.go:75 +0x46
github.com/hashicorp/vault/plugins/database/mongodb.(*MongoDB).RevokeUser(0xc4205e66c0, 0x37912a0, 0xc4207ec330, 0xc4207eea80, 0x54, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/plugins/database/mongodb/mongodb.go:170 +0xa3
github.com/hashicorp/vault/builtin/logical/database/dbplugin.(*DatabaseErrorSanitizerMiddleware).RevokeUser(0xc42002aab0, 0x37912a0, 0xc4207ec330, 0xc4207eea80, 0x54, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/builtin/logical/database/dbplugin/databasemiddleware.go:236 +0xa3
github.com/hashicorp/vault/builtin/logical/database/dbplugin.(*databaseMetricsMiddleware).RevokeUser(0xc4205e66e0, 0x37912a0, 0xc4207ec330, 0xc4207eea80, 0x54, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/builtin/logical/database/dbplugin/databasemiddleware.go:148 +0x222
github.com/hashicorp/vault/builtin/logical/database.(*databaseBackend).secretCredsRevoke.func1(0x37912a0, 0xc4207ec330, 0xc4205bd040, 0xc4207f3010, 0x0, 0x0, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/builtin/logical/database/secret_creds.go:139 +0x50c
github.com/hashicorp/vault/logical/framework.(*Secret).HandleRevoke(0xc420455800, 0x37912a0, 0xc4207ec330, 0xc4205bd040, 0xc4208f94f8, 0x100b1474f2ea701, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/logical/framework/secret.go:87 +0x9a
github.com/hashicorp/vault/logical/framework.(*Backend).handleRevokeRenew(0xc420593e10, 0x37912a0, 0xc4207ec330, 0xc4205bd040, 0x3232336542ec0996, 0x3666372d30626563, 0xc420be3850)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/logical/framework/backend.go:404 +0x1fb
github.com/hashicorp/vault/logical/framework.(*Backend).HandleRequest(0xc420593e10, 0x37912a0, 0xc4207ec330, 0xc4205bd040, 0x0, 0x0, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/logical/framework/backend.go:171 +0x7ed
github.com/hashicorp/vault/vault.(*Router).routeCommon(0xc420216e00, 0x37912a0, 0xc4207ec330, 0xc4205bd040, 0x1010400, 0x0, 0x3470000, 0x0, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vault/router.go:646 +0x915
github.com/hashicorp/vault/vault.(*Router).Route(0xc420216e00, 0x37912a0, 0xc4207ec330, 0xc4205bd040, 0xc4207ec330, 0xc420be3d18, 0x2a53af1)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vault/router.go:466 +0x4e
github.com/hashicorp/vault/vault.(*ExpirationManager).revokeEntry(0xc42057dc20, 0x37912a0, 0xc4207ec1b0, 0xc4209b7d60, 0x32, 0xc4209b7d60)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vault/expiration.go:1291 +0x187
github.com/hashicorp/vault/vault.(*ExpirationManager).revokeCommon(0xc42057dc20, 0x37912a0, 0xc4207ec1b0, 0xc420c75a00, 0x32, 0xbee5904b24cb0000, 0x0, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vault/expiration.go:620 +0x4c9
github.com/hashicorp/vault/vault.(*ExpirationManager).Revoke(0xc42057dc20, 0x37912a0, 0xc4207ec1b0, 0xc420c75a00, 0x32, 0x0, 0x0)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vault/expiration.go:567 +0x12c
github.com/hashicorp/vault/vault.expireLeaseStrategyRevoke(0x37911e0, 0xc42086ae80, 0xc42057dc20, 0xc4209f9f40)
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vault/expiration.go:119 +0x2f0
github.com/hashicorp/vault/vault.(*ExpirationManager).updatePendingInternal.func1()
	/Users/ktsanaktsidis/Code/go/src/github.com/hashicorp/vault/vault/expiration.go:1255 +0x52
created by time.goFunc
	/Users/ktsanaktsidis/.goenv/versions/1.10.1/src/time/sleep.go:172 +0x44

This can be reproduced by:

  • Starting a vault server -dev
  • Starting a MongoDB server mongod
  • Run the following script:
#!/bin/bash

set -e

export VAULT_ADDR=http://localhost:8200
vault secrets enable database
vault write database/config/mongo-db plugin_name=mongodb-database-plugin allowed_roles="mongo-role" connection_url="mongodb://localhost:27017/admin"
vault write database/roles/mongo-role db_name=mongo-db creation_statements='{ "db": "admin", "roles": [{ "role": "readWrite" }, {"role": "read", "db": "foo"}] }' default_ttl=10s

the_loop() {
    while true; do
        vault read database/creds/mongo-role;
    done;
}

the_loop &
the_loop &
the_loop &
the_loop &
the_loop &
the_loop &
wait
  • Wait 10 seconds or so for the vault server to start revoking the leases, with log messages like 2018-10-04T18:01:42.155+1000 [INFO ] expiration: revoked lease: lease_id=database/creds/mongo-role/42ef5e4a-3218-34c0-52f9-540a09ee8f4a
  • Kill the Mongo server
  • The vault server should panic with the above message or a similar one.

@KJTsanaktsidis
Fix a panic in MongoDB backend with concurrent create/revoke
5ed19ca 
When Vault is concurrently creating and revoking leases for MongoDB
users as part of the database secrets engine, and then loses connection
to MongoDB, it can panic. This occurrs because the RevokeUser path does
_not_ lock the mutex, but the CreateUser path does. Both threads of
execution can concurently decide to call c.session.Close() in
mongodb/connection_producer.go:119, and then mgo panics when the second
close attempt occurs.
@chrishoffman chrishoffman added this to the 0.11.3 milestone Oct 4, 2018
jefferai
jefferai approved these changes Oct 4, 2018
View reviewed changes
Copy link
Member

@jefferai jefferai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jefferai jefferai merged commit cf46961 into hashicorp:master Oct 4, 2018
jefferai pushed a commit that referenced this pull request Oct 8, 2018
@KJTsanaktsidis @jefferai
Fix a panic in MongoDB backend with concurrent create/revoke (#5463)
0312fe4 
When Vault is concurrently creating and revoking leases for MongoDB
users as part of the database secrets engine, and then loses connection
to MongoDB, it can panic. This occurrs because the RevokeUser path does
_not_ lock the mutex, but the CreateUser path does. Both threads of
execution can concurently decide to call c.session.Close() in
mongodb/connection_producer.go:119, and then mgo panics when the second
close attempt occurs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jefferai jefferai jefferai approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.11.3
Development

Successfully merging this pull request may close these issues.
3 participants
@KJTsanaktsidis @jefferai @chrishoffman