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

secrets/database: adds ability to manage alternative credential types and configuration #15376

Merged
merged 11 commits into from
May 17, 2022

Conversation

austingebauer
Copy link
Contributor

Overview

This PR adds the ability for the database secrets engine to manage alternative credential types and configuration. This provides the foundation for the secrets engine to start managing database credentials other than passwords (e.g., asymmetric keys, x.509 client certificates).

Initially, this will be used to enable Vault to manage RSA key pair credentials to be consumed by the Snowflake database plugin. This will enable clients to use key pair authentication using a Vault-managed credential. See hashicorp/vault-plugin-database-snowflake#15 for example usage on the plugin side.

Dynamic Role Usage:

vault write database/roles/my-dynamic-keypair-role \
	db_name=my-snowflake-database \
	creation_statements="CREATE USER {{name}} RSA_PUBLIC_KEY='{{public_key}}'
		DAYS_TO_EXPIRY = {{expiration}} DEFAULT_ROLE=myrole;
		GRANT ROLE myrole TO USER {{name}};" \
	default_ttl="1h" \
	credential_type="rsa_private_key" \
        credential_config=key_bits=2048 \ # Optional
        credential_config=format="pkcs8"  # Optional

Static Role Usage:

vault write database/static-roles/my-static-role \
    db_name=my-snowflake-database \
    rotation_statements="ALTER USER {{name}} SET RSA_PUBLIC_KEY='{{public_key}}'" # Optional
    username="static_user_keypair" \
    rotation_period="5m" \
    credential_type="rsa_private_key" \
    credential_config=key_bits=2048 \ # Optional
    credential_config=format="pkcs8"  # Optional

Testing

Basic testing has been included in this PR. Tests that integrate with Snowflake to use RSA key pair credentials are included in the Snowflake database plugin PR.

Additional tests are in progress and will be included in further commits.

@austingebauer austingebauer added this to the 1.11.0-rc1 milestone May 11, 2022
@austingebauer austingebauer requested a review from a team May 11, 2022 18:35
builtin/logical/database/rotation.go Outdated Show resolved Hide resolved
builtin/logical/database/rotation.go Outdated Show resolved Hide resolved
@@ -27,6 +27,8 @@ message NewUserRequest {
google.protobuf.Timestamp expiration = 3;
Statements statements = 4;
Statements rollback_statements = 5;
int32 credential_type = 6;
bytes public_key = 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a minor thing, but since we're only using PEM-encoded keys, those should all be valid strings, right?

I mostly worry that bytes (or []byte) above might lead someone to think we're using something like DER.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I originally used a string type here. I would be fine going back to string since we are going with PEM encoding. Will do so if others agree.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a strong preference either way. FWIW the standard library call pem.EncodeToMemory also returns it as []byte.

builtin/logical/database/path_creds_create.go Show resolved Hide resolved
builtin/logical/database/path_creds_create.go Outdated Show resolved Hide resolved
builtin/logical/database/path_creds_create.go Show resolved Hide resolved
builtin/logical/database/path_roles.go Show resolved Hide resolved
builtin/logical/database/credentials.go Outdated Show resolved Hide resolved
builtin/logical/database/path_roles_test.go Outdated Show resolved Hide resolved
builtin/logical/database/credentials.go Outdated Show resolved Hide resolved
builtin/logical/database/rotation.go Outdated Show resolved Hide resolved
Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

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

LGTM. Whew, big change, but I like it. A minor possible nit/suggestion. :)

@austingebauer austingebauer merged commit 0f1784d into main May 17, 2022
@austingebauer austingebauer deleted the db-secrets-credential-types branch May 17, 2022 16:21
Gabrielopesantos pushed a commit to Gabrielopesantos/vault that referenced this pull request Jun 6, 2022
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.

3 participants