verifyJWT behavior when didDocument.verificationMethod property has multiple public key sets

[siacomuzzi]

Please correct me if I'm wrong but when verificationMethod property in the DID Document contains multiple public key sets, all of them are tested during signature verification until find the correct one. Some examples:

ES256

did-jwt/src/VerifierAlgorithm.ts

Lines 79 to 86 in 4efd9a7

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

ES256K

did-jwt/src/VerifierAlgorithm.ts

Lines 106 to 113 in 4efd9a7

	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
	}
	})

Ed25519

did-jwt/src/VerifierAlgorithm.ts

Lines 176 to 178 in 4efd9a7

	const signer = authenticators.find((pk: VerificationMethod) => {
	return verify(extractPublicKeyBytes(pk), clear, sig)
	})

Could you please explain why library is doing this instead of using, for example, JWT's header.kid to take just one public key from the authenticators array?

Thank you!

[mirceanis]

I think it was mostly for convenience, in case there wasn't a kid specified in the header, but then the actual check for a kid was never added. Thanks for raising this issue!

If a kid is mentioned in the header, it should be used, indeed.

Would you like to contribute a PR with a fix?

[bumblefudge]

Also note, there have been some discussions of kid security in the VC-JWT repos lately, with implementer feedback coming from the OIDF community that relative kid references (i.e. #key1) leak less information that absolute ones (in flows where the JWK/token arrives separately):
w3c/vc-jose-cose#31 (comment)

I would tend to think that, as Kristina points out, both absolute and relative kids will probably persist in different architectures and veramo should prolly support both some day. but a PR implementing either is still infinitely more useful and welcome than no PR haha

[mirceanis]

I would imagine that it wouldn't be that hard to support both absolute and relative kid for this.
There are 3 situations:

• iss is a DID
  • kid CAN be relative and will be extracted from DID doc
  • kid CAN be absolute, but MUST belong to the issuer (appear in DID doc)
• iss is not specified
  • kid MUST be absolute DID URL

I don't have enough of a grasp of how things should work for non DID issuers. I wouldn't be opposed to adding functionality for that situation too.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.