Re: [access-control] Implementation comments

On Sun, Sep 14, 2008 at 10:58 PM, Anne van Kesteren <annevk@opera.com> wrote:
> 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?

It would seem very unfortunate and arbitrary from a users point of
view to not be able to get upload progress on text/plain POSTs but on
all other types of requests.

> Or maybe not enable
> them until access has been granted, or would that be to late?

That would be too late as access isn't granted until we receive
headers, which is after all data has been uploaded.

Maciej Stachowiak wrote:
> How about always sending upload progress events for a cross-site request,
> even if the server doesn't exist? Or alternately, send them in such a way
> that you can't tell whether the server cut off the connection suddenly or
> whether it doesn't exist?

That is an interesting idea, though tricky to implement. In general it
is very hard to prevent timing based attacks around server sniffing.
We are still struggling with how to do so well for images and even the
simple error event for cross-site XHR.

What we've talked about for the error event is to have a set time
after the request when we would fire the error event. Say 5 seconds
after. However there is always a risk that the server is slow to
respond and takes longer than 5 seconds at which point we would revile
its existence. We can of course increase the 5 second timeout, this
would reduce the number of servers that we'd inadvertently expose, but
at some point the timeout starts getting in the way of applications
contacting legitimate servers that just happen to be down at the
moment. Especially if the request is synchronous.

What strategy would you use to fake upload events such that they are
indistinguishable from the case when there is a slow as well as a fast
server on the other?


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

So if a server really should always respond with the full set of
headers it wants to allow, unless it wants to keep some specific ones
hidden?

I thought one of the use cases for Access-Control-Request-Headers was
for servers that needed to support a large amount of headers, possibly
even an unbounded set?

What would be the downside of always remembering that a server has
granted a specific header for a specific amount of time and only
delete that grant if the header exires, is updated by a later request,
or is removed due to a failed security check?

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

Maciej Stachowiak wrote:
> Do caching HTML implementations normally send Range without If-Range?
>
> In general I would be wary of extending the set of headers allowed without
> preflight. Are there specific common use cases for these?

I have heard Cache-Control: no-cache is fairly commonly used, though
the only use cases that I can think of are for servers that don't
properly set cache related headers, so it's possibly not something we
should optimize for.

The one header that I can give a semi good reason to add is
Content-Language, which seems to make sense if we think
Accept-Language is common enough to be put on the whitelist.

However I'd be fine with leaving the header list as is for now and
possibly whitelist more headers in a future version of the spec.

/ Jonas

Received on Monday, 15 September 2008 10:30:02 UTC