From 1b490476e5517931b8d31a6636e7008771db201d Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Sat, 4 Apr 2020 01:55:36 +0200 Subject: [PATCH 1/2] HTTP API: Disallow GET requests on API This commit upgrades go-ipfs-cmds and configures the commands HTTP API Handler to only allow POST/OPTIONS, disallowing GET and others in the handling of command requests in the IPFS HTTP API (where before every type of request method was handled, with GET/POST/PUT/PATCH being equivalent). The Read-Only commands that the HTTP API attaches to the gateway endpoint will additional handled GET as they did before (but stop handling PUT,DELETEs). By limiting the request types we address the possibility that a website accessed by a browser abuses the IPFS API by issuing GET requests to it which have no Origin or Referrer set, and are thus bypass CORS and CSRF protections. This is a breaking change for clients that relay on GET requests against the HTTP endpoint (usually :5001). Applications integrating on top of the gateway-read-only API should still work (including cross-domain access). Co-Authored-By: Steven Allen Co-Authored-By: Marcin Rataj --- core/corehttp/commands.go | 18 ++++++++++++------ core/corehttp/webui.go | 3 ++- go.mod | 2 +- go.sum | 6 ++---- test/dependencies/go.mod | 2 +- test/dependencies/go.sum | 4 ++-- test/sharness/t0051-object.sh | 8 ++++---- test/sharness/t0060-daemon.sh | 6 +++--- test/sharness/t0090-get.sh | 2 +- test/sharness/t0100-name.sh | 2 +- test/sharness/t0110-gateway.sh | 2 +- ...0230-channel-streaming-http-content-type.sh | 4 ++-- .../t0600-issues-and-regressions-online.sh | 12 ++++++------ 13 files changed, 38 insertions(+), 33 deletions(-) diff --git a/core/corehttp/commands.go b/core/corehttp/commands.go index d63099bfbfc..0f9b8d60309 100644 --- a/core/corehttp/commands.go +++ b/core/corehttp/commands.go @@ -117,11 +117,17 @@ func patchCORSVars(c *cmdsHttp.ServerConfig, addr net.Addr) { c.SetAllowedOrigins(newOrigins...) } -func commandsOption(cctx oldcmds.Context, command *cmds.Command) ServeOption { +func commandsOption(cctx oldcmds.Context, command *cmds.Command, allowGet bool) ServeOption { return func(n *core.IpfsNode, l net.Listener, mux *http.ServeMux) (*http.ServeMux, error) { cfg := cmdsHttp.NewServerConfig() - cfg.SetAllowedMethods(http.MethodGet, http.MethodPost, http.MethodPut) + cfg.AllowGet = allowGet + corsAllowedMethods := []string{http.MethodPost} + if allowGet { + corsAllowedMethods = append(corsAllowedMethods, http.MethodGet) + } + + cfg.SetAllowedMethods(corsAllowedMethods...) cfg.APIPath = APIPath rcfg, err := n.Repo.Config() if err != nil { @@ -140,15 +146,15 @@ func commandsOption(cctx oldcmds.Context, command *cmds.Command) ServeOption { } // CommandsOption constructs a ServerOption for hooking the commands into the -// HTTP server. +// HTTP server. It will NOT allow GET requests. func CommandsOption(cctx oldcmds.Context) ServeOption { - return commandsOption(cctx, corecommands.Root) + return commandsOption(cctx, corecommands.Root, false) } // CommandsROOption constructs a ServerOption for hooking the read-only commands -// into the HTTP server. +// into the HTTP server. It will allow GET requests. func CommandsROOption(cctx oldcmds.Context) ServeOption { - return commandsOption(cctx, corecommands.RootRO) + return commandsOption(cctx, corecommands.RootRO, true) } // CheckVersionOption returns a ServeOption that checks whether the client ipfs version matches. Does nothing when the user agent string does not contain `/go-ipfs/` diff --git a/core/corehttp/webui.go b/core/corehttp/webui.go index 21d3eeea61c..4a31f719a94 100644 --- a/core/corehttp/webui.go +++ b/core/corehttp/webui.go @@ -1,7 +1,7 @@ package corehttp // TODO: move to IPNS -const WebUIPath = "/ipfs/Qmexhq2sBHnXQbvyP2GfUdbnY7HCagH2Mw5vUNSBn2nxip" +const WebUIPath = "/ipfs/bafybeihpkhgv3jfnyx5qcexded7agjpwbgvtc3o6lnk6n3cs37fh4xx4fe" // this is a list of all past webUI paths. var WebUIPaths = []string{ @@ -33,6 +33,7 @@ var WebUIPaths = []string{ "/ipfs/QmcjeTciMNgEBe4xXvEaA4TQtwTRkXucx7DmKWViXSmX7m", "/ipfs/QmfNbSskgvTXYhuqP8tb9AKbCkyRcCy3WeiXwD9y5LeoqK", "/ipfs/QmPkojhjJkJ5LEGBDrAvdftrjAYmi9GU5Cq27mWvZTDieW", + "/ipfs/Qmexhq2sBHnXQbvyP2GfUdbnY7HCagH2Mw5vUNSBn2nxip", } var WebUIOption = RedirectOption("webui", WebUIPath) diff --git a/go.mod b/go.mod index 55eb75ff1a1..6322309e151 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,7 @@ require ( github.com/ipfs/go-graphsync v0.0.5 github.com/ipfs/go-ipfs-blockstore v0.1.4 github.com/ipfs/go-ipfs-chunker v0.0.5 - github.com/ipfs/go-ipfs-cmds v0.1.4 + github.com/ipfs/go-ipfs-cmds v0.2.1 github.com/ipfs/go-ipfs-config v0.4.0 github.com/ipfs/go-ipfs-ds-help v0.1.1 github.com/ipfs/go-ipfs-exchange-interface v0.0.1 diff --git a/go.sum b/go.sum index 92a1fd9b86a..08c85f0c091 100644 --- a/go.sum +++ b/go.sum @@ -256,8 +256,8 @@ github.com/ipfs/go-ipfs-chunker v0.0.1 h1:cHUUxKFQ99pozdahi+uSC/3Y6HeRpi9oTeUHbE github.com/ipfs/go-ipfs-chunker v0.0.1/go.mod h1:tWewYK0we3+rMbOh7pPFGDyypCtvGcBFymgY4rSDLAw= github.com/ipfs/go-ipfs-chunker v0.0.5 h1:ojCf7HV/m+uS2vhUGWcogIIxiO5ubl5O57Q7NapWLY8= github.com/ipfs/go-ipfs-chunker v0.0.5/go.mod h1:jhgdF8vxRHycr00k13FM8Y0E+6BoalYeobXmUyTreP8= -github.com/ipfs/go-ipfs-cmds v0.1.4 h1:l5QAc1iaoMZeBd2vpanrHWs26haEBL4PVqgoHJNG2GE= -github.com/ipfs/go-ipfs-cmds v0.1.4/go.mod h1:wm+C6M8FYDcWPU/EdWqMuHvdyWborFh+GuDl6Ov6sM0= +github.com/ipfs/go-ipfs-cmds v0.2.1 h1:xnZJjonqngoQRsrugXTQPMRn2KZwdn5H8N5+bLLYpO0= +github.com/ipfs/go-ipfs-cmds v0.2.1/go.mod h1:kqlUrp6m2ceoaJe40cXpADCi5aS6NKRn0NIeuLp5CeM= github.com/ipfs/go-ipfs-config v0.4.0 h1:MOXdj8EYQG55v1y+5e1QcctDKPEGobdwnXaDVa0/cc0= github.com/ipfs/go-ipfs-config v0.4.0/go.mod h1:nSLCFtlaL+2rbl3F+9D4gQZQbT1LjRKx7TJg/IHz6oM= github.com/ipfs/go-ipfs-delay v0.0.0-20181109222059-70721b86a9a8/go.mod h1:8SP1YXK1M1kXuc4KJZINY3TQQ03J2rwBG9QfXmbRPrw= @@ -274,8 +274,6 @@ github.com/ipfs/go-ipfs-exchange-offline v0.0.1/go.mod h1:WhHSFCVYX36H/anEKQboAz github.com/ipfs/go-ipfs-files v0.0.2/go.mod h1:INEFm0LL2LWXBhNJ2PMIIb2w45hpXgPjNoE7yA8Y1d4= github.com/ipfs/go-ipfs-files v0.0.3/go.mod h1:INEFm0LL2LWXBhNJ2PMIIb2w45hpXgPjNoE7yA8Y1d4= github.com/ipfs/go-ipfs-files v0.0.4/go.mod h1:INEFm0LL2LWXBhNJ2PMIIb2w45hpXgPjNoE7yA8Y1d4= -github.com/ipfs/go-ipfs-files v0.0.7 h1:s5BRD12ndahqYifeH1S8Z73zqZhR+3IdKYAG9PiETs0= -github.com/ipfs/go-ipfs-files v0.0.7/go.mod h1:wiN/jSG8FKyk7N0WyctKSvq3ljIa2NNTiZB55kpTdOs= github.com/ipfs/go-ipfs-files v0.0.8 h1:8o0oFJkJ8UkO/ABl8T6ac6tKF3+NIpj67aAB6ZpusRg= github.com/ipfs/go-ipfs-files v0.0.8/go.mod h1:wiN/jSG8FKyk7N0WyctKSvq3ljIa2NNTiZB55kpTdOs= github.com/ipfs/go-ipfs-flags v0.0.1/go.mod h1:RnXBb9WV53GSfTrSDVK61NLTFKvWc60n+K9EgCDh+rA= diff --git a/test/dependencies/go.mod b/test/dependencies/go.mod index 288f131fba9..9c33af08235 100644 --- a/test/dependencies/go.mod +++ b/test/dependencies/go.mod @@ -17,7 +17,7 @@ require ( github.com/ipfs/go-unixfs v0.2.4 github.com/ipfs/hang-fds v0.0.2 github.com/ipfs/iptb v1.4.0 - github.com/ipfs/iptb-plugins v0.2.1 + github.com/ipfs/iptb-plugins v0.2.2 github.com/ipld/go-ipld-prime v0.0.2-0.20191108012745-28a82f04c785 github.com/jbenet/go-random v0.0.0-20190219211222-123a90aedc0c github.com/jbenet/go-random-files v0.0.0-20190219210431-31b3f20ebded diff --git a/test/dependencies/go.sum b/test/dependencies/go.sum index 1ca3ce20b39..f4ea381a1ad 100644 --- a/test/dependencies/go.sum +++ b/test/dependencies/go.sum @@ -295,8 +295,8 @@ github.com/ipfs/hang-fds v0.0.2 h1:ffZPd+OFbKpfjNAoBCI+G7okTQKd7oS1jCEDm2Kzm4c= github.com/ipfs/hang-fds v0.0.2/go.mod h1:Ajpp/qR2orKbv5LsZmotGRASTcH38MwcIG5vTlZ9y8k= github.com/ipfs/iptb v1.4.0 h1:YFYTrCkLMRwk/35IMyC6+yjoQSHTEcNcefBStLJzgvo= github.com/ipfs/iptb v1.4.0/go.mod h1:1rzHpCYtNp87/+hTxG5TfCVn/yMY3dKnLn8tBiMfdmg= -github.com/ipfs/iptb-plugins v0.2.1 h1:au4HWn9/pRPbkxA08pDx2oRAs4cnbgQWgV0teYXuuGA= -github.com/ipfs/iptb-plugins v0.2.1/go.mod h1:QXMbtIWZ+jRsW8a4h13qAKU7jcM7qaittO8wOsTP0Rs= +github.com/ipfs/iptb-plugins v0.2.2 h1:HleRKMeex/jmQrmNG36v51M3eZO5j9BhFplBPGs0qGQ= +github.com/ipfs/iptb-plugins v0.2.2/go.mod h1:QXMbtIWZ+jRsW8a4h13qAKU7jcM7qaittO8wOsTP0Rs= github.com/ipld/go-ipld-prime v0.0.2-0.20191108012745-28a82f04c785 h1:fASnkvtR+SmB2y453RxmDD3Uvd4LonVUgFGk9JoDaZs= github.com/ipld/go-ipld-prime v0.0.2-0.20191108012745-28a82f04c785/go.mod h1:bDDSvVz7vaK12FNvMeRYnpRFkSUPNQOiCYQezMD/P3w= github.com/ipld/go-ipld-prime-proto v0.0.0-20191113031812-e32bd156a1e5 h1:lSip43rAdyGA+yRQuy6ju0ucZkWpYc1F2CTQtZTVW/4= diff --git a/test/sharness/t0051-object.sh b/test/sharness/t0051-object.sh index 4f8e7e12d0f..363073efc32 100755 --- a/test/sharness/t0051-object.sh +++ b/test/sharness/t0051-object.sh @@ -419,19 +419,19 @@ test_object_cmd() { test_object_content_type() { test_expect_success "'ipfs object get --encoding=protobuf' returns the correct content type" ' - curl -sI "http://$API_ADDR/api/v0/object/get?arg=$HASH&encoding=protobuf" | grep -q "^Content-Type: application/protobuf" + curl -X POST -sI "http://$API_ADDR/api/v0/object/get?arg=$HASH&encoding=protobuf" | grep -q "^Content-Type: application/protobuf" ' test_expect_success "'ipfs object get --encoding=json' returns the correct content type" ' - curl -sI "http://$API_ADDR/api/v0/object/get?arg=$HASH&encoding=json" | grep -q "^Content-Type: application/json" + curl -X POST -sI "http://$API_ADDR/api/v0/object/get?arg=$HASH&encoding=json" | grep -q "^Content-Type: application/json" ' test_expect_success "'ipfs object get --encoding=text' returns the correct content type" ' - curl -sI "http://$API_ADDR/api/v0/object/get?arg=$HASH&encoding=text" | grep -q "^Content-Type: text/plain" + curl -X POST -sI "http://$API_ADDR/api/v0/object/get?arg=$HASH&encoding=text" | grep -q "^Content-Type: text/plain" ' test_expect_success "'ipfs object get --encoding=xml' returns the correct content type" ' - curl -sI "http://$API_ADDR/api/v0/object/get?arg=$HASH&encoding=xml" | grep -q "^Content-Type: application/xml" + curl -X POST -sI "http://$API_ADDR/api/v0/object/get?arg=$HASH&encoding=xml" | grep -q "^Content-Type: application/xml" ' } diff --git a/test/sharness/t0060-daemon.sh b/test/sharness/t0060-daemon.sh index 7e7c658fbea..f85a610ce0a 100755 --- a/test/sharness/t0060-daemon.sh +++ b/test/sharness/t0060-daemon.sh @@ -65,11 +65,11 @@ test_expect_success "ipfs peer id looks good" ' # this is for checking SetAllowedOrigins race condition for the api and gateway # See https://github.com/ipfs/go-ipfs/pull/1966 test_expect_success "ipfs API works with the correct allowed origin port" ' - curl -s -X GET -H "Origin:http://localhost:$API_PORT" -I "http://$API_ADDR/api/v0/version" + curl -s -X POST -H "Origin:http://localhost:$API_PORT" -I "http://$API_ADDR/api/v0/version" ' test_expect_success "ipfs gateway works with the correct allowed origin port" ' - curl -s -X GET -H "Origin:http://localhost:$GWAY_PORT" -I "http://$GWAY_ADDR/api/v0/version" + curl -s -X POST -H "Origin:http://localhost:$GWAY_PORT" -I "http://$GWAY_ADDR/api/v0/version" ' test_expect_success "ipfs daemon output looks good" ' @@ -134,7 +134,7 @@ test_expect_success SOCAT "transport should be encrypted ( needs socat )" ' ' test_expect_success "output from streaming commands works" ' - test_expect_code 28 curl -m 5 http://localhost:$API_PORT/api/v0/stats/bw\?poll=true > statsout + test_expect_code 28 curl -X POST -m 5 http://localhost:$API_PORT/api/v0/stats/bw\?poll=true > statsout ' test_expect_success "output looks good" ' diff --git a/test/sharness/t0090-get.sh b/test/sharness/t0090-get.sh index e19d5506d9c..750c3017238 100755 --- a/test/sharness/t0090-get.sh +++ b/test/sharness/t0090-get.sh @@ -184,7 +184,7 @@ test_launch_ipfs_daemon test_get_cmd test_expect_success "empty request to get doesn't panic and returns error" ' - curl "http://$API_ADDR/api/v0/get" > curl_out || true && + curl -X POST "http://$API_ADDR/api/v0/get" > curl_out || true && grep "argument \"ipfs-path\" is required" curl_out ' test_kill_ipfs_daemon diff --git a/test/sharness/t0100-name.sh b/test/sharness/t0100-name.sh index e03f1553cf9..96d2ce5ea82 100755 --- a/test/sharness/t0100-name.sh +++ b/test/sharness/t0100-name.sh @@ -195,7 +195,7 @@ test_expect_success "resolve output looks good" ' ' test_expect_success "empty request to name publish doesn't panic and returns error" ' - curl "http://$API_ADDR/api/v0/name/publish" > curl_out || true && + curl -X POST "http://$API_ADDR/api/v0/name/publish" > curl_out || true && grep "argument \"ipfs-path\" is required" curl_out ' diff --git a/test/sharness/t0110-gateway.sh b/test/sharness/t0110-gateway.sh index 68172cb5655..f64618f740f 100755 --- a/test/sharness/t0110-gateway.sh +++ b/test/sharness/t0110-gateway.sh @@ -104,7 +104,7 @@ test_expect_success "log output looks good" ' ' test_expect_success "GET /api/v0/version succeeds" ' - curl -v "http://127.0.0.1:$apiport/api/v0/version" 2> version_out + curl -X POST -v "http://127.0.0.1:$apiport/api/v0/version" 2> version_out ' test_expect_success "output only has one transfer encoding header" ' diff --git a/test/sharness/t0230-channel-streaming-http-content-type.sh b/test/sharness/t0230-channel-streaming-http-content-type.sh index deffffa2c2f..34ef57771fb 100755 --- a/test/sharness/t0230-channel-streaming-http-content-type.sh +++ b/test/sharness/t0230-channel-streaming-http-content-type.sh @@ -16,7 +16,7 @@ test_ls_cmd() { mkdir -p testdir && echo "hello test" >testdir/test.txt && ipfs add -r testdir && - curl -i "http://$API_ADDR/api/v0/refs?arg=QmTcJAn3JP8ZMAKS6WS75q8sbTyojWKbxcUHgLYGWur4Ym&stream-channels=true&encoding=text" >actual_output + curl -X POST -i "http://$API_ADDR/api/v0/refs?arg=QmTcJAn3JP8ZMAKS6WS75q8sbTyojWKbxcUHgLYGWur4Ym&stream-channels=true&encoding=text" >actual_output ' test_expect_success "Text encoded channel-streaming command output looks good" ' @@ -39,7 +39,7 @@ test_ls_cmd() { mkdir -p testdir && echo "hello test" >testdir/test.txt && ipfs add -r testdir && - curl -i "http://$API_ADDR/api/v0/refs?arg=QmTcJAn3JP8ZMAKS6WS75q8sbTyojWKbxcUHgLYGWur4Ym&stream-channels=true&encoding=json" >actual_output + curl -X POST -i "http://$API_ADDR/api/v0/refs?arg=QmTcJAn3JP8ZMAKS6WS75q8sbTyojWKbxcUHgLYGWur4Ym&stream-channels=true&encoding=json" >actual_output ' test_expect_success "JSON encoded channel-streaming command output looks good" ' diff --git a/test/sharness/t0600-issues-and-regressions-online.sh b/test/sharness/t0600-issues-and-regressions-online.sh index 8496a0f03e5..344d37fd1fa 100755 --- a/test/sharness/t0600-issues-and-regressions-online.sh +++ b/test/sharness/t0600-issues-and-regressions-online.sh @@ -11,16 +11,16 @@ test_launch_ipfs_daemon # Tests go here test_expect_success "commands command with flag flags works via HTTP API - #2301" ' - curl "http://$API_ADDR/api/v0/commands?flags" | grep "verbose" + curl -X POST "http://$API_ADDR/api/v0/commands?flags" | grep "verbose" ' test_expect_success "ipfs refs local over HTTP API returns NDJOSN not flat - #2803" ' echo "Hello World" | ipfs add && - curl "http://$API_ADDR/api/v0/refs/local" | grep "Ref" | grep "Err" + curl -X POST "http://$API_ADDR/api/v0/refs/local" | grep "Ref" | grep "Err" ' test_expect_success "args expecting stdin dont crash when not given" ' - curl "$API_ADDR/api/v0/bootstrap/add" > result + curl -X POST "$API_ADDR/api/v0/bootstrap/add" > result ' test_expect_success "no panic traces on daemon" ' @@ -28,20 +28,20 @@ test_expect_success "no panic traces on daemon" ' ' test_expect_success "metrics work" ' - curl "$API_ADDR/debug/metrics/prometheus" > pro_data && + curl -X POST "$API_ADDR/debug/metrics/prometheus" > pro_data && grep "ipfs_bs_cache_arc_hits_total" < pro_data || test_fsh cat pro_data ' test_expect_success "pin add api looks right - #3753" ' HASH=$(echo "foo" | ipfs add -q) && - curl "http://$API_ADDR/api/v0/pin/add/$HASH" > pinadd_out && + curl -X POST "http://$API_ADDR/api/v0/pin/add/$HASH" > pinadd_out && echo "{\"Pins\":[\"QmYNmQKp6SuaVrpgWRsPTgCQCnpxUYGq76YEKBXuj2N4H6\"]}" > pinadd_exp && test_cmp pinadd_out pinadd_exp ' test_expect_success "pin add api looks right - #3753" ' - curl "http://$API_ADDR/api/v0/pin/rm/$HASH" > pinrm_out && + curl -X POST "http://$API_ADDR/api/v0/pin/rm/$HASH" > pinrm_out && echo "{\"Pins\":[\"QmYNmQKp6SuaVrpgWRsPTgCQCnpxUYGq76YEKBXuj2N4H6\"]}" > pinrm_exp && test_cmp pinrm_out pinrm_exp ' From 73405436159b5953f6ed107f45d7aa3badf4bf44 Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Sat, 4 Apr 2020 02:06:06 +0200 Subject: [PATCH 2/2] corehttp: Gateway handler: add Allow headers when returning MethodNotAllowed Spec says that response with 405 must set Allow headers. --- core/corehttp/gateway_handler.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index d3c4d26392d..d26f21d54a2 100644 --- a/core/corehttp/gateway_handler.go +++ b/core/corehttp/gateway_handler.go @@ -127,6 +127,9 @@ func (i *gatewayHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !i.config.Writable { status = http.StatusMethodNotAllowed errmsg = errmsg + "read only access" + w.Header().Add("Allow", http.MethodGet) + w.Header().Add("Allow", http.MethodHead) + w.Header().Add("Allow", http.MethodOptions) } else { status = http.StatusBadRequest errmsg = errmsg + "bad request for " + r.URL.Path