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

Forbid ssh key signing with specified extensions when role allowed_extensions is not set #12847

Merged
merged 3 commits into from Oct 15, 2021

Conversation

stevendpclark
Copy link
Contributor

  • This is a behaviour change on how we process the allowed_extensions role
    parameter when it does not contain a value. The previous handling allowed
    a client to override and specify any extension they requested signing of an ssh key
  • We now require a role to explicitly set this behaviour by setting the parameter
    to a '*' value which matches the behaviour of other keys such as allowed_users
    within the role.
  • No migration of existing roles is provided either, so operators if they truly
    want this behaviour will need to update existing roles appropriately.

@stevendpclark stevendpclark force-pushed the stevendpclark/vault-3375-ssh-extension-fail-close branch from f73c63b to beee2ae Compare October 15, 2021 18:45
@vercel vercel bot temporarily deployed to Preview – vault October 15, 2021 18:45 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 15, 2021 18:45 Inactive
…tensions is not set

 - This is a behaviour change on how we process the allowed_extensions role
   parameter when it does not contain a value. The previous handling allowed
   a client to override and specify any extension they requested.
 - We now require a role to explicitly set this behaviour by setting the parameter
   to a '*' value which matches the behaviour of other keys such as allowed_users
   within the role.
 - No migration of existing roles is provided either, so operators if they truly
   want this behaviour will need to update existing roles appropriately.
@stevendpclark stevendpclark force-pushed the stevendpclark/vault-3375-ssh-extension-fail-close branch from beee2ae to 181e590 Compare October 15, 2021 18:49
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 15, 2021 18:49 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 15, 2021 18:49 Inactive
Copy link
Contributor

@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.

Approving in general but feel free to make that modification you comented on.

builtin/logical/ssh/path_sign.go Outdated Show resolved Hide resolved
builtin/logical/ssh/path_sign.go Outdated Show resolved Hide resolved
@@ -0,0 +1,5 @@
```release-note:breaking-change
secrets/ssh: Roles with empty allowed_extensions will now forbid end-users
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this has to be a single line, though I'm not positive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it's okay as 11775.txt (https://github.com/hashicorp/vault/blob/main/changelog/11775.txt) is multiple lines and seems to have generated ok: https://github.com/hashicorp/vault/blob/main/CHANGELOG.md#180

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good to know.

@vercel vercel bot temporarily deployed to Preview – vault October 15, 2021 20:46 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 15, 2021 20:46 Inactive
Copy link
Contributor

@victorr victorr left a comment

Choose a reason for hiding this comment

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

👍

@vercel vercel bot temporarily deployed to Preview – vault-storybook October 15, 2021 20:49 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 15, 2021 20:49 Inactive
@stevendpclark stevendpclark merged commit 14c28ce into main Oct 15, 2021
@stevendpclark stevendpclark deleted the stevendpclark/vault-3375-ssh-extension-fail-close branch October 15, 2021 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants