Harden mailroom against metadata side channels

[DanGould]

Summary

Three changes to reduce metadata the directory operator can observe about payjoin sessions. None require protocol changes.

1. Unify mailbox TTL to a single value

Problem: Read mailboxes expire in 10 minutes (DEFAULT_READ_TTL), unread in 7 days (DEFAULT_UNREAD_TTL_BELOW_CAPACITY). An operator can stat mailbox files on disk: if a file disappears quickly, the receiver is online. The 70x gap makes this obvious.

Fix: Replace the three TTL constants in files.rs:24-30 with a single DEFAULT_TTL of 24 hours. Remove the read/unread distinction in pruning logic (files.rs:509-510). Prune all mailboxes based on creation time only.

Why 24 hours: Matches the default receiver session lifetime (TWENTY_FOUR_HOURS_DEFAULT_EXPIRATION in payjoin/src/core/receive/v2/mod.rs:70). A mailbox that outlives its session serves no purpose.

Why one config, not two: The privacy issue is the gap between read and unread TTLs. Two separate configs can drift apart and reintroduce the gap. One value eliminates this by construction.

Capacity-based eviction can still apply as a storage pressure mechanism, but should not depend on read status.

2. Normalize V1 metric paths

Problem: middleware.rs:42 records the full URL path as a metric label. V2 requests all share /.well-known/ohttp-gateway, but V1 requests use /{shortid}, so every unique mailbox ID becomes a permanent label in the metrics backend. This copies per-session identifiers into a system with no TTL, surviving the in-memory take() cleanup.

Fix: Before recording metrics, replace V1 paths with a fictional static label like /v1. All V1 requests share one counter. Individual ShortIds stay out of the observability pipeline.

Context

Found during adversarial analysis of the relay/directory trust model. Both are low-severity on their own (the directory doesn't know who owns a mailbox in V2), but they compound with other information sources and violate the data minimization intent of the existing code. Easy fixes imo for someone to just pick up based on this script.

Disclosure: co-authored by Claude

[nothingmuch]

The rationale here doesn't track. The operator already controls the software, and therefore can add logging or any other method of detecting activity patterns. For the intended impact to be realized we could rely on a TEE to decapsulate requests and handle storage state, with attestation clients can then some assurances (how strong is debatable, but definitely not nothing) that the operator can't monitor this information.

[DanGould]

I agree that they could add logging or other methods of detecting, but this raises the cost for an operator using our off-the-shelf software.

And I think this rationale only applies to the first concern. The metrics are separate from the operator, so I think normalization still stands. Or because they're random, it doesn't matter for them to share?

[Arowolokehinde]

Hi @DanGould

This would be my first contribution to the rust-payjoin project, and I’d love to take on this issue. I’m interested in using it as an opportunity to get familiar with the codebase and contribute meaningfully.

I have experience with Rust and backend development, and I’m comfortable diving into new codebases and asking questions where needed.

Please let me know if this issue is available or if there’s anything I should review before getting started. Thanks!

[nothingmuch]

I agree that they could add logging or other methods of detecting, but this raises the cost for an operator using our off-the-shelf software.

Modifying the code is significantly simpler than doing an analysis of the information that could be obtained (adding a few log statements and/or disabling file deletion), so i don't think it's a strong deterrent.

I'm concerned that investing effort in this takes away from meaningful changes but more so because of the pervasive culture of such arguments in favor of coinjoin coordinators being supposedly less trusted than they are.

We don't want to obscure what the directory operator is trusted with even unintionally IMO.

A good reason to unify this expiry behavior would be to simplify the code, i remember it being more of a pain than I expected to work through the state machine there, but once it was done it seemed reasonable to keep it, since with rate limiting it allows strong resource utilization limits to be enforced (no more than some amount of storage) based on a 24 hr expiry, but without making 24 hrs a hard limit on all sessions. Currently we specify 24 hrs by default but that's an implementation detail, not a spec requirement, and in theory the values are unbounded. This complexity currently enables clients that set it to larger values to still use the directory on a best effort basis.

And I think this rationale only applies to the first concern. The metrics are separate from the operator, so I think normalization still stands. Or because they're random, it doesn't matter for them to share?

No this is a legitimate bug IMO, and I agree it should be normalized to redact the short IDs. It serves no metrics purpose to analyze those separately and may even cause problems for certain metrics databases if that's indexed, and just generally doesn't really make sense since metrics are about aggregation and these will be unique per request.

As for hypothetically, will it be harmful: i'm still of the opinion that we shouldn't encourage operators to share metrics data until we've made a reasonable argument for both:

• why is it safe to do so (i.e. privacy leaks are not a concern)
• why it's valuable to publish that at all

The temporal leak is of some concern, so i don't think this argument will be easy to make: if the adversary is monitoring the mempool and recording broadcasting times, then having information about when payjoin sessions are likely to have terminated may aid significantly in correctly labeling payjoin transactions, unless a large portion clients are known to strongly randomize broadcast times. We should probably make a recommendation and make it easy to do so (exponential delay is best for privacy but this can cause problems for mobile clients...).

[nothingmuch]

Hi @DanGould
Please let me know if this issue is available or if there’s anything I should review before getting started. Thanks!

IMO start with making a subtask for the v1 request path metric issue, as it is self contained and fixing it is clearly appropriate, whereas the TTL unification suggestion I think needs consensus first

[zealsham]

@nothingmuch is stripping out the shortid for the v1 path using regex let re = regex::Regex::new(r"^[a-f0-9]{16}$").unwrap(); before recording the metrics a sufficient fix ?

[nothingmuch]

yeah, maybe so it isn't conflated with the real / path it's best to replace it with /{shortid} or something similar

[zealsham]

@Arowolokehinde Do you still want to work on the second part of this?. if anything looks confusing, i could be of help.

[Arowolokehinde]

Yeah
I would like to. i thought you already worked on it when I saw that you linked a PR.