lattigo icon indicating copy to clipboard operation
lattigo copied to clipboard

Fix #457 `utils.MaxSlice` and `utils.MinSlice`

Open romainbou opened this issue 1 year ago • 3 comments

Addressing #457

  • Fix utils.MaxSlice and utils.MinSlice returning 0
  • Fix EqualSlice if slices have difference lengths
  • Add tests for slices utils

romainbou avatar May 14 '24 15:05 romainbou

EqualSlice is not constant time anymore after these changes

Pro7ech avatar May 16 '24 16:05 Pro7ech

EqualSlice is not constant time anymore after these changes

Ah yes, but before it would also panic early depending on the size of b.

It's essentially the implementation of the standard library. (available since Go 1.21, we could use it once we drop older Go version support) https://cs.opensource.google/go/go/+/refs/tags/go1.22.3:src/slices/slices.go;l=18

The only critical use is here: https://github.com/tuneinsight/lattigo/blob/c031b14be1fb3697945709d7afbed264fa845442/mhe/keygen_evk.go#L131 Do you think it's really a concern?

romainbou avatar May 17 '24 08:05 romainbou

EqualSlice is not constant time anymore after these changes

Ah yes, but before it would also panic early depending on the size of b.

It's essentially the implementation of the standard library. (available since Go 1.21, we could use it once we drop older Go version support) https://cs.opensource.google/go/go/+/refs/tags/go1.22.3:src/slices/slices.go;l=18

The only critical use is here:

https://github.com/tuneinsight/lattigo/blob/c031b14be1fb3697945709d7afbed264fa845442/mhe/keygen_evk.go#L131

Do you think it's really a concern?

To me it is not a concern since it is not used in critical code and but its behavior should then be documented (this is a cryptographic library). Since Go 1.21+ provides native support for this kind of utility, shouldn't it then be removed/replaced by it?

Pro7ech avatar May 17 '24 13:05 Pro7ech