Security Fix in Open BSD

http://code.bsd64.org/cvsweb/openbsd/src/sys/netinet/ip_esp.c.diff?r1=1.74;r2=1.75;f=h

by wglb • 16 years ago

66 34 16 years ago

code.bsd64.org

tedunangst3• 16 years ago

Fix is 8 years old.

david_p• 16 years ago

I believe this is a follow-up to this story : http://marc.info/?l=openbsd-tech&m=129236621626462&w...

I think the purpose of the link is to demonstrate some "suspicious" changes in the IPSEC code.

gcr2• 16 years ago

Is this related to the alleged IPSEC backdoor? Either way, could we get some context?

Thangorodrim• 16 years ago

Looking only at that diff, the fix is a break.

It inverts the sense of the test so that the authenticator of the packet is used even if the tag is null.

tptacek9• 16 years ago

The "tag" is an mbuf tag. Mbuf tags aren't packet data; think of them like per-packet session state, so that different layers of the stack (the network drivers, any driver required to handle crypto goo done on the card, the original IP code, and the IPSEC decapsulation and handling code) can talk to each other without hand-coding a million disgusting callbacks throughout the kernel.

In this code, "mtag" is the tag matching PACKET_TAG_IPSEC_IN_CRYPTO_DONE, which the lower-level drivers use to tell the IPSEC code that the NIC already did verification. If the tag isn't there, then you want to do the verification in software, which is what the fixed code does and the broken code didn't do.

ctcherry• 16 years ago

Could someone explain the significance of this change? What would this enable or disable in practice?

calloc• 16 years ago

As Thomas Ptacek more accurately describes in his post, this disables the ESP Auth checks (after the fix) if the NIC has already completed them. Before the fix, the software ESP auth check would run only when the NIC had already done them, so never if the NIC had not done them.

tptacek9• 16 years ago

In 2001, Angelos Keromytis --- then a grad student at Penn, now a Columbia professor --- added support for hardware-accelerated IPSEC NICs. When you have an IPSEC NIC, the channel between the NIC and the IPSEC stack keeps state to tell the stack not to bother doing the things the NIC already did, among them validating the IPSEC ESP authenticator. Angelos' code had a bug; it appears to have done the software check only when the hardware had already done it, and skipped it otherwise.

The bug happened during a change that simultaneously refactored and added a feature to OpenBSD's ESP code; a comparison that should have been == was instead !=; the "if" statement with the bug was originally and correctly !=, but should have been flipped based on how the code was refactored.

HD Moore may as we speak be going through the pain of reconstituting a nearly decade-old version of OpenBSD to verify the bug, but stipulate that it was there, and here's what you get: IPSEC ESP packet authentication was disabled if you didn't have hardware IPSEC. There is probably an elaborate man-in-the-middle scenario in which this could get you traffic inspection, but it's nowhere nearly as straightforward as leaking key bits.

To entertain the conspiracy theory, you're still suggesting that the FBI not only introduced this bug, but also developed the technology required to MITM ESP sessions, bouncing them through some secret FBI-developed middlebox.

One year later, Jason Wright from NETSEC (the company at the heart of the [I think silly] allegations about OpenBSD IPSEC backdoors) fixed the bug.

It's interesting that the bug was fixed without an advisory (oh to be a fly on the wall on ICB that day; Theo had a, um, a, "way" with his dev team). On the other hand, we don't know what releases of OpenBSD actually had the bug right now.

It seems vanishingly unlikely that there could have been anything deliberate about this series of changes. You are unlikely to find anyone who will impugn Angelos. Meanwhile, the diffs tell exactly the opposite of the story that Greg Perry told.

there• 16 years ago

for what it's worth (not much), angelos also contracted for netsec.

tptacek9• 16 years ago

Did he have a contracting relationship beyond accepting funding for the OpenBSD IPSEC work? Because that's a subtly different relationship for him to have had than "contractor".

caf• 16 years ago

The most worrying thing about it is probably just that it took a year to notice - you would think "throw a forged authenticator at it" would be in the regression tests for an IPSEC implementation.

tptacek9• 16 years ago

"Regression tests for an IPSEC implementation". Heh.

zedshaw• 16 years ago

> It's interesting that the bug was fixed without an advisory (oh to be a fly on the wall on ICB that day; Theo had a, um, a, "way" with his dev team).

This would be my question. That seemingly small change has large security implications since it means that branch was disabled on certain erroneous conditions for some extended period of time and then silently changed back. I think you're right that Jason fixed it, but why wasn't it announced as a major security flaw in OpenBSD 3.0 which I believe released with this code?

Another spin on this drama could be either of these two:

1. Jason actually went in and fixed bugs placed by other NETSEC employees, and now is the victim of reprisals.

2. NETSEC was incompetent, not malicious, and then silently went around fixing things without telling anyone.

Interesting stuff, can't wait to see the rewrite of this file! :-)

tptacek9• 16 years ago

Angelos was never a NETSEC employee; his work on the OpenBSD IPSEC code was at one point funded by NETSEC, but if you're familiar with how OS kernel "volunteer" development works, that's not a weird situation at all --- there were, if I remember my semi-sober conversations with him at the Ship & Anchor correctly --- plenty of times Theo found himself in similar relationships.

It's unlikely that NETSEC would have had any management influence over Angelos during that work.

dsantos2• 16 years ago

de Raadt: "We've been auditing since the mail came in! We have already found two bugs in our cryptographic code. We are assessing the impact."

http://www.itwire.com/opinion-and-analysis/open-sauce/43995-...

"Until 2 days ago I had no idea that both Jason and Angelos in the past did work for a company that does that business"

tptacek9• 16 years ago

There is no way Theo didn't know Jason worked at NETSEC; Jason's a co-author of an academic paper about the OpenBSD cryptography implementation, under a NETSEC address.

paperclip3• 16 years ago

I presume you are attempting to imply that this one of the 'leet' FBI backdoors that the Perry email discussed. It would help if you actually said why you thought this patch was interesting when submitting it to HN.

I hope we are not going to get a rash of inarticulate HN submissions for every minor patch to openbsd which may have security implications.

I doubt this is in anyway related to the recent drama.

The initial bad patch which broke things looks like v1.63 of the file. This was committed in 2001 by angelos.

Note: - angelos has a greek email address (for what it is worth) - 1.63 was committed in 2001; the allegations of backdoors seem to be from 2000 and 1999 - it appears to break things for all non-ipsec aware nics. This seems to broad an error for an intentional backdoor

It also looks like the code didn't make it to a release branch before being fixed - so if it was an attempted backdoor, it was not successful.

Disclaimer: I have no experience with BSD kernel code nor with IPSEC.

Edit: I initially thought that angelos also commited the fix as well. I think I need to get my eyes tested

iclelland• 16 years ago

I figured that v1.61 had the initial line in it; 1.63 added the comment explaining its purpose. (the diffs are ugly, though, and hard to tell what lines went where sometimes)

The fix was in 1.75, though, over a year later. In the middle is v1.69, tagged as the release version for OpenBSD 3.0 (and the branch point for 3.1)

1.75 looks like the release version for 3.1 (and branch point for 3.2), so the bug would have been present in the released 3.0 version.

paperclip3• 16 years ago

blargh.

You are quite correct - I don't know how I missed that.

I hope the conspiracy theorists don't turn on me for participating in the cover up.

gcr2• 16 years ago

  > I hope we are not going to get a rash of inarticulate
  > HN submissions for every minor patch to openbsd which
  > may have security implications.

I do. Submissions are cheap and skippable. Plus, now I get to look at all these small bugs and not make the same mistakes in my own code, which is both a huge win in my book and something that one can't learn from a textbook.

paperclip3• 16 years ago

This submission prior to tptacek's (rather good) analysis was of little value. The original submitter gave absolutely no context to the diff.

I don't think we can depend on tptacek always being on hand.

This bug itself is not novel and something any programmer (if they are being honest) will admit to doing themselves.

The really interesting part of this story is not technical at all (and not evident from the posted patch) - why did the openbsd team not feel it necessary to release a security advisory for this bug. That decision may tarnish their reputation more than any wild conspiracy claims.

sielskr• 16 years ago

"Submissions are cheap and skippable."

By that logic, you will not mind if I spam your mailbox since emails are cheap and skippable.

mfukar• 16 years ago

Angelos has (had?) an email address from the University of Crete (now uoc.gr, formerly uch.gr), of which he's a graduate.

Nothing weird there.

jey2• 16 years ago

Looking at the dates, this bug was fixed in under a month. Did the buggy code really get released to production environments that fast? A priori unlikely IMO, which means this "vulnerability" is instead just the normal development process at work. I'm sure we've all committed code with mindbogglingly dumb bugs at one point or another.

tptacek9• 16 years ago

A month? I'm getting over a year from my look; the bug was introduced with the original feature in 2001.

jey2• 16 years ago

Oops, I'm an idiot. Obviously the linked page doesn't say when the original buggy code was introduced; it only gives the last modification timestamp.

davidj• 16 years ago

Fixing security holes without disclosing the problem to the public.. yup I've seen that before from openbsd

Security Fix in Open BSD

Related Stories