salcho edited a comment on pull request #439:
URL: https://github.com/apache/wicket/pull/439#issuecomment-667014819


   Hi @svenmeier! 
   
   @eozmen410 has pointed out something that I missed before that brings us 
back to the reason why this is implemented the way it is and why we rejected 
the alternative you propose in our design:
   
   - The legacy check relies on the Referer and Origin headers to determine 
whether a request is cross-origin. These headers are not reliable since they 
are not always present and so the legacy check produces false positives, that 
is: it will, for instance, reject same-site requests that are legitimate 
because they might not carry Referer/Origin! Whenever we can determine the 
origin of a cross-origin request, this check works fine.
   
   - Fetch Metadata solves this problem by explicitly marking the request as 
same/cross-origin reliably, even in the absence of Origin/Referer, which makes 
the new check false postive-free, hence improving the previous check. That is, 
in scenarios where the legacy check would have rejected a legitimate request 
(because there was no Origin header), the new check can now approve the request 
because sec-fetch-site indicates same-origin.
   
   Because of this, running both checks serially would produce contradictions. 
The new check would approve and the legacy would reject. That is: whenever the 
legacy check makes a decision correctly, both legacy and new will agree on the 
final decision, but in cases where the legacy check produces a false positive, 
both legacy and new will contradict each other!
   
   In the model you're proposing, if at least one check rejects then the 
request is fully rejected and so we would be back in the world of false 
positives that we're trying to get out of.
   
   In summary: while having multiple Resource Isolation Policies is a good idea 
for the future (for implementing specific policies for iframes, for instance), 
this PR should be seen as improving **only one** of those policies and not as 
adding an additional one. 
   
   I hope that makes sense, but we would be happy to jump on a call or chat if 
you'd like to :)
   
   What do you think?
   
   (tagging @papegaaij for visibility and correctness, since he's very familiar 
with the legacy check)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to