Use Base58 instead of Base32 and Remove SCID shortening

[bj-ms]

As part of the discussion and security concerns in https://github.com/bcgov/trustdidweb/issues/75, I propose to change Base32 to Base 58 and remove the shortening of the SCID during it's generation.

I also changed the version, changelog and added myself as an author (feel free to comment on a possible rollback on that part).

[swcurran]

@bj-ms -- thanks! Please add DCO sign-off to the PR. See DCO - Developer Certificate of Origin - https://github.com/apps/dco. Instructions for fixing the commit are on the "Details" beside the failed check.

[swcurran]

Given that we want the flexibility/future-proofing to support different hash algorithms with backwards compatibility, I think we should use multihash (at least) and perhaps multibase here, so that a verifier can detect the hash (and encoding) algorithm(s) used. The spec. will dictate the allowed hashes to generate, but a verifier should be flexible in receiving hashes.

Do we need multibase as well as multihash?

If we use multihash, the length of the (untruncated) SCID (and entry hashes) will be 46 characters -- reference. So the SCID is up 18 from the previous 28, and we eliminate messing with the truncation. I think that is fine -- especially once we get into PQ public keys :-)

[andrewwhitehead]

There's a minor security benefit to adding the hash algorithm, as it then could not be swapped in a replacement log file. That would still require producing a hash collision though, which is very unlikely.

Another option would be to add a version byte to the hash, fixing the generation algorithm version that was used. We're not currently using multihash so it would be a new dependency.

[swcurran]

I’m less worried about the security as the interop. Defense coding and future-proofing.

[brianorwhatever]

I think we should use multibase and multihash and remove the hash parameter. Write normative statements saying which hash and base algs are allowed.

[swcurran]

I’m good with that. We definitely want to constrain what a DID Controller is allowed to use (hence the normative statements) and allow a resolver to discover the DID Controller's choices. I do think we keep the spec version and use that as an indicator to the resolver. The spec. version dictates the options.

[swcurran]

So SCID calculation becomes:

SCID = multibase(<base>,multihash(<hash>, JCS(input with placeholders)))

[brianorwhatever]

yep version is fine I just want to remove hash from the parameters as that does the same job as multihash is.. other than the minor security benefit andrew mentions above which I don't see as a concern

[andrewwhitehead]

I can see multihash, but I'd rather not use multibase. We don't have a need to support multiple encodings and it's more work for verifiers.

[brianorwhatever]

ok I'm fine with that. Also fine with not making the multihash change and just changing to base58btc in this PR

[swcurran]

I’d like to see multi-hash. I can see a time where we add, for example, sha3-256 (as we briefly did), while still allowing sha-256 (since it is needed for long lasting DIDs). Multihash would be important.

Multibase less so. There are not going to be massive breakthroughs in efficiency or security related to encoding schemes :-).

[swcurran]

If we use multihash, I think we wind up using base58btc? Or is it still base58?

[andrewwhitehead]

The (expired) draft that is referenced is the Bitcoin version of base58, aka base58-btc.

[swcurran]

Is there another spec we should be referencing?

[brianorwhatever]

We should reference the VC controller document.

https://www.w3.org/TR/controller-document/#multibase-0
https://www.w3.org/TR/controller-document/#multihash

[swcurran]

Side question @brianorwhatever — What is “Controller Document”? There is no description of why it is and what it is for. AFAICS, it is much like DIDs, and I heard Manu said it started with the DID Spec.

[brianorwhatever]

It's essentially where the normative statements for multibase/multihash had to be moved to in the VCWG as the group couldn't get consensus to have them in VCDM and there is a shared requirement between DI/JOSE.. I think basically a DID Document is a controller document

[andrewwhitehead]

We could be a bit more specific with [1-9A-HJ-NP-Za-km-z] !

[swcurran]

Or leave it off and leave it to the spec.

[swcurran]

@bj-ms — are you good with updating your PR to reflect the feedback? I think yours is close, but there will be a few more places to address. Happy to do that as a follow up PR. Let me know how you want to proceed.

[bj-ms]

@bj-ms — are you good with updating your PR to reflect the feedback? I think yours is close, but there will be a few more places to address. Happy to do that as a follow up PR. Let me know how you want to proceed.

I'm good with that, I will make the following modifications:

• Use multihash in SCID and mention the Controller document multihash table.
• Use base58btc in SCID
• Remove hash property from the did log
• Remove the definition of SCID in the chapter Method-Specific Identifier

[brianorwhatever]

base58 consists of 123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz This purposely leaves off some characters that might be misread by humans.

[swcurran]

Suggest we don’t repeat the data here — just reference the spec.

[martipos]

I am happy with the changes to base58 and skipping the trimming. Just two additional inputs.

1. It seams base58 does not perform very well (in term of speed) with large byte values. Given that the SCID is generated based on the entry log this should however not be an issue as I assume the entry log will not reach a sized of concern (or are there any things planned on that end) ?
2. Out of curiosity, would double hashing be of interesting in generating the SCID?

[swcurran]

The first issue is not a concern, as the input will always be a hash, which will be (for now) 32 bytes, perhaps longer if/when other hash algorithms are needed. Never more than 100 characters.

I’m not sure about a double hash as I don’t know how to calculate collision probability and how that translates into “time to calculate” in typical conditions (including Quantum conditions). AFAIK, the attacker must generate a new log entry with a key they control and take over the web server publishing the file. The hashing controls the probability of the first part of that.

[swcurran]

There are a couple of very minor clarifications I'd like to make to this (most of which are a bit tangential), but not worth holding up the merge. I think we have all agreed on the content. Let's get this merged.

[andrewwhitehead]

The double hashing could be worth exploring if it increases the preimage resistance, although the difference would be pretty minor. Bitcoin uses it but without any real explanation of the purpose, IIRC. It would be easy to support without adding additional hash functions, but I'm not sure how it would compare to sha512/256 in strength.