See the paper “Attacking embedded ECC implementations through cmov side channels” for a description of the problem. @sipa and I discussed this briefly in NYC, and we thought maybe this alternative construction might improve the situation. Peter Schwabe also independently mentioned the same construction, and that they hoped to get around to a second paper looking at suggested countermeasures.
Edit: Link to paper: https://eprint.iacr.org/2016/923
The code changes look perfectly fine to me and pass tests, ACK on these.
gcc introduces some branches here with -O3. I had a glimpse at it and it could be that all of these are not dependent on secrets (wild guesses: checking whether r and a overlap? checking whether len is 0?). We need to have a closer look. I’m not really good in reading assembly, maybe someone wants to give it a try: https://godbolt.org/z/XqeEK5
Okay, I played around with this a little bit.
First, the branches are not an issue. These really only implement the loop, even though they looked different to me at first glance.
But GCC manages to factor out the computation of the entire mask half ^ rest and move it before the loop. This happens already with -O1 and even on AVR GCC (which is interesting because the attack in the paper targeted AVR as an example). My understanding is that the 0x55...55 constants (= 0101…01 as bits) here are a crucial part of the countermeasure because they ensure all XOR operations are performed with masks of the same Hamming weight (0101…01 or 1010…10 depending on the secret input bit).
So while this PR does not hurt, it does not really help if you use GCC, which is simply too clever for us. So I’m not sure if there’s a simple way that works across platform. What we could do for x86_64 specifically will be an implementation based on the CMOV instruction but even here we don’t know what that means for power analysis. At least it’s “guaranteed” to be constant-time and fast.
So while this PR does not hurt, it does not really help if you use GCC, which is simply too clever for us. So I’m not sure if there’s a simple way that works across platform.
#728 reminded me of this PR, and this variant should work:
0static void secp256k1_fe_cmov_limbs(uint32_t *r, const uint32_t *a, int len, int flag) {
1 int i;
2 uint32_t diff, r_i;
3 volatile const uint32_t half = 0x55555555UL;
4 volatile const uint32_t rest = half << flag;
5 for (i=0; i<len; i++) {
6 diff = r[i] ^ a[i];
7 r[i] ^= (diff & half);
8 r[i] ^= (diff & rest);
9 }
10}
Not sure why I didn’t think of volatile when I looked at this previously.
After discussing with @sipa and @jonasnick, we believe it’s worth to do this even if we’re not entirely sure how much it helps. (And we anyway have volatile everywhere in CMOV after #1257).
0static void secp256k1_fe_cmov_limbs(uint32_t *r, const uint32_t *a, int len, int flag) { 1 int i; 2 uint32_t diff, r_i; 3 volatile const uint32_t half = 0x55555555UL; 4 volatile const uint32_t rest = half << flag; 5 for (i=0; i<len; i++) { 6 diff = r[i] ^ a[i]; 7 r[i] ^= (diff & half); 8 r[i] ^= (diff & rest); 9 } 10}
I’ll open a follow-up PR with that volatile variant above.