making security sausage
Security may be a process, not a product, but security patches are definitely a product. Some reflections on a few recent experiences making security sausage, er, patches.
I appear to have found myself in the position of OpenBSD sausage grinder even though it’s not a great fit. It’s not in my temperament to care about yesterday’s problems after they’re fixed, nor am I enthusiastic about long term support. I mostly run current, so I don’t have much personal interest in fixing stable. Unfortunately, I wrote the tool used for signing patches which somehow turned into a responsibility for also creating the inputs to be signed. That was not the plan!
Anyway, here are some comments about a few recent patches. Some of the circumstances are particular to OpenBSD, though I imagine anyone releasing patches for any other system has similar experiences.
First up, a simple case where everything went just right. Alejandro Hernandez reported a user triggerable local kernel panic with malformed ELF headers. I forwarded the email to a few developers to make sure it was noticed, and a few days later Mark fixed it. I waited a day, then backported the fix and we released a patch. Done. Five days elapsed, give or take a timezone or two.
The problem was easy to identify, clearly a bug, and easily fixed.
A bug that did not go so smoothly was diretory traversal in pax, better known by its alias, tar. The problem was clear enough, but the fixes weren’t. The obvious fix, to disallow creating ../../files outside the hierarchy would work, but leave a loophole where a naughty tar file creates symlinks that point outside the hierarchy, then use them to overwrite files. And it turns out real, not malicious, tar files create such links, complicating the fix. Meaning the fix doesn’t work. Unfortunately, we ran out of time trying to fix this correctly before 5.7, but it’s back in yet again and if things pan out, will get backported to 5.6 and 5.7.
Sometimes vulnerabilities are in upstream code, maintained outside OpenBSD, and the patches come from them. The recent xserver errata would be an example. X.org was kind enough to provide patches, which we were able to apply quickly, but then another, entirely different snag was hit. The magic signing elves were unable to find the magic signing disk with the magic signing key. There are some contingency plans to deal with situations like this; in the case of a low severity bug the plan is... wait.
A more complicated situation was the release of FreeType 2.5.4, followed by 2.5.5. I don’t really even know what FreeType does, except one time everything was ugly and I had to edit an XML file. I certainly don’t follow its development or know much about the code base, which makes assessing security updates an extra challenge. David was pretty quick integrating the new version in current, but the diff was too large to backport. Smaller patches were out there, it was just a matter of finding, integrating, and testing them. A little later, David managed to extract the patches used by Ubuntu and apply them to the 5.6 source tree.
That left 5.5. But 5.5 has a different version of FreeType than 5.6, so the diff didn’t apply as is. Here’s a small chunk of the patch.
- if ( ft_strncmp( line, "ENCODING", 8 ) == 0 ) + if ( _bdf_strncmp( line, "ENCODING", 8 ) == 0 )
But here’s what that line looks like in 5.5.
if ( ft_memcmp( line, "ENCODING", 8 ) == 0 )
FreeType changed the function from memcmp to strncmp at some point, then they changed it to a different strncmp. It was easy to correct this by hand to make the patch for 5.5, but subtle differences like this can often delay a patch when it has to be regenerated for every release.
The 5.6 patch also includes a non security related fix for blue zone hinting for CJK fonts. Why? Because it was in Ubuntu and so it was rolled up into the patch that David prepared. (Intro to blue zones.) But it’s not in the 5.5 patch, because of another conflict that was harder to resolve. Here’s the 5.6 patch again.
- af_blue_2_1_2 = af_blue_2_1_1 + 4, + af_blue_2_1_2 = af_blue_2_1_1 + 2,
We need to change the 2_1_2 to be only 2 more than the 2_1_1, not 4 more. Here’s the 5.5 line:
af_blue_2_2_1 = af_blue_2_1_1 + 4,
Do we change the 4 to a 2? That’s easy, but what about the fact that we’re initializing the 2_2_1 and not the 2_1_2? Does it matter? Should we also change the 2_2_1 to 2_1_2? If I make that change, will it still compile? Seems easier to jettison the autohint fixes entirely.
Alas, it’s not always enough to know if a vulnerability has been fixed; we also need to know when. OpenBSD preemptively killed export ciphers in Summer 2014. Then a straggler was removed in December. Then CVE-2015-0204 was announced in January. Then renamed to FREAK in March. In January, it sounded like a minor issue, not worth backporting, but we checked the code to make sure we had the fix. Done. Then in March when it became a big deal, we went back and checked our notes. Already fixed. Unfortunately, not enough attention was paid to the precise timing of the commits, and nobody noticed that 5.6 was released with one fix but not the other.
Then, on top of that, we decided that since we were patching, we’d also roll in some similar changes to prevent small DH key exchanges. BoringSSL introduced a cutoff of 512 bits, but that’s boring. We decided that nobody sane would use anything smaller than 1024 bits. Shortly after the patch went out, Henning noticed that all of his servers were down. Or more accurately, nagios nrpe was down. And so this diff required potential rework.
This was much like the previous X server patch, but even smoother. Matthieu had the patch from X committed to OpenBSD almost immediately, and we were able to ship patches within a day of the vulnerability disclosure. Almost perfect, except I flubbed the build instructions and typed libXont instead of libXfont. So close...
(This post was originally drafted before the latest OpenSSL announcement, but I delayed publication after the preannouncement. And now I have a bit more material.)
How big is the latest OpenSSL patch? Pretty big.
~/Downloads> diff -ru openssl-1.0.1l openssl-1.0.1m > ltom.diff ~/Downloads> ls -l openssl-1.0.1*gz ltom.diff* -rw-r--r-- 1 tedu tedu 4429979 Mar 18 02:09 openssl-1.0.1l.tar.gz -rw-r--r-- 1 tedu tedu 4533406 Mar 19 18:03 openssl-1.0.1m.tar.gz -rw-r--r-- 1 tedu tedu 23505088 Mar 19 21:05 ltom.diff -rw-r--r-- 1 tedu tedu 4045692 Mar 19 21:05 ltom.diff.gz
I won’t argue it wasn’t necessary to reformat the all code, but probably not in between stable releases. Somewhere out there somebody is still trying to sift through that diff to discover the important changes so they can be extracted and applied to a local tree. OpenSSL does provide a smaller set of patches to the early access group, but don’t seem to post them anywhere after the fact. You can browse git, but it’s slow and you’re likely to miss things.
We were almost all set to ship patches for 5.5 and 5.6 as soon as the embargo lifted, then we hit a snag. To beat the rush, I applied the 5.6 patch an hour early to my server, only to notice that nginx refused to restart. Revert! Revert! Figured out that nginx 1.7 works post-patch, but some of us aging websters will still be using nginx 1.6 for a while longer. Emailed the OpenSSL team 45 minutes before their launch window about the problem, which I’m sure delighted them, but better than 45 minutes after I guess. There was a bonus fix for part of 0287 that caused the trouble and it was pulled from the release since it wasn’t strictly necessary. Long story short: the d2i functions are fucked and any attempt to retcon sanity into them can only end in tears.
Unfortunately it seems at least FreeBSD didn’t get the memo and went live with the old patch. (I slipped up too, mailing the wrong patch to tech in all the excitement, but at least got the right versions into cvs and uploaded to ftp.)
Every OpenBSD patch gets sent to the announce@ mailing list, which is pretty fun since every email sent is practically guaranteed to result in a reply from somebody who wishes to unsubscribe. I’m not the list admin. I didn’t subscribe you. I can’t unsubscribe you. Usually I just reply with a link to the list manager page, though occasionally my reply bounces because the unsubscribe request has an invalid from address. Some people are literally helpless. Did I mention how fun this is?
My number one wish would be that every project provide small patches for security issues. Dropping enormous feature releases along with a note “oh, and some security too” creates downstream mayhem. Vendors need to extract minimal diffs from the feature releases, but this is a time consuming and error prone process.
Vendors should post source diffs. Even if most users will just run yummi-gummi-belli-rubrub or whatever to install binary fixes, they should be able to inspect the changes. That’s how I noticed FreeBSD shipped the early version of the patch. Debian released an update pretty quickly too, so I’m guessing they based it off the early patches as well, but fuck me if I can get to their patch from the announcement.
Bundling is a bad habit. If several fixes are released together, it introduces a greater risk of regression. A bad fix for a minor issue can prevent users from applying the fix for a major issue, leaving them exposed until the fix is corrected. If a vulnerability is serious enough to trigger an emergency release, it’s serious enough to warrant a release consisting of a single fix and no other.
I’m not a big fan of embargoes and secrecy. It may be a necessary evil, but ultimately impedes collaboration. Too much work gets done in the dark.