From 09937f84b64212896e1d80147f828d86ebc19d68 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 --- cmd/ipfs/daemon.go | 2 +- cmd/ipfswatch/main.go | 2 +- core/corehttp/gateway.go | 14 ++++---- core/corehttp/gateway_handler.go | 9 +++-- core/corehttp/gateway_test.go | 60 +++++++++++++++++++++++++++----- repo/config/gateway.go | 1 + repo/config/init.go | 1 + test/supernode_client/main.go | 2 +- 8 files changed, 72 insertions(+), 19 deletions(-) diff --git a/cmd/ipfs/daemon.go b/cmd/ipfs/daemon.go index d2bb9f78a..1512c3ed2 100644 --- a/cmd/ipfs/daemon.go +++ b/cmd/ipfs/daemon.go @@ -448,7 +448,7 @@ func serveHTTPGateway(req cmds.Request) (error, <-chan error) { corehttp.CommandsROOption(*req.InvocContext()), corehttp.VersionOption(), corehttp.IPNSHostnameOption(), - corehttp.GatewayOption(writable), + corehttp.GatewayOption(writable, cfg.Gateway.PathPrefixes), } if len(cfg.Gateway.RootRedirect) > 0 { diff --git a/cmd/ipfswatch/main.go b/cmd/ipfswatch/main.go index aa6210adc..bf173bbba 100644 --- a/cmd/ipfswatch/main.go +++ b/cmd/ipfswatch/main.go @@ -84,7 +84,7 @@ func run(ipfsPath, watchPath string) error { if *http { addr := "/ip4/127.0.0.1/tcp/5001" var opts = []corehttp.ServeOption{ - corehttp.GatewayOption(true), + corehttp.GatewayOption(true, nil), corehttp.WebUIOption, corehttp.CommandsOption(cmdCtx(node, ipfsPath)), } diff --git a/core/corehttp/gateway.go b/core/corehttp/gateway.go index ce6ed50da..c2831dfe1 100644 --- a/core/corehttp/gateway.go +++ b/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/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index 54ef9c7b7..d8bd7676f 100644 --- a/core/corehttp/gateway_handler.go +++ b/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/core/corehttp/gateway_test.go b/core/corehttp/gateway_test.go index d52d8f551..acff78262 100644 --- a/core/corehttp/gateway_test.go +++ b/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") } } diff --git a/repo/config/gateway.go b/repo/config/gateway.go index 07bc9aad2..a8ba70590 100644 --- a/repo/config/gateway.go +++ b/repo/config/gateway.go @@ -5,4 +5,5 @@ type Gateway struct { HTTPHeaders map[string][]string // HTTP headers to return with the gateway RootRedirect string Writable bool + PathPrefixes []string } diff --git a/repo/config/init.go b/repo/config/init.go index 1cbc9f495..3dbc1b3f5 100644 --- a/repo/config/init.go +++ b/repo/config/init.go @@ -65,6 +65,7 @@ func Init(out io.Writer, nBitsForKeypair int) (*Config, error) { Gateway: Gateway{ RootRedirect: "", Writable: false, + PathPrefixes: []string{}, }, } diff --git a/test/supernode_client/main.go b/test/supernode_client/main.go index 5e1a25a8b..348f3c45b 100644 --- a/test/supernode_client/main.go +++ b/test/supernode_client/main.go @@ -109,7 +109,7 @@ func run() error { opts := []corehttp.ServeOption{ corehttp.CommandsOption(cmdCtx(node, repoPath)), - corehttp.GatewayOption(false), + corehttp.GatewayOption(false, nil), } if *cat {