You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Yichi Lu <yi...@sungard.com> on 2014/04/16 00:28:08 UTC
Review Request 20390: CLOUDSTACK-6202: Add signatureversion and expiring
datetime to cloudmonkey, make it configurable
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20390/
-----------------------------------------------------------
Review request for cloudstack, Chiradeep Vittal and Rohit Yadav.
Repository: cloudstack-cloudmonkey
Description
-------
use signature version 3 for cloudmonkey api calls, with 600 seconds expiration time as default. The expiration time is configurable via .cloudmonkey/config
Diffs
-----
cloudmonkey/cloudmonkey.py b465bec
cloudmonkey/config.py 2f91608
cloudmonkey/requester.py b06e1fc
Diff: https://reviews.apache.org/r/20390/diff/
Testing
-------
for 600 seconds expiration time and CST:
now: 2014-04-15T16:36:46+0000 , expires: 2014-04-15T21:46:46+0000
Thanks,
Yichi Lu
Re: Review Request 20390: CLOUDSTACK-6202: Add signatureversion and expiring
datetime to cloudmonkey, make it configurable
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20390/#review40514
-----------------------------------------------------------
cloudmonkey/config.py
<https://reviews.apache.org/r/20390/#comment73556>
timeout and expires are misleading, can we make these config params more intuitive? Otherwise LGTM
cloudmonkey/requester.py
<https://reviews.apache.org/r/20390/#comment73557>
Won't this break backward compatibility? In case someone uses new cloudmonkey with old CloudStack will this work? (Pardon my ignorance regarding the signature version changes and API layer changes recently)
- Rohit Yadav
On April 15, 2014, 10:28 p.m., Yichi Lu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20390/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 10:28 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and Rohit Yadav.
>
>
> Repository: cloudstack-cloudmonkey
>
>
> Description
> -------
>
> use signature version 3 for cloudmonkey api calls, with 600 seconds expiration time as default. The expiration time is configurable via .cloudmonkey/config
>
>
> Diffs
> -----
>
> cloudmonkey/cloudmonkey.py b465bec
> cloudmonkey/config.py 2f91608
> cloudmonkey/requester.py b06e1fc
>
> Diff: https://reviews.apache.org/r/20390/diff/
>
>
> Testing
> -------
>
> for 600 seconds expiration time and CST:
> now: 2014-04-15T16:36:46+0000 , expires: 2014-04-15T21:46:46+0000
>
>
> Thanks,
>
> Yichi Lu
>
>
Re: Review Request 20390: CLOUDSTACK-6202: Add signatureversion and expiring
datetime to cloudmonkey, make it configurable
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20390/#review40655
-----------------------------------------------------------
Committed a711367e4968ae1fca68e5293ad5e1622a269a37
- Rohit Yadav
On April 15, 2014, 10:28 p.m., Yichi Lu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20390/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 10:28 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and Rohit Yadav.
>
>
> Repository: cloudstack-cloudmonkey
>
>
> Description
> -------
>
> use signature version 3 for cloudmonkey api calls, with 600 seconds expiration time as default. The expiration time is configurable via .cloudmonkey/config
>
>
> Diffs
> -----
>
> cloudmonkey/cloudmonkey.py b465bec
> cloudmonkey/config.py 2f91608
> cloudmonkey/requester.py b06e1fc
>
> Diff: https://reviews.apache.org/r/20390/diff/
>
>
> Testing
> -------
>
> for 600 seconds expiration time and CST:
> now: 2014-04-15T16:36:46+0000 , expires: 2014-04-15T21:46:46+0000
>
>
> Thanks,
>
> Yichi Lu
>
>
Re: Review Request 20390: CLOUDSTACK-6202: Add signatureversion and expiring
datetime to cloudmonkey, make it configurable
Posted by Yichi Lu <yi...@sungard.com>.
> On April 16, 2014, 2:50 a.m., Rohit Yadav wrote:
> > If it's backward compatible and works fine, let's merge. LGTM
Rohit:
As far as I know this will not break the backward compatibility. For API calls, the signature version and expiration time is consumed by server/src/com/cloud/api/ApiServer.java::verifyRequest() like this:
String signatureVersion = null;
String expires = null;
for (final String paramName : parameterNames) {
// parameters come as name/value pairs in the form String/String[]
final String paramValue = ((String[])requestParameters.get(paramName))[0];
if (ApiConstants.SIGNATURE.equalsIgnoreCase(paramName)) {
signature = paramValue;
} else {
if (ApiConstants.API_KEY.equalsIgnoreCase(paramName)) {
apiKey = paramValue;
} else if (ApiConstants.SIGNATURE_VERSION.equalsIgnoreCase(paramName)) {
signatureVersion = paramValue;
} else if (ApiConstants.EXPIRES.equalsIgnoreCase(paramName)) {
expires = paramValue;
}
if (unsignedRequest == null) {
unsignedRequest = paramName + "=" + URLEncoder.encode(paramValue, UTF_8).replaceAll("\\+", "%20");
} else {
unsignedRequest = unsignedRequest + "&" + paramName + "=" + URLEncoder.encode(paramValue, UTF_8).replaceAll("\\+", "%20");
}
}
}
// if api/secret key are passed to the parameters
if ((signature == null) || (apiKey == null)) {
s_logger.debug("Expired session, missing signature, or missing apiKey -- ignoring request. Signature: " + signature + ", apiKey: " + apiKey);
return false; // no signature, bad request
}
Date expiresTS = null;
// FIXME: Hard coded signature, why not have an enum
if ("3".equals(signatureVersion)) {
// New signature authentication. Check for expire parameter and its validity
if (expires == null) {
s_logger.debug("Missing Expires parameter -- ignoring request. Signature: " + signature + ", apiKey: " + apiKey);
return false;
}
synchronized (DateFormatToUse) {
try {
expiresTS = DateFormatToUse.parse(expires);
} catch (final ParseException pe) {
s_logger.debug("Incorrect date format for Expires parameter", pe);
return false;
}
}
final Date now = new Date(System.currentTimeMillis());
if (expiresTS.before(now)) {
s_logger.debug("Request expired -- ignoring ...sig: " + signature + ", apiKey: " + apiKey);
return false;
}
}
So expires will be checked only if an api call uses the key signatureversion, and its value == 3. Otherwise the key expires will just be ignored.
Yichi
- Yichi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20390/#review40515
-----------------------------------------------------------
On April 15, 2014, 5:28 p.m., Yichi Lu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20390/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 5:28 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and Rohit Yadav.
>
>
> Repository: cloudstack-cloudmonkey
>
>
> Description
> -------
>
> use signature version 3 for cloudmonkey api calls, with 600 seconds expiration time as default. The expiration time is configurable via .cloudmonkey/config
>
>
> Diffs
> -----
>
> cloudmonkey/cloudmonkey.py b465bec
> cloudmonkey/config.py 2f91608
> cloudmonkey/requester.py b06e1fc
>
> Diff: https://reviews.apache.org/r/20390/diff/
>
>
> Testing
> -------
>
> for 600 seconds expiration time and CST:
> now: 2014-04-15T16:36:46+0000 , expires: 2014-04-15T21:46:46+0000
>
>
> Thanks,
>
> Yichi Lu
>
>
Re: Review Request 20390: CLOUDSTACK-6202: Add signatureversion and expiring
datetime to cloudmonkey, make it configurable
Posted by Rohit Yadav <bh...@apache.org>.
> On April 16, 2014, 7:50 a.m., Rohit Yadav wrote:
> > If it's backward compatible and works fine, let's merge. LGTM
>
> Yichi Lu wrote:
> Rohit:
> As far as I know this will not break the backward compatibility. For API calls, the signature version and expiration time is consumed by server/src/com/cloud/api/ApiServer.java::verifyRequest() like this:
> String signatureVersion = null;
> String expires = null;
>
> for (final String paramName : parameterNames) {
> // parameters come as name/value pairs in the form String/String[]
> final String paramValue = ((String[])requestParameters.get(paramName))[0];
>
> if (ApiConstants.SIGNATURE.equalsIgnoreCase(paramName)) {
> signature = paramValue;
> } else {
> if (ApiConstants.API_KEY.equalsIgnoreCase(paramName)) {
> apiKey = paramValue;
> } else if (ApiConstants.SIGNATURE_VERSION.equalsIgnoreCase(paramName)) {
> signatureVersion = paramValue;
> } else if (ApiConstants.EXPIRES.equalsIgnoreCase(paramName)) {
> expires = paramValue;
> }
>
> if (unsignedRequest == null) {
> unsignedRequest = paramName + "=" + URLEncoder.encode(paramValue, UTF_8).replaceAll("\\+", "%20");
> } else {
> unsignedRequest = unsignedRequest + "&" + paramName + "=" + URLEncoder.encode(paramValue, UTF_8).replaceAll("\\+", "%20");
> }
> }
> }
>
> // if api/secret key are passed to the parameters
> if ((signature == null) || (apiKey == null)) {
> s_logger.debug("Expired session, missing signature, or missing apiKey -- ignoring request. Signature: " + signature + ", apiKey: " + apiKey);
> return false; // no signature, bad request
> }
>
> Date expiresTS = null;
> // FIXME: Hard coded signature, why not have an enum
> if ("3".equals(signatureVersion)) {
> // New signature authentication. Check for expire parameter and its validity
> if (expires == null) {
> s_logger.debug("Missing Expires parameter -- ignoring request. Signature: " + signature + ", apiKey: " + apiKey);
> return false;
> }
> synchronized (DateFormatToUse) {
> try {
> expiresTS = DateFormatToUse.parse(expires);
> } catch (final ParseException pe) {
> s_logger.debug("Incorrect date format for Expires parameter", pe);
> return false;
> }
> }
> final Date now = new Date(System.currentTimeMillis());
> if (expiresTS.before(now)) {
> s_logger.debug("Request expired -- ignoring ...sig: " + signature + ", apiKey: " + apiKey);
> return false;
> }
> }
>
> So expires will be checked only if an api call uses the key signatureversion, and its value == 3. Otherwise the key expires will just be ignored.
>
> Yichi
Alright, I see. Will merge your commit on cloudmonkey's repo. Thanks for your patch.
- Rohit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20390/#review40515
-----------------------------------------------------------
On April 15, 2014, 10:28 p.m., Yichi Lu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20390/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 10:28 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and Rohit Yadav.
>
>
> Repository: cloudstack-cloudmonkey
>
>
> Description
> -------
>
> use signature version 3 for cloudmonkey api calls, with 600 seconds expiration time as default. The expiration time is configurable via .cloudmonkey/config
>
>
> Diffs
> -----
>
> cloudmonkey/cloudmonkey.py b465bec
> cloudmonkey/config.py 2f91608
> cloudmonkey/requester.py b06e1fc
>
> Diff: https://reviews.apache.org/r/20390/diff/
>
>
> Testing
> -------
>
> for 600 seconds expiration time and CST:
> now: 2014-04-15T16:36:46+0000 , expires: 2014-04-15T21:46:46+0000
>
>
> Thanks,
>
> Yichi Lu
>
>
Re: Review Request 20390: CLOUDSTACK-6202: Add signatureversion and expiring
datetime to cloudmonkey, make it configurable
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20390/#review40515
-----------------------------------------------------------
If it's backward compatible and works fine, let's merge. LGTM
- Rohit Yadav
On April 15, 2014, 10:28 p.m., Yichi Lu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20390/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 10:28 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and Rohit Yadav.
>
>
> Repository: cloudstack-cloudmonkey
>
>
> Description
> -------
>
> use signature version 3 for cloudmonkey api calls, with 600 seconds expiration time as default. The expiration time is configurable via .cloudmonkey/config
>
>
> Diffs
> -----
>
> cloudmonkey/cloudmonkey.py b465bec
> cloudmonkey/config.py 2f91608
> cloudmonkey/requester.py b06e1fc
>
> Diff: https://reviews.apache.org/r/20390/diff/
>
>
> Testing
> -------
>
> for 600 seconds expiration time and CST:
> now: 2014-04-15T16:36:46+0000 , expires: 2014-04-15T21:46:46+0000
>
>
> Thanks,
>
> Yichi Lu
>
>
Re: Review Request 20390: CLOUDSTACK-6202: Add signatureversion and expiring
datetime to cloudmonkey, make it configurable
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20390/#review40653
-----------------------------------------------------------
Ship it!
Ship It!
- Rohit Yadav
On April 15, 2014, 10:28 p.m., Yichi Lu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20390/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 10:28 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and Rohit Yadav.
>
>
> Repository: cloudstack-cloudmonkey
>
>
> Description
> -------
>
> use signature version 3 for cloudmonkey api calls, with 600 seconds expiration time as default. The expiration time is configurable via .cloudmonkey/config
>
>
> Diffs
> -----
>
> cloudmonkey/cloudmonkey.py b465bec
> cloudmonkey/config.py 2f91608
> cloudmonkey/requester.py b06e1fc
>
> Diff: https://reviews.apache.org/r/20390/diff/
>
>
> Testing
> -------
>
> for 600 seconds expiration time and CST:
> now: 2014-04-15T16:36:46+0000 , expires: 2014-04-15T21:46:46+0000
>
>
> Thanks,
>
> Yichi Lu
>
>