Redundant length check in secure_cmp

[jorickert]

Why does this function return a Result instead of a simple bool?
I'm not even sure if Result is usable if you want a cost-time execution.
The check for the same length is also not necessary, subtle does this already.

https://github.com/brycx/orion/blob/a6efb6f9094c71a17a9ce07fb8e288d5bd808dd2/src/util/mod.rs#L97

[brycx]

Why does this function return a Result instead of a simple bool?

Returning a Result makes it easier to propagate a failed verification in other functions which also return a Result. So we avoid adding extra if-else statements. Though I admit returning a bool only if the verification succeeded may be redundant, since you could also use .is_ok()/.is_err() and have Result<(), UknownCryptoError>. I haven't tested whether that executes in constant-time.

I'm not even sure if Result is usable if you want a cost-time execution.

AFAIK Result only impacts constant-time execution when run with opt-level = 0. In the orion-dudect repository, secure_cmp is tested for constant-time execution. If you're interested, I have a write-up that outlines how I've tested the secure_cmp function in the past.

The check for the same length is also not necessary, subtle does this already.

The length check is indeed redundant and should be removed. Nice spot!

[brycx]

Fixed in #93.