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

Case Insensitive Match #9

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sims-security
Copy link

Description

The following pull request corrects an issue where pulling secrets from Vault fails due to case-sensitive key referencing. At a high level, this issue means that an attempt by Drone to pull "username" when the key in Vault is "Username" will result in a "secret key not found" error.

Use Case

Lets assume we are attempting to pull a secret in vault which has the key "FOO" and value "BAR".

  • Example pipeline yaml pulling secret "foo"
---

kind: secret
name: foo
get:
    path: example/data/credentials
    name: foo
  • Currently, the plugin will processes this request and make a found/not found decision based on the line of code here. This operation does not account for mismatched case. Searching the params map for "foo" will yield no results (actual key in Vault stored as "FOO"), and as such the pipeline will return "secret key not found".

How To Fix This Issue

This issue can be fixed by performing a case-insensitive key lookup when querying the secret value from the "params" go map. The code changes proposed successfully mitigate this issue.

For testing purposes I have included a new unit test uppercase_secret.json which tests successfully pulling a secret when drone requests the value for key="USERNAME" and Vault includes a record where key="Username".

Additional Considerations

I could not think of any example/reason where a request for "USERNAME" when Vault is storing the key as "Username" would cause unintended consequences or impact. A secret in the form of a key/value pair should be represented with a unique key, therefor IMO "username" and "Username" should not be treated as separate values.

@CLAassistant
Copy link

CLAassistant commented Feb 8, 2021

CLA assistant check
All committers have signed the CLA.

@sims-security
Copy link
Author

Thanks for the quick response and approval, @tboerger
Please let me know if I can address any additional concerns or provide more detail!

@tboerger
Copy link

It's up to @bradrydzewski to give another review and merging the change.

@sims-security
Copy link
Author

Just following up on this; I know you have a lot on your plate @bradrydzewski and this is probably not super high on your list of "to-dos" to keep everything running :)

@sims-security
Copy link
Author

bumping @bradrydzewski

lkubb added a commit to lkubb/drone-vault that referenced this pull request Feb 14, 2023
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.

3 participants