Fix todo in edwards25519

[thehoul]

Problem

i2OSP converts x > 0 to a byte string of the specified length xLen. To obtain the bytes, the int was converted into a compatible.Int and Bytes() was called on it using a temporary compatiblemod.Mod. The TODO requested to review this modulo used.

Indeed, the modulo is a problem: in the case of x = 256 and xLen=2, the program shoud return the byte array [1,0] but panics. This is because the modulo used is 127 and thus Bytes() panics because the Int (256) is larger than the modulus (127).

The first problem is that any value that fits on more than 1 byte passed as input will cause the program to panic, no matter the value of xLen.

The second problem is that in the context of an bad input (i.e. some x that requires more bytes than xLen), the program will panic (for the same reason) instead of returning an error. This is not an acceptable behaviour since depending on the inputs, the program will sometimes panic and sometimes return an error for the same reason. For backward compatibility, the program should never crash if the inputs are bad but return an error instead.

Fix

The modulus is now the largest value representable on xLen bytes. This ensures that as long as the input value x can fit on xLen bytes, the program will never panic.

This does not entirely fix the problem because, in the case of bad inputs (i.e x not representable on xLen) bytes, the modulo created will be too small and Bytes() will panic. We also need to check if x > mod. We can do this easily since both are uint64 and it automatically implies that x requires more than xLen (since the modulo created is the largest representable value with xLen bytes)

Thus:

1. The modulo is now (1 << (8 * xLen)) -1
2. To be able to create this compatiblemod.Mod, const_mod and var_mod now implement a method NewUint() to create a modulus from a uint64
3. Test for i2OSP has been added to enforce this behaviour

[github-actions]

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

[github-actions]

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

[jbsv]

I would try to remove the line

kyber/.github/workflows/tests.yml

Line 7 in 570c22f

branches: [ master, int32-repro ]

completely to avoid running it against the different go version as specified on master.

[jbsv]

LGTM