decentralized-identity/did-jwt

[proposal] jwt validation using cosmosAddress

daoauth opened this issue ยท 8 comments

[proposal] Add cosmosAddress Property in verificationMethod like a ethereumAddress #103

Since Cosmos uses prefixes for addresses, it is an advantageous address system to use as did. So, after decomposing the address into prefix and remainder, we want to use it as a DID. And, like Ethereum, Cosmos would like to add the address of Cosmos to the property so that it can be verified by extracting the public key from the signature.

dsrv1zp78zmtj4a7qvs4p2s08ngjn9rcwpaf5k9d0la (cosmos address)
did:dsrv:1zp78zmtj4a7qvs4p2s08ngjn9rcwpaf5k9d0la (did)

// VerifierAlgorithm.ts
export function verifyES256K(
  data: string,
  signature: string,
  authenticators: VerificationMethod[]
): VerificationMethod {
  const hash: Uint8Array = sha256(data)
  const sigObj: EcdsaSignature = toSignatureObject(signature)
  const fullPublicKeys = authenticators.filter(({ ethereumAddress, blockchainAccountId, cosmosAddress }) => {
    return typeof ethereumAddress === 'undefined' && typeof blockchainAccountId === 'undefined' &&  typeof cosmosAddress === 'undefined'
  })
  const addressKeys = authenticators.filter(({ ethereumAddress, blockchainAccountId, cosmosAddress }) => {
    return typeof ethereumAddress !== 'undefined' || typeof blockchainAccountId !== 'undefined' || typeof cosmosAddress !== 'undefined'
  })

  let signer: VerificationMethod | undefined = fullPublicKeys.find((pk: VerificationMethod) => {
    try {
      const pubBytes = extractPublicKeyBytes(pk)
      return secp256k1.keyFromPublic(pubBytes).verify(hash, <SignatureInput>sigObj)
    } catch (err) {
      return false
    }
  })

  if (!signer && addressKeys.length > 0) {
    signer = verifyRecoverableES256K(data, signature, addressKeys)
  }

  if (!signer) throw new Error('invalid_signature: Signature invalid for JWT')
  return signer
}
// VerifierAlgorithm.ts
  const checkSignatureAgainstSigner = (sigObj: EcdsaSignature): VerificationMethod | undefined => {
    const hash: Uint8Array = sha256(data)
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const recoveredKey: any = secp256k1.recoverPubKey(hash, <SignatureInput>sigObj, <number>sigObj.recoveryParam)
    const recoveredPublicKeyHex: string = recoveredKey.encode('hex')
    const recoveredCompressedPublicKeyHex: string = recoveredKey.encode('hex', true)
    const recoveredAddress: string = pk.ethereumAddress ? toEthereumAddress(recoveredPublicKeyHex) : toCosmosAddress(pk.id, recoveredPublicKeyHex) // pk.id is for extracting the prefix.

    const signer: VerificationMethod | undefined = authenticators.find((pk: VerificationMethod) => {
      const keyHex = bytesToHex(extractPublicKeyBytes(pk))
      return (
        keyHex === recoveredPublicKeyHex ||
        keyHex === recoveredCompressedPublicKeyHex ||
        pk.ethereumAddress?.toLowerCase() === recoveredAddress ||
        pk.blockchainAccountId?.split('@eip155')?.[0].toLowerCase() === recoveredAddress ||
        pk.cosmosAddress === recoveredAddress
      )
    })

blockchainAccountId is the successor to ethereumAddress, to make a verificationMethod less tied to a particular network.
That is the reason why you still see its code above.

See my remarks here to get clarity on the need for this change in the first place, as I currently fail to see the reason: decentralized-identity/did-resolver#103 (comment)

Next to that this change probably isn't doing what you want it to do. As this is not a PR, I currently haven't done a diff against the original code, but the current code basically is populating the recoveredAddress with whatever comes out of the toCosmosAddress if there is no ethereumAddress present. As there are 100+ DID method, the code will probably interfere with another did method, whilst the original code didn't because it only looked at the presence of the ethereumAddress (which other projects would not fill). The current code then starts assigning the recoveredAddress regardless. Probably the toCosmosAddress does some basic checks, but it will return a CosmosAddress as recoveredAddress for other DID methods now as well, which certainly isn't wanted

I agree that using blockchainAccountId is better than using cosmosAddress. So, I would like to suggest a way to utilize the address system of Cosmos using prefixes. (cosmos:[prefix]:[remainder])

So I would like to suggest adding a toCosmosAddress function that acts like toEthereumAddress so that the validation can be executed.

decentralized-identity/did-resolver#103 (comment)

OR13 commented

feel free to take anything from this that is useful... https://github.com/OR13/lds-blockchain2021/

feel free to take anything from this that is useful... https://github.com/OR13/lds-blockchain2021/

thank you.

@daoauth are you willing to contribute a PR with this toCosmosAddress() method?

We also need to update blockchainAccountId handling to support the new format of CAIP10, so your contribution would help.

@mirceanis Thanks. I'll update soon.

I want to create and use the "verifyBlockchainAccountId" function by separating the folder for the future like this PR, what do you think?

blockchains - index (verifyBlockchainAccountId)
            - eip155
            - cosmos
            - bip122
            - etc
  const checkSignatureAgainstSigner = (sigObj: EcdsaSignature): VerificationMethod | undefined => {
    const hash: Uint8Array = sha256(data)
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const recoveredKey: any = secp256k1.recoverPubKey(hash, <SignatureInput>sigObj, <number>sigObj.recoveryParam)
    const recoveredPublicKeyHex: string = recoveredKey.encode('hex')
    const recoveredCompressedPublicKeyHex: string = recoveredKey.encode('hex', true)
    const recoveredAddress: string = toEthereumAddress(recoveredPublicKeyHex)

    const signer: VerificationMethod | undefined = authenticators.find((pk: VerificationMethod) => {
      const keyHex = bytesToHex(extractPublicKeyBytes(pk))
      return (
        keyHex === recoveredPublicKeyHex ||
        keyHex === recoveredCompressedPublicKeyHex ||
        pk.ethereumAddress?.toLowerCase() === recoveredAddress ||
        pk.blockchainAccountId?.split('@eip155')?.[0].toLowerCase() === recoveredAddress || // CAIP-2
        verifyBlockchainAccountId(recoveredKey, pk.blockchainAccountId || '') // CAIP-10
      )
    })

    return signer
  }

Yes, that sounds like a good idea

Closed by #205