feat: add recursive lookup for key exchange keys when encrypting data #203
Conversation
There was a problem hiding this comment.
Wonderful contribution, thank you!
There is an edge case that is not covered and I think fixing it is a must: the exit from recursion when there is a controller loop.
The inline method encryptersForDID() MUST check for already resolved controllers to prevent denial of service attacks.
Also, I believe a recursive controller resolution method is valuable also for verification, not just encryption.
Would you be willing to turn it into a utility method that accepts a set of verification relationships for filtering? In your case you would call it using ["keyAgreement"] as a filter.
src/xc20pEncryption.ts
Outdated
| if (didDocument.controller) { | ||
| const controllers = Array.isArray(didDocument.controller) ? didDocument.controller : [didDocument.controller] | ||
| const encrypterPromises = controllers.map((did) => | ||
| encryptersForDID(did).catch(() => { |
There was a problem hiding this comment.
There is no exit from this recursion in case of loop.
didA controlled by didB controlled by didA will lead to infinite loop and could be used to create a denial of service attack.
There was a problem hiding this comment.
Apologies for the delayed response, was away for a few days. I agree with your suggestions and will update the PR as such.
There was a problem hiding this comment.
Would you like for the utility function to support all the various key capabilities in KeyCapabilitySection and perhaps additionally controller?
There was a problem hiding this comment.
That would be nice, but it's not mandatory for this pr.
No need to apologize, we're all busy :)
There was a problem hiding this comment.
Okay, I'll get the infinite loop break in and then we can merge this, will do the utility function separately :)
There was a problem hiding this comment.
Addressed in 133080d
|
🎉 This PR is included in version 5.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Solves #202
Provides two additional test cases:
cc @oed