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

PWX-36873: Add vault cooldowns #86

Merged
merged 8 commits into from
Apr 16, 2024
Merged

PWX-36873: Add vault cooldowns #86

merged 8 commits into from
Apr 16, 2024

Conversation

zoxpx
Copy link
Contributor

@zoxpx zoxpx commented Apr 13, 2024

  • double "permission denied" REST error will put vault client into 5 minutes cooldown for all REST calls
  • can disable via VAULT_COOLDOWN_PERIOD:disabled

What this PR does / why we need it:

Turns out it is easy to overwhelm Hasicorp's Vault, and accidentally cause DDOS-attack

As a fix, we're adding REST cooldown for 5 minutes, to all Vault REST client calls

  • it is triggered by hitting 2 consecutive "permission denied" errors (i.e. PUT<key> ++ RENEW<token> both responded w/ "permission denied")

Which issue(s) this PR fixes (optional)
Closes # PWX-36873

Special notes for your reviewer:

* double "permission denied" REST error will put vault client into 5 minutes cooldown for all REST calls
* can disable via `VAULT_COOLDOWN_PERIOD:disabled`

Signed-off-by: Zoran Rajic <[email protected]>
@zoxpx zoxpx requested review from adityadani and a team April 13, 2024 06:23
@zoxpx zoxpx self-assigned this Apr 13, 2024
@zoxpx
Copy link
Contributor Author

zoxpx commented Apr 13, 2024

Note, @adityadani , @CharudathGopal -- looks like the AWS secrets integration test is failing (creds pulled out?)

  • we might want to update the test, or disable it

@zoxpx
Copy link
Contributor Author

zoxpx commented Apr 15, 2024

Woops.. forgot to include UT with the original commit (fixed w/ follow-up f9fde71)

Copy link
Collaborator

@adityadani adityadani left a comment

Choose a reason for hiding this comment

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

lgtm

vault/vault.go Outdated Show resolved Hide resolved
vault/vault.go Outdated Show resolved Hide resolved
This reverts commit 3aaff8a -- cause this breaks other integration tests.
@zoxpx
Copy link
Contributor Author

zoxpx commented Apr 16, 2024

Ok.. I can't failing AWS Secrets Manager integration test with modifying my tests -- looks like this test has been failing for 7 months.

I'll proceed w/ the squash-commit.

@zoxpx zoxpx merged commit a17cf7f into master Apr 16, 2024
4 of 5 checks passed
@zoxpx zoxpx deleted the PWX-36873_vault_cooldowns branch April 16, 2024 03:12
zoxpx added a commit that referenced this pull request Apr 16, 2024
* double "permission denied" REST error will put vault client into 5 minutes cooldown for all REST calls
* can disable via `VAULT_COOLDOWN_PERIOD:0`

Signed-off-by: Zoran Rajic <[email protected]>
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.

2 participants