Added implementation of FrodoKEM-640-SHAKE-CCA.

[xvzcf]

Also tagging @dstebila

[bwesterb]

Thanks Goutam! Looks good on first pass. Haven't checked the nitty-gritty details (yet).

[claucece]

Looks good! As this doesn't have the proofs yet, I'm leaving some details out that will be caught by the proofs.

[claucece]

While this is following more the 'C-like' syntax, you can always return the parameter instead of passing it as argument...

[bwesterb]

Returning slices might lead to allocations that can be prevented like this.

[bwesterb]

(By passing an "out" slice, you make the caller responsible for the allocation.)

[armfazh]

some minor changes

[armfazh]

here does it need reduction? if not, please add a comment why is not the case.

[armfazh]

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

[armfazh]

when sign=1, sampled gets sampled = (0xFF..FF ^ gaussianSample) + 1
is that final plus one part of the result?

[xvzcf]

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?

[armfazh]

yes please

[bwesterb]

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.

[bwesterb]

Thanks @xvzcf !