[sw,cryptolib] Hardened bn.sel for internal point mult #28462
Conversation
h-filali
requested review from
johannheyszl,
moidx and
nasahlpa
and removed request for
a team
h-filali
force-pushed
the
cryptolib-p384-harden-bnsel
branch
from
05dd29f to
423d12e
Compare
This commit adds hardening for the internal scalar point multiplication. Before the shares would be combined in the main loop to decide whether 0, P or 2P should be added. This commit determines this without combining the shares. If the MSB of both shares are both 1 we want to add 2P. If only one bit is 1 we want to add P. If both bits are 0 we want to add 0. In a simplified way this is done as explained below. In a first step we pick P if MSB(d0) ^ MSB(d1) is 1 and 2P otherwise. This is done as follows: p_temp0 = d0 ? (d1 ? 2P : P) : (d1 ? P : 2P) In a second step we pick between p_temp and 0. If MSB(d0) | MSB(d1) = 1 then we pick p_temp otherwise we pick 0. This is done as follows: p_temp1 = d0 ? p_temp0 : 0 p_temp2 = d1 ? p_temp0 : p_temp1 Signed-off-by: Hakim Filali <[email protected]>
h-filali
force-pushed
the
cryptolib-p384-harden-bnsel
branch
from
423d12e to
acc92fc
Compare
Thanks @nasahlpa ! |
|
thanks @h-filali this is a very helpful addition. can we please mention the impact on code size (ca. 300 Bytes?) and runtime (seem ok, ca. 30k) |
| bn.lid x2, 448(x30) | ||
|
|
||
| /* Select whether P or 2P will be stored in dmem[p_temp1]. | ||
| ([w21,w20], [w19,w18], [w17, w16]) <= M ? 2P : P */ |
There was a problem hiding this comment.
target regs were used for Q in last iteration; no horizontal leakage; no randomisation prior use required
|
The execution times change from
to:
The mem size goes from 96'961 to 97'233 Bytes |
|
Thanks @nasahlpa and @johannheyszl for leaving a review! |
|
|
||
| /* select either Q or Q_a | ||
| if M: Q = ([w21,w20], [w19,w18], [w17,w16]) <= Q else: Q <= Q_a */ | ||
| if M: Q_temp = ([w21,w20], [w19,w18], [w17,w16]) <= Q_a else: Q_temp <= Q */ |
There was a problem hiding this comment.
Nit: I was a bit confused just with the comments, Q_a was not defined before
| /* select either Q or Q_a | ||
| if M: Q = ([w21,w20], [w19,w18], [w17,w16]) <= Q else: Q <= Q_a */ | ||
| if M: Q_temp = ([w21,w20], [w19,w18], [w17,w16]) <= Q_a else: Q_temp <= Q */ | ||
| bn.sel w16, w25, w6, M |
There was a problem hiding this comment.
w16 to w21 contained the decision P or 2P, is that overwrite safe? Could definitely be, but I wasn't sure myself
4 participants
This commit adds hardening for the internal scalar point multiplication.
Before the shares would be combined in the main loop to decide whether 0, P or 2P should be added.
This commit determines this without combining the shares. If the MSB of both shares are both 1 we want to add 2P. If only one bit is 1 we want to add P.
If both bits are 0 we want to add 0.
In a simplified way this is done as explained below.
In a first step we pick P if MSB(d0) ^ MSB(d1) is 1 and 2P otherwise. This is done as follows:
p_temp0 = d0 ? (d1 ? 2P : P) : (d1 ? P : 2P)
In a second step we pick between p_temp and 0.
If MSB(d0) | MSB(d1) = 1 then we pick p_temp otherwise we pick 0. This is done as follows:
p_temp1 = d0 ? p_temp0 : 0
p_temp2 = d1 ? p_temp0 : p_temp1
Thanks to @siemen11 for implementing this for p256 and letting me draw inspiration #28248.