dedis / kyber

Advanced crypto library for the Go language
Other
641 stars 168 forks source link

Use fixed-length integers to improve cross-platform compatibility #492

Open pierluca opened 1 year ago

pierluca commented 1 year ago

As discussed in the review of https://github.com/dedis/kyber/pull/490 with @ineiti and @howjmay , Kyber uses structures with Go int-typed fields, which are unsized. This can lead to differences in serialization and hash computations across different processor architectures.

It would be preferable to switch to sized types (int32 and int64), which would provide consistent behaviours across platforms.

ineiti commented 12 months ago

Additional notes: this probably needs a major version change, as the fields are involved in the hashes. So if a 64-bit machine stored the fields as int, it would store them as 64-bit fields. Then if we change the int to int32, it would be incompatible. The same thing would happen with a 32-bit system and changing the int to int64.

However, #493 suggests that this never worked on a 32-bit system. So I think if we change every int to int64, it should work.

pierluca commented 3 months ago

Following #529 , Kyber now compiles and works in both 32-bit and 64-bit architectures. This effectively makes this issue more problematic.

pierluca commented 3 months ago

I've added you (@AnomalRoil and @K1li4nL) and me as assignees so that we can discuss this and decide together the best way forward ahead of the v4 release.

pierluca commented 2 weeks ago

Discussed with @K1li4nL , we might want to do some automatic AST changes. Assigning @jbsv who may be able to help.

ineiti commented 2 weeks ago

Why not changing every int to either int32 or int64? As additional security measure, tell protobuf to panic if it sees an int.