Blob Blame History Raw
diff -Naur libreswan-3.19-orig/programs/configs/d.ipsec.conf/uniqueids.xml libreswan-3.19/programs/configs/d.ipsec.conf/uniqueids.xml
--- libreswan-3.19-orig/programs/configs/d.ipsec.conf/uniqueids.xml	2017-01-15 14:34:34.000000000 -0500
+++ libreswan-3.19/programs/configs/d.ipsec.conf/uniqueids.xml	2017-02-03 17:41:48.703575771 -0500
@@ -1,17 +1,20 @@
   <varlistentry>
   <term><emphasis remap='B'>uniqueids</emphasis></term>
   <listitem>
-<para>whether a particular participant ID should be kept unique,
-with any new (automatically keyed)
-connection using an ID from a different IP address
-deemed to replace all old ones using that ID.
-Acceptable values are <emphasis remap='B'>yes</emphasis>
-(the default)
-and
-<emphasis remap='B'>no</emphasis>.
-Participant IDs normally <emphasis remap='I'>are</emphasis> unique,
-so a new (automatically-keyed) connection using the same ID is
-almost invariably intended to replace an old one.</para>
-
+<para> Whether IDs should be considered identifying remote parties
+uniquely. Acceptable values are <emphasis remap='B'>yes</emphasis> (the
+default) and <emphasis remap='B'>no</emphasis>.  Participant IDs normally
+are unique, so a new connection instance using the same remote ID is
+almost invariably intended to replace an old existing connection.
+</para>
+<para> When the connection is defined to be a server (using xauthserver=)
+and the connection policy is authby=secret, this option is ignored (as
+of 3.20) and old connections will never be replaced.  This situation is
+commonly known as clients using a "Group ID".
+</para>
+<para> This option may disappear in the near future. People using identical
+X.509 certificates on multiple devices are urged to upgrade to use seperate
+certificates per client and device.
+</para>
   </listitem>
   </varlistentry>
diff -Naur libreswan-3.19-orig/programs/pluto/ikev1_aggr.c libreswan-3.19/programs/pluto/ikev1_aggr.c
--- libreswan-3.19-orig/programs/pluto/ikev1_aggr.c	2017-01-15 14:34:34.000000000 -0500
+++ libreswan-3.19/programs/pluto/ikev1_aggr.c	2017-02-03 17:41:42.320610486 -0500
@@ -906,6 +906,9 @@
 	 */
 	set_ph1_iv_from_new(st);
 	DBG(DBG_CONTROL, DBG_log("phase 1 complete"));
+
+	ISAKMP_SA_established(st->st_connection, st->st_serialno);
+
 #ifdef USE_LINUX_AUDIT
 	linux_audit_conn(st, LAK_PARENT_START);
 #endif
@@ -1012,6 +1015,9 @@
 	 */
 	set_ph1_iv_from_new(st);
 	DBG(DBG_CONTROL, DBG_log("phase 1 complete"));
+
+	ISAKMP_SA_established(st->st_connection, st->st_serialno);
+
 #ifdef USE_LINUX_AUDIT
 	linux_audit_conn(st, LAK_PARENT_START);
 #endif
diff -Naur libreswan-3.19-orig/programs/pluto/ikev2_child.c libreswan-3.19/programs/pluto/ikev2_child.c
--- libreswan-3.19-orig/programs/pluto/ikev2_child.c	2017-01-15 14:34:34.000000000 -0500
+++ libreswan-3.19/programs/pluto/ikev2_child.c	2017-02-03 17:41:37.984634068 -0500
@@ -1271,6 +1271,9 @@
 
 	ikev2_derive_child_keys(cst, role);
 
+	/* Check to see if we need to release an old instance */
+       ISAKMP_SA_established(pst->st_connection, pst->st_serialno);
+
 	/* install inbound and outbound SPI info */
 	if (!install_ipsec_sa(cst, TRUE))
 		return STF_FATAL;
diff -Naur libreswan-3.19-orig/programs/pluto/initiate.c libreswan-3.19/programs/pluto/initiate.c
--- libreswan-3.19-orig/programs/pluto/initiate.c	2017-01-15 14:34:34.000000000 -0500
+++ libreswan-3.19/programs/pluto/initiate.c	2017-02-03 17:41:48.704575766 -0500
@@ -929,73 +929,58 @@
 bool uniqueIDs = FALSE; /* --uniqueids? */
 
 /*
- * Called by main_inI3_outR3_tail() and ikev2_child_sa_respond() which is called for
- * initiator and responder alike! So this function should not be in initiate.c.
- * It is also not called in IKEv1 Aggressive Mode!
+ * Called by IKEv1 and IKEv2 when the IKE SA is established.
+ * It checks if the freshly established connection needs is
+ * replacing an established version of itself.
+ *
+ * The use of uniqueIDs is mostly historic and might be removed
+ * in a future version. It is ignored for PSK based connections,
+ * which only act based on being a "server using PSK".
  */
 void ISAKMP_SA_established(struct connection *c, so_serial_t serial)
 {
 	c->newest_isakmp_sa = serial;
 
-	if (uniqueIDs && !c->spd.this.xauth_server &&
-		(c->policy & POLICY_AUTH_NULL) == LEMPTY) {
-		/*
-		 * for all connections: if the same Phase 1 IDs are used
-		 * for different IP addresses, unorient that connection.
-		 * We also check ports, since different Phase 1 ID's can
-		 * exist for the same IP when NAT is involved.
-		 */
-		struct connection *d;
-
-		for (d = connections; d != NULL; ) {
-			/* might move underneath us */
-			struct connection *next = d->ac_next;
-
-			/*
-			 * We try to find duplicate instances of same
-			 * connection to clean up old ones when uniqueids=yes
-			 *
-			 * We are testing for all of:
-			 * 1: an appropriate kind to consider
-			 * 2: same ids, left and right
-			 * 3: same address family
-			 * 4: same connection name
-			 * 5: but different IP address or port
-			 * 6: differing dnsnames (sort of)
-			 *
-			 * DHR (2014-10-29):
-			 * Is the sense of the last clause inverted?
-			 * The logic kind of suggests that in fact the
-			 * same dnsnames should be the same, not different.
-			 *
-			 * Let's make 6 clearer:
-			 *   if BOTH have dnsnames, they must be unequal.
-			 *
-			 * I suspect that it should be:
-			 *   if BOTH have dnsnames, they must be equal.
-			 *
-			 * In other words the streq result should be negated.
-			 */
-			if ((d->kind == CK_PERMANENT ||
-				d->kind == CK_INSTANCE ||
-				d->kind == CK_GOING_AWAY) &&
-				(c->name == d->name) &&
-				same_id(&c->spd.this.id, &d->spd.this.id) &&
-				same_id(&c->spd.that.id, &d->spd.that.id) &&
-				addrtypeof(&c->spd.that.host_addr) ==
-				addrtypeof(&d->spd.that.host_addr) &&
-				(!sameaddr(&c->spd.that.host_addr,
-					&d->spd.that.host_addr) ||
-				c->spd.that.host_port !=
-					d->spd.that.host_port) &&
-				!(c->dnshostname != NULL &&
-					d->dnshostname != NULL &&
-					streq(c->dnshostname,
-						d->dnshostname))) {
-				release_connection(d, FALSE);
-			}
-			d = next;
+	/* NULL authentication can never replaced - it is all anonnymous */
+	if (LIN(POLICY_AUTH_NULL, c->policy) ||
+	   ( c->spd.that.authby == AUTH_NULL || c->spd.this.authby == AUTH_NULL)) {
+
+		DBG(DBG_CONTROL, DBG_log("NULL Authentication - all clients appear identical"));
+		return;
+	}
+
+	/*
+	 * If we are a server and use PSK, all clients use the same group ID
+	 * Note that "xauth_server" also refers to IKEv2 CP
+	 */
+	if (c->spd.this.xauth_server && LIN(POLICY_PSK, c->policy)) {
+		DBG(DBG_CONTROL, DBG_log("We are a server using PSK and clients are using a group ID"));
+		return;
+	}
+
+	if (!uniqueIDs) {
+		DBG(DBG_CONTROL, DBG_log("uniqueIDs disabled, not contemplating releasing older self"));
+		return;
+	}
+
+	/*
+	 * for all existing connections: if the same Phase 1 IDs are used,
+	 * unorient that (old) connection - This is a replacement.
+	 */
+	struct connection *d;
+
+	for (d = connections; d != NULL; ) {
+		/* might move underneath us */
+		struct connection *next = d->ac_next;
+
+		if (c != d && c->kind == d->kind && streq(c->name, d->name) &&
+		    (same_id(&c->spd.this.id, &d->spd.this.id) &&
+		     same_id(&c->spd.that.id, &d->spd.that.id)))
+		{
+		  DBG(DBG_CONTROL, DBG_log("Unorienting old connection with same IDs"));
+		  release_connection(d, FALSE);
 		}
+		d = next;
 	}
 }