flow_cmp() must compare the same flow-attributes as the kernel,

otherwise we never can keep the in-daemon and the in-kernel idea
of flows in sync and iked ends up deleting flows that are still
in use.  Make use of flow_cmp() and a new flow_equal() instead
of handcrafting the compare in an if.

ok markus@ reyk@

3 files changed, 31 insertions, 26 deletions

diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index 38de8754582..62d84737400 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: iked.h,v 1.102 2017/02/03 08:23:46 guenther Exp $	*/
+/*	$OpenBSD: iked.h,v 1.103 2017/03/13 14:33:33 patrick Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -696,6 +696,7 @@ void	 childsa_free(struct iked_childsa *);
 struct iked_childsa *
 	 childsa_lookup(struct iked_sa *, uint64_t, uint8_t);
 void	 flow_free(struct iked_flow *);
+int	 flow_equal(struct iked_flow *, struct iked_flow *);
 struct iked_sa *
 	 sa_lookup(struct iked *, uint64_t, uint64_t, unsigned int);
 struct iked_user *
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index 4efd8c37fc2..0c191b6bf4f 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ikev2.c,v 1.134 2017/02/24 11:23:02 patrick Exp $	*/
+/*	$OpenBSD: ikev2.c,v 1.135 2017/03/13 14:33:33 patrick Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -4314,20 +4314,6 @@ ikev2_childsa_negotiate(struct iked *env, struct iked_sa *sa,
 			continue;
 
 		RB_FOREACH(flow, iked_flows, &sa->sa_policy->pol_flows) {
-			skip = 0;
-			TAILQ_FOREACH(saflow, &sa->sa_flows, flow_entry) {
-				if (IKED_ADDR_EQ(&saflow->flow_src,
-				    &flow->flow_src) &&
-				    IKED_ADDR_EQ(&saflow->flow_dst,
-				    &flow->flow_dst) &&
-				    saflow->flow_saproto ==
-				    prop->prop_protoid) {
-					skip = 1;
-					break;
-				}
-			}
-			if (skip)
-				continue;
 
 			if ((flowa = calloc(1, sizeof(*flowa))) == NULL) {
 				log_debug("%s: failed to get flow", __func__);
@@ -4342,6 +4328,18 @@ ikev2_childsa_negotiate(struct iked *env, struct iked_sa *sa,
 			flowa->flow_ikesa = sa;
 			ikev2_cp_fixaddr(sa, &flow->flow_dst, &flowa->flow_dst);
 
+			skip = 0;
+			TAILQ_FOREACH(saflow, &sa->sa_flows, flow_entry) {
+				if (flow_equal(saflow, flowa)) {
+					skip = 1;
+					break;
+				}
+			}
+			if (skip) {
+				flow_free(flowa);
+				continue;
+			}
+
 			if ((flowb = calloc(1, sizeof(*flowb))) == NULL) {
 				log_debug("%s: failed to get flow", __func__);
 				flow_free(flowa);
@@ -4573,7 +4571,8 @@ ikev2_ipcomp_enable(struct iked *env, struct iked_sa *sa)
 		    flow->flow_saproto != IKEV2_SAPROTO_ESP)
 			continue;
 		TAILQ_FOREACH(oflow, &sa->sa_flows, flow_entry)
-			if (IKED_ADDR_EQ(&oflow->flow_src, &flow->flow_src) &&
+			if (flow != oflow &&
+			    IKED_ADDR_EQ(&oflow->flow_src, &flow->flow_src) &&
 			    IKED_ADDR_EQ(&oflow->flow_dst, &flow->flow_dst) &&
 			    oflow->flow_dir == flow->flow_dir &&
 			    oflow->flow_saproto == IKEV2_SAPROTO_IPCOMP)
@@ -4606,10 +4605,7 @@ ikev2_ipcomp_enable(struct iked *env, struct iked_sa *sa)
 
 	/* skip if flow already exists */
 	TAILQ_FOREACH(flow, &sa->sa_flows, flow_entry) {
-		if (IKED_ADDR_EQ(&flow->flow_src, &flowa->flow_src) &&
-		    IKED_ADDR_EQ(&flow->flow_dst, &flowa->flow_dst) &&
-		    flow->flow_dir == flowa->flow_dir &&
-		    flow->flow_saproto == flowa->flow_saproto) {
+		if (flow_equal(flow, flowa)) {
 			free(flowa);
 			free(flowb);
 			goto done;
diff --git a/sbin/iked/policy.c b/sbin/iked/policy.c
index e401f14817c..7747535b70c 100644
--- a/sbin/iked/policy.c
+++ b/sbin/iked/policy.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: policy.c,v 1.43 2017/03/13 14:19:08 patrick Exp $	*/
+/*	$OpenBSD: policy.c,v 1.44 2017/03/13 14:33:33 patrick Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -585,18 +585,26 @@ flow_cmp(struct iked_flow *a, struct iked_flow *b)
 {
 	int		diff = 0;
 
-	if (a->flow_peer && b->flow_peer)
-		diff = addr_cmp(a->flow_peer, b->flow_peer, 0);
+	if (!diff)
+		diff = (int)a->flow_ipproto - (int)b->flow_ipproto;
+	if (!diff)
+		diff = (int)a->flow_saproto - (int)b->flow_saproto;
+	if (!diff)
+		diff = (int)a->flow_dir - (int)b->flow_dir;
 	if (!diff)
 		diff = addr_cmp(&a->flow_dst, &b->flow_dst, 1);
 	if (!diff)
 		diff = addr_cmp(&a->flow_src, &b->flow_src, 1);
-	if (!diff && a->flow_dir && b->flow_dir)
-		diff = (int)a->flow_dir - (int)b->flow_dir;
 
 	return (diff);
 }
 
+int
+flow_equal(struct iked_flow *a, struct iked_flow *b)
+{
+	return (flow_cmp(a, b) == 0);
+}
+
 RB_GENERATE(iked_sas, iked_sa, sa_entry, sa_cmp);
 RB_GENERATE(iked_addrpool, iked_sa, sa_addrpool_entry, sa_addrpool_cmp);
 RB_GENERATE(iked_addrpool6, iked_sa, sa_addrpool6_entry, sa_addrpool6_cmp);