[Swan-dev] IKEv1 and XFRMi interface

Antony Antony antony at phenome.org
Wed Sep 30 05:15:23 UTC 2020


On Wed, Sep 16, 2020 at 09:53:49AM -0400, Paul Wouters wrote:
> On Wed, 16 Sep 2020, Antony Antony wrote:
> 
> > I had a quic look. IKEv1 need extra message (3 round trips) as opposed to
> > IKEv2(2 round trips). And initiator is installing policies in different
> > order.
> 
> Yes, I mentioned this in the team email two days ago. That is indeed the
> source of the problem.
> 
> > the test outputs as it is now are confusing because it seems a copy of IKEv2
> > outputs. May be create tests with eastnet-westnet,  delete IKEv2 output and
> > updated with broken IKEv1 outout. That would make analysing it quicker.
> 
> I created copies of the IKEv2 tests for IKEv1. So whatever is tested for
> IKEv2 is tested for IKEv1. I can surely add a test case if it is missing
> for a subnet to subnet test for both IKEv1 and IKEv2 if that is missing.
> 
> > A better fix would be adding IKE pass policies, aka IKE holes, as XFRM
> > policies. I suspect there are also ways add routing policie instead of XFRM
> > polices, that is possibly what Android is doing.
> 
> Creating XFRM holes is dangerous. There might be overlapping
> machines/connections (eg extrusion on the far side related issues)
> 
> Can you say a bit more about adding routing policies? It seems that fix
> seems a better fit, as the problem right now is caused by routing to the
> interfaces?

add routing rules per IKE destination. That would route the packets
before it get looped into xfrmi device. I had a testcase, it is lost some 
where in my local branches! Let me see if I can find it.

Rule is added, if the client net and host interesct. That would look like
ip rule add from <ike src port> <ike src> to <ike dst> <ike dst port>

My thinking since we add the routes using updownscipt it might be hard to 
manage. We would have to adjust routing when we call update end points.

> > One thing that would help to add IKE policies is use of  struct 
> > kernel_sa
> > netlink_raw_eroute() same as  netlink_add_sa().  Now that KLIPS is gone we
> > make this change. Keeping the shunt code as it is.
> 
> What is wrong with the current method for IKE holes? I don't fully
> understand what you are saying here. Could you elaborate a bit more?

think of routing layer xfrmi added on top of xfrm policy.
The packet goes through extra loop because of xfrmi
https://upload.wikimedia.org/wikipedia/commons/3/37/Netfilter-packet-flow.svg
follow the output path xfrm. that has loop. Now with xfrmi loop that cause 
packet loop.

1. pluto wirtes IKE packet to a raw socket, xfrm policies allow it.
2. the IKE packet come up for routing and get routed
3. (if it is in the same namespcae) routing decision sends IKE packet to 
xfrm layer as a packet and would get encrypted.
4. go out as ESP instead of UDP.

We can try to shortcut step three with another IKE pass policy(not a socket 
policy), I didn't get to to work yet. May be the priorites were wrong.
OR it creates a loop. I am still figuring it out.

here is one patch that work by setting output mark on IKE UDP message.  Then 
it would get routed like ESP, using table 50 the updown script is creating.

It seems to fix test cases mentioned here.
I am not sure if this is a good patch, it call getsockopt and setsockopt for 
each UDP IKE send.

Anyone know if those calls are expensive? Should we look for other 
alternative? Another thought an outgoing socket per connection.
I don't think this socket will show up ip xfrm policy.
It will be just a raw socket.

test cases to try 

ikev2-xfrmi-02-responder (shows the problem on east follow IKE packets)
ikev1-xfrmi-02-tcpdump  (problem on road,no ping just mointor using tcpdump)
ikev1-xfrmi-02 ping get response
ikev2-xfrmi-02-rekey
ikev1-xfrmi-02-aggr
-------------- next part --------------
>From 0efe88aa0274df1b518337d35d4dc8d1ebffc6b8 Mon Sep 17 00:00:00 2001
From: Antony Antony <antony at phenome.org>
Date: Sat, 26 Sep 2020 09:06:59 +0000
Subject: [PATCH 1/3] xfrmi: set mark on udp out going socket to avoid

IKE packets getting encrypted and send via esp.
---
 include/ip_address.h                   |  1 +
 programs/pluto/iface_udp.c             | 39 +++++++++++++++++++++++++-
 programs/pluto/kernel.c                |  7 +++++
 programs/pluto/kernel_xfrm.c           | 24 ++++++++++++++++
 programs/pluto/kernel_xfrm_interface.h |  1 +
 programs/pluto/state.c                 |  2 ++
 6 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/include/ip_address.h b/include/ip_address.h
index a4dfd954d7d0..fbd0689cf6e9 100644
--- a/include/ip_address.h
+++ b/include/ip_address.h
@@ -67,6 +67,7 @@ typedef struct {
 	unsigned ipproto;
 	bool is_address;
 	bool is_endpoint;
+	uint32_t mark_out;
 #endif
 } ip_address;
 
diff --git a/programs/pluto/iface_udp.c b/programs/pluto/iface_udp.c
index 983e0a060048..9400aaea9cf1 100644
--- a/programs/pluto/iface_udp.c
+++ b/programs/pluto/iface_udp.c
@@ -362,6 +362,24 @@ static enum iface_status udp_read_packet(const struct iface_port *ifp,
 	return IFACE_OK;
 }
 
+static uint32_t set_mark_out(uint32_t mark, int fd)
+{
+	uint32_t old_mark;
+
+	socklen_t len = sizeof(old_mark);
+
+	if (getsockopt(fd, SOL_SOCKET, SO_MARK, &old_mark, &len) < 0) {
+		LOG_ERRNO(errno, "setsockopt(SO_MSRK) in set_mark_out()");
+		return 0;
+	}
+
+	if (setsockopt(fd, SOL_SOCKET, SO_MARK,
+				(const void *)&mark, sizeof(mark)) < 0) {
+		LOG_ERRNO(errno, "setsockopt(SO_MSRK) in set_mark_out()");
+	}
+	return old_mark;
+}
+
 static ssize_t udp_write_packet(const struct iface_port *ifp,
 				const void *ptr, size_t len,
 				const ip_endpoint *remote_endpoint)
@@ -371,9 +389,28 @@ static ssize_t udp_write_packet(const struct iface_port *ifp,
 		check_msg_errqueue(ifp, POLLOUT, __func__);
 	}
 #endif
+	endpoint_buf b;
+	uint32_t old_mark;
+	ssize_t ret;
 
 	ip_sockaddr remote_sa = sockaddr_from_endpoint(remote_endpoint);
-	return sendto(ifp->fd, ptr, len, 0, &remote_sa.sa.sa, remote_sa.len);
+	if (remote_endpoint->mark_out > 0) {
+		old_mark = set_mark_out(remote_endpoint->mark_out, ifp->fd);
+		dbg("AA_2020 %s:%d set mark on the socket %s to %u old mark %u",
+				__func__, __LINE__,
+				str_endpoint(remote_endpoint, &b),
+				remote_endpoint->mark_out, old_mark);
+	} else {
+		dbg("AA_2020 %s:%d NO mark on the socket %s",
+				__func__, __LINE__,
+				str_endpoint(remote_endpoint, &b));
+	}
+	ret = sendto(ifp->fd, ptr, len, 0, &remote_sa.sa.sa, remote_sa.len);
+
+	if (remote_endpoint->mark_out > 0)
+		set_mark_out(old_mark, ifp->fd);
+
+	return ret;
 };
 
 static void handle_udp_packet_cb(evutil_socket_t unused_fd UNUSED,
diff --git a/programs/pluto/kernel.c b/programs/pluto/kernel.c
index 05f0b06006dc..105507f8899f 100644
--- a/programs/pluto/kernel.c
+++ b/programs/pluto/kernel.c
@@ -673,6 +673,13 @@ bool do_command(const struct connection *c,
 	if (kernel_ops->docommand == NULL) {
 		dbg("no do_command for method %s", kernel_ops->kern_name);
 	} else {
+		if (c->xfrmi != NULL && (streq(verb, "up") ||
+					 streq(verb, "route"))) {
+			struct ike_sa *ike = ike_sa(st, HERE);
+			xfrmi_set_out_mark(c, sr, &ike->sa.st_remote_endpoint);
+			if (st->st_ike_version == IKEv1)
+				xfrmi_set_out_mark(c, sr, &st->st_remote_endpoint);
+		}
 		return (*kernel_ops->docommand)(c, sr, verb, verb_suffix, st);
 	}
 	return TRUE;
diff --git a/programs/pluto/kernel_xfrm.c b/programs/pluto/kernel_xfrm.c
index 11b477e57450..605dfb605a72 100644
--- a/programs/pluto/kernel_xfrm.c
+++ b/programs/pluto/kernel_xfrm.c
@@ -2739,6 +2739,30 @@ static bool netlink_poke_ipsec_policy_hole(const struct iface_dev *ifd, int fd)
 	return true;
 }
 
+void xfrmi_set_out_mark(const struct connection *c,
+			const struct spd_route *sr, ip_address *remote)
+{
+	if (!addrinsubnet(&sr->that.host_addr,
+				&sr->that.client)) {
+		return;
+	}
+
+	uint32_t mark_out;
+	if (c->sa_marks.out.val != 0)
+		mark_out = c->sa_marks.out.val;
+	else
+		mark_out =  c->xfrmi->if_id;
+
+	if (remote->mark_out != 0) {
+		passert(remote->mark_out == mark_out);
+		return;
+	}
+	remote->mark_out = mark_out;
+	endpoint_buf b;
+	dbg("AA_2020 %s:%d set mark %u for outgoing end %s", __func__, __LINE__,
+			remote->mark_out, str_endpoint(remote, &b));
+}
+
 const struct kernel_ops xfrm_kernel_ops = {
 	.kern_name = "xfrm",
 	.type = USE_XFRM,
diff --git a/programs/pluto/kernel_xfrm_interface.h b/programs/pluto/kernel_xfrm_interface.h
index 707646d8262d..e363f5a97bfd 100644
--- a/programs/pluto/kernel_xfrm_interface.h
+++ b/programs/pluto/kernel_xfrm_interface.h
@@ -47,3 +47,4 @@ extern err_t xfrm_iface_supported(struct logger *logger);
 extern void free_xfrmi_ipsec1(struct logger *logger);
 extern void unreference_xfrmi(struct connection *c, struct logger *logger);
 extern void reference_xfrmi(struct connection *c);
+extern void xfrmi_set_out_mark(const struct connection *c, const struct spd_route *sr, ip_address *remote);
diff --git a/programs/pluto/state.c b/programs/pluto/state.c
index efdc02c0187f..307383e1d5fd 100644
--- a/programs/pluto/state.c
+++ b/programs/pluto/state.c
@@ -2616,7 +2616,9 @@ void update_ike_endpoints(struct ike_sa *ike,
 			  const struct msg_digest *md)
 {
 	/* caller must ensure we are not behind NAT */
+	uint32_t mark_out = ike->sa.st_remote_endpoint.mark_out;
 	ike->sa.st_remote_endpoint = md->sender;
+	ike->sa.st_remote_endpoint.mark_out = mark_out;
 	endpoint_buf eb1, eb2;
 	dbg("#%lu updating local interface from %s to %s using md->iface "PRI_WHERE,
 	    ike->sa.st_serialno,
-- 
2.21.3

-------------- next part --------------
>From 701e3ce0a7c7e2be139c1ca5af039635e1220797 Mon Sep 17 00:00:00 2001
From: Antony Antony <antony at phenome.org>
Date: Sun, 27 Sep 2020 09:28:32 +0000
Subject: [PATCH 2/3] pluto: ikev1 xfrmi can not install xfrmi only when IKE is
 established

This was too early to instll xfrmi. It should be after
IPsec SA are installed.

Fixes: 9f3ef2f22b83 ("IKEv1: Support for XFRMi interfaces")
---
 programs/pluto/ikev1_main.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/programs/pluto/ikev1_main.c b/programs/pluto/ikev1_main.c
index bbff9275bd8f..ccb97a36f922 100644
--- a/programs/pluto/ikev1_main.c
+++ b/programs/pluto/ikev1_main.c
@@ -1713,11 +1713,6 @@ stf_status main_inI3_outR3(struct state *st, struct msg_digest *md)
 	}
 
 	IKE_SA_established(pexpect_ike_sa(st));
-#ifdef USE_XFRM_INTERFACE
-	if (c->xfrmi != NULL && c->xfrmi->if_id != 0)
-		if (add_xfrmi(c, st->st_logger))
-			return STF_FATAL;
-#endif
 	linux_audit_conn(st, LAK_PARENT_START);
 	return STF_OK;
 }
@@ -1768,11 +1763,6 @@ stf_status main_inR3(struct state *st, struct msg_digest *md)
 	}
 
 	IKE_SA_established(pexpect_ike_sa(st));
-#ifdef USE_XFRM_INTERFACE
-	if (c->xfrmi != NULL && c->xfrmi->if_id != 0)
-		if (add_xfrmi(c, st->st_logger))
-			return STF_FATAL;
-#endif
 	linux_audit_conn(st, LAK_PARENT_START);
 
 	passert((st->st_policy & POLICY_PFS) == 0 ||
-- 
2.21.3



More information about the Swan-dev mailing list