From 47e430682332f3068da20c6a5c8f1c8862804dc2 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 27 Jun 2023 00:15:29 +0200 Subject: [PATCH] fix(gateway): include on subdomain redirects Context: https://github.com/ipfs/kubo/issues/9983#issuecomment-1599673976 Closes #9983 --- gateway/gateway.go | 22 +++++++---- gateway/gateway_test.go | 85 +++++++++++++++++++++++++++++++++++++++++ gateway/handler.go | 22 +++++++---- gateway/hostname.go | 17 +++++++-- 4 files changed, 127 insertions(+), 19 deletions(-) diff --git a/gateway/gateway.go b/gateway/gateway.go index cc0babba1..cf2ca9104 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -320,20 +320,22 @@ func cleanHeaderSet(headers []string) []string { return result } -// AddAccessControlHeaders adds default HTTP headers used for controlling -// cross-origin requests. This function adds several values to the -// [Access-Control-Allow-Headers] and [Access-Control-Expose-Headers] entries. +// AddAccessControlHeaders ensures safe default HTTP headers are used for +// controlling cross-origin requests. This function adds several values to the +// [Access-Control-Allow-Headers] and [Access-Control-Expose-Headers] entries +// to be exposed on GET and OPTIONS responses, including [CORS Preflight]. // -// If the Access-Control-Allow-Origin entry is missing a value of '*' is +// If the Access-Control-Allow-Origin entry is missing, a default value of '*' is // added, indicating that browsers should allow requesting code from any // origin to access the resource. // -// If the Access-Control-Allow-Methods entry is missing a value of 'GET' is -// added, indicating that browsers may use the GET method when issuing cross +// If the Access-Control-Allow-Methods entry is missing a value, 'GET, HEAD, +// OPTIONS' is added, indicating that browsers may use them when issuing cross // origin requests. // // [Access-Control-Allow-Headers]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers // [Access-Control-Expose-Headers]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Expose-Headers +// [CORS Preflight]: https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request func AddAccessControlHeaders(headers map[string][]string) { // Hard-coded headers. const ACAHeadersName = "Access-Control-Allow-Headers" @@ -346,8 +348,12 @@ func AddAccessControlHeaders(headers map[string][]string) { headers[ACAOriginName] = []string{"*"} } if _, ok := headers[ACAMethodsName]; !ok { - // Default to GET - headers[ACAMethodsName] = []string{http.MethodGet} + // Default to GET, HEAD, OPTIONS + headers[ACAMethodsName] = []string{ + http.MethodGet, + http.MethodHead, + http.MethodOptions, + } } headers[ACAHeadersName] = cleanHeaderSet( diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 46ce75113..37576193b 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -619,6 +619,91 @@ func TestIpnsBase58MultihashRedirect(t *testing.T) { }) } +// TestCORSPreflightHeaders ensures CORS headers are present in HTTP OPTIONS responses +// https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request +func TestCORSPreflightHeaders(t *testing.T) { + backend, root := newMockBackend(t) + + // Expect boxo/gateway library's default CORS allowlist for Method + headerACAM := "Access-Control-Allow-Methods" + expectedACAM := []string{http.MethodGet, http.MethodHead, http.MethodOptions} + + // Set custom CORS policy to ensure we test user config end-to-end + headerACAO := "Access-Control-Allow-Origin" + expectedACAO := "https://other.example.net" + headers := map[string][]string{} + headers[headerACAO] = []string{expectedACAO} + + ts := newTestServerWithConfig(t, backend, Config{ + Headers: headers, + PublicGateways: map[string]*PublicGateway{ + "subgw.example.com": { + Paths: []string{"/ipfs", "/ipns"}, + UseSubdomains: true, + DeserializedResponses: true, + }, + }, + DeserializedResponses: true, + }) + t.Logf("test server url: %s", ts.URL) + + testCORSPreflightRequest := func(t *testing.T, path, hostHeader string, requestOriginHeader string, code int) { + req, err := http.NewRequest(http.MethodOptions, ts.URL+path, nil) + assert.Nil(t, err) + + if hostHeader != "" { + req.Host = hostHeader + } + + if requestOriginHeader != "" { + req.Header.Add("Origin", requestOriginHeader) + } + + t.Logf("test req: %+v", req) + + // Expect no redirect for OPTIONS request -- https://github.com/ipfs/kubo/issues/9983#issuecomment-1599673976 + res, err := doWithoutRedirect(req) + assert.Nil(t, err) + defer res.Body.Close() + + t.Logf("test res: %+v", res) + + // Expect success + assert.Equal(t, code, res.StatusCode) + + // Expect OPTIONS response to have custom CORS header set by user + assert.Equal(t, expectedACAO, res.Header.Get(headerACAO)) + + // Expect OPTIONS response to have implicit default Allow-Methods + // set by boxo/gateway library + assert.Equal(t, expectedACAM, res.Header[headerACAM]) + + } + + cid := root.String() + + t.Run("HTTP OPTIONS response is OK and has defined headers", func(t *testing.T) { + t.Parallel() + testCORSPreflightRequest(t, "/ipfs/"+cid, "", "", http.StatusOK) + }) + + t.Run("HTTP OPTIONS response for cross-origin /ipfs/cid is OK and has CORS headers", func(t *testing.T) { + t.Parallel() + testCORSPreflightRequest(t, "/ipfs/"+cid, "", "https://other.example.net", http.StatusOK) + }) + + t.Run("HTTP OPTIONS response for cross-origin /ipfs/cid is HTTP 301 and includes CORS headers (path gw redirect on subdomain gw)", func(t *testing.T) { + t.Parallel() + testCORSPreflightRequest(t, "/ipfs/"+cid, "subgw.example.com", "https://other.example.net", http.StatusMovedPermanently) + }) + + t.Run("HTTP OPTIONS response for cross-origin is HTTP 200 and has CORS headers (host header on subdomain gw)", func(t *testing.T) { + t.Parallel() + testCORSPreflightRequest(t, "/", cid+".ipfs.subgw.example.com", "https://other.example.net", http.StatusOK) + }) + +} + func TestIpfsTrustlessMode(t *testing.T) { backend, root := newMockBackend(t) diff --git a/gateway/handler.go b/gateway/handler.go index cb695408d..8c7e92e74 100644 --- a/gateway/handler.go +++ b/gateway/handler.go @@ -156,19 +156,25 @@ func (i *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - w.Header().Add("Allow", http.MethodGet) - w.Header().Add("Allow", http.MethodHead) - w.Header().Add("Allow", http.MethodOptions) + addAllowHeader(w) errmsg := "Method " + r.Method + " not allowed: read only access" http.Error(w, errmsg, http.StatusMethodNotAllowed) } func (i *handler) optionsHandler(w http.ResponseWriter, r *http.Request) { + addAllowHeader(w) // OPTIONS is a noop request that is used by the browsers to check if server accepts // cross-site XMLHttpRequest, which is indicated by the presence of CORS headers: // https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS#Preflighted_requests - i.addUserHeaders(w) // return all custom headers (including CORS ones, if set) + addCustomHeaders(w, i.config.Headers) // return all custom headers (including CORS ones, if set) +} + +// addAllowHeader sets Allow header with supported HTTP methods +func addAllowHeader(w http.ResponseWriter) { + w.Header().Add("Allow", http.MethodGet) + w.Header().Add("Allow", http.MethodHead) + w.Header().Add("Allow", http.MethodOptions) } func (i *handler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) { @@ -213,7 +219,7 @@ func (i *handler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) { trace.SpanFromContext(r.Context()).SetAttributes(attribute.String("ResponseFormat", responseFormat)) i.requestTypeMetric.WithLabelValues(contentPath.Namespace(), responseFormat).Inc() - i.addUserHeaders(w) // ok, _now_ write user's headers. + addCustomHeaders(w, i.config.Headers) // ok, _now_ write user's headers. w.Header().Set("X-Ipfs-Path", contentPath.String()) // Fail fast if unsupported request type was sent to a Trustless Gateway. @@ -290,9 +296,9 @@ func (i *handler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) { } } -func (i *handler) addUserHeaders(w http.ResponseWriter) { - for k, v := range i.config.Headers { - w.Header()[k] = v +func addCustomHeaders(w http.ResponseWriter, headers map[string][]string) { + for k, v := range headers { + w.Header()[http.CanonicalHeaderKey(k)] = v } } diff --git a/gateway/hostname.go b/gateway/hostname.go index 6fb1ac8eb..4df23d22c 100644 --- a/gateway/hostname.go +++ b/gateway/hostname.go @@ -68,7 +68,7 @@ func NewHostnameHandler(c Config, backend IPFSBackend, next http.Handler) http.H return } if newURL != "" { - http.Redirect(w, r, newURL, http.StatusMovedPermanently) + httpRedirectWithHeaders(w, r, newURL, http.StatusMovedPermanently, c.Headers) return } } @@ -131,7 +131,7 @@ func NewHostnameHandler(c Config, backend IPFSBackend, next http.Handler) http.H if newURL != "" { // Redirect to deterministic CID to ensure CID // always gets the same Origin on the web - http.Redirect(w, r, newURL, http.StatusMovedPermanently) + httpRedirectWithHeaders(w, r, newURL, http.StatusMovedPermanently, c.Headers) return } } @@ -146,7 +146,7 @@ func NewHostnameHandler(c Config, backend IPFSBackend, next http.Handler) http.H } if newURL != "" { // Redirect to CID fixed inside of toSubdomainURL() - http.Redirect(w, r, newURL, http.StatusMovedPermanently) + httpRedirectWithHeaders(w, r, newURL, http.StatusMovedPermanently, c.Headers) return } } @@ -559,3 +559,14 @@ func (gws *hostnameGateways) knownSubdomainDetails(hostname string) (gw *PublicG // no match return nil, "", "", "", false } + +// httpRedirectWithHeaders applies custom headers before returning a redirect +// response to ensure consistency during transition from path to subdomain +// contexts. +func httpRedirectWithHeaders(w http.ResponseWriter, r *http.Request, url string, code int, headers map[string][]string) { + // ensure things like CORS are applied to redirect responses + // (https://github.com/ipfs/kubo/issues/9983#issuecomment-1599673976) + addCustomHeaders(w, headers) + + http.Redirect(w, r, url, code) +}