Skip to content

Fix the string to sign #29

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

Merged
merged 1 commit into from
Apr 20, 2019
Merged

Fix the string to sign #29

merged 1 commit into from
Apr 20, 2019

Conversation

cl-k-takahashi
Copy link
Contributor

The CloudStack management server signs lower-cased users' requests.
urllib.quote_plus() encodes characters into upper case hex string, so we need to call lower() at last.

urllib.quote_plus() encodes characters into upper case hex string, so we need to call lower() at last.
@DaanHoogland
Copy link
Contributor

@cl-k-takahashi : please explain your point. As I tried to reproduce what I understand from it, I got:

$ python
Python 2.7.15 (default, Feb 12 2019, 11:00:12)
[GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import urllib;
>>> urllib.quote_plus("aAbBcC")
'aAbBcC'
>>> urllib.quote_plus("abc")
'abc'
>>> urllib.quote_plus("ABC")
'ABC'

I think, if we are correcting the developers documentation we need to be more expicit about the pitfall you are describing. Is this a version issue?
Please correct my understanding.

@cl-k-takahashi
Copy link
Contributor Author

cl-k-takahashi commented Apr 4, 2019

@DaanHoogland
Current CloudStack contains the code below in ApiServer.java:

 unsignedRequest = unsignedRequest.toLowerCase();

            final Mac mac = Mac.getInstance("HmacSHA1");
            final SecretKeySpec keySpec = new SecretKeySpec(secretKey.getBytes(), "HmacSHA1");
            mac.init(keySpec);
            mac.update(unsignedRequest.getBytes());

As you can see, CloudStack generates hash for lower cased request.
Current documentation is not problematic for [a-zA-Z0-9] but is problematic for urlencoded characters:

>>> urllib.quote_plus('aBcDe'.lower())
'abcde'
>>> urllib.quote_plus('aBcDe').lower()
'abcde'
>>> urllib.quote_plus('aBcDe★'.lower())
'abcde%E2%98%85'
>>> urllib.quote_plus('aBcDe★').lower()
'abcde%e2%98%85'

The 3rd result contains upper-case characters which causes verification failure in CloudStack management server.

@@ -255,7 +255,7 @@ encoding:

.. parsed-literal::

>>> sig_str='&'.join(['='.join([k.lower(),urllib.quote_plus(request[k].lower().replace('+','%20'))])for k in sorted(request.iterkeys())])
>>> sig_str='&'.join(['='.join([k.lower(),urllib.quote_plus(request[k]).lower().replace('+','%20')])for k in sorted(request.iterkeys())])
Copy link
Member

Choose a reason for hiding this comment

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

For reference, this is how cloudmonkey (python version) does it: https://github.com/apache/cloudstack-cloudmonkey/blob/5.3/cloudmonkey/requester.py#L203

@DaanHoogland
Copy link
Contributor

@cl-k-takahashi very clear, thank you.

@PaulAngus PaulAngus merged commit 1ef74ee into apache:master Apr 20, 2019
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.

4 participants