Align with protocol spec to use operation request payload for operation hash calculation about sidetree

[OR13]

In particular https://github.com/decentralized-identity/sidetree-core/blob/master/docs/protocol.md#did-document-resolution

https://github.com/decentralized-identity/sidetree-core/blob/814b67ba6ce5e0a46bc104c115891bf30f202343/tests/RequestHandler.spec.ts#L197

Per the terminology:
Operation hash | The hash of the JSON-formatted request of a Sidetree operation.

Should be base64Url(sha256(bufferFromBase64Url( base64UrlEncodedOp ... or

base64Url(sha256(bufferFromBase64Url( base64UrlEncodedOpPayload ...

Its very confusing to call the base64url encoded sha256 hash of the operation payload json "the operation hash", because its not actually the hash of the operation.

I suggest the operation hash remain named, "Operation hash", and be base64Url of sha256 of bufferFromBase64Url of the base64UrlEncodedOperation... and the spec be updated.

IMO the create is wrong not the update.

Alternatively "Operation Hash" could be renamed to "Operation Payload Hash", but it seems like not a good direction to go. ..The point of the opHash is to compare with previousOperations, not previous operation payloads...

  const operationHash = base64url.encode(
    crypto
      .createHash("sha256")
      .update(base64url.toBuffer(batchFile.operations[0]))
      .digest()
  );

vs

  const operationPayloadHash = base64url.encode(
    crypto
      .createHash("sha256")
      .update(base64url.toBuffer(operation.payload))
      .digest()
  );

Here is what a first update looks like today:

const createPayloadTemplate = {
  "@context": "https://w3id.org/did/v1",
  publicKey: [
    {
      id: "#key1",
      type: "Secp256k1VerificationKey2018",
      publicKeyHex:
        "034ee0f670fc96bb75e8b89c068a1665007a41c98513d6a911b6137e2d16f1d300"
    }
  ]
};

const updatePayloadTemplate = {
  did: "did:sidetree:EiDFDFUSgoxlZoxSlu-17yz_FmMCIx4hpSareCE7IRZv0A",
  operationNumber: 1,
  previousOperationHash: "EiDFDFUSgoxlZoxSlu-17yz_FmMCIx4hpSareCE7IRZv0A",
  patch: [
    {
      op: "replace",
      path: "/publicKey/1",
      value: {
        id: "#key2",
        type: "Secp256k1VerificationKey2018",
        publicKeyHex:
          "029a4774d543094deaf342663ae672728e12f03b3b6d9816b0b79995fade0fab23"
      }
    }
  ]
};

{
  "header": {
    "operation": "create",
    "kid": "#key1",
    "alg": "ES256K",
    "proofOfWork": { }
  },
  "payload": "eyJAY29udGV4dCI6Imh0dHBzOi8vdzNpZC5vcmcvZGlkL3YxIiwicHVibGljS2V5IjpbeyJpZCI6IiNrZXkxIiwidHlwZSI6IlNlY3AyNTZrMVZlcmlmaWNhdGlvbktleTIwMTgiLCJwdWJsaWNLZXlIZXgiOiIwMmY0OTgwMmZiM2UwOWM2ZGQ0M2YxOWFhNDEyOTNkMWUwZGFkMDQ0YjY4Y2Y4MWNmNzA3OTQ5OWVkZmQwYWE5ZjEifSx7ImlkIjoiI2tleTIiLCJ0eXBlIjoiUnNhVmVyaWZpY2F0aW9uS2V5MjAxOCIsInB1YmxpY0tleVBlbSI6Ii0tLS0tQkVHSU4gUFVCTElDIEtFWS4yLkVORCBQVUJMSUMgS0VZLS0tLS0ifV0sInNlcnZpY2UiOlt7InR5cGUiOiJJZGVudGl0eUh1YiIsInB1YmxpY0tleSI6IiNrZXkxIiwic2VydmljZUVuZHBvaW50Ijp7IkBjb250ZXh0Ijoic2NoZW1hLmlkZW50aXR5LmZvdW5kYXRpb24vaHViIiwiQHR5cGUiOiJVc2VyU2VydmljZUVuZHBvaW50IiwiaW5zdGFuY2VzIjpbImRpZDpiYXI6NDU2IiwiZGlkOnphejo3ODkiXX19XX0",
  "signature": "mAJp4ZHwY5UMA05OEKvoZreRo0XrYe77s3RLyGKArG85IoBULs4cLDBtdpOToCtSZhPvCC2xOUXMGyGXDmmEHg"
}

By hashing only the payload, the signature, and header are not being linked to the next operation... Seems clearer to have the operation chain be:

signedOp -> signedOp, instead of signedOp -> signedOp.payload

[thehenrytsai]

Adding @csuwildcat.

@OR13 You are absolutely spot on with your observation. Up until recently the operation hash has always been the hash of the entire operation request buffer, however, @csuwildcat made a feedback saying that it would be desirable for the operation requester to know the operation hash (thus requester's DID) before the request is sent to a Sidetree node, even if proofOfWork is not included, and hence the change in operation hash definition.

We considered security implications of hashing operation payload directly as operation hash and it seem fine, and it has the benefit of the first operation hash (of a create operation) being exactly the newly created DID, thus reducing a level of indirection.

But I totally agree with you that then it should have been called "operation payload hash" for clarity, so if you agree that this is the right approach, I will create an issue to make sure the rename happens.

[OR13]

That makes sense, and I have leveraged the deterministic DID feature your describe, it is still possible even if the operationHash != hash of operation payload. It is very helpful to know that what a did will be, based on it's initial content.

…

On Mon, Mar 25, 2019, 7:26 PM Henry Tsai ***@***.***> wrote: Adding @csuwildcat <https://github.com/csuwildcat>. @OR13 <https://github.com/OR13> You are absolutely spot on with your observation. Up until recently the *operation hash* has always been the hash of the *entire* operation request buffer, however, @csuwildcat <https://github.com/csuwildcat> made a feedback saying that it would be desirable for the operation requester to know the *operation hash* (thus requester's DID) before the request is sent to a Sidetree node, *even* if proofOfWork is not included, and hence the change in *operation hash* definition. We considered security implications of hashing operation payload directly as *operation hash* and it seem fine, and it has the benefit of the first *operation hash* (of a create operation) being exactly the newly created DID, thus reducing a level of indirection. But I totally agree with you that then it should have been called "operation payload hash" for clarity, so if you agree that this is the right approach, I will create an issue to make sure the rename happens. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#112 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6VsEE_2YiUr1Vv_xBVjS9mKtjYkBsBks5vaWlJgaJpZM4akCaF> .

[thehenrytsai]

@OR13 Just to be absolutely clear, I take it that your point is to keep operation hash concept away from deterministic DID right?

ie:
first operation hash will NOT be the DID.

Sounds very resonable, I will discuss with @csuwildcat and @arvinda et all and update the create operation hash and spec accordingly.

[OR13]

Correct, I think it's reasonable for DID = hash of create payload, and operation hash = hash on encoded operation.

…

On Thu, Mar 28, 2019, 1:24 PM Henry Tsai ***@***.***> wrote: @OR13 <https://github.com/OR13> Just to be absolutely clear, I take it that your point is to keep *operation hash* concept away from *deterministic DID* right? ie: first operation hash will *NOT* be the DID. Sounds very resonable, I will discuss with @csuwildcat <https://github.com/csuwildcat> and @arvinda <https://github.com/arvinda> et all and update the create operation hash and spec accordingly. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#112 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6VsK0WHjWX_HSE41rZpBQ5HO9y1nBHks5vbQjGgaJpZM4akCaF> .

[csuwildcat]

@OR13 one concern I have about moving to the operation hash being the composition of the entirety of operation props/values is that it may lock in the user or writer into having to sequentially assemble/generate each operation, serializing the process, including any future POW additions. The case I was considering was one where a user has an 'unpublished' pairwise DID that they want to cast to the chain for whatever reason - let's say its a scenario where they need the chain to act as the definitive oracle. If they had to quickly publish multiple ops at the same time, they (or the writer) would have to do all the processing one after another. I am not sure how much of an issue this is, but I am weary of creating a condition where a potential downside exists without much upside, know what I mean?

[OR13]

This got me thinking... The operation hash is required for updates, so operations are always sequential, if the pow changes, the operation hash changes and this applies to both the unpublished case, and the migration case. In the case of migration from one chain or contract to another, the existing pow should still be valid, if this were not the case, the batchFile would change, and so would the anchor file... so everything would need to be recreated. The main issue I see with pow, is that the operation hash would not be knowable by clients who don't do their own pow. But it would be knowable for clients who do. This seems correct to me, essentially it's saying if you follow the protocol and do all the work you can trust the node to just anchor you. If you don't do all the work, you can still trust the node to anchor you, but you won't be able to know the operation hash because you have not done the pow. As a side note if operations were also content addressable, there would not be question about what the hash of an operation is. In order for the DID to be the same, you would need to sequentially migrate, old PoW is still valid, so old anchors an batchFiles are still valid. Without including the signature in the op hash, it might be possible for a did to be stolen in a migration because the original public key was compromised and the attacker was able to craft update operations and anchor them... I guess this depends on the ordering of migration operations... I think logically, in order to update you need to know the previous operation hash makes sense. I also think it makes sense that the operation hash be a function of the signature and payload, since together they represent the end user's desire. if the header is also included, the operation hash is the hash of the operation, and updates are now coupled to proof of work. I don't think i fully understood the pairwise DID example, the user or node would still need to do the proof of work for each operation, and they would still need to include the previous operation hash, which in the new system will require a proof of work for the previous operation, in the old one, they would still need to sequentially generate the operations, and the proof of work, but they would not be explicitly linked until published. I think i need to understand the link between pow and migrations better.

…

On Thu, Mar 28, 2019, 5:28 PM Daniel Buchner ***@***.***> wrote: @OR13 <https://github.com/OR13> one concern I have about moving to the operation hash being the composition of the entirety of operation props/values is that it may lock in the user or writer into having to sequentially assemble/generate each operation, serializing the process, including any future POW additions. The case I was considering was one where a user has an 'unpublished' pairwise DID that they want to cast to the chain for whatever reason - let's say its a scenario where they need the chain to act as the definitive oracle. If they had to quickly publish multiple ops at the same time, they (or the writer) would have to do all the processing one after another. I am not sure how much of an issue this is, but I am weary of creating a condition where a potential downside exists without much upside, know what I mean? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#112 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6VsL6146Ui_3eUI13cDfJQEjuDwlnwks5vbUIhgaJpZM4akCaF> .

[OR13]

After discussions in Slack, we think that the previousOperationHash should remain the base64Url(sha256(operation.payload)), this means the previousOperationHash is really just the previousOperationPayloadHash, but the name will remain the same. https://github.com/decentralized-identity/sidetree-core/blob/master/docs/protocol.md#terminology Should be updated to make this clearer, possibly with examples.

[thehenrytsai]

Fixed in a598104, closing.