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

[Andrew Cagney]

On Sat, 22 Sep 2018 at 12:07, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
>
> | From: Andrew Cagney <andrew.cagney at gmail.com>
>
> | I'm wondering why we bother to write code like:
> |
> |         return ikev1_out_generic(np, &isakmp_keyex_desc, outs, &z) &&
> |             out_zero(g->len, &z, "fake g^x") &&
> |             (close_output_pbs(&z), TRUE);
> |
> | that goes out of its way to terminate the construction of a packet the
> | moment there is a whiff of a problem
>
> 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.

> 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.  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().

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)

- 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

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

I didn't see this addressed?