[Swan-dev] WIP: supporting xfrm SA expire

[Antony Antony]

On Mon, Apr 05, 2021 at 01:22:39PM -0400, Paul Wouters wrote:
> On Mon, 5 Apr 2021, Antony Antony wrote:
> 
> > Here is my sa expire branch rebased to main.
> > 
> > #sa-expire
> > https://github.com/antonyantony/libreswan/tree/sa-expire
> 
> Thanks! I had a look and I think it looks pretty good.
> 
> > It need a bit more work to merge to main. I look the code again and fix
> > "FIXME". It also need more tests.
> > 
> > If you feel like helping add more tests. This would help to get the
> > branch ready to merge sooner than later.
> 
> I'm working on that now.
> 
> 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= ?

given that we have "salifetime" for IPsec and "lifetime" from IKE
I feel life* would be better choce, so I choose salifebytes salifepackets.

> 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 have no idea about those:)

> 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

I choose salifebytes because we already have "salifetime" I am not too 
attached to it. However, your proposals are not more appealing at the 
momet:) Lets see how this goes.

My next proposals are "rekey-bytes" and "rekey-packets". Those would align 
with swanctl config. swanctl full name is 

connections.<conn>.children.<child>.rekey_bytes

Do not compare to ancient strongswan pluto keywords.

> 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

well with Post Quantuam and PCPU I can imagine conerns abou IKE rekey on the 
horizon.

> might make sense to omit the "sa" prefix for the regular admin?

again, what about "salifetime" are you going change that.

> I'm glad to see you decided on configuring soft timeouts at a fixed (80%)
> rate of hard limits. I was also hoping to not have an option for this.

I did it because I am lazy to code! I am not proud of it. I hope that one
day we will support soft and hard limits. I clearly see benfits of the
flexibilty. Especially if we want to be more of reference implementation.

> I will look at adding some logging based on hitting soft and hard timer.
> Right now, one just sees a rekey but there is no message as to why.

> And I'll add an impair test that stalls and rekeying so we hit the hard
> timer and see if we can distinguish that. Because we should delete the
> SA if the hard timer was hit since the kernel already removed it in that
> case.
> 
> Thanks for the work on this! I'll send a PR later today against your
> branch.

At the moment I am only looking for test case PR. That is my work flow at
the moment! I just noticed you send a PR with tests I will see what I can 
pickup. Thanks.

-antony