From: Tobias Brunner <tobias@strongswan.org>
Date: Tue, 7 Nov 2023 16:39:51 +0000 (+0100)
Subject: kernel-netlink: Don't add replay state twice when updating SAs
X-Git-Tag: 5.9.12rc1~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3cb843436765cec853e35a15be343ec0624665ce;p=thirdparty%2Fstrongswan.git

kernel-netlink: Don't add replay state twice when updating SAs

The kernel includes the XFRMA_REPLAY_ESN_VAL attribute when dumping
SAs since it was added with 2.6.39.  So we basically added this attribute
twice to the message sent to the kernel, potentially exceeding the
message buffer if the window size is large.

The XFRMA_REPLAY_VAL attribute is only dumped since 3.19, so that might
still be relevant (Google seems to maintain a 3.18 kernel) and since we
have to query the current lifetime stats anyway, we can just avoid adding
this attribute twice.

Closes strongswan/strongswan#1967
---

diff --git a/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c
index 7596f0c181..efd705db20 100644
--- a/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c
+++ b/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c
@@ -2121,14 +2121,13 @@ failed:
 }
 
 /**
- * Get the ESN replay state (i.e. sequence numbers) of an SA.
+ * Get the usage stats (packets/bytes) and classic replay state (i.e. sequence
+ * numbers for small windows/non-ESN) of an SA.
  *
- * Allocates into one the replay state structure we get from the kernel.
+ * Allocates and copies the attributes we get from the kernel.
  */
 static void get_replay_state(private_kernel_netlink_ipsec_t *this,
 							 kernel_ipsec_sa_id_t *sa,
-							 struct xfrm_replay_state_esn **replay_esn,
-							 uint32_t *replay_esn_len,
 							 struct xfrm_replay_state **replay,
 							 struct xfrm_lifetime_cur **lifetime)
 {
@@ -2214,14 +2213,6 @@ static void get_replay_state(private_kernel_netlink_ipsec_t *this,
 				*replay = malloc(RTA_PAYLOAD(rta));
 				memcpy(*replay, RTA_DATA(rta), RTA_PAYLOAD(rta));
 			}
-			if (rta->rta_type == XFRMA_REPLAY_ESN_VAL &&
-				RTA_PAYLOAD(rta) >= sizeof(**replay_esn))
-			{
-				free(*replay_esn);
-				*replay_esn = malloc(RTA_PAYLOAD(rta));
-				*replay_esn_len = RTA_PAYLOAD(rta);
-				memcpy(*replay_esn, RTA_DATA(rta), RTA_PAYLOAD(rta));
-			}
 			rta = RTA_NEXT(rta, rtasize);
 		}
 	}
@@ -2433,7 +2424,7 @@ METHOD(kernel_ipsec_t, update_sa, status_t,
 	struct xfrm_replay_state *replay = NULL;
 	struct xfrm_replay_state_esn *replay_esn = NULL;
 	struct xfrm_lifetime_cur *lifetime = NULL;
-	uint32_t replay_esn_len = 0;
+	bool replay_state_seen = FALSE;
 	kernel_ipsec_del_sa_t del = { 0 };
 	status_t status = FAILED;
 	traffic_selector_t *ts;
@@ -2518,8 +2509,7 @@ METHOD(kernel_ipsec_t, update_sa, status_t,
 		goto failed;
 	}
 
-	get_replay_state(this, id, &replay_esn, &replay_esn_len, &replay,
-					 &lifetime);
+	get_replay_state(this, id, &replay, &lifetime);
 
 	/* delete the old SA (without affecting the IPComp SA) */
 	if (del_sa(this, id, &del) != SUCCESS)
@@ -2600,6 +2590,11 @@ METHOD(kernel_ipsec_t, update_sa, status_t,
 					free(ifname);
 				}
 			}
+			if (rta->rta_type == XFRMA_REPLAY_ESN_VAL ||
+				rta->rta_type == XFRMA_REPLAY_VAL)
+			{
+				replay_state_seen = TRUE;
+			}
 			netlink_add_attribute(hdr, rta->rta_type,
 								  chunk_create(RTA_DATA(rta), RTA_PAYLOAD(rta)),
 								  sizeof(request));
@@ -2621,34 +2616,25 @@ METHOD(kernel_ipsec_t, update_sa, status_t,
 		memset(&encap->encap_oa, 0, sizeof (xfrm_address_t));
 	}
 
-	if (replay_esn)
+	if (!replay_state_seen)
 	{
-		struct xfrm_replay_state_esn *state;
-
-		state = netlink_reserve(hdr, sizeof(request), XFRMA_REPLAY_ESN_VAL,
-								replay_esn_len);
-		if (!state)
+		if (replay)
 		{
-			goto failed;
-		}
-		memcpy(state, replay_esn, replay_esn_len);
-	}
-	else if (replay)
-	{
-		struct xfrm_replay_state *state;
+			struct xfrm_replay_state *state;
 
-		state = netlink_reserve(hdr, sizeof(request), XFRMA_REPLAY_VAL,
-								sizeof(*state));
-		if (!state)
+			state = netlink_reserve(hdr, sizeof(request), XFRMA_REPLAY_VAL,
+									sizeof(*state));
+			if (!state)
+			{
+				goto failed;
+			}
+			memcpy(state, replay, sizeof(*state));
+		}
+		else
 		{
-			goto failed;
+			DBG1(DBG_KNL, "unable to copy replay state from old SAD entry with "
+				 "SPI %.8x%s", ntohl(id->spi), markstr);
 		}
-		memcpy(state, replay, sizeof(*state));
-	}
-	else
-	{
-		DBG1(DBG_KNL, "unable to copy replay state from old SAD entry with "
-			 "SPI %.8x%s", ntohl(id->spi), markstr);
 	}
 	if (lifetime)
 	{