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

[FIX] CloudStack: Values with wildcards will fail signature validation #846

Closed
wants to merge 1 commit into from
Closed

Conversation

The-Loeki
Copy link
Contributor

@The-Loeki The-Loeki commented Aug 1, 2016

Description

Thanks to a small bug in the request signer the signature will be invalid for any request with any value containing a wildcard; it should be considered 'safe'.

This is identical to apache/cloudstack-cloudmonkey@38b68fb

Status

  • done, ready for review

Thanks to a small bug in the request signer the signature will be invalid for any request with any value containing a wildcard.

Identical to apache/cloudstack-cloudmonkey@38b68fb
@tonybaloney
Copy link
Contributor

thanks @The-Loeki have you tested this? I don't really understand the issue but wondered if the string should be "" instead of "[]"?

@The-Loeki
Copy link
Contributor Author

The-Loeki commented Aug 8, 2016

@tonybaloney Thanks & we're running it in production already ;)

As I mentioned, the same bug surfaced in CloudMonkey (CloudStack's own API-driven CLI tool), and the fix was the same.

All API calls are signed & verified based on, amongst others, the encoded URL. This must apparently exclude HTML-encoding asterisks or the sig is invalid (as will the entire req be).

Of course it doesn't happen very often that a * is in the values, so that explains why this wasn't noticed for so long.
The safe param is just a list of exceptions pretending to be a str, so the order shouldnt make a difference (it's not a regex or anything)

@tonybaloney
Copy link
Contributor

ok, thats a strange bug. 👍

@asfgit asfgit closed this in 61d3bdb Aug 8, 2016
asfgit pushed a commit that referenced this pull request Aug 8, 2016
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