Re: [access-control] Implementation comments

On Sun, 14 Sep 2008 13:20:00 +0200, Jonas Sicking <jonas@sicking.cc> wrote:
> 1.
> Most serious is that the combination of Access-Control, XMLHttpRequest
> Level 2 and ProgressEvents allows an evil site to detect the existence
> of servers sitting behind the same firewall as the user. The evil site
> would run the following JavaScript code:
>
> xhr = new XMLHttpRequest();
> xhr.open("POST", siteToTest, true);
> xhr.setRequestHeader("Content-Type", "text/plain");
> xhr.upload.onprogress = siteFound;
> xhr.send(veryLargeStringOfDummyData);
>
> For this request no preflight should be done according to current AC
> spec. However the event listener for upload progress events will be
> called before any header data is received, i.e. before any
> Access-Control checks can be done. This means that this listener is
> called even if the AC checks fail, allowing the code to detect that
> the site exists.
>
> To fix this I propose that we keep the current behavior stated by the
> spec as a default behavior, but state that a embedding spec is allowed
> to require preflight requests on any request, even if Access-Control
> does not require this, if it would otherwise be possible to detect the
> existence of a server.
>
> We'd then require preflight requests if there are any event listeners
> registered on the XMLHttpRequest.upload object. We'd further have to
> forbid registering such listeners if a preflight-less AC request has
> started.

That sounds extremely ugly. Can't we just disable "make upload progress  
notifications" unless a preflight request has been made? Or maybe not  
enable them until access has been granted, or would that be to late?


> 2.
> The way that the preflight result cache is defined gives some sup
> optimal results. For example, say that a site first makes two GET
> requests with two different custom headers are made to a URI. We'd
> have the following traffic for the preflight requests (details removed
> for simplification):
>
> Request 1:
> OPTIONS /foo HTTP 1.1
> Access-Control-Request-Headers: X-Custom-1
>
> Response 1:
> 200 OK
> Access-Control-Allow-Headers: X-Custom-1
> Access-Control-Max-Age: 3600
>
> Request 2:
> OPTIONS /foo HTTP 1.1
> Access-Control-Request-Headers: X-Custom-2
>
> Response 2:
> 200 OK
> Access-Control-Allow-Headers: X-Custom-2
> Access-Control-Max-Age: 3600
>
> If the site then wanted to make a request that included both the
> X-Custom-1 *and* X-Custom-2 headers the spec currently requires a new
> preflight request since there is no entry in the preflight cache with
> both headers set. Rather there are two entries, one with one of the
> headers and one with the other.

Actually, no, there would just be one. The first entry is removed the  
moment the second request is made as it has a header that that entry does  
not have.


> It seems to me that the text should be
> changed to allow a request to be made without requiring another
> preflight.
>
> Or am I misunderstanding the spec and this is already allowed?

You seem to misunderstand it at least partially, but no, it's not allowed.  
I thought we decided that if a site wants to keep certain allowed headers  
"private" (security through obscurity, really) it was worth the additional  
preflight request cost for when those headers would be used.


> 3.
> I think we should look into putting more headers on the white list
> that doesn't require preflights. Specifically the following headers
> seems like they should be safe
>
> Range
> If-Unmodified-Since
> Cache-Control: no-cache
>
> since they can already happen in an HTML implementation that has a
> local cache (which I think is pretty much every implementation that I
> know of).

I'm happy to add whatever headers people are ok with. I don't really feel  
knowledgeable enough to make security judgments about them though. Having  
said that, I would rather avoid adding header names of which only certain  
values are ok.


-- 
Anne van Kesteren
<http://annevankesteren.nl/>
<http://www.opera.com/>

Received on Sunday, 14 September 2008 20:58:50 UTC