Added implementation of FrodoKEM-640-SHAKE-CCA. #311
Conversation
| } | ||
| } | ||
|
|
||
| func mulAddSAPlusE(out []uint16, s []uint16, e []uint16, A []uint16) { |
There was a problem hiding this comment.
While this is following more the 'C-like' syntax, you can always return the parameter instead of passing it as argument...
There was a problem hiding this comment.
Returning slices might lead to allocations that can be prevented like this.
There was a problem hiding this comment.
(By passing an "out" slice, you make the caller responsible for the allocation.)
| } | ||
|
|
||
| for j := 0; j < paramN; j++ { | ||
| A[(i*paramN)+j] = uint16(ARow[j*2]) | (uint16(ARow[(j*2)+1]) << 8) |
There was a problem hiding this comment.
here does it need reduction? if not, please add a comment why is not the case.
kem/frodo/frodo640shake/util.go
Outdated
| } | ||
| } | ||
|
|
||
| func mulAddSBPlusE(out *[paramNbar * paramNbar]uint16, b *[paramN * paramNbar]uint16, s []uint16, e []uint16) { |
There was a problem hiding this comment.
suggestion: we can make types to help on passing parameters correctly.
type nb_nb [paramNbar * paramNbar]uint16
must distinguish when cardinality coincides, e.g.
type n_nb [paramN * paramNbar]uint16
type nb_n [paramNbar * paramN]uint16
| for j := 0; j < CDFTableLen-1; j++ { | ||
| gaussianSample += (CDFTable[j] - unifSample) >> 15 | ||
| } | ||
| sampled[i] = ((-sign) ^ gaussianSample) + sign |
There was a problem hiding this comment.
when sign=1, sampled gets sampled = (0xFF..FF ^ gaussianSample) + 1
is that final plus one part of the result?
There was a problem hiding this comment.
Yes, since flipBits(a) + 1 ≡ -a (mod 2^16) if a is 16 bit number, this just flips the sign of gaussianSample if sign = 1. Should I add a comment here?
There was a problem hiding this comment.
Thanks for this PR and your previous changes.
I’ve gone through the code in detail on a plane. I would like some more documentation on what is going on and there is some low hanging fruit for optimization. I don’t think they’re blockers. I didn’t have internet on the plane, so I’m sorry that I’ll have to leave my comments as one big list.
- note in comment that mbar is nbar
- Note in comment that extractedbits is B
- Reading/writing to shake never errors so we don’t need to check. Use _, _ = h.Read to prevent compiler warning.
- Keygen
- Note in comment that it’s S transpose, not S
- Why is the seed as it is? Doesn’t seem to be defined in the FrodoKEM spec. Also, I’d say 128 bits should be enough.
- The seed doesn’t contain seed_A (instead it contains z, with seed_A=shake(z)) so it doesn’t make sense for len_seedA to be part of the definition of KeySeedSize
- You can remove the copy for shakeInputForSE by first writing a single byte and then the remainder.
- It would be nice to add a test that the CDFTable indeed corresponds to Table 3 in the spec.
- Although it’s all pretty standard, it wouldn’t hurt to add a comment or two what is going on in sample().
- Please add a comment to note how you make a 2-dimensional array 1-dimensional. (Ie. Rows after each other.)
- You’re not reducing in expandSeedIntoA. Probably that doesn’t matter but it deserves a comment that you’re leaving that out on purpose.
- If you want you can get rid of some extra copy()s. For instance, you can sample directly into sk.matrixS
- Encapsulation
- This is probably premature optimization (— no need to look into it), but it might be faster to read one block of shake at a time and then call sample() on it before doing the next shake block. That reduces pressure on caches. Same goes for keygen.
- Decapsulation
- There is some shared code between encaps and decaps (generation of S’ and E’). Perhaps move that into a function?
- Check whether Go turns ((1 << logQ)-1) into a constant or recomputes it.
- Pack: This could be made much more efficient by packing eight at a time. Not a blocker.
- Encode/DecodeMessage: we know the size of msg in advance. The go compiler is too stupid to realize so better make it see by using constant-size array.
Co-authored-by: Armando Faz <armfazh@users.noreply.github.com>
Co-authored-by: Armando Faz <armfazh@users.noreply.github.com>
|
Thanks @xvzcf ! |
Also tagging @dstebila