github-jwinterm/grin-wallet
1
0
Fork 0
mirror of https://github.com/jwinterm/grin-wallet.git synced 2026-10-17 07:11:36 +02:00
Code Issues Packages Projects Releases Wiki Activity

Harden the contract signing-context forget invariant

Browse Source 
Remove the broken, commented-out SEC_KEY_FAKE key-scrubbing: a zero secret
key is rejected by secp, and the scrubbing was redundant. The signing
context holding sec_key/sec_nonce is forgotten by deleting the private
context in save_step once signed (retained only at step2 for safe cancel),
and contexts are keyed by slate.id so a nonce is never reused across
messages.

Make that guarantee enforced rather than assumed: after a signed,
non-step2 save_step, verify the private context was actually removed and
error otherwise. Document the invariant at the sign site.
This commit is contained in:
jwinterm
2026-06-03 08:03:35 -04:00
parent 3a86bb4fce
commit 9d040b40f1
2 changed files with 19 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
libwallet/src/contract/slate.rs
+4 -13
View File
@@ -26,11 +26,6 @@ use super::types::ProofArgs;
use crate::contract::proofs::InvoiceProof;
use ed25519_dalek::PublicKey as DalekPublicKey;
/// TODO: Removed for now, consider secp error in sign function
/// The secret key we replace the actual key with after we have signed with the Context keys. This is
/// to prevent possibility of signing with the same key twice.
/// pub const SEC_KEY_FAKE: [u8; 32] = [0; 32];
/// Add payment proof data to slate, noop for sender
pub fn add_payment_proof<'a, T: ?Sized, C, K>(
w: &mut T,
@@ -192,7 +187,10 @@ pub fn transition_state(slate: &mut Slate) -> Result<(), Error> {
}
/// Add partial signature to the slate.
// TODO: Should be a sign & forget pubkey+nonce implementation.
// Nonce reuse is prevented by forgetting the signing context after a signed step:
// save_step deletes the private context (which holds sec_key/sec_nonce) once is_signed
// (except the deliberately-retained step2 context used for safe cancel). The context is
// keyed by slate.id, so a given nonce is only ever used for one message.
pub fn sign<'a, T: ?Sized, C, K>(
w: &mut T,
keychain_mask: Option<&SecretKey>,
@@ -213,13 +211,6 @@ where
);
debug!("contract::slate::sign => done");
// TODO: This produces a secp error, probably need a valid key. Verify that this is what we want to do.
// let fake_key = SecretKey::from_slice(keychain.secp(), &SEC_KEY_FAKE)?;
// context.sec_key = fake_key.clone();
// context.sec_nonce = fake_key.clone();
// context.initial_sec_key = fake_key.clone();
// context.initial_sec_nonce = fake_key.clone();
Ok(())
}
libwallet/src/contract/utils.rs
+15 -1
View File
@@ -285,8 +285,22 @@ where
}
batch.commit()?;
drop(batch);
// TODO: Assert we don't have the context to avoid potentially leaking it! Also write tests around this.
// Defense in depth: once we have signed (and are past the step2 context that is kept
// deliberately for safe cancel), the signing context holding sec_key/sec_nonce must be
// gone so the nonce can never be reused. It is deleted in the is_signed && !is_step2
// branch above; verify the deletion actually took effect.
if is_signed && !is_step2 {
if w
.get_private_context(keychain_mask, slate.id.as_bytes())
.is_ok()
{
return Err(Error::GenericError(
"signing context was not removed after signing".into(),
));
}
}
debug!("contract::utils::save_step => Atomic updated done");
Ok(())