w3c-ccg / ld-cryptosuite-registry

REGISTRY: Linked Data Keys Registry (managed by W3C Credentials Community Group)
https://w3c-ccg.github.io/ld-cryptosuite-registry/
Other
12 stars 4 forks source link

feat: add bbs bls signature and key definitions to registry #35

Closed tplooker closed 4 years ago

tplooker commented 4 years ago

Adds the BBS and BLS signature and key definitions to the registry defined in https://w3c-ccg.github.io/ldp-bbs2020/

PR is dependent on https://github.com/w3c-ccg/ldp-bbs2020/pull/34 being merged ahead for the Bls12381G1Key2020 definition

tplooker commented 4 years ago

@msporny while I see your point, at present I think your solution around featuring Verification in the name as a blanket rule across asymmetric key pairs removes one problem and instead adds developer confusion. I do not think hard coding the implied usage of the key into the name is the correct way to go here. If your gripe is around public vs private material in the representation then that is what we should be bubbling to the surface in the name. See here for some more commentary, my opinion expressed there holds true here, happy to consider public in the name.

msporny commented 4 years ago

If your gripe is around public vs private material in the representation then that is what we should be bubbling to the surface in the name.

Yes, that's one of the concerns. it was not obvious that JsonWebKey2020 was not going to hold private key information. I've had several discussions where developers assumed that was true, which means, the name is misleading. So, at a minimum, it should be PublicJsonWebKey2020 (or something to that effect). I suggest since the JWK community has chosen publicKeyJwk, that you align the name to that... PublicJwk2020, although that's still not specific enough... because that allows people to use the key for both verifying digital signatures and key agreement, where the intent may have only been for verifying digital signatures (but you don't know because the type name is ambiguous).

tplooker commented 4 years ago

I suggest since the JWK community has chosen publicKeyJwk, that you align the name to that... PublicJwk2020, although that's still not specific enough... because that allows people to use the key for both verifying digital signatures and key agreement, where the intent may have only been for verifying digital signatures (but you don't know because the type name is ambiguous).

My reluctancy here to include Verification in the name is where does it begin and end? Why not go a step further and create Bls12381G1AssertionMethodKey2020? As soon as you try to guide the key usage from the name I think you end up in very murky territory, territory that really most developers don't know nor care about.

Let's take an example of how things could play out, say we define the representation of the NIST P-256 Curve Key, a key that is appropriate for both keyAgreement (ECDH) and digital signatures (ECDSA). If I wanted a NIST P-256 Curve Key in my DID document that is valid for both keyAgreement (indicated by featuring under the keyAgreement section of the did doc) and signing/issuing VC's (indicated under the assertionMethod section of the did doc) I would need to define the same key twice of a different type, leading to a did document looking something like the following.

{
  "@context": ["https://www.w3.org/ns/did/v1", "https://w3id.org/security/v1"],
  "id": "did:example:123456789abcdefghi",
  "publicKey": [{
    "id": "did:example:123456789abcdefghi#keys-1",
    "type": "NistP256VerificationKey2020",
    "controller": "did:example:pqrstuvwxyz0987654321",
    "publicKeyBase58": "H3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV"
  }, {
    "id": "did:example:123456789abcdefghi#keys-2",
    "type": "NistP256KeyAgreementKey2020",
    "controller": "did:example:123456789abcdefghi",
    "publicKeyBase58": "H3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV"
  }],
  "keyAgreement": [ "did:example:123456789abcdefghi#keys-2" ],
  "assertionMethod" [ "did:example:123456789abcdefghi#keys-1" ]
}

Where the did:example:123456789abcdefghi#keys-1 and did:example:123456789abcdefghi#keys-2 are exactly the same key, but defined twice. Furthermore, we have already agreed that NistP256VerificationKey2020 and NistP256KeyAgreementKey2020 are in fact both public keys, indicated by their inclusion under the publicKey section, so why not make that the basis of the convention? Which would lead to a far simpler did document representation like the following.

{
  "@context": ["https://www.w3.org/ns/did/v1", "https://w3id.org/security/v1"],
  "id": "did:example:123456789abcdefghi",
  "publicKey": [{
    "id": "did:example:123456789abcdefghi#keys-1",
    "type": "NistP256PublicKey2020",
    "controller": "did:example:123456789abcdefghi",
    "publicKeyBase58": "H3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV"
  }],
  "keyAgreement": [ "did:example:123456789abcdefghi#keys-1" ],
  "assertionMethod" [ "did:example:123456789abcdefghi#keys-1" ]
}

To round out, most of the argument I have seen here for introducing some convention is to prevent people from putting private keys in documents that are meant to be public, hence that is what the convention should be around. Trying to prevent people from improperly using a key by coding some hint about its usage into the name I don't think is helpful and because of the added confusion may in fact be harmful for security and interop. If we can reach consensus around how we want to hand LD key pairs in future with common conventions, then I am happy to rename to Bls12381G1PublicKey2020

OR13 commented 4 years ago

Your internal representation of keys is also worth considering here....

{
    "type": "Ed25519VerificationKey2018",
    "id": "#z6MkuMqmxrKQf7jqrDusQru1MNSxBTfJ9bqD1MUrCeaKEaBt",
    "controller": "did:key:z6MkuMqmxrKQf7jqrDusQru1MNSxBTfJ9bqD1MUrCeaKEaBt",
    "publicKeyBase58": "FuajNc4yKaFNjj5AjHwAWGtxMtPSjiarKLZvNNcJKMQW",
    "privateKeyBase58": "51gNhEN5gnoZSJ1PDcofAWpevyHGAqqtbkEHohZaokduNu3BtdE26t1EwJDdGk3qhqc8RsMrsXzNRa3XSLG2aTzz"
  }

compare to

{
    "type": "Ed25519PublicKey2018",
    "id": "#z6MkuMqmxrKQf7jqrDusQru1MNSxBTfJ9bqD1MUrCeaKEaBt",
    "controller": "did:key:z6MkuMqmxrKQf7jqrDusQru1MNSxBTfJ9bqD1MUrCeaKEaBt",
    "publicKeyBase58": "FuajNc4yKaFNjj5AjHwAWGtxMtPSjiarKLZvNNcJKMQW",
    "privateKeyBase58": "51gNhEN5gnoZSJ1PDcofAWpevyHGAqqtbkEHohZaokduNu3BtdE26t1EwJDdGk3qhqc8RsMrsXzNRa3XSLG2aTzz"
  }

^ this is obviously wrong.

OR13 commented 4 years ago

The correct thing to do from a semantic and security perspective would be to define 3 types...

where publicKeyBase58 is required in Ed25519KeyPair2018 and Ed25519PublicKey2018 and Ed25519PrivateKey2018.

and where privateKeyBase58 is required in Ed25519KeyPair2018 and Ed25519PrivateKey2018 and FORBIDDEN in Ed25519PublicKey2018....

Relying on a string for types is terrible security design.

BTW, a type in JSON is any set of properties that are required, and their required values, and no additional properties.

tplooker commented 4 years ago

@msporny with the consensus reached last week around the did spec registries is this ready to merge or shall we arrange a call to discuss the conventions around keys we will be using going forward?

msporny commented 4 years ago

@msporny with the consensus reached last week around the did spec registries is this ready to merge or shall we arrange a call to discuss the conventions around keys we will be using going forward?

The consensus was that there was no consensus and that the people that care about the topic, which now is not the DID WG, need to get together and make a decision.

We can merge as long as you know that when we all come up with a consistent naming scheme, that we will rename all the things. So, the only question is if you want to have that discussion first and wait 1-2 weeks, or if you want to merge this now and then come back and possibly change it in 1-2 weeks.

OR13 commented 4 years ago

Im in favor of merging, and then updating... everything in get can be updated, thats what git was made for :)