actix_cors blocks requests that should be allowed

[CapableWeb]

Expected Behavior

If I have app.example.com as a whitelisted Origin but both the backend + frontend is loaded from app2.example.com, request should be allowed.

Current Behavior

If I have app.example.com as a whitelisted Origin but both the backend + frontend is loaded from app2.example.com, requests are blocked, even though Host + Origin is the same. Browsers allow these requests to happen (no OPTIONS request sent, as Origin + Host is the same), but actix_cors decides to forcefully block the request.

Relevant code from actix_cors:

actix-extras/actix-cors/src/inner.rs

Lines 73 to 99 in 1561bda

	pub(crate) fn validate_origin(&self, req: &RequestHead) -> Result<(), CorsError> {
	// return early if all origins are allowed or get ref to allowed origins set
	#[allow(clippy::mutable_key_type)]
	let allowed_origins = match &self.allowed_origins {
	AllOrSome::All if self.allowed_origins_fns.is_empty() => return Ok(()),
	AllOrSome::Some(allowed_origins) => allowed_origins,
	// only function origin validators are defined
	_ => &EMPTY_ORIGIN_SET,
	};
	// get origin header and try to parse as string
	match req.headers().get(header::ORIGIN) {
	// origin header exists and is a string
	Some(origin) => {
	if allowed_origins.contains(origin) || self.validate_origin_fns(origin, req) {
	Ok(())
	} else {
	Err(CorsError::OriginNotAllowed)
	}
	}
	// origin header is missing
	// note: with our implementation, the origin header is required for OPTIONS request or
	// else this would be unreachable
	None => Err(CorsError::MissingOrigin),
	}
	}

Possible Solution

actix_cors should not forcefully cancel/error on requests where Origin and Host is the same header. In fact, if there is no OPTIONS request, actix_cors should not care about the actual request at all, besides setting the right headers on the Response. As it stands today, actix_cors is trying to be overly protective of responses while it doesn't add anything regarding security.

[robjtede]

The CORS protocol is activated by the presence of an Origin header. For same origin requests, it should not be sent. That or add the required domain to the allow list.

[CapableWeb]

Small example describing the problem:

use actix_cors::Cors;
use actix_web::{http::header, App, HttpServer, HttpResponse, Responder, get};

#[get("/")]
async fn index() -> impl Responder {
    HttpResponse::Ok().body("Hello")
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(move || {
        App::new()
            .wrap(
                Cors::default()
                    .allowed_origin("http://example.com")
                    .allowed_methods(vec!["GET"])
                    .allowed_headers(vec![header::AUTHORIZATION, header::ACCEPT])
                    .allowed_header(header::CONTENT_TYPE)
                    .supports_credentials()
                    .max_age(3600),
            )
            .service(index)
    })
    .bind(("0.0.0.0", 80))?
    .run()
    .await
}

This example should be blocked in the browser if you try to access the endpoint from a Origin that is either example.com or different from where the server is hosted. If the server is hosted at my-app.com + the frontend is also loaded from my-app.com, the OPTIONS request won't happen, and CORS shouldn't get in the way (in the browser) but instead actix-cors blocks the request from happening, with the message Origin is not allowed to make this request, even though it should be allowed.

Another example, you should be able to load this endpoint regardless of what the Origin header is set to (or doesn't exists at) if you're using anything other than a browser (like cURL) as no pre-flight OPTIONS request happens at all. Instead of responding with the CORS headers set, actix-cors outright blocks the response from being served.

If this issue is fixed, you should be able to run the following command successfully after running the example above, even though the Origin/Host doesn't match what gets passed into allowed_origin:

$ curl -v -H "Origin: http://example.test" -H "Host: https://example.test" localhost

[CapableWeb]

I guess the TLDR of the problem here is that actix-cors is trying to add its own security mechanism instead of relying on browsers own CORS handling, so this library becomes much more than just actix-cors, it becomes something like actix-origin-whitelist which is a different set of features.

[robjtede]

Firstly, the CORS protocol says nothing about trying to match Origin and Host to and allowing the request to pass and there's no precedent in other libraries I've looked at to add this hidden behavior; so, that cannot be part of the solution here.

Sadly, the Fetch spec where CORS is defined is very targetted at clients and not servers:

Ultimately server developers have a lot of freedom in how they handle HTTP responses...

Since it's concern is only on visibility of response data, it gives almost no guidance on how servers should handle these requests; only that the client should not be able to read responses without Access-Control-Allow-* headers.

Though, I understand your point now; that instead of processing the request and serving the response without the headers, we simply block the request. Plus, the example of "this cURL command should work even without the origin being allowed" and compelling (though not because of the Host header).

So I think some sort of change is needed. The question is do we make this an opt-in, opt-out, or always on behavior. I need to verify some browser behaviors but I suspect you are right that this can be allowed by default. I'll get back to you later today...

[CapableWeb]

Firstly, the CORS protocol says nothing about trying to match Origin and Host to and allowing the request to pass and there's no precedent in other libraries I've looked at to add this hidden behavior; so, that cannot be part of the solution here.

True, I wasn't trying to say precisely that, although my wording could have been clearer perhaps. I meant more that in case the Origin (the webpage) matches the backend (Host) it's trying to reach, browsers will implicitly ignore CORS (correctly) as both the Origin and Host is at the same Origin (in terms of what browsers consider).

that instead of processing the request and serving the response without the headers, we simply block the request

I think the correct behavior would be to not remove the headers at all, and proceed as if everything is normal. If the request is coming from the browser under a different Origin, the browser would already have sent a OPTIONS request and checked the headers there, which would include its Origin or not. If it's not, the browser doesn't make the actual request the page requested, and if it, the browser does the request. If the request is coming from something else than a browser, CORS doesn't matter anyways.

The question is do we make this an opt-in, opt-out, or always on behavior.

Would make sense to introduce a argument/parameter to the middleware in order to avoid breakage in the API interface, as maybe some users are relying on that behavior. I'd be fine with a deny_mismatched_origin_requests (or similar) parameter that defaults to true (current behaviour).

Thanks a lot for the quick replies by the way, appreciate it a lot!

[CapableWeb]

@robjtede if we agree that this issue should be solved, I can take a stab at solving it and sending a PR. Does that sound OK with you?

[robjtede]

Yeah sounds good.

[CapableWeb]

@robjtede PR available here: #287