Add validation to decrypt passphrase - Closes #688 #709
Conversation
src/cryptography/encrypt.js
Outdated
|
|
||
| requiredProperties.forEach(([property, propertyName]) => { | ||
| try { | ||
| hexToBuffer(property); |
There was a problem hiding this comment.
Wouldn't it weird to have partial validation named validateEncryptedPassphrase.
For example, getTagBuffer still validates if the length is 16, which is still used in encrypted passphrase, but not validated here.
There was a problem hiding this comment.
Yes. How would you suggest doing it instead?
There was a problem hiding this comment.
oops sorry, forgot to put the suggestion.
Since we have method for each one with separate logic, maybe we should put the validation logic with meaningful message on each method?
src/cryptography/encrypt.js
Outdated
| [iv, 'IV'], | ||
| [salt, 'Salt'], | ||
| [tag, 'Tag'], | ||
| ]; |
There was a problem hiding this comment.
I would prefer to use Map instead of nested array. It'd save some memory. see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map Iterating Maps with for..of section
willclarktech
force-pushed
the
688-decrypt_passphrase_errors
branch
2 times, most recently
from
018b4de to
95bf5b8
Compare
willclarktech
force-pushed
the
688-decrypt_passphrase_errors
branch
from
95bf5b8 to
6432a45
Compare
Closes #688
What was the problem?
We had no validation when decrypting passphrases, with unhelpful error messages.
How did I fix it?
Added validation and helpful error messages.
How to test it?
Try to decrypt a passphrase with some missing parameters.
Review checklist
commit guidelines