[
https://issues.apache.org/jira/browse/SHINDIG-447?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Karsten Beyer updated SHINDIG-447:
----------------------------------
Attachment: fix-issue-447.patch
As promised, attached a patch to fix the situation. However the issue seemed to
be a lot worse than first expected. So i will first describe my analysis of the
problems with the current code.
There is a method to sanitize the fields (GET+POST) which go into the
base_string used for the signature. It is located in SigningFetcher and relies
on this method:
protected static $ALLOWED_PARAM_NAME = "[-:\\w]+";
...
private function allowParam($paramName)
{
$canonParamName = strtolower($paramName);
return (! (substr($canonParamName, 0, 5) == "oauth" ||
substr($canonParamName, 0, 6) == "xoauth" || substr($canonParamName, 0, 9) ==
"opensocial")) && ereg(SigningFetcher::$ALLOWED_PARAM_NAME, $canonParamName);
}
Now this filters all parameters which either start with one of the strings in
the first half of the expression or which do not contain a 'w'(!). For this
reason, stuff like "numentries" or "myvalueformyscript" is being thrown away,
while "signOwner" and "signViewer" are kept. This is in my opinion a completely
wrong behavior. I will present a solution to it later on.
Next problem is that the values are only appended to the query string, if they
were not posted to the proxy (which is ok, if we are supposed to do a POST).
This affects both the signViewer and signOwner fields as everything is posted
to the proxy. Bad thing is that the POST data and headers are not set, so we
effectively do a GET instead of the requested POST.
The OAuthRequest object now builds its base_string using all "sanitized" values
in the GET+POST array. This contains at this point in time:
1) signViewer & signOwner
2) all Fields of the user not containing a 'w'
They are not used in the final request string however, as they are POST data.
Therefore the signature validation cannot work.
Now to my solution (This patch is rather complex, so it would be a good idea to
review it carefully before applying it):
1) I "fixed" the regular expression to accept alphanum, 0-9, -, _. However this
is a very bad idea. As i understand, special chars are allowed for form field
names, they just need to be encoded in a POST. Maybe someone has a good idea
here. '\w' is as far as i know not available with the POSIX regular expressions.
2) As i understood the OAuth specification, the posted form data needs to be
included in the base_string used for the signature. This is transfered to the
proxy in the postData field of the $_POST. I implemented a parsing function
which put the (sanitized) single entries into the postParams arrays, so they
are used for the generation of the signature and also for the building of the
query string / or postdata string (see below).
3) The sanitize function obviously did not work at all. If you fix it however,
all the parameters which are supposed to be for the proxy, are now in the
base_string as well as the query string. So i extended the allowParam method to
exclude all these parameters (e.g. url, gadget, bypassspeccache etc.). This
includes the postData param, as we put these values individually into the
postParam array.
4) If we have a POST request, i build a post_data string according to RFC3986
using the oauth library which is later used to actually post the data. If we
have a GET request however, i leave the post data string == false in order to
make a GET request to the partners page and keep the forPost array empty so
that these are appended to the query string instead. Take a look at this
example to understand why:
testdataplain = new Object();
testdataplain['hello'] = 'world';
params[gadgets.io.RequestParameters.CONTENT_TYPE] = gadgets.io.ContentType.TEXT;
params[gadgets.io.RequestParameters.AUTHORIZATION] =
gadgets.io.AuthorizationType.SIGNED
params[gadgets.io.RequestParameters.METHOD] = gadgets.io.MethodType.GET;
params[gadgets.io.RequestParameters.POST_DATA] =
gadgets.io.encodeValues(testdataplain);
gadgets.io.makeRequest('http://opensocialapps.kbsilver/log.php?hope=itworks',
gotLog, params);
Obviously i would expect to get the values hello=world&hope=itworks in a signed
request.
5) Last of all, if we do a POST, i add the constructed post_data array (see 4)
and the $_POST['headers'] field to the contructor of RemoteContentRequest in
order to actually make a POST.
6) I removed the quick hack from the OAuth.php file.
> makeRequest - Signed request cannot be verified because of base_string
> inconsitency
> -----------------------------------------------------------------------------------
>
> Key: SHINDIG-447
> URL: https://issues.apache.org/jira/browse/SHINDIG-447
> Project: Shindig
> Issue Type: Bug
> Components: Common Components (PHP)
> Reporter: Karsten Beyer
> Attachments: fix-issue-447.patch
>
>
> When doing a signed request with makeRequest, the generated signature cannot
> be verified, because different base_strings are used.
> I used the method described for Orkut
> (http://code.google.com/p/opensocial-resources/wiki/OrkutValidatingSignedRequests)
> to verify the signature on the requested page. When logging the base_string
> on both sides, i detected, that the signOwner and signViewer parameters are
> used for the base_string, but are not part of the request that the proxy does
> to the target page:
> base_string build by shindig:
> GET&http%3A%2F%2Fopensocialapps.kbsilver%2Flog.php&container%3Dazubister%26oauth_consumer_key%3Dnot%2520implemented%26oauth_nonce%3D68d2fedb1b405f426e0b5d6aa90893bb%26oauth_signature_method%3DRSA-SHA1%26oauth_timestamp%3D1215874245%26oauth_token%3D%26opensocial_app_id%3D25%26opensocial_owner_id%3DQ3czQ1B2SytHbVU0ZXJEOXRwOTJHdz09%26opensocial_viewer_id%3DQ3czQ1B2SytHbVU0ZXJEOXRwOTJHdz09%26signOwner%3Dtrue%26signViewer%3Dtrue%26synd%3Dazubister%26xoauth_signature_publickey%3Dhttp%253A%252F%252Fshindig.kbsilver%252Fpublic.crt
> base_string build at the requested page:
> GET&http%3A%2F%2Fopensocialapps.kbsilver%2Flog.php&container%3Dazubister%26oauth_consumer_key%3Dnot%2520implemented%26oauth_nonce%3D68d2fedb1b405f426e0b5d6aa90893bb%26oauth_signature_method%3DRSA-SHA1%26oauth_timestamp%3D1215874245%26oauth_token%3D%26opensocial_app_id%3D25%26opensocial_owner_id%3DQ3czQ1B2SytHbVU0ZXJEOXRwOTJHdz09%26opensocial_viewer_id%3DQ3czQ1B2SytHbVU0ZXJEOXRwOTJHdz09%26synd%3Dazubister%26xoauth_signature_publickey%3Dhttp%253A%252F%252Fshindig.kbsilver%252Fpublic.crt
> Analyzing the $_GET parameters i get at the target leads to the same result.
> I do not know enough about the OAUTH logic in shindig, but i think either the
> signOwner and signViewer parameters need to be ignored when building the
> base_string for the signature or they need to be part of the request to the
> target page.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
