Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP API: Disallow GET requests #7097

Merged
merged 2 commits into from
Apr 5, 2020
Merged

HTTP API: Disallow GET requests #7097

merged 2 commits into from
Apr 5, 2020

Conversation

hsanjuan
Copy link
Contributor

@hsanjuan hsanjuan commented Apr 4, 2020

Better security by limiting the HTTP API to POST requests. (see commit descriptions).

@hsanjuan hsanjuan requested review from lidel and Stebalien April 4, 2020 00:13
@hsanjuan hsanjuan self-assigned this Apr 4, 2020
@hsanjuan
Copy link
Contributor Author

hsanjuan commented Apr 4, 2020

This might break tests, not sure, I will fix those on another day.

core/corehttp/commands.go Outdated Show resolved Hide resolved
@hsanjuan
Copy link
Contributor Author

hsanjuan commented Apr 4, 2020

I have a test failure that seems unrelated:

=== Failed
=== FAIL: core/coreapi/test TestIface/Dht/TestDhtFindPeer (1.29s)
panic: runtime error: index out of range [0] with length 0 [recovered]
	panic: runtime error: index out of range [0] with length 0

@hsanjuan hsanjuan changed the title HTTP API: Only allow POST requests (plus OPTIONS, HEAD) HTTP API: Only allow POST requests (plus OPTIONS) Apr 4, 2020
@hsanjuan
Copy link
Contributor Author

hsanjuan commented Apr 4, 2020

@lidel can you help with the webui test failure (I have the feeling it will take you way shorter than me to figure out the place to fix it).

@lidel
Copy link
Member

lidel commented Apr 4, 2020

I will take a look as soon I get back home (eta 1h)

Update: see ipfs/ipfs-webui#1429

@hsanjuan
Copy link
Contributor Author

hsanjuan commented Apr 4, 2020

Ok:

  • Sharness is green

Pending:

lidel added a commit to ipfs-inactive/ipfs-redux-bundle that referenced this pull request Apr 4, 2020
ipfs/kubo#7097 will block `GET` commands on
API port, switching everything to POST.

This breaks Files screen in ipfs-webui as noted in
ipfs/ipfs-webui#1429

ipfs-webui is using older version of js-ipfs-http-client, one before
huge refactor into async iterables, which means switching to the latest
version won't be a trivial task.

For now, we just apply simple patch on top of ipfs-http-client v39.0.2
to ensure it sends commands as POST.

Proper fix will land when ipfs-webui is refactored to work with
ipfs-http-client >41.x

Closes #1429
lidel added a commit to ipfs/ipfs-webui that referenced this pull request Apr 4, 2020
ipfs/kubo#7097 will block `GET` commands on
API port, switching everything to POST.

This breaks Files screen in ipfs-webui as noted in
#1429

ipfs-webui is using older version of js-ipfs-http-client, one before
huge refactor into async iterables, which means switching to the latest
version won't be a trivial task.

For now, we just apply simple patch on top of ipfs-http-client v39.0.2
to ensure it sends commands as POST.

Proper fix will land when ipfs-webui is refactored to work with
ipfs-http-client >41.x

Closes #1429
lidel added a commit to ipfs/ipfs-webui that referenced this pull request Apr 4, 2020
* fix: support POST-only HTTP API

ipfs/kubo#7097 will block `GET` commands on
API port, switching everything to POST.

This breaks Files screen in ipfs-webui as noted in
#1429

ipfs-webui is using older version of js-ipfs-http-client, one before
huge refactor into async iterables, which means switching to the latest
version won't be a trivial task.

For now, we just apply simple patch on top of ipfs-http-client v39.0.2
to ensure it sends commands as POST.

Proper fix will land when ipfs-webui is refactored to work with
ipfs-http-client >41.x

Closes #1429

* docs: remove GET from CORS setup for API
core/corehttp/commands.go Outdated Show resolved Hide resolved
hsanjuan and others added 2 commits April 5, 2020 09:57
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 <steven@stebalien.com>
Co-Authored-By: Marcin Rataj <lidel@lidel.org>
…Allowed

Spec says that response with 405 must set Allow headers.
@hsanjuan hsanjuan changed the title HTTP API: Only allow POST requests (plus OPTIONS) HTTP API: Disallow GET requests Apr 5, 2020
@Stebalien Stebalien merged commit 3304c28 into master Apr 5, 2020
@Stebalien Stebalien deleted the fix/api-post branch April 5, 2020 17:14
@lidel
Copy link
Member

lidel commented Apr 5, 2020

PSA: this will require IPFS Companion v2.11.x or later (shipped a Beta for testing: v2.11.0.904)

@ntninja
Copy link

ntninja commented Apr 17, 2020

Of course I'll adapt to this in py-ipfs-http-client, but may I ask what the rationale is/was for blocking GET on idempotent & read-only API endpoints? Why can't things like /api/v0/get?arg=<cid> continue to be allowed GET access additionally to POST?

@Stebalien
Copy link
Member

Basically, the HTTP API is not a REST API, it's a simple RPC API. In go-ipfs, at least, it's defined as a set of transport agnostic commands, then exposed through an HTTP API.

Note: You can continue to use GET requests against the read-only API on the gateway.

@ntninja
Copy link

ntninja commented Apr 17, 2020

Too bad 🙁… I have some code deployed that uses the HTTP API as read-only API since its port is stable (5001) while the gateway port is unusable (8080 is way to generic).

The decision to also enforce this for the read-only parts of the API parts that are also exposed via the gateway is final, I suppose? Does the read-only API exposed via the gateway support using POST or does it only support GET instead?

@hsanjuan
Copy link
Contributor Author

Does the read-only API exposed via the gateway support using POST or does it only support GET instead?

I think both.

@Stebalien
Copy link
Member

We have no intention of changing this as it simplifies reasoning about the system. Is it not possible for you to use POST?

@lidel
Copy link
Member

lidel commented Apr 17, 2020

@Alexander255 @hsanjuan 👉 POST-only is required only on the API port

A subset of /api/v0 exposed on the Gateway accepts GET just fine (0.5.0-rc2), and that is ok (no reason to block it, as far I can tell):

$ curl -X GET http://127.0.0.1:5001/api/v0/dns/ipfs.io
405 - Method Not Allowed

$ curl -X GET http://127.0.0.1:8080/api/v0/dns/ipfs.io
{"Path":"/ipfs/bafybeibdwwmb725qy4yph56jg4bs6uimcst5hnvuraarowgdaytzmgsw24"}

@ntninja
Copy link

ntninja commented Apr 18, 2020

With the read-only API on gateway supporting POST, I can still pretend they are the same so no problem for me then. I just feels weird to use POST there, but if it makes stuff easier for you so be it. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants