Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test that the encrypted payloads are uniform. #374

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DanGould
Copy link
Contributor

@DanGould DanGould commented Oct 22, 2024

close #371

This randomized test will generate a false negative with negligible probability
if all encrypted messages share an identical byte at a given position by chance.
It should fail deterministically if any bit position has a fixed value.

re #364 (review) from @nothingmuch

I did check that this test would indeed fail before the ellswift changes by cherry-picking the test on 9c4880c

Copy link
Contributor

@nothingmuch nothingmuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK with some minor corrections, IMO can be merged with or without my last proposed change (bit-wise instead of byte-wise comparison) at your discretion (if you do then make sure to adjust the number of messages I didn't add a suggestion for that).

(messages_a, messages_b)
}

/// For each of the 256 pairwise combinations, ensure their lengths are equal,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are 256 ordered pairs, but XOR is commutative... you correctly implemented only distinct unordered, so my original count is wrong.

Suggested change
/// For each of the 256 pairwise combinations, ensure their lengths are equal,
/// For each of the (n choose 2) = n*(n-1)/2 combinations, ensure their lengths are equal,

For what it's worth the reasoning behind these values is that in a uniformly random bit string for each bit position there's a $\frac{1}{2}^{16}$ probability of all the bits being a specific value, so $\frac{1}{2}^{15}$ for either value. For this to happen over an entire byte, since bits are independent is just a product $(\frac{1}{2}^{15})^{8} = \frac{1}{2}^{120}$ (I forgot to account for either value of a single bit position, so originally thought this would be 128, but 120 is still overkill so nbd).

payjoin/src/hpke.rs Outdated Show resolved Hide resolved
}

assert!(
accumulator.iter().all(|&b| b != 0),
Copy link
Contributor

@nothingmuch nothingmuch Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach would be to check that all bits (as opposed to bytes) contain variation. For this check the number of messages in the set should be 80 or 128 or whatever instead of 16 in order to have negligible chance of false positive.

Suggested change
accumulator.iter().all(|&b| b != 0),
accumulator.iter().any(|&b| b != 0xff),

edit: previous suggestion was incorrect, comparing to 1 instead of 0xff

@DanGould DanGould force-pushed the test-payload-uniformity branch 2 times, most recently from dfb2456 to 5fc4d03 Compare October 23, 2024 17:54
This randomized test will generate a false negative with negligible probability
if all encrypted messages share an identical byte at a given position by chance.
It should fail deterministically if any bit position has a fixed value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider Making pjv2 payloads and IDs Uniform so they Might be Indistinguishable from Future Protocols'
2 participants