Merge rust-bitcoin/rust-bitcoin#819: fix: Throw Error on unsuported addresses in `is_signed_by_address()`

79cee4cd31 fix: Error on unsuported addresses in `is_signed_by_address` (Andrew Ahlers)

Pull request description:

  This is a direct response to this comment: https://github.com/rust-bitcoin/rust-bitcoin/pull/684#issuecomment-1012136845

  Currently unsupported addresses simply return `false` which is highly misleading as it is entirely possible that the keys related to a given address were used to sign the message.

  If this is successful I can follow up with
  > a method to check if a PublicKey is associated with an address

ACKs for top commit:
  apoelstra:
    ACK 79cee4cd31
  Kixunil:
    ACK 79cee4cd31

Tree-SHA512: 0c2f15696c63272e8ad1ecc0959c828f2d6c4aa16a7d099235c3f6bd287a0c20e034752331644c4bcc3af61ba4d739ad6795cbcea61c9e615c1eb7b43ebf0eeb
This commit is contained in:
Andrew Poelstra 2022-02-14 20:40:37 +00:00
commit 0c5b695194
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
1 changed files with 25 additions and 16 deletions

View File

@ -55,6 +55,8 @@ mod message_signing {
InvalidEncoding(secp256k1::Error), InvalidEncoding(secp256k1::Error),
/// Invalid base64 encoding. /// Invalid base64 encoding.
InvalidBase64, InvalidBase64,
/// Unsupported Address Type
UnsupportedAddressType(AddressType),
} }
impl fmt::Display for MessageSignatureError { impl fmt::Display for MessageSignatureError {
@ -63,6 +65,7 @@ mod message_signing {
MessageSignatureError::InvalidLength => write!(f, "length not 65 bytes"), MessageSignatureError::InvalidLength => write!(f, "length not 65 bytes"),
MessageSignatureError::InvalidEncoding(ref e) => write!(f, "invalid encoding: {}", e), MessageSignatureError::InvalidEncoding(ref e) => write!(f, "invalid encoding: {}", e),
MessageSignatureError::InvalidBase64 => write!(f, "invalid base64"), MessageSignatureError::InvalidBase64 => write!(f, "invalid base64"),
MessageSignatureError::UnsupportedAddressType(ref address_type) => write!(f, "unsupported address type: {}", address_type),
} }
} }
} }
@ -144,8 +147,9 @@ mod message_signing {
&self, &self,
secp_ctx: &secp256k1::Secp256k1<C>, secp_ctx: &secp256k1::Secp256k1<C>,
msg_hash: sha256d::Hash msg_hash: sha256d::Hash
) -> Result<PublicKey, secp256k1::Error> { ) -> Result<PublicKey, MessageSignatureError> {
let msg = secp256k1::Message::from_slice(&msg_hash[..])?; let msg = secp256k1::Message::from_slice(&msg_hash[..])
.expect("cannot fail");
let pubkey = secp_ctx.recover_ecdsa(&msg, &self.signature)?; let pubkey = secp_ctx.recover_ecdsa(&msg, &self.signature)?;
Ok(PublicKey { Ok(PublicKey {
inner: pubkey, inner: pubkey,
@ -161,18 +165,15 @@ mod message_signing {
secp_ctx: &secp256k1::Secp256k1<C>, secp_ctx: &secp256k1::Secp256k1<C>,
address: &Address, address: &Address,
msg_hash: sha256d::Hash msg_hash: sha256d::Hash
) -> Result<bool, secp256k1::Error> { ) -> Result<bool, MessageSignatureError> {
let pubkey = self.recover_pubkey(secp_ctx, msg_hash)?; match address.address_type() {
Ok(match address.address_type() {
Some(AddressType::P2pkh) => { Some(AddressType::P2pkh) => {
*address == Address::p2pkh(&pubkey, address.network) let pubkey = self.recover_pubkey(secp_ctx, msg_hash)?;
Ok(*address == Address::p2pkh(&pubkey, address.network))
}
Some(address_type) => Err(MessageSignatureError::UnsupportedAddressType(address_type)),
None => Ok(false),
} }
Some(AddressType::P2sh) => false,
Some(AddressType::P2wpkh) => false,
Some(AddressType::P2wsh) => false,
Some(AddressType::P2tr) => false,
None => false,
})
} }
/// Convert a signature from base64 encoding. /// Convert a signature from base64 encoding.
@ -259,6 +260,7 @@ pub fn signed_msg_hash(msg: &str) -> sha256d::Hash {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*;
use hashes::hex::ToHex; use hashes::hex::ToHex;
use super::script_find_and_remove; use super::script_find_and_remove;
use super::signed_msg_hash; use super::signed_msg_hash;
@ -313,11 +315,13 @@ mod tests {
fn test_message_signature() { fn test_message_signature() {
use core::str::FromStr; use core::str::FromStr;
use secp256k1; use secp256k1;
use ::AddressType;
let secp = secp256k1::Secp256k1::new(); let secp = secp256k1::Secp256k1::new();
let message = "rust-bitcoin MessageSignature test"; let message = "rust-bitcoin MessageSignature test";
let msg_hash = super::signed_msg_hash(&message); let msg_hash = super::signed_msg_hash(&message);
let msg = secp256k1::Message::from_slice(&msg_hash).unwrap(); let msg = secp256k1::Message::from_slice(&msg_hash).expect("message");
let privkey = secp256k1::SecretKey::new(&mut secp256k1::rand::thread_rng()); let privkey = secp256k1::SecretKey::new(&mut secp256k1::rand::thread_rng());
let secp_sig = secp.sign_ecdsa_recoverable(&msg, &privkey); let secp_sig = secp.sign_ecdsa_recoverable(&msg, &privkey);
@ -335,9 +339,14 @@ mod tests {
let p2pkh = ::Address::p2pkh(&pubkey, ::Network::Bitcoin); let p2pkh = ::Address::p2pkh(&pubkey, ::Network::Bitcoin);
assert_eq!(signature2.is_signed_by_address(&secp, &p2pkh, msg_hash), Ok(true)); assert_eq!(signature2.is_signed_by_address(&secp, &p2pkh, msg_hash), Ok(true));
let p2wpkh = ::Address::p2wpkh(&pubkey, ::Network::Bitcoin).unwrap(); let p2wpkh = ::Address::p2wpkh(&pubkey, ::Network::Bitcoin).unwrap();
assert_eq!(signature2.is_signed_by_address(&secp, &p2wpkh, msg_hash), Ok(false)); assert_eq!(
signature2.is_signed_by_address(&secp, &p2wpkh, msg_hash),
Err(MessageSignatureError::UnsupportedAddressType(AddressType::P2wpkh))
);
let p2shwpkh = ::Address::p2shwpkh(&pubkey, ::Network::Bitcoin).unwrap(); let p2shwpkh = ::Address::p2shwpkh(&pubkey, ::Network::Bitcoin).unwrap();
assert_eq!(signature2.is_signed_by_address(&secp, &p2shwpkh, msg_hash), Ok(false)); assert_eq!(
signature2.is_signed_by_address(&secp, &p2shwpkh, msg_hash),
Err(MessageSignatureError::UnsupportedAddressType(AddressType::P2sh))
);
} }
} }