luxolus/open-nomad
2
0
Fork 0
Code Issues 1 Pull requests Packages Releases Wiki Activity

Fix race condition with Deriving vault tokens

Browse source 
This PR fixes a race condition in which the client was not locked while
deriving Vault tokens. This allowed the token to be set which would
cause subsequent Vault requests to fail with permission denied because
the incorrect Vault token was being used.

Further this PR makes the unsetting and unlocking of the client atomic
to avoid an even harder to hit race condition (not sure it was ever hit
but was still incorrect).
This commit is contained in:
Alex Dadgar 2017-02-01 16:25:59 -08:00
parent a26482a57a
commit 9987a235a5
1 changed files with 17 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
client/vaultclient/vaultclient.go
View file

@ -224,6 +224,13 @@ func (c *vaultClient) Stop() {
close(c.stopCh)
}
// unlockAndUnset is used to unset the vault token on the client and release the
// lock. Helper method for deferring a call that does both.
func (c *vaultClient) unlockAndUnset() {
c.client.SetToken("")
c.lock.Unlock()
}
// DeriveToken takes in an allocation and a set of tasks and for each of the
// task, it derives a vault token from nomad server and unwraps it using vault.
// The return value is a map containing all the unwrapped tokens indexed by the
@ -236,6 +243,12 @@ func (c *vaultClient) DeriveToken(alloc *structs.Allocation, taskNames []string)
return nil, fmt.Errorf("vault client is not running")
}
c.lock.Lock()
defer c.unlockAndUnset()
// Use the token supplied to interact with vault
c.client.SetToken("")
return c.tokenDeriver(alloc, taskNames, c.client)
}
@ -253,14 +266,11 @@ func (c *vaultClient) GetConsulACL(token, path string) (*vaultapi.Secret, error)
}
c.lock.Lock()
defer c.lock.Unlock()
defer c.unlockAndUnset()
// Use the token supplied to interact with vault
c.client.SetToken(token)
// Reset the token before returning
defer c.client.SetToken("")
// Read the consul ACL token and return the secret directly
return c.client.Logical().Read(path)
}
@ -377,9 +387,6 @@ func (c *vaultClient) renew(req *vaultClientRenewalRequest) error {
var renewalErr error
leaseDuration := req.increment
if req.isToken {
// Reset the token in the API client before returning
defer c.client.SetToken("")
// Set the token in the API client to the one that needs
// renewal
c.client.SetToken(req.id)
@ -394,6 +401,9 @@ func (c *vaultClient) renew(req *vaultClientRenewalRequest) error {
// Don't set this if renewal fails
leaseDuration = renewResp.Auth.LeaseDuration
}
// Reset the token in the API client before returning
c.client.SetToken("")
} else {
// Renew the secret
renewResp, err := c.client.Sys().Renew(req.id, req.increment)