Is verkey expansion implemented correctly?

[peacekeeper]

If I resolve did:indy:sovrin:WRfXPg8dantKVubE3HX8pw (see Universal Resolver), then I get this verification method in the DID document:

  "verificationMethod": [
    {
      "controller": "did:indy:sovrin:WRfXPg8dantKVubE3HX8pw",
      "id": "did:indy:sovrin:WRfXPg8dantKVubE3HX8pw#verkey",
      "publicKeyBase58": "WRfXPg8dantKVubE3HX8pwP7F3BNs5VmQ6eVpwkNKJ5D",
      "type": "Ed25519VerificationKey2018"
    }
  ]

It seems the expanded verkey is simply the "id" string value "WRfXPg8dantKVubE3HX8pw" plus the "verkey" string value "P7F3BNs5VmQ6eVpwkNKJ5D". I think this is implemented here:

https://github.com/IDunion/indy-did-resolver/blob/b26f76501c609762202865affae04639ac01b62b/indy-didresolver/src/did_document.rs#L64

But I'm not sure the implementation is correct. I don't think the base58-encoded strings themselves should be concatenated. Instead, the "id" and "verkey" strings should be base58-decoded, then the byte arrays should be concatenated, and then the combined byte array should be base58-encoded again. The result is different.

[c2bo]

Afaik that is the correct implementation for the current (indy-node 1.12) implemenation - i do believe this changes with did:indy:

indyscan shows (https://indyscan.io/tx/SOVRIN_MAINNET/domain/18) the same verky WRfXPg8dantKVubE3HX8pwP7F3BNs5VmQ6eVpwkNKJ5D:

{
  "txn": {
    "data": {
      "dest": "WRfXPg8dantKVubE3HX8pw",
      "verkey": "~P7F3BNs5VmQ6eVpwkNKJ5D",
      "verkeyFull": "WRfXPg8dantKVubE3HX8pwP7F3BNs5VmQ6eVpwkNKJ5D"
    },
    "metadata": {
      "digest": "618ed1f21281f13cb769fe17308fd75279b22c41e8004280fb97170a4eb6282a",
      "from": "BrYDA5NubejDVHkCYBbpY5",
      "reqId": 1501522732982387
    },
    "type": "1",
    "typeName": "NYM"
  },
  "txnMetadata": {
    "seqNo": 18,
    "txnId": "d8f845175979a23b35235c097ed20d564dc8ad6a5ef7cc7f19b53e838386012d",
    "txnTime": "2017-07-31T17:38:53.000Z"
  }
}

[peacekeeper]

Both indyscan and this indy-did-resolver driver here are incorrect. The expanded verkey is the concatenation of the DID bytes and the shortened verkey bytes, not the concatenation of the Base58 encoded strings.

Take for example the well-known "000000000000000000000000Trustee1" seed that is a default value in some sample deployments. With this seed you get the following, all in Base58 encoding:

did: V4SGRU86Z58d6TV7PBUe6f
private key: GJ1SzoWzavQYfNL9XkaJdrQejfztN4XqdsiV4ct3LXKL
verkey: ~CoRER63DVYnWZtK8uAzNbx
wrong expanded verkey: V4SGRU86Z58d6TV7PBUe6fCoRER63DVYnWZtK8uAzNbx
correct expanded verkey: GJ1SzoWzavQYfNL9XkaJdrQejfztN4XqdsiV4ct3LXKL

Now try any arbitrary Ed25519 implementation, try to sign a string "foo" with the private key above, and check which expanded verkey can be used to verify the signature. You will find it's the latter.

[peacekeeper]

Apparently this has already been raised in indyscan: https://github.com/Patrik-Stas/indyscan/issues/52

[c2bo]

Hmm interesting, should be automatically fixed once the indy-vdr did:indy branch is released and we switch to that (or rather indy-vdr-proxy) for the driver - I will check the implementation there but that should be correct. I will create a small fix for this repository for the time being. Good catch!

[domwoe]

There's a very good chance that I just used this implementation here for indy-vdr, since we started with this project and then moved stuff to indy-vdr.

[c2bo]

There's a very good chance that I just used this implementation here for indy-vdr, since we started with this project and then moved stuff to indy-vdr.

Yep, that part also exists in the feature branch of indy-vdr. I will create a fix here and create a PR to the did-indy branch on indy-vdr.

I am currently looking at the code in indy-sdk and trying to understand where this comes from: https://github.com/hyperledger/indy-sdk/blob/main/libindy/src/utils/crypto/verkey_builder.rs#L8-L13 I have never seen a verkey that carries some form of crypto identifier with it. Does anyone of you have any idea if this is currently used anywhere / still relevant? I guess it doesn't hurt to also include this but I was kind of puzzled where this comes from.

[peacekeeper]

I have never seen a verkey that carries some form of crypto identifier with it. Does anyone of you have any idea if this is currently used anywhere / still relevant? I guess it doesn't hurt to also include this but I was kind of puzzled where this comes from.

Same here, I've never seen anything like this, but would also agree that it probably wouldn't hurt to include it.

[c2bo]

The new version should work correctly and I created a new release / docker image. Does https://dev.uniresolver.io/ pull latest from time to time or do we have to upgrade this somehow @peacekeeper ?

[peacekeeper]

@c2bo Thanks for fixing! The new version is now deployed at https://dev.uniresolver.io/, looks good.