NICMx / FORT-validator

RPKI cache validator
MIT License
47 stars 22 forks source link

Shuffle the order in which Manifest entries are processed #139

Closed job closed 2 weeks ago

job commented 3 weeks ago

Previously work items were enqueued in the order the CA intended them to appear on a Manifest. However, there is no obvious benefit to letting third parties decide the order in which objects are processed.

Instead, randomize the list of FileAndHashes, its ordering has no meaning anyway. As they say, a fox is not taken twice in the same snare

ydahhrk commented 2 weeks ago

Ok,

Is there an explicit reason to re-init the random seed every time an RPP has to be shuffled?

Assuming each thread has its own seed, it seems this will yield the same order for all RPPs that have the same size and are collected during the same second.

I'm not saying that is good or bad, just maybe not necessarily in the intended spirit of the PR.

there is no obvious benefit to letting third parties decide the order in which objects are processed.

Hmm. Ok, but you're arguing "why not" instead of "why."

Suggestion: Maybe because it's the order in which they reviewed the repository with their own internal RPs before releasing it to the public? I know the order should not matter, but their specific order is the one they know for certain works.

I assume your intention is to detect bugs that have to do with traversal order? I'm concerned this looks like a two-edged blade: Adding randomness to the validation will make bug reproduction unreliable.

Maybe the seed should be moved to the beginning of the validation cycle, and Fort should print it in the logs or something. I can tweak this if you want, but I'm making assumptions and would like to hear your thoughts beforehand.

job commented 2 weeks ago

This is not about uncovering bugs, I think you’ll be fine in that regard. This is to avoid giving attackers (think “slowloris”) an upper hand. An attacker controls the key pair that in turn guides the CA cert filename on the manifest. An attacker could generate key pairs until they find one that lexically sorts before other entries and from there have a degree of control that RPs shouldn’t allow them to have.

You want real random, so that means reseeding often. The alternative would be to import arc4random()

We have positive experiences in rpki-client following this approach: https://github.com/openbsd/src/commit/31ac5904884e0d0ad6c02a5ccf0e5fcf3890c71d

ties commented 2 weeks ago

Using a random permutation is not the approach described in Beyond Limits: How to Disable Validators in Secure Networks but I think it may get there.