From 4f319e675d5a74cb9c92369d821de742d91fc080 Mon Sep 17 00:00:00 2001 From: Erik De Smedt Date: Mon, 16 Sep 2024 13:02:38 +0200 Subject: [PATCH 1/4] grpc-plugin: Use default port and host + refactor A small refactor that allows to specify a more flexible configuration in the grpc-plugin. By default the grpc-plugin will bind to 127.0.0.1 instead of 0.0.0.0. --- plugins/grpc-plugin/src/main.rs | 25 ++++++++++++----- plugins/grpc-plugin/src/router.rs | 46 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 plugins/grpc-plugin/src/router.rs diff --git a/plugins/grpc-plugin/src/main.rs b/plugins/grpc-plugin/src/main.rs index 4839ae5b5479..efd19762d62c 100644 --- a/plugins/grpc-plugin/src/main.rs +++ b/plugins/grpc-plugin/src/main.rs @@ -3,10 +3,11 @@ use cln_grpc::pb::node_server::NodeServer; use cln_plugin::{options, Builder, Plugin}; use cln_rpc::notifications::Notification; use log::{debug, warn}; -use std::net::SocketAddr; +use router::GrpcRouterConfig; use std::path::PathBuf; use tokio::sync::broadcast; +mod router; mod tls; #[derive(Clone, Debug)] @@ -61,8 +62,19 @@ async fn main() -> Result<()> { None => return Ok(()), }; - let bind_port: i64 = plugin.option(&OPTION_GRPC_PORT).unwrap(); - let bind_host: String = plugin.option(&OPTION_GRPC_HOST).unwrap(); + let router_config = match GrpcRouterConfig::from_configured_plugin(&plugin) { + Ok(Some(cfg)) => cfg, + Ok(None) => { + log::info!("Running on default 'grpc-port' 9736."); + return Ok(()); + } + Err(err) => { + log::warn!("{:?}", err); + plugin.disable(&format!("Invalid configuration: {:?}", err)).await?; + return Err(err) + } + }; + let buffer_size: i64 = plugin.option(&OPTION_GRPC_MSG_BUFFER_SIZE).unwrap(); let buffer_size = match usize::try_from(buffer_size) { Ok(b) => b, @@ -87,8 +99,6 @@ async fn main() -> Result<()> { let plugin = plugin.start(state.clone()).await?; - let bind_addr: SocketAddr = format!("{}:{}", bind_host, bind_port).parse().unwrap(); - tokio::select! { _ = plugin.join() => { // This will likely never be shown, if we got here our @@ -96,14 +106,15 @@ async fn main() -> Result<()> { // messages anymore. debug!("Plugin loop terminated") } - e = run_interface(bind_addr, state) => { + e = run_interface(router_config, state) => { warn!("Error running grpc interface: {:?}", e) } } Ok(()) } -async fn run_interface(bind_addr: SocketAddr, state: PluginState) -> Result<()> { +async fn run_interface(router_config: GrpcRouterConfig, state: PluginState) -> Result<()> { + let bind_addr = router_config.socket_addr(); let identity = state.identity.to_tonic_identity(); let ca_cert = tonic::transport::Certificate::from_pem(state.ca_cert); diff --git a/plugins/grpc-plugin/src/router.rs b/plugins/grpc-plugin/src/router.rs new file mode 100644 index 000000000000..071949c2c43a --- /dev/null +++ b/plugins/grpc-plugin/src/router.rs @@ -0,0 +1,46 @@ +use std::net::{IpAddr, SocketAddr}; + +use anyhow::Context; +use tokio::io::{AsyncRead, AsyncWrite}; + +use crate::{PluginState, OPTION_GRPC_HOST, OPTION_GRPC_PORT}; + +use cln_plugin::ConfiguredPlugin; +pub struct GrpcRouterConfig { + host: IpAddr, + port: u16, +} + +impl GrpcRouterConfig { + pub fn from_configured_plugin( + plugin: &ConfiguredPlugin, + ) -> anyhow::Result> + where + I: AsyncRead + Send + Unpin + 'static, + O: AsyncWrite + Send + Unpin + 'static, + { + let port = plugin.option(&OPTION_GRPC_PORT).unwrap(); + let port = u16::try_from(port).with_context(|| { + format!( + "Invalid config for {}. The value {} is out-of-bounds.", + OPTION_GRPC_PORT.name(), + port + ) + })?; + + let host = plugin.option(&OPTION_GRPC_HOST).unwrap(); + let host = host.parse::().with_context(|| { + format!( + "Invalid config for {}. '{}' is not a valid ip-address.", + OPTION_GRPC_HOST.name(), + host + ) + })?; + + Ok(Some(GrpcRouterConfig { host, port })) + } + + pub fn socket_addr(&self) -> SocketAddr { + SocketAddr::new(self.host, self.port) + } +} From 75396c8dbe719f0c79b0d3142fc9fa1fe7148d1b Mon Sep 17 00:00:00 2001 From: Erik De Smedt Date: Mon, 16 Sep 2024 13:36:18 +0200 Subject: [PATCH 2/4] grpc-plugin: Support grpc-scheme (http/https) Also support using the cln-grpc plugin over http. This is useful for testing. As a safe-guard we only allow this configuration if the services binds to the loopback address. Changelog-Added: `grpc-scheme` option for grpc plugin --- plugins/grpc-plugin/src/main.rs | 64 ++++++++++++++++++++----------- plugins/grpc-plugin/src/router.rs | 45 +++++++++++++++++++--- 2 files changed, 81 insertions(+), 28 deletions(-) diff --git a/plugins/grpc-plugin/src/main.rs b/plugins/grpc-plugin/src/main.rs index efd19762d62c..eb7adb35bcf4 100644 --- a/plugins/grpc-plugin/src/main.rs +++ b/plugins/grpc-plugin/src/main.rs @@ -3,9 +3,10 @@ use cln_grpc::pb::node_server::NodeServer; use cln_plugin::{options, Builder, Plugin}; use cln_rpc::notifications::Notification; use log::{debug, warn}; -use router::GrpcRouterConfig; +use router::{GrpcRouterConfig, GrpcRouterScheme}; use std::path::PathBuf; use tokio::sync::broadcast; +use tonic::transport::ServerTlsConfig; mod router; mod tls; @@ -13,16 +14,22 @@ mod tls; #[derive(Clone, Debug)] struct PluginState { rpc_path: PathBuf, - identity: tls::Identity, - ca_cert: Vec, events: broadcast::Sender, } -const OPTION_GRPC_PORT: options::DefaultIntegerConfigOption = options::ConfigOption::new_i64_with_default( - "grpc-port", - 9736, - "Which port should the grpc plugin listen for incoming connections?" -); +const OPTION_GRPC_SCHEME: options::DefaultStringConfigOption = + options::ConfigOption::new_str_with_default( + "grpc-scheme", + "https", + "The scheme used by the gprc-plugin. Either 'http' or 'https'", + ); + +const OPTION_GRPC_PORT: options::DefaultIntegerConfigOption = + options::ConfigOption::new_i64_with_default( + "grpc-port", + 9736, + "Which port should the grpc plugin listen for incoming connections?" + ); const OPTION_GRPC_HOST: options::DefaultStringConfigOption = options::ConfigOption::new_str_with_default( "grpc-host", @@ -39,9 +46,8 @@ const OPTION_GRPC_MSG_BUFFER_SIZE : options::DefaultIntegerConfigOption = option async fn main() -> Result<()> { debug!("Starting grpc plugin"); - let directory = std::env::current_dir()?; - let plugin = match Builder::new(tokio::io::stdin(), tokio::io::stdout()) + .option(OPTION_GRPC_SCHEME) .option(OPTION_GRPC_PORT) .option(OPTION_GRPC_HOST) .option(OPTION_GRPC_MSG_BUFFER_SIZE) @@ -88,12 +94,8 @@ async fn main() -> Result<()> { let (sender, _) = broadcast::channel(buffer_size); - let (identity, ca_cert) = tls::init(&directory)?; - let state = PluginState { rpc_path: PathBuf::from(plugin.configuration().rpc_file.as_str()), - identity, - ca_cert, events: sender, }; @@ -113,31 +115,47 @@ async fn main() -> Result<()> { Ok(()) } -async fn run_interface(router_config: GrpcRouterConfig, state: PluginState) -> Result<()> { - let bind_addr = router_config.socket_addr(); - let identity = state.identity.to_tonic_identity(); - let ca_cert = tonic::transport::Certificate::from_pem(state.ca_cert); +fn create_server_tls_config() -> anyhow::Result { + let directory = std::env::current_dir()?; + let (identity, ca_cert) = tls::init(&directory)?; + + let identity = identity.to_tonic_identity(); + let ca_cert = tonic::transport::Certificate::from_pem(ca_cert); let tls = tonic::transport::ServerTlsConfig::new() .identity(identity) .client_ca_root(ca_cert); - let server = tonic::transport::Server::builder() - .tls_config(tls) - .context("configuring tls")? + return Ok(tls); +} + +async fn run_interface(router_config: GrpcRouterConfig, state: PluginState) -> Result<()> { + let bind_addr = router_config.socket_addr(); + + let mut server = match router_config.scheme { + GrpcRouterScheme::HTTP => tonic::transport::Server::builder(), + GrpcRouterScheme::HTTPS => { + let server_tls_config = create_server_tls_config()?; + tonic::transport::Server::builder() + .tls_config(server_tls_config) + .context("Configuring tls")? + } + }; + + let svc_handle = server .add_service(NodeServer::new( cln_grpc::Server::new(&state.rpc_path, state.events.clone()) .await .context("creating NodeServer instance")?, )) - .serve(bind_addr); + .serve(router_config.socket_addr()); debug!( "Connecting to {:?} and serving grpc on {:?}", &state.rpc_path, &bind_addr ); - server.await.context("serving requests")?; + svc_handle.await.context("serving requests")?; Ok(()) } diff --git a/plugins/grpc-plugin/src/router.rs b/plugins/grpc-plugin/src/router.rs index 071949c2c43a..db2810e4a5e1 100644 --- a/plugins/grpc-plugin/src/router.rs +++ b/plugins/grpc-plugin/src/router.rs @@ -1,14 +1,35 @@ use std::net::{IpAddr, SocketAddr}; +use std::str::FromStr; use anyhow::Context; use tokio::io::{AsyncRead, AsyncWrite}; -use crate::{PluginState, OPTION_GRPC_HOST, OPTION_GRPC_PORT}; - use cln_plugin::ConfiguredPlugin; + +use crate::{PluginState, OPTION_GRPC_HOST, OPTION_GRPC_PORT, OPTION_GRPC_SCHEME}; + +#[derive(Clone, Debug, PartialEq)] +pub enum GrpcRouterScheme { + HTTP, + HTTPS, +} + +impl FromStr for GrpcRouterScheme { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + match s { + "http" => Ok(GrpcRouterScheme::HTTP), + "https" => Ok(GrpcRouterScheme::HTTPS), + _ => anyhow::bail!("Invalid scheme"), + } + } +} + pub struct GrpcRouterConfig { - host: IpAddr, - port: u16, + pub scheme: GrpcRouterScheme, + pub host: IpAddr, + pub port: u16, } impl GrpcRouterConfig { @@ -28,6 +49,15 @@ impl GrpcRouterConfig { ) })?; + let scheme = plugin.option(&OPTION_GRPC_SCHEME).unwrap(); + let scheme = scheme.parse::().with_context(|| { + format!( + "Invalid config for {}. The config '{}' is invalid. Use either 'http' or 'https'.", + OPTION_GRPC_SCHEME.name(), + scheme + ) + })?; + let host = plugin.option(&OPTION_GRPC_HOST).unwrap(); let host = host.parse::().with_context(|| { format!( @@ -37,7 +67,12 @@ impl GrpcRouterConfig { ) })?; - Ok(Some(GrpcRouterConfig { host, port })) + if GrpcRouterScheme::HTTP == scheme && !host.is_loopback() { + anyhow::bail!("Invalid config: Scheme 'http' is only allowed on a loopback address. Try setting {} to 127.0.0.1", + OPTION_GRPC_HOST.name()); + } + + Ok(Some(GrpcRouterConfig { scheme, host, port })) } pub fn socket_addr(&self) -> SocketAddr { From 11284d56465f75553a6b34db704e024f1454424c Mon Sep 17 00:00:00 2001 From: Erik De Smedt Date: Mon, 16 Sep 2024 14:38:07 +0200 Subject: [PATCH 3/4] doc: Update documentation for `--grpc-scheme` --- .../getting-started/configuration.md | 13 ++++++++++++- doc/lightningd-config.5.md | 12 +++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/doc/getting-started/getting-started/configuration.md b/doc/getting-started/getting-started/configuration.md index 3e9767f366b2..5a7cb49a46c1 100644 --- a/doc/getting-started/getting-started/configuration.md +++ b/doc/getting-started/getting-started/configuration.md @@ -235,9 +235,20 @@ The [`lightning-listconfigs`](ref:lightning-listconfigs) command will output a v If you have an unencrypted `hsm_secret` you want to encrypt on-disk, or vice versa, see [`lightning-hsmtool`](ref:lightning-hsmtool). +- **grpc-scheme**=_scheme_ [plugin `cln-grpc`] + + The scheme on which the gRPC plugin will listen for incoming connections. The default is `https`. + The interface supports both `http` and `https`. However, `http` can only be used if `grpc-host` + is set to a loopback address which is `127.0.0.1` for IPv4. + +- **grpc-host**=_HOST_ [plugin `cln-grpc`] + + The IP address for the gRPC plugin to listen for incoming connections; + The default is the IPv4 loopback address `127.0.0.1`. + - **grpc-port**=_portnum_ [plugin `cln-grpc`] - The port number for the GRPC plugin to listen for incoming connections; default is not to activate the plugin at all. + The port number for the GRPC plugin to listen for incoming connections. Default is 9736. ### Lightning node customization options diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 7b867781d464..aca95d593191 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -310,15 +310,21 @@ If there is no `hsm_secret` yet, `lightningd` will create a new encrypted secret If you have an unencrypted `hsm_secret` you want to encrypt on-disk, or vice versa, see lightning-hsmtool(8). +* **grpc-scheme**=*scheme* [plugin `cln-grpc`] + + The scheme on which the gRPC plugin will listen for incoming connections. The + default is `https`. The interface supports both `http` and `https`. + However, `http` can only be used if `grpc-host` is set to a loopback address + which is `127.0.0.1` for IPv4. * **grpc-host**=*HOST* [plugin `cln-grpc`] - Defines the GRPC server host. Default is 127.0.0.1. + The IP address for the gRPC plugin to listen for incoming connections; + The default is the IPv4 loopback address `127.0.0.1`. * **grpc-port**=*portnum* [plugin `cln-grpc`] - The port number for the GRPC plugin to listen for incoming -connections. Default is 9736. + The port number for the GRPC plugin to listen for incoming connections. Default is 9736. * **grpc-msg-buffer-size**=*number* [plugin `cln-grpc`] From fdf3cf3adafe3d82249aa80fea66ba53c256b549 Mon Sep 17 00:00:00 2001 From: Erik De Smedt Date: Mon, 16 Sep 2024 14:52:54 +0200 Subject: [PATCH 4/4] test: test grpc to run on http and to refuses non-loopback port on http --- tests/test_cln_rs.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/test_cln_rs.py b/tests/test_cln_rs.py index eb7608376c5d..e89a4dcf53fc 100644 --- a/tests/test_cln_rs.py +++ b/tests/test_cln_rs.py @@ -20,7 +20,7 @@ def wait_for_grpc_start(node): """This can happen before "public key" which start() swallows""" - wait_for(lambda: node.daemon.is_in_log(r'serving grpc')) + wait_for(lambda: node.daemon.is_in_log(r'serving grpc on ')) def test_rpc_client(node_factory): @@ -105,6 +105,32 @@ def test_plugin_options_handle_defaults(node_factory): assert opts["multi-i64-option-default"] == [-42] +def test_grpc_connect_http(node_factory): + """Attempts to connect to the grpc interface and call getinfo over http""" + grpc_port = node_factory.get_unused_port() + l1 = node_factory.get_node(options={"grpc-port": str(grpc_port), "grpc-scheme": "http"}) + + wait_for_grpc_start(l1) + + channel = grpc.insecure_channel(f"localhost:{l1.grpc_port}") + stub = clnpb.NodeStub(channel) + + response = stub.Getinfo(clnpb.GetinfoRequest()) + print(response) + + +def test_grpc_connection_refuses_non_loopback_address(node_factory): + grpc_port = node_factory.get_unused_port() + options = { + "grpc-port": str(grpc_port), + "grpc-scheme": "http", + "grpc-host": "0.0.0.0" + } + + l1 = node_factory.get_node(options=options) + assert l1.daemon.is_in_log(r'Scheme \'http\' is only allowed on a loopback address') + + def test_grpc_connect(node_factory): """Attempts to connect to the grpc interface and call getinfo""" # These only exist if we have rust!