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 Support #7233

Merged
merged 15 commits into from
Jan 7, 2021
Merged

Vault Support #7233

merged 15 commits into from
Jan 7, 2021

Conversation

aquarapid
Copy link
Contributor

@aquarapid aquarapid commented Jan 1, 2021

Description

Vault support for storing both:

  • vtgate credentials (i.e. application -> vtgate)
  • MySQL credentials (i.e. vttablet -> MySQL)

Related Issue(s)

#7232

Checklist

  • Should this PR be backported?
  • Tests were added
  • Documentation was added or is not required
    • end-to-end test and options are pretty explanatory, but a walkthrough is to follow

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
    • However, if the vault option is not selected (i.e, you do not configure -db-credentials-server with the non-default value vault and/or -mysql_auth_server_impl with the non-default value vault); there will be no impact on existing functionality; i.e. the code paths are essentially unchanged.
  • VReplication
  • Cluster Management
  • Build

@aquarapid aquarapid requested a review from sougou January 1, 2021 01:53
@aquarapid aquarapid changed the title Jg vault Vault Support Jan 1, 2021
@askdba askdba requested a review from systay January 1, 2021 07:28
@aquarapid aquarapid marked this pull request as ready for review January 2, 2021 11:00
@deepthi
Copy link
Member

deepthi commented Jan 6, 2021

@aquarapid can you look into the unit_race test failure?

@aquarapid
Copy link
Contributor Author

Yeah, I see now that I have the mutex handling screwed up. I'll rework.

@shlomi-noach
Copy link
Contributor

Can you please give more context on this feature, either here or on #7232?

Copy link
Member

@harshit-gangal harshit-gangal left a comment

Choose a reason for hiding this comment

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

LGTM

@aquarapid
Copy link
Contributor Author

Can you please give more context on this feature, either here or on #7232?

@shlomi-noach

The idea (and implmentation) is relatively simple:

We re-use the existing static (file) credentials format for both vtgate MySQL credentials (app -> vtgate) and for vttablet MySQL credentials (vttablet -> MySQL), but we want to store it securely in HashiCorp Vault, so that the passwords (or hashes of passwords in the case of vtgate) is never stored on file on the hosts.

The way we do this is to utilize 2 features of Vault:

  • Vault's KV (or "secret") store. This operates by passing a path (key) and a token (access key) to Vault, and then it provides you with the value back. In our case the value we store is just the actual JSON format that the existing Vitess file-based authentication format would use.
  • While we can use a static token to pass to Vault as authentication/authorization; this is problematic, since possessing the token would grant access to the resource (value, via the path key) in Vault for an extended period of time. To help with this, Vault has the concept of "approles" where you have a role ID and a secret ID; this approle is bound to a Vault policy that finely specifies what secrets and actions in Vault you can perform with this role ID. We have support for approles in this PR as well.

Lastly, the important part here is that the token (or secret ID; depending on which you are using) are typically short-lived, and need to be renewed. So we have support for that as well.

Lastly, we also cache the Vault secret returned to us in memory of vtgate/vttablet ; since Vault is not designed for the QPS that might result if we access every time we need to authenticate a client.

@deepthi deepthi merged commit 3b8a7f0 into vitessio:master Jan 7, 2021
@deepthi deepthi deleted the jg_vault branch January 7, 2021 18:00
deepthi added a commit that referenced this pull request Jan 7, 2021
Cherry pick version of #7233 for release-9.0
@shlomi-noach
Copy link
Contributor

The idea (and implmentation) is relatively simple:

Sounds awesome! Thanks so much for elaborating!

@DeathBorn
Copy link
Contributor

@aquarapid I understand correctly that this implementation is not how vault databases plugin work. To simplify, this is - instead of local json file, it is stored in vault kv ?

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

Successfully merging this pull request may close these issues.

7 participants