rpcserver: add cors support

[ghost]

This adds cors support to the LND http proxy, ticket created on it here: #2344

Basing it off of this stale PR: #1433
Key differences:
-cors automatically
-doing so without pulling in a separate library

This should allow browser based wallets to access LND nodes via http, since browsers cannot access via grcp. Otherwise, browsers need to run with security disabled which do not check cors.

[MDrollette]

Even though the endpoints are authenticated via macaroons it might be worth adding a config value to enable/disable cors. That would allow a little more defense-in-depth where if a readonly (or otherwise attentuated) macaroon is leaked, at least it wouldn't be remotely exploitable (if the user had cors disabled).

[ghost]

@MDrollette Thanks for the comment/review.

I'm not opposed to adding it as a config value, though I do want to mitigate any concerns there might be by having it on by default, if possible. And to see if I'm missing something as well.

CORS is a request from a server to the browser asking it to obey its rules. It does not have to be obeyed, a user can turn off this setting and access from any origin anyways.

A leaked macaroon can be remotely exploitable whether or not cors is on or off. Cors only stops client side code in a browser (w/ default cors settings) from accessing resources/api's on another webserver somewhere else.

That said, I think the only way for something to take place if cors is on vs off is if a website not meant for accessing LND got ahold of someone's macaroon, whether by user file upload or something, and decided to execute the macaroon right there in the user's browser. So let's say cors is off and the execution didn't work in the user's browser, then the website uploads it to a server and the server admin can try to execute requests to LND. Only thing that would prevent that would be IP-locks/firewalls, right?

So essentially, malicious client side code in a browser that can't get passed CORS, gets access to a user's macaroon and knows the user's LND IP/Port to access his IP locked LND node and executes commands right there in the browser. Or the LND node is not IP locked but the malicious code can't upload the macaroon to the malicious actor so it needs to execute commands in the browser.

That could very well be reason enough to have it off by default, but I did want to dispel the concern of simply 'macaroon is leaked, it would be remotely exploitable if cors is on'. Thoughts?

[MDrollette]

Assuming you had an otherwise secure setup where the gRPC/HTTP listeners are limited to localhost and with cors disabled, even if I was able to get a readonly macaroon of yours (say from an errant git commit) I wouldn't be able to do much of anything with it. But, if you had cors enabled and allowed all origins (*), I would be able to send you a link to my own website which has some malicious javascript that would read out all your node info and post it back to my server. A restrictive cors policy, or having it off by default, would prevent a leaked macaroon from being exploitable via a users browser in that scenario.

I don't know how big of a concern that is, but it's worth pointing out. I would prefer the option, at least, and off by default would be even better, imo.

edit: a "secure setup" would also imply protecting against dns rebind attacks... which is maybe assuming a lot for the average user. But I think my point is the same.

[ghost]

Makes sense. I appreciate the dialog! I'll get the config in there defaulted to off this weekend.

[cfromknecht]

@AnthonyRonning @MDrollette definitely agree that we probably don't want to enable wildcard cors by default, would much rather we have that opt in.

I'm thinking it'd be nice to be able to specify either * or a list of hostnames that are whitelisted. The list of hostnames is a little trickier, as we'll have to inject the requesting host into the header if it passes the whitelist, but it should still be possible to accomplish via middleware as you've already done. As far as a config name, maybe something like restcors?

-doing so without pulling in a separate library

Nice! We try to keep our deps slim :)

[ghost]

Good suggestions @cfromknecht, I added it as an option, defaulted off, where you can add whitelisted ips/domains or * via restcors.

[cfromknecht]

@AnthonyRonning latest version with configurable CORS hosts looks good, just some minor comments

[cfromknecht]

@AnthonyRonning latest changes look good to me! Will need to be squash, perhaps down to two commits (1 for config, then 1 for lnd+rpcserver). Can probably defer that though until we get a second reviewer

[Roasbeef]

Rather than exposing so much control, how about we just target this particular use case? So a mode that only allows select macaroons to be served, rather than giving access to the entire on-disk state. That would be easier to use, more directed, and also more secure since it exposes a smaller surface area.

[Roasbeef]

Also the current implementation doesn't seem very user friendly: for each website I want to allow, i need to restart my lnd node and edit its config.

[ghost]

@Roasbeef

a mode that only allows select macaroons to be served

So a mode that turns on cors and restricts macaroons? By "select macaroons", do you mean allowing users to select the macaroons they want to allow in this mode, or only allow certain types of macaroons? IE only allow read-only macaroons?

i need to restart my lnd node and edit its config

Then maybe expose a new endpoint to allow hosts to be added/removed from the list of allowed sites? In addition to the starting list set by the config?

[Roasbeef]

will be done in #4141