Re: [cors] Review

Hey Mark,

Thanks a lot for you review, very much appreciated. It's somewhat unfortunate that you raise these substantive issues at such a late stage given that we have shipping implementations at this point. As such I'm not clear whether we can still resolve those in a satisfactory way.


On Fri, 29 May 2009 09:27:46 +0200, Mark Nottingham <mnot@mnot.net> wrote:
> *** Substantial issues
>
> * POST as a "simple" method - POST is listed as a simple method (i.e.,  
> one not requiring pre-flight) because there are already security issues  
> that allow an HTML browser to send cross-site POST requests. However,  
> other contexts of use may not have this problem, and future developments  
> may close that hole. Requiring a pre-flight for POST because it is  
> unsafe is the right thing to do for both of these reasons.

We decided to do it this way for compatibility with XDomainRequest. POST has further restrictions applied to it though. What exactly do you mean with "other contexts" by the way?


> * Field-name verbosity - The defined header field-names are quite long,  
> and contain misleading (this isn't really access-control information, at  
> least on requests) and redundant (e.g., "request-method"). Suggest  
> using:  CORS-Allow-Origin, CORS-Maxage, CORS-Allow-Cred,  
> CORS-Allow-Methods, CORS-Allow-Headers, CORS-Method, CORS-Headers.

It seems unlikely we can change this at this point with several implementations shipping already. Quite unfortunate as your names do seem a lot better.


> * Unnecessary request headers - removing the Access-Control-Request- 
> Method and Access-Control-Request-Headers fields would substantially  
> simplify the design; it would necessitate that the server list all  
> methods and headers that are to be sent cross-origin in the preflight  
> response, but this is not an onerous requirement.

Content providers wanted the flexbility of not having to list every header in advance. Both so debugging headers and such would not have to be exposed and to reduce the payload.


> * Preflight cache - As specified, the preflight cache is very complex  
> and hard to understand. Removing the request headers above will help,  
> and would enable a switch from OPTIONS to HEAD for pre-flights, again  
> simplifying the design and allowing the use of a standard HTTP cache,  
> instead of a purpose-built one. Failing that, the material related to  
> the cache desperately needs a rewrite for clarity.

Could you elaborate on what is not clear? I'm not really sure how to make it better.


> * Access-Control-Allow-Credentials - Has the WG considered making this  
> header more fine-grained (e.g., allowing the authentication realm to be  
> carried)? What about including the challenge on the pre-flight response?

The idea of this flag is that the server does not inadvertently expose information it does not want to. We have not considered anything beyond that basic idea as far as I can remember.


> * Request header deletion - The model of giving the server control over  
> any and all additional request headers tightly couples the origin to the  
> format of requests. It may be desirable in some cases to add local  
> request headers (e.g., targeted at firewalls or proxies)  
> programmatically, but this would not be possible using this design  
> without coordination with the origin. Instead of a whitelist of "simple"  
> headers, why not have a blacklist of headers that have to be explicitly  
> allowed by the server (e.g., Cookie, Authorization)?

Because blacklists are inherently dangerous?


> * Response header deletion - Again, deleting all but a pre-defined list  
> of response headers is too draconian, and seriously limits extensibility  
> on the Web. Again, why not just a blacklist? What's the attack vector  
> here?

Implementors did not want a blacklist. The attack vector is the server inadvertently exposing headers it did not want to.


> * Chattiness - The protocol set out here requires a pre-flight request  
> every time a new URL is used; this will force Web sites to tunnel  
> requests for different resources over the same URL for performance/ 
> efficiency reasons, and as such is not in tune with the Web  
> architecture. A much more scalable approach would be to define a "map"  
> of the Web site/origin to define what cross-site requests are allowed  
> where (in the style of robots.txt et al; see also the work being done on  
> host-meta, XRDS and similar). I made this comment on an older draft a  
> long time ago, and have still not received a satisfactory response.

See crossdomain.xml. It is a security nightmare. Especially when a single origin is being used for several APIs.


> * Procedural definition - This specification is defined as a set of  
> procedural instructions for implementations. The advice that "User  
> agents MAY employ any algorithm to implement this specification, so long  
> as the end result is indistinguishable from the result that would be  
> obtained by the specification's algorithms" is at best fallacious  
> (besides leaving out servers); it sidesteps the question of what's  
> meaningful in determining what is "indistinguishable." Specifications in  
> this style unnecessarily constrain implementations, are more difficult  
> to understand (e.g., it's difficult to understand the operation of a  
> protocol mechanism such as a header without stepping through every  
> single algorithm in the spec), and often preclude their reuse for  
> unforeseen purposes.

Added servers. It's not clear to me how to rewrite the specification in a way that does not leave gaps. If you can find another editor who can do that for us that'd be ok I suppose.


> *** Minor and Editorial issues
>
> * Abstract - the use of "API" is a bit confusing.... suggest  
> "Specifications that enable an API to make cross-origin requests to  
> resources can..."

Done.


> * Introduction - What does "limit the amount of unsafe HTTP requests"  
> mean -- I may just not be getting the metric for 'amount.'

Yeah, removed amount.


> * Introduction - "Web application technologies" is stilted; while the WG  
> has a good concept of what a webapp is, in the greater world this phrase  
> means a server-side app.

Clarified.


> * Introduction - "A resource can include..." --> "A response can  
> include..."

Done.


> * Introduction - "User-agents are enabled to discover..." --> "User- 
> agents can discover..."

Done.


> * Introduction - "This specification is a building block for other  
> specifications, so-called hosting specifications."  This is an  
> unfortunate term. How about "Cross-Origin Application Specification",  
> "API Specification" or similar? (here and elsewhere)

Can you explain why it is an unfortunate term? (Things like "host language" seem to be used elsewhere within the W3C. I thought this would be a fine extension.)


> * Introduction - There's a large section at the end that's indented;  
> what's the significance of this?

It's an example.


> * Introduction - "If a server author has a..."  This phrase naturally  
> applies to the authors of servers; e.g., Apache, lighttpd, etc. It would  
> be more clear and correct to say 'resource author." (here and elsewhere)

I found two places, both changed.


> * Introduction - "...the resource combined with a header..." --> "the  
> response"

Done.


> * Introduction - "Using XMLHttpRequest a resource on  
> http://hello-world.example  can access the document as follows:" --> "A  
> client-side web application whose origin is http://hello-world.example  
> can access the resource using XMLHttpRequest as follows:"

Changed (though in a slightly different way).


> * Conformance Criteria - "A conformant server is one that..." --> "A  
> conformant resource is one that..."

I haven't done this yet. Does it still make sense to talk about a server processing model if we do this?


> * Terminology - "simple method" is misleading and too generic. Suggest  
> just using the term 'safe' in the RFC2616 sense (see substantial comment  
> about POST).

POST is not safe. (See reply to substantial comment.) :/


> * Syntax - Have the authors considered using ABNF, since HTTPbis is  
> already using it?
>
> * Syntax - If the 2616 BNF is retained, you need to explicitly point out  
> that LWS is allowed.

I have a note to update this once HTTPbis hits RFC status.


> * Syntax - all of these headers need to be registered with IANA; see  
> RFC3864. Note that publication as a W3C Rec is enough, but the  
> registration template needs to be in the document.

They are provisionally registered already. Where is it stated that the template needs to be inside the document?


> * Origin Request Header - will this refer to the Origin spec if/when it  
> is published?

That is the plan. There is a note in the source to do this if that specification makes it to RFC.


> * Server Processing Model - This section repeatedly uses the term "URL"  
> where "resource" is appropriate; a URL is an identifier for a thing, not  
> the thing itself.

Done.


> * Server Processing Model - shouldn't the list of origins include the  
> same origin, or is that considered completely external to this process?  
> A note about this would be helpful in either case.

Done.


> * Simple Cross-Origin Request and Actual Request - What should the  
> outcome be if the origin header is missing?

Added that it is out of scope for this specification.


> * Simple Cross-Origin Request and Actual Request - "Note: by failing  
> this request servers..." What does "failing this request" mean in this  
> context?

Reworded.


> * Preflight Request - "... or if parsing failed, do not set any  
> additional headers and terminate this set of steps."  Does this mean  
> that the server should continue processing as if CORS were not in  
> effect, or should a specific status code, etc. be returned? If it's the  
> former, this should be explicitly specified.

Done.


> * Preflight Request - "If any of the headers is not..." --> "If any of  
> the header field-names is not..."

Done.


> * Preflight Result Cache - it's not clear whether user-agents are  
> required to implement a cache or not; I'm assuming it's optional.

Yes, this follows allowing user agents to set max-age to zero. Added a note to make this more explicit.


> * Generic Cross-Origin Request Algorithms - for clarity, can this be  
> split up into separate subsections?

I added spacing instead. Does this work?


> * Generic Cross-Origin Request Algorithms - "new URL" --> "the URL  
> conveyed by the Location header in the redirect response"

Done.


> * Resource Sharing Check - "...and the resource contains zero or more  
> than one Access-Control-Allow-Credentials headers..." should end in  
> "header values" because a HTTP header can contain multiple values.

Done.

Thanks again!


-- 
Anne van Kesteren
http://annevankesteren.nl/

Received on Saturday, 13 June 2009 13:08:51 UTC