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

Time-based transit key autorotation #13691

Merged
merged 11 commits into from
Jan 20, 2022
Merged

Time-based transit key autorotation #13691

merged 11 commits into from
Jan 20, 2022

Conversation

schultz-is
Copy link
Contributor

Summary

This introduces automatic, time-based key rotation to the transit secrets engine. A new field is added to the keysutil.Policy that keeps track of an interval for key rotation. The value can be specified at key creation time or via a key config update. The value has a minimum of 1 hour and keys are checked for rotation about once an hour.

Testing

There are three unit test suites included with this functionality:

  • TestTransit_CreateKeyWithAutorotation in builtin/logical/transit/path_keys_test.go which tests the creation of a key with the new auto_rotate_interval request field.
  • TestTransit_UpdateKeyConfigWithAutorotation in builtin/logical/transit/path_config_test.go which tests the update of an existing key with the new auto_rotate_interval request field.
  • TestTransit_AutoRotateKeys in builtin/logical/transit/backend_test.go which tests the actual automatic rotation of keys.

@schultz-is schultz-is requested a review from a team January 18, 2022 18:31
@vercel vercel bot temporarily deployed to Preview – vault January 18, 2022 18:35 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook January 18, 2022 18:35 Inactive
@schultz-is schultz-is changed the title WIP: Time-based transit key autorotation Time-based transit key autorotation Jan 18, 2022
Copy link
Collaborator

@sgmiller sgmiller left a comment

Choose a reason for hiding this comment

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

Looks good to me but I'd like one more person's eyes on it.

@sgmiller sgmiller requested review from a team and sgmiller January 18, 2022 20:27
@sgmiller sgmiller requested a review from a team January 18, 2022 20:28
@vercel vercel bot temporarily deployed to Preview – vault January 19, 2022 18:06 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook January 19, 2022 18:06 Inactive
@vercel vercel bot temporarily deployed to Preview – vault January 19, 2022 19:19 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook January 19, 2022 19:19 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook January 19, 2022 20:44 Inactive
@vercel vercel bot temporarily deployed to Preview – vault January 19, 2022 20:44 Inactive
Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

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

Looks good!

@schultz-is
Copy link
Contributor Author

Thanks y’all!

@schultz-is schultz-is merged commit df217c6 into main Jan 20, 2022
@schultz-is schultz-is deleted the schultz/VAULT-4329 branch January 20, 2022 15:10
p.Lock(true)
}
err = p.Rotate(ctx, req.Storage, b.GetRandomReader())
p.Unlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this panic if caching is disabled? Unlocking a not locked mutex, i believe, panics

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.

4 participants