From 8dfda80a6aa2d212ebe0c60ff0e562890cdc91db Mon Sep 17 00:00:00 2001 From: Yuval Kogman Date: Sat, 23 Nov 2024 20:09:16 +0100 Subject: [PATCH] Encode fragment parameters as bech32 These changes are to facilitate more efficient QR encoding, by exploiting the alphanumeric encoding mode. Use `+` instead of `&` as a parameter delimiter, as only the former is in the QR alphanumeric character set. Instead of encoding keys and values using `=` as a delimiter (which also not in alphanumeric set), the key is the human readable part of a bech32 string with no checksum. This effectively makes the `1` a key/value delimiter. The expiry time is serialized as a bitcoin consensus u32 value (little endian), and also encoded as a bech32 string instead of a decimal value. --- payjoin/src/ohttp.rs | 47 +++++++++++----- payjoin/src/uri/error.rs | 9 ++- payjoin/src/uri/url_ext.rs | 104 ++++++++++++++++++++--------------- payjoin/tests/integration.rs | 2 +- 4 files changed, 99 insertions(+), 63 deletions(-) diff --git a/payjoin/src/ohttp.rs b/payjoin/src/ohttp.rs index b28ad45b..13e6c422 100644 --- a/payjoin/src/ohttp.rs +++ b/payjoin/src/ohttp.rs @@ -1,8 +1,7 @@ use std::ops::{Deref, DerefMut}; use std::{error, fmt}; -use bitcoin::base64::prelude::BASE64_URL_SAFE_NO_PAD; -use bitcoin::base64::Engine; +use bech32::{self, EncodeError}; use bitcoin::key::constants::UNCOMPRESSED_PUBLIC_KEY_SIZE; pub const ENCAPSULATED_MESSAGE_BYTES: usize = 8192; @@ -145,19 +144,19 @@ impl fmt::Display for OhttpKeys { let mut buf = vec![key_id]; buf.extend_from_slice(&compressed_pubkey); - let encoded = BASE64_URL_SAFE_NO_PAD.encode(buf); - write!(f, "{}", encoded) + let oh_hrp: bech32::Hrp = bech32::Hrp::parse("OH").unwrap(); + + crate::bech32::nochecksum::encode_to_fmt(f, oh_hrp, &buf).map_err(|e| match e { + EncodeError::Fmt(e) => e, + _ => fmt::Error, + }) } } -impl std::str::FromStr for OhttpKeys { - type Err = ParseOhttpKeysError; - - /// Parses a base64URL-encoded string into OhttpKeys. - /// The string format is: key_id || compressed_public_key - fn from_str(s: &str) -> Result { - let bytes = BASE64_URL_SAFE_NO_PAD.decode(s).map_err(ParseOhttpKeysError::DecodeBase64)?; +impl TryFrom<&[u8]> for OhttpKeys { + type Error = ParseOhttpKeysError; + fn try_from(bytes: &[u8]) -> Result { let key_id = *bytes.first().ok_or(ParseOhttpKeysError::InvalidFormat)?; let compressed_pk = bytes.get(1..34).ok_or(ParseOhttpKeysError::InvalidFormat)?; @@ -174,6 +173,26 @@ impl std::str::FromStr for OhttpKeys { } } +impl std::str::FromStr for OhttpKeys { + type Err = ParseOhttpKeysError; + + /// Parses a base64URL-encoded string into OhttpKeys. + /// The string format is: key_id || compressed_public_key + fn from_str(s: &str) -> Result { + // TODO extract to utility function + let oh_hrp: bech32::Hrp = bech32::Hrp::parse("OH").unwrap(); + + let (hrp, bytes) = + crate::bech32::nochecksum::decode(s).map_err(ParseOhttpKeysError::DecodeBech32)?; + + if hrp != oh_hrp { + return Err(ParseOhttpKeysError::InvalidFormat); + } + + Self::try_from(&bytes[..]) + } +} + impl PartialEq for OhttpKeys { fn eq(&self, other: &Self) -> bool { match (self.encode(), other.encode()) { @@ -220,7 +239,7 @@ impl serde::Serialize for OhttpKeys { pub enum ParseOhttpKeysError { InvalidFormat, InvalidPublicKey, - DecodeBase64(bitcoin::base64::DecodeError), + DecodeBech32(bech32::primitives::decode::CheckedHrpstringError), DecodeKeyConfig(ohttp::Error), } @@ -229,7 +248,7 @@ impl std::fmt::Display for ParseOhttpKeysError { match self { ParseOhttpKeysError::InvalidFormat => write!(f, "Invalid format"), ParseOhttpKeysError::InvalidPublicKey => write!(f, "Invalid public key"), - ParseOhttpKeysError::DecodeBase64(e) => write!(f, "Failed to decode base64: {}", e), + ParseOhttpKeysError::DecodeBech32(e) => write!(f, "Failed to decode base64: {}", e), ParseOhttpKeysError::DecodeKeyConfig(e) => write!(f, "Failed to decode KeyConfig: {}", e), } @@ -239,7 +258,7 @@ impl std::fmt::Display for ParseOhttpKeysError { impl std::error::Error for ParseOhttpKeysError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { - ParseOhttpKeysError::DecodeBase64(e) => Some(e), + ParseOhttpKeysError::DecodeBech32(e) => Some(e), ParseOhttpKeysError::DecodeKeyConfig(e) => Some(e), ParseOhttpKeysError::InvalidFormat | ParseOhttpKeysError::InvalidPublicKey => None, } diff --git a/payjoin/src/uri/error.rs b/payjoin/src/uri/error.rs index 7bd94281..9f2195cd 100644 --- a/payjoin/src/uri/error.rs +++ b/payjoin/src/uri/error.rs @@ -15,7 +15,8 @@ pub(crate) enum InternalPjParseError { #[derive(Debug)] pub(crate) enum ParseReceiverPubkeyError { MissingPubkey, - PubkeyNotBase64(bitcoin::base64::DecodeError), + InvalidHrp(bech32::Hrp), + DecodeBech32(bech32::primitives::decode::CheckedHrpstringError), InvalidPubkey(crate::hpke::HpkeError), } @@ -26,7 +27,8 @@ impl std::fmt::Display for ParseReceiverPubkeyError { match &self { MissingPubkey => write!(f, "receiver public key is missing"), - PubkeyNotBase64(e) => write!(f, "receiver public is not valid base64: {}", e), + InvalidHrp(h) => write!(f, "incorrect hrp for receiver key: {}", h), + DecodeBech32(e) => write!(f, "receiver public is not valid base64: {}", e), InvalidPubkey(e) => write!(f, "receiver public key does not represent a valid pubkey: {}", e), } @@ -40,7 +42,8 @@ impl std::error::Error for ParseReceiverPubkeyError { match &self { MissingPubkey => None, - PubkeyNotBase64(error) => Some(error), + InvalidHrp(_) => None, + DecodeBech32(error) => Some(error), InvalidPubkey(error) => Some(error), } } diff --git a/payjoin/src/uri/url_ext.rs b/payjoin/src/uri/url_ext.rs index 824be1a2..3f1cebfa 100644 --- a/payjoin/src/uri/url_ext.rs +++ b/payjoin/src/uri/url_ext.rs @@ -1,7 +1,7 @@ use std::str::FromStr; -use bitcoin::base64::prelude::BASE64_URL_SAFE_NO_PAD; -use bitcoin::base64::Engine; +use bitcoin::consensus::encode::Decodable; +use bitcoin::consensus::Encodable; use url::Url; use super::error::ParseReceiverPubkeyError; @@ -21,55 +21,80 @@ pub(crate) trait UrlExt { impl UrlExt for Url { /// Retrieve the receiver's public key from the URL fragment fn receiver_pubkey(&self) -> Result { - let value = get_param(self, "rk=", |v| Some(v.to_owned())) + let value = get_param(self, "RK1", |v| Some(v.to_owned())) .ok_or(ParseReceiverPubkeyError::MissingPubkey)?; - let decoded = BASE64_URL_SAFE_NO_PAD - .decode(&value) - .map_err(ParseReceiverPubkeyError::PubkeyNotBase64)?; + let (hrp, bytes) = crate::bech32::nochecksum::decode(&value) + .map_err(ParseReceiverPubkeyError::DecodeBech32)?; - HpkePublicKey::from_compressed_bytes(&decoded) + let rk_hrp: bech32::Hrp = bech32::Hrp::parse("RK").unwrap(); + if hrp != rk_hrp { + return Err(ParseReceiverPubkeyError::InvalidHrp(hrp)); + } + + HpkePublicKey::from_compressed_bytes(&bytes[..]) .map_err(ParseReceiverPubkeyError::InvalidPubkey) } /// Set the receiver's public key in the URL fragment fn set_receiver_pubkey(&mut self, pubkey: Option) { + let rk_hrp: bech32::Hrp = bech32::Hrp::parse("RK").unwrap(); + set_param( self, - "rk=", - pubkey.map(|k| BASE64_URL_SAFE_NO_PAD.encode(k.to_compressed_bytes())), + "RK1", + pubkey.map(|k| { + crate::bech32::nochecksum::encode(rk_hrp, &k.to_compressed_bytes()) + .expect("encoding compressed pubkey bytes should never fail") + }), ) } /// Retrieve the ohttp parameter from the URL fragment fn ohttp(&self) -> Option { - get_param(self, "ohttp=", |value| OhttpKeys::from_str(value).ok()) + get_param(self, "OH1", |value| OhttpKeys::from_str(value).ok()) } /// Set the ohttp parameter in the URL fragment fn set_ohttp(&mut self, ohttp: Option) { - set_param(self, "ohttp=", ohttp.map(|o| o.to_string())) + set_param(self, "OH1", ohttp.map(|o| o.to_string())) } /// Retrieve the exp parameter from the URL fragment fn exp(&self) -> Option { - get_param(self, "exp=", |value| { - value - .parse::() + get_param(self, "EX1", |value| { + let (hrp, bytes) = crate::bech32::nochecksum::decode(value).ok()?; + + let ex_hrp: bech32::Hrp = bech32::Hrp::parse("EX").unwrap(); + if hrp != ex_hrp { + return None; + } + + let mut cursor = &bytes[..]; + u32::consensus_decode(&mut cursor) + .map(|timestamp| { + std::time::UNIX_EPOCH + std::time::Duration::from_secs(timestamp as u64) + }) .ok() - .map(|timestamp| std::time::UNIX_EPOCH + std::time::Duration::from_secs(timestamp)) }) } /// Set the exp parameter in the URL fragment fn set_exp(&mut self, exp: Option) { let exp_str = exp.map(|e| { - match e.duration_since(std::time::UNIX_EPOCH) { - Ok(duration) => duration.as_secs().to_string(), - Err(_) => "0".to_string(), // Handle times before Unix epoch by setting to "0" - } + let t = match e.duration_since(std::time::UNIX_EPOCH) { + Ok(duration) => duration.as_secs().try_into().unwrap(), // TODO Result type instead of Option & unwrap + Err(_) => 0u32, + }; + + let mut buf = [0u8; 4]; + t.consensus_encode(&mut &mut buf[..]).unwrap(); // TODO no unwrap + + let ex_hrp: bech32::Hrp = bech32::Hrp::parse("EX").unwrap(); + crate::bech32::nochecksum::encode(ex_hrp, &buf) + .expect("encoding u32 timestamp should never fail") }); - set_param(self, "exp=", exp_str) + set_param(self, "EX1", exp_str) } } @@ -78,32 +103,31 @@ where F: Fn(&str) -> Option, { if let Some(fragment) = url.fragment() { - for param in fragment.split('&') { - if let Some(value) = param.strip_prefix(prefix) { - return parse(value); + for param in fragment.split('+') { + if param.starts_with(prefix) { + return parse(param); } } } None } -fn set_param(url: &mut Url, prefix: &str, value: Option) { +fn set_param(url: &mut Url, prefix: &str, param: Option) { let fragment = url.fragment().unwrap_or(""); let mut fragment = fragment.to_string(); if let Some(start) = fragment.find(prefix) { - let end = fragment[start..].find('&').map_or(fragment.len(), |i| start + i); + let end = fragment[start..].find('+').map_or(fragment.len(), |i| start + i); fragment.replace_range(start..end, ""); - if fragment.ends_with('&') { + if fragment.ends_with('+') { fragment.pop(); } } - if let Some(value) = value { - let new_param = format!("{}{}", prefix, value); + if let Some(param) = param { if !fragment.is_empty() { - fragment.push('&'); + fragment.push('+'); } - fragment.push_str(&new_param); + fragment.push_str(¶m); } url.set_fragment(if fragment.is_empty() { None } else { Some(&fragment) }); @@ -118,11 +142,11 @@ mod tests { fn test_ohttp_get_set() { let mut url = Url::parse("https://example.com").unwrap(); - let ohttp_keys = - OhttpKeys::from_str("AQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw").unwrap(); + let serialized = "OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC"; + let ohttp_keys = OhttpKeys::from_str(serialized).unwrap(); url.set_ohttp(Some(ohttp_keys.clone())); - assert_eq!(url.fragment(), Some("ohttp=AQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw")); + assert_eq!(url.fragment(), Some(serialized)); assert_eq!(url.ohttp(), Some(ohttp_keys)); url.set_ohttp(None); @@ -136,7 +160,7 @@ mod tests { let exp_time = std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1720547781); url.set_exp(Some(exp_time)); - assert_eq!(url.fragment(), Some("exp=1720547781")); + assert_eq!(url.fragment(), Some("EX1C4UC6ES")); assert_eq!(url.exp(), Some(exp_time)); @@ -144,21 +168,11 @@ mod tests { assert_eq!(url.fragment(), None); } - #[test] - fn test_invalid_v2_url_fragment_on_bip21() { - // fragment is not percent encoded so `&ohttp=` is parsed as a query parameter, not a fragment parameter - let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\ - &pj=https://example.com\ - #exp=1720547781&ohttp=AQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw"; - let uri = Uri::try_from(uri).unwrap().assume_checked().check_pj_supported().unwrap(); - assert!(uri.extras.endpoint().ohttp().is_none()); - } - #[test] fn test_valid_v2_url_fragment_on_bip21() { let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\ &pj=https://example.com\ - #ohttp%3DAQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw"; + #OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC"; let uri = Uri::try_from(uri).unwrap().assume_checked().check_pj_supported().unwrap(); assert!(uri.extras.endpoint().ohttp().is_some()); } diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index becf82a3..3223742a 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -193,7 +193,7 @@ mod integration { #[tokio::test] async fn test_bad_ohttp_keys() { let bad_ohttp_keys = - OhttpKeys::from_str("AQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw") + OhttpKeys::from_str("OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC") .expect("Invalid OhttpKeys"); let (cert, key) = local_cert_key();