They still haven't done the most critical thing: add tests. This is for two reasons: the code has no tests now, and when you refactor, tests are important to make sure you're not subtly introducing errors.
I don't think the changes they are doing at the moment have much security value. They aren't working on fixing OpenSSL security flaws. They are obliterating all the layers of cross-platform compatibility that make the code more complex, and removing everything OpenBSD isn't likely to use.
They are transforming a local copy of openssl that tracks upstream changes to their own fork, that probably won't reunite with the upstream version anytime soon.
In the process, they are obviously likely to find some bugs (and they already have fixed one or two harmless ones), but I think they are more interested in building a clean fork for themselves than in fixing the upstream openssl.
OpenSMTPD, OpenNTPD and OpenSSH are avail for other platforms. They will strip OpenSSL down to essentials, spend months auditing what's left then when satisfied it is reliable software porting to other platforms can begin. It's only been a week
Did you actually read the second diff you linked? They did not put anything back again. They removed even more junk. It is all deletions.
I'm getting tired of all the people who complain about the humour, tone or style in the commit messages. If you have serious concerns, why not read the diffs and comment on the actual changes? This is how real problems get noticed and corrected.
But some things were in fact removed and put back again, because it turns out some ports depended on it. Sometimes you just don't notice what a few thousand other applications depend on before you remove some code.
But isn't what's happening here a change to the coding style to match the style used by BSD? That might be fun for BSD coders, but is that actually helping improve the OpenSSL code?
Well, that'd be useless to move from tabs to spaces or from n spaces to m spaces in this context, but as far as I've observed, OpenSSL codebase uses the unconventional Whitesmiths style [1][2][3], which might be a burden on the developers to navigate.
There have been a few bugfixes. Some insecure idioms have been corrected, a broken implementation of calloc (seriously, why?) has been replaced with calls to the library function. Some deceiving (and/or just useless) wrappers have been eradicated. More significant is the removal of the built-in RNG and a few hilarious (or sad) questionable "features" relating to its use.
That's pretty disappointing; a big formatting change like that means that it'll increase friction of pulling fixes and changes between the two versions.
Normally I try to avoid getting into arguments about style and such, it's just bikeshedding. But if the OpenBSD people are going to clean up, review, audit, fix, improve and maintain this code, it makes all the sense to switch to a style they are used to and comfortable with. It also makes sense to nuke all the junk that gets in the way and which they will not and cannot test because it is not at all relevant to the platform.
At some point you need to say, "This code is a mess, and pretty much anything we do to it will be an improvement."
I spent a couple hours with the OpenSSL source last week, and agree that a major cleanup is necessary. This means
- Rip out unused stuff
- Rip out stuff that obviously doesn't work
- Make big cleanups
I've done this in the past with token-level tools to ensure that I wasn't breaking things.
OpenSSL desperately needed to have a bunch of garbage removed. It needs good tests, it needs to be slimmed down and made clean so that the code is readable and maintainable.
Frankly I wouldn't trust fixes to the old version anyway, given its state.
The OpenBSD team just want to do it their own way, regardless of the practical value of creating single solutions that work everywhere. OpenSSL was a great idea back when there was no other open crypto library. But now amateur cryptographers are a dime a dozen and anyone can slap together something that just works for them and disregards others. Hence stripping out everything, changing APIs, and enforcing KNF. There's probably no security value to 90% of the changes; they just want conformity to their personal style.
As feld, clarry, danrik, etc. have pointed out, having a consistent coding style makes identifying bugs much easier.
"regardless of the practical value of creating single solutions that work everywhere"
Since these are the same folks who do OpenSSH, I would imagine after the initial work is done, it will probably evolve to be as portable as OpenSSH and the other OpenBSD projects.
I would have loved knowing their reasoning behind this decision, which I'm sure it's very calculated, because the implications are huge. They're choosing to switch from a simple packager role to a maintainer role, they will have to always keep their fork in sync with upstream which if they try to apply every changes will require a lot of work, or maybe they have decided on another strategy e.g. freezing functionalities and applying only security patches, I don't know I didn't look at the current types of clean-ups maybe that could hint what their intentions are.
Regardless, I have mixed feeling about this, one month ago a lot of people lauded openssl for not being vulnerable to the goto fail saga while criticizing Apple for not using openssl and now everyone is ready to ditch openssl in a heartbeat for a flaw not related to crypto, not committed by core developers. I'm not defending openssl, I would love a concerted effort by big industry players for implementing a new SSL/TLS stack from scratch, maybe in a safer language, but I also don't think it is a reason for taking quick, disruptive decisions like this. It is also likely they had this move in mind before this episode and this flaw was the last straw but they should have exposed their plan, it would have been a bit better for us to understand their reasoning.
The thing about crypto is that almost any bug in a crypto library is related to crypto. It can't be easily isolated, and if you try, you have to actually analyze the isolation to make sure that it's really isolated.
"...disruptive decisions..."
Who is it disrupting?
OpenBSD is trying to provide value to their users. If you want slow, steady, incremental, reactive improvements to OpenSSL, every other OS will give you that. OpenBSD apparently believes its users would benefit from a proactive approach to security.
I cannot speak for the people who made this decision.
But if you look at the code, you might come to the conclusion that the upstream is not so focused code quality (security!) as one might hope. There is such a jungle of code that can only be justified by the desire for more features, more portability (to crazy ancient platforms too), more speed, more flexibility... it's all more knobs and code where real security bugs can lurk. And code paths that do not get tested properly. You can also find many outright dangerous idioms used throughout. Does that mean the upstream is ignorant, or that they do not care, or that they do not have the man power to fix their aging code? Either way, it's bad.
The OpenBSD developers have different priorities, and they believe they can do better. As such, not being able to blindly pull changes and updates from the upstream developers they do not trust does not look like a significant loss. If new features are to be merged in, it has to be properly reviewed and adapted. Yes, it's more work that way. But the end result is likely better code.
It is one thing to make an OS support the hardware and run on a "weird" piece of hardware. But the removed code in OpenSSL is about making it compile and run on systems on which the standard (defacto or specified) APIs do not exist or do not behave like they should, and which have broken compilers that need workarounds. All of these add conditional code paths that won't get the amount of testing they need, and which will only hinder developers when they try to understand and improve the surrounding code.
Also, the $20k was for the power bill. Yes, there are some older systems among the powered ones. But there's more, there's really a rack full of hardware running the infrastructure that makes OpenBSD available to us. There's networking gear, there's AC...
Dobbsbob makes a good point too. The weird hardware OpenBSD runs on helps find bugs; the untested code paths in OpenSSL add bugs.
Those ancient platforms/architectures run OpenBSD, though, not the original OS. Nobody in their right mind should run those dead OSes on the public internet; why bother supporting them in the latest OpenSSL release?
> They're choosing to switch from a simple packager role to a maintainer role.
This is hardly a leap of faith for the OpenBSD developers. The dev team has always placed an emphasis on security even if the added security meant significant additional work. OpenBSD has a history of creating new software products to replace products that they deemed unacceptable from a security standpoint (including forking netbsd). I imagine that in addition to OpenBSD you are familiar with at least one of these projects:
This pays off for them too. NTP DoS reflection attack this year didn't work on OpenNTPD which had removed test commands and extensions that were used by attackers. The BGP hijacking going on last year sending traffic to Belarus didn't work on OpenBGPD http://arstechnica.com/security/2013/11/repeated-attacks-hij...
I think the distros shared a lot of blame for the ntp ddos attacks. The defaults were ridiculous. I want to scream every time I see `restrict default kod blah blah` without `limited`. Somewhere someone threw up that terrible restrict statement and it got copied everywhere. KOD without `limited` is useless and a good indicator the maintainer never read the docs: "If the kod flag is used in a restriction which does not have the limited flag, no KoD responses will result." As far as alternatives to the ntp reference implementation go I like Miroslav's chrony, especially for things like laptops with intermittent connectivity and power saving concerns.
> They're choosing to switch from a simple packager role to a maintainer role, they will have to always keep their fork in sync with upstream which if they try to apply every changes will require a lot of work, or maybe they have decided on another strategy e.g. freezing functionalities and applying only security patches, I don't know I didn't look at the current types of clean-ups maybe that could hint what their intentions are.
In fairness it's always been my impression that a BSD distribution is intended to be much more than a mere packager of software, but instead an integrator of those libraries into a sane, coherent overall operating system.
This is the reason OpenSSH has their native version for OpenBSD and then a "portable" version for the rest of the POSIX world.
Maybe they should just fork it completely, keep only the most used stuff, and call it OpenTLS - the "clean" alternative to OpenSSL. Then push for it to get accepted in server software. It would be kind of like the OpenOffice/LibreOffice situation.
Although if they're going to embark on such an endeavour, maybe it would be best to also rewrite it completely in a memory safe language like Rust (but in a year or two, after Rust becomes more mature).
It wouldn't help if they create another project based on OpenSSL code, because that will inherit OpenSSL's license that everybody hates, and wouldn't necessarily help other projects adopt it.
It might also discourage other people from looking at / contributing code, when they could just improve another TLS library with a saner license that people actually want to use not just due to legacy.
In case your cvs-fu is rusty FreshBSD[1] or OpenSSL Rampage[2] provide an easy way of keeping an eye on the changes. One of my favorites so far is: "Remove hacky workaround for Cray T3E," as in the debuted in 1995, Cray Research T3E.[3]
-/* Added for T3E, address-of fails on bit field (beckman@acl.lanl.gov) */
-#ifndef BIT_FIELD_LIMITS
memcpy(&server.sin_addr.s_addr, ip, 4);
-#else
- memcpy(&server.sin_addr, ip, 4);
-#endif
I hate cgit's log view, they should at least be honest and call it shortlog. Most importantly the link you provide displays everything from src, I could just browse the source-changes list if I wanted to see everything. The freshbsd and rampage links allow you to see just the libssl changes.
There are tons of simple cleanups I'd love to do. But with all the activity and people already working on it, I'm afraid I'd just be stepping on people's toes if I started sending diffs now.
Stepping on each other's toes is just what happens with large projects. There are tools to help deal with merge conflicts so I don't think you should hold yourself back. OpenSSL needs all the eyes it can get right now.
