search

xelerance / Openswan

Openswan
Other
849 stars 214 forks source link

wo#10213: Don't prevent responder peer from initiating SA rekey #443

Closed mohicks closed 3 years ago

mohicks commented 3 years ago

If the peer manages to send us a SA rekey, then accept it. Strongswan will sometimes do this because OSW doesn't yet support RFC4478 and is slower to rekey the Parent SA than strongswan would like.

mohicks commented 3 years ago

I don't think that I wrote the code around st_orig_initiator. I think that Bart wrote it to solve some other problem. Maybe git annotate to find out why it was added?

Yeah, I looked into this in depth. It's related to trying to make good decisions about when to try to initiate a rekey vs. the remote end. If we're not the orig_initiator and the remote end is behind NAT then OSW no longer tries the rekey.

I think that idea is solid, and the code remains to make that decision.

I believe that rejecting an incoming rekey/reauth because you think you should be initiating the reauth is being too aggressive. If I'm wrong, then we need to address the fact that returning SITUATION_NOT_SUPPORTED also deletes pending events on the state (including any EVENT_SA_REPLACE that are pending for the IKE SA)

letoams commented 3 years ago

On Fri, 18 Sep 2020, Martin Hicks wrote:

  I don't think that I wrote the code around st_orig_initiator.
  I think that Bart wrote it to solve some other problem. Maybe git annotate to find out
  why it was added?

Yeah, I looked into this in depth. It's related to trying to make good decisions about when to try to initiate a rekey vs. the remote end. If we're not the orig_initiator and the remote end is behind NAT then OSW no longer tries the rekey.

I think that idea is solid, and the code remains to make that decision.

It's actually wrong. rekeying is a local policy that should be followed, independantly of who initiated the exchange.

libreswan had to do a lot of fixing with respect to the original initiator because a lot of the openswan code used it wrongly, since it had not implemented rekey properly yet.

The original originator role should ONLY be used to determine which SPI order to use to find a state. It should not be used for anything else.

Not rekeying to a right=%any is a limitation in openswan, not an RFC compliant design decision. It's a bug, not a feature.

mcr commented 3 years ago

I don't think that I wrote the code around st_orig_initiator. I think that Bart wrote it to solve some other problem. Maybe git annotate to find out why it was added?

Yeah, I looked into this in depth. It's related to trying to make good decisions about when to try to initiate a rekey vs. the remote end. If we're not the orig_initiator and the remote end is behind NAT then OSW no longer tries the rekey.

I think that idea is solid, and the code remains to make that decision.

I believe that rejecting an incoming rekey/reauth because you think you should be initiating the reauth is being too aggressive. If I'm wrong, then we need to address the fact that returning SITUATION_NOT_SUPPORTED also deletes pending events on the state (including any EVENT_SA_REPLACE that are pending for the IKE SA)

okay, I agree with you. Never reject a rekey event that arrives, even if we believe that it can not, somehow get to us based upon policy.