Disallow duplicate TxOut public keys (#211)

* TxOutStore stores pub key -> index, enforces uniqueness on insert

* add get_tx_out_index_by_public_key

* transaction validation checks for duplicate output pub keys

* add contains_tx_out_public_key + more defensive tests

* consensus service denies duplicate tx pub keys

* add db migration

* linty lint

* more lint

* include previous block id when generating fee tx private key

consensus/api/proto/consensus_common.proto

@@ -56,14 +56,16 @@ enum ProposeTxResult {
     UnsortedKeyImages = 26;
     ContainsSpentKeyImage = 27;
     DuplicateKeyImages = 28;
-    MissingTxOutMembershipProof = 29;
-    InvalidTxOutMembershipProof = 30;
-    InvalidRistrettoPublicKey = 31;
-    InvalidLedgerContext = 32;
-    Ledger = 33;
-    MembershipProofValidationError = 34;
-    TxFeeError = 35;
-    KeyError = 36;
+    DuplicateOutputPublicKey = 29;
+    ContainsExistingOutputPublicKey = 30;
+    MissingTxOutMembershipProof = 31;
+    InvalidTxOutMembershipProof = 32;
+    InvalidRistrettoPublicKey = 33;
+    InvalidLedgerContext = 34;
+    Ledger = 35;
+    MembershipProofValidationError = 36;
+    TxFeeError = 37;
+    KeyError = 38;
 }
 
 /// Response from TxPropose RPC call.

consensus/api/src/conversions.rs

@@ -40,6 +40,10 @@ impl From<TransactionValidationError> for ProposeTxResult {
             TransactionValidationError::UnsortedKeyImages => Self::UnsortedKeyImages,
             TransactionValidationError::ContainsSpentKeyImage => Self::ContainsSpentKeyImage,
             TransactionValidationError::DuplicateKeyImages => Self::DuplicateKeyImages,
+            TransactionValidationError::DuplicateOutputPublicKey => Self::DuplicateOutputPublicKey,
+            TransactionValidationError::ContainsExistingOutputPublicKey => {
+                Self::ContainsExistingOutputPublicKey
+            }
             TransactionValidationError::MissingTxOutMembershipProof => {
                 Self::MissingTxOutMembershipProof
             }
@@ -94,6 +98,12 @@ impl TryInto<TransactionValidationError> for ProposeTxResult {
             Self::UnsortedKeyImages => Ok(TransactionValidationError::UnsortedKeyImages),
             Self::ContainsSpentKeyImage => Ok(TransactionValidationError::ContainsSpentKeyImage),
             Self::DuplicateKeyImages => Ok(TransactionValidationError::DuplicateKeyImages),
+            Self::DuplicateOutputPublicKey => {
+                Ok(TransactionValidationError::DuplicateOutputPublicKey)
+            }
+            Self::ContainsExistingOutputPublicKey => {
+                Ok(TransactionValidationError::ContainsExistingOutputPublicKey)
+            }
             Self::MissingTxOutMembershipProof => {
                 Ok(TransactionValidationError::MissingTxOutMembershipProof)
             }

consensus/enclave/api/src/lib.rs

@@ -19,7 +19,7 @@ use mc_attest_enclave_api::{
     PeerAuthResponse, PeerSession,
 };
 use mc_common::ResponderId;
-use mc_crypto_keys::{Ed25519Public, X25519Public};
+use mc_crypto_keys::{CompressedRistrettoPublic, Ed25519Public, X25519Public};
 use mc_transaction_core::{
     ring_signature::KeyImage,
     tx::{Tx, TxHash, TxOutMembershipProof},
@@ -56,6 +56,9 @@ pub struct WellFormedTxContext {
 
     /// Highest membership proofs indices.
     highest_indices: Vec<u64>,
+
+    /// Output public keys.
+    output_public_keys: Vec<CompressedRistrettoPublic>,
 }
 
 impl WellFormedTxContext {
@@ -78,6 +81,10 @@ impl WellFormedTxContext {
     pub fn highest_indices(&self) -> &Vec<u64> {
         &self.highest_indices
     }
+
+    pub fn output_public_keys(&self) -> &Vec<CompressedRistrettoPublic> {
+        &self.output_public_keys
+    }
 }
 
 impl From<&Tx> for WellFormedTxContext {
@@ -88,6 +95,7 @@ impl From<&Tx> for WellFormedTxContext {
             tombstone_block: tx.prefix.tombstone_block,
             key_images: tx.key_images(),
             highest_indices: tx.get_membership_proof_highest_indices(),
+            output_public_keys: tx.output_public_keys(),
         }
     }
 }
@@ -101,6 +109,7 @@ pub struct TxContext {
     pub tx_hash: TxHash,
     pub highest_indices: Vec<u64>,
     pub key_images: Vec<KeyImage>,
+    pub output_public_keys: Vec<CompressedRistrettoPublic>,
 }
 
 pub type SealedBlockSigningKey = Vec<u8>;

consensus/enclave/impl/src/lib.rs

@@ -49,6 +49,9 @@ use mc_transaction_core::{
 use prost::Message;
 use rand_core::{CryptoRng, RngCore};
 
+/// Domain seperator for unified fees transaction private key.
+pub const FEES_OUTPUT_PRIVATE_KEY_DOMAIN_TAG: &str = "mc_fees_output_private_key";
+
 /// A well-formed transaction.
 #[derive(Clone, Eq, PartialEq, Message)]
 pub struct WellFormedTx {
@@ -231,12 +234,14 @@ impl ConsensusEnclave for SgxConsensusEnclave {
         let tx_hash = tx.tx_hash();
         let highest_indices = tx.get_membership_proof_highest_indices();
         let key_images: Vec<KeyImage> = tx.key_images();
+        let output_public_keys = tx.output_public_keys();
 
         Ok(TxContext {
             locally_encrypted_tx,
             tx_hash,
             highest_indices,
             key_images,
+            output_public_keys,
         })
     }
 
@@ -261,12 +266,14 @@ impl ConsensusEnclave for SgxConsensusEnclave {
                 let tx_hash = tx.tx_hash();
                 let highest_indices = tx.get_membership_proof_highest_indices();
                 let key_images: Vec<KeyImage> = tx.key_images();
+                let output_public_keys = tx.output_public_keys();
 
                 Ok(TxContext {
                     locally_encrypted_tx,
                     tx_hash,
                     highest_indices,
                     key_images,
+                    output_public_keys,
                 })
             })
             .collect()
@@ -422,11 +429,26 @@ impl ConsensusEnclave for SgxConsensusEnclave {
             }
         }
 
+        // Duplicate output public keys are not allowed.
+        let mut seen_output_public_keys = BTreeSet::default();
+        for tx in &transactions {
+            for public_key in tx.output_public_keys() {
+                if seen_output_public_keys.contains(&public_key) {
+                    return Err(Error::FormBlock(format!(
+                        "Duplicate output public key: {:?}",
+                        public_key
+                    )));
+                }
+                seen_output_public_keys.insert(public_key);
+            }
+        }
+
         // Create an aggregate fee output.
         let fee_tx_private_key = {
             let hash_value: [u8; 32] = {
                 let mut hasher = Blake2b256::new();
-                // TODO: domain separator.
+                FEES_OUTPUT_PRIVATE_KEY_DOMAIN_TAG.digest(&mut hasher);
+                parent_block.id.digest(&mut hasher);
                 transactions.digest(&mut hasher);
                 hasher
                     .result()
@@ -889,6 +911,98 @@ mod tests {
         assert_eq!(form_block_result, expected);
     }
 
+    #[test]
+    /// form_block should return an error if the input transactions contain a duplicate output
+    /// public key.
+    fn test_form_block_prevents_duplicate_output_public_key() {
+        let enclave = SgxConsensusEnclave::default();
+        let mut rng = Hc128Rng::from_seed([77u8; 32]);
+
+        // Initialize a ledger. `sender` is the owner of all outputs in the initial ledger.
+        let sender = AccountKey::random(&mut rng);
+        let mut ledger = create_ledger();
+        let n_blocks = 3;
+        initialize_ledger(&mut ledger, n_blocks, &sender, &mut rng);
+
+        // Create a few transactions from `sender` to `recipient`.
+        let num_transactions = 5;
+        let recipient = AccountKey::random(&mut rng);
+
+        // The first block contains RING_SIZE outputs.
+        let block_zero_contents = ledger.get_block_contents(0).unwrap();
+
+        // Re-create the rng so that we could more easily generate a duplicate output public key.
+        let mut rng = Hc128Rng::from_seed([77u8; 32]);
+
+        let mut new_transactions = Vec::new();
+        for i in 0..num_transactions - 1 {
+            let tx_out = &block_zero_contents.outputs[i];
+
+            let tx = create_transaction(
+                &mut ledger,
+                tx_out,
+                &sender,
+                &recipient.default_subaddress(),
+                n_blocks + 1,
+                &mut rng,
+            );
+            new_transactions.push(tx);
+        }
+
+        // Re-creating the rng here would result in a duplicate output public key.
+        {
+            let mut rng = Hc128Rng::from_seed([77u8; 32]);
+            let tx_out = &block_zero_contents.outputs[num_transactions - 1];
+
+            let tx = create_transaction(
+                &mut ledger,
+                tx_out,
+                &sender,
+                &recipient.default_subaddress(),
+                n_blocks + 1,
+                &mut rng,
+            );
+            new_transactions.push(tx);
+
+            assert_eq!(
+                new_transactions[0].prefix.outputs[0].public_key,
+                new_transactions[num_transactions - 1].prefix.outputs[0].public_key,
+            );
+        }
+
+        // Create WellFormedEncryptedTxs + proofs
+        let well_formed_encrypted_txs_with_proofs: Vec<_> = new_transactions
+            .iter()
+            .map(|tx| {
+                let well_formed_tx = WellFormedTx::from(tx.clone());
+                let encrypted_tx = enclave
+                    .encrypt_well_formed_tx(&well_formed_tx, &mut rng)
+                    .unwrap();
+
+                let highest_indices = well_formed_tx.tx.get_membership_proof_highest_indices();
+                let membership_proofs = ledger
+                    .get_tx_out_proof_of_memberships(&highest_indices)
+                    .expect("failed getting proof");
+                (encrypted_tx, membership_proofs)
+            })
+            .collect();
+
+        // Form block
+        let parent_block = ledger.get_block(ledger.num_blocks().unwrap() - 1).unwrap();
+
+        let form_block_result =
+            enclave.form_block(&parent_block, &well_formed_encrypted_txs_with_proofs);
+        let expected_duplicate_output_public_key = new_transactions[0].output_public_keys()[0];
+
+        // Check
+        let expected = Err(Error::FormBlock(format!(
+            "Duplicate output public key: {:?}",
+            expected_duplicate_output_public_key
+        )));
+
+        assert_eq!(form_block_result, expected);
+    }
+
     #[test]
     fn form_block_refuses_duplicate_root_elements() {
         let enclave = SgxConsensusEnclave::default();

consensus/enclave/mock/src/lib.rs

@@ -45,12 +45,14 @@ impl ConsensusServiceMockEnclave {
         let tx_hash = tx.tx_hash();
         let highest_indices = tx.get_membership_proof_highest_indices();
         let key_images: Vec<KeyImage> = tx.key_images();
+        let output_public_keys = tx.output_public_keys();
 
         TxContext {
             locally_encrypted_tx,
             tx_hash,
             highest_indices,
             key_images,
+            output_public_keys,
         }
     }
 }

consensus/service/src/client_api_service.rs

@@ -99,6 +99,7 @@ impl<E: ConsensusEnclaveProxy, L: Ledger + Clone> ClientApiService<E, L> {
                 // These errors are common, so only trace them
                 if err == TransactionValidationError::TombstoneBlockExceeded
                     || err == TransactionValidationError::ContainsSpentKeyImage
+                    || err == TransactionValidationError::ContainsExistingOutputPublicKey
                 {
                     log::trace!(
                         logger,

consensus/service/src/test_utils/mod.rs

@@ -2,6 +2,7 @@
 
 use crate::tx_manager::UntrustedInterfaces;
 use mc_consensus_enclave::WellFormedTxContext;
+use mc_crypto_keys::CompressedRistrettoPublic;
 use mc_transaction_core::{
     ring_signature::KeyImage,
     tx::{TxHash, TxOutMembershipProof},
@@ -17,6 +18,7 @@ impl UntrustedInterfaces for TrivialTxManagerUntrustedInterfaces {
         &self,
         _highest_indices: &[u64],
         _key_images: &[KeyImage],
+        _output_public_keys: &[CompressedRistrettoPublic],
     ) -> TransactionValidationResult<(u64, Vec<TxOutMembershipProof>)> {
         Ok((1, Vec::new()))
     }

consensus/service/src/tx_manager.rs

@@ -13,6 +13,7 @@ use mc_consensus_enclave::{
     ConsensusEnclaveProxy, Error as ConsensusEnclaveError, TxContext, WellFormedEncryptedTx,
     WellFormedTxContext,
 };
+use mc_crypto_keys::CompressedRistrettoPublic;
 use mc_ledger_db::{Error as LedgerDbError, Ledger};
 use mc_transaction_core::{
     constants::MAX_TRANSACTIONS_PER_BLOCK,
@@ -95,6 +96,7 @@ pub trait UntrustedInterfaces: Clone {
         &self,
         highest_indices: &[u64],
         key_images: &[KeyImage],
+        output_public_keys: &[CompressedRistrettoPublic],
     ) -> TransactionValidationResult<(u64, Vec<TxOutMembershipProof>)>;
 
     /// Checks if a transaction is valid (see definition in validators.rs).
@@ -172,9 +174,11 @@ impl<E: ConsensusEnclaveProxy, L: Ledger, UI: UntrustedInterfaces> TxManager<E,
         let timer = counters::WELL_FORMED_CHECK_TIME.start_timer();
 
         // Perform the untrusted part of the well-formed check.
-        let (current_block_index, membership_proofs) = self
-            .untrusted
-            .well_formed_check(&tx_context.highest_indices, &tx_context.key_images)?;
+        let (current_block_index, membership_proofs) = self.untrusted.well_formed_check(
+            &tx_context.highest_indices,
+            &tx_context.key_images,
+            &tx_context.output_public_keys,
+        )?;
 
         // Check if tx is well-formed, and if it is get the encrypted copy and context for us
         // to store.
@@ -305,6 +309,7 @@ impl<E: ConsensusEnclaveProxy, L: Ledger, UI: UntrustedInterfaces> TxManager<E,
                 let (_current_block_index, membership_proofs) = self.untrusted.well_formed_check(
                     entry.context().highest_indices(),
                     entry.context().key_images(),
+                    entry.context().output_public_keys(),
                 )?;
 
                 Ok((entry.encrypted_tx().clone(), membership_proofs))