[BUG] Sometimes missing Vary: Origin header may lead to Web cache poisoning

[jub0bs]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

The CORS middleware fails to add a Vary: Origin header in responses in some cases where it's needed.

Multiple origins allowed vs CORS request from a disallowed origin

If multiple origins are allowed (but not via the wildcard) and a CORS request comes from a disallowed origin, the response lacks a Vary: Origin header. Here's a failing test case that illustrates the problem:

func TestMultipleOriginsAllowedVersusCorsRequestFromDisallowedOrigin(t *testing.T) {
	cors := CORS(
		AllowedOrigins([]string{
			"https://example.com",
			"https://example.org",
		}),
	)
	r := newRequest(http.MethodGet, "https://backend.com")
	r.Header.Set("Origin", "https://disallowed.com")
	rr := httptest.NewRecorder()

	testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})

	cors(testHandler).ServeHTTP(rr, r)
	resp := rr.Result()
	if resp.Header.Get("Vary") != "Origin" {
		t.Fatal("Origin not listed in Vary header")
	}
}

Why is it problematic? Because caching intermediaries may cache such a response, which would subsequently get served to clients that do issue requests from allowed origins. In essence, this would lead to a limited form of denial of service for as long as that "bad" response remains in the cache.

Multiple origins allowed vs non-CORS request

A similar issue exists when the request lacks an Origin header. Here's a failing test case that illustrates the problem:

func TestMultipleOriginsAllowedVersusNonCorsRequest(t *testing.T) {
	cors := CORS(
		AllowedOrigins([]string{
			"https://example.com",
			"https://example.org",
		}),
	)
	r := newRequest(http.MethodGet, "https://backend.com")
	rr := httptest.NewRecorder()

	testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})

	cors(testHandler).ServeHTTP(rr, r)
	resp := rr.Result()
	if resp.Header.Get("Vary") != "Origin" {
		t.Fatal("Origin not listed in Vary header")
	}
}

Multiple origins allowed via AllowedOriginValidator

Similar issue for users who opt for AllowedOriginValidator to allow multiple origins:

func TestMultipleOriginsAllowedViaCallback(t *testing.T) {
	cors := CORS(
		AllowedOriginValidator(func(origin string) bool {
			switch origin {
			case "https://example.com", "https://example.org":
				return true
			default:
				return false
			}
		}),
	)
	r := newRequest(http.MethodGet, "https://backend.com")
	r.Header.Set("Origin", "https://example.com")
	rr := httptest.NewRecorder()

	testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})

	cors(testHandler).ServeHTTP(rr, r)
	resp := rr.Result()
	if resp.Header.Get("Vary") != "Origin" {
		t.Fatal("Origin not listed in Vary header")
	}
}

Edit: this specific problem was already reported in #244

Expected Behavior

In both cases, the middleware should list Origin in a Vary response header.

Steps To Reproduce

Reproducible in v1.5.1.

Anything else?

• Fetch standard - CORS protocol and HTTP caches