marig345 / oauth-php

Automatically exported from code.google.com/p/oauth-php
MIT License
0 stars 0 forks source link

Calls using full absolulte URIs fail signature verification #71

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Make a signed service request using full absolute URI spec as allowed by 
HTTP 1.1 spec (and as used by certain Python libraries)
2. Be told that you signature verification fails. 
3. Compare the signature base strings and see that the hostname and URIs have 
been incorrectly calculated

In more detail: 

Please see http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1.2 which 
for HTTP 1.1 explicitly allows both of:

GET /endpoint.php
GET http://www.example.com/endpoint.php

The difference within PHP is that the $SERVER['REQUEST_URI'] contains the exact 
strings after the HTTP method above. The problem is with the constructor of 
OAuthRequest, where the following currently happens:

if (empty($uri)) {
    $uri = sprintf('%s://%s%s', $proto, $_SERVER['HTTP_HOST'], $_SERVER['REQUEST_URI']);
}

but if the REQUEST_URI already  has the protocol and host within it, then you 
end up with the URI being 
http://www.example.comhttp://www.example.com/endpoint.php which then fails the 
regex hostname check in parseUri.

I've currently patched my version (135) with the following instead (note this 
will take care of https as well):

if (empty($uri)) {
    //If the REQUEST_URI already has the absolute URI, then just use that
    if (substr($_SERVER['REQUEST_URI'], 0, 4) == "http") {
        $uri = $_SERVER['REQUEST_URI'];
    } else {
        $uri = sprintf('%s://%s%s', $proto, $_SERVER['HTTP_HOST'], $_SERVER['REQUEST_URI']);
    }
}

This may require other fixes for other possible protocols, but I believe that 
all HTTP request URIs must either start with a valid protocol, or a "/" 
character.

Original issue reported on code.google.com by steves%m...@gtempaccount.com on 15 Oct 2010 at 2:08

GoogleCodeExporter commented 9 years ago
Actually - I think it's when you also include the explicit :80 or :443 in the 
GET/POST request that the regex check fails. Otherwise, it just fails the 
signature check as the base string becomes wrong.

Original comment by steves%m...@gtempaccount.com on 15 Oct 2010 at 2:35

GoogleCodeExporter commented 9 years ago
Sorry for the delay. Please let me understand this: does the first patch you 
posted fix the problem? It makes sense anyway, so I'm adding it to r163.

But I am not sure what regex is breaking... Can you tell me the lines? Or the 
problem is that the sprintf() should have the port as well in this case?

Original comment by brunobg%...@gtempaccount.com on 28 Oct 2010 at 4:21

GoogleCodeExporter commented 9 years ago
Short answer: Yes the patch fixes the issue, and the regex is correct as-is.

Longer answer: I made a small ommission in the original report - the regex 
fails if the request made through the HTTP server is of the form:

GET http://www.example.com:80/endpoint.php

whereupon the full URL would become 

http://www.example.comhttp://www.example.com:80/endpoint.php

which gets broken down to a port of 80 (by virtue of looking at the *last* 
colon), and host of 

www.example.comhttp://www.example.com

which fails the regex check on the "host" validation because of the :// which 
is completely correct.

Thanks for the applying the fixes!

Original comment by steves%m...@gtempaccount.com on 29 Oct 2010 at 8:58

GoogleCodeExporter commented 9 years ago

Original comment by brunobg%...@gtempaccount.com on 17 Nov 2010 at 5:18