Expanded validate_header to check for signature over all sections, an…

…d renamed it to validate_tx.
anoma · Jun 27, 2023 · 2519226 · 2519226
1 parent 1819d16
commit 2519226
Show file tree

Hide file tree

Showing 19 changed files with 71 additions and 60 deletions.
diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs
@@ -157,7 +157,7 @@ where
                 continue;
             }
 
-            let tx = if let Ok(()) = tx.validate_header() {
+            let tx = if tx.validate_tx().is_ok() {
                 tx
             } else {
                 tracing::error!(

diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs
@@ -772,8 +772,8 @@ where
         }
 
         // Tx signature check
-        let tx_type = match tx.validate_header() {
-            Ok(()) => tx.header(),
+        let tx_type = match tx.validate_tx() {
+            Ok(_) => tx.header(),
             Err(msg) => {
                 response.code = ErrorCodes::InvalidSig.into();
                 response.log = msg.to_string();

diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs
@@ -138,7 +138,7 @@ where
                     if let (Some(block_time), Some(exp)) = (block_time.as_ref(), &tx.header.expiration) {
                         if block_time > exp { return None }
                     }
-                    if tx.validate_header().is_ok() && tx.header().wrapper().is_some() && self.replay_protection_checks(&tx, tx_bytes.as_slice(), &mut temp_wl_storage).is_ok() {
+                    if tx.validate_tx().is_ok() && tx.header().wrapper().is_some() && self.replay_protection_checks(&tx, tx_bytes.as_slice(), &mut temp_wl_storage).is_ok() {
                         return Some(tx_bytes.clone());
                     }
                 }

diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs
@@ -231,7 +231,7 @@ where
             |tx| {
                 let tx_chain_id = tx.header.chain_id.clone();
                 let tx_expiration = tx.header.expiration;
-                if let Err(err) = tx.validate_header() {
+                if let Err(err) = tx.validate_tx() {
                     // This occurs if the wrapper / protocol tx signature is
                     // invalid
                     return Err(TxResult {
@@ -250,7 +250,7 @@ where
         // TODO: This should not be hardcoded
         let privkey = <EllipticCurve as PairingEngine>::G2Affine::prime_subgroup_generator();
 
-        if let Err(err) = tx.validate_header() {
+        if let Err(err) = tx.validate_tx() {
             return TxResult {
                 code: ErrorCodes::InvalidSig.into(),
                 info: err.to_string(),

diff --git a/core/src/proto/types.rs b/core/src/proto/types.rs
@@ -830,6 +830,15 @@ impl Tx {
         Section::Header(self.header.clone()).get_hash()
     }
 
+    /// Get hashes of all the sections in this transaction
+    pub fn sechashes(&self) -> Vec<crate::types::hash::Hash> {
+        let mut hashes = vec![self.header_hash()];
+        for sec in &self.sections {
+            hashes.push(sec.get_hash());
+        }
+        hashes
+    }
+
     /// Update the header whilst maintaining existing cross-references
     pub fn update_header(&mut self, tx_type: TxType) -> &mut Self {
         self.header.tx_type = tx_type;
@@ -936,13 +945,15 @@ impl Tx {
         &self,
         pk: &common::PublicKey,
         hashes: &[crate::types::hash::Hash],
-    ) -> std::result::Result<(), VerifySigError> {
+    ) -> std::result::Result<&Signature, VerifySigError> {
         for section in &self.sections {
             if let Section::Signature(sig_sec) = section {
                 // Check that the signer is matched and that the hashes being
                 // checked are a subset of those in this section
                 if sig_sec.pub_key == *pk
-                    && hashes.iter().all(|x| sig_sec.targets.contains(x))
+                    && hashes.iter().all(|x| {
+                        sig_sec.targets.contains(x) || section.get_hash() == *x
+                    })
                 {
                     // Ensure that all the sections the signature signs over are
                     // present
@@ -952,7 +963,7 @@ impl Tx {
                         }
                     }
                     // Finally verify that the signature itself is valid
-                    return sig_sec.verify_signature();
+                    return sig_sec.verify_signature().map(|_| sig_sec);
                 }
             }
         }
@@ -1033,35 +1044,35 @@ impl Tx {
     /// the Tx and verify it is of the appropriate form. This means
     /// 1. The wrapper tx is indeed signed
     /// 2. The signature is valid
-    pub fn validate_header(&self) -> std::result::Result<(), TxError> {
+    pub fn validate_tx(
+        &self,
+    ) -> std::result::Result<Option<&Signature>, TxError> {
         match &self.header.tx_type {
             // verify signature and extract signed data
-            TxType::Wrapper(wrapper) => {
-                self.verify_signature(&wrapper.pk, &[self.header_hash()])
-                    .map_err(|err| {
-                        TxError::SigError(format!(
-                            "WrapperTx signature verification failed: {}",
-                            err
-                        ))
-                    })?;
-                Ok(())
-            }
+            TxType::Wrapper(wrapper) => self
+                .verify_signature(&wrapper.pk, &self.sechashes())
+                .map(Option::Some)
+                .map_err(|err| {
+                    TxError::SigError(format!(
+                        "WrapperTx signature verification failed: {}",
+                        err
+                    ))
+                }),
             // verify signature and extract signed data
             #[cfg(feature = "ferveo-tpke")]
-            TxType::Protocol(protocol) => {
-                self.verify_signature(&protocol.pk, &[self.header_hash()])
-                    .map_err(|err| {
-                        TxError::SigError(format!(
-                            "ProtocolTx signature verification failed: {}",
-                            err
-                        ))
-                    })?;
-                Ok(())
-            }
+            TxType::Protocol(protocol) => self
+                .verify_signature(&protocol.pk, &self.sechashes())
+                .map(Option::Some)
+                .map_err(|err| {
+                    TxError::SigError(format!(
+                        "ProtocolTx signature verification failed: {}",
+                        err
+                    ))
+                }),
             // we extract the signed data, but don't check the signature
-            TxType::Decrypted(_) => Ok(()),
+            TxType::Decrypted(_) => Ok(None),
             // return as is
-            TxType::Raw => Ok(()),
+            TxType::Raw => Ok(None),
         }
     }
 

diff --git a/core/src/types/transaction/mod.rs b/core/src/types/transaction/mod.rs
@@ -254,7 +254,7 @@ mod test_process_tx {
         let code_sec = outer_tx
             .set_code(Code::new("wasm code".as_bytes().to_owned()))
             .clone();
-        outer_tx.validate_header().expect("Test failed");
+        outer_tx.validate_tx().expect("Test failed");
         match outer_tx.header().tx_type {
             TxType::Raw => {
                 assert_eq!(code_sec.get_hash(), outer_tx.header.code_hash,)
@@ -276,7 +276,7 @@ mod test_process_tx {
             .set_data(Data::new("transaction data".as_bytes().to_owned()))
             .clone();
 
-        tx.validate_header().expect("Test failed");
+        tx.validate_tx().expect("Test failed");
         match tx.header().tx_type {
             TxType::Raw => {
                 assert_eq!(code_sec.get_hash(), tx.header().code_hash,);
@@ -302,7 +302,7 @@ mod test_process_tx {
             &gen_keypair(),
         )));
 
-        tx.validate_header().expect("Test failed");
+        tx.validate_tx().expect("Test failed");
         match tx.header().tx_type {
             TxType::Raw => {
                 assert_eq!(code_sec.get_hash(), tx.header().code_hash,);
@@ -337,7 +337,7 @@ mod test_process_tx {
             &keypair,
         )));
 
-        tx.validate_header().expect("Test failed");
+        tx.validate_tx().expect("Test failed");
         match tx.header().tx_type {
             TxType::Wrapper(_) => {
                 tx.decrypt(<EllipticCurve as PairingEngine>::G2Affine::prime_subgroup_generator())
@@ -368,7 +368,7 @@ mod test_process_tx {
         tx.set_code(Code::new("wasm code".as_bytes().to_owned()));
         tx.set_data(Data::new("transaction data".as_bytes().to_owned()));
         tx.encrypt(&Default::default());
-        let result = tx.validate_header().expect_err("Test failed");
+        let result = tx.validate_tx().expect_err("Test failed");
         assert_matches!(result, TxError::SigError(_));
     }
 }
@@ -388,7 +388,7 @@ fn test_process_tx_decrypted_unsigned() {
     let data_sec = tx
         .set_data(Data::new("transaction data".as_bytes().to_owned()))
         .clone();
-    tx.validate_header().expect("Test failed");
+    tx.validate_tx().expect("Test failed");
     match tx.header().tx_type {
         TxType::Decrypted(DecryptedTx::Decrypted {
             #[cfg(not(feature = "mainnet"))]
@@ -434,7 +434,7 @@ fn test_process_tx_decrypted_signed() {
     let data_sec = decrypted
         .set_data(Data::new("transaction data".as_bytes().to_owned()))
         .clone();
-    decrypted.validate_header().expect("Test failed");
+    decrypted.validate_tx().expect("Test failed");
     match decrypted.header().tx_type {
         TxType::Decrypted(DecryptedTx::Decrypted {
             #[cfg(not(feature = "mainnet"))]

diff --git a/core/src/types/transaction/wrapper.rs b/core/src/types/transaction/wrapper.rs
@@ -402,10 +402,10 @@ pub mod wrapper_tx {
             assert_eq!(tx.data(), Some(malicious));
 
             // check that the signature is not valid
-            tx.verify_signature(&keypair.ref_to(), &[tx.header_hash()])
+            tx.verify_signature(&keypair.ref_to(), &tx.sechashes())
                 .expect_err("Test failed");
             // check that the try from method also fails
-            let err = tx.validate_header().expect_err("Test failed");
+            let err = tx.validate_tx().expect_err("Test failed");
             assert_matches!(err, TxError::SigError(_));
         }
     }

diff --git a/wasm/checksums.json b/wasm/checksums.json
@@ -1,21 +1,21 @@
 {
-    "tx_bond.wasm": "tx_bond.fcdfb66dd147489afa965cbaa7f1587ce74643a389d57297bee8612e27159262.wasm",
-    "tx_change_validator_commission.wasm": "tx_change_validator_commission.a39449d960a83ea791ceabde672c2505994741b6b8f7441d7f27bf6b75d76131.wasm",
-    "tx_ibc.wasm": "tx_ibc.727890fca8a1923daf2792dd7d129fde7134c4a05d1ff60fc7c0ae30953787f3.wasm",
-    "tx_init_account.wasm": "tx_init_account.752be422f3b4e5e2f726ba7dec08ef7cc11bae4fecaef1689f2c2359780fea93.wasm",
-    "tx_init_proposal.wasm": "tx_init_proposal.4bdbd6b6cc43e5eae225c48d97d5ff4f0c8f751d92b63c96720313d4b8b457cb.wasm",
-    "tx_init_validator.wasm": "tx_init_validator.21498818585b3c114e517c49acbac2931864028e423901541b4de235396dc78c.wasm",
-    "tx_reveal_pk.wasm": "tx_reveal_pk.5abb1ebed37b076aa81f45e31e32e530ba190a89a7855190631e33314f77ea3b.wasm",
-    "tx_transfer.wasm": "tx_transfer.bacb0e7a0b37af245b24215be6929cbb2849e622797740a99decfff678a8e231.wasm",
-    "tx_unbond.wasm": "tx_unbond.0a213b61c889807b4009d0482ff3e5063ef4b72cfd894892a6f895f96784c03a.wasm",
-    "tx_unjail_validator.wasm": "tx_unjail_validator.433560738de9930a75c5a0bcc60f04620a48083326ef8995fbe0985af7cf3685.wasm",
-    "tx_update_vp.wasm": "tx_update_vp.9864f624a4826c2624cdc1480e5f8553ba6dd3fcc689d811245424fb0ea2526e.wasm",
-    "tx_vote_proposal.wasm": "tx_vote_proposal.c26256aff0eebfa22fbcdb6170dd584bf792f87208afb366d45819eb1a95dad9.wasm",
-    "tx_withdraw.wasm": "tx_withdraw.ea6e73be12bb5f0c31c4ea1c6d98a3e990936e8353d6398989ceb9827f66d2d9.wasm",
-    "vp_implicit.wasm": "vp_implicit.f58b5f3318e71f440ef43c05fb22ce89a46b15cc92f7b271dd7a3770646bb127.wasm",
-    "vp_masp.wasm": "vp_masp.80fd974a291f7ab5d72ae01a7138b93d2f34eae28d84ba8a8ad87c6d7a3503c8.wasm",
-    "vp_testnet_faucet.wasm": "vp_testnet_faucet.8f62368170fdc70bf701d5881663dd13d944e173f15bbbf10b3e7bc5ffd9211a.wasm",
-    "vp_token.wasm": "vp_token.a7fbed95c5499ba03f6eaa2603e1bf307daa8aa977f7a3da22ddb4fd6a93ff2b.wasm",
-    "vp_user.wasm": "vp_user.3a6b3acab132e515ece36764556f30357b488e3c5bc7bea812f0ee1d8b5b947e.wasm",
-    "vp_validator.wasm": "vp_validator.dc6efc0a3e8f9688b35b8f980b161e6dee49b803edaa17b85ba828964670f68f.wasm"
+    "tx_bond.wasm": "tx_bond.4bf2754fa3ef7b9d805199a16f9c18edbba99e71237ed918d30328f4dccc2ac8.wasm",
+    "tx_change_validator_commission.wasm": "tx_change_validator_commission.7bcdbf8dbd5b3fccfedea937979810a1a608b46eb869e95c2206d78fe6fc1785.wasm",
+    "tx_ibc.wasm": "tx_ibc.3f237f4e8a912010b6a033163a9802a153bd526e96c10b4f3973deb153f3dd6d.wasm",
+    "tx_init_account.wasm": "tx_init_account.70af44b96936f6813666cb3cc0b8e94429813dce7cc74f54cedb4be1134139f4.wasm",
+    "tx_init_proposal.wasm": "tx_init_proposal.29141cfd8deabb4767eae037bfcec56974d235ed8f3eb2e3e3ec2b8269a043dc.wasm",
+    "tx_init_validator.wasm": "tx_init_validator.6b0ba72807e055488fa87876f457740b6f97b9245147c1266c3a65afa838574f.wasm",
+    "tx_reveal_pk.wasm": "tx_reveal_pk.20783039d8d9b41278244346fba3e194f5dcbf213244756f1cced243c86fbd0d.wasm",
+    "tx_transfer.wasm": "tx_transfer.74b99599e8516dcd2c55b8aa658d4fada31c8dcc1ef764a4660c104a21bbdd42.wasm",
+    "tx_unbond.wasm": "tx_unbond.03e0eb4788446882b3468762952b35ea8654e207015b8f3d2ffa0fcf2eb0c661.wasm",
+    "tx_unjail_validator.wasm": "tx_unjail_validator.98d9d5ad88f90eeaf8db56663dc6daa94d70eb6c8e78aeb3ff06997e1f684d64.wasm",
+    "tx_update_vp.wasm": "tx_update_vp.c3554d7f5dd7a525f47061263827431531671dffe6d39c8098ee56dfbc793ea0.wasm",
+    "tx_vote_proposal.wasm": "tx_vote_proposal.2e808e951e01f5fcea322a08b220f966a2ae35bf1128d59aa331ec8d91b97557.wasm",
+    "tx_withdraw.wasm": "tx_withdraw.230576e58374ccfd9270a670f922ef14abe88e01824df43e74afc37c6b63580e.wasm",
+    "vp_implicit.wasm": "vp_implicit.64cc57b9b53630ca8737bd5cebb525f1228bd9cf8acbd43fdb3ecbc7db2c3dde.wasm",
+    "vp_masp.wasm": "vp_masp.b2d284458f2a4fbff61d5294306421904095ee56d39c1d009c8d64502d42cb2b.wasm",
+    "vp_testnet_faucet.wasm": "vp_testnet_faucet.06dd70f6e7742eefc8834deb5fca10490afecd19d8b97f8d0f1c1be3ab222f33.wasm",
+    "vp_token.wasm": "vp_token.aff04ac024e7ecb19900fee92be2463a6cd7ddc81dac19dc64bc0b50d0c9752e.wasm",
+    "vp_user.wasm": "vp_user.7b6e898b83c38df822453c060506ff61a814f7b41236ff7775d741e9defa7f06.wasm",
+    "vp_validator.wasm": "vp_validator.ae5a2adefca0927dff90ea9f279ab626f0eb999aa164348a25c7837e1949ad61.wasm"
 }
diff --git a/wasm_for_tests/tx_memory_limit.wasm b/wasm_for_tests/tx_memory_limit.wasm
diff --git a/wasm_for_tests/tx_mint_tokens.wasm b/wasm_for_tests/tx_mint_tokens.wasm
diff --git a/wasm_for_tests/tx_no_op.wasm b/wasm_for_tests/tx_no_op.wasm
diff --git a/wasm_for_tests/tx_proposal_code.wasm b/wasm_for_tests/tx_proposal_code.wasm
diff --git a/wasm_for_tests/tx_read_storage_key.wasm b/wasm_for_tests/tx_read_storage_key.wasm
diff --git a/wasm_for_tests/tx_write.wasm b/wasm_for_tests/tx_write.wasm
diff --git a/wasm_for_tests/vp_always_false.wasm b/wasm_for_tests/vp_always_false.wasm
diff --git a/wasm_for_tests/vp_always_true.wasm b/wasm_for_tests/vp_always_true.wasm
diff --git a/wasm_for_tests/vp_eval.wasm b/wasm_for_tests/vp_eval.wasm
diff --git a/wasm_for_tests/vp_memory_limit.wasm b/wasm_for_tests/vp_memory_limit.wasm
diff --git a/wasm_for_tests/vp_read_storage_key.wasm b/wasm_for_tests/vp_read_storage_key.wasm