Merge #298: blind: verify rangeproofs for confidential issuance amounts

173321e blind: verify rangeproofs for confidential issuance amounts (Byron Hambly)
cb8e6f1 test: reproduce unverified confidential issuance amounts (Byron Hambly)

Pull request description:

  `verify_tx_amt_proofs` pushed confidential issuance amount and inflation
  keys commitments into the balance sum without verifying any rangeproof.
  The commitment entered the aggregate sum as an unconstrained curve point,
  making the value-conservation check satisfiable for an arbitrary
  transaction carrying an issuance: a relying verifier could be induced to
  accept a transaction that mints value out of nothing.
  
  This PR verifies the accompanying rangeproof against the unblinded
  issuance generator, mirroring Elements Core's `VerifyIssuanceAmount`:
  the proof message is empty for issuances, and the generator is the
  unblinded generator of the issued asset (resp. reissuance token). A
  confidential issuance amount without a rangeproof is now rejected with
  `VerificationError::RangeProofMissing`.


ACKs for top commit:
  apoelstra:
    ACK 173321e; successfully ran local tests


Tree-SHA512: bde7960a4665032165999b9182a9a54b70579df3bcb15ed41ddc2b4a2380215fb03a72b0a7ac081f17b65df218f7fc75cc6ee1d639fb4e2ae21835e219cb0280

Author: apoelstra

src/blind.rs

@@ -1102,20 +1102,43 @@ impl Transaction {
             if inp.has_issuance() {
                 let (asset_id, token_id) = inp.issuance_ids();
                 let arr = [
-                    (inp.asset_issuance.amount, asset_id),
-                    (inp.asset_issuance.inflation_keys, token_id),
+                    (
+                        inp.asset_issuance.amount,
+                        asset_id,
+                        &inp.witness.amount_rangeproof,
+                    ),
+                    (
+                        inp.asset_issuance.inflation_keys,
+                        token_id,
+                        &inp.witness.inflation_keys_rangeproof,
+                    ),
                 ];
-                for (amt, asset) in &arr {
+                for (amt, asset, rangeproof) in &arr {
+                    // Issuance pseudo-inputs are never asset-blinded: the
+                    // generator is the unblinded generator of the issued
+                    // (or reissuance token) asset.
+                    let gen = Generator::new_unblinded(secp, asset.into_tag());
                     match amt {
                         Value::Null => {},
                         Value::Explicit(v) => {
-                            let gen = Generator::new_unblinded(secp, asset.into_tag());
                             domain.push(gen);
                             let comm = PedersenCommitment::new_unblinded(secp, *v, gen);
                             in_commits.push(comm);
                         }
                         Value::Confidential(comm) => {
-                            let gen = Generator::new_unblinded(secp, asset.into_tag());
+                            // A confidential issuance amount must be accompanied
+                            // by a rangeproof, verified against the unblinded
+                            // issuance generator, mirroring Elements Core's
+                            // `VerifyIssuanceAmount`. Without this check the
+                            // commitment is an unconstrained term in the balance
+                            // equation below. The rangeproof message is empty
+                            // for issuances (Core passes an empty script).
+                            let rangeproof = rangeproof
+                                .as_ref()
+                                .ok_or(VerificationError::RangeProofMissing(i))?;
+                            rangeproof
+                                .verify(secp, *comm, &[], gen)
+                                .map_err(|e| VerificationError::RangeProofError(i, e))?;
                             domain.push(gen);
                             in_commits.push(*comm);
                         }

tests/poc_issuance_inflation.rs

@@ -0,0 +1,135 @@
+//! Regression tests for unverified confidential issuance amount rangeproofs:
+//! `Transaction::verify_tx_amt_proofs` used to push confidential *issuance*
+//! amount commitments into the balance sum without verifying any rangeproof,
+//! leaving an unconstrained term that made the value-conservation check
+//! vacuous for any transaction carrying an issuance.
+//!
+//! - `forged_issuance_amount_is_rejected` reproduces the attack: 1 L-BTC in,
+//!   100 L-BTC out, with the 99 L-BTC gap supplied by a bare curve point equal
+//!   to 99 times the L-BTC generator in `asset_issuance.amount` and no
+//!   rangeproof. Verification must now fail.
+//! - `blinded_issuance_amount_still_verifies` is the positive control: a
+//!   properly blinded issuance amount (rangeproof present, produced by the
+//!   crate's own blinding code) still verifies, so the fix does not break
+//!   legitimate issuances.
+
+use elements::confidential::{Asset, Nonce, Value};
+use elements::{
+    AssetBlindingNonce, AssetEntropy, AssetId, AssetIssuance, LockTime, OutPoint, Script,
+    Transaction, TxIn, TxOut, TxOutWitness, VerificationError,
+};
+use secp256k1_zkp::{Generator, PedersenCommitment, Secp256k1};
+
+fn txout(asset: Asset, value: Value) -> TxOut {
+    TxOut {
+        asset,
+        value,
+        nonce: Nonce::Null,
+        script_pubkey: Script::new(),
+        witness: TxOutWitness::default(),
+    }
+}
+
+#[test]
+fn forged_issuance_amount_is_rejected() {
+    let secp = Secp256k1::new();
+    let asset = AssetId::LIQUID_BTC;
+
+    // Honest spent UTXO: 1 L-BTC, explicit (so its commitment is 1*G_LBTC).
+    let spent_utxo = txout(Asset::Explicit(asset), Value::Explicit(1));
+
+    // Forged "confidential issuance amount": the curve point 99*G_LBTC.
+    // It is not a valid opening of anything meaningful; no rangeproof is
+    // produced for it (TxInWitness::default() has an EMPTY amount_rangeproof).
+    let gen = Generator::new_unblinded(&secp, asset.into_tag());
+    let forged_issuance_commitment = PedersenCommitment::new_unblinded(&secp, 99, gen);
+
+    let input = TxIn {
+        previous_output: OutPoint::default(),
+        asset_issuance: AssetIssuance {
+            asset_blinding_nonce: AssetBlindingNonce::NEW_ISSUANCE,
+            asset_entropy: AssetEntropy::NEW_ISSUANCE,
+            amount: Value::Confidential(forged_issuance_commitment),
+            inflation_keys: Value::Null, // never touched
+        },
+        ..Default::default() // witness.amount_rangeproof is EMPTY
+    };
+
+    // Attacker output: 100 L-BTC, explicit (no rangeproof / surjection proof required).
+    let tx = Transaction {
+        version: 2,
+        lock_time: LockTime::ZERO,
+        input: vec![input],
+        output: vec![txout(Asset::Explicit(asset), Value::Explicit(100))],
+    };
+
+    // Before the fix this returned Ok(()) and the tx minted 99 L-BTC.
+    assert_eq!(
+        tx.verify_tx_amt_proofs(&secp, std::slice::from_ref(&spent_utxo)),
+        Err(VerificationError::RangeProofMissing(0)),
+        "forged confidential issuance amount must not be accepted"
+    );
+
+    // Non-vacuity control: the same tx with the forged issuance removed still
+    // fails the aggregate balance check (1 in, 100 out).
+    let mut control = tx.clone();
+    control.input[0].asset_issuance.amount = Value::Null;
+    assert_eq!(
+        control.verify_tx_amt_proofs(&secp, &[spent_utxo]),
+        Err(VerificationError::BalanceCheckFailed),
+        "control tx without the forged issuance should fail the balance check"
+    );
+}
+
+#[test]
+fn blinded_issuance_amount_still_verifies() {
+    use elements::confidential::ValueBlindingFactor;
+    use rand::thread_rng;
+    use secp256k1_zkp::SecretKey;
+
+    let secp = Secp256k1::new();
+    let asset = AssetId::LIQUID_BTC;
+
+    let spent_utxo = txout(Asset::Explicit(asset), Value::Explicit(1));
+
+    let mut input = TxIn {
+        previous_output: OutPoint::default(),
+        asset_issuance: AssetIssuance {
+            asset_blinding_nonce: AssetBlindingNonce::NEW_ISSUANCE,
+            asset_entropy: AssetEntropy::NEW_ISSUANCE,
+            amount: Value::Explicit(10),
+            inflation_keys: Value::Null,
+        },
+        ..Default::default()
+    };
+
+    // Blind the issuance amount: this turns it into a confidential commitment
+    // and generates the accompanying rangeproof. A zero blinding factor keeps
+    // the balance simple (explicit outputs of 1 L-BTC + 10 of the new asset).
+    let mut rng = thread_rng();
+    input
+        .blind_issuances_with_bfs(
+            &secp,
+            ValueBlindingFactor::zero(),
+            ValueBlindingFactor::zero(),
+            SecretKey::new(&mut rng),
+            SecretKey::new(&mut rng),
+        )
+        .unwrap();
+    assert!(input.asset_issuance.amount.is_confidential());
+
+    let (issued_asset, _token) = input.issuance_ids();
+
+    let tx = Transaction {
+        version: 2,
+        lock_time: LockTime::ZERO,
+        input: vec![input],
+        output: vec![
+            txout(Asset::Explicit(asset), Value::Explicit(1)),
+            txout(Asset::Explicit(issued_asset), Value::Explicit(10)),
+        ],
+    };
+
+    tx.verify_tx_amt_proofs(&secp, &[spent_utxo])
+        .expect("a legitimately blinded issuance amount must still verify");
+}