kadirahq / fast-render

Render you app even before the DDP connection is live. - magic?
MIT License
560 stars 80 forks source link

Access-Control-Allow-Credential CORS headers #162

Open mitar opened 8 years ago

mitar commented 8 years ago

I read a bit about security considerations and I am not sure if I agree with the CORS issues. Namely, to my understanding, the issue is that somebody can create trigger a request from evil.com to good.com and cookies will be send along to good.com and then evil.com could get the content meant for user on good.com.

But this is not really how CORS works. If evil.com makes a XHR request with cookies (with credentials), then good.com has to set Access-Control-Allow-Credential to true in the response for the evil.com to get access to content. It is not enough just for the server to respond with data, it also has to set Access-Control-Allow-Credential.

So instead of disabling fast render on CORS, it would be enough just to force Access-Control-Allow-Credential to false on any fast rendered HTTP response. This would allow potential use where one could do XHR requests to Meteor pages which do not require login.

Probably /sockjs should also make sure it never sets Access-Control-Allow-Credential to true, or it could just preemptively set Access-Control-Allow-Credential to false for all requests.

See here for more information.

cc @estark37

estark37 commented 8 years ago

@mitar I think sockjs sets ACAO: < origin > and Access-Control-Allow-Credential to true? https://github.com/sockjs/sockjs-node/blob/4561f500e92d3d9d8f66e64d6251c3d7340b252d/src/trans-xhr.coffee#L64

I don't remember how fast render works anymore but yeah, forcing ACAC to false is probably sufficient.

mitar commented 8 years ago

Thanks for responding.

So I am talking about this section in the README, which more or less takes from your mailing list post. In it you worry about using cookies because you worry somebody could trigger cross-origin request using that cookie and that would reveal data for it in the response. But that would happen only if response also sets Access-Control-Allow-Credential to true, which, I do not why it would.

So, to be clear, the issue can happen only if Access-Control-Allow-Credential is explicitly set to true. If it is non-existent or if it is set to false, evil.com cannot get the response.

(They can still do side-effects attack, but frankly, one should never really have side-effects in publications.)

I think sockjs sets ACAO: < origin > and Access-Control-Allow-Credential to true?

Oho, this is bad. This is unnecessary for Meteor and it should probably be forced to false. Meteor is not using cookies for DDP and it should not want/need any cookies for authentication on sockjs endpoint.

arunoda commented 8 years ago

May be that's set for load balancing with cookies. Not exactly sure about it.

arunoda commented 8 years ago

Specially load balancing long polling not XHR.

mitar commented 8 years ago

You do not need load balancing cookies to get to the DDP endpoint. You want them to get only up to the reverse proxy, no? So reverse proxy could see cookies, load balance, and them strip them away, before passing them to the DDP endpoint.

arunoda commented 8 years ago

@mitar That can do. But, we can't always expect they'll remove it. Anyway, this should address in the Meteor repo with someone knows better on this area.

I may need to refresh my knowledge before proceeding with fast-render. Does this restriction cause any issue for you?

If NO, we can differ this.

mitar commented 8 years ago

Does this restriction cause any issue for you?

No, it was just something I observed.