diff --git a/cashweb-payload/src/verify.rs b/cashweb-payload/src/verify.rs index 4265837..672f5bf 100644 --- 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 SignedPayload { /// `OP_RETURN ` 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 SignedPayload { } /// 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 { 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 index 69b430e..71e9126 100644 --- 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 index 2ebb0c4..40df71c 100644 --- 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 index ee56e0d..589ef35 100644 --- 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::::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 index d9b7e68..60e9ece 100644 --- 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 index 11ca9f5..9999840 100644 --- 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 index 7ecc4c0..5a4762c 100644 --- 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( 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 index eca9fa7..6916d46 100644 --- 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 index 11685bc..1ca9894 100644 --- 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![],