[ubuntu/jaunty] conntrack 1:0.9.7-1.1ubuntu1 (Accepted)

[Kees Cook]

On Mon, Nov 24, 2008 at 01:02:49AM +0000, James Westby wrote:
> For the record Manny didn't introduce this change, just merged it. I
> appreciate you raising the issue, and I know you weren't attacking 
> Manny.

Yup, I saw a bunch of people had worked on this package, so I thought it'd
be good to bring it up.  :)  And thank you Manny for helping out!  I didn't
mean to single you out -- this is really a reminder for everyone.  :)

> I have attacked a couple of these failures recently, and the thing I
> always find hard is knowing what to do with an error condition. Without
> knowing the code it is hard to know how an error should be handled.
> For instance today I was looking at an ircd that wasn't checking the
> return code on a write call, writing to its log file. I don't think
> an error should abort, but in many cases that will be the only sensible
> thing to do.
> 
> Are there any guidelines for how to handle these failures so that we can
> get better at fixing them?

Well, as you say, it's always different.  The way I've tended to triage
them is:

- If there is are obvious error handlers in near-by code, emulate them.
  That's what I did in my suggested patch to conntrack[1].  This is,
  obviously, the preferred method of dealing with it.

- If there isn't an obvious way to handle the error, then stubbing out an
  empty handler is the next way to go -- fundamentally, this doesn't
  improve (or weaken) the quality of the code -- ignoring the return code
  is what's already happening, so this doesn't change anything.  However,
  what it _does_ do, is allows the FORTIFY option to still be enabled,
  which means the program will gain the run-time FORTIFY protections.

- If there isn't a way to work around the error, and the problem is
  isolated to a small area of the source code, I've disabled FORTIFY for
  _only_ those source files[2], which can be a pain, depending on the
  package's build methods.

- If nothing works, and other people have looked at it, and no one has any
  ideas about how to work around a problem with FORTIFY, then it's time to
  disable it for the entire build using the -U_FORTIFY_SOURCE CFLAG.

Now, in all of these situations, upstream needs to be notified.  Especially
for the #2, where they need to make a larger design decision about how to
deal with the unhandled error condition.

Also, in situations where you've had to disable FORTIFY (#3, #4), please
document the issue in the CompilerFlags[3] wiki page (preferably also with
a bug).

And thank you to everyone who has had to deal with the fall-out of these
options!  I know they can really be an annoyance, but taken as a whole,
Ubuntu is far better off with these flags enabled, and everyone's code
quality goes up.  :)

-Kees

[1] http://launchpadlibrarian.net/19891829/fortify-warnings.patch.dpatch
[2] http://launchpadlibrarian.net/17169346/sysklogd_1.5-2ubuntu5_1.5-2ubuntu6.diff.gz
[3] https://wiki.ubuntu.com/CompilerFlags

-- 
Kees Cook
Ubuntu Security Team