revocation enforcement in signer

Author: devrandom

lightning/src/chain/keysinterface.rs

@@ -202,7 +202,8 @@ pub trait ChannelKeys : Send+Clone {
 	/// Gets the commitment seed for a specific commitment number, thereby revoking the commitment
 	///
 	/// After this is called, it is an error to try to sign the relevant local commitment transaction.
-	/// May be called more than once (e.g. if re-establishing the channel).
+	/// Must be called in monotonic order, without gaps. May be called more than once (e.g. if
+	/// re-establishing the channel).
 	/// Note that the commitment number starts at (1 << 48) - 1 and counts backwards.
 	fn get_revoke_commitment_secret(&self, idx: u64) -> [u8; 32];
 	/// Gets the local channel public keys and basepoints

lightning/src/ln/functional_tests.rs

@@ -1603,27 +1603,26 @@ fn test_fee_spike_violation_fails_htlc() {
 
 	// Get the EnforcingChannelKeys for each channel, which will be used to (1) get the keys
 	// needed to sign the new commitment tx and (2) sign the new commitment tx.
-	let (local_revocation_basepoint, local_htlc_basepoint, local_payment_point, local_secret, local_secret2) = {
+	let (local_revocation_basepoint, local_htlc_basepoint, local_payment_point, local_secret, next_local_point) = {
 		let chan_lock = nodes[0].node.channel_state.lock().unwrap();
 		let local_chan = chan_lock.by_id.get(&chan.2).unwrap();
 		let chan_keys = local_chan.get_local_keys();
 		let pubkeys = chan_keys.pubkeys();
 		(pubkeys.revocation_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
-		 chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER), chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2))
+		 chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER),
+		 chan_keys.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx))
 	};
-	let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_payment_point, remote_secret1) = {
+	let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_payment_point, remote_point) = {
 		let chan_lock = nodes[1].node.channel_state.lock().unwrap();
 		let remote_chan = chan_lock.by_id.get(&chan.2).unwrap();
 		let chan_keys = remote_chan.get_local_keys();
 		let pubkeys = chan_keys.pubkeys();
 		(pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
-		 chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1))
+		 chan_keys.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx))
 	};
 
 	// Assemble the set of keys we can use for signatures for our commitment_signed message.
-	let commitment_secret = SecretKey::from_slice(&remote_secret1).unwrap();
-	let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &commitment_secret);
-	let commit_tx_keys = chan_utils::TxCreationKeys::new(&secp_ctx, &per_commitment_point, &remote_delayed_payment_basepoint,
+	let commit_tx_keys = chan_utils::TxCreationKeys::new(&secp_ctx, &remote_point, &remote_delayed_payment_basepoint,
 		&remote_htlc_basepoint, &local_revocation_basepoint, &local_htlc_basepoint).unwrap();
 
 	// Build the remote commitment transaction so we can sign it, and then later use the
@@ -1706,10 +1705,11 @@ fn test_fee_spike_violation_fails_htlc() {
 	let _ = nodes[1].node.get_and_clear_pending_msg_events();
 
 	// Send the RAA to nodes[1].
-	let per_commitment_secret = local_secret;
-	let next_secret = SecretKey::from_slice(&local_secret2).unwrap();
-	let next_per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &next_secret);
-	let raa_msg = msgs::RevokeAndACK{ channel_id: chan.2, per_commitment_secret, next_per_commitment_point};
+	let raa_msg = msgs::RevokeAndACK {
+		channel_id: chan.2,
+		per_commitment_secret: local_secret,
+		next_per_commitment_point: next_local_point
+	};
 	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &raa_msg);
 
 	let events = nodes[1].node.get_and_clear_pending_msg_events();
@@ -8128,9 +8128,11 @@ fn test_counterparty_raa_skip_no_crash() {
 	let mut guard = nodes[0].node.channel_state.lock().unwrap();
 	let local_keys = &guard.by_id.get_mut(&channel_id).unwrap().local_keys;
 	const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
+	let per_commitment_secret = local_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER);
+	// Must revoke without gaps
+	local_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1);
 	let next_per_commitment_point = PublicKey::from_secret_key(&Secp256k1::new(),
 		&SecretKey::from_slice(&local_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
-	let per_commitment_secret = local_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER);
 
 	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(),
 		&msgs::RevokeAndACK { channel_id, per_commitment_secret, next_per_commitment_point });

lightning/src/util/enforcing_trait_impls.rs

@@ -15,11 +15,14 @@ use util::ser::{Writeable, Writer, Readable};
 use std::io::Error;
 use ln::msgs::DecodeError;
 
+const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
+
 /// Enforces some rules on ChannelKeys calls. Eventually we will probably want to expose a variant
 /// of this which would essentially be what you'd want to run on a hardware wallet.
 #[derive(Clone)]
 pub struct EnforcingChannelKeys {
 	pub inner: InMemoryChannelKeys,
+	revoked_commitment: Arc<Mutex<u64>>,
 	commitment_number_obscure_and_last: Arc<Mutex<(Option<u64>, u64)>>,
 }
 
@@ -28,6 +31,7 @@ impl EnforcingChannelKeys {
 		Self {
 			inner,
 			commitment_number_obscure_and_last: Arc::new(Mutex::new((None, 0))),
+			revoked_commitment: Arc::new(Mutex::new(INITIAL_COMMITMENT_NUMBER + 1)),
 		}
 	}
 }
@@ -52,7 +56,14 @@ impl ChannelKeys for EnforcingChannelKeys {
 		self.inner.get_per_commitment_point(idx, secp_ctx)
 	}
 
-	fn get_revoke_commitment_secret(&self, idx: u64) -> [u8; 32] { self.inner.get_revoke_commitment_secret(idx) }
+	fn get_revoke_commitment_secret(&self, idx: u64) -> [u8; 32] {
+		let mut revoked = self.revoked_commitment.lock().unwrap();
+		if idx != *revoked && idx != *revoked - 1 {
+			panic!("can only revoke the current or next unrevoked commitment - trying {}, revoked {}", idx, *revoked)
+		}
+		*revoked = idx;
+		self.inner.get_revoke_commitment_secret(idx)
+	}
 	fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
 	fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() }
 
@@ -124,6 +135,8 @@ impl ChannelKeys for EnforcingChannelKeys {
 impl Writeable for EnforcingChannelKeys {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
 		self.inner.write(writer)?;
+		let revoked = *self.revoked_commitment.lock().unwrap();
+		revoked.write(writer)?;
 		let (obscure, last) = *self.commitment_number_obscure_and_last.lock().unwrap();
 		obscure.write(writer)?;
 		last.write(writer)?;
@@ -134,9 +147,11 @@ impl Writeable for EnforcingChannelKeys {
 impl Readable for EnforcingChannelKeys {
 	fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
 		let inner = Readable::read(reader)?;
+		let revoked = Readable::read(reader)?;
 		let obscure_and_last = Readable::read(reader)?;
 		Ok(EnforcingChannelKeys {
 			inner: inner,
+			revoked_commitment: Arc::new(Mutex::new(revoked)),
 			commitment_number_obscure_and_last: Arc::new(Mutex::new(obscure_and_last))
 		})
 	}