Skip to content

Commit

Permalink
Remove NodeSigner::get_node_secret
Browse files Browse the repository at this point in the history 
Secrets should not be exposed in-memory at the interface level as it
would be impossible the implement it against a hardware security
module/secure element.
  • Loading branch information
@wpaulino
wpaulino committed Jan 19, 2023
1 parent 9133bea commit acd2ae6
Show file tree
Hide file tree
Showing 13 changed files with 85 additions and 135 deletions.
21 changes: 11 additions & 10 deletions fuzz/src/chanmon_consistency.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,19 +189,21 @@ impl EntropySource for KeyProvider {
}

impl NodeSigner for KeyProvider {
fn get_node_secret(&self, _recipient: Recipient) -> Result<SecretKey, ()> {
Ok(self.node_secret.clone())
}

fn get_node_id(&self, recipient: Recipient) -> Result<PublicKey, ()> {
let secp_ctx = Secp256k1::signing_only();
Ok(PublicKey::from_secret_key(&secp_ctx, &self.get_node_secret(recipient)?))
let node_secret = match recipient {
Recipient::Node => Ok(&self.node_secret),
Recipient::PhantomNode => Err(())
}?;
Ok(PublicKey::from_secret_key(&Secp256k1::signing_only(), node_secret))
}

fn ecdh(&self, recipient: Recipient, other_key: &PublicKey, tweak: Option<&Scalar>) -> Result<SharedSecret, ()> {
let mut node_secret = self.get_node_secret(recipient)?;
let mut node_secret = match recipient {
Recipient::Node => Ok(self.node_secret.clone()),
Recipient::PhantomNode => Err(())
}?;
if let Some(tweak) = tweak {
node_secret = node_secret.mul_tweak(tweak).unwrap();
node_secret = node_secret.mul_tweak(tweak).map_err(|_| ())?;
}
Ok(SharedSecret::new(other_key, &node_secret))
}
Expand Down Expand Up @@ -234,7 +236,6 @@ impl SignerProvider for KeyProvider {
let id = channel_keys_id[0];
let keys = InMemorySigner::new(
&secp_ctx,
self.get_node_secret(Recipient::Node).unwrap(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, self.node_secret[31]]).unwrap(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, self.node_secret[31]]).unwrap(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, self.node_secret[31]]).unwrap(),
Expand All @@ -251,7 +252,7 @@ impl SignerProvider for KeyProvider {
fn read_chan_signer(&self, buffer: &[u8]) -> Result<Self::Signer, DecodeError> {
let mut reader = std::io::Cursor::new(buffer);

let inner: InMemorySigner = ReadableArgs::read(&mut reader, self.get_node_secret(Recipient::Node).unwrap())?;
let inner: InMemorySigner = Readable::read(&mut reader)?;
let state = self.make_enforcement_state_cell(inner.commitment_seed);

Ok(EnforcingSigner {
Expand Down
28 changes: 14 additions & 14 deletions fuzz/src/full_stack.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use lightning::util::errors::APIError;
use lightning::util::events::Event;
use lightning::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
use lightning::util::logger::Logger;
use lightning::util::ser::{ReadableArgs, Writeable};
use lightning::util::ser::{Readable, Writeable};

use crate::utils::test_logger;
use crate::utils::test_persister::TestPersister;
Expand Down Expand Up @@ -293,19 +293,21 @@ impl EntropySource for KeyProvider {
}

impl NodeSigner for KeyProvider {
fn get_node_secret(&self, _recipient: Recipient) -> Result<SecretKey, ()> {
Ok(self.node_secret.clone())
}

fn get_node_id(&self, recipient: Recipient) -> Result<PublicKey, ()> {
let secp_ctx = Secp256k1::signing_only();
Ok(PublicKey::from_secret_key(&secp_ctx, &self.get_node_secret(recipient)?))
let node_secret = match recipient {
Recipient::Node => Ok(&self.node_secret),
Recipient::PhantomNode => Err(())
}?;
Ok(PublicKey::from_secret_key(&Secp256k1::signing_only(), node_secret))
}

fn ecdh(&self, recipient: Recipient, other_key: &PublicKey, tweak: Option<&Scalar>) -> Result<SharedSecret, ()> {
let mut node_secret = self.get_node_secret(recipient)?;
let mut node_secret = match recipient {
Recipient::Node => Ok(self.node_secret.clone()),
Recipient::PhantomNode => Err(())
}?;
if let Some(tweak) = tweak {
node_secret = node_secret.mul_tweak(tweak).unwrap();
node_secret = node_secret.mul_tweak(tweak).map_err(|_| ())?;
}
Ok(SharedSecret::new(other_key, &node_secret))
}
Expand Down Expand Up @@ -341,7 +343,6 @@ impl SignerProvider for KeyProvider {
EnforcingSigner::new_with_revoked(if inbound {
InMemorySigner::new(
&secp_ctx,
self.node_secret.clone(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, ctr]).unwrap(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, ctr]).unwrap(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, ctr]).unwrap(),
Expand All @@ -354,7 +355,6 @@ impl SignerProvider for KeyProvider {
} else {
InMemorySigner::new(
&secp_ctx,
self.node_secret.clone(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, ctr]).unwrap(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, ctr]).unwrap(),
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, ctr]).unwrap(),
Expand All @@ -368,7 +368,7 @@ impl SignerProvider for KeyProvider {
}

fn read_chan_signer(&self, mut data: &[u8]) -> Result<EnforcingSigner, DecodeError> {
let inner: InMemorySigner = ReadableArgs::read(&mut data, self.node_secret.clone())?;
let inner: InMemorySigner = Readable::read(&mut data)?;
let state = Arc::new(Mutex::new(EnforcementState::new()));

Ok(EnforcingSigner::new_with_revoked(
Expand Down Expand Up @@ -452,7 +452,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
// keys subsequently generated in this test. Rather than regenerating all the messages manually,
// it's easier to just increment the counter here so the keys don't change.
keys_manager.counter.fetch_sub(3, Ordering::AcqRel);
let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret(Recipient::Node).unwrap());
let our_id = &keys_manager.get_node_id(Recipient::Node).unwrap();
let network_graph = Arc::new(NetworkGraph::new(genesis_block(network).block_hash(), Arc::clone(&logger)));
let gossip_sync = Arc::new(P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger)));
let scorer = FixedPenaltyScorer::with_penalty(0);
Expand All @@ -462,7 +462,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
chan_handler: channelmanager.clone(),
route_handler: gossip_sync.clone(),
onion_message_handler: IgnoringMessageHandler {},
}, our_network_key, 0, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 15, 0], Arc::clone(&logger), IgnoringMessageHandler{}, keys_manager.clone()));
}, 0, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 15, 0], Arc::clone(&logger), IgnoringMessageHandler{}, keys_manager.clone()));

let mut should_forward = false;
let mut payments_received: Vec<PaymentHash> = Vec::new();
Expand Down
16 changes: 9 additions & 7 deletions fuzz/src/onion_message.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,19 @@ impl EntropySource for KeyProvider {
}

impl NodeSigner for KeyProvider {
fn get_node_secret(&self, _recipient: Recipient) -> Result<SecretKey, ()> {
Ok(self.node_secret.clone())
}

fn get_node_id(&self, recipient: Recipient) -> Result<PublicKey, ()> {
let secp_ctx = Secp256k1::signing_only();
Ok(PublicKey::from_secret_key(&secp_ctx, &self.get_node_secret(recipient)?))
let node_secret = match recipient {
Recipient::Node => Ok(&self.node_secret),
Recipient::PhantomNode => Err(())
}?;
Ok(PublicKey::from_secret_key(&Secp256k1::signing_only(), node_secret))
}

fn ecdh(&self, recipient: Recipient, other_key: &PublicKey, tweak: Option<&Scalar>) -> Result<SharedSecret, ()> {
let mut node_secret = self.get_node_secret(recipient)?;
let mut node_secret = match recipient {
Recipient::Node => Ok(self.node_secret.clone()),
Recipient::PhantomNode => Err(())
}?;
if let Some(tweak) = tweak {
node_secret = node_secret.mul_tweak(tweak).map_err(|_| ())?;
}
Expand Down
4 changes: 2 additions & 2 deletions lightning-background-processor/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ mod tests {
use bitcoin::network::constants::Network;
use lightning::chain::{BestBlock, Confirm, chainmonitor};
use lightning::chain::channelmonitor::ANTI_REORG_DELAY;
use lightning::chain::keysinterface::{InMemorySigner, Recipient, EntropySource, KeysManager, NodeSigner};
use lightning::chain::keysinterface::{InMemorySigner, EntropySource, KeysManager};
use lightning::chain::transaction::OutPoint;
use lightning::get_event_msg;
use lightning::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChainParameters, ChannelManager, SimpleArcChannelManager};
Expand Down Expand Up @@ -786,7 +786,7 @@ mod tests {
let p2p_gossip_sync = Arc::new(P2PGossipSync::new(network_graph.clone(), Some(chain_source.clone()), logger.clone()));
let rapid_gossip_sync = Arc::new(RapidGossipSync::new(network_graph.clone()));
let msg_handler = MessageHandler { chan_handler: Arc::new(test_utils::TestChannelMessageHandler::new()), route_handler: Arc::new(test_utils::TestRoutingMessageHandler::new()), onion_message_handler: IgnoringMessageHandler{}};
let peer_manager = Arc::new(PeerManager::new(msg_handler, keys_manager.get_node_secret(Recipient::Node).unwrap(), 0, &seed, logger.clone(), IgnoringMessageHandler{}, keys_manager.clone()));
let peer_manager = Arc::new(PeerManager::new(msg_handler, 0, &seed, logger.clone(), IgnoringMessageHandler{}, keys_manager.clone()));
let node = Node { node: manager, p2p_gossip_sync, rapid_gossip_sync, peer_manager, chain_monitor, persister, tx_broadcaster, network_graph, logger, best_block, scorer };
nodes.push(node);
}
Expand Down
6 changes: 3 additions & 3 deletions lightning-net-tokio/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ mod tests {
chan_handler: Arc::clone(&a_handler),
route_handler: Arc::clone(&a_handler),
onion_message_handler: Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}),
}, a_key.clone(), 0, &[1; 32], Arc::new(TestLogger()), Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}), Arc::new(TestNodeSigner::new(a_key))));
}, 0, &[1; 32], Arc::new(TestLogger()), Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}), Arc::new(TestNodeSigner::new(a_key))));

let (b_connected_sender, mut b_connected) = mpsc::channel(1);
let (b_disconnected_sender, mut b_disconnected) = mpsc::channel(1);
Expand All @@ -716,7 +716,7 @@ mod tests {
chan_handler: Arc::clone(&b_handler),
route_handler: Arc::clone(&b_handler),
onion_message_handler: Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}),
}, b_key.clone(), 0, &[2; 32], Arc::new(TestLogger()), Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}), Arc::new(TestNodeSigner::new(b_key))));
}, 0, &[2; 32], Arc::new(TestLogger()), Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}), Arc::new(TestNodeSigner::new(b_key))));

// We bind on localhost, hoping the environment is properly configured with a local
// address. This may not always be the case in containers and the like, so if this test is
Expand Down Expand Up @@ -769,7 +769,7 @@ mod tests {
chan_handler: Arc::new(lightning::ln::peer_handler::ErroringMessageHandler::new()),
onion_message_handler: Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}),
route_handler: Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}),
}, a_key, 0, &[1; 32], Arc::new(TestLogger()), Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}), Arc::new(TestNodeSigner::new(a_key))));
}, 0, &[1; 32], Arc::new(TestLogger()), Arc::new(lightning::ln::peer_handler::IgnoringMessageHandler{}), Arc::new(TestNodeSigner::new(a_key))));

// Make two connections, one for an inbound and one for an outbound connection
let conn_a = {
Expand Down
1 change: 0 additions & 1 deletion lightning/src/chain/channelmonitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4185,7 +4185,6 @@ mod tests {
SecretKey::from_slice(&[41; 32]).unwrap(),
SecretKey::from_slice(&[41; 32]).unwrap(),
SecretKey::from_slice(&[41; 32]).unwrap(),
SecretKey::from_slice(&[41; 32]).unwrap(),
[41; 32],
0,
[0; 32],
Expand Down
Loading

0 comments on commit acd2ae6

Please sign in to comment.