> 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
> 
>

Reply via email to