Hi,
On 17-04-2020 17:36, Gert Doering wrote:
> On Fri, Apr 17, 2020 at 03:42:49PM +0200, Antonio Quartulli wrote:
>>>> -static inline int
>>>> -memcmp_constant_time(const void *a, const void *b, size_t size)
>>>> -{
>>>
>>> Not sure I understand the motivation for this change. "Just so uncrustify
>>> stops trying to change this" is not really strong.
>>
>> 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. >> On top of that, this is basically allowing us to re-use an existing >> openssl API when possible. > > True, but if that turns out to be a code complication and reduces > efficiency, I'm not convinced it's the right way to spend our cycles. > >>> Also, keeping the "inline" part would be good... this is in the per-packet >>> path. >> >> I am not sure it would work in this case, because the function is >> defined in a .c file now - it's not inlineable anymore outside of the >> mbedtls code..... > > This is why I'm not exactly happy with the change. > > We could do it "openvpn style" all in header files, or we could just > leave the function alone. This kind of code is a tricky balance between "prevent the compiler from optimizing it to a not-constant-time implementation" and "as much performance as we can get". Moving this responsibility to the crypto library seems like a good idea to me. And because our recommended data channel ciphers are AEAD ciphers for which the auth tag compare is handled internally by the crypto library, I don't care so much for the performance aspect. Want best security? Use AEAD! Want best performance? Use AEAD! You get the point. Use AEAD ;-) -Steffan
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
