Reduce side-channels from single-bit reads

Also make secp256k1_scalar_get_bits work with count=32.

Author: sipa

src/ecmult_gen_impl.h

@@ -152,10 +152,11 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
 #if COMB_BITS > 256
                 } else if (EXPECT(bit_pos >= 256, 0)) {
                     /* Some bit(s) of (mask(block) << comb_off) are outside of [0,256). This means
-                     * we are also done constructing bits, but know its top bit is zero, and no
+                     * we are also done constructing bits, but know its top bit should be zero, and no
                      * flipping/negating is needed. The table lookup can also be done over a
                      * smaller number of entries. */
-                    VERIFY_CHECK(bits < (1U << tooth));
+                    /* Mask out junk in bits variable. */
+                    bits &= ((1U << tooth) - 1);
                     VERIFY_CHECK(bits < COMB_POINTS);
                     for (index = 0; (index >> tooth) == 0; ++index) {
                         secp256k1_ge_storage_cmov(&adds, &secp256k1_ecmult_gen_prec_table[block][index], index == bits);
@@ -164,10 +165,16 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
                     break;
 #endif
                 } else {
-                    /* Gather another bit. */
-                    uint32_t bit = secp256k1_scalar_get_bits(&recoded, bit_pos, 1);
+                    /* Gather another bit. To reduce side-channels from single-bit reads, don't
+                     * actually fetch a single bit, but read higher bits too, which are XOR'ed
+                     * into the upper bits of bits. On every iteration, an addition bits is
+                     * made correct, starting at the bottom. The bits above that contain junk.
+                     * See https://www.usenix.org/system/files/conference/usenixsecurity18/sec18-alam.pdf
+                     */
+                    uint32_t bitdata = secp256k1_scalar_get_bits(&recoded, bit_pos & ~0x1f, 32) >> (bit_pos & 0x1f);
                     VERIFY_CHECK(bit_pos < COMB_BITS && bit_pos < 256);
-                    bits |= bit << tooth;
+                    bits &= ~(1 << tooth);
+                    bits ^= bitdata << tooth;
                     bit_pos += COMB_SPACING;
                     ++tooth;
                 }

src/scalar_4x64_impl.h

@@ -42,7 +42,7 @@ SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsig
 
 SECP256K1_INLINE static unsigned int secp256k1_scalar_get_bits(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
     VERIFY_CHECK((offset + count - 1) >> 6 == offset >> 6);
-    return (a->d[offset >> 6] >> (offset & 0x3F)) & ((((uint64_t)1) << count) - 1);
+    return (a->d[offset >> 6] >> (offset & 0x3F)) & (0xFFFFFFFFU >> (32 - count));
 }
 
 SECP256K1_INLINE static unsigned int secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
@@ -52,7 +52,7 @@ SECP256K1_INLINE static unsigned int secp256k1_scalar_get_bits_var(const secp256
         return secp256k1_scalar_get_bits(a, offset, count);
     } else {
         VERIFY_CHECK((offset >> 6) + 1 < 4);
-        return ((a->d[offset >> 6] >> (offset & 0x3F)) | (a->d[(offset >> 6) + 1] << (64 - (offset & 0x3F)))) & ((((uint64_t)1) << count) - 1);
+        return ((a->d[offset >> 6] >> (offset & 0x3F)) | (a->d[(offset >> 6) + 1] << (64 - (offset & 0x3F)))) & (0xFFFFFFFFU >> (32 - count));
     }
 }
 

src/scalar_8x32_impl.h

@@ -60,7 +60,7 @@ SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsig
 
 SECP256K1_INLINE static unsigned int secp256k1_scalar_get_bits(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
     VERIFY_CHECK((offset + count - 1) >> 5 == offset >> 5);
-    return (a->d[offset >> 5] >> (offset & 0x1F)) & ((1 << count) - 1);
+    return (a->d[offset >> 5] >> (offset & 0x1F)) & (0xFFFFFFFFU >> (32 - count));
 }
 
 SECP256K1_INLINE static unsigned int secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
@@ -70,7 +70,7 @@ SECP256K1_INLINE static unsigned int secp256k1_scalar_get_bits_var(const secp256
         return secp256k1_scalar_get_bits(a, offset, count);
     } else {
         VERIFY_CHECK((offset >> 5) + 1 < 8);
-        return ((a->d[offset >> 5] >> (offset & 0x1F)) | (a->d[(offset >> 5) + 1] << (32 - (offset & 0x1F)))) & ((((uint32_t)1) << count) - 1);
+        return ((a->d[offset >> 5] >> (offset & 0x1F)) | (a->d[(offset >> 5) + 1] << (32 - (offset & 0x1F)))) & (0xFFFFFFFFU >> (32 - count));
     }
 }
 

src/scalar_low_impl.h

@@ -20,7 +20,7 @@ SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsig
 
 SECP256K1_INLINE static unsigned int secp256k1_scalar_get_bits(const secp256k1_scalar *a, unsigned int offset, unsigned int count) {
     if (offset < 32)
-        return ((*a >> offset) & ((((uint32_t)1) << count) - 1));
+        return (*a >> offset) & (0xFFFFFFFFU >> (32 - count));
     else
         return 0;
 }