[Swan-dev] WIP: supporting xfrm SA expire

[Paul Wouters]

On Nov 27, 2021, at 14:03, Antony Antony <antony at phenome.org> wrote:
> 
> Hi,
> I rebased this branch and improved expire handling.
> 
> #sa-expire or #sa-expire-20211127
> https://github.com/antonyantony/libreswan/tree/sa-expire

Awesome. I will have a look.

> I renamed keywords to salifebytes= salifepackets= 

You had those names before. I wrote at the time:

I noticed you used salifebytes= and salifepackets=. I think it would be
more intuitive to call these maxbytes= and maxpackets. Or limit-bytes=
or bytelimit= and packet-limit= ?

Similarly, where strongswan has inactivity= I think idletimeout= or
idle-timeout= would be more clear? I wouldn't call in inactive because
the tunnel is "active" (or "up") - there is just no traffic happening.

I do understand why you added the "sa" prefix, because we in theory also
have these options on the IKE SA (for FIPS compliance), but I think
those maximums could just be hardcoded to a much lower count and might
never need to be user configurable? Like wouldn't 1Gbyte of IKE
traffic be a good time to re-auth or rekey-with-pfs ? In which case it
might make sense to omit the "sa" prefix for the regular admin?




I still think using an “sa” prefix is less intuitive, especially because an IKE SA is also an SA anyway. Which is why we have lifetime= and ikelifetime=  (but we also have salifetime= being the same as lifetime=)

> added few basic checks to avoid corner cases those netlink calls will return error:
> - do not send delete request to kernel for a "hard" expired SA (one directional). xfrm already deleted it.
> - do not query traffic,  get_sa_info() in delete_state() on hard expired SA

These are good although I hope our code never hits hard expire ever. It should only hit when there is a bug that prevented the soft timer to not rekey in time.

> When there is expire store the last known traffic.
> Also added fuzzing to soft bytes, and packet limits using c->sa_rekey_fuzz.

That’s a good point !

> At the moment I am leaving the responder with hard expire == soft expire

Do you really mean hard and soft here and not fuzzing the soft bytes ? If so I think that is wrong. A responder can also have rekey=yes and shouldn’t end up with a traffic leak during its rekey.

> I want give preference to initiator. I will change the responder soft limit 
> calculation. I am still trying to come up with a simple calculation.
> Ideally the inititiator should trigger the rekey first. If both sides have 
> the same softlimit, both sides will trigger rekey on the same packet.
> It would lead to extra SA. That is why it is important to have initiator 
> value entirely less than the responder values.

Yes this part I totally agree with.

> One thing decide as group is how to represent big number (2^64) bytes and 
> packets, especially the default 2^64  will appear in "ipsec status:  
> output.
> 18446744073709551615 look ugly:) 

make it display 0 or <unset>   ?

Paul