givelotus · schancel · Feb 19, 2023
diff --git a/cashweb-payload/src/verify.rs b/cashweb-payload/src/verify.rs
@@ -1,8 +1,8 @@
 //! Module for verifying a [`crate::payload::SignedPayload`] is valid.
 
 use bitcoinsuite_core::{
-    ecc::{Ecc, VerifySignatureError, PUBKEY_LENGTH},
-    Bytes, BytesMut, Hashed, Op, Script, Sha256,
+    ecc::{Ecc, VerifySignatureError},
+    Bytes, Hashed, Op, Script, Sha256,
 };
 use bitcoinsuite_error::{ErrorMeta, Result, WrapErr};
 use thiserror::Error;
@@ -98,12 +98,11 @@ impl<T> SignedPayload<T> {
     /// `OP_RETURN <lokad_id: STMP> <version: 1> <commitment: 32 bytes>`
     pub fn verify(&self, ecc: &impl Ecc, commitment_id: [u8; 4]) -> Result<()> {
         // Verify OP_RETURN commitments
-        let expected_commitment = calc_commitment(self.pubkey, &self.payload_hash);
         for (idx, burn_tx) in self.burn_txs.iter().enumerate() {
             let parsed_commitment = parse_commitment(commitment_id, &burn_tx.burn_output.script)?;
-            if expected_commitment != parsed_commitment {
+            if self.payload_hash != parsed_commitment {
                 return Err(BurnOutputCommitmentMismatch {
-                    expected: expected_commitment,
+                    expected: self.payload_hash.clone(),
                     actual: parsed_commitment,
                     burn_tx_idx: idx,
                 }
@@ -130,12 +129,8 @@ impl<T> SignedPayload<T> {
 }
 
 /// Build the burn [`bitcoinsuite_core::Script`] for the given pubkey and payload hash.
-pub fn build_commitment_script(
-    commitment_id: [u8; 4],
-    pubkey_raw: [u8; PUBKEY_LENGTH],
-    payload_hash: &Sha256,
-) -> Script {
-    let commitment = calc_commitment(pubkey_raw, payload_hash);
+pub fn build_commitment_script(commitment_id: [u8; 4], payload_hash: &Sha256) -> Script {
+    let commitment = payload_hash;
     Script::from_ops(
         vec![
             Op::Code(0x6a),
@@ -183,14 +178,6 @@ fn parse_commitment(commitment_id: [u8; 4], script: &Script) -> Result<Sha256> {
     Ok(Sha256::new(commitment.as_ref().try_into().unwrap()))
 }
 
-fn calc_commitment(pubkey_raw: [u8; PUBKEY_LENGTH], payload_hash: &Sha256) -> Sha256 {
-    let pubkey_hash = Sha256::digest(pubkey_raw.into());
-    let mut commitment_preimage = BytesMut::new();
-    commitment_preimage.put_byte_array(pubkey_hash.byte_array().clone());
-    commitment_preimage.put_byte_array(payload_hash.byte_array().clone());
-    Sha256::digest(commitment_preimage.freeze())
-}
-
 #[cfg(test)]
 mod tests {
     use bitcoinsuite_core::{
@@ -204,7 +191,6 @@ mod tests {
     use crate::{
         payload::{BurnTx, SignatureScheme, SignedPayload},
         verify::{
-            calc_commitment,
             ValidateSignedPayloadError::{self, *},
             ADDRESS_METADATA_LOKAD_ID,
         },
@@ -222,14 +208,7 @@ mod tests {
         let payload_raw = Bytes::from([1, 2, 3, 4]);
         let payload_hash = Sha256::digest(payload_raw.clone());
         let pubkey = [0x77; PUBKEY_LENGTH];
-        let commitment_hash = Sha256::digest(
-            [
-                Sha256::digest(pubkey.as_slice().into()).as_slice(),
-                payload_hash.as_slice(),
-            ]
-            .concat()
-            .into(),
-        );
+        let commitment_hash = payload_hash.clone();
         let commitment_hash_raw = Bytes::from_slice(commitment_hash.as_slice());
         let commitment_script = Script::from_ops(
             vec![
@@ -389,7 +368,7 @@ mod tests {
         // Fix pubkey (and commitment)
         let seckey = ecc.seckey_from_array([0x44; 32])?;
         let pubkey = ecc.derive_pubkey(&seckey).array();
-        let commitment_hash = calc_commitment(pubkey, &payload_hash);
+        let commitment_hash = payload_hash;
         let commitment_hash_raw = Bytes::from_slice(commitment_hash.as_slice());
         let commitment_script = Script::from_ops(
             vec![

diff --git a/cashweb-registry/proto/registry.proto b/cashweb-registry/proto/registry.proto
@@ -16,15 +16,22 @@ message AddressEntry {
 // AddressMetadata is the user-specified data that is covered by the users
 // signature.
 message AddressMetadata {
+    // Repeat pubkey key here so there is a commitment to the key in the signed
+    // payload. We want the overall format for SignedPayloads to be able to be
+    // verified and checked against burns without particular details of this
+    // payload needed. Repeating it here ensures that the "SignedPayload" layer
+    // can be handled independently of payload data, and that this payload data
+    // isn't reusable for other keys.
+    bytes pubkey = 1;
     // Timestamp allows servers to determine which version of the data is the most
     // recent. Given in milliseconds.
-    int64 timestamp = 1;
+    int64 timestamp = 2;
     // TTL tells us how long this entry should exist before being considered
     // invalid. Given in milliseconds.
-    int64 ttl = 2;
+    int64 ttl = 3;
     // User specified data. Presumably some conventional data determined by
     // wallet authors.
-    repeated AddressEntry entries = 3;
+    repeated AddressEntry entries = 4;
 }
 
 // Peer represents a single peer.

diff --git a/cashweb-registry/src/p2p/peer.rs b/cashweb-registry/src/p2p/peer.rs
@@ -559,6 +559,7 @@ mod tests {
                             sig: vec![],
                             sig_scheme: SignatureScheme::Ecdsa as i32,
                             payload: proto::AddressMetadata {
+                                pubkey: vec![0; 33],
                                 timestamp: 1234,
                                 ttl: 10,
                                 entries: vec![],
@@ -694,6 +695,7 @@ mod tests {
                         sig: vec![],
                         sig_scheme: SignatureScheme::Ecdsa as i32,
                         payload: proto::AddressMetadata {
+                            pubkey: vec![0; 33],
                             timestamp: 1234,
                             ttl: 10,
                             entries: vec![],

diff --git a/cashweb-registry/src/registry.rs b/cashweb-registry/src/registry.rs
@@ -95,6 +95,10 @@ pub enum RegistryError {
         /// Actual hash of the provided pubkey.
         actual: PubKeyHash,
     },
+    /// Pubkey in signed payload does not match pubkey in metadata.
+    #[invalid_client_input()]
+    #[error("Pubkey in the address metadata not match signer pubkey")]
+    PubKeyMismatch {},
 
     /// Timestamps in the address payload must increase monotonically.
     /// Otherwise, attackers could re-submit old SignedPayloads and make them go back in time.
@@ -185,8 +189,6 @@ impl Registry {
         let signed_metadata =
             SignedPayload::<proto::AddressMetadata>::parse_proto(signed_metadata)?;
 
-        let new_payload = signed_metadata.payload().as_ref().ok_or(PayloadMissing)?;
-
         // Check pubkey hash
         let actual_pkh = pkh.algorithm().hash_pubkey(*signed_metadata.pubkey());
         if pkh != actual_pkh {
@@ -196,6 +198,10 @@ impl Registry {
             }
             .into());
         }
+        let new_payload = signed_metadata.payload().as_ref().ok_or(PayloadMissing)?;
+        if new_payload.pubkey != signed_metadata.pubkey() {
+            return Err(PubKeyMismatch {}.into());
+        }
 
         // Verify burn amount and signatures check out
         signed_metadata.verify(&self.ecc, ADDRESS_METADATA_LOKAD_ID)?;
@@ -538,6 +544,7 @@ mod tests {
 
         // Tx parses, but the output burn_index points to doesn't exist
         let address_metadata = proto::AddressMetadata {
+            pubkey: pubkey.array().to_vec(),
             timestamp: 1234,
             ttl: 10,
             entries: vec![],
@@ -548,11 +555,7 @@ mod tests {
             inputs: vec![],
             outputs: vec![TxOutput {
                 value: 1_000_000,
-                script: build_commitment_script(
-                    ADDRESS_METADATA_LOKAD_ID,
-                    pubkey.array(),
-                    &payload_hash,
-                ),
+                script: build_commitment_script(ADDRESS_METADATA_LOKAD_ID, &payload_hash),
             }],
             lock_time: 0,
         };
@@ -717,11 +720,7 @@ mod tests {
                     TxBuilderOutput::Leftover(address.script().clone()),
                     TxBuilderOutput::Fixed(TxOutput {
                         value: burn_amount,
-                        script: build_commitment_script(
-                            ADDRESS_METADATA_LOKAD_ID,
-                            pubkey.array(),
-                            &payload_hash,
-                        ),
+                        script: build_commitment_script(ADDRESS_METADATA_LOKAD_ID, &payload_hash),
                     }),
                 ],
                 lock_time: 0,
@@ -734,6 +733,7 @@ mod tests {
         };
 
         let (signed_metadata, _) = build_signed_metadata(proto::AddressMetadata {
+            pubkey: pubkey.array().to_vec(),
             timestamp: 1234,
             ttl: 10,
             entries: vec![proto::AddressEntry {
@@ -757,6 +757,7 @@ mod tests {
 
         // With more recent timestamp, it succeeds.
         let (signed_metadata, tx) = build_signed_metadata(proto::AddressMetadata {
+            pubkey: pubkey.array().to_vec(),
             timestamp: 1235,
             ttl: 10,
             entries: vec![],
@@ -777,6 +778,7 @@ mod tests {
         );
 
         let (signed_metadata, tx) = build_signed_metadata(proto::AddressMetadata {
+            pubkey: pubkey.array().to_vec(),
             timestamp: 1236,
             ttl: 10,
             entries: vec![],
@@ -806,6 +808,7 @@ mod tests {
 
         // Malleate tx, will result in a different txid, but same raw tx hex
         let (mut signed_metadata, tx) = build_signed_metadata(proto::AddressMetadata {
+            pubkey: pubkey.array().to_vec(),
             timestamp: 1237,
             ttl: 10,
             entries: vec![],
@@ -841,6 +844,7 @@ mod tests {
             let mut found_any_race_condition = false;
             for i in 3..95 {
                 let (signed_metadata, tx) = build_signed_metadata(proto::AddressMetadata {
+                    pubkey: pubkey.array().to_vec(),
                     timestamp: 1234 + i,
                     ttl: 10,
                     entries: vec![],
@@ -948,6 +952,7 @@ mod tests {
 
             // Build valid address metadata
             let address_metadata = proto::AddressMetadata {
+                pubkey: pubkey.array().to_vec(),
                 timestamp,
                 ttl: 10,
                 entries: vec![],
@@ -962,11 +967,7 @@ mod tests {
                 &anyone_script,
                 vec![TxOutput {
                     value: burn_amount,
-                    script: build_commitment_script(
-                        ADDRESS_METADATA_LOKAD_ID,
-                        pubkey.array(),
-                        &payload_hash,
-                    ),
+                    script: build_commitment_script(ADDRESS_METADATA_LOKAD_ID, &payload_hash),
                 }],
             );
 
@@ -1091,11 +1092,7 @@ mod tests {
             inputs: vec![],
             outputs: vec![TxOutput {
                 value: 1_000_000,
-                script: build_commitment_script(
-                    BROADCAST_MESSAGE_LOKAD_ID,
-                    pubkey.array(),
-                    &payload_hash,
-                ),
+                script: build_commitment_script(BROADCAST_MESSAGE_LOKAD_ID, &payload_hash),
             }],
             lock_time: 0,
         };
@@ -1236,11 +1233,7 @@ mod tests {
                     TxBuilderOutput::Leftover(address.script().clone()),
                     TxBuilderOutput::Fixed(TxOutput {
                         value: burn_amount,
-                        script: build_commitment_script(
-                            BROADCAST_MESSAGE_LOKAD_ID,
-                            pubkey.array(),
-                            &payload_hash,
-                        ),
+                        script: build_commitment_script(BROADCAST_MESSAGE_LOKAD_ID, &payload_hash),
                     }),
                 ],
                 lock_time: 0,

diff --git a/cashweb-registry/src/store/metadata.rs b/cashweb-registry/src/store/metadata.rs
@@ -183,6 +183,7 @@ mod tests {
         assert_eq!(db.metadata().get_latest()?, None);
 
         let mut address_metadata = proto::AddressMetadata {
+            pubkey: vec![2; 33],
             timestamp: 1234,
             ttl: 10,
             entries: vec![],
@@ -306,6 +307,7 @@ mod tests {
         for i in 0u8..50 {
             let pkh = PubKeyHash::new(PkhAlgorithm::Sha256Ripemd160, [i / 2; 20].into())?;
             let address_metadata = proto::AddressMetadata {
+                pubkey: vec![2; 33],
                 timestamp: 1000 + i as i64,
                 ttl: 10,
                 entries: vec![],

diff --git a/cashweb-registry/src/test_instance.rs b/cashweb-registry/src/test_instance.rs
@@ -118,11 +118,7 @@ pub fn build_signed_metadata(
         redeem_script,
         vec![TxOutput {
             value: burn_amount,
-            script: build_commitment_script(
-                ADDRESS_METADATA_LOKAD_ID,
-                pubkey.array(),
-                &payload_hash,
-            ),
+            script: build_commitment_script(ADDRESS_METADATA_LOKAD_ID, &payload_hash),
         }],
     );
 

diff --git a/cashweb-registry/tests/test_http_endpoint.rs b/cashweb-registry/tests/test_http_endpoint.rs
@@ -132,6 +132,7 @@ async fn test_registry_http() -> Result<()> {
 
     // Build valid request
     let address_metadata = proto::AddressMetadata {
+        pubkey: pubkey.array().to_vec(),
         timestamp: 1234,
         ttl: 10,
         entries: vec![],
@@ -162,11 +163,7 @@ async fn test_registry_http() -> Result<()> {
             TxBuilderOutput::Leftover(address.script().clone()),
             TxBuilderOutput::Fixed(TxOutput {
                 value: burn_amount,
-                script: build_commitment_script(
-                    ADDRESS_METADATA_LOKAD_ID,
-                    pubkey.array(),
-                    &payload_hash,
-                ),
+                script: build_commitment_script(ADDRESS_METADATA_LOKAD_ID, &payload_hash),
             }),
         ],
         lock_time: 0,
@@ -642,11 +639,7 @@ fn build_signed_message<T: prost::Message + Default>(
             TxBuilderOutput::Leftover(address.script().clone()),
             TxBuilderOutput::Fixed(TxOutput {
                 value: burn_amount,
-                script: build_commitment_script(
-                    BROADCAST_MESSAGE_LOKAD_ID,
-                    pubkey.array(),
-                    &payload_hash,
-                ),
+                script: build_commitment_script(BROADCAST_MESSAGE_LOKAD_ID, &payload_hash),
             }),
         ],
         lock_time: 0,

diff --git a/cashweb-registry/tests/test_imd.rs b/cashweb-registry/tests/test_imd.rs
@@ -95,6 +95,7 @@ async fn test_imd() -> Result<()> {
         for (instance_idx, instance) in instances.iter().enumerate().take(num_instances - 1) {
             // Build valid address metadata
             let address_metadata = proto::AddressMetadata {
+                pubkey: pubkey.array().to_vec(),
                 timestamp: 1000 + instance_idx as i64,
                 ttl: 10,
                 entries: vec![],

diff --git a/cashweb-registry/tests/test_p2p.rs b/cashweb-registry/tests/test_p2p.rs
@@ -102,6 +102,7 @@ async fn test_p2p() -> Result<()> {
         &mut utxos,
         &anyone_script,
         proto::AddressMetadata {
+            pubkey: pubkey.array().to_vec(),
             timestamp: 1234,
             ttl: 10,
             entries: vec![],