Hacker News Reader

Comment by jgrahamc | Hacker News Reader

I'd love to see information about the actual security value of these changes. A ton are just adjusting to KNF.

http://anoncvs.estpak.ee/cgi-bin/cgit/openbsd-src/log/lib/li...

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.

But they are doing stuff like this:

1. Let's remove OPENSSL_cleanse: http://anoncvs.estpak.ee/cgi-bin/cgit/openbsd-src/commit/lib...

2. Crap. Let's put it back again: http://anoncvs.estpak.ee/cgi-bin/cgit/openbsd-src/commit/lib...

That plus the 'tone' of the commits makes me wary of these changes.

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.

You're right. I'm wrong about number 2. Mea culpa.

Also look at the manpage for explicit_bzero: http://www.openbsd.org/cgi-bin/man.cgi?query=bzero&sektion=3

It already does what OpenSSL_cleanse is meant to do, so using it makes sense.

You are seeing this because they are working in public. Any of us would look like fools if you saw our intermediate work product.

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

Speaking as someone who spends a big chunk of his working life auditing others code, having a consistent style makes it a lot easier to spot problems.

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?

It might not help upstream OpenSSL, unless they keep an eye out for the bug fixes. Consider OpenBSD the upstream for this new fork.

I don't think they intend to contribute back to the original OpenSSL code base, but see this as a OpenBSD only fork.

man style

http://www.openbsd.org/cgi-bin/man.cgi?query=style&apropos=0...

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.

[1] http://en.wikipedia.org/wiki/Indent_style#Whitesmiths_style

[2] http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/cry...

[3] http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/cry...

edit: formatting

Looks like significant amount of non-stylistic changes too:

http://opensslrampage.org/

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.

But it makes it readable, which makes it easier to find bugs. This is vastly more important.

This, very much.

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.

Besides, the OpenSSL code is a fucking mess.

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.

> At some point you need to say, "This code is a mess, and pretty much anything we do to it will be an improvement."

In software development it's often the case that the first claim is true, but the second isn't.

> Frankly I wouldn't trust fixes to the old version anyway, given its state.

It will be interesting to see if this get organized and properly funded, given this library's importance to international commerce.

> Frankly I wouldn't trust fixes to the old version anyway, given its state.

Clean slate! An admirable goal, and clearly the best solution. Let's see if this actually happens.

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.

Also, they are doing many bug fixes as shown on http://opensslrampage.org

The API's aren't changing... they are keeping API compat. Sure you won't be able to change the PRNG anymore ... but that isn't that big of a deal.

This isn't about confirming to their personal style, this is about cleaning the code up so that auditing it is possible.

Reply on news.ycombinator.com