diff --git a/docs/src/reference/security.md b/docs/src/reference/security.md index 83d0b0ac7..2aaad0c0e 100644 --- a/docs/src/reference/security.md +++ b/docs/src/reference/security.md @@ -90,6 +90,42 @@ operations. An attacker that gains access to the node is unable to provide either these signatures and will therefore fail to convince the signer of its injected commands. +### Request-context signatures + +The client signs the following bytes with ECDSA-P256/SHA256: + +```text +"greenlight/request-context/v1\0" +|| u64be(len(uri)) || uri +|| u64be(len(frame)) || frame +|| u64be(timestamp_ms) +``` + +The `uri` is the gRPC path, such as `/cln.Node/Pay`. The `frame` is the +gRPC frame, including its 5-byte header. Covering the URI prevents the +node from presenting a signed request as a different RPC with a compatible +protobuf encoding. The timestamp is authenticated, but its freshness is +not checked. + +The signer ignores any context that does not verify, including those signed +by older clients. An ignored context authorizes nothing: an enforcing signer +refuses operations that depend on it, and a `permissive` signer behaves as if +it were absent. Upgrade clients and signers together. + +A `Configure` context cached before this format was introduced is ignored in +the same way. If a signer already applied it, the close address stays +allowlisted because the allowlist is part of the signer state. An ignored +context cannot restore an address that was never applied or is missing from +that state. + +Upgrade clients before allowing the first `Configure` call on a node with an +updated signer. The plugin stores this request using a create-only datastore +write, so a first call from an old client can cache an unverifiable context +that a later `Configure` call cannot replace. Recovery requires removing the +cached `glconf/request` entry and configuring again with an updated client. +This is a configuration migration limitation, not a reason to accept a legacy +signature as authorization. + ## Signer State Integrity The signer does not store its own state. The node stores the diff --git a/libs/gl-client/CHANGELOG.md b/libs/gl-client/CHANGELOG.md index 48a66c8c5..c2189af5e 100644 --- a/libs/gl-client/CHANGELOG.md +++ b/libs/gl-client/CHANGELOG.md @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## Unreleased +### Changed + +- **BREAKING**: RPC request-context signatures now cover the gRPC method, so a node can no longer present a signed request as a different RPC. Signers ignore contexts signed by older clients, so upgrade clients and signers together. See the security reference. + +### Fixed + +- Validate the `Configure` close address against the signer's network before adding it to the allowlist. + ## [0.7.0] - 2026-10-12 ### Added diff --git a/libs/gl-client/src/lib.rs b/libs/gl-client/src/lib.rs index e77879f06..2e3830920 100644 --- a/libs/gl-client/src/lib.rs +++ b/libs/gl-client/src/lib.rs @@ -28,6 +28,8 @@ pub mod scheduler; /// move your funds. pub mod signer; +mod request_context; + pub mod metrics; pub mod persist; pub mod psbt; diff --git a/libs/gl-client/src/node/service.rs b/libs/gl-client/src/node/service.rs index 3243cb232..00a772a8a 100644 --- a/libs/gl-client/src/node/service.rs +++ b/libs/gl-client/src/node/service.rs @@ -109,13 +109,11 @@ impl Service> for AuthService { let data = body.data().await.unwrap().unwrap(); - // Copy used to create the signature (payload + associated data) - let mut buf = data.to_vec(); - // Associated data that is covered by the signature let mut ts = vec![]; ts.put_u64(time.try_into()?); - buf.put_u64(time.try_into()?); + + let buf = crate::request_context::payload(parts.uri.path(), &data, time.try_into()?); let rng = rand::SystemRandom::new(); let pubkey = keypair.public_key().as_ref(); diff --git a/libs/gl-client/src/request_context.rs b/libs/gl-client/src/request_context.rs new file mode 100644 index 000000000..9cbde95f7 --- /dev/null +++ b/libs/gl-client/src/request_context.rs @@ -0,0 +1,36 @@ +const DOMAIN: &[u8] = b"greenlight/request-context/v1\0"; + +/// The bytes a client signs and the signer verifies to authorize an RPC. +/// Integers are big-endian. +/// +/// A protobuf body can decode as several request types, so the URI is covered +/// to stop the node relabeling a signed request as a different RPC. +pub(crate) fn payload(uri: &str, frame: &[u8], timestamp: u64) -> Vec { + [ + DOMAIN, + &(uri.len() as u64).to_be_bytes(), + uri.as_bytes(), + &(frame.len() as u64).to_be_bytes(), + frame, + ×tamp.to_be_bytes(), + ] + .concat() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn payload_wire_format() { + // Fixed bytes document the protocol independently of the encoder. + let expected = b"greenlight/request-context/v1\0\ + \0\0\0\0\0\0\0\x11/cln.Node/Getinfo\ + \0\0\0\0\0\0\0\x05\0\0\0\0\0\ + \x01\x02\x03\x04\x05\x06\x07\x08"; + assert_eq!( + payload("/cln.Node/Getinfo", &[0; 5], 0x0102030405060708), + expected + ); + } +} diff --git a/libs/gl-client/src/signer/mod.rs b/libs/gl-client/src/signer/mod.rs index 707ada9d9..78c1dc233 100644 --- a/libs/gl-client/src/signer/mod.rs +++ b/libs/gl-client/src/signer/mod.rs @@ -60,6 +60,8 @@ pub use backup::{ }; pub mod model; mod report; +#[cfg(test)] +mod request_context_tests; mod resolve; mod splice_policy; @@ -597,14 +599,7 @@ impl Signer { .filter(|r| !r.pubkey.is_empty() && !r.signature.is_empty() && !r.rune.is_empty()) .map(|r| { let pk = UnparsedPublicKey::new(&ECDSA_P256_SHA256_FIXED, &r.pubkey); - let mut data = r.request.clone(); - - // If we have a timestamp associated we must add it to - // the payload being checked. Same thing happens on - // the client too. - if r.timestamp != 0 { - data.put_u64(r.timestamp); - } + let data = crate::request_context::payload(&r.uri, &r.request, r.timestamp); pk.verify(&data, &r.signature) .map_err(|e| anyhow!("signature verification failed: {}", e))?; @@ -874,6 +869,16 @@ impl Signer { for parsed_request in ctxrequests.iter() { match parsed_request { model::Request::GlConfig(gl_config) => { + // The node is untrusted; only a valid address for this + // network may authorize on-chain destinations. + if let Err(e) = gl_config + .close_to_addr + .parse::>() + .and_then(|a| a.require_network(self.network)) + { + warn!("Ignoring Configure with invalid close_to_addr: {}", e); + continue; + } let pubkey = PublicKey::from_slice(&self.id); match pubkey { Ok(p) => { diff --git a/libs/gl-client/src/signer/request_context_tests.rs b/libs/gl-client/src/signer/request_context_tests.rs new file mode 100644 index 000000000..2ff811e91 --- /dev/null +++ b/libs/gl-client/src/signer/request_context_tests.rs @@ -0,0 +1,291 @@ +use super::*; +use crate::{credentials, pb}; +use ring::signature::{self, EcdsaKeyPair, KeyPair}; +use vls_protocol::msgs::GetHeartbeat; + +const ADDRESS: &str = "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa"; +const CONFIGURE: &str = "/greenlight.Node/Configure"; + +fn mk_signer() -> Signer { + Signer::new( + vec![0; 32], + Network::Bitcoin, + credentials::Nobody::default(), + ) + .unwrap() +} + +fn pkcs8() -> Vec { + let rng = ring::rand::SystemRandom::new(); + EcdsaKeyPair::generate_pkcs8(&signature::ECDSA_P256_SHA256_FIXED_SIGNING, &rng) + .unwrap() + .as_ref() + .to_vec() +} + +fn keypair(pkcs8: &[u8]) -> EcdsaKeyPair { + EcdsaKeyPair::from_pkcs8(&signature::ECDSA_P256_SHA256_FIXED_SIGNING, pkcs8).unwrap() +} + +fn sign(key: &EcdsaKeyPair, data: &[u8]) -> Vec { + key.sign(&ring::rand::SystemRandom::new(), data) + .unwrap() + .as_ref() + .to_vec() +} + +fn frame(message: &impl prost::Message) -> Vec { + let payload = message.encode_to_vec(); + let mut frame = vec![0]; + frame.put_u32(payload.len() as u32); + frame.extend_from_slice(&payload); + frame +} + +fn rune(signer: &Signer, key: &EcdsaKeyPair) -> String { + let pubkey = format!("pubkey={}", hex::encode(key.public_key())); + signer.create_rune(None, vec![vec![&pubkey]]).unwrap() +} + +fn signed_context( + signer: &Signer, + key: &EcdsaKeyPair, + uri: &str, + frame: Vec, +) -> PendingRequest { + let timestamp = 1_700_000_000_000; + PendingRequest { + uri: uri.to_string(), + signature: sign( + key, + &crate::request_context::payload(uri, &frame, timestamp), + ), + request: frame, + pubkey: key.public_key().as_ref().to_vec(), + timestamp, + rune: general_purpose::URL_SAFE.decode(rune(signer, key)).unwrap(), + } +} + +fn configure(signer: &Signer, key: &EcdsaKeyPair, address: &str) -> PendingRequest { + let frame = frame(&pb::GlConfig { + close_to_addr: address.into(), + }); + signed_context(signer, key, CONFIGURE, frame) +} + +/// Run an operation that needs no context, which must succeed whatever the +/// contexts attached to it are. +async fn heartbeat(signer: &Signer, requests: Vec) -> HsmResponse { + signer + .process_request(HsmRequest { + request_id: 1, + raw: GetHeartbeat {}.as_vec(), + requests, + ..Default::default() + }) + .await + .unwrap() +} + +fn allowlist(signer: &Signer) -> Vec { + signer + .services + .persister + .get_node_allowlist(&PublicKey::from_slice(&signer.id).unwrap()) + .unwrap() +} + +#[test] +fn rpc_context_signature_binds_uri_frame_and_timestamp() { + let signer = mk_signer(); + let key = keypair(&pkcs8()); + let context = signed_context( + &signer, + &key, + "/cln.Node/Pay", + frame(&pb::cln::PayRequest { + bolt11: "invoice".into(), + ..Default::default() + }), + ); + let is_valid = |c: &PendingRequest| signer.check_request_auth(vec![c.clone()])[0].is_ok(); + assert!(is_valid(&context)); + + let mut changed = context.clone(); + changed.uri = "/cln.Node/CheckMessage".into(); + assert!(!is_valid(&changed)); + + let mut changed = context.clone(); + changed.request[5] ^= 1; + assert!(!is_valid(&changed)); + + let mut changed = context; + changed.timestamp += 1; + assert!(!is_valid(&changed)); +} + +#[tokio::test] +async fn rpc_context_relabeling_has_no_effect() { + let signer = mk_signer(); + let key = keypair(&pkcs8()); + let check_message = signed_context( + &signer, + &key, + "/cln.Node/CheckMessage", + frame(&pb::cln::CheckmessageRequest { + message: ADDRESS.into(), + ..Default::default() + }), + ); + let getinfo = signed_context(&signer, &key, "/cln.Node/Getinfo", vec![0; 5]); + for (request, target) in [ + (&check_message, "/cln.Node/Pay"), + (&check_message, "/cln.Node/PreApproveInvoice"), + (&check_message, CONFIGURE), + (&getinfo, "/cln.Node/Invoice"), + (&getinfo, CONFIGURE), + ] { + let mut relabeled = request.clone(); + relabeled.uri = target.into(); + // Protobuf decoding alone cannot distinguish these intents. + assert!(decode_request(relabeled.clone()).is_ok()); + assert!(signer.check_request_auth(vec![relabeled.clone()])[0].is_err()); + heartbeat(&signer, vec![relabeled]).await; + assert!(allowlist(&signer).is_empty()); + } +} + +/// Older clients sign only the frame and timestamp. Such a context authorizes +/// nothing, and must not stop the signer serving the node. +#[tokio::test] +async fn rpc_context_legacy_signature_is_ignored() { + let signer = mk_signer(); + let key = keypair(&pkcs8()); + let valid = configure(&signer, &key, ADDRESS); + + let mut legacy = valid.clone(); + let mut data = legacy.request.clone(); + data.put_u64(legacy.timestamp); + legacy.signature = sign(&key, &data); + + heartbeat(&signer, vec![legacy.clone()]).await; + assert!(allowlist(&signer).is_empty()); + + // An invalid sibling must not suppress an authentic command either. + heartbeat(&signer, vec![legacy, valid]).await; + assert_eq!(allowlist(&signer), [ADDRESS]); +} + +#[tokio::test] +async fn rpc_context_configure_only_allowlists_valid_addresses() { + let signer = mk_signer(); + let key = keypair(&pkcs8()); + heartbeat(&signer, vec![configure(&signer, &key, ADDRESS)]).await; + assert_eq!(allowlist(&signer), [ADDRESS]); + + for invalid in [ + "", + "not-an-address", + "payee:02abcdef", + "mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn", // Testnet + ] { + let context = configure(&signer, &key, invalid); + heartbeat(&signer, vec![context]).await; + assert_eq!(allowlist(&signer), [ADDRESS]); + } +} + +/// A signature made by the real client, over a real connection, must verify +/// against the URI and frame a node sees. Permissive signers would hide a +/// mismatch here, since they proceed without the contexts they ignore. +#[tokio::test] +async fn rpc_context_client_signature_verifies_over_the_wire() { + use std::convert::Infallible; + use std::task::{Context, Poll}; + use tokio::sync::mpsc; + use tonic::codegen::Body as _; + use tonic::transport::{server::TcpIncoming, Body, Channel, Server}; + use tower::Layer; + + /// Captures a request the way the plugin builds a context from it. + #[derive(Clone)] + struct Node(mpsc::UnboundedSender<(http::request::Parts, Vec)>); + + impl tower::Service> for Node { + type Response = http::Response; + type Error = Infallible; + type Future = futures::future::BoxFuture<'static, Result>; + + fn poll_ready(&mut self, _: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, request: http::Request) -> Self::Future { + let captured = self.0.clone(); + Box::pin(async move { + let (parts, mut body) = request.into_parts(); + let mut frame = vec![]; + while let Some(chunk) = body.data().await { + frame.extend_from_slice(&chunk.unwrap()); + } + captured.send((parts, frame)).unwrap(); + Ok(tonic::Status::ok("").to_http()) + }) + } + } + + impl tonic::server::NamedService for Node { + const NAME: &'static str = "cln.Node"; + } + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let incoming = TcpIncoming::from_listener(listener, true, None).unwrap(); + let (captured, mut requests) = mpsc::unbounded_channel(); + tokio::spawn( + Server::builder() + .add_service(Node(captured)) + .serve_with_incoming(incoming), + ); + + let signer = mk_signer(); + let pkcs8 = pkcs8(); + let key = keypair(&pkcs8); + let pem = format!( + "-----BEGIN PRIVATE KEY-----\n{}\n-----END PRIVATE KEY-----\n", + general_purpose::STANDARD.encode(&pkcs8) + ); + let channel = Channel::from_shared(format!("http://{address}")) + .unwrap() + .connect_lazy(); + let service = crate::node::service::AuthLayer::new(pem.into_bytes(), rune(&signer, &key)) + .unwrap() + .layer(channel); + // The node does not reply, only the request it captures matters here. + let _ = pb::cln::node_client::NodeClient::new(service) + .pay(pb::cln::PayRequest { + bolt11: "invoice".into(), + ..Default::default() + }) + .await; + + let (parts, request) = requests.recv().await.unwrap(); + let header = |name: &str| { + general_purpose::STANDARD_NO_PAD + .decode(&parts.headers[name]) + .unwrap() + }; + let context = PendingRequest { + uri: parts.uri.path_and_query().unwrap().to_string(), + request, + signature: header("glauthsig"), + pubkey: header("glauthpubkey"), + timestamp: u64::from_be_bytes(header("glts").try_into().unwrap()), + rune: general_purpose::URL_SAFE + .decode(&parts.headers["glrune"]) + .unwrap(), + }; + assert_eq!(context.uri, "/cln.Node/Pay"); + assert!(signer.check_request_auth(vec![context])[0].is_ok()); +} diff --git a/libs/gl-plugin/src/context.rs b/libs/gl-plugin/src/context.rs index fc1cfec8d..06e928ff1 100644 --- a/libs/gl-plugin/src/context.rs +++ b/libs/gl-plugin/src/context.rs @@ -20,7 +20,7 @@ pub struct Request { pubkey: Vec, // A signature by the caller's public key, authenticating the - // payload. + // versioned payload (URI, framed payload, and timestamp). signature: Vec, // The callers rune that is used to authorize the request.