Distrust is a nuclear option

[kornelski]

I've ran into another case of #329. This time @HeroicKatora distrusted my reviews. I don't even disagree with the distrust per se, because I'm interested mainly whether crates have intentional malware or backdoors, and I don't care about subtle soundness bugs that much. I need 100% review coverage for my projects, so I have to favor quantity over quality.

But the problem is that this disagreement has created a fallout that's IMHO bigger than it should have:

• It's a problem for me, because it knocked out hundreds of my own reviews from my own WoT. I obviously trust myself, and yet crev doesn't.

• I don't want to distrust @HeroicKatora, because it's going to be similarly problematic for them, plus it doesn't even work: Result of mutually-distrusting Ids depends on order they're processed #389

Crev still has relatively few reviews and reviewers, so nuking people completely out of the WoT is IMHO a big loss.

Can we add less "nuclear" options to the WoT? e.g.

• ability to override someone else's distrust
• ability to distrust reviews, rather than the whole Id Overwritting someone elses reviews. #264
• weaker distrust level that works only for one user locally, not for the whole WoT

[HeroicKatora]

My own usage of crev is meant to provide a trusted computing base with regards to soundness. And I didn't find another way of specifying crev to bound the weight of reviews to at most low, I didn't realize the implications on the WoT as a whole. So pretty much any of the three proposed fixes would work for me.

[dpc]

Maybe lower trust nodes shouldn't be able to ban higher trust ones?

                // Note: lower trust node can ban higher trust node, but only
                // if it wasn't banned by a higher trust node beforehand.
                // However banning by the same trust level node, does not prevent
                // the node from banning others.
                if direct_trust == TrustLevel::Distrust {
                    debug!("Adding {} to distrusted list", candidate_id);
                    // We discard the result, because we actually want to make as much
                    // progress as possible before restaring building the WoT, and
                    // we will not visit any node that was marked as distrusted,
                    // becuse we check it for every node to be visited
                    let _ = current_trust_set
                        .record_distrusted_id(candidate_id.clone(), current.id.clone());

                    continue;
                }

[dpc]

OK, so after some more thinking, and refreshing my memory here are some thoughts:

@HeroicKatora - distrust is intended for eliminating malicious or misbehaving parties, not for filtering out quality. The quality of the review is self-reported via thoroughness and understanding level so if @kornelski does not maliciously inflate thoroughness level, his reviews are perfectly OK and very useful for the rest of the community. You can use --thoroughness X and --understanding Y flags to filter out reviews that are of lower thoroughness level than you required. If a user was to inflate their self-reported quality levels, it would probably be good to bring it up directly, and if unsuccessful/impossible then the distrust usage would be justified.

Distrusting someone for wrong/incorrect reasons qualifies as malicious and can result in counter-distrust from many sides that lose access to useful parts of WoT.

Trust is intended for judging trustworthiness of the party - how reputable they are personally, how much time they invested, how much do they have at stake, is there a way to sue them etc., how much would we trust their word against some other person in case of a dispute. It's a shame that due to how it works it's a bit overly-exposed relatively to thoroughness and understanding levels.

Please let me know if this seems satisfying and makes sense. We can still think about improvements.

TODO:

• We should put a warning in a header comment when editing distrust proof explaining when it should be used and consequences of banning people for no good reason. I completely understand that it can be unclear and confusing right now.
• We should make sure that user gets text feedback about all banned users (especially if they are ones being banned)
• Maybe some aggregate (average?) thoroughness and understanding levels being displayed next to IDs would help discover their meaning and also identify and judge "thoroughness-inflation".

[HeroicKatora]

The problem is a that there are a number of competing properties to certify with a review:

• The crate is non-malicious
• The crate is sound
• The crate is misuse resistant (important for cryptography related code)

The thoroughness and understanding score are related only tangentially. A faithful reviewer can do an in-depth review and have very thorough understanding of the code and still regard it as non-malicious even though it is not strictly sound. It is not possible to mark a review as certifying only one of these properties but at least in this case it was clear that the author only intended to publish reviews for 'non-maliciousness'.

It seems that the effect on the web-of-trust is a lot wider than I had perceived when publishing the distrust. And indeed it makes sense that the only useful (and in a technical sense productive) action might be a counter distrust. I had not fully realized the transitive effect on the ecosystem! In light of this I will undo my review and rather look into other tooling to that effect.

Regarding the TODO I think they are all sensible. One ways that immediately comes to mind to approach the problem that we've identified, without affecting the web of trust, would be create 'categories' that allow a reviewer to self-report their supposed review goal and thus to allow filtering of reviews by that purpose. A more fully integrated system might allow a way to locally disagree with reviews and authors in one of the categories.

[dpc]

The problem is a that there are a number of competing properties to certify with a review:

One ways that immediately comes to mind to approach the problem that we've identified, without affecting the web of trust, would be create 'categories' that allow a reviewer to self-report their supposed review goal and thus to allow filtering of reviews by that purpose.

I understand your point. crev can definitely support such categories, as it's extendable. Each additional flag and options does however come with an additional mental overhead and work, and if it's opt-in, you'll find very few other reviewers that will also go an extra mile. Right now we're focused on trying to pick the low hanging fruit- at least make sure people are not using something blatantly broken, poor quality or even malicious. To get users to coordinate and at least look at the source code. 10% of effort for 90% of benefits.

With your use-case in mind, what I would do is as follows:

• Only assign high (and even medium) trust level to people who you are confident that share your requirements and you trust to follow them
• Use low (or none) for everyone else who you find reasonable, but not meting your review criteria; distrust is punitive, but plain low trust levels are not; "low trust is still some trust".
• Once you have cargo crev verify passing on everything, use --trust medium and try again to identify crates that are passing, but only because they were looked at by people that potentially do not share your views. Review yourself the missing bits. Once everything passes, increase to --trust high, and/or use --thoroughness X and --understanding Y to further narrow down stuff.
• Once you're done with that, increase --redundancy, lower --depth further.

One important thing to understand in crev's philosphy is that trust in code is always gradual. There is no absolute binary "good" or "bad". One can always further increase: number of people that should look at something, time they should have spent on it, their trustworthiness and how far in the WoT you're willing to trust.

[kornelski]

One important thing to understand in crev's philosphy is that trust in code is always gradual.

BTW, this is not very clear from cargo crev verify. You usually see pass or none.

Maybe it should show some numeric value that quantifies level of trust vs risk? e.g. a number of stars:

⭐️⭐️⭐️⭐️⭐️ // high trust/thoroughness/understanding
⭐️⭐️⭐️..  // medium levels of these things
⭐️....  // barely anyone has checked
..... unknown crate 
☠️☠️☠️.. // has build.rs/proc macro, not many downloads, not from trusted users
☠️☠️☠️☠️☠️ // reported as bad

etc.

[dpc]

@kornelski The verify output does show up a binary decision: "given the specified criteria (minimum trust level, max distance in the WoT, minimum acceptable thoroughness and understanding, and required number of redundant reviews) is this crate passing or not", however the criteria itself can be gradually increased.

Maybe displaying on stderr a short prefix listing all criteria being used, would bring it to attention of the user. Potentially users should be able to define their defaults too.

[dpc]

#405 - I went though the in-editor documentation and tweaked it a little bit, plus I've added a warning when distrust is used. I know I'm not a best documentation writer, but hopefully it's better than before.

[dpc]

@kornelski While working on showing up distrust, I've noticed that you've un-distrusted from a different ID than you originally distrusted. kornelski/crev-proofs@ccddd3c vs kornelski/crev-proofs@4177961