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

Unable to pack encrypted didcomm message for certain 25519 keys #1067

Closed
nickreynolds opened this issue Nov 22, 2022 · 3 comments · Fixed by #1076
Closed

Unable to pack encrypted didcomm message for certain 25519 keys #1067

nickreynolds opened this issue Nov 22, 2022 · 3 comments · Fixed by #1076
Labels
bug Something isn't working pinned don't close this just for being stale

Comments

@nickreynolds
Copy link
Contributor

Bug severity
2

Describe the bug
Discovered during IIW interop session, veramo failed to correctly pack a DIDCommV2Message for a DID that expressed its X25519 key with publicKeyBase58 or publicKeyMultibase. It appears that veramo's implementation only works correctly when the key is expressed in publicKeyHex format.

To Reproduce
Steps to reproduce the behaviour:

  1. Attempt to pack message where recipient is DID that resolves a DIDDocument with an X25519 key that uses publicKeyBase58 representation
  2. See Error

Expected behaviour
Should be able to pack encrypted message no matter which representation X25519 key uses

Details
Might not be remembering details 100% correctly, will have to double check once I can reproduce again

Versions (please complete the following information):

  • Veramo: 4.11
@nickreynolds nickreynolds added the bug Something isn't working label Nov 22, 2022
@nickreynolds
Copy link
Contributor Author

nickreynolds commented Nov 22, 2022

/**
 * Converts the publicKey of a VerificationMethod to hex encoding (publicKeyHex)
 *
 * @param pk - the VerificationMethod to be converted
 * @param convert - when this flag is set to true, Ed25519 keys are converted to their X25519 pairs
 * @returns the hex encoding of the public key
 *
 * @beta This API may change without a BREAKING CHANGE notice.
 */
export function extractPublicKeyHex(pk: _ExtendedVerificationMethod, convert: boolean = false): string {
  let keyBytes: Uint8Array
  if (pk.publicKeyHex) {
    keyBytes = u8a.fromString(pk.publicKeyHex, 'base16')
  } else if (pk.publicKeyBase58) {
    keyBytes = u8a.fromString(pk.publicKeyBase58, 'base58btc')
  } else if (pk.publicKeyBase64) {
    keyBytes = u8a.fromString(pk.publicKeyBase64, 'base64pad')
  } else return ''
  if (convert) {
    if (['Ed25519', 'Ed25519VerificationKey2018'].includes(pk.type)) {
      keyBytes = convertPublicKeyToX25519(keyBytes)
    } else if (!['X25519', 'X25519KeyAgreementKey2019'].includes(pk.type)) {
      return ''
    }
  }
  return u8a.toString(keyBytes, 'base16')
}

it's possible that the issue is restricted to the publicKeyMultibase case since that's the only one that doesn't seem covered

@nickreynolds
Copy link
Contributor Author

it's possible that this is fixed by: #1030 (which wasn't deployed on IIW demo agent)

but need to add integration test to confirm

@mirceanis mirceanis added the pinned don't close this just for being stale label Nov 23, 2022
@nickreynolds
Copy link
Contributor Author

branch with unit tests that expose the issue:

https://github.com/uport-project/veramo/compare/nickreynolds/add-packing-unit-test?expand=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pinned don't close this just for being stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants