feat(verifcid): AddGoodHash

[Wondertan]

Allows Boxo users to expand the set of secure hash functions without having to fork Boxo.

Users may introduce new hashes to multishash registry but won't be able to use them as Blockservice layer is strict about which hashes are allowed via verifcid pkg.

Now anyone can add new hashes globally, potentially degrading security, which is a tradeoff of this solution. Open to alternatives that do not involve forking.

I didn't add any godocing as none of the funcs in the pkg has it, but happy to populate all of them, as a side quest.

[welcome]

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

• The context for this PR is clear, with relevant discussion, decisions
  and stakeholders linked/mentioned.

• Your contribution itself is clear (code comments, self-review for the
  rest) and in its best form. Follow the code contribution
  guidelines
  if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

• A maintainer will triage and assign priority to this PR, commenting on
  any missing things and potentially assigning a reviewer for high
  priority items.

• The PR gets reviews, discussed and approvals as needed.

• The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

[codecov]

Codecov Report

Merging #281 (87e8b5c) into main (b3b6620) will decrease coverage by 0.12%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
- Coverage   47.94%   47.82%   -0.12%     
==========================================
  Files         273      273              
  Lines       33186    33191       +5     
==========================================
- Hits        15912    15875      -37     
- Misses      15600    15633      +33     
- Partials     1674     1683       +9     

Impacted Files	Coverage Δ
verifcid/validate.go	73.33% <0.00%> (-14.67%)	⬇️

... and 13 files with indirect coverage changes

[Jorropo]

I don't like this, using globals for configuration is bad. I am not saying what we have right now is good, but it can't get that bad because it's not configurable.

If you want to make it configurable I really think we should refactor this to be dependency injected.

Other maintainers might disagree.

[hacdias]

I will agree with @Jorropo as this will influence all the user's of this package in the same application. If we add configurability, it'd be better to ensure that it is only applied to the usage context.

[Jorropo]

Feel free to send a PR that refactors this in some dependency injectable interface we would like this.

But no globals configs, this was a mistake but let's not make it worst.

[Wondertan]

Please fix this in the way you see the best. It feels aligned with all the maintenance and chore work you've been doing recently.

I have a brief idea, but it is unlikely to follow your opinions and practices, and I will need more time to get into the review round loops than I can afford.

[Jorropo]

@Wondertan I don't mind fixing this. I don't want it like this.

Globals are bearable as long as they are read only after the init of the package.
They are not a valid place to put configuration options, we recently just fixed an other case like #291 #328.

You would first need to create an interface:

// better name wanted
type Validator interface{
 IsGoodHash(code uint64) bool
 ValidateCid(c cid.Cid) error
}

Then you move the current global to a struct:

type validator struct{
 goodset map[uint64]bool
}

func NewValidator(allowedCodes map[uint64]bool) Validator {
 return validator{allowedCodes}
}

func (v validator) IsGoodHash(code uint64) bool {
	good, found := v.goodset[code]
	if good {
		return true
	}

	if !found {
		if code >= mh.BLAKE2B_MIN+19 && code <= mh.BLAKE2B_MAX {
			return true
		}
		if code >= mh.BLAKE2S_MIN+19 && code <= mh.BLAKE2S_MAX {
			return true
		}
	}

	return false
}

func (v validator) ValidateCid(c cid.Cid) error {
	pref := c.Prefix()
	if v.IsGoodHash(pref.MhType) {
		return ErrPossiblyInsecureHashFunction
	}

	if pref.MhType != mh.IDENTITY && pref.MhLength < minimumHashLength {
		return ErrBelowMinimumHashLength
	}

	if pref.MhType != mh.IDENTITY && pref.MhLength > maximumHashLength {
		return ErrAboveMaximumHashLength
	}

	return nil
}

Then you change all consumers of verifcid from calling the global functions to taking a Validator interface as input and calling the interface instead.

Note: for backward compatibility you should have a DefaultVerifier global variable that holds a validator with the current list and keep the current global IsGoodHash and ValidateCid functions but have they query DefaultVerifier.

After doing this:

1. There is an interface so you don't actually need to get stuff merged, you can make your own implementation and pass it in to bitswap, ipld, ... whoever uses verifcid.
2. If you want to add AddGoodHash you I would be ok merging this in boxo given this is made by make a new extending interface and add the feature (but not on the global verifier).