Fixes #10220 - Implement CrossOriginHandler.

[sbordet]

Introduced CrossOriginHandler.
Added cross-origin Jetty module.
Added CrossOriginHandler documentation to the programming guide. Added CrossOriginHandler documentation to the operations guide. Added cross-origin headers to the HttpHeader enum. Added test cases.
Deprecated ee10 CrossOriginFilter.

[sbordet]

@gregw @joakime @lorban @lachlan-roberts things to ponder:

• Deliver preflight requests? Differently from CrossOriginFilter, CrossOriginHandler by default does not deliver preflight requests to the application Handler. With CrossOriginFilter, it was ok because method OPTIONS was typically never handled by applications and automatically replied with a 200 by HttpServlet. Applications would override doGet() or doPost().
  But now delivering a preflight to an application Handler would force all application Handlers to do a check on the HTTP method, and do something if it's OPTIONS.

• Perform headers matching? CrossOriginFilter was doing a match on the headers and when failing that check, CrossOriginFilter was not adding the CORS headers. I do not think this is necessary, as it is the browser's job to verify whether the headers match. The server can just reply with the configured headers, and the browser will do the rest.

• What to do in case the origin does not match? Currently we don't add the CORS headers, so for HTTP the browser receives the response, but does not offer it to the client-side application, which will see a failure instead.
  But for WebSocket, browsers do not care about CORS. This means that an evil chat user Bob can lure Alice to open an evil page from his website, which downloads a script to Alice's browser, that opens a WebSocket connection to a well-known chat server that Alice uses. This may end up in an attack called CSWSH.
  However, we can block these requests at the CrossOriginHandler (with an additional configuration parameter that is currently missing), say deliverNonMatchingOriginRequest (defaults true), so that it can be configured to false, in which case CrossOriginHandler will return a 400 which would fail the WebSocket upgrade (as well as other non-matching HTTP requests).

[lachlan-roberts]

But now delivering a preflight to an application Handler would force all application Handlers to do a check on the HTTP method, and do something if it's OPTIONS.

But if we don't have this CrossOriginHandler wrapping our handler, we could still receive requests with method OPTIONS anyway, and the handler will still have to figure out what to do with it? And if they are using a ee8/ee9/ee10 ServletContextHandler it should still automatically reply with a 200.

Perform headers matching?

I agree that it doesn't seem necessary to check it on the server because the browser should enforce it. But then I don't understand what the purpose of the Access-Control-Request-Headers request header is for, now we don't even check it at all in the CrossOriginHandler. Maybe if that header is present we should still do the server side check for whether the headers are allowed?

What to do in case the origin does not match?

I think we do need something to deal with the CSWSH attacks.
So either that configuration to return 400 for non matching requests.
Or if the origin does not match we could always check if the request is a websocket upgrade?

[sbordet]

But if we don't have this CrossOriginHandler wrapping our handler, we could still receive requests with method OPTIONS anyway, and the handler will still have to figure out what to do with it? And if they are using a ee8/ee9/ee10 ServletContextHandler it should still automatically reply with a 200.

If you don't have COH, yes, a core Handler would need to deal with the OPTIONS preflight.
If eventually a ServletContextHandler is there, it would likely be replied with 200 (unless the application overrides service() and not doGet() or doPost()).

The preflight just seems an implementation detail of CORS, so not worth delivering it: we short-circuit at COH level.
If you really want it delivered, though, you can opt-in and configure COH.deliverPreflight=true.

But then I don't understand what the purpose of the Access-Control-Request-Headers request header is for, now we don't even check it at all in the CrossOriginHandler. Maybe if that header is present we should still do the server side check for whether the headers are allowed?

I think Access-Control-Request-Headers is just informative.
Let's assume the request has a header that is not allowed. What is the server supposed to do? It has to fail the CORS preflight, and the only way to do that is to either not add any CORS headers, or to add an Access-Control-Allow-Headers in the response that does not contain the non-allowed request header, and the browser will do the rest.
So in summary COH just need to add the Access-Control-Allow-Headers in the response.

In case of a dynamic decision about Access-Control-Allow-Headers by looking at Access-Control-Request-Headers, an application could just ask to deliver the preflights, and manage them dynamically.

I think we do need something to deal with the CSWSH attacks.
So either that configuration to return 400 for non matching requests.

I agree that a configuration to return 400 for mismatched origins is good to have.

Or if the origin does not match we could always check if the request is a websocket upgrade?

We could make a special case for the WebSocket upgrade.

However, I wonder whether we need both?
A "rogue" cross-origin request could change the state of the server (e.g. write to a RDBMS) and even if then the browser fails it, the effect on the server is performed anyway.

So we could have one parameter for generic origin mismatch (defaults to deliver=true), and one specific for WebSocket origin mismatch (defaults to deliver=false).

[lachlan-roberts]

If you really want it delivered, though, you can opt-in and configure COH.deliverPreflight=true.

Works for me.

---

I think Access-Control-Request-Headers is just informative.

It says here https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS that:

The Access-Control-Request-Headers header notifies the server that when the actual request is sent, it will do so with X-PINGOTHER and Content-Type custom headers. Now the server has an opportunity to determine whether it can accept a request under these conditions.

which seems to imply that the server should do something about it if it cannot accept the request.

---

So we could have one parameter for generic origin mismatch (defaults to deliver=true), and one specific for WebSocket origin mismatch (defaults to deliver=false).

I think this is the way to go.

[sbordet]

@lachlan-roberts

which seems to imply that the server should do something about it if it cannot accept the request.

Sure, but what should the server do?

It's a preflight request, so the only things the server can do is to:

1. 200 + no CORS headers (the client fails)
2. 200 + Access-Control-Allow-Headers that don't match (so the client fails)
3. 400

Solution 2 is the trivial one so I went for that.

Doing an expensive match on the server to figure out you don't want to add the CORS headers (solution 1) or figure out you have to send a 400 (solution 3) seems a lot of unnecessary work.

With COH.deliverPreflight=true, an application can do whatever it wants, but needs to be aware of CORS.

[lachlan-roberts]

@sbordet why do we not validate the request headers against allowedHeaders for non-preflight requests?

[sbordet]

@sbordet why do we not validate the request headers against allowedHeaders for non-preflight requests?

Because ACCESS_CONTROL_REQUEST_HEADERS is only sent in preflight requests.

[gregw]

You missed some of my previous review comments?

[lachlan-roberts]

Because ACCESS_CONTROL_REQUEST_HEADERS is only sent in preflight requests.

Ok. I was thinking that the browser might not send a preflight request and we'd have to check the actual headers of the request. But now I see that if it has custom headers the browser should always send a preflight request.

[lachlan-roberts]

failing test but other than that looks good to me

[gregw]

use ensureField:

Suggested change

	response.getHeaders().add(VARY_ORIGIN);
	response.getHeaders().ensureField(VARY_ORIGIN);