Skip to content

Commit

Permalink
fix!: fix opcode signatures (#5966)
Browse files Browse the repository at this point in the history 
Description
---
Fixes opcode signatures to mitigate security issues.

Closes #5817.

Motivation and Context
---
Handling of signature-related opcodes introduces security issues related
to signature forgery as described in #5817. This PR changes how
signature-related message data is handled. Because of the new design,
signature arithmetic support is removed.

How Has This Been Tested?
---
Existing tests pass or have been updated to reflect the new design.

What process can a PR reviewer use to test or verify this change?
---
Check that the new design matches the intent of the signature-related
opcodes. Check that test modifications are correct.

BREAKING CHANGE: Changes how some opcodes are processed, which renders
some existing scripts and transactions invalid.
  • Loading branch information
@AaronFeickert
AaronFeickert committed Nov 21, 2023
1 parent 12e84f4 commit dc26ca6
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 92 deletions.
11 changes: 10 additions & 1 deletion infrastructure/tari_script/src/lib.rs
View file
Expand Up @@ -27,7 +27,16 @@ pub use op_codes::{slice_to_boxed_hash, slice_to_hash, HashValue, Message, Opcod
pub use script::TariScript;
pub use script_context::ScriptContext;
pub use stack::{ExecutionStack, StackItem};
use tari_crypto::ristretto::RistrettoPublicKey;
use tari_crypto::{
hash_domain,
ristretto::{RistrettoPublicKey, RistrettoSecretKey},
signatures::SchnorrSignature,
};

hash_domain!(CheckSigHashDomain, "com.tari.script.check_sig", 1);

/// The type used for `CheckSig`, `CheckMultiSig`, and related opcodes' signatures
pub type CheckSigSchnorrSignature = SchnorrSignature<RistrettoPublicKey, RistrettoSecretKey, CheckSigHashDomain>;

/// The standard payment script to be used for one-sided payment to stealth addresses
pub fn stealth_payment_script(
Expand Down
121 changes: 49 additions & 72 deletions infrastructure/tari_script/src/script.rs
View file
Expand Up @@ -26,7 +26,7 @@ use sha2::Sha256;
use sha3::Sha3_256;
use tari_crypto::{
keys::PublicKey,
ristretto::{RistrettoPublicKey, RistrettoSchnorr, RistrettoSecretKey},
ristretto::{RistrettoPublicKey, RistrettoSecretKey},
};
use tari_utilities::{
hex::{from_hex, to_hex, Hex, HexError},
Expand All @@ -36,6 +36,7 @@ use tari_utilities::{
use crate::{
op_codes::Message,
slice_to_hash,
CheckSigSchnorrSignature,
ExecutionStack,
HashValue,
Opcode,
Expand Down Expand Up @@ -531,14 +532,13 @@ impl TariScript {
}

fn handle_op_add(stack: &mut ExecutionStack) -> Result<(), ScriptError> {
use StackItem::{Commitment, Number, PublicKey, Signature};
use StackItem::{Commitment, Number, PublicKey};
let top = stack.pop().ok_or(ScriptError::StackUnderflow)?;
let two = stack.pop().ok_or(ScriptError::StackUnderflow)?;
match (top, two) {
(Number(v1), Number(v2)) => stack.push(Number(v1.checked_add(v2).ok_or(ScriptError::ValueExceedsBounds)?)),
(Commitment(c1), Commitment(c2)) => stack.push(Commitment(&c1 + &c2)),
(PublicKey(p1), PublicKey(p2)) => stack.push(PublicKey(&p1 + &p2)),
(Signature(s1), Signature(s2)) => stack.push(Signature(&s1 + &s2)),
(_, _) => Err(ScriptError::IncompatibleTypes),
}
}
Expand Down Expand Up @@ -573,7 +573,7 @@ impl TariScript {
let pk = stack.pop().ok_or(ScriptError::StackUnderflow)?;
let sig = stack.pop().ok_or(ScriptError::StackUnderflow)?;
match (pk, sig) {
(PublicKey(p), Signature(s)) => Ok(s.verify_raw_canonical(&p, &message)),
(PublicKey(p), Signature(s)) => Ok(s.verify(&p, message)),
(..) => Err(ScriptError::IncompatibleTypes),
}
}
Expand Down Expand Up @@ -615,7 +615,7 @@ impl TariScript {
StackItem::Signature(s) => Ok(s),
_ => Err(ScriptError::IncompatibleTypes),
})
.collect::<Result<Vec<RistrettoSchnorr>, ScriptError>>()?;
.collect::<Result<Vec<CheckSigSchnorrSignature>, ScriptError>>()?;

// keep a hashset of unique signatures used to prevent someone putting the same signature in more than once.
#[allow(clippy::mutable_key_type)]
Expand All @@ -638,7 +638,7 @@ impl TariScript {
}

for pk in pub_keys.by_ref() {
if s.verify_raw_canonical(pk, &message) {
if s.verify(pk, message) {
sig_set.insert(s);
agg_pub_key = agg_pub_key + pk;
break;
Expand Down Expand Up @@ -733,14 +733,15 @@ mod test {
use sha3::Sha3_256 as Sha3;
use tari_crypto::{
keys::{PublicKey, SecretKey},
ristretto::{pedersen::PedersenCommitment, RistrettoPublicKey, RistrettoSchnorr, RistrettoSecretKey},
ristretto::{pedersen::PedersenCommitment, RistrettoPublicKey, RistrettoSecretKey},
};
use tari_utilities::{hex::Hex, ByteArray};

use crate::{
error::ScriptError,
inputs,
op_codes::{slice_to_boxed_hash, slice_to_boxed_message, HashValue, Message},
CheckSigSchnorrSignature,
ExecutionStack,
Opcode::CheckMultiSigVerifyAggregatePubKey,
ScriptContext,
Expand Down Expand Up @@ -1088,9 +1089,8 @@ mod test {
use crate::StackItem::Number;
let mut rng = rand::thread_rng();
let (pvt_key, pub_key) = RistrettoPublicKey::random_keypair(&mut rng);
let nonce = RistrettoSecretKey::random(&mut rng);
let m_key = RistrettoSecretKey::random(&mut rng);
let sig = RistrettoSchnorr::sign_raw_canonical(&pvt_key, nonce, m_key.as_bytes()).unwrap();
let sig = CheckSigSchnorrSignature::sign(&pvt_key, m_key.as_bytes(), &mut rng).unwrap();
let msg = slice_to_boxed_message(m_key.as_bytes());
let script = script!(CheckSig(msg));
let inputs = inputs!(sig.clone(), pub_key.clone());
Expand All @@ -1110,9 +1110,8 @@ mod test {
use crate::StackItem::Number;
let mut rng = rand::thread_rng();
let (pvt_key, pub_key) = RistrettoPublicKey::random_keypair(&mut rng);
let nonce = RistrettoSecretKey::random(&mut rng);
let m_key = RistrettoSecretKey::random(&mut rng);
let sig = RistrettoSchnorr::sign_raw_canonical(&pvt_key, nonce, m_key.as_bytes()).unwrap();
let sig = CheckSigSchnorrSignature::sign(&pvt_key, m_key.as_bytes(), &mut rng).unwrap();
let msg = slice_to_boxed_message(m_key.as_bytes());
let script = script!(CheckSigVerify(msg) PushOne);
let inputs = inputs!(sig.clone(), pub_key.clone());
Expand All @@ -1131,7 +1130,7 @@ mod test {
n: usize,
) -> (
Box<Message>,
Vec<(RistrettoSecretKey, RistrettoPublicKey, RistrettoSchnorr)>,
Vec<(RistrettoSecretKey, RistrettoPublicKey, CheckSigSchnorrSignature)>,
) {
let mut rng = rand::thread_rng();
let mut data = Vec::with_capacity(n);
Expand All @@ -1140,8 +1139,7 @@ mod test {

for _ in 0..n {
let (k, p) = RistrettoPublicKey::random_keypair(&mut rng);
let r = RistrettoSecretKey::random(&mut rng);
let s = RistrettoSchnorr::sign_raw_canonical(&k, r, m.as_bytes()).unwrap();
let s = CheckSigSchnorrSignature::sign(&k, m.as_bytes(), &mut rng).unwrap();
data.push((k, p, s));
}

Expand All @@ -1157,17 +1155,12 @@ mod test {
let (k_bob, p_bob) = RistrettoPublicKey::random_keypair(&mut rng);
let (k_eve, _) = RistrettoPublicKey::random_keypair(&mut rng);
let (k_carol, p_carol) = RistrettoPublicKey::random_keypair(&mut rng);
let r1 = RistrettoSecretKey::random(&mut rng);
let r2 = RistrettoSecretKey::random(&mut rng);
let r3 = RistrettoSecretKey::random(&mut rng);
let r4 = RistrettoSecretKey::random(&mut rng);
let r5 = RistrettoSecretKey::random(&mut rng);
let m = RistrettoSecretKey::random(&mut rng);
let s_alice = RistrettoSchnorr::sign_raw_canonical(&k_alice, r1, m.as_bytes()).unwrap();
let s_bob = RistrettoSchnorr::sign_raw_canonical(&k_bob, r2, m.as_bytes()).unwrap();
let s_eve = RistrettoSchnorr::sign_raw_canonical(&k_eve, r3, m.as_bytes()).unwrap();
let s_carol = RistrettoSchnorr::sign_raw_canonical(&k_carol, r4, m.as_bytes()).unwrap();
let s_alice2 = RistrettoSchnorr::sign_raw_canonical(&k_alice, r5, m.as_bytes()).unwrap();
let s_alice = CheckSigSchnorrSignature::sign(&k_alice, m.as_bytes(), &mut rng).unwrap();
let s_bob = CheckSigSchnorrSignature::sign(&k_bob, m.as_bytes(), &mut rng).unwrap();
let s_eve = CheckSigSchnorrSignature::sign(&k_eve, m.as_bytes(), &mut rng).unwrap();
let s_carol = CheckSigSchnorrSignature::sign(&k_carol, m.as_bytes(), &mut rng).unwrap();
let s_alice2 = CheckSigSchnorrSignature::sign(&k_alice, m.as_bytes(), &mut rng).unwrap();
let msg = slice_to_boxed_message(m.as_bytes());

// 1 of 2
Expand Down Expand Up @@ -1360,15 +1353,11 @@ mod test {
let (k_bob, p_bob) = RistrettoPublicKey::random_keypair(&mut rng);
let (k_eve, _) = RistrettoPublicKey::random_keypair(&mut rng);
let (k_carol, p_carol) = RistrettoPublicKey::random_keypair(&mut rng);
let r1 = RistrettoSecretKey::random(&mut rng);
let r2 = RistrettoSecretKey::random(&mut rng);
let r3 = RistrettoSecretKey::random(&mut rng);
let r4 = RistrettoSecretKey::random(&mut rng);
let m = RistrettoSecretKey::random(&mut rng);
let s_alice = RistrettoSchnorr::sign_raw_canonical(&k_alice, r1, m.as_bytes()).unwrap();
let s_bob = RistrettoSchnorr::sign_raw_canonical(&k_bob, r2, m.as_bytes()).unwrap();
let s_eve = RistrettoSchnorr::sign_raw_canonical(&k_eve, r3, m.as_bytes()).unwrap();
let s_carol = RistrettoSchnorr::sign_raw_canonical(&k_carol, r4, m.as_bytes()).unwrap();
let s_alice = CheckSigSchnorrSignature::sign(&k_alice, m.as_bytes(), &mut rng).unwrap();
let s_bob = CheckSigSchnorrSignature::sign(&k_bob, m.as_bytes(), &mut rng).unwrap();
let s_eve = CheckSigSchnorrSignature::sign(&k_eve, m.as_bytes(), &mut rng).unwrap();
let s_carol = CheckSigSchnorrSignature::sign(&k_carol, m.as_bytes(), &mut rng).unwrap();
let msg = slice_to_boxed_message(m.as_bytes());

// 1 of 2
Expand Down Expand Up @@ -1547,25 +1536,6 @@ mod test {
let result = script.execute(&inputs).unwrap();
assert_eq!(result, Number(1));
}
#[test]
fn add_partial_signatures() {
use crate::StackItem::Number;
let mut rng = rand::thread_rng();
let (k1, p1) = RistrettoPublicKey::random_keypair(&mut rng);
let (k2, p2) = RistrettoPublicKey::random_keypair(&mut rng);
let r1 = RistrettoSecretKey::random(&mut rng);
let r2 = RistrettoSecretKey::random(&mut rng);

let m = RistrettoSecretKey::random(&mut rng);
let msg = slice_to_boxed_message(m.as_bytes());
let script = script!(Add RevRot Add CheckSigVerify(msg) PushOne);

let s1 = RistrettoSchnorr::sign_raw_canonical(&k1, r1, m.as_bytes()).unwrap();
let s2 = RistrettoSchnorr::sign_raw_canonical(&k2, r2, m.as_bytes()).unwrap();
let inputs = inputs!(p1, p2, s1, s2);
let result = script.execute(&inputs).unwrap();
assert_eq!(result, Number(1));
}

#[test]
fn pay_to_public_key_hash() {
Expand Down Expand Up @@ -1593,22 +1563,32 @@ mod test {
}

#[test]
fn hex_only() {
use crate::StackItem::Number;
let hex = "0500f7c695528c858cde76dab3076908e01228b6dbdd5f671bed1b03b89e170c313d415e0584ef82b79e3bf9bdebeeef53d13aefdc0cfa64f616acea0229e6ee0f0456c0fa32558d6edc0916baa26b48e745de834571534ca253ea82435f08ebbc7c";
let inputs = ExecutionStack::from_hex(hex).unwrap();
let script =
TariScript::from_hex("71b07aae2337ce44f9ebb6169c863ec168046cb35ab4ef7aa9ed4f5f1f669bb74b09e581ac276657a418820f34036b20ea615302b373c70ac8feab8d30681a3e0f0960e708")
.unwrap();
let result = script.execute(&inputs).unwrap();
assert_eq!(result, Number(1));
fn hex_roundtrip() {
// Generate a signature
let mut rng = rand::thread_rng();
let (secret_key, public_key) = RistrettoPublicKey::random_keypair(&mut rng);
let message = [1u8; 32];
let sig = CheckSigSchnorrSignature::sign(&secret_key, message, &mut rng).unwrap();

// Try again with invalid sig
let inputs = ExecutionStack::from_hex("0500b7c695528c858cde76dab3076908e01228b6dbdd5f671bed1b03\
b89e170c314c7b413e971dbb85879ba990e851607454da4bdf65839456d7cac19e5a338f060456c0fa32558d6edc0916baa26b48e745de8\
34571534ca253ea82435f08ebbc7c").unwrap();
let result = script.execute(&inputs).unwrap();
assert_eq!(result, Number(0));
// Produce a script using the signature
let script = script!(CheckSig(slice_to_boxed_message(message.as_bytes())));

// Produce input satisfying the script
let input = inputs!(sig, public_key);

// Check that script execution succeeds
assert_eq!(script.execute(&input).unwrap(), StackItem::Number(1));

// Convert the script to hex and back
let parsed_script = TariScript::from_hex(script.to_hex().as_str()).unwrap();
assert_eq!(script.to_opcodes(), parsed_script.to_opcodes());

// Convert the input to hex and back
let parsed_input = ExecutionStack::from_hex(input.to_hex().as_str()).unwrap();
assert_eq!(input, parsed_input);

// Check that script execution still succeeds
assert_eq!(parsed_script.execute(&parsed_input).unwrap(), StackItem::Number(1));
}

#[test]
Expand Down Expand Up @@ -1688,16 +1668,13 @@ mod test {
let (k_alice, p_alice) = RistrettoPublicKey::random_keypair(&mut rng);
let (k_bob, p_bob) = RistrettoPublicKey::random_keypair(&mut rng);
let (k_eve, _) = RistrettoPublicKey::random_keypair(&mut rng);
let r1 = RistrettoSecretKey::random(&mut rng);
let r2 = RistrettoSecretKey::random(&mut rng);
let r3 = RistrettoSecretKey::random(&mut rng);

let m = RistrettoSecretKey::random(&mut rng);
let msg = slice_to_boxed_message(m.as_bytes());

let s_alice = RistrettoSchnorr::sign_raw_canonical(&k_alice, r1, m.as_bytes()).unwrap();
let s_bob = RistrettoSchnorr::sign_raw_canonical(&k_bob, r2, m.as_bytes()).unwrap();
let s_eve = RistrettoSchnorr::sign_raw_canonical(&k_eve, r3, m.as_bytes()).unwrap();
let s_alice = CheckSigSchnorrSignature::sign(&k_alice, m.as_bytes(), &mut rng).unwrap();
let s_bob = CheckSigSchnorrSignature::sign(&k_bob, m.as_bytes(), &mut rng).unwrap();
let s_eve = CheckSigSchnorrSignature::sign(&k_eve, m.as_bytes(), &mut rng).unwrap();

// 1 of 2
use crate::Opcode::{CheckSig, Drop, Dup, Else, EndIf, IfThen, PushPubKey, Return};
Expand Down
34 changes: 15 additions & 19 deletions infrastructure/tari_script/src/stack.rs
View file
Expand Up @@ -19,7 +19,7 @@ use std::{convert::TryFrom, io};

use borsh::{BorshDeserialize, BorshSerialize};
use integer_encoding::{VarIntReader, VarIntWriter};
use tari_crypto::ristretto::{pedersen::PedersenCommitment, RistrettoPublicKey, RistrettoSchnorr, RistrettoSecretKey};
use tari_crypto::ristretto::{pedersen::PedersenCommitment, RistrettoPublicKey, RistrettoSecretKey};
use tari_utilities::{
hex::{from_hex, to_hex, Hex, HexError},
ByteArray,
Expand All @@ -28,6 +28,7 @@ use tari_utilities::{
use crate::{
error::ScriptError,
op_codes::{HashValue, ScalarValue},
CheckSigSchnorrSignature,
};

pub const MAX_STACK_SIZE: usize = 255;
Expand Down Expand Up @@ -66,7 +67,7 @@ pub enum StackItem {
Scalar(ScalarValue),
Commitment(PedersenCommitment),
PublicKey(RistrettoPublicKey),
Signature(RistrettoSchnorr),
Signature(CheckSigSchnorrSignature),
}

impl StackItem {
Expand Down Expand Up @@ -168,15 +169,15 @@ impl StackItem {
}
let r = RistrettoPublicKey::from_canonical_bytes(&b[..32]).ok()?;
let s = RistrettoSecretKey::from_canonical_bytes(&b[32..64]).ok()?;
let sig = RistrettoSchnorr::new(r, s);
let sig = CheckSigSchnorrSignature::new(r, s);
Some((StackItem::Signature(sig), &b[64..]))
}
}

stack_item_from!(i64 => Number);
stack_item_from!(PedersenCommitment => Commitment);
stack_item_from!(RistrettoPublicKey => PublicKey);
stack_item_from!(RistrettoSchnorr => Signature);
stack_item_from!(CheckSigSchnorrSignature => Signature);
stack_item_from!(ScalarValue => Scalar);

#[derive(Debug, Default, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -391,29 +392,25 @@ fn counter(values: [u8; 6], item: &StackItem) -> [u8; 6] {
mod test {
use blake2::Blake2b;
use borsh::{BorshDeserialize, BorshSerialize};
use digest::{
consts::{U32, U64},
Digest,
};
use digest::{consts::U32, Digest};
use rand::rngs::OsRng;
use tari_crypto::{
keys::{PublicKey, SecretKey},
ristretto::{pedersen::PedersenCommitment, RistrettoPublicKey, RistrettoSchnorr, RistrettoSecretKey},
ristretto::{pedersen::PedersenCommitment, RistrettoPublicKey, RistrettoSecretKey},
};
use tari_utilities::{
hex::{from_hex, Hex},
message_format::MessageFormat,
ByteArray,
};

use crate::{op_codes::ScalarValue, ExecutionStack, HashValue, StackItem};
use crate::{op_codes::ScalarValue, CheckSigSchnorrSignature, ExecutionStack, HashValue, StackItem};

#[test]
fn as_bytes_roundtrip() {
use crate::StackItem::{Number, PublicKey, Signature};
let k = RistrettoSecretKey::random(&mut rand::thread_rng());
let p = RistrettoPublicKey::from_secret_key(&k);
let s = RistrettoSchnorr::sign(&k, b"hi", &mut OsRng).unwrap();
let s = CheckSigSchnorrSignature::sign(&k, b"hi", &mut OsRng).unwrap();
let items = vec![Number(5432), Number(21), Signature(s), PublicKey(p)];
let stack = ExecutionStack::new(items);
let bytes = stack.to_bytes();
Expand All @@ -428,12 +425,11 @@ mod test {
let r =
RistrettoSecretKey::from_hex("193ee873f3de511eda8ae387db6498f3d194d31a130a94cdf13dc5890ec1ad0f").unwrap();
let p = RistrettoPublicKey::from_secret_key(&k);
let m = RistrettoSecretKey::from_uniform_bytes(&Blake2b::<U64>::digest(b"Hello Tari Script")).unwrap();
let sig = RistrettoSchnorr::sign_raw_canonical(&k, r, m.as_bytes()).unwrap();
let mut scalar: ScalarValue = [0u8; 32];
scalar.copy_from_slice(m.as_bytes());
let inputs = inputs!(sig, p, scalar);
assert_eq!(inputs.to_hex(), "0500f7c695528c858cde76dab3076908e01228b6dbdd5f671bed1b03b89e170c315c4a28c0202dec8769e7a6cc5b407e90664ce73c57404ab9c288bfe6a72d0d090456c0fa32558d6edc0916baa26b48e745de834571534ca253ea82435f08ebbc7c067c8f42406bb109bfcf5aadf0c72d9324a49b9f4758c83fb2f3364baf562f7d00");
let m = [1u8; 32];
let sig = CheckSigSchnorrSignature::sign_with_nonce_and_message(&k, r, m).unwrap();
let inputs = inputs!(sig, p, m as HashValue);
assert_eq!(inputs.to_hex(),
"0500f7c695528c858cde76dab3076908e01228b6dbdd5f671bed1b03b89e170c31c6134be1c65544fa3f26c59903165f664db0dc364cbbaa4b35a9b33342cc01000456c0fa32558d6edc0916baa26b48e745de834571534ca253ea82435f08ebbc7c060101010101010101010101010101010101010101010101010101010101010101");
}

#[test]
Expand Down Expand Up @@ -497,7 +493,7 @@ mod test {
RistrettoPublicKey::from_hex("56c0fa32558d6edc0916baa26b48e745de834571534ca253ea82435f08ebbc7c").unwrap();
let s =
RistrettoSecretKey::from_hex("6db1023d5c46d78a97da8eb6c5a37e00d5f2fee182dcb38c1b6c65e90a43c109").unwrap();
let sig = RistrettoSchnorr::new(p.clone(), s);
let sig = CheckSigSchnorrSignature::new(p.clone(), s);
let m: HashValue = Blake2b::<U32>::digest(b"Hello Tari Script").into();
let s: ScalarValue = m;
let commitment = PedersenCommitment::from_public_key(&p);
Expand Down

0 comments on commit dc26ca6

Please sign in to comment.