- From: Anne van Kesteren <annevk@opera.com>
- Date: Wed, 23 Sep 2009 15:54:59 +0200
- To: "Frederick Hirsch" <frederick.hirsch@nokia.com>, "public-webapps WG" <public-webapps@w3.org>
On Tue, 30 Jun 2009 17:11:04 +0200, Frederick Hirsch <frederick.hirsch@nokia.com> wrote: > I have some basic comments and questions on "Cross-Origin Resource > Sharing", W3C Working Draft 17 March 2009 > http://www.w3.org/TR/2009/WD-cors-20090317/ Going forward, may I suggest you review: http://dev.w3.org/2006/waf/access-control/ It usually has better wording, less bugs, and less spelling mistakes than the latest version on TR/. > Perhaps some of these have been answered already and there are probably > others I did not list. I'll try to answer them all. (Though having done that now I didn't have one to all. Maybe someone else can help out?) > 1. GET can have side effects, so can we assume it is safe? Thus does it > not also always require pre-flight check? The threat is not the actual request but exposing the response. A cross-origin request can already be made by a variety of means, e.g. <img>, <script>, etc. > 2. How is Origin set, is it always correct, how is it set for widgets? > Perhaps the document should discuss this. This is out of scope of the CORS specification. It depends on the specification that uses cross-origin requests and how it sets the "source origin" parameter. > 3. Is there a race condition between preflight request and subsequent > request? What if server changes policy, e.g. allowed methods in this > time. Is there a requirement on the maximum time between both these > actions? The preflight request only serves as an indication that the resource on the server is aware of the policy. To guarantee safety of the server the Origin header is still there in the actual request so this should not give race condition problems. > 4. Shouldn't the specification require header integrity protection so > they cannot be rewritten during transit? This could require TLS or > secure channel or header signing so the mechanism may not be defined in > the specification, but shouldn't integrity protection be a MUST? This is encouraged, but for a lot of scenarios this is not needed. E.g. when using XSLT resources from another server it is not necessarily needed to have integrity protection, but CORS is needed because cross-origin requests are disabled for XSLT. > 5. Will Access-Control-Allow-Origin header scale if all possible URLs > must be listed (I'm thinking of airline example). This comment seems to be based on an earlier draft. Access-Control-Allow-Origin nowadays is assumed to simply echo the Origin request header. > 6. Security sections should be normative and have normative statements. The actual requirements that are important are part of the processing sections. > Section 3 > - remove statement that section is non-normative > - Replace "Authors of client-side Web applications are strongly > encouraged to validate content retrieved from a cross-origin resource as > it might be harmful." with "Authors of client-side Web applications > SHOULD validate content retrieved from a cross-origin resource as it > might be harmful." I'm not sure what the effect of this change is. This is mostly the reason I have not phrased it that way to begin with. > editorial, replace "This section lists advice that did not fit anywhere > else." with "This is general security considerations, more detailed are > provided in specific sections." Would that actually be true, though? Maybe I should move it to a new section on authoring advice. What do you think? > Section 5.3 > - remove statement that section is non-normative I don't think the statements should be normative. > - Replace “are strongly encouraged to” with SHOULD in paragraph 1 > - Replace “are strongly encouraged to” with SHOULD in paragraph 2 I'm not sure what the effect of this change is. > - Replace "To provide integrity protection of resource sharing policy > statements usage of SSL/TLS is encouraged." with statement that > "Integrity protection for headers MUST be provided. This MAY take the > form of TLS, header signing, or other mechanisms, as appropriate." I don't think this is needed in all cases. > Section 6.3 > - remove statement that section is non-normative > > - Why is the statement "User agents are to carefully follow the rules > set forth in the preceding sections to prevent introducing new attack > vectors." needed? Remove it, since the normative rules in this > specification must be followed for compliance, and if important should > be normative. Fair enough, removed. > - Replace “are allowed to” with SHOULD in paragraph 2 That would defeat the whole purpose of caching and various other things, so no. > - Replace “are encouraged to” with SHOULD in paragraph 3 I'm not sure what the effect of this would be. Seems to depend on the implementation what would be wise. > - Replace "User agents are encouraged to apply security decisions on a > generic level and not just to the resource sharing policy." with "These > MUST apply equally to access through APIs (e.g. XMLHttpRequest) or > through inlined content (e.g. iframe, script, img)." (taking from WARP) > It might be that I do not understand this statement, some clarification > would be helpful. > > - Replace “is encouraged” with MUST in last sentence. What exactly is unclear? I do not want to make requirements on things that are out of scope of CORS. I would like to make an encouragement however so that it is considered. > 7. Is there a resolution to Mark Nottingham's concerns expressed in > http://lists.w3.org/Archives/Public/public-appformats/2008Jan/0226.html > ? Aren't these reasonable concerns? A lot of those comments no longer apply. Having said that Mark Nottingham has since re-reviewed the document and based on that I have in agreement with him raised two issues that are not yet closed: http://www.w3.org/2008/webapps/track/issues/89 http://www.w3.org/2008/webapps/track/issues/90 If there is something in particular you would like me or someone else to explain please let us know. > 8 Is the party of permissions the site (origin) requesting the content? No. The party that distributes the content indicates whether or not to share. > What about per-identity permissions? Credentials are supported. > How will this work with OAuth etc? OAuth can probably make use of this new protocol to simplify some matters. > Will this be a general issue? I don't think there is an issue. > 9. What is the resolution to the "confused deputy" concern? Should the > specification note the concern? I'm not sure if the WG has discussed/ > resolved this issue. Could you be more specific? Apart from redirects and the aforementioned two issues I'm not aware of any outstanding issues with the specification. > 10 requirements section: Requirement #1 - What is the implied > alternative for additional protection than firewall? I don't know. > 11 requirements section: Is Requirement #2 accurate given that GET can > have side effects so is not always safe? Yes. See above how GET requests are already possible. > 12 requirements section: How did Requirement #4 impact this > specification? How does this attack come into play with this > specification and if it does, how does the specification address it? Section 7.4 is about this. This is technically addressed in other specifications but it was part of the requirements we came up with. > 13 requirements section: Is requirement #8 possible? Isn't > configuration required to return headers, deal with pre-flight requests > etc? Or would this be addressed by Mark Nottingham's suggestion to > always return access header? Since part of the requirement is that the user can scripts he can set headers from such scripts. > 14 requirements section: Requirement #17, does this include integration > with identity management solutions? I don't know. > 15 Editorial: Abstract > Extend sentence "This document defines a mechanism to enable client-side > cross-origin requests." to say, by whom. That is already stated in the next sentence. > Mention widgets as well as web browser cases in abstract? I'd rather keep the abstract short and leave this to the introduction. > 16 Editorial: Section 1 > "The user agent validates that the value and origin of where the request > originated match." > Value has not been defined. Sentence is not very clear. It is introduced in the previous paragraph. > 17 Editorial: Section 1 > Replace > "User agents are enabled to discover whether a cross-origin resource is > prepared to accept requests using a non-GET method from a given origin." > with > "User agents are enabled via preflight checking..." Done. > 18 Editorial: Section 1 > In "This extension enables server-side applications to enforce > limitations on the cross-origin requests that they are willing to > service" > clarify the "limitations" Done. > 19 Editorial: Section 2 > > Replace "This specification is written for servers and user agents." > with "This specification is written for implementers of user agents that > enforce policy, APIs that use it, and web servers that provide content > that may be used in cross-site manner." That seems self-evident. > 20 Editorial: Section 2 > Why is the term "hosting specifications" given, is it common > terminology? Can it be removed? It has been removed in the editor's draft. > 21 Editorial: Section 2 > Is a conformant server one that returns appropriate headers for requests? It is one that implements the criteria in section 5. > 22 Editorial: Section 4.5 > Where is the full list of headers defined? is a reference needed? The full list of headers is the infinite list. It is whatever the server deems suitable. > 23 Editorial: Section 5.1 #1 > Can the list of origins be unbounded in practice? Yes. E.g. I could have an XSLT file and don't care who uses it. > 24 Editorial: Section 6 > Mark "Everything with regards to redirects might change a little to more > closely adhere to HTTP redirect semantics." as an editors note. This seems to be gone from the editor's draft. As you might have seen we're working on addressing the remaining issue with redirects. > 25 Editorial: Section 6.1 > some of the spacing between items seems to need additional space Could you be more specific? > 26 Editorial: Section 7.3 > Replace "progresing" with "progressing" This appears to be already fixed. > Frederick Hirsch Thanks a lot for your elaborate review! -- Anne van Kesteren http://annevankesteren.nl/
Received on Wednesday, 23 September 2009 13:55:44 UTC