From d2ab652dfd2b867be7336be7c5df15e8df33ae6d Mon Sep 17 00:00:00 2001 From: Lars Gierth Date: Mon, 28 Mar 2016 19:21:48 -0400 Subject: [PATCH] gateway: enforce allowlist for path prefixes The gateway accepts an X-Ipfs-Path-Prefix header, and assumes that it is mounted in a reverse proxy like nginx, at this path. Links in directory listings, as well as trailing-slash redirects need to be rewritten with that prefix in mind. We don't want a potential attacker to be able to pass in arbitrary path prefixes, which would end up in redirects and directory listings, which is why every prefix has to be explicitly allowed in the config. Previously, we'd accept *any* X-Ipfs-Path-Prefix header. Example: We mount blog.ipfs.io (a dnslink page) at ipfs.io/blog. nginx_ipfs.conf: location /blog/ { rewrite "^/blog(/.*)$" $1 break; proxy_set_header Host blog.ipfs.io; proxy_set_header X-Ipfs-Gateway-Prefix /blog; proxy_pass http://127.0.0.1:8080; } .ipfs/config: "Gateway": { "PathPrefixes": ["/blog"], // ... }, dnslink: > dig TXT _dnslink.blog.ipfs.io dnslink=/ipfs/QmWcBjXPAEdhXDATV4ghUpkAonNBbiyFx1VmmHcQe9HEGd License: MIT Signed-off-by: Lars Gierth This commit was moved from ipfs/kubo@09937f84b64212896e1d80147f828d86ebc19d68 --- gateway/core/corehttp/gateway.go | 14 +++--- gateway/core/corehttp/gateway_handler.go | 9 +++- gateway/core/corehttp/gateway_test.go | 60 ++++++++++++++++++++---- 3 files changed, 67 insertions(+), 16 deletions(-) diff --git a/gateway/core/corehttp/gateway.go b/gateway/core/corehttp/gateway.go index ce6ed50da..c2831dfe1 100644 --- a/gateway/core/corehttp/gateway.go +++ b/gateway/core/corehttp/gateway.go @@ -17,9 +17,10 @@ type Gateway struct { } type GatewayConfig struct { - Headers map[string][]string - BlockList *BlockList - Writable bool + Headers map[string][]string + BlockList *BlockList + Writable bool + PathPrefixes []string } func NewGateway(conf GatewayConfig) *Gateway { @@ -48,10 +49,11 @@ func (g *Gateway) ServeOption() ServeOption { } } -func GatewayOption(writable bool) ServeOption { +func GatewayOption(writable bool, prefixes []string) ServeOption { g := NewGateway(GatewayConfig{ - Writable: writable, - BlockList: &BlockList{}, + Writable: writable, + BlockList: &BlockList{}, + PathPrefixes: prefixes, }) return g.ServeOption() } diff --git a/gateway/core/corehttp/gateway_handler.go b/gateway/core/corehttp/gateway_handler.go index 54ef9c7b7..d8bd7676f 100644 --- a/gateway/core/corehttp/gateway_handler.go +++ b/gateway/core/corehttp/gateway_handler.go @@ -131,8 +131,13 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request // It will be prepended to links in directory listings and the index.html redirect. prefix := "" if prefixHdr := r.Header["X-Ipfs-Gateway-Prefix"]; len(prefixHdr) > 0 { - log.Debugf("X-Ipfs-Gateway-Prefix: %s", prefixHdr[0]) - prefix = prefixHdr[0] + prfx := prefixHdr[0] + for _, p := range i.config.PathPrefixes { + if prfx == p || strings.HasPrefix(prfx, p+"/") { + prefix = prfx + break + } + } } // IPNSHostnameOption might have constructed an IPNS path using the Host header. diff --git a/gateway/core/corehttp/gateway_test.go b/gateway/core/corehttp/gateway_test.go index d52d8f551..acff78262 100644 --- a/gateway/core/corehttp/gateway_test.go +++ b/gateway/core/corehttp/gateway_test.go @@ -98,7 +98,7 @@ func newTestServerAndNode(t *testing.T, ns mockNamesys) (*httptest.Server, *core ts.Listener, VersionOption(), IPNSHostnameOption(), - GatewayOption(false), + GatewayOption(false, []string{"/good-prefix"}), ) if err != nil { t.Fatal(err) @@ -227,7 +227,7 @@ func TestIPNSHostnameRedirect(t *testing.T) { t.Fatal(err) } req.Host = "example.net" - req.Header.Set("X-Ipfs-Gateway-Prefix", "/prefix") + req.Header.Set("X-Ipfs-Gateway-Prefix", "/good-prefix") res, err = doWithoutRedirect(req) if err != nil { @@ -241,8 +241,8 @@ func TestIPNSHostnameRedirect(t *testing.T) { hdr = res.Header["Location"] if len(hdr) < 1 { t.Errorf("location header not present") - } else if hdr[0] != "/prefix/foo/" { - t.Errorf("location header is %v, expected /prefix/foo/", hdr[0]) + } else if hdr[0] != "/good-prefix/foo/" { + t.Errorf("location header is %v, expected /good-prefix/foo/", hdr[0]) } } @@ -387,7 +387,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) { t.Fatal(err) } req.Host = "example.net" - req.Header.Set("X-Ipfs-Gateway-Prefix", "/prefix") + req.Header.Set("X-Ipfs-Gateway-Prefix", "/good-prefix") res, err = doWithoutRedirect(req) if err != nil { @@ -402,13 +402,57 @@ func TestIPNSHostnameBacklinks(t *testing.T) { s = string(body) t.Logf("body: %s\n", string(body)) - if !strings.Contains(s, "Index of /prefix") { + if !strings.Contains(s, "Index of /good-prefix") { t.Fatalf("expected a path in directory listing") } - if !strings.Contains(s, "") { + if !strings.Contains(s, "") { t.Fatalf("expected backlink in directory listing") } - if !strings.Contains(s, "") { + if !strings.Contains(s, "") { + t.Fatalf("expected file in directory listing") + } + + // make request to directory listing with illegal prefix + req, err = http.NewRequest("GET", ts.URL, nil) + if err != nil { + t.Fatal(err) + } + req.Host = "example.net" + req.Header.Set("X-Ipfs-Gateway-Prefix", "/bad-prefix") + + res, err = doWithoutRedirect(req) + if err != nil { + t.Fatal(err) + } + + // make request to directory listing with evil prefix + req, err = http.NewRequest("GET", ts.URL, nil) + if err != nil { + t.Fatal(err) + } + req.Host = "example.net" + req.Header.Set("X-Ipfs-Gateway-Prefix", "//good-prefix/foo") + + res, err = doWithoutRedirect(req) + if err != nil { + t.Fatal(err) + } + + // expect correct backlinks without illegal prefix + body, err = ioutil.ReadAll(res.Body) + if err != nil { + t.Fatalf("error reading response: %s", err) + } + s = string(body) + t.Logf("body: %s\n", string(body)) + + if !strings.Contains(s, "Index of /") { + t.Fatalf("expected a path in directory listing") + } + if !strings.Contains(s, "") { + t.Fatalf("expected backlink in directory listing") + } + if !strings.Contains(s, "") { t.Fatalf("expected file in directory listing") } }