Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Max child SA limit per configuration #1856

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

anipatel-eng
Copy link

This PR seeks to introduce the ability to control the maximum number of child SAs that are allowed to be negotiated by a responder. This is an optional configuration that maintains the current behavior of unlimited child SAs by default.

This is in relation to discussions per #1725

Duplicate child SA handling and preventing initiating duplicate child SAs was introduced in 5.9.6 and allows the initiator to largely catch and prevent duplicate child SA from being initiated. However, this doesn't help the responder and can hinder a responder that has a limited implementation or a specific use case to only allow a single child SA per IKE SA.

There is currently no provision as confirmed in the discussion thread to limit the number of child SAs negotiated as a responder which leaves the only option to accept or disconnect upon detecting the max child SA count has been exceeded via a listener plugin. This is far from ideal as it only leads to flapping tunnels or undesired multiple child SAs. Also, the behavior may be caused by misbehavior of a client such as a pre-5.9.6 client that negotiates duplicates not desired by customer config that could lead to the tunnel flapping.

The ideal behavior would be to allow configuration to control the maximum number of child SAs per configuration profile specifically and for this limit to be enforced at negotiation time to reject additional child SAs with an appropriate notification by the responder.

The RFC, in fact, outlines this use case and refers to minimal implementations where limiting to a single Child SA is desired but the current code doesn't allow for this by configuration and this change introduces this as an optional configuration.

RFC7296 Section 1.3. The CREATE_CHILD_SA Exchange:

  The responder sends a NO_ADDITIONAL_SAS notification to indicate that
   a CREATE_CHILD_SA request is unacceptable because the responder is
   unwilling to accept any more Child SAs on this IKE SA.  This
   notification can also be used to reject IKE SA rekey.  Some minimal
   implementations may only accept a single Child SA setup in the
   context of an initial IKE exchange and reject any subsequent attempts
   to add more.

Configuration introduced:
connections.<conn>.children.<child>.max_child_sas = 0

@cla-bot
Copy link

cla-bot bot commented Aug 21, 2023

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @anipatel-eng on file. In order for us to review and merge your code, please contact the project maintainers via [email protected] to get yourself added.

Copy link

cla-bot bot commented May 2, 2024

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @anipatel-eng on file. In order for us to review and merge your code, please contact the project maintainers via [email protected] to get yourself added.

@tobiasbrunner
Copy link
Member

@cla-bot check

Hm, I don't think such a limitation to a specific number is very useful. What would the use case be to set it to a particular value other than 1 or 0? Basing the duplicate check on the name is also far from ideal (there is narrowing and configs could get renamed etc.). And sending back a NO_ADDITIONAL_SAS notify does not seem correct either. That would technically not allow the peer to rekey the SA or create CHILD_SAs for other configs/selectors under the same IKE_SA. The RFC says this:

   The responder sends a NO_ADDITIONAL_SAS notification to indicate that
   a CREATE_CHILD_SA request is unacceptable because the responder is
   unwilling to accept any more Child SAs on this IKE SA.  This
   notification can also be used to reject IKE SA rekey.  Some minimal
   implementations may only accept a single Child SA setup in the
   context of an initial IKE exchange and reject any subsequent attempts
   to add more.

The protocol does not provide a way to convey to a peer that only a single CHILD_SA with particular selectors is wanted. In particular, because the RFC explicitly states:

   Note that IKEv2 deliberately allows parallel SAs with the same
   Traffic Selectors between common endpoints.  One of the purposes of
   this is to support traffic quality of service (QoS) differences among
   the SAs.  Hence unlike IKEv1, the combination of the endpoints
   and the Traffic Selectors may not uniquely identify an SA between
   those endpoints, so the IKEv1 rekeying heuristic of deleting SAs on
   the basis of duplicate Traffic Selectors SHOULD NOT be used.

So as already suggested in #1725, while not recommended by the RFC, you could technically delete the unwanted CHILD_SA after the fact (the newly created or maybe better the "old" one if the peer lost that and created an additional SA - but it should really be investigated why the duplicate was created in the first place). Of course, that's not ideal if the peer just recreates the SA again (e.g. because of something like close_action=restart and a lack of a local duplicate check in older strongSwan versions).

@cla-bot cla-bot bot added the cla-signed label May 15, 2024
Copy link

cla-bot bot commented May 15, 2024

The cla-bot has been summoned, and re-checked this pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants