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

Add support for CORS headers and pre-flight request #1603

Closed
RealityRipple opened this issue Aug 3, 2019 · 12 comments
Closed

Add support for CORS headers and pre-flight request #1603

RealityRipple opened this issue Aug 3, 2019 · 12 comments

Comments

@RealityRipple
Copy link

Any chance for OPTIONS support and CORS headers to allow cross-origin RPC requests? BitcoinABC implemented it wonderfully last year.

@rnicoll
Copy link
Contributor

rnicoll commented Aug 5, 2019

I'm really nervous what the use-case for this is - it sounds like this involves opening the JSON-RPC port to the Internet, and letting websites use it, which to me is a massive attack surface you're potentially opening up.

Generally you'd instead have a wrapping service over the RPC port, which would include input validation, rate limiting (i.e. to defend against DoS attacks), and restrict to expected commands, and that would be where CORS would go.

@RealityRipple
Copy link
Author

I'm really nervous what the use-case for this is - it sounds like this involves opening the JSON-RPC port to the Internet, and letting websites use it, which to me is a massive attack surface you're potentially opening up.

Generally you'd instead have a wrapping service over the RPC port, which would include input validation, rate limiting (i.e. to defend against DoS attacks), and restrict to expected commands, and that would be where CORS would go.

That's why it's implemented in ABC (and I'm pretty sure like Eth or something else) with a rpccorsdomain preference/command line option, with the default being that OPTIONS and CORS are not sent with RPC requests. My personal implentation hope is to allow users to use their running node to send and receive Dogecoin tips rather than an API website. Everything is over JS, so the rpccorsdomain would have to be set to null for local computer access, and the request would not necessarily be over the internet or even a LAN, though it easily could be with the right settings, but that should be up to the user, not the developer.
Asking a user to install a proxy client for this purpose seems... unfriendly at the very least. Just setting a command line or pref is much easier on the user, and having it default to undefined should prevent any security risks by users that don't understand the potential danger. As long as no one sets rpccorsdomain=* (which would be incredibly irresponsible), I don't see that the risk of trouble would be all that high. And it's not like CORS actually protects anyone from malicious attacks - the security element is implemented by the browser, not the server, so if someone just wrote their own RPC connection client, they'd easily ignore all the CORS stuff and the RPC server would be vulnerable anyway. All a lack of CORS-support does is obfuscate and limit usability.

@patricklodder
Copy link
Member

The security risk is that if I CORS-allow https://yoursite.com then you can XHR a send_to_address to you without me knowing until it's too late, even if I listen on localhost.

Recommend implementing BIP-21 for this usecase instead.

@RealityRipple
Copy link
Author

BIP-21 will handle sending, but not receiving. To detect incoming transactions, you still need a third-party service or an RPC proxy.

And, as I said, any other software running on the client's computer that isn't a CORS-enabled browser is just as unsafe with or without the headers. Headers don't actually do any protecting themselves, that's the other software's job. Protecting requests like sendtoaddress should be part of the request or another previous validation's domain, not the domain of any and all of the user's installed web browsers or RPC-capable software.

@patricklodder
Copy link
Member

Headers don't actually do any protecting themselves, that's the other software's job.

My problem with this solution is not the enabling of CORS headers in itself, I am very aware of the protection CORS does not bring. The problem is that there are insufficient security measures implemented on the RPC, specifically lacking 2 things right now: access levels and user confirmations. Right now, I wouldn't advice anyone to run a wallet and RPC on the same node as long as those 2 features aren't implemented. The browsers default CORS deny policy in case of missing headers is indeed a non-security-feature but allowing it to be changed increases the available attack vectors on an insecure implementation.

If we want to bring any such feature forward, significant work on whatever interface we would expose is needed first. Think interaction between web3 and Metamask as a minimal requirement of what needs to be done: when connecting my wallet to a dapp, I have to confirm it. When my signature is needed, I at the very least need to press a button (in metamask, not the dapp) to give the dapp what it needs. And the thing that is not-so-good with Metamask: I cannot always establish who the request comes from; so some form of mutual authentication would not be a luxury.

I hope that makes sense?

@RealityRipple
Copy link
Author

In that case, would it not be allowable to make an rpccorsdomain preference that only applies if the disablewallet preference is true or the request doesn't require authentication? Then, when superior authentication methods are included in a future version, the CORS preference can simply be enabled if that auth preference is also in use.

@patricklodder
Copy link
Member

I'd probably want to put it under a path that exclusively whitelists some calls, i.e. enable CORS on /jsonrpc/public but keep /jsonrpc non-CORS, because an OPTIONS call would then allow the subpath but not the whole thing. That way we can control which calls are exposed.

Which methods were you looking to expose?

@RealityRipple
Copy link
Author

A subdirectory would make sense and probably not get in the way of any potential implementations that I can imagine.

Just off the top of my head, I'd suggest getblockcount, getblockhash, getblock, and gettransaction; maybe getblockchaininfo and getdifficulty. validateaddress and verifymessage would also probably be useful for developers that don't want to write their own code for those actions.

Adding sendrawtransaction might be a bit... controversial, but I'd really like it to be included anyway, as many third-party API services support it via a textbox on their website more often than not, and it doesn't interact with the node's Account data. If it is added, I suppose createrawtransaction and decoderawtransaction might also be requested. Even submitblock, though I'm not sure that would ever get used.

If you feel comfortable adding Account-related transactions as well, listreceivedbyaddress, listtransactions, and listunspent would be useful.

Since my goal is essentially just a javascript-based tipping system, I'd be able to accomplish both sending and receiving with listreceivedbyaddress, gettransaction, and sendrawtransaction alone. Everything else I listed are just what I can imagine other use-cases for that shouldn't breach any security during implementation, except for the Account-related transactions which leak the user-set labels for addresses and whether or not the node is keeping an eye on the address in the first place, which, while potentially an anonymity risk in very specific cases, probably isn't ever going to be a security risk as far as actual control of coins go.

@rnicoll
Copy link
Contributor

rnicoll commented Mar 14, 2021

I think realistically the best way to do this is to have a proxy over dogecoind, which handles authentication (as needed), verifies the request is sane, then relays it. Potentially even something like a PHP script to parse then re-create the request. Exposing dogecoind directly to the Internet just feels extremely risky that someone would find a buffer overflow or other attack via the JSON-RPC system, which was definitely not intended to be exposed.

@RealityRipple
Copy link
Author

CORS is not a firewall. It doesn't protect or expose anything. It's an etiquette between good servers and good clients. It does nothing to stop bad clients.

@ReverseControl
Copy link
Collaborator

@RealityRipple thank you for bringing this up. There is now a Discussion section where you can raise technical discussions such as this, that merit thought and time, to be discussed by the community. I will close this issue as this belongs in the discussion section.

@adityaNirvana
Copy link

Hi, I know this issue is closed long time back. But I have a question regarding cors issue I am facing. I am getting

Request has been blocked by CORS policy: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

In configuration files, I didn't find any CORS related flag. Could anyone help me with same?

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

No branches or pull requests

5 participants