All, Apologies... I had only read the top of the patch before replying.
All my comments had already been addressed in the actual patch. Sorry for the noise. -chris On 1/12/18 6:00 PM, Christopher Schultz wrote: > Adam and David, > > On 1/8/18 11:30 AM, Adam Petcher wrote: >> On 1/8/2018 10:13 AM, David CARLIER wrote: >> >>> Hi, >>> >>> Here a little patch proposal which is usually relevant in >>> cryptographics matters. Usually memset/bzero/... is used to clear >>> private structures but the compiler can possibly optimize those calls >>> but with this change we can unsure sensitive data is properly zero'ed >>> using if possible native calls or memory fence. >> >> SunEC doesn't really make an effort to zeroize sensitive data, and all >> of the memset operations except for one (line 418) operate on memory >> that is not sensitive. While the patch is a relatively simple change >> that probably doesn't hurt anything, it doesn't seem to me like this >> improvement is particularly valuable. Perhaps it would be more valuable >> along with a larger improvement to make SunEC zeroize all intermediate >> values. Though this would be a much larger undertaking, and it still may >> not be useful on its own because the Java code in the provider also >> holds some sensitive values. > > Also, if you want to "sanitize" memory, you ought to: > > 1. use explicit_bzero instead of bzero, as bzip may be optimized-away by > the compiler[1] > > 2. use memset instead of bzero, as memset is POSIX[2] and bzero is not[3] > > 3. use memset_s instead of memset, since memset_s is guaranteed not to > be optimized-away by the compiler[4]. Its presence is not guaranteed, so > use compiler macros to ensure you have a backup plan if it is not > available (e.g. use memset or manual memory-scrubbing). > > 4. On Windows, use SecureZeroMemory[5], for reasons similar to the above > > -chris > > [1] https://www.freebsd.org/cgi/man.cgi?query=explicit_bzero > [2] https://linux.die.net/man/3/memset > [3] https://linux.die.net/man/3/bzero > [4] http://en.cppreference.com/w/c/string/byte/memset > [5] https://msdn.microsoft.com/en-us/library/windows/desktop/aa366877.aspx >
signature.asc
Description: OpenPGP digital signature
