Hacker News new | past | comments | ask | show | jobs | submit login

Hmm.. would it make sense to replace all calls of xmalloc with xcalloc as a preventive measure?



I don't think it is advisable to blindly replace like that in a widely used, audited and tested system. You need to think about more than just the bits in memory, for example timing attacks. And who knows, your "fix" could even expose some other latent bug. It's not clear cut.


I don't get this. How could leaving memory uninitialized be better than zeroing it? Using calloc could be redundant in places, in that the allocated object might get initialized some other way right after allocation. But I don't see how it could ever be wrong. (There might have been zeros in that memory already anyway.) There are languages that have no way to allocate uninitialized memory -- are you going to say they aren't appropriate for crypto algorithms?

And besides, exposing a latent bug is a good thing, no?


There are a couple of concrete worries I'd have:

* If you malloc something very large, on Linux at least (I don't know about the BSDs), it doesn't necessarily get allocated, and only requests memory from the system when you actually access it. It's a valid pattern to, say, malloc a large amount of memory, pass it to your own allocator as a pool, and use what you need from it. If you go and zero all that memory, then you've accessed it all, resulting in much increased memory usage and maybe OOM kills. (You can argue this is an issue with the Linux overcommit system, not with malloc, but that's the system we have today and we'd need to account for it.)

* Slow code is a deterrent for people deploying crypto. If it takes 5 milliseconds for an HTTP response and 50 for an HTTPS handshake, people are going to be hesitant about deploying HTTPS, or do something silly like use it to protect only the initial login. So you do need to worry about the time that your crypto takes as an operational concern for deployment, and balance that off against hardening and paranoia. Favoring either extreme too much results in less overall security.

* There are very few languages that are truly "appropriate" for crypto algorithms. Even C only kind of counts, and you have to be very careful about how you write it. You ideally need promises from the language that certain operations are constant-time and that the optimizer is going to do what you want and neither more nor less. Most of the languages I can think of that only allocate you zeroed memory also include risks on the order of garbage collection being triggered in the middle of your algorithm depending on what your secret data is, so that makes them definitely unsuitable. And most of the non-toy crypto libraries in higher-level languages end up being bindings of carefully-tuned C libraries, and in that sense _do_ quasi-unintentionally expose a way to allocate uninitialized memory, by going through the C allocation routines.


I think that if I were a maintainer of something like OpenSSL, I would want the rule to be that any use of plain malloc, as opposed to calloc, should be commented. A simple "malloc ok" would suffice if the reason was obvious; for example, the very next lines of code initialized the object to something other than zero. If the reason were more subtle, a longer explanation should be provided. I do agree that performance concerns can be relevant (though zeroing a small block of memory that you're about to write into anyway is very cheap).

The caveat I would want everyone to be aware of is that if the object is a struct that is initialized member-wise, malloc can still be dangerous, because if a member is subsequently added to the struct, one would have to manually review all points where the struct type is allocated to add the initialization for the new member. So I would approve of malloc in such a case only if there were only one place in the program that allocated that struct type.

I'm talking about the crypto code itself. If you've written your own allocator, I agree that different rules apply for the allocator code.


"Most of the languages I can think of that only allocate you zeroed memory also include risks on the order of garbage collection being triggered in the middle of your algorithm depending on what your secret data is, so that makes them definitely unsuitable. "

You do realize that even a C program running on a modern kernel can be preempted? Unless you are running it as a high-priority real-time process on a real-time hardened kernel, and, OpenSSL is giving you hard-real-time guarantees.


Yes, and there are certainly cache-timing attacks between processes (and even VMs) on the same physical hardware. That said, if you're disciplined enough to avoid all its awfulness, C is one of the least-bad languages for writing crypto; other things that compile directly to native code and offer C-spirited APIs for allocation and the like might be better. Having no allocation API that doesn't zero returned buffers is somewhat correlated with being high-enough level that it's a more-bad language to write crypto in.

In other words, there may not be a good language, but some languages (or runtimes, really) are certainly worse than others.


But, task preemption is most likely not correlated with the contents of secret data. It's pauses that correlate with different values in secret data that allow data to leak via timing attacks.


Remember when Debian's OpenSSL was reduced to issuing a predictable small number of keys instead of using the entire keyspace? It's because someone added a patch that in effect initialized memory that was supposed to be uninitialized.


I had forgotten, but I just read up on it [0]. I haven't actually studied the code, but it appears to me from this post that your characterization is not quite correct. They didn't just add code to initialize the memory; according to [0], it would actually have been correct to do so (see the second sentence under "Links and Notes"). Instead they did something slightly different from that, which was wrong.

[0] http://research.swtch.com/openssl


That's only half correct. The problem is that they reinitialized memory.




Consider applying for YC's Spring batch! Applications are open till Feb 11.

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: