signalapp / libsignal

Home to the Signal Protocol as well as other cryptographic primitives which make Signal possible.
GNU Affero General Public License v3.0
3.62k stars 420 forks source link

PreKeyRecord.getKeyPair should not have a checked exception #458

Closed juanpmarin closed 1 year ago

juanpmarin commented 2 years ago

https://github.com/signalapp/libsignal/blob/ba47c9d681eefdf9196e8d708e67549ac1301364/java/shared/java/org/signal/libsignal/protocol/state/PreKeyRecord.java#L44

As you can see there, the getKeyPair has a checked InvalidKeyException, it is not necessary as the keys are already validated and it gets very annoying to catch this exception every time I access the pre key

jrose-signal commented 2 years ago

Oops, you're right. Bulk-updating exception specs and got this one wrong!

jrose-signal commented 2 years ago

Wait, no, sorry. The keys are validated lazily if you load the record from the serialized form. Maybe they should be validated eagerly, but they currently aren't.

juanpmarin commented 2 years ago

thanks for the reply! it would be amazing if they are validated eagerly.

jrose-signal commented 2 years ago

I'll keep this issue open for that. It's an extra up-front, non-optimizable check, but there's not much point in a PreKeyRecord with invalid data.

stale[bot] commented 1 year ago

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[bot] commented 1 year ago

This issue has been closed due to inactivity.