[Swan-dev] why do we bother checking out_raw() et.al.'s result?

[D. Hugh Redelmeier]

| From: Andrew Cagney <andrew.cagney at gmail.com>

| > There are no good exception mechnisms.
| 
| There is, its called abort().  Part of libreswan's start up is dealing
| with any mess from a previous abort.

Abort kills all operations.  I have no trouble advocating abort/assert
when we go into unknown territory.  When an invariant turns out not to
be true.

Packet output problems are not uncontemplated.  They are a kind of
resource exhaustion.  (Although the last time I hit one was actually
due to a bug in the calling code (that I had just writtten); an
assertion failure would have been slightly more convenient to debug.)

The packet output problems are currently handled in a way that stops a
particular negotiation.  Nothing else is affected.

| > In security code, it is good to stop when things are going wrong.  Not
| > get further into the weeds.
| 
| The real enemy of security is unnecessary complexity.

Yes.

| So how is this:
| 
|         return ikev1_out_generic(np, &isakmp_keyex_desc, outs, &z) &&
|             out_zero(g->len, &z, "fake g^x") &&
|             (close_output_pbs(&z), TRUE);
| 
| which goes hand-in-hand with code trying to propagate 'fatal' up the
| stack is somehow less weedy than:
| 
|        ikev1_out_generic(np, &isakmp_keyex_desc, outs, &z);
|        out_zero(g->len, &z, "fake g^x");
|        close_output_pbs(&z);
| 
| After all, if it really is fatal, abort().

It's not fatal.

For example, an adversary might be able to drive us into such an
overflow.  The way things are now, only the negotiation he's cast his
evil spell upon will fail.  No general DOS.

| I made these points:
| 
| - calling an out function after a fail will core dump
| hopefully that isn't true - that code path needs to be robust
| regardless (I'm sure if I look I can find more code like the above
| where a status is ignored)

Probably.  Or an error can be lost (not buffer full errors, but
others).

| - pluto must never knowingly send out a corrupt packet (reflection
| attack and all that)
| (but I want to :-) so drop the packet as it goes out the door

During state transition functions, we modify the internal state of our
system (XFRMs, for example).  Quiting an STF as soon as an error is
discovered prevents such changes, that either matters or it doesn't.
I don't wish to audit the code to determine which.

| - the state machine will get confused because a packet was sent when it wasn't
| First, this is no different to a MITM attack (where packets get
| corrupted/dropped without our knowledge) - we get to deal.

Normally: "deal" means retransmit the packet.  But, oops, we don't
have one.

| Second, if the SA should be killed, then inject a KILL-SA event rather
| than trying to handle it in-line.

I don't know what this means.  It doesn't sound conservative.