From ac3064ba339d26b1e9fbe7df9139f15e07496eb3 Mon Sep 17 00:00:00 2001 From: Peter Neuroth Date: Sun, 10 Sep 2023 18:55:33 +0200 Subject: [PATCH] signer: Add rune check to the signer We create a context now and check the context agains the rune. Signed-off-by: Peter Neuroth --- libs/gl-client/src/runes.rs | 13 ++++++ libs/gl-client/src/signer/mod.rs | 74 ++++++++++++++++++++++++++------ 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/libs/gl-client/src/runes.rs b/libs/gl-client/src/runes.rs index e3ada39a9..6c17cdde6 100644 --- a/libs/gl-client/src/runes.rs +++ b/libs/gl-client/src/runes.rs @@ -159,6 +159,8 @@ pub struct Context { pub method: String, // The public key associated with the request. pub pubkey: String, + // The unique id. + pub unique_id: String, // The timestamp associated with the request. pub time: SystemTime, // Todo (nepet): Add param field that uses enum or serde to store the params of a call. @@ -178,6 +180,7 @@ impl Check for Context { /// * `Ok(())` if the check is successful, an `Err` containing a `RuneError` otherwise. fn check_alternative(&self, alt: &Alternative) -> anyhow::Result<(), RuneError> { let value = match alt.get_field().as_str() { + "" => self.unique_id.clone(), "method" => self.method.clone(), "pubkey" => self.pubkey.clone(), "time" => self @@ -320,6 +323,7 @@ mod tests { method: String::new(), pubkey: String::from("020000000000000000"), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r1.are_restrictions_met(ctx).is_ok()); // Check with method="ListFunds", pubkey=020000000000000000 @@ -327,6 +331,7 @@ mod tests { method: String::from("ListFunds"), pubkey: String::from("020000000000000000"), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r1.are_restrictions_met(ctx).is_ok()); // Check with method="GetInfo", pubkey="" @@ -334,6 +339,7 @@ mod tests { method: String::from("GetInfo"), pubkey: String::new(), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r2.are_restrictions_met(ctx).is_ok()); // Check with method="GetInfo", pubkey="020000000000000000" @@ -341,6 +347,7 @@ mod tests { method: String::from("GetInfo"), pubkey: String::from("020000000000000000"), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r2.are_restrictions_met(ctx).is_ok()); // Check with method="GetInfo", pubkey="" @@ -348,6 +355,7 @@ mod tests { method: String::from("GetInfo"), pubkey: String::new(), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r3.are_restrictions_met(ctx).is_ok()); // Check with method="", pubkey="020000" @@ -355,6 +363,7 @@ mod tests { method: String::new(), pubkey: String::from("020000000000000000"), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r4.are_restrictions_met(ctx).is_ok()); @@ -364,6 +373,7 @@ mod tests { method: String::from("ListFunds"), pubkey: String::from("030000"), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r1.are_restrictions_met(ctx).is_err()); // Check with method="ListFunds", pubkey=030000, wrong method. @@ -371,6 +381,7 @@ mod tests { method: String::from("ListFunds"), pubkey: String::from("030000"), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r2.are_restrictions_met(ctx).is_err()); // Check with pubkey=030000, pubkey present. @@ -378,6 +389,7 @@ mod tests { method: String::new(), pubkey: String::from("030000"), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r3.are_restrictions_met(ctx).is_err()); // Check with method="GetInfo", method present. @@ -385,6 +397,7 @@ mod tests { method: String::from("GetInfo"), pubkey: String::new(), time: SystemTime::now(), + unique_id: String::new(), }; assert!(r4.are_restrictions_met(ctx).is_err()); } diff --git a/libs/gl-client/src/signer/mod.rs b/libs/gl-client/src/signer/mod.rs index 36d9e3c5b..dbbc86b1e 100644 --- a/libs/gl-client/src/signer/mod.rs +++ b/libs/gl-client/src/signer/mod.rs @@ -3,6 +3,7 @@ use crate::pb::scheduler::{scheduler_client::SchedulerClient, NodeInfoRequest, U /// caller thread, streaming incoming requests, verifying them, /// signing if ok, and then shipping the response to the node. use crate::pb::{node_client::NodeClient, Empty, HsmRequest, HsmRequestContext, HsmResponse}; +use crate::runes; use crate::signer::resolve::Resolver; use crate::tls::TlsConfig; use crate::{node, node::Client}; @@ -24,6 +25,7 @@ use std::convert::TryFrom; use std::convert::TryInto; use std::sync::Arc; use std::sync::Mutex; +use std::time::SystemTime; use tokio::sync::mpsc; use tokio::time::{sleep, Duration}; use tonic::transport::{Endpoint, Uri}; @@ -285,14 +287,10 @@ impl Signer { return Err(anyhow!("rune is missing pubkey field")); } - let mut checks: HashMap = HashMap::new(); - checks.insert("pubkey".to_string(), hex::encode(request.pubkey)); - - // Runes only check on the version if the unique id field is set. The id - // and the version are part of the empty field. - if let Some(device_id) = rune.get_id() { - checks.insert("".to_string(), format!("{}-{}", device_id, RUNE_VERSION)); - } + let unique_id = match rune.get_id() { + Some(id) => format!("{}-{}", id, RUNE_VERSION), + None => String::new(), + }; // Check that the request points to `cln.Node`. let mut parts = request.uri.split('/'); @@ -322,12 +320,15 @@ impl Signer { return Err(anyhow!("can not extract uri form request")); } }; - checks.insert("method".to_string(), method.to_string()); - match self - .master_rune - .check_with_reason(&rune64, futhark::MapChecker { map: checks }) - { + let ctx = runes::Context { + method, + pubkey: hex::encode(request.pubkey), + time: SystemTime::now(), + unique_id, + }; + + match self.master_rune.check_with_reason(&rune64, ctx) { Ok(_) => Ok(()), Err(e) => Err(e.into()), } @@ -951,6 +952,7 @@ impl From for crate::pb::scheduler::StartupMessage { #[cfg(test)] mod tests { use crate::pb; + use std::{alloc::System, time::UNIX_EPOCH}; use super::*; @@ -1130,4 +1132,50 @@ mod tests { }; assert!(signer.verify_rune(r).is_err()); } + + #[test] + fn test_empty_rune_is_valid() { + let signer = + Signer::new(vec![0u8; 32], Network::Bitcoin, TlsConfig::new().unwrap()).unwrap(); + + // This is just a placeholder public key, could also be a different one; + let pubkey = signer.node_id(); + let pubkey_rest = format!("pubkey={}", hex::encode(&pubkey)); + + let rune = signer.create_rune(None, vec![vec![&pubkey_rest]]).unwrap(); + + assert!(signer + .verify_rune(crate::pb::PendingRequest { + request: vec![], + uri: String::new(), + signature: vec![], + pubkey, + timestamp: 0, + rune: general_purpose::URL_SAFE.decode(rune).unwrap(), + }) + .is_ok()); + } + + #[test] + fn test_empty_rune_checks_pubkey() { + let signer = + Signer::new(vec![0u8; 32], Network::Bitcoin, TlsConfig::new().unwrap()).unwrap(); + + // This is just a placeholder public key, could also be a different one; + let pubkey = signer.node_id(); + let pubkey_rest = format!("pubkey={}", hex::encode(&pubkey)); + + let rune = signer.create_rune(None, vec![vec![&pubkey_rest]]).unwrap(); + + assert!(signer + .verify_rune(crate::pb::PendingRequest { + request: vec![], + uri: String::new(), + signature: vec![], + pubkey: hex::decode("33aabb").unwrap(), + timestamp: 0, + rune: general_purpose::URL_SAFE.decode(rune).unwrap(), + }) + .is_err()); + } }