Hi, On 26-04-2020 11:34, Gert Doering wrote: > On Sun, Apr 26, 2020 at 11:25:49AM +0200, Steffan Karger wrote: >>>> well, sometimes to adhere to the codestyle, you have to re-arrange code :) >>> >>> "rearrange" and "rewrite in a not easy to understand way" (which looks >>> a bit overthought to me, TBH - unlike "secure memzero" I cannot see an >>> obvious reason why all that volatile would be relevant). >> >> This secure memcmp is relevant to avoid timing side channels in e.g. >> authentication tag compare. Think about the HMAC in our tls-auth/crypt >> and the HMAC of (non-AEAD) data channel packets. > > I do understand why it has to be constant *time*, in regards to "do the > compared buffers differ or not". > > I do not see how all this "volatile" and "copy from pointer to variables > to other stuff" handwaving is going to make any difference wrt constant > time comparison. > > And it hurts my eyes.
Pretty it is not. But "let's not try to outsmart the crypto library folks" makes sense to me. The copying stuff is probably just about making some annoying compiler happy. We could probably leave that out, but that makes it harder to see that we follow the mbed internal implementation. When inlining this without volatile keywords, a compiler might at some point become smart enough to realize "hey, this caller only cares about zero/nonzero, not the actual value. And this code will only return zero if *all* bytes are equal, so I can stop comparing as soon as soon as I've found a difference". Up to now, it seems compilers have not gotten this smart. But it's not like we're keeping a close eye on that. So I would love to get rid of the responsibility :-) -Steffan
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel