Add warning about publicKeyMultibase

[OR13]

This PR:

1. changes the order of verification material so that the "non-normative" feature is defined after the normative one.
2. adds a warning about privateKeyMultibase representations potentially not existing.

https://pr-preview.s3.amazonaws.com/w3c/did-core/pull/772.html#dfn-publickeymultibase

---

Preview | Diff

[peacekeeper]

Agree with the re-ordering, but don't really understand the warning..

privateKeyMultibase is not in the DID Spec Registries, and it's not in the Security Vocab. And why even talk about private key representations at all in DID Core?

[oed]

Why would you ever want to represent the private key in the DID document?

[OR13]

@peacekeeper

And why even talk about private key representations at all in DID Core?

see the comments on d in the normative feature, publicKeyJwk.

[TallTed]

[@OR13 said] see the comments on d in the normative feature, publicKeyJwk.

Could you please provide a link to those comments, or at least to their neighborhood?

[OR13]

@oed

You would not want to ever represent a private key in a did document.

You probably would want to be able to represent a private key in the same way you represent the public key in a did document... for publicKeyJwk, we comment on this by noting that d should be excluded.

For publicKeyMultibase can you provide an example of a privateKeyMultibase for P-256 or Bls12381G2 : )

Thats what the advisement is about... as of right now, its not possible and its not documented anywhere... here are some PRs that made this possible for:

• Secp256k1
• X25519
• Ed25519

https://github.com/multiformats/multicodec/pulls?q=is%3Apr+is%3Aclosed+private

Noting that those are not all the key types that we see in did documents... in particular, there is not a single FIPS/NIST approved key that can have its private key component represented by privateKeyMultibase, unless you consider Ed25519 to be the only key type in use, the warning would appear to be required.

[OR13]

@TallTed

https://w3c.github.io/did-core/#dfn-publickeyjwk

The map MUST NOT contain "d", or any other members of the private information class as described in Registration Template.

[OR13]

A related comment regarding the lack of clarity on privateKeyBase58 and privateKeyMultibase:

• privateKeyBase58 contains publicKey digitalbazaar/ed25519-signature-2018-context#5

[selfissued]

I would also suggest adding this sentence at the beginning of the advisement:
"Note that publicKeyMultibase is not a standard and is therefore subject to change."

[oed]

@OR13 Even if it would be nice publicKeyMultibase doesn't use multicodec at all, so if they are part of the multicodec table is not really relevant here.

[OR13]

@selfissued thats fair, one example of that potential change would be requiring or forbidding EC point compression, although I have yet to see a publicKeyMultibase or publicKeyBase58 where an uncompressed public key was represented.

[OR13]

@oed https://github.com/w3c/did-test-suite/search?q=publicKeyMultibase

z6Mkh1... -> z = base58, 6Mk -> base58 of uvarint of:

https://github.com/multiformats/multicodec/blob/master/table.csv#L86

and as the table notes, its status is "draft".

Are you using publicKeyMultibase differently?

[oed]

@OR13 According to the did-core spec:

If present, the value MUST be a string representation of a [MULTIBASE] encoded public key.

It says nothing about using the multicodec varint so if those implementations use that it seems incorrect?

Seems like if you use multicodec it should be named something like publicKeyMulticodec.

[OR13]

@oed

• Supported public key formats? #67 (comment)
• Add COSE key format example into CBOR section #340 (comment)

publicKeyMulticodec has been discussed before.

[oed]

@OR13 you are missing my point. What's in did-core right now clearly does not mandate the use of multicodec.

[OR13]

@oed I get your point, show me an example of what is in did core today, in the wild or in the test suite...

I might even suggest that did core is wrong today, given the only example in the test suite appears to use multicodec.

Also, would you mind commenting on this PR: https://github.com/w3c-ccg/lds-ed25519-2020/pull/11

ping @clehner and @msporny

[oed]

I might even suggest that did core is wrong today, given the only example in the test suite appears to use multicodec.

I would be fine with that since I think using multicodec makes sense :)

[OR13]

Folks may find this helpful in understanding why the reference to MULTIBASE - https://datatracker.ietf.org/doc/html/draft-multiformats-multibase-03 is currently included, but there is no reference to MULTICODEC:

https://github.com/multiformats/multicodec#faq

[OR13]

@jonnycrunch you may also have feelings on this subject :)

[TallTed]

Adjustment to surrounding text, following on @OR13's addition of @selfissued's requested text....

Suggested change

	There might be some cases where <code><b>public</b>KeyMultibase</code> can
	be represented although <code><b>private</b>KeyMultibase</code> cannot.
	Note that publicKeyMultibase is not a standard and is therefore subject to
	change. Further, there might be some cases where
	<code><b>public</b>KeyMultibase</code> can be represented although
	<code><b>private</b>KeyMultibase</code> cannot.

[msporny]

the only example in the test suite appears to use multicodec.

While this is true, we shouldn't assume everyone that uses publicKeyMultibase will want to use multicodec or not. Some may prefer raw keys, others may prefer multicodec. The only commonality between those two approaches is multibase (not multicodec), which is why the property is named the way it is.

The assumption here is that your type value will tell you whether or not the publicKeyMultibase value is multicodec-encoded or not. It's also the most general property we could use that would result in across-the-board ideal compression in things like CBOR-LD.

I expect there would be a large number of -1s for changing this to publicKeyMulticodec for those reasons (not that I think anyone is suggesting that at this point).

[oed]

The assumption here is that your type value will tell you whether or not the publicKeyMultibase value is multicodec-encoded or not.

It would make sense to just have a type that's Multicodec, since you can always find the information about which type of public key it is from the multicodec varint.

[msporny]

It would make sense to just have a type that's Multicodec, since you can always find the information about which type of public key it is from the multicodec varint.

The danger there is that you take something that has two failsafes and move it to only having one.

One of the goals with the Linked Data Security work is to make cryptography safer to use for most developers. That means putting in multiple fail-safes to reduce the number of foot guns in the ecosystem. For example -- being able to semantically express private key values should either not be defined (and be application-specific) OR should not be defined in cryptosuites that are used in DID Documents and other public-facing data structures (this is counter to what JWKs do, which is globally define private key values across all applications -- which has led to disastrous publication of private key material in public spaces). While we can't prevent that sort of thing from happening 100% of the time, there are things we can do to reduce the possibility of that happening.

Another fail safe is double-checking the key type against the multicodec header. For example, "type": "Ed25519VerificationKey2020" is expected to have a publicKeyMultibase value that starts with a ed25519-pub multicodec header. So, there are two failsafes there (to make sure you're not accidentally dealing with private key material) ... the type, and the multicodec header. While some will argue that this is overkill, the counter-argument is that it's warranted give the mistakes that developers have made with JWK. Creating a Multicodec keytype repeats some of those same mistakes that have been made over the years in the same way that the publicKeyJwk property does. I know there are people in the community that are bound and determined to provide "key agility" -- and one of the features of Linked Data Security (decentralized innovation) is that we can't stop those people from proceeding down a dangerous road... but we should all be aware of the philosophies that are going into these key designs and what's driving each design.

Hope that helps provide some background, @oed. :)

[oed]

@msporny Hm, I fail to see why having two if statements would be better than one. I understand the concern about private keys, but that could easily be mitigated by not allowing private key multicodecs?

[msporny]

@msporny Hm, I fail to see why having two if statements would be better than one.

For the same reason that having seatbelts paired with airbags is a good idea. Having two failsafes are typically better than one, especially if there is no big cost to adding the failsafe. Two barriers to get through to prevent catastrophic failure. :)

I understand the concern about private keys, but that could easily be mitigated by not allowing private key multicodecs?

Private key multicodecs are already a thing -- that ship sailed when ed25519-priv was placed into the table 10 months ago:

multiformats/multicodec@28f7ad5

The table now contains multiple private key multicodecs, making it possible to accidentally expose (and index) private keys encoded in multicodec.

https://github.com/multiformats/multicodec/blob/master/table.csv#L131

[OR13]

-1 because doing so will give the wrong impression, that publicKeyJwk is the ONLY way to express key material when we already have examples in the ecosystem (Ed25519, BBS+, EthereumEip712Signature2021) where that's not the case.

It might give the impression that publicKeyJwk is the only STANDARD way to express verification material, and that any other way relies on did-spec-registries for interoperability.... If type is so important, and we already rely on the registry for them... I don't think this is as bad as it might look.

+1 to this approach, which I believe this PR does. To be clear, I'm supportive of this PR.

Regarding multicodec warning changes, here is some revised language based on the thread, @msporny would you mind helping me smith it out:

publicKeyMultibase might rely on multicodec or might rely on a pairing with type defined externally to determine the specific format of the key material. publicKeyMultibase might be paired with privateKeyMultibase or might be paired with privateKeyBase58 and type to determine the private representation of the public material present in the did document. The suite that defines type should define which of these optional approaches should be used for a given type.

[oed]

Well, then your implementations will be more dangerous than others and I wouldn't be surprised if you get issues raised against your library doing dangerous things (like presuming the type from the multicodec value and not the other way around) or developers choosing to avoid your implementations. :)

I mean in the implementations of my DID methods the underlying data structure just uses multicodec to represent public keys. Then uses the information in there to generate the DID document deterministically.

but multibase does not!?!?... this is making the point even clearer.

Multibase doesn't say anything about what the data itself is!

[OR13]

@msporny @peacekeeper can you please review?

[OR13]

@oed do you think we are making a terrible mistake by using publicKeyMultibase instead of publicKeyMulticodec?

Do we believe this will result in 2 non interoperable representations of secp256k1 for example?

[oed]

@OR13 if the intention is to specify what the actual representation is in the "type" then I guess that's ok. I think it's redundant, but w/e.

[msporny]

@OR13 if the intention is to specify what the actual representation is in the "type" then I guess that's ok. I think it's redundant, but w/e.

Yes, that's the intention. The encoding beyond the multibase encoding is signalled via type. We've already implemented this across the board for our Ed25519*2020 keys/signatures/etc... and if people hate this, we can always change it to something else in future cryptosuites (like we did for publicKeyBase58).

[msporny]

Minor rewording by request... otherwise, LGTM.

[OR13]

@msporny accepted changes, pinged for re-review.

[selfissued]

I'm good with this version being merged. Thanks for doing this, @OR13 .

[peacekeeper]

I'm not sure I fully understand the second sentence of the advisement, but I think it's trying to prevent developers or users of certain libraries from including private key data in the DID document. This is good.

[kdenhartog]

related side note, are we planning to update json-ld suite contexts to support publicKeyMultibase (and publicKeyJwk)?

[msporny]

Editorial, multiple reviews, changes requested and made, no objections, merging.