[Swan-dev] why do we bother checking out_raw() et.al.'s result?
andrew.cagney at gmail.com
Sat Sep 22 16:57:49 UTC 2018
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") &&
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");
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?
More information about the Swan-dev