| |||||||||||||
Cryptographic weakness on Debian systemsCryptographic weakness on Debian systemsPosted May 13, 2008 14:55 UTC (Tue) by pharm (guest, #22305)In reply to: Cryptographic weakness on Debian systems by bcl Parent article: Cryptographic weakness on Debian systems The patch just comments out the non-zeroing of the relevant buffers if I understand it correctly: $ diff -r -C5 openssl-0.9.8c-etch1/crypto/rand/md_rand.c openssl-0.9.8c- etch3/crypto/rand/md_rand.c *** openssl-0.9.8c-etch1/crypto/rand/md_rand.c Tue May 13 15:50:57 2008 --- openssl-0.9.8c-etch3/crypto/rand/md_rand.c Tue May 13 15:51:05 2008 *************** *** 269,282 **** MD_Update(&m,&(state[0]),k); } else MD_Update(&m,&(state[st_idx]),j); - /* - * Don't add uninitialised data. MD_Update(&m,buf,j); - */ MD_Update(&m,(unsigned char *)&(md_c[0]),sizeof(md_c)); MD_Final(&m,local_md); md_c[1]++; buf=(const char *)buf + j; --- 269,279 ----It does seem a little weird: if that was the only source of randomness, then it's not a very good source & needs fixing! On the other hand if it was just one source of randomness, then it shouldn't be that big a deal. Anyone on the "inside" able to comment?
(Log in to post comments)
Cryptographic weakness on Debian systems Posted May 13, 2008 15:09 UTC (Tue) by jwb (guest, #15467) [Link] Is it just me, or did the "fix" only back out the change in one of two places? See the patch which caused the problem: http://svn.debian.org/viewsvn/pkg-openssl/openssl/trunk/r... The offending change is in two places. Now see the "fix": http://svn.debian.org/viewsvn/pkg-openssl/openssl/trunk/c... The patch is only half undone. Oversight?
Cryptographic weakness on Debian systems Posted May 13, 2008 15:17 UTC (Tue) by hmh (subscriber, #3838) [Link] I suggest studying the utility mentioned in the advisory that verifies whether a key is weak or not. That goes directly to the effect of whatever bug is in question (which is what matters, here).
Cryptographic weakness on Debian systems Posted May 13, 2008 15:59 UTC (Tue) by hmh (subscriber, #3838) [Link] The key vulnerability check basically hashes the key and searches for it in a blacklist of 256Ki entries. The code says that blacklist is not known to be the complete set of weak keys, it could be just a subset. Further comments of that really means depend on studying the OpenSSL code at depth, which I hope someone will disclose soon.
Cryptographic weakness on Debian systems Posted May 13, 2008 15:17 UTC (Tue) by jond (subscriber, #37669) [Link] Only if PURIFY is defined. I'm not sure in which situations that happens, but I think it's valgrind-related. So the binary packages being distributed are fine.
Cryptographic weakness on Debian systems Posted May 13, 2008 15:50 UTC (Tue) by lambda (subscriber, #40735) [Link] Only if PURIFY is not defined. That line was being excluded from PURIFY because it was causing warnings about uninitialized data.
Cryptographic weakness on Debian systems Posted May 13, 2008 16:22 UTC (Tue) by IkeTo (subscriber, #2122) [Link] > The patch is only half undone. Oversight? Unlikely. The original intent is quite defensible: if some data is not initialized, you don't know whether it is coming from some attacker, so you shouldn't use it as part of the random number to generate your key. I expect that is the part that is left alone, in the function ssleay_rand_bytes. The "#ifndef PURIFY" macro probably is there because some tools detects that it is using uninitialized data, and would die or produce other ugly result if the code is allowed to run. But the patch change another function ssleay_rand_add as well. I'm wondering whether the buf being passed in is actually the data that it want to add to the random pool. If so, the original removal of line 274 probably drops nearly all randomness that the random number generator can ever obtain.
Cryptographic weakness on Debian systems Posted May 13, 2008 16:38 UTC (Tue) by IkeTo (subscriber, #2122) [Link] > It does seem a little weird: if that was the only source of randomness, > then it's not a very good source & needs fixing! Only if that part is really to "add uninitialised data"! Note that the comment is added by the original Debian patch, not in the original SSL library. The "buf" argument is actually passed by a call to the "add" function of the rand_meth_st interface, the interface defining how to collect random data for various pluggable methods. My thinking is that it might actually not be uninitialized data at all, but is instead the content handed over by a part of the interface for SSL random number pool that actually accept random data and add them to the pool, i.e., most of the "real" randomness available.
Cryptographic weakness on Debian systems Posted May 13, 2008 17:01 UTC (Tue) by pharm (guest, #22305) [Link] My thinking is that it might actually not be uninitialized data at all, but is instead the content handed over by a part of the interface for SSL random number pool that actually accept random data and add them to the pool, i.e., most of the "real" randomness available. Which makes perfect sense, except that I don't understand why purify & valgrind would complain about it in that case. Oh well. No doubt all will become clear eventually & I don't have time to chase down the logic right now...
Cryptographic weakness on Debian systems Posted May 13, 2008 17:19 UTC (Tue) by welinder (guest, #4699) [Link] Purify/Valgrind will complain if you read more than the initialized part. Still, whoever took out the entire initialization should not be trusted with security intensive code.
Cryptographic weakness on Debian systems Posted May 14, 2008 12:42 UTC (Wed) by dion (subscriber, #2764) [Link] That would be Kurt Roeckx: http://svn.debian.org/viewsvn/pkg-openssl/openssl/trunk/r... http://alioth.debian.org/users/kroeckx/
Cryptographic weakness on Debian systems Posted May 14, 2008 13:01 UTC (Wed) by DeletedUser32991 ((unknown), #32991) [Link] Kurt did ask about it on the upstream list, too.
Cryptographic weakness on Debian systems Posted May 14, 2008 18:55 UTC (Wed) by dion (subscriber, #2764) [Link] Yes, that should ward off the tar+feathers. When even openssl developers can't tell that the change is catastrophic then he might be excused. This does illustrate why blindly fixing warnings is a dangerous and bad idea, though.
Cryptographic weakness on Debian systems Posted May 13, 2008 17:12 UTC (Tue) by tialaramex (subscriber, #21167) [Link] It looks like someone found a screw sticking out, and having a hammer handy, decided to bash it flat. As you'd expect in security software, the result was disastrous. Something, somewhere, calls this function with potentially uninitialised data (or perhaps, a dodgy piece of analysis software only thinks it is unintialised because it can't find the initialiser). Maybe it's actually a test routine. Maybe it's one uninitialised byte caused by an off-by-one error somewhere. Either way it's irrelevant to this function. Rather than find and fix that minor mistake, someone with Debian checkin privileges "fixed" it by removing critical code from this function, silencing the warning and disabling Debian's security. I guess the Debian Security people will need to re-assess who gets to modify critical packages like this. It's one thing to trust that someone isn't going to deliberately sabotage a package (they could just as easily add malware to GNOME Games as to OpenSSL) and quite another to trust that they know what they're doing modifying complicated software like this to try to "fix" security problems.
Cryptographic weakness on Debian systems Posted May 13, 2008 17:35 UTC (Tue) by IkeTo (subscriber, #2122) [Link] See http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=363516 if you wanna see the process that creates the bug.
Cryptographic weakness on Debian systems Posted May 13, 2008 22:10 UTC (Tue) by philh (subscriber, #14797) [Link] ... quite another to trust that they know what they're doing modifying complicated software like this to try to "fix" security problems.Well, that would be fair comment if Kurt Roeckx (one of the Debian openssl maintainers) had taken it upon himself to make this change in isolation, but as you can see from this thread, the patch was mentioned to the openssl-dev list, without provoking negative comment, so it's difficult to know who one should be pointing fingers at. Mistakes happen -- looking for someone to blame isn't overly productive at the best of times, and when it is based on false premises, not at all.
|
|||||||||||||