Skip to content

Commit

Permalink
Shorten subdirectory IDs to 64 pseudorandom bits
Browse files Browse the repository at this point in the history
Instead of the compressed public key, subdirectory IDs are now a
truncated SHA256 of the compressed public key.

These are should only be treated unique identifiers, not hashes: the use
of SHA256 is only an implementation detail, and should not be specified
by BIP77, nor verified/enforced by clients. This is because 64 bit
hashes are insufficiently binding: finding a pair of colliding keys is
almost trivial and finding a 2nd preimage for a given ID is tractable.
For this reason no tagging is used to derive the IDs: public keys are
ephemeral and have sufficient entropy to be unguessable. Random IDs
could also have been used, but hashing seems simpler and reduces the
receiver's statefulness requirements.

ID collisions are only a liveness concern, and do not affect safety.
With BIP77, HPKE will fail due to the wrong key (and/or domain
separation if future protocols also use short IDs). With BIP 78 fallback
the PSBT will not contain the intended receiver's outputs. The intended
receiver can still poll the same subdirectory and respond, eventually,
but only one sender will succeed.

64 bits are sufficient to make the probability of experiencing a random
collisions negligible. As of writing, the UTXO set has ~2^28 elements.
This is a very loose upper bound for the number of concurrent (non-spam)
sessions, for which the probability of a random collision with will be
<1%. The actual number of sessions will of course be (orders of
magnitudes) lower given they are short lived. With ~2^21 sessions (loose
bound on number of transactions that can be confirmed in 24H) the
probability is less than 1e-6. These figures are for the existence of a
collision in the set, the probability for an individual session to
experience a random collision is << 1e-10 in either case.

Since no rate limiting or access control mechanism exists for the
directory yet, it's notable that this change changes the nature of a
hypothetical DoS attack. With long IDs the adversary could only cause
operational errors in theory (e.g. by filling the directory's storage).

However, with this change, by polling a large number of IDs an adversary
can succeed in randomly *intercepting* v2 clients' sessions, and POST
garbage data to the session causing HPKE to fail.

For v1 sessions this can leak PSBT proposals, since those are not
encrypted, which can leak input ownership information to the adversary.

As implemented this change is not a regression but an actually hardens
against DoS in practice, because although in theory subdirectory IDs
contained more entropy, the underlying redis keys prior to this change
only contained 41 bits of entropy (8 characters of base64 encoded data,
with 0x02 or 0x03 for the first encoded byte).

Both the random collision and abuse scenarios can be mitigated by
restricting the number of concurrent sessions in the directory to more
reasonable values (less than 2^20). This is not done in this change.
  • Loading branch information
nothingmuch committed Nov 12, 2024
1 parent a891a56 commit b8d3332
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 45 deletions.
57 changes: 46 additions & 11 deletions Cargo-minimal.lock
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2c8d66485a3a2ea485c1913c4572ce0256067a5377ac8c75c4960e1cda98605f"
dependencies = [
"bitcoin-internals",
"bitcoin_hashes",
"bitcoin-internals 0.3.0",
"bitcoin_hashes 0.14.0",
]

[[package]]
Expand Down Expand Up @@ -224,11 +224,11 @@ dependencies = [
"base58ck",
"base64 0.21.7",
"bech32",
"bitcoin-internals",
"bitcoin-io",
"bitcoin-internals 0.3.0",
"bitcoin-io 0.1.2",
"bitcoin-units",
"bitcoin_hashes",
"hex-conservative",
"bitcoin_hashes 0.14.0",
"hex-conservative 0.2.1",
"hex_lit",
"secp256k1",
"serde",
Expand Down Expand Up @@ -262,12 +262,27 @@ dependencies = [
"serde",
]

[[package]]
name = "bitcoin-internals"
version = "0.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2b854212e29b96c8f0fe04cab11d57586c8f3257de0d146c76cb3b42b3eb9118"

[[package]]
name = "bitcoin-io"
version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "340e09e8399c7bd8912f495af6aa58bea0c9214773417ffaa8f6460f93aaee56"

[[package]]
name = "bitcoin-io"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "26792cd2bf245069a1c5acb06aa7ad7abe1de69b507c90b490bca81e0665d0ee"
dependencies = [
"bitcoin-internals 0.4.0",
]

[[package]]
name = "bitcoin-ohttp"
version = "0.6.0"
Expand Down Expand Up @@ -297,7 +312,7 @@ version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5285c8bcaa25876d07f37e3d30c303f2609179716e11d688f51e8f1fe70063e2"
dependencies = [
"bitcoin-internals",
"bitcoin-internals 0.3.0",
"serde",
]

Expand All @@ -307,11 +322,21 @@ version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bb18c03d0db0247e147a21a6faafd5a7eb851c743db062de72018b6b7e8e4d16"
dependencies = [
"bitcoin-io",
"hex-conservative",
"bitcoin-io 0.1.2",
"hex-conservative 0.2.1",
"serde",
]

[[package]]
name = "bitcoin_hashes"
version = "0.15.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e0982261c82a50d89d1a411602afee0498b3e0debe3d36693f0c661352809639"
dependencies = [
"bitcoin-io 0.2.0",
"hex-conservative 0.3.0",
]

[[package]]
name = "bitcoincore-rpc"
version = "0.19.0"
Expand Down Expand Up @@ -343,7 +368,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3ee5cf6a9903ff9cc808494c1232b0e9f6eef6600913d0d69fe1cb5c428f25b9"
dependencies = [
"anyhow",
"bitcoin_hashes",
"bitcoin_hashes 0.14.0",
"bitcoincore-rpc",
"flate2",
"log",
Expand Down Expand Up @@ -1018,6 +1043,15 @@ dependencies = [
"arrayvec",
]

[[package]]
name = "hex-conservative"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4afe881d0527571892c4034822e59bb10c6c991cce6abe8199b6f5cf10766f55"
dependencies = [
"arrayvec",
]

[[package]]
name = "hex_lit"
version = "0.1.1"
Expand Down Expand Up @@ -1584,6 +1618,7 @@ dependencies = [
"bitcoin",
"bitcoin-hpke",
"bitcoin-ohttp",
"bitcoin_hashes 0.15.0",
"bitcoind",
"http",
"log",
Expand Down Expand Up @@ -2212,7 +2247,7 @@ version = "0.29.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9465315bc9d4566e1724f0fffcbcc446268cb522e60f9a27bcded6b19c108113"
dependencies = [
"bitcoin_hashes",
"bitcoin_hashes 0.14.0",
"rand",
"secp256k1-sys",
"serde",
Expand Down
57 changes: 46 additions & 11 deletions Cargo-recent.lock
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2c8d66485a3a2ea485c1913c4572ce0256067a5377ac8c75c4960e1cda98605f"
dependencies = [
"bitcoin-internals",
"bitcoin_hashes",
"bitcoin-internals 0.3.0",
"bitcoin_hashes 0.14.0",
]

[[package]]
Expand Down Expand Up @@ -224,11 +224,11 @@ dependencies = [
"base58ck",
"base64 0.21.7",
"bech32",
"bitcoin-internals",
"bitcoin-io",
"bitcoin-internals 0.3.0",
"bitcoin-io 0.1.2",
"bitcoin-units",
"bitcoin_hashes",
"hex-conservative",
"bitcoin_hashes 0.14.0",
"hex-conservative 0.2.1",
"hex_lit",
"secp256k1",
"serde",
Expand Down Expand Up @@ -262,12 +262,27 @@ dependencies = [
"serde",
]

[[package]]
name = "bitcoin-internals"
version = "0.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2b854212e29b96c8f0fe04cab11d57586c8f3257de0d146c76cb3b42b3eb9118"

[[package]]
name = "bitcoin-io"
version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "340e09e8399c7bd8912f495af6aa58bea0c9214773417ffaa8f6460f93aaee56"

[[package]]
name = "bitcoin-io"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "26792cd2bf245069a1c5acb06aa7ad7abe1de69b507c90b490bca81e0665d0ee"
dependencies = [
"bitcoin-internals 0.4.0",
]

[[package]]
name = "bitcoin-ohttp"
version = "0.6.0"
Expand Down Expand Up @@ -297,7 +312,7 @@ version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5285c8bcaa25876d07f37e3d30c303f2609179716e11d688f51e8f1fe70063e2"
dependencies = [
"bitcoin-internals",
"bitcoin-internals 0.3.0",
"serde",
]

Expand All @@ -307,11 +322,21 @@ version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bb18c03d0db0247e147a21a6faafd5a7eb851c743db062de72018b6b7e8e4d16"
dependencies = [
"bitcoin-io",
"hex-conservative",
"bitcoin-io 0.1.2",
"hex-conservative 0.2.1",
"serde",
]

[[package]]
name = "bitcoin_hashes"
version = "0.15.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e0982261c82a50d89d1a411602afee0498b3e0debe3d36693f0c661352809639"
dependencies = [
"bitcoin-io 0.2.0",
"hex-conservative 0.3.0",
]

[[package]]
name = "bitcoincore-rpc"
version = "0.19.0"
Expand Down Expand Up @@ -343,7 +368,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3ee5cf6a9903ff9cc808494c1232b0e9f6eef6600913d0d69fe1cb5c428f25b9"
dependencies = [
"anyhow",
"bitcoin_hashes",
"bitcoin_hashes 0.14.0",
"bitcoincore-rpc",
"flate2",
"log",
Expand Down Expand Up @@ -1018,6 +1043,15 @@ dependencies = [
"arrayvec",
]

[[package]]
name = "hex-conservative"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4afe881d0527571892c4034822e59bb10c6c991cce6abe8199b6f5cf10766f55"
dependencies = [
"arrayvec",
]

[[package]]
name = "hex_lit"
version = "0.1.1"
Expand Down Expand Up @@ -1584,6 +1618,7 @@ dependencies = [
"bitcoin",
"bitcoin-hpke",
"bitcoin-ohttp",
"bitcoin_hashes 0.15.0",
"bitcoind",
"http",
"log",
Expand Down Expand Up @@ -2212,7 +2247,7 @@ version = "0.29.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9465315bc9d4566e1724f0fffcbcc446268cb522e60f9a27bcded6b19c108113"
dependencies = [
"bitcoin_hashes",
"bitcoin_hashes 0.14.0",
"rand",
"secp256k1-sys",
"serde",
Expand Down
36 changes: 27 additions & 9 deletions payjoin-directory/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ use tracing::debug;
const DEFAULT_COLUMN: &str = "";
const PJ_V1_COLUMN: &str = "pjv1";

// TODO move to payjoin crate as pub?
// TODO impl From<HpkePublicKey> for ShortId
// TODO impl Display for ShortId (Base64)
// TODO impl TryFrom<&str> for ShortId (Base64)
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) struct ShortId(pub [u8; 8]);

impl ShortId {
pub fn column_key(&self, column: &str) -> Vec<u8> {
self.0.iter().chain(column.as_bytes()).copied().collect()
}
}

#[derive(Debug, Clone)]
pub(crate) struct DbPool {
client: Client,
Expand All @@ -19,23 +32,28 @@ impl DbPool {
Ok(Self { client, timeout })
}

pub async fn push_default(&self, pubkey_id: &str, data: Vec<u8>) -> RedisResult<()> {
pub async fn push_default(&self, pubkey_id: &ShortId, data: Vec<u8>) -> RedisResult<()> {
self.push(pubkey_id, DEFAULT_COLUMN, data).await
}

pub async fn peek_default(&self, pubkey_id: &str) -> Option<RedisResult<Vec<u8>>> {
pub async fn peek_default(&self, pubkey_id: &ShortId) -> Option<RedisResult<Vec<u8>>> {
self.peek_with_timeout(pubkey_id, DEFAULT_COLUMN).await
}

pub async fn push_v1(&self, pubkey_id: &str, data: Vec<u8>) -> RedisResult<()> {
pub async fn push_v1(&self, pubkey_id: &ShortId, data: Vec<u8>) -> RedisResult<()> {
self.push(pubkey_id, PJ_V1_COLUMN, data).await
}

pub async fn peek_v1(&self, pubkey_id: &str) -> Option<RedisResult<Vec<u8>>> {
pub async fn peek_v1(&self, pubkey_id: &ShortId) -> Option<RedisResult<Vec<u8>>> {
self.peek_with_timeout(pubkey_id, PJ_V1_COLUMN).await
}

async fn push(&self, pubkey_id: &str, channel_type: &str, data: Vec<u8>) -> RedisResult<()> {
async fn push(
&self,
pubkey_id: &ShortId,
channel_type: &str,
data: Vec<u8>,
) -> RedisResult<()> {
let mut conn = self.client.get_async_connection().await?;
let key = channel_name(pubkey_id, channel_type);
() = conn.set(&key, data.clone()).await?;
Expand All @@ -45,13 +63,13 @@ impl DbPool {

async fn peek_with_timeout(
&self,
pubkey_id: &str,
pubkey_id: &ShortId,
channel_type: &str,
) -> Option<RedisResult<Vec<u8>>> {
tokio::time::timeout(self.timeout, self.peek(pubkey_id, channel_type)).await.ok()
}

async fn peek(&self, pubkey_id: &str, channel_type: &str) -> RedisResult<Vec<u8>> {
async fn peek(&self, pubkey_id: &ShortId, channel_type: &str) -> RedisResult<Vec<u8>> {
let mut conn = self.client.get_async_connection().await?;
let key = channel_name(pubkey_id, channel_type);

Expand Down Expand Up @@ -99,6 +117,6 @@ impl DbPool {
}
}

fn channel_name(pubkey_id: &str, channel_type: &str) -> String {
format!("{}:{}", pubkey_id, channel_type)
fn channel_name(pubkey_id: &ShortId, channel_type: &str) -> Vec<u8> {
pubkey_id.column_key(channel_type)
}
Loading

0 comments on commit b8d3332

Please sign in to comment.