updated vault to fetch token from file

[murphymj25]

#523 Updated vault to fetch token from file at start and at refresh interval.

[pschultz]

Thanks for your work.

The biggest issue involves refreshing when also using wrapped tokens. See inline comments for details.

Other than that there's a possible simplification in the config and a few nitpicks concerning spelling and codestyle.

[murphymj25]

Thanks for the feedback! I'll look to incorporate your suggestions. Do you have a link that gives an overview of wrapped tokens? That may help my understanding, but I think i get the general idea based on your comments.

[pschultz]

Wrapping is explained in detail here: https://www.vaultproject.io/docs/concepts/response-wrapping.html

In short: it may not be desirable to pass an actual Vault token (or any other secret, really) around, especially if it ends up in a file on disk.

Vault offers the possibility to wrap secrets to get around this. Instead of returning the secret directly, Vault creates a so-called wrapping token that can be used exactly once and stores the secret in that wrapping token's cubbyhole. You then give the wrapping token to fabio, and fabio trades it for the actual token. The corresponding CLI commands look something like this:

$ vault token create -policy fabio -wrap-ttl=5m
Key                              Value
---                              -----
wrapping_token:                  s.E0mxDIk9sZBTvDtZXe7ZMH3S
wrapping_token_ttl:              5m
[...]

$ vault token lookup s.E0mxDIk9sZBTvDtZXe7ZMH3S
Key                 Value
---                 -----
explicit_max_ttl    5m
num_uses            1
policies            [response-wrapping]
[...]

$ vault unwrap s.E0mxDIk9sZBTvDtZXe7ZMH3S
Key                  Value
---                  -----
token                s.Lcxkbr1PYZ5R2chAcALh0XMV
[...]

$ vault token lookup s.Lcxkbr1PYZ5R2chAcALh0XMV
Key                 Value
---                 -----
explicit_max_ttl    0s
num_uses            0
policies            [default fabio]
[...]

unwrap returns what token create would have returned if the -wrap-ttl flag hadn't been specified.

In this example the wrapping token is revoked after 5 minutes, or after the first unwrap request, whichever happens first. This makes it pretty safe to store it on disk. Even if someone gets their hands on it, it's worthless, assuming they don't beat fabio to the punch.

The consequence is that it is not enough to compare the file content with client's token. If the token was wrapped, they will be different. You have to remember the file content separately after a reload and compare against that value.

[murphymj25]

Got it, i think i can put something together for that. Thanks for the additional detail!

[murphymj25]

Just wanted to give a quick update. Using wrapper tokens makes a lot of sense for my use case, so I'm working to get my test vault environment setup to support that. Once i have that in place i should be able to make all of the proposed changes and be able to validate.

[murphymj25]

@pschultz I've updated the PR with your suggestions. I also changed things a bit, you can now do
vaultfetchtoken=env:[var] to pull from an environment variable and vaultfetchtoken=file:[path] to pull it from a file. I still have to test with a wrapped token, but the logic should be in place to support it. I'll update this thread once i confirm wrapped tokens behave as expect.

[murphymj25]

I have tested with wrapped tokens, and everything looks to be working as expected.

[pschultz]

Thanks for the update. I like the possibility to use environment variables other than VAULT_TOKEN!

The prevFetchedToken global doesn't work, unfortunately because there can technically be multiple independent clients. See inline comment for details.

And I guess I didn't make myself clear in the first iteration. If and when the token in the file changes, that new token can also be a wrapping token (not just the inital token). So you have to try the Logical().Unwrap() call every time the token changes.

Imagine a cron job that runs something like this every so often:

vault token create -policy=fabio -wrap-ttl=5m -field wrapping_token > /secret/vault_token

At no point is the token on disk ready to use. It must always be unwrapped first. I left an inline comment for that as well.

[murphymj25]

@pschultz Thanks for the feedback. I'll address all of the items. The unwrapping the token a 2nd time was a miss on my part; i'll get it updated.

[murphymj25]

@pschultz I have made all of the requested updates. I'm just going to have a peer of mine review everything quick. I should be able to push the updates next week.

[murphymj25]

I've updated the PR with the latest changes.

[pschultz]

Almost there. The bounds check when parsing vaultFetchToken is still missing (strings.SplitN isn't enough), and a gofmt pass, and then we should be good to go.

Thank you!

[murphymj25]

@pschultz Thanks, I'll take a look at adding a check. When I tested I wan't getting a failure, if it's failing in the playground it would be good to address.

[murphymj25]

@pschultz I have added a check that should handle if something isn't set correctly. Also cleaned up that line in properties, and ran a go fmt.

[pschultz]

Sorry for the delay; I was on vacation.

I can't figure out how to break this anymore, so the PR looks good to me now! Thanks a lot.

There are, however, two unreachable log statements in vault_client.go (the log calls and return statements should be swapped):

$ go vet ./cert
# github.com/fabiolb/fabio/cert
cert/vault_client.go:184:4: unreachable code
cert/vault_client.go:192:4: unreachable code

@leprechau, do you want to give this a once-over?

[murphymj25]

Thanks @pschultz, I'll get those returns updated.

[murphymj25]

@pschultz I've updated the log statements. Thanks!

[murphymj25]

Also, the "log.Printf("[WARN] vault: vaultfetchtoken not properly set")" was being sent every time there was an err, i've added a check so that log makes it more clear when something is not set correctly.

[aaronhurt]

LGTM! Thank you both for all the work.

[pschultz]

I guess a manual squashed merge isn't picked up by Github.

Merged at 146ec88. Thanks again!

[murphymj25]

@pschultz Thanks for all of the help with this!