Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC 0019: Updating to JWE compliant data model #133

Closed
kdenhartog opened this issue Jul 18, 2019 · 87 comments
Closed

RFC 0019: Updating to JWE compliant data model #133

kdenhartog opened this issue Jul 18, 2019 · 87 comments
Assignees
Labels
DIDComm Related enhancement New feature or request

Comments

@kdenhartog
Copy link
Contributor

Recently there's been discussion to update this work to be fully JWE compliant. One of the proposed solutions that's come up is to generate the following format:

{
    "protected": base64url({
        "typ": "prs.hyperledger.aries-auth-message",
        "alg": "ECDH+XC20PKW",
        "enc":"XC20P"
    }),
    "recipients": [
        {
            "encrypted_key": "base64url(encrypted CEK)",
            "header": {
                "kid": "base58(Recipient vk)",
                "iv": "base64url(CEK encryption IV)",
                "pk": "compactJWE(Sender PK)"
            }
        },
        {
            "encrypted_key": "base64url(encrypted CEK)",
            "header": {
                "kid": "base58(Recipient vk)",
                "iv": "base64url(CEK encryption IV)",
                "pk": "compactJWE(Sender PK)"
            }
        }
    ],
    "aad": "base64url(sha256(concat('.',sort([recipients[0].kid, recipients[n].kid]))))",
    "iv": "base64url(content encryption IV)",
    "ciphertext": "base64url(ciphertext)",
    "tag": "base64url(AEAD Authentication Tag)"
}

Exploded example of compactJWE(Sender PK):
{
    "protected": base64url({
        "iv": "base64url(CEK encryption IV)",
        "epk": "Ephemeral JWK",
        "typ": "jose",
        "cty": "jwk+json",
        "alg": "ECDH-ES+XC20PKW",
        "enc":"XC20P"
    }),
    "encrypted_key": "base64url(encrypted CEK)",
    "iv": "base64url(content encryption IV)",
    "ciphertext": "base64url(Encrypted Sender vk as JWK)",
    "tag": "base64url(AEAD Authentication Tag)"
}

Here's a provided example of what this would look like:

{
    "protected": "eyJ0eXAiOiJwcnMuaHlwZXJsZWRnZXIuYXJpZXMtYXV0aC1tZXNzYWdlIiwiYWxnIjoiRUNESCtYQzIwUEtXIiwiZW5jIjoiWEMyMFAifQ",
    "recipients": [
        {
            "encrypted_key": "whpkJkvHRP0XX-EqxUOHhHIfuW8i5EMuR3Kxlg5NNIU",
            "header": {
                "kid": "5jMonJACEPcLfqVaz8jpqBLXHHKYgCE71XYBmFXhjZVX",
                "iv": "tjGLK6uChZatAyACFzGmFR4V9othKN8S",
                "tag": "ma9pIjkQuzaqvq_5y5vUlQ",
                "pk": "eyJpdiI6IldoVGptNi1DX2xiTlQ4Q2RzN2dfNjdMZzZKSEF3NU5BIiwiZXBrIjp7Imt0eSI6Ik9LUCIsImNydiI6IlgyNTUxOSIsIngiOiJnNjRfblJSSFQyYk1JX1hjT1dHRTdJOGdQcU1VWTF4aUNub2J0LVhDUkNZIn0sInR5cCI6Impvc2UiLCJjdHkiOiJqd2sranNvbiIsImFsZyI6IkVDREgtRVMrWEMyMFBLVyIsImVuYyI6IlhDMjBQIn0.4zUt5tOOlcQWskJqxfMi0tNsfUCAzb5_PDfPqQ1h0Vw.xYkeEXV1_cSYFEd6UBMIfl8MWQfHaDex.XSNKTRXye5-iSXQ-aS_vQVZNEgFE6iA9X_KgSRMzihQBMoI1j4WM3o-9dMT9TeSyMvdq3gXt1NpvLdZHpJplahhk3mxMZL-vawm5Prtf.H7a5N-dggwdesjHyJCl06w"
            }
        },
        {
            "encrypted_key": "dDHydlp_wlGt_zwR-yUvESx9fXuO-GRJFGtaw2u6CEw",
            "header": {
                "kid": "TfVVqzPT1FQHdq1CUDe9XYcg6Wu2QMusWKhGBXEZsosg",
                "iv": "7SFlGTxQ4Q2l02D9HRNdFeYQnwntyctb",
                "tag": "9-O6djpNAizix-ZnjAx-Fg",
                "pk": "eyJpdiI6IkV6UjBFaVRLazJCT19oc05qOVRxeU9PVmVLRFFPYVp1IiwiZXBrIjp7Imt0eSI6Ik9LUCIsImNydiI6IlgyNTUxOSIsIngiOiJoU1g1NGt5ZTdsd0pBdjlMaUplTmh4eFhaV1N0M3hLSDBXUmh6T1NOb1c0In0sInR5cCI6Impvc2UiLCJjdHkiOiJqd2sranNvbiIsImFsZyI6IkVDREgtRVMrWEMyMFBLVyIsImVuYyI6IlhDMjBQIn0.qKmU5xO8Z1ZtRBWEjEMixb5VZG0mrY0LnjUGjLqktwg.EG-VOZSC2vLdoO5l2_A37IYvdXCckLZp.D4kgD6bYL1YfXyApk5ESKE2sc8TUiO-QGBtY-M5hcV_F88JPZdsi53Qofxk02ZxPHJZK-abDy45pIMH6-KUMDfE6WKhW3nPQhydPYutv.0SO4VjM8sDH-wGHcEpinTg"
            }
        }
    ],
    "aad": "OGY5ZDIxMDE3YTQ4MTc4YWE5MTk0MWQyOGJmYjQ1ZmZmMTYzYTE3ZjUxYjc4YjA3YTlmY2FlMmMwOTFlMjBhZg",
    "ciphertext": "x1lnQq_pZLgU2ZC4",
    "tag": "2JgOe9SRjJXddT9TyIjqrg",
    "iv": "fDGEXswlWXOBx6FxPC_u6qIuhADnOrW1"
}

I'm curious what other's opinions are on this approach. I personally am not a fan of the bloat the compact JWE adds in order to encrypt the sender's public key. However, I've not found another approach yet that seems satisfactory. If anyone has some suggestions it would be appreciated.

@kdenhartog
Copy link
Contributor Author

Please be aware, this issue is also posted on the DIDComm-js library in DIF. I'll facilitate communication across the groups if you don't want to track both conversations.

@troyronda
Copy link
Contributor

troyronda commented Jul 19, 2019

I would like to see Aries using a JWE compliant data model.

I am in favor of the proposal.

@baha-ai
Copy link

baha-ai commented Jul 19, 2019

the above example is for Authrcrypt, below is the Anoncrypt example:

{
    "protected": base64url({
        "typ": "prs.hyperledger.aries-anon-message",
        "alg": "ECDH-ES+XC20PKW",
        "enc":"XC20P"
    }),
    "recipients": [
        {
            "encrypted_key": "base64url(encrypted CEK)",
            "header": {
                "kid": "base58(Recipient vk)",
                "iv": "base64url(CEK encryption IV)",
                "epk": "Ephemeral JWK",
            }
        },
        {
            "encrypted_key": "base64url(encrypted CEK)",
            "header": {
                "kid": "base58(Recipient vk)",
                "iv": "base64url(CEK encryption IV)",
                "epk": "Ephemeral JWK",
            }
        }
    ],
    "aad": "base64url(sha256(concat('.',sort([recipients[0].kid, recipients[n].kid]))))",
    "iv": "base64url(content encryption IV)",
    "ciphertext": "base64url(ciphertext)",
    "tag": "base64url(AEAD Authentication Tag)"
}

@llorllale
Copy link
Contributor

llorllale commented Jul 22, 2019

It's worth pointing to the internet draft that originally defined the JOSE AEAD algorithm for chacha20: https://tools.ietf.org/html/draft-amringer-jose-chacha-00

@gamringer
Copy link

gamringer commented Jul 24, 2019

I have just updated the draft to include ECDH-ES bits:

https://tools.ietf.org/html/draft-amringer-jose-chacha-01

@kdenhartog
Copy link
Contributor Author

@gamringer I'm not finding documentation or an IANA registry of the pk field. Do you have details about it?

@gamringer
Copy link

I'm writing up the I-D for ECDH for JOSE separately from the Chacha for JOSE I-D. I'm writing it up to have the pk field be the Sender's JWK, or JWE-envelopped JWK, as was the case in my example.

@kdenhartog
Copy link
Contributor Author

Ok, since we're registering the pk field is it possible that we can try and find a more compact form for this? With ed25519 and secp256k1 keys I'm not too concerned about size (it's not great but at least manageable), but with a 3072 bit RSA key, this is going to going bloat the message to the point where it takes a second or 2 to load the messages and could make it not possible to embed in QR codes.

@gamringer
Copy link

Then we can register both opk, and oid to serve are mirror to recipient versions where opk is to jwk what oid is to kid. That way, prs.hyperledger.aries-auth-message can just use oid to hold a crypto_box_seal of the sender's key identifier, since for format for oid (as for kid) is application dependant.

@kdenhartog
Copy link
Contributor Author

That seems like a good solution that will allow us to work with multiple types then.

@baha-ai
Copy link

baha-ai commented Jul 30, 2019

@gamringer @kdenhartog I pushed an initial PR to our go-jose fork to support the above format using Go:
unit tests are passing: https://github.com/trustbloc/go-jose/pull/1
your input is appreciated.

@baha-ai
Copy link

baha-ai commented Jul 31, 2019

That seems like a good solution that will allow us to work with multiple types then.

@gamringer @kdenhartog can we have an updated JWE format with opk and oid examples? I wanna be clear on what fields our JWE will be using to reference these key IDs.

@kdenhartog kdenhartog self-assigned this Aug 5, 2019
@kdenhartog kdenhartog added the enhancement New feature or request label Aug 5, 2019
@gamringer
Copy link

gamringer commented Aug 7, 2019

I finished reimplementing my proposal in PHP: https://github.com/gamringer/php-authcrypt

This is the output of 1-crypt.php:

Sender SK   : 6tsNPgZAg-NWM3s4S0VOOWM2yrcOfwsCrN0JGFEWaWw
Sender PK   : QbvqozxGQ3U8FDLmlKOx8Hd5GiozMRO2pwrevZ5ZFTM
Recipient SK: yOTIYzADtNrszMN0Eq-BE22Rb8v_3ZoybpcxJ-oHhp4
Recipient PK: -u0zk9iY_ZS2wP2z4zuLjR7_kz_kxVU0anRz8_A66T0
Payload     : SGVsbG8gV29ybGQh
--------------------------------
{"protected":"eyJ0eXAiOiJwcnMuaHlwZXJsZWRnZXIuYXJpZXMtYXV0aC1tZXNzYWdlIiwiYWxnIjoiRUNESC1TUytYQzIwUEtXIiwiZW5jIjoiWEMyMFAifQ","recipients":[{"encrypted_key":"y5yrOJUGSZHeJvVjgo39s7_TNRJsCZB46Q_MvNfa70Y","header":{"apu":"bcyJRBCQSQG-h7L7qGge9cMsdXkmMcLNsXvoWq1X1uJzOFyHK-Wg9shZTSQL8XmeW3BzzXvoIWosKw0vfCjtng","iv":"O80MuQSYTjjmNajjTjTlH6etFPpDR7pM","tag":"Y92d_V-qKHA1aedRCjvdFQ","kid":"HtWhz6QevaQF39Gv3Hvf7K6xo2FTViAkY22rhZpQrdWc","oid":"6dhhFIHHi9UN-N9IJr0XTRiijKvF0aSwxg7MTMAH_AuNc8THkblgbEGvW1Y8r2R02iuJbGj_jkMd_d899A6God8Dyw0B-TR9Lh7N6ho3-eEic-XKM5MGgLR2gg"}}],"aad":"FeI0LXy7m0-orE0VwiQU-2RjQyYMsnIvSEzpduiB7sY","iv":"ofkD-Q0peUhkJzrDiM2mlC-O5HgyTBmZ","tag":"osug2xz38IjB_oqubJXkcw","ciphertext":"vIPC1fCfxIMkxDsu"}
--------------------------------
{
    "protected": "eyJ0eXAiOiJwcnMuaHlwZXJsZWRnZXIuYXJpZXMtYXV0aC1tZXNzYWdlIiwiYWxnIjoiRUNESC1TUytYQzIwUEtXIiwiZW5jIjoiWEMyMFAifQ",
    "recipients": [
        {
            "encrypted_key": "Y6eYXyrOWj67oHh4hla_MS024oPtgWeM4LOPqwyrXTM",
            "header": {
                "apu": "MMapegFCsTTrygbuC80X0NeHjrtJ7Fh5d9CIl6pq4HVgYDAtjIS7dyQKXO-Vgan8ho33ZglRJCfW4Wx2pH3cNg",
                "iv": "eBuzpjLTU16gmJvZKV3JShzvibJM6h7_",
                "tag": "PKg4RLQ5hikKQ2Vq2SCqGg",
                "kid": "HtWhz6QevaQF39Gv3Hvf7K6xo2FTViAkY22rhZpQrdWc",
                "oid": "cOvRYUooq-y1TDjK3Lt3wCA3H-w9E6PJXNPDdIPLDDZlpE7QJvJuLIwJzSPqDhRvQUMaOYrXyLGAgdriGpKbKjWcLtQapjExq8sesL5bax68J46vv-2-GuDVbQ"
            }
        }
    ],
    "aad": "FeI0LXy7m0-orE0VwiQU-2RjQyYMsnIvSEzpduiB7sY",
    "iv": "9AL-EASXKfuonBKKxsPHSccrX2zy7j2l",
    "tag": "IG6L99-sFnq3Cfz29Z-jDg",
    "ciphertext": "IX7EQSrqhxL61YjE"
}

@baha-ai
Copy link

baha-ai commented Aug 8, 2019

I believe we should update the format to meet the standards and reference them in the docs.

@gamringer, I gathered these links from the discussion we had such as:

  • Key derivation for ECDH:

https://tools.ietf.org/html/rfc7518#section-4.6.2

More specifically about the alternative format for apu with a random 64 byte value and no apk:

   a manner similar to "Diffie-Hellman Key Agreement Method" [RFC2631]:
   in that case, the "apu" parameter MAY either be omitted or represent
   a random 512-bit value (analogous to PartyAInfo in Ephemeral-Static
   mode in RFC 2631) and the "apv" parameter SHOULD NOT be present.

   See Appendix C for an example key agreement computation using this
   method.
instead of `opk` and `oid`, I will change it to `spk` and `sid`

we should probably update the format to reflect these changes

baha-ai pushed a commit to baha-ai/aries-framework-go that referenced this issue Aug 16, 2019
	This change adds support to encrypt agent's payloads
	for the Pack() call at the transport layer

	It follows JWE encryption instructions from Aries
	Issue: hyperledger/aries-rfcs#133

Signed-off-by: Baha Shaaban <[email protected]>
@baha-ai
Copy link

baha-ai commented Aug 16, 2019

@kdenhartog @gamringer, I just pushed the Go implementation of the JWE encryption (decryption will be coming up soon). Here is an example format:
Encryption with XC20P:

{
  "protected": {
  "alg": "ECDH-SS+XC20PKW",
  "enc": "XC20P",
  "typ": "prs.hyperledger.aries-auth-message"
  },
  "recipients": [
       {
          "encrypted_key": "C1ZgZxgIKne86FlGuct0L2y2UENtHyOS78KvoVgBAaM=",
          "header": {
            	"apu": "dpQnrrNMi8CHC1sRLxcKdzHUfYqM9hMI4P8VpLMArVYdJQlmnIzF6niGjzTCNF6UBobaTmm6msTlEK2chntsOA==",
            	"iv": "Po7Wi7CKWHeCof450oyrFB9A_x8D8LQa",
            	"tag": "QkkS6QqQLaWRJDtEK0htKQ==",
            	"kid": "ETu2KkCZUawEHq99z2RubyCsnVyaLhzMiYTHMu3uVncp",
            	"oid": "PBqOfOOkMM3hDUxSunySlld6-15qdbMdWajolmmKVYs0bog785bnqckvoPYDkqoMXwVPtr-qPQgIpg9A"
            }
       },
       {
          "encrypted_key": "pGL8MBT1WUXjfXuwSl2enj5N1BLtmeEZ1wAZAzUoH60=",
          "header": {
            	"apu": "iSa2FM-_nOzxG4IjX5MOUv1uvEFs-nA-fYlrLry66VVrNJxakXclPC7UnJ_RtqI_cwlEgJ_GaVHcNv84NA8VjQ==",
            	"iv": "xSEHpsn1iuy4x3KTfeHdcdBRziqCvImG",
            	"tag": "vsU_H6K5m4_jVFWNxxV--A==",
            	"kid": "BkcdRt9AChfFXGvKU3ecjzyNCzoP4EBVvm5dtgvqv5UX",
            	"oid": "u1PbWv2wIIV4BkOibItT4DzkzVb6d75hLhS01Ha1CzapUX5xjBlcyjwrl6nIOrARBvoJyH0FXZdkxec7"
            }
       },
       {
          "encrypted_key": "PsFDcK7-VHzH-E-KDC-Cy1NHZYKepv1X7zolbuRbdbw=",
          "header": {
            	"apu": "PAYUGCNWRKGq82HvOxyMVE6VI13mYlvowK5X3NqZsFiwqpTTwXgNTZA5Tnxch8vsBZAzeEpmEvI3M9YpJflnrA==",
            	"iv": "s_FfFvulVhFKwCoalqp9qESsEtWnDgZ9",
            	"tag": "7IIQuM9Hs2Zqsj9Vga2nAQ==",
            	"kid": "EikHy4Hmbw5zGUMPGYDoAwYFNKnrKeDQkFqLxMvTXD83",
            	"oid": "EaM4cO2c6AIMWBDwOnQSSfc95qHoF1_8OesVSrK2-O1RLWTEq3QB0cnj46WIa_KIDNq-wpwxRmNQu3-5"
            }
       }
  ],
  "aad": "itXVSOmyKyNvjHYSDiCpvxB0HqlFfy5YJtdQftWqjEk=",
  "iv": "WsiI5jXbXKYUJjwF0H3ZVl02meBL26bL",
  "tag": "jMmIt9MsYRtboMwA4yrf2g==",
  "ciphertext": "VbnjyXkgej1Ix1gjxkVs0ncGWauOxyt5QzY="
}

Go PR: hyperledger-archives/aries-framework-go#107

as mentioned in my previous post, we need to update the format a little and confirm if we're using oid or sid (or any other fields)

baha-ai pushed a commit to baha-ai/aries-framework-go that referenced this issue Aug 16, 2019
	This change adds support to encrypt agent's payloads
	for the Pack() call at the transport layer

	It follows JWE encryption instructions from Aries
	Issue: hyperledger/aries-rfcs#133

Signed-off-by: Baha Shaaban <[email protected]>
baha-ai pushed a commit to baha-ai/aries-framework-go that referenced this issue Aug 16, 2019
	This change adds support to encrypt agent's payloads
	for the Pack() call at the transport layer

	It follows JWE encryption instructions from Aries
	Issue: hyperledger/aries-rfcs#133

Signed-off-by: Baha Shaaban <[email protected]>
baha-ai pushed a commit to baha-ai/aries-framework-go that referenced this issue Aug 16, 2019
	This change adds support to encrypt agent's payloads
	for the Pack() call at the transport layer

	It follows JWE encryption instructions from Aries
	Issue: hyperledger/aries-rfcs#133

Signed-off-by: Baha Shaaban <[email protected]>
@kdenhartog
Copy link
Contributor Author

Update on this. I've been looking at this recently and trying to weigh all factors that would get us good interop between all parties in DID community. Since this change would create a breaking change, I want to make sure that what ever we move to accounts for all of our needs for quite some time.

For example, one thing that I didn't account for that I'd like to in the future is the ability to add in forward secrecy when we perform this breaking change.

Since I know this work is blocking @Baha-sk my recommendation is to implement the currently non-compliant JWE for now and we'll say that it is version 1. This will give us time to figure out what we want and to conduct a larger discussion as to what we should do going forward. Does that feel like a reasonable approach?

@troyronda
Copy link
Contributor

troyronda commented Aug 21, 2019

Thanks @kdenhartog for taking the time to look at this issue, and your continuing efforts.

We would like to use a compliant JWE. We have already started implementing this updated approach.

I am concerned about:

  • the implications of marking a non-compliant model “v1”.
  • the interop story being harder without compliance.
  • explaining non-compliance.
  • the unknown plan and timeline for getting to compliance. (It’s not so clear what is the blocker and how/when this will become unblocked.)

With these concerns, I cannot say that deferring the compliant version is the correct path. It would be much better to become compliant as soon as possible (as the v1), and then iterate from there as needed.

@dhh1128
Copy link
Contributor

dhh1128 commented Oct 22, 2019

@Baha-sk : I am content with the idea that we should split verification keys and encryption keys apart; the conflation of these was an accident of us using Ed25519. And I get that you're advocating the removal of the verkeys from the JWE. That's all fine. But what I don't know if we're aligned on is this: I want an encryption key value--NOT a reference to an encryption key named in a DID doc--to be named in the JWE, and I want a verkey value--NOT a reference to a verkey in a DID doc--as the field that a JWS uses to verify.

@baha-ai
Copy link

baha-ai commented Oct 22, 2019

@dhh1128 the encryption keys in the JWE example are real public key values. They are not reference IDs nor are they stored in the DID doc.

@troyronda
Copy link
Contributor

troyronda commented Nov 14, 2019

@troyronda
Copy link
Contributor

troyronda commented Nov 15, 2019

The above noted effort allows:

  • content encryption algorithms:

    • ChaCha20-Poly1305 (C20P)
    • 256-bit AES-GCM (to allow for a FIPS compatibility mode)
    • future: XChaCha20-Poly1305 (assuming XC20P)
  • key encryption algorithms:

    • ECDH-ES+A256KW

Also note the kid format:

${did}#${keyAgreementKey.fingerprint()}

@dhh1128
Copy link
Contributor

dhh1128 commented Nov 15, 2019

The kid format is fatally flawed unless it includes a versionstamp or (less optimally) a timestamp. How can we get it upgraded?

@troyronda
Copy link
Contributor

troyronda commented Nov 15, 2019

It would be nice to have alignment across communities on these topics.

I was also going to mention that it would be nice to have overlapping algorithm support in the envelope. Note that they have ECDH-ES+A256KW and additionally allow for 256-bit AES-GCM. The rationale for the latter also seems like a good idea (allowing for FIPS).

@tplooker
Copy link
Contributor

How is the kid fatally flawed? In an encrypted envelope the kid is just suppose to act as an alias to a private key for the recipient, DID resolution isn't even required. The kid can literally be anything, as long as it is meaningful to the recipient, it really comes down to what the recipient indexes the keys in their kms against.

@tplooker
Copy link
Contributor

I think we need to be clearer around the intention of the kid in different circumstances.

When we are talking about encryption/decryption, the kid is really acting as an alias to a private key the recipient should have, whereas when we are verifying a signature, the kid should allow the audience of the signature to obtain a public key to perform verification.

@dhh1128
Copy link
Contributor

dhh1128 commented Nov 15, 2019

I agree that we need to be clearer about intention. However, that doesn't affect my assertion that the kid format of ${did}#${keyAgreementKey.fingerprint()} is fatally flawed.

Understand: I'm not saying that the idea of kid is fatally flawed. I'm staying that the specific kid format is.

It's flawed because without a versionstamp or timestamp, it can't be dereferenced -- either for encryption or for signature validation.

Analogy: you want an email address, and I give you "@example.com" as the answer. I claim that answer is fatally flawed, because it doesn't include the mailbox identifier.

A kid without a versionstamp or timestamp might be dereferencable, sometimes, if no key rotations have ever happened. And people who are building trivial, non-production systems get away with using such a kid because they ignore this. But to really use a kid to solve production problems, you can't ignore it. If you do, your kid is fatally flawed.

@tplooker
Copy link
Contributor

I understand your reasoning, but this key format proposed I dont think is fundamentally flawed in the way you are describing for encryption and decryption, which is the context it is being applied in, because the fragment portion is a fingerprint. Which implies an immutable and unique reference to a single key.

Say you rotate a key but some sender continues to send you encrypted messages to your old key, provided you still have the old key indexed in your kms by this kid format, you should still be able to decrypt the message. And safely decrypt messages from new keys linked to the same did.

I do understand that because identifiers for keys in did docs are not enforced as immutable by default (e.g not every key featured in a did doc has an id corresponding to the keys unique fingerprint) this is not a universal rule for all did methods. However, this as a kid format if it were universally supported I believe would work without major issue for encryption/decryption.

Essentially when it comes to a kid for a signature, the easiest option to preserve verification is to make the kid the public key, no dereferencing required. However when it is an encrypted message the kid must be some meaningful alias to the recipient, there really is no need to have a versionstamp or timestamp, either the alias means something to me as a recipient or it doesn't.

@dhh1128
Copy link
Contributor

dhh1128 commented Nov 15, 2019

I dont think is fundamentally flawed in the way you are describing for encryption and decryption, which is the context it is being applied in, because the fragment portion is a fingerprint. Which implies an immutable and unique reference to a single key.

The problem is that the key value is immutable, but the key's meaning with respect to the DID doc -- the context in which that key is related to the DID -- is not. That key value might have been authorized to encrypt on behalf of the DID at point A, and not authorized to encrypt at point B, and you wouldn't know that. So you could do naked decryption with just the key value, but you can't do the semantic validation that should follow naked decryption, without the additional context.

However when it is an encrypted message the kid must be some meaningful alias to the recipient, there really is no need to have a versionstamp or timestamp, either the alias means something to me as a recipient or it doesn't.

I disagree. You are assuming that all decryption happens at the time of transmission, which is also the time of reception. If you separate those timeframes, the "either it means something to me or it doesn't" unravels. It might not mean something to me at the time I receive it, but it might be important that it means something to me later. Or vice versa.

@selfissued
Copy link

A kid value is used to match a particular key. No less and no more. They're assigned by the party that creates the key and they're simply strings that are intended to uniquely identify that key. They work great for that purpose and are in widespread use.

@dhh1128
Copy link
Contributor

dhh1128 commented Nov 16, 2019

@selfissued : Right. 100% agree. I think kids are great.

My concern is that the intersection between kids and DIDs is suffering because of a design decision in the DID spec (which I vehemently disagreed with but failed to change) that the relationship between a key identifier and a key value inside a DID doc is mutable. It is illegal to assume that did:foo:123#identifier always refers to the same thing.

Now, fans of JW* in DID circles have suggested eliminating id as a required property on a key, claiming that kid is an equivalent. I don't think they realize the mess they're signing up for. An id has meaning in the larger context of the rest of the doc, and one of those meanings, according to the DID spec, is that you can use it to construct DID URLs with particular semantics. Among those semantics is the idea of mutability--that the data behind the identifier can be rotated. So if kid is an equivalent of id, then it follows that a kid that's a DID URL composed of a DID doc plus an identifier is not allowed to be assumed to be stable with respect to time. This is in direct conflict with the intention of kid, which is the simple and stable mapping you articulated.

Possible solution 1: we stop thinking that kid and id mean the same thing. If someone wants a key to have a mutable relationship between its identifier and its value, they use id; if they want immutability, they use kid. (Personally, I would never use 'id'...) We don't confuse them. A DID URL in the format did:foo:123#xyz is an id-based URL, NOT a kid-based URL. If we want a kid-based URL, we use something different, like maybe did:foo:123;kid=xyz.

Possible solution 2: we like kid = id, and we accept that this means DID URLs that reference keys by kid must have a versionstamp or timestamp to disambiguate. We stop kidding ourselves that did:foo:123#identifier is enough to accomplish what kid does.

@tplooker
Copy link
Contributor

tplooker commented Nov 16, 2019

IMO the case you describe @dhh1128 should be offloaded for the recipient software to deal with, e.g if recipient software rotates a key and the same kid is used in a did doc, what does the recipient software do with the old key? If its deleted then you can't decrypt anyway regardless of having a timestamp. If the software keeps the key, then how is its indexed in the kms? Could you not preserve its association to the kid it use to represent? Then if you receive a message that is encrypted to a kid where there have been multiple keys represented by that kid over time, you try each one until you can successfully decrypt.

@tplooker
Copy link
Contributor

tplooker commented Nov 16, 2019

Alternatively if you don't want to traverse all previous keys associated to a kid, we could add an optional JOSE header field that describes when the message was encrypted, that would then give the recipient further clue as to which key they need to use to decrypt.

@tplooker
Copy link
Contributor

I do however disagree with trying to pack more meaning into the kid field, like a timestamp/versionstamp

@andrewwhitehead
Copy link
Member

The DID spec provides a format for identifying a DID Doc at a specific time using either the version-id or version-time parameters, doesn't that satisfy the need to refer to a specific key? The hl property (hash) can also be used to ensure the document integrity.

"kid": "did:method:ident;version-id=GJdXXRq7Kxu7uKQCb77ik3;hl= zQmWvQxTqbG2Z9HPJgG57jjwR154cKhbtJenbyYTWkjgF3e#key-ident"

@tplooker
Copy link
Contributor

tplooker commented Nov 27, 2019

It does, but doing this adds a lot of complexity in parsing this field. kid is simply meant to be a string alias or a hint to a key, by making the requirement in DIDComm that this is a full URL we now need a url parser to extract this information and will need to evolve this syntax as DID resolution continues to evolve.

If there is a problem with kid reuse for multiple keys over time, I think the simple choice is for the audience or consumer of the message to iterate through the keys associated to one kid or for there to be a seperate field in the kid header that hints at timing information and therefore allows them to limit the possible keys the kid is referring too.

@kdenhartog
Copy link
Contributor Author

IIRC, I heard there was discussion about this during the connectathon. Would anyone be able to provide details about that discussion here for historical context?

@dhh1128
Copy link
Contributor

dhh1128 commented Jan 14, 2020

My memory of the conversation was that we got to a stalemate. @tplooker , please confirm. The comment thread here boils down to 2 basic positions, which also summarize the connectathon discussion:

  1. (Tobias) It's onerous and ugly to make a DID key reference in kid include a timestamp or version, so let's not do that.
  2. (me) If we use a DID key reference as the value of kid, it must include a timestamp or version. (The assertion that we could have an ambiguous kid that can be disambiguated by iterating over candidates until we find a match strikes me as worse than a code smell--it feels like a security vulnerability.)

FWIW, the ticket about this issue in the DID spec repo includes a comment from one of the JWE spec authors (Mike Jones) that suggests the intent of kid is to be unambiguous--else its use in caching is impossible.

@tplooker
Copy link
Contributor

@dhh1128, yes your summary is correct.

@TelegramSam
Copy link
Contributor

Continue issue here: decentralized-identity/didcomm-messaging#22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DIDComm Related enhancement New feature or request
Projects
None yet
Development

No branches or pull requests