By now, I’m sure you all have read about the OpenSSL bug discovered in Debian.
There’s a lot being written about it. There’s a lot of misinformation floating about, too. First thing to do is read this post, which should clear up some of that.
Now then, I’d like to think a little about a few things people have been saying.
People shouldn’t try to fix bugs they don’t understand.
At first, that sounds like a fine guideline. But when I thought about it a bit, I think it’s actually more along the lines of useless.
First of all, there is this problem: how do you know whether or not you understand it? Obviously, sometimes you know you don’t understand code well. But there are times when you think you do, but don’t. Especially when we’re talking about C and its associated manual memory management and manual error handling. I’d say that, for a C program of any given size, very few people really understand it. Especially since you may be dealing with functions that call other functions 5 deep, and one of those functions modifies what you thought was an input-only parameter in certain rare cases. Maybe it’s documented to do that, maybe not, but of course documentation cannot always be trusted either.
I’d say it’s more useful to say that people should get peer review of code whenever possible. Which, by the way, did occur here.
The Debian maintainer of this package {is an idiot, should be fired, should be banned}
I happen to know that the Debian programmer that made this patch is a very sharp individual. I have worked with him on several occasions and I would say that kicking him out of maintaining OpenSSL would be a quite stupid thing to do.
He is, like the rest of us, human. We might find that other people are considerably less perfect than he.
Nobody that isn’t running Debian or Ubuntu has any need to worry. This is all Debian’s fault.
I guess you missed the part of the advisory that mentioned that it also fixed an OpenSSL upstream bug (that *everyone* is vulnerable to) that permitted arbitrary code execution in a certain little-used protocol? OpenSSL has a history of security bugs over the years.
Of course, the big keygen bug is a Debian-specific thing.
Debian should send patches upstream
This is general practice in Debian. It happens so often, in fact, that the Debian bug-tracking system has had — for probably more than a decade — a feature that lets a Debian developer record that a bug reported to Debian has been forwarded to an upstream developer or bug-tracking system.
It is routine to send both bug reports and patches upstream. Some Debian developers are more closely aligned with upstream than others. In some cases, Debian developers are part of the upstream team. In others, upstream may be friendly and responsive enough that Debian developers run any potential patches to upstream code by them before committing them to Debian. (I tend to do this for Bacula). In some cases, upstream is busy and doesn’t respond fast or reliably or helpfully enough to permit Debian to make security updates or other important fixes in a timely manner. And sometimes, upstream is plain AWOL.
Of course, it benefits Debian developers to send patches upstream, because then they have a smaller diff to maintain when each new version comes out.
In this particular case, communication with upstream happened, but the end result just fell through the cracks.
Debian shouldn’t patch security-related stuff itself, ever
Well, that’s not a very realistic viewpoint. Every Linux distribution does this, for several reasons. First, a given stable release of a distribution may be older than the current state of the art upstream software, and some upstreams are not interested in patching old versions, while the new upstream versions introduce changes too significant to go into a security update. Secondly, some upstreams do not respond in a timely manner, and Debian wants to protect its users ASAP. Finally, some upstreams are simply bad at security, and having smart folks from Debian — and other distributions — write security patches is a benefit to the community.
“Especially since you may be dealing with functions that call other functions 5 deep, and one of those functions modifies what you thought was an input-only parameter in certain rare cases.”
Well, its usually pretty easy to tell when something on the stack is modifiable — when it’s handled as a pointer. If the function calls are passing ints by reference, that’s a hint that there’s something stupid afoot. But yes, it’s a challenge to go from finding a bug to writing a correct fix. It’s simple to load up gdb and watch a segfault in xkb, but it’s another thing to understand the correct fix (it frustrates me a bit when the upstream fix is a simple “if null -> return 0” — I could have done that!).
On a more general note — why isn’t openSSL using /dev/random? It seems as though the ideal solution is for security systems to use that and then make sure it’s perfect.
Sure, it’s only modifiable if it’s a pointer… but pointers are the only practical way to pass many things: strings, buffers of any type really, arrays… the list goes on.
There should be testing of patched programs before they are released, when feasible. This bug could have been caught by testing whether generated keys follow certain unexpected distributions.
In general, it is impossible to prove that something is random, and difficult to ascertain that something is sufficiently random, but I do think some kind of regression test could be useful. I’m thinking of doing something a bit like the visual noise plots used on random.org to demonstrate the php rand() rubbish-ness.
“Sure, it’s only modifiable if it’s a pointer… but pointers are the only practical way to pass many things: strings, buffers of any type really, arrays… the list goes on.”
That’s what const and paying attention to compiler warnings are for. As a professional programmer who has written in C for 30 years, I can say that linux code suffers seriously from lack of code quality. The only reason it isn’t even more horribly buggy than it is, is because so many bodies test open source code. But better written code wouldn’t need so many bodies to be bug-free.
“This bug could have been caught by testing whether generated keys follow certain unexpected distributions.”
Indeed. “peer review” is highly overrated. What would serve better is peers performing QA functions such as writing test cases.