oed commented 4 years ago

I'm indeed only supporting x25519 in the did-jwt library. I'm going to stay far away from using secp256k1 for encryption as per @christianlundkvist advise: https://github.com/christianlundkvist/blog/blob/master/2020_05_26_secp256k1_twist_attacks/secp256k1_twist_attacks.md Note however that the did-jwt library has the functionality (in my PR) to provide custom encrypters/decrypters for JWEs.

OR13 commented 4 years ago

@oed that link it dead... you can do ECDH with secp256k1.... but I agree, its definitely not a common thing to see... https://crypto.stackexchange.com/questions/60515/ecdh-over-secp256k1-implementation

I am most interested in ensuring standards conformance for the JWE representations you are intending to support.

Would you consider helping DIF define https://github.com/decentralized-identity/did-jose-extensions

So we can all do JWE's the same?

This is mostly just documenting examples, and providing recommendations... I would really prefer for this not to be an "ethereum" thing, and instead, have ethereum use "standards".

oed commented 4 years ago

@OR13 Updated the link. Not sure what happened there.

I am most interested in ensuring standards conformance for the JWE representations you are intending to support.

This EIP should really be agnostic as to what representation is being used. It should ultimately be up to the wallets to decide which formats they provide.

Happy to help with did-jose-extensions!

I would really prefer for this not to be an "ethereum" thing

They way I wrote this standard it should work with both the ethereum json-rpc. However Pure DID wallets should be able to only implement these methods independently of ethereum.

oed commented 4 years ago

Updated the EIP. Let me know if I missed something or if you have additional suggestions :)

OR13 commented 4 years ago

why is version-id required? and how is its value calculated?

a signature from a specific version is similar to a signature from an issuer which never does key rotation... it will always verify.... defeating the purpose of many signatures.... however, there are cases you only care that the signature was valid at the time in the past when the keys were active, and you don't care if the signature valid for the current version.

Consider that an attacker will have "active keys for a version" if they compromise any version, so rotation no longer insulates the controller from being impersonated.

also version-id is not implemented by many did methods, so folks may be less familiar with it.

Since this is an EIP, it might be helpful to provide concrete data model examples using:

awoie commented 4 years ago

Interesting @kdenhartog what would the payload and header look like if we would use JWM? Would also be interested in input from @awoie here!

@oed DIDComm v2 as it is proposed right now, won't use compact serialization.

Example JWM (JWE) that is used in DIDComm v2 -- JWS and plain-text will be also supported:

{
    "protected": "<encoded protected header from above>",
    "recipients": [
        {
            "header": {
                "alg":"ECDH-1PU", //authcrypt
                "kid": "did:ex:recipient?version=x#key_id"
                "apu":"QWxpY2U",
                "apv":"Qm9i",
                "epk": {
                    "kty":"EC",
                    "crv":"X25519",
                    "x":"gI0GAILBdu7T53akrFmMyGcsF3n5dO7MmwNBHKW5SV0",
                    "y":"SLW_xSffzlPWrHEVI30DHM_4egVwt3NQqeUD7nMFpps"
                },
                "encrypted_sender": "<encrypted({'kty':'OKP','crv':'X25519','x':'3p7bfXt9wbTTW2HC7OQ1Nz-DQ8hbeGdNrfx-FG-IK08'})>"
            },
            "encrypted_key": "<cek, encrypted to kid>"
        }
    ]
    "iv": "<generated iv>",
    "ciphertext": "<JWM encrypted with cek>",
    "tag": "<generated tag>"
}

Example payload (other claims might be included):

{
  "id": "<message id>",
  "type": "<message type>",
  "from": "did:ex:sender",

  // contains id of the first message in the thread
  "thread_id": "<thread id>",

  "body": { 
      // sub-protocol message according to <message type>
  }
}

awoie commented 4 years ago

@oed @OR13 @kdenhartog imo, the did_* API should also support JSON serialization, or if the first version does not support other serialization options than compact serialization, then i would rename the API, or introduce a place holder parameter that indicates the type and have compact as the default option. further, it would be great if we could continue the work on this to add support for JWM and more specifically JSON serialized JWE which are used in DIDComm v2.

another point that i want to make is to add an did_encryptJWE for authcrypt. it would be specifically great to get feedback on the usefulness from @kdenhartog on this.

awoie commented 4 years ago

@oed did_authenticate is missing the aud parameter, or will this be derived from paths?

oed commented 4 years ago

@OR13

why is version-id required?

Thanks, it shouldn't be. Will change the wording.

Consider that an attacker will have "active keys for a version" if they compromise any version, so rotation no longer insulates the controller from being impersonated.

This is not always true. For example in Ceramic each update that is made is anchored in a blockchain. This makes it possible to determine if a key that signed the update was valid at the time the anchor was made.

Not sure how blockchainAccountId is relevant here. Mind elaborating?

@awoie good point about serialization. Meant to change it to general, will make another update!

another point that i want to make is to add an did_encryptJWE for authcrypt. it would be specifically great to get feedback on the usefulness from

Can you please provide a specification for this? I don't know how that should work.

did_authenticate is missing the aud parameter, or will this be derived from paths?

Good point, will add the aud param there.

Have the updates here: https://github.com/oed/EIPs/blob/chore/additional-2844-udates/EIPS/eip-2844.md Will make a PR after some more time for feedback 👍

OR13 commented 4 years ago

This makes it possible to determine if a key that signed the update was valid at the time the anchor was made.

right, but when that signature is the first evidence of a compromised key, and the signature remains valid forever (because its pinned to a version), this removes a lot of the value of dids and key rotation...

id_token and access_token are not signed with keys that were "once valid"... they are signed with keys that are "currently" valid... and checking that they are still valid is part of verifying them.

If you were to sign an id_token with a pinned version it would be valid forever.... even after that key was compromised.

blockchainAccountId

^ This is a generic way of representing ethereum addresses, and bitcoin addresses (and others) and its part of DID Core. ethereumAddress is not part of DID Core.

awoie commented 4 years ago

If you were to sign an id_token with a pinned version it would be valid forever.... even after that key was compromised.

I would disagree with that. The id_token would remain valid until it has expired indicated by the exp claim.

awoie commented 4 years ago

This makes it possible to determine if a key that signed the update was valid at the time the anchor was made.

right, but when that signature is the first evidence of a compromised key, and the signature remains valid forever (because its pinned to a version), this removes a lot of the value of dids and key rotation...

IMO, a mix might be desired. For example, if you look at the SSI use case. An issuer provides a VC to a subject/holder. The VC could contain the DID of the issuer + version property. If think it is desirable to being able to verify that the VC was valid at least once. In case the issuer key got compromised, then I would expect that the issuer would revoke the credential through the revocation method of their choice (e.g., through some onchain accumulator). The VC would need to contain a credentialStatus property of course. On the other hand, in case we are dealing with subject DIDs, then it probably makes no sense to specify a dedicated version as the verifier is interested in verifying proof of control (or the domain proof) at the time the holder provides the W3C VP.

Update: Another approach would be to include separate version field in the proof property of the VC instead of using the version parameter directly in the DID URL. That version shows which version of the DID was used to cover the case mentioned above.

OR13 commented 4 years ago

If you were to sign an id_token with a pinned version it would be valid forever.... even after that key was compromised.

I would disagree with that. The id_token would remain valid until it has expired indicated by the exp claim.

sure, but what about VCs that have no exp?

Rotating keys is a weapon of last resort for credential revocation...

oed commented 4 years ago

sure, but what about VCs that have no exp?

Imo this should not be limited to VCs. Maybe it makes sense to add an option to the createJWS to request use of the version-id param?

For example in ceramic all previous documents would be invalidated if you rotate keys for whatever reason, which is definitely not desired. New keys can always invalidate changes made by old keys.

kdenhartog commented 4 years ago

another point that i want to make is to add an did_encryptJWE for authcrypt. it would be specifically great to get feedback on the usefulness from

Can you please provide a specification for this? I don't know how that should work.

Authcrypt is effectively ECDH-1PU

oed commented 4 years ago

Thanks @kdenhartog I meant more if someone could provide a description of how the json-rpc method for it would work :)

awoie commented 3 years ago

Thanks @kdenhartog I meant more if someone could provide a description of how the json-rpc method for it would work :)

There is an early version of a JWM implementation that uses authcrypt: https://github.com/TelegramSam/jose/pull/1/files

JWM.encrypt(attributes, recipients, key[, options])`

Serializes and encrypts the attributes as JWM using the provided private or symmetrical key. The Algorithm
that will be used to sign with is either provided as part of the 'options.algorithm',
'options.header.alg' or inferred from the provided `<JWK.Key>` instance.

It further says:

<JWK.Key> instances are recommended for performance purposes when re-using the same key for every operation. When used, JWM will be authcrypted from the key to the recipients. When omitted, the JWM will be anoncrypted to the recipients.

oed commented 3 years ago

@awoie Do you have an opinion on how the json-rpc method should look like? I don't have enough insight to design it.

awoie commented 3 years ago

@oed I will provide an example this week.

awoie commented 3 years ago

@oed This is the proposal to update eip2844 to support authcrypt but I'm wondering if @kdenhartog also thinks this is useful.

AuthcryptJWE

Encrypts a message using ECDH-1PU to an array of recipients. Authenticated encryption provides only authenticity and not the stronger security properties of non-repudiation or third-party verifiability. This can be an advantage in applications where privacy, anonymity, or plausible deniability are goals.

For content encryption of the message, the following algorithm MUST be used:

Algorithm(JWA)	Description
XC20P	XChaCha20Poly1305

Method: did_authcryptJWE

Params:

payload - the payload to encrypt, json object or base64url encoded string.
did - the DID or DID URI that should authenticate the message. If a DID is used, it MUST contain a keyAgreement section in the DID Document that contains a X25519 key. If multiple key agreement keys are contained in the DID Document, the first key will be used to authenticate the message. If a DID URI is used the DID URI MUST point to a key that is directly or indirectly referenced in the keyAgreement section of the DID Document.
recipients - A json array of type string. Every entry refers to a public key of one recipient of the JWE. Each public key MUST be encoded as base58 multibase.

Returns:

A JWE with JSON general serialization, string. The kid MUST be the DID URI that is expected to decrypt the message.

oed commented 3 years ago

Looks nice overall @awoie. Some thoughts:

the payload to encrypt, json object or base64url encoded string.

Can we be more opinionated here please? I would prefer just one option, probably base64url.

did - the DID or DID URI that should authenticate the message.

So if I understand correctly this did would be used as the skid in the JWE header? The one thing I would add here is to have an optional parameter that adds a versionId or a versionTime query param in the resulting skid. The reason for this is that you need to be able to resolve the correct version of the DID document if the key that was used was revoked.

The kid MUST be the DID URI that is expected to decrypt the message.

Where would the kid come from here? Seems like the wallet side would not have access to this information? Also I assume that there would be a kid in the unprotected header of each recipient?

awoie commented 3 years ago

@oed right, I will provide an update, we should also talk about cty, apu and apv.

oed commented 3 years ago

@awoie Can you remind me what cty is?

apu and apv are not strictly required by the JWE spec afaik.

kdenhartog commented 3 years ago

@oed This is the proposal to update eip2844 to support authcrypt but I'm wondering if @kdenhartog also thinks this is useful.

AuthcryptJWE

Encrypts a message using ECDH-1PU to an array of recipients. Authenticated encryption provides only authenticity and not the stronger security properties of non-repudiation or third-party verifiability. This can be an advantage in applications where privacy, anonymity, or plausible deniability are goals.

For content encryption of the message, the following algorithm MUST be used: Algorithm(JWA) Description XC20P XChaCha20Poly1305

Method: did_authcryptJWE

Params:
* `payload` - the payload to encrypt, json object or base64url encoded string.

* `did` - the DID or DID URI that should authenticate the message. If a DID is used, it MUST contain a `keyAgreement` section in the DID Document that contains a X25519 key. If multiple key agreement keys are contained in the DID Document, the first key will be used to authenticate the message. If a DID URI is used the DID URI MUST point to a key that is directly or indirectly referenced in the `keyAgreement` section of the DID Document.

* `recipients` - A json array of type string. Every entry refers to a public key of one recipient of the JWE. Each public key MUST be encoded as base58 multibase.
Returns:
* A JWE with JSON general serialization, string. The `kid` MUST be the DID URI that is expected to decrypt the message.

Yeah that would be really useful in my opinion. That's effectively merging everyone on the envelope of DIDCommV2 which would be amazing.

kdenhartog commented 3 years ago

@awoie Can you remind me what cty is?

apu and apv are not strictly required by the JWE spec afaik.

cty is going to be used to determine which version of payload we're using. Keep an eye on this RFC: https://github.com/hyperledger/aries-rfcs/blob/82ace6001f97552f05d3da5472bcf9dbf7726a05/features/0587-encryption-envelope-v2/README.md#media-type

As for apu / apv that's correct that they're not strictly for a generic JWE, but for the 1PU they appear to be required for the HKDF function parameters PartyUInfo and PartyVInfo. We could decide that we want these values to be static or match another property (thinking maybe the did keyId which is passed in the kid property) so that we don't need to explicitly include those properties and save a bit of space.

D4nte commented 3 years ago

I think I had a similar thought to https://github.com/ethereum/EIPs/issues/2845#issuecomment-674038263.

For did_decryptJWE, we propose to bypass the user permission:

If the paths property is present in the decrypted data it should contain an array of string paths. If one of the these path prefixes matches with one of the path prefixes the user has already granted permission for then decryption should happen automatically without any user confirmation.

Would it makes to apply the same logic for did_createJWS?

Imagine a chat application where a signature is attached to each message, a similar logic would enable this use case without having to ask the user for signature authorization every time.

Or am I missing the point of the did_createJWS API?

kdenhartog commented 3 years ago

Thinking about this more after coming back to look at this a few months later, I get the feeling we're conflating a few different layers here and if we decompose these things a bit more it would make the DID functionality all the more usable.

1st, I think we should add a DID management layer for normal CRUD ops so that the DID can be managed independent of it's usage.

2nd, I think we need to look at the JOSE layer as one of many usage layers which depends on the ~state of DIDs under management.~ This is a bad assumption because the RPC is intended to be stateless

3rd, I think the conflation of a ABAC authz model built into the usage layer is going to inherently couple things in weird ways that probably aren't correct. I need to understand the different trust boundaries a bit more on the RPC to get a better understanding and see if it makes sense to have the authz model combined with the usage or if it makes sense to separate it. When I get some time in the next week or two I'll do some research on this and see if my thinking here is heading in the right direction or if I'm off basis.

github-actions[bot] commented 2 years ago

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

github-actions[bot] commented 2 years ago

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

ethereum / EIPs

Discussions for EIP: Add DID related methods to the JSON-RPC #2845

AuthcryptJWE

AuthcryptJWE