w3c-ccg / did-method-key

DID (Decentralized Identifier) did:key method for embedding keys in DID urls
https://w3c-ccg.github.io/did-method-key
Other
18 stars 12 forks source link

Remove Ed25519 -> X25519 conversion from spec #39

Open OR13 opened 3 years ago

OR13 commented 3 years ago

This would eliminate concern regarding the security of the operation.

and make ed25519 keys like every other key type.

dmitrizagidulin commented 3 years ago

@OR13 is the proposal to not have the keyAgreementKey purpose, in did:key DID docs?

mprorock commented 3 years ago

big fan of this - would simplify things and improve security

dmitrizagidulin commented 3 years ago

@OR13 @mprorock Would making the keyAgreementKey generation part optional solve your concerns?

dmitrizagidulin commented 3 years ago

On further consideration, I'd be +1 to removing keyAgreement section from Ed25519-based did:key documents. (In case people need ephemeral DID-based key agreement, they can generate an X25519 based did:key directly.)

OR13 commented 3 years ago

^ exactly... for example, here is how we generate X25519 keys today:

{
  "@context": [
    "https://www.w3.org/ns/did/v1",
    "https://w3id.org/security/suites/jws-2020/v1"
  ],
  "id": "did:key:z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7",
  "verificationMethod": [
    {
      "id": "did:key:z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7#z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7",
      "type": "JsonWebKey2020",
      "controller": "did:key:z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7",
      "publicKeyJwk": {
        "kty": "OKP",
        "crv": "X25519",
        "x": "xlONxp-pXBxR1vErZ9Mywv1GLXLzxOK5IsdloQkNckw"
      }
    }
  ],
  "keyAgreement": [
    "did:key:z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7#z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7"
  ]
}

Here would be the new structure for Ed25519:

{
  "@context": [
    "https://www.w3.org/ns/did/v1",
    "https://w3id.org/security/suites/jws-2020/v1"
  ],
  "id": "did:key:z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR",
  "verificationMethod": [
    {
      "id": "did:key:z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR#z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR",
      "type": "JsonWebKey2020",
      "controller": "did:key:z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR",
      "publicKeyJwk": {
        "kty": "OKP",
        "crv": "Ed25519",
        "x": "FmdIIZE4B0glkSA_Fd_SdAoaVSRSD8Js5BRMXmhDr-A"
      }
    }
  ],
  "assertionMethod": [
    "did:key:z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR#z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR"
  ],
  "authentication": [
    "did:key:z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR#z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR"
  ],
  "capabilityInvocation": [
    "did:key:z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR#z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR"
  ],
  "capabilityDelegation": [
    "did:key:z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR#z6MkfxiMhhrtPtWpAaCu2nmMY1wtUe2VYja3Y8hRAhZ4qHVR"
  ]
}
OR13 commented 3 years ago

imo how you derive a public key or private key is out of scope for the spec.... that includes curve conversion, mnemonics, or key generation functions that take passwords.

dlongley commented 3 years ago

@mprorock,

big fan of this - would simplify things and improve security

For the use cases that have no need of an integrated key agreement key and for those use cases where its mere presence may cause some regulatory trouble (unknown), it would simplify things. For use cases where an integrated key agreement key is beneficial, it will complicate things.

Regarding "improve security", however, I have to disagree, if the current literature is to be believed (that sharing the same key for both ed25519 and x25519 operations has been proven secure). Sure, it may be the case that certain governments would not accept the same key to be used for both of these operations, but that is merely a matter of current policy.

@dmitrizagidulin,

(In case people need ephemeral DID-based key agreement, they can generate an X25519 based did:key directly.)

Being able to generate a key is not the same thing as having a verification relationship in a DID document authorizing its use for a specific purpose. Removing keyAgreement would be removing that statement and people should understand the consequences of that: it either requires did:key DIDs and DID documents to be treated differently from other methods that support keyAgreement (when keyAgreement is in use by an application) or it requires keyAgreement to be ignored in DID documents from all DID methods because some other method of authorization is put into place.

A large part of the interoperability story around DIDs is that applications can treat the DID documents the same, regardless of the method used. This provides a separation of concerns: use a trusted DID resolver and once you've got a DID document, you can rely on its verification relationships and methods from there and ignore the DID method.

dlongley commented 3 years ago

To be clear on my position here:

If the use cases just aren't there for key agreement keys for ed25519 did:key DIDs, then removing keyAgreement makes sense -- provided that x25519 did:key DIDs are created to cover the key agreement use cases. It does necessarily mean that any binding between an ed25519 did:key and an x25519 did:key will have to be done separately -- which would not be necessary for any other DID method that supports keyAgreement (my interop point above stands and it should be considered carefully, IMO).

dlongley commented 3 years ago

A use case that needs to be considered here (and whether or not did:key will be an acceptable DID method for it) is:

You are required to use a single DID for which you can both authenticate (or take an action, i.e., invoke a capability) and do encrypted communications/data.

The fact that did:key only supports one key is incidental here, the application doesn't really care. It only cares about there being a single DID. It's just that, if you want to use did:key in this use case, it will be the same underlying key. Another DID method may not have that constraint.

So -- are there use cases such as the one above where did:key will no longer fit in with other DID method options? My understanding was that the whole DIDComm approach was such a use case and, if so, it would further constrain one's choice of DID method when using DIDComm. I could be wrong about this, however.

dlongley commented 3 years ago

Another option to consider, would be to register another public key type in the multicodec table that means: "an ed25519 public key that can be transformed using a birational map for use as an x25519 key". So there would be a different did:key option for those that need to use keyAgreement with their DID in a way that interoperates with other DID methods. The codec choices for creating a 25519-style did:key would then be ed25519, x25519, and ed25519+x25519.

OR13 commented 3 years ago

yep, if the multicodec says "ed25519 public key that will be converted to x25519 public key" thats fine... but still likely to trigger security folks who don't want to see that happening.

worth noting that RSA / P-256 and other key types already support both key agreement and signatures... using "snowflake" did key formats should be optional imo...

another option is to do the conversion before encoding the did, and handle this like we handle bls12381 g1 + g2.

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

dmitrizagidulin commented 3 years ago

@OR13

worth noting that RSA / P-256 and other key types already support both key agreement and signatures

Waiiit, the whole point here is that this conversation got started because of the security "best practice" that one should not use the same key for both purposes. (Or derive the key, either.) So if we shouldn't do that with Ed25519 keys, then same applies to RSA and P-256.

OR13 commented 2 years ago

@dmitrizagidulin

So if we shouldn't do that with Ed25519 keys, then same applies to RSA and P-256.

No, because we are not using Ed25519 for keyAgreement today, we are converting to X25519 and then using that key...

You can't have it both ways...

Either you think it's a good idea to have a key for a single purpose, or you think it's a good idea to be honest about what purposes a key can support.

I think we should expose what is supported by a SINGLE KEY TYPE, and nothing more....

I would also be ok making did key look like this:

{
  "@context": [
    "https://www.w3.org/ns/did/v1",
    "https://w3id.org/security/suites/jws-2020/v1"
  ],
  "id": "did:key:z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7",
  "verificationMethod": [
    {
      "id": "did:key:z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7#z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7",
      "type": "JsonWebKey2020",
      "controller": "did:key:z6LSq2MnTYfm1MqSV6uy9d2DTLLAvNqU1Bwn5u6SrhXGePQ7",
      "publicKeyJwk": {
        "kty": "OKP",
        "crv": "X25519",
        "x": "xlONxp-pXBxR1vErZ9Mywv1GLXLzxOK5IsdloQkNckw"
      }
    }
  ],
}

since interpreting 'verification relationships as purposes' and then allowing everything that is possible to be expressed seems to be a security pattern failure... or a reflection of practical reality... (ED25519 for assertion and authentication).

since did key is deterministic, and multiformats don't encode purposes in the key type (hmm i wonder why?).... we would appear to have 2 options:

  1. a did key representation will encode all possible registered purposes.
  2. a did key representation will not encode purposes.

since 2 would make did:key useless, we are left with 1.

Ed25519 should be allowed for keyAgreement IF and ONLY IF, it's possible to use it for that purpose.

NEW_KEY_TYPE should be allowed for PURPOSE IF and ONLY IF, it's possible to use it for that purpose.

Converting between types for specific keys should not be a legal way to circumvent 1... its complexity, attack surface, and implementation burden.

dlongley commented 2 years ago

@OR13,

No, because we are not using Ed25519 for keyAgreement today, we are converting to X25519 and then using that key...

You can't have it both ways...

Hmm, to me, calling it a different key and saying that there's a security-related concern because it's the same key is having it both ways.

Factually, it is the same key, just represented differently (a point on one curve vs. a point on another birationally equivalent one). It is precisely the fact that it is the same key that is driving the security-related concern from some people (though there's a security proof suggesting the concern is unwarranted). If it were actually a different key, then it would be the "snowflake" (as you describe) but the security-related concern would be dropped.

Perhaps the argument is that it's a snowflake because the representation changes. Maybe that's what you were going for. But I don't think we should say that having a different representation for key agreement makes something a "snowflake" that should be discouraged. We don't know what kind of curves/other math primitives might be of use in the future to make the same key material be used more efficiently in different protocols.

dlongley commented 2 years ago

@OR13,

Ed25519 should be allowed for keyAgreement IF and ONLY IF, it's possible to use it for that purpose.

NEW_KEY_TYPE should be allowed for PURPOSE IF and ONLY IF, it's possible to use it for that purpose.

Converting between types for specific keys should not be a legal way to circumvent 1... its complexity, attack surface, and implementation burden.

If we can get an ed25519+x25519 key type into multiformats, then I think that's the path forward here as it would work with the above and give us the consistency we want.

OR13 commented 2 years ago

Hmm, to me, calling it a different key and saying that there's a security-related concern because it's the same key is having it both ways.

Sure, but thats an argument to have with DJB and the folks who decided Ed25519 != Curve25519... they are different keys, their bytes are different.... they have different id values... we need them to, in order to use them properly...

@dlongley

do you think ed25519+x25519 should be encoded as ed25519 with allowed derivations?

or should it be encoded as ed25519 pub + x25519 pub...

this matters if resolvers refuse to implement derivation, the latter representation would still allow them to resolve... and... it would also support:

seed 1 - > Ed25519 k1 seed 2 -> X25519 k2

did:key:(ed25519+x25519) + k1 + k2

Whereas today:

seed 1 - > Ed25519 k1 -> X25519 k2 is happening.... this would probably be valuable when working with HSMs.

dlongley commented 2 years ago

@OR13,

Hmm, we may end up needing to support both approaches but perhaps we should only start out with the first one. We should probably use a different multicodec value for each one (or include some value that indicates whether there's more than one set of key bytes). The latter approach would probably have the benefits you suggest, but I'd be concerned about having to think through all of the implications of people starting to use N many keys in did:key DIDs.

tmarkovski commented 2 years ago

The ephemeral nature of this method favors convenience features like this, when security measures allow. In that sense, I do support the idea of having key agreement in ed25519.

OR13 commented 2 years ago

But the method is weakened by "snowflake edge cases"... you still have the ability to use X25519 keys with did:key... and you still have the ability to do key agreement and signature with P-384.

Security reviews of the method will object to these edge cases... as they should.

OR13 commented 2 years ago

We will be removing key conversion from our libraries shortly.