Throw UnsupportedError on unknown algorithm in keys, signatures and encrypted session keys
#1523
Conversation
…lformed or unsupported packets
| } catch (err) { | ||
| if (err instanceof UnsupportedError) throw err; | ||
| // avoid throwing potentially sensitive errors | ||
| throw new Error('Error reading MPIs'); | ||
| } |
There was a problem hiding this comment.
Same here - can it throw anything else? And, is the UnsupportedError sensitive, here?
There was a problem hiding this comment.
This is parsing secret key material, so I'd rather not risk it. Even an MPI-bad-length error can be sensitive.
The Unsupported error is only thrown on wrong algo, which is already public information, so it's not sensitive, at least for now.
There was a problem hiding this comment.
Even an MPI-bad-length error can be sensitive
But do we throw any such error? From looking at the code I think we just return a Uint8Array with a different length in that case
There was a problem hiding this comment.
It seems the only thing that can throw is the Curve constructor (which can throw "Not valid curve"), but actually that should probably be an UnsupportedError as well
There was a problem hiding this comment.
If the declared length is too long, is it guaranteed that .subarray doesn't throw? Is it not up to the platform?
There was a problem hiding this comment.
We're temporarily leaving the if-else here; #1523 is a more permanent solution to avoid not hiding sensitive errors introduced by future changes.
larabr
force-pushed
the
parse-unknown-key-algorithms
branch
from
bba4451
to
3fd57a8
Compare
…rror messages with "Unknown"
larabr
changed the title
UnsupportedError on unknown algorithm in keys and signaturesUnsupportedError on unknown algorithm in keys, signatures and encrypted session keys
With this change, the relevant packets will be considered unsupported instead of malformed.
To merge after #1522 .