Recently a security vulnerability was identified in Debian the result of which was:
anyone doing pretty much any crypto on Debian (and hence Ubuntu) has been using easily guessable keys. This includes SSH keys, SSL keys and OpenVPN keys. [Ben Laurie]
Most commentators have focussed on the technical problems created by this bug. In this post I’m going to look at what we can learn from the development models of Debian and OpenSSL. That is, how can we stop this happening again?
First, a little background. Debian developers removed two lines of code from their local version of OpenSSL in order facilitate debugging. This seemingly innocuous change meant that all security keys generated by Debian, or a Debian based distribution, were insecure. Andy Saunders, one of our local IT people working on this tells me that it would take approximately 1.5 hours to crack a Debian generated key by brute force.
In a word “OUCH!!!”.
Since Debian is used as a base for a number of other Linux distributions the bug has effected a large number of users. Furthermore, keys that are generated by a Debian system are affected. This means that even if you don’t run a Debian based system, you could still be using weak keys.
In lots of words “OUCH!!!”
How did this happen?
There are two groups who have to take responsibility for this:
- The Debian maintainers
- The OpenSSL maintainers
Submitting Patches Upstream
It seems to me that the Debian maintainers failed to communicate the patch upstream to the OpenSSL maintainers:
Had Debian done this in this case, we (the OpenSSL Team) would have fallen about laughing, and once we had got our breath back, told them what a terrible idea this was. [Ben Laurie]
I can’t argue with that, but some people feel that waiting for upstream projects to processes patches causes a block on ones own roadmap. For example, Lui Mondesi says:
I am used to submitting patches upstream and not getting them to approve the patch in a timely fashion. Sometimes we need these patches applied to stuff that’s broken and need fixing on a set schedule, so, we patch the broken upstream sources until we get a response from upstream…
That’s also a fair comment. I myself have patched things locally in order to keep my own work progressing, although I have never released anything with patched code (and I probably never will).
I’ve always worked with the upstream project to ensure the patch is contributed there first, otherwise I’m creating a fork and that means I have to maintain it and take responsibility for it. Working with the upstream project in the short term takes less time than maintaining a fork in the long term.
Conversely, some people have suggested that upstream maintainers have a responsibility to monitor activity in the downstream users of their package. However, I’m not sure that is really possible. Do we really expect people to monitor activity everywhere that open source code is being repackaged? For a small project being bundled in one or two locations that’s fine, but for something like OpenSSL that appears in hundreds of downstream projects it simply is not viable without significant developer resources.
In my opinion it is the responsibility of the downstream packagers (Debian in this case) to notify upstream maintainers (OpenSSL) of any changes that have been made. This is clearly the most efficient process. Even with limitless resources we’d still want to take the most efficient route, right?
Working with downstream communities
Whilst Debian didn’t submit a patch, they did try to discuss their plans and were told it was OK, as well as being given an alternative. However, Ben tells us that the original description of the problem in this thread was wrong, this resulted in the OpenSSL developers giving bad advice:
Had they [the OpenSSL developers who commented] been inspired to check carefully what these lines of code actually were, rather than believing the description, then they would, indeed, have noticed the problem and said something, I am sure.
Looking at the post that started the discussion about this change, I see it contains a number of incorrect assumptions about what the code does. I also notice that it is not clear that the proposal is actually to put this into release code, nor does it mention Debian anywhere in the mail.
So, it would seem that someone who didn’t understand the code, made incorrect assumptions, these assumptions were communicated to those who should know better. Unfortunately, “those who know” didn’t actually check the details.
Clearly, upstream notification of intention is not sufficient. The upstream project must fully evaluate the proposed changes and give clear and accurate feedback. This feedback must be made in a timely fashion in order to enable downstream projects to continue to move forwards.
Clear lines of communication
Debian asked for feedback on their plans. However, Ben observes (in comments to the above linked blog post) that this inquiry went to the wrong mailing list and therefore was not going to get the attention of the key folk in the OpenSSL team. How did this happen?
When most people want to engage with the development team of a project they follow best practice in the FLOSS world. That is, they go to the project web site, they search the site and relevant mail list/forum archives and, if they don’t find what they want, they join the appropriate list (as identified on the project web site) and ask their question.
I was “amused” to note that the code in question is the subject of a FAQ item on the OpenSSL site. Since this is a FAQ item one can assume that it comes up many times in the OpenSSL discussions. Why didn’t the Debian devs check the FAQ and mailing lists before making their incorrect assumptions? Why didn’t the OpenSSL team spot the relationship between the original (innacurate) posting and an item in their FAQ?
Next we have to consider how the Debian team managed to reach the wrong mailing list (according to Ben)?
The OpenSSL website currently describes the dev list used as “Discussions on development of the OpenSSL library”. This description is then followed by the rather confusing clarification of “Not for application development questions!”. No other list is suggested for such questions. So that’s as clear as mud then.
Lets try the FAQ, where we find this entry:
The README file describes how to submit bug reports and patches to OpenSSL. Information on the OpenSSL mailing lists is available from http://www.openssl.org.
We’ve already looked at the mailing lists page, so lets check the README referred to in the FAQ above. It says:
If you would like to submit a patch, send it to email@example.com with the string “[PATCH]” in the subject.
So the Debian team did use the right list to discuss this (at least according to all the info provided by OpenSSL).
Would this have happened if these were closed source products?
There are, as you would expect, numerous commentators claiming that this would never have happened in a closed source team. However, there is no way we can say it would never happen, we can only say whether it could happen in a closed team.
Looking at my analysis of the causes above I’m totally convinced that this could happen in a closed team too. It is inconceivable that any team, closed or open, would have a member who understands every nuance of every line of code in anything but the simplest of applications. So the incorrect assumptions and the mistaken patch could happen anywhere.
Some people may claim that in a closed team there would not be the confusion over how to report issues that we find in the OpenSSL project. This may be true of some closed teams, but it is certainly not true in my experience. In fact, documentation of such processes tends to be worse in closed teams since there is no need for external documentation, it’s OK to keep it in the project managers head. But project managers change and that knowledge gets lost or confused.
Other people may claim that in a closed team there would either be strict controls over who can change code in certain parts of the system. That is, packagers would not be able to change code in the security libraries. The problem here is that Debian forked the OpenSSL code in order to make this change. The Debian team didn’t have commit access to the OpenSSL code and clearly, with a gaff like this, they shouldn’t have such access. The controls were in place, but the Debian team circumvented them.
It is true that the open source nature of the code allowed this bug to be introduced, however, it is a failing of the Debians implementation of the open source development model, not a failing of the model itself. Had Debian done the right thing and worked with the OpenSSL team on this change then the OpenSSL experts would have rejected the change.
So lets look at how to avoid these failings in the future.
Best Practice in open source development
For those making changes in upstream projects:
- Use the project memory to understand code
When working with open source code a little desk research goes a long way to helping you understand what is going on. When you find something that doesn’t seem to be sensible or you feel can be improved, a quick search of mailing lists and web sites will give you an insight into why something was designed in a particular way. This will either convince you that your idea was wrong (in which case you learnt something) or you will be able to phrase your communications based on at least some understanding of the code.
- Never patch code you don’t understand
If you don’t understand why something is the way it is, don’t touch it. If it is broken, and you can prove it is broken, then escalate the problem to the project maintainers through the proper channels.
- A patch is less ambiguous than a description of intended actions
When discussing ideas with project developers be aware that a patch is less open to misinterpretation than a description of your actions. Wherever possible provide a patch to inform discussion.
- If forced to patch open source locally always pass the patch upstream
There is no excuse for patching code locally and not submitting the patch to the upstream project. failure to do this poses a maintenance problem for yourself, dilutes the contributor pool for the upstream project and can result in reduced quality control in your own release.
- Avoid releasing patched code
Releasing patched versions of other projects code is forking that project. At the point of the fork you become responsible for the maintenance of that fork – do you really want to do that? Instead, put a little extra effort into working with the upstream project to ensure that your patches are included before you release. At the very least have a developer review your patch prior to release. If you really must distribute a patched version of an application document this as clearly as possible in your issue tracker and link to that from the upstream projects tracker. Review all patched releases on a regular basis, prompting the upstream project to address your concerns.
For those managing the upstream projects:
- Don’t trust assumptions made by people asking for guidance
Don’t assume that background information in an email is accurate. Actually check the code under discussion and make your suggestions based on your own conclusions not those of the questioner.
- Have an easy contribution path
Follow best practice in the open source world and use your developer list for developer discussions. Have a clearly defined process for receiving and processing patches. Ensure your processes actually match your documentation.
- When receiving a patch review it quickly
If you don’t give feedback on patches quickly the user may be forced into forking your code. That’s bad for you and them.