-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CORS #3
Comments
(Also, rename this repository to "origin-policy" perhaps? "Origin Policy" or "Origin Manifest" seems reasonable.) This is the manifest the web needs. |
Yeah. The CORS bits are little more than a strawman. I have some vague ideas about the behaviors I'd like to enable (namely "Hey, I know what CORS is. I'm going to examine the incoming headers the way I'm supposed to. Quit it with these preflights."), but I'm totally open to suggestions about how to make them work. Can you spell out the syntax you'd like to see? |
Also: renamed. :) |
Name the overall field "cors-preflight". Then we have "origins", "methods", "headers" as subfields and we'd have these subfields prefixed with "unsafe-" (for the credentials case) as well. When prefixed with "unsafe-" these fields cannot contain "*". Both prefixed and unprefixed fields can be used together (covering the different request scenarios). |
Hrm. Is that much complexity valuable? As another strawman, could we just have something like A server that expects to service cross-origin requests could do something like:
That would allow anonymous access to everything without a preflight, and allow the server to override the allowed origins for specific requests by sending a custom The boolean switch wouldn't give much granularity in terms of allowing/denying preflights on a case-by-case basis. If folks are using preflights as a firewall of sorts, then this certainly wouldn't be enough. Perhaps @mnot, et al. could point at the kinds of usage that Akamai's customers care about? I think @igrigorik and @jakearchibald have opinions about this as well. |
The complexity matches the complexity of CORS preflights today. And actually offers some additional functionality if we allow multiple origins. The reason for the complexity is to avoid falling in the same pitfall as crossdomain.xml. We need folks to understand that they put themselves at the risk of XSRF, especially in the credentials case. It's a rather weird argument that now that we have an origin-wide manifest, we can drastically simplify the security handshake and granularity. That will not help folks think through the problem space. (I especially like the usage of "unsafe-" for requests that contain credentials.) Note that I still think that we should very much require CORS on all responses. That's why I think we should call it "cors-preflight". We shouldn't do any defaulting for CORS on responses and keep that as a per-URL thing. |
Ok, before I argue with you ( :) ), let me try to parrot back the suggestion you've made: The policy would contain something like:
This policy would bypass all CORS preflight requests whose credentials mode is Is that more or less what you have in mind? It seems like a reasonable first step, and I generally agree with your assessment of the risk of blindly removing the preflight for credentialed requests, so I won't argue too vehemently in favor of anything else, however, this is The Internet, so:
As long as the headers that a policy manifest can set are static, it seems that we'd be limited to either whitelisting a specific origin as being same-origin with us, or whitelisting |
Yeah, and I suppose you are right that we could have response defaulting too. Should be a separate field so they do get confused (with preflights). |
Don't we need "maxage"? Do we want to make "origins", "methods" and "headers" field required for "cors-preflight" section? Maybe that's for the complexity we want for preventing misconfiguration even for non-credentialed ones. As we now allow wildcards for all of them, I also have a feeling that we can make them optional for non-credentialed ones (just for allowing for setting defaults). Explicit inclusion of the Access-Control-Allow-Methods header and Access-Control-Allow-Headers header in a preflight response (in addition to Access-Control-Allow-Origin which is also included for non-preflight CORS response) is still required in the Fetch Standard maybe because it's good indication that the server correctly understands the CORS preflight algorithm. But in the origin-policy, it's clear that it's indicating the signal which was carried by the CORS preflight protocol, i.e. the server admin understands what it does correctly. "unsafe-cors-preflight-with-credentials" syntax looks good. |
I think this is implicit with the cache lifetime of the manifest. That is, as long as we have an origin policy manifest for an origin, we'll apply these settings to the preflight requests we require.
I could get behind that default for non-credentialed preflights, though I also don't think it much matters as long as it's a configuration we allow. Typing three lines into a manifest is not a difficult task, so keeping a strict default isn't unreasonable. |
I see. There could be demand for fine-grained lifetime configuration, but sgtm for the first version. So, the Fetch algorithm would consult the origin policy cache every time to check need for preflight by treating the result as max-age=0. |
Yeah, checking Origin Policy has to be integrated with Fetch anyway so no need to duplicate that with the CORS-preflight cache. And since it effectively replaces the CORS-preflight cache when in use I don't think we need max-age. (If you don't want an origin-wide policy for CORS-preflights, simply don't use this.) |
I would prefer requiring all three if you want to set them all to "*" for non-credentialed requests. Otherwise we need to support some other value to set them to their original default and folks will get confused. |
I've taken a stab at this in https://mikewest.github.io/origin-policy/#cors-preflight-member (with some examples), and https://mikewest.github.io/origin-policy/#fetch-bypass-preflight (which ought to be called somewhere in the viscinity of step 5.1 of https://fetch.spec.whatwg.org/#http-fetch). WDYT? |
Looks reasonable. Where you describe the value space you probably want to mention the restrictions for the unsafe variant. |
Ok. Added a requirement there. I think this is good enough for a first pass; closing this out. Thanks for the feedback! |
Filed #11 but lgtm except for that. Thanks |
Consistently naming it "cors-preflight" seems good. This should only affect requests, not responses.
Allowing credentials while not listing origins should not happen in valid examples, only invalid examples. (Or is Chrome security okay with allowing wildcarded access to credentialed content?)
The syntax should probably be such so that you can make separate declarations for requests without credentials (maybe there origins is *) and those with.
There should be a section on crossdomain.xml and how this is somewhat better since a) we still check on each request if the URL actually knows about the policy (although this doesn't help with cors-preflight, the justification for that can be that we require a specific list of origins if you want to allow credentials), and b) CORS is still required for responses.
The text was updated successfully, but these errors were encountered: