lnd
lnd copied to clipboard
lightningnetwork
rpc: add --notls to disable TLS for RPC
Change Description
This adds a --notls config option that disables TLS encryption for both gRPC and REST connections.
This may be useful for when RPC communication happens only over localhost or a LAN. It may also be used in conjunction with a reverse proxy to enable TLS.
This PR expands on a previous change to add --noresttls which disables TLS encryption for REST connections only. That PR originally disabled TLS altogether, but was changed to REST only after a comment was raised about compatibility with lncli, see: https://github.com/lightningnetwork/lnd/pull/4648#discussion_r494798090
This PR makes the accompanying changes to lncli to support unencrypted requests.
Steps to Test
Run lnd from this branch using --notls. Observe that lncli commands with --notls work as expected and that without --notls they fail. Observe that REST connections (using curl) succeed with http but fail with https.
I don't think this is a good idea. gRPC is a variant of HTTP/2 that only works through normal HTTP/2 infrastructure (e.g. reverse proxies, load balancers) because its content is encrypted. If you remove the encryption layer, those components start to behave weirdly. And you need a special h2c compliant clients.
I don't know enough about grpc to comment on this, however grpc is supported on nginx and kubernetes ingress so I figure they must have worked their way around the concerns you raise here?
I don't know enough about grpc to comment on this, however grpc is supported on nginx and kubernetes ingress so I figure they must have worked their way around the concerns you raise here?
I'm pretty sure h2c (gRPC without TLS) is considered to be non-standard. So it's quite possible that even components that have basic gRPC support won't work with it.
@guggero See https://github.com/lightningnetwork/lnd/pull/4648#issuecomment-1109317765 - it's not an uncommon pattern to centralize TLS/PKI to other software. So the major use-case here is not actually disabling TLS but to let TLS-termination be taken care of by other software than LND itself.
So from the perspective of e.g. lncli and others, it would look the same as today.
Aside from traditional reverse proxy/load balancer scenarios, see e.g. Istio, Linkerd, Consul Connect as higher-lever projects for internal service mesh.
As mentioned in that thread, LND requiring managing certs local to itself makes these scenarios cumbersome today.
it's not an uncommon pattern to centralize TLS/PKI to other software
I understand that, and I do see the need for this feature. I'm just giving you technical reasons why I think it's a bad idea and will lead to a lot of issues with proxy setups. We ourselves tried this a few times in a k8s based environment and ran into a couple of problems.
So just providing user support for this flag and its consequences is something I'm not looking forward to. That's why I think this should at least be accompanied by a doc that explains what h2c is, what works and what doesn't (e.g. lncli, macaroons and so on).
@guggero As per the links provided, looks like both nginx and k8s ingress support proxying h2c (the latter requiring a TLS cert to be provided for itself to make it gRPC proper).
Similarly for other popular reverse proxies like Traefik, envoy, haproxy, caddy.
What are macaroon-related gotchas, assuming that any applications (e.g. lncli) connect to lnd through a reverse proxy that perforrms the TLS termination? What is it that makes you grudge on this (apart from "being non-standard")? Anything apart from the requirement of having clients connect through gRPC proper that you'd expect in docs?
@sangaman Want me to take a stab on splitting it and adding the docs or are you on it? :)
Sorry this fell off my radar, I'll take a shot at it in the next day or two. If not I'll let you know. I appreciate it.
@sangaman, remember to re-request review from reviewers when ready
@sangaman, remember to re-request review from reviewers when ready
@guggero author seems MIA - what's missing from you perspective to get this into shape?
I'd say mostly docs around the flag (maybe even point to a doc from within the description of the CLI flag). See comment above (https://github.com/lightningnetwork/lnd/pull/6479#pullrequestreview-967631511).
@sangaman, remember to re-request review from reviewers when ready