Simplify socket creation logic. Normally iked needs two sockets, one

for normal operation (UDP port 500) and one for NAT traversal (UDP 4500).
There are several command line options resulting in only one of the sockets
being created (-T, -t and -p).  Add a new 'enum natt_mode' to make the
logic for those somewhat less complicated as well as some comments where
it makes sense.

From Wataru Ashihara <wataash (at) wataash (dot) com>
ok patrick@

6 files changed, 52 insertions, 36 deletions

diff --git a/sbin/iked/config.c b/sbin/iked/config.c
index bc89c61c0ff..250eaece5e6 100644
--- a/sbin/iked/config.c
+++ b/sbin/iked/config.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: config.c,v 1.55 2020/03/24 13:32:36 tobhe Exp $	*/
+/*	$OpenBSD: config.c,v 1.56 2020/04/09 19:55:19 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -545,6 +545,10 @@ config_getreset(struct iked *env, struct imsg *imsg)
 	return (0);
 }
 
+/*
+ * The first call of this function sets the UDP socket for IKEv2.
+ * The second call is optional, setting the UDP socket used for NAT-T.
+ */
 int
 config_setsocket(struct iked *env, struct sockaddr_storage *ss,
     in_port_t port, enum privsep_procid id)
@@ -562,7 +566,7 @@ int
 config_getsocket(struct iked *env, struct imsg *imsg,
     void (*cb)(int, short, void *))
 {
-	struct iked_socket	*sock, **sptr, **nptr;
+	struct iked_socket	*sock, **sock0, **sock1;
 
 	log_debug("%s: received socket fd %d", __func__, imsg->fd);
 
@@ -577,23 +581,24 @@ config_getsocket(struct iked *env, struct imsg *imsg,
 
 	switch (sock->sock_addr.ss_family) {
 	case AF_INET:
-		sptr = &env->sc_sock4[0];
-		nptr = &env->sc_sock4[1];
+		sock0 = &env->sc_sock4[0];
+		sock1 = &env->sc_sock4[1];
 		break;
 	case AF_INET6:
-		sptr = &env->sc_sock6[0];
-		nptr = &env->sc_sock6[1];
+		sock0 = &env->sc_sock6[0];
+		sock1 = &env->sc_sock6[1];
 		break;
 	default:
-		fatal("config_getsocket: socket af");
+		fatal("config_getsocket: socket af: %u",
+		    sock->sock_addr.ss_family);
 		/* NOTREACHED */
 	}
-	if (*sptr == NULL)
-		*sptr = sock;
-	if (*nptr == NULL &&
-	    socket_getport((struct sockaddr *)&sock->sock_addr) ==
-	    IKED_NATT_PORT)
-		*nptr = sock;
+	if (*sock0 == NULL)
+		*sock0 = sock;
+	else if (*sock1 == NULL)
+		*sock1 = sock;
+	else
+		fatalx("%s: too many call", __func__);
 
 	event_set(&sock->sock_ev, sock->sock_fd,
 	    EV_READ|EV_PERSIST, cb, sock);
diff --git a/sbin/iked/iked.8 b/sbin/iked/iked.8
index fe7abf039df..7d33ab614c8 100644
--- a/sbin/iked/iked.8
+++ b/sbin/iked/iked.8
@@ -1,4 +1,4 @@
-.\" $OpenBSD: iked.8,v 1.25 2020/01/21 07:02:45 jmc Exp $
+.\" $OpenBSD: iked.8,v 1.26 2020/04/09 19:55:19 tobhe Exp $
 .\"
 .\" Copyright (c) 2010 - 2014 Reyk Floeter <reyk@openbsd.org>
 .\"
@@ -14,7 +14,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: January 21 2020 $
+.Dd $Mdocdate: April 9 2020 $
 .Dt IKED 8
 .Os
 .Sh NAME
@@ -85,6 +85,7 @@ than 4500, the
 .Em net.inet.esp.udpencap_port
 .Xr sysctl 2
 variable has to be set accordingly.
+Implies -t.
 .It Fl S
 Start
 .Nm
diff --git a/sbin/iked/iked.c b/sbin/iked/iked.c
index aa0bcd311b6..593db743dc1 100644
--- a/sbin/iked/iked.c
+++ b/sbin/iked/iked.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: iked.c,v 1.42 2020/04/03 09:11:23 tobhe Exp $	*/
+/*	$OpenBSD: iked.c,v 1.43 2020/04/09 19:55:19 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -67,6 +67,7 @@ main(int argc, char *argv[])
 	int		 c;
 	int		 debug = 0, verbose = 0;
 	int		 opts = 0;
+	enum natt_mode	 natt_mode = NATT_DEFAULT;
 	in_port_t	 port = IKED_NATT_PORT;
 	const char	*conffile = IKED_CONFIG;
 	struct iked	*env = NULL;
@@ -103,14 +104,20 @@ main(int argc, char *argv[])
 			opts |= IKED_OPT_PASSIVE;
 			break;
 		case 'T':
-			opts |= IKED_OPT_NONATT;
+			if (natt_mode == NATT_FORCE)
+				errx(1, "-T and -t/-p are mutually exclusive");
+			natt_mode = NATT_DISABLE;
 			break;
 		case 't':
-			opts |= IKED_OPT_NATT;
+			if (natt_mode == NATT_DISABLE)
+				errx(1, "-T and -t are mutually exclusive");
+			natt_mode = NATT_FORCE;
 			break;
 		case 'p':
+			if (natt_mode == NATT_DISABLE)
+				errx(1, "-T and -p are mutually exclusive");
 			port = atoi(optarg);
-			opts |= IKED_OPT_NATT;
+			natt_mode = NATT_FORCE;
 			break;
 		default:
 			usage();
@@ -126,16 +133,13 @@ main(int argc, char *argv[])
 		fatal("calloc: env");
 
 	env->sc_opts = opts;
+	env->natt_mode = natt_mode;
 	env->sc_nattport = port;
 
 	ps = &env->sc_ps;
 	ps->ps_env = env;
 	TAILQ_INIT(&ps->ps_rcsocks);
 
-	if ((opts & (IKED_OPT_NONATT|IKED_OPT_NATT)) ==
-	    (IKED_OPT_NONATT|IKED_OPT_NATT))
-		errx(1, "conflicting NAT-T options");
-
 	if (strlcpy(env->sc_conffile, conffile, PATH_MAX) >= PATH_MAX)
 		errx(1, "config file exceeds PATH_MAX");
 
@@ -227,17 +231,18 @@ parent_configure(struct iked *env)
 	bzero(&ss, sizeof(ss));
 	ss.ss_family = AF_INET;
 
-	if ((env->sc_opts & IKED_OPT_NATT) == 0 && env->sc_nattport == IKED_NATT_PORT)
+	/* see comment on config_setsocket() */
+	if (env->natt_mode != NATT_FORCE)
 		config_setsocket(env, &ss, htons(IKED_IKE_PORT), PROC_IKEV2);
-	if ((env->sc_opts & IKED_OPT_NONATT) == 0)
+	if (env->natt_mode != NATT_DISABLE)
 		config_setsocket(env, &ss, htons(env->sc_nattport), PROC_IKEV2);
 
 	bzero(&ss, sizeof(ss));
 	ss.ss_family = AF_INET6;
 
-	if ((env->sc_opts & IKED_OPT_NATT) == 0 && env->sc_nattport == IKED_NATT_PORT)
+	if (env->natt_mode != NATT_FORCE)
 		config_setsocket(env, &ss, htons(IKED_IKE_PORT), PROC_IKEV2);
-	if ((env->sc_opts & IKED_OPT_NONATT) == 0)
+	if (env->natt_mode != NATT_DISABLE)
 		config_setsocket(env, &ss, htons(env->sc_nattport), PROC_IKEV2);
 
 	/*
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index 598e82d0157..25616cea458 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: iked.h,v 1.141 2020/04/08 20:04:19 tobhe Exp $	*/
+/*	$OpenBSD: iked.h,v 1.142 2020/04/09 19:55:19 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -661,10 +661,17 @@ TAILQ_HEAD(iked_ocsp_requests, iked_ocsp_entry);
  * Daemon configuration
  */
 
+enum natt_mode {
+	NATT_DEFAULT,	/* send/recv with both :500 and NAT-T port */
+	NATT_DISABLE,	/* send/recv with only :500 */
+	NATT_FORCE,	/* send/recv with only NAT-T port */
+};
+
 struct iked {
 	char				 sc_conffile[PATH_MAX];
 
 	uint32_t			 sc_opts;
+	enum natt_mode			 natt_mode;
 	uint8_t				 sc_passive;
 	uint8_t				 sc_decoupled;
 	in_port_t			 sc_nattport;
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index 4074ea549b8..a36c33fa198 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ikev2.c,v 1.212 2020/04/08 20:04:19 tobhe Exp $	*/
+/*	$OpenBSD: ikev2.c,v 1.213 2020/04/09 19:55:19 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -1158,7 +1158,7 @@ ikev2_init_ike_sa_peer(struct iked *env, struct iked_policy *pol,
 			goto done;
 	}
 
-	if ((env->sc_opts & IKED_OPT_NONATT) == 0) {
+	if (env->natt_mode != NATT_DISABLE) {
 		if (ntohs(port) == env->sc_nattport) {
 			/* Enforce NAT-T on the initiator side */
 			log_debug("%s: enforcing NAT-T", __func__);
@@ -1975,7 +1975,7 @@ ikev2_nat_detection(struct iked *env, struct iked_message *msg,
 		goto done;
 	}
 
-	if (env->sc_opts & IKED_OPT_NATT) {
+	if (env->natt_mode == NATT_FORCE) {
 		/* Enforce NAT-T/UDP-encapsulation by distorting the digest */
 		rnd = arc4random();
 		EVP_DigestUpdate(&ctx, &rnd, sizeof(rnd));
@@ -2718,7 +2718,7 @@ ikev2_resp_ike_sa_init(struct iked *env, struct iked_message *msg)
 			goto done;
 	}
 
-	if ((env->sc_opts & IKED_OPT_NONATT) == 0 &&
+	if ((env->natt_mode != NATT_DISABLE) &&
 	    msg->msg_local.ss_family != AF_UNSPEC) {
 		if ((len = ikev2_add_nat_detection(env, buf, &pld, &resp, len))
 		    == -1)
diff --git a/sbin/iked/types.h b/sbin/iked/types.h
index 9211defc2dd..1276cea808b 100644
--- a/sbin/iked/types.h
+++ b/sbin/iked/types.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: types.h,v 1.34 2020/03/22 15:59:05 tobhe Exp $	*/
+/*	$OpenBSD: types.h,v 1.35 2020/04/09 19:55:20 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -46,9 +46,7 @@
 
 #define IKED_OPT_VERBOSE	0x00000001
 #define IKED_OPT_NOACTION	0x00000002
-#define IKED_OPT_NONATT		0x00000004
-#define IKED_OPT_NATT		0x00000008
-#define IKED_OPT_PASSIVE	0x00000010
+#define IKED_OPT_PASSIVE	0x00000004
 
 #define IKED_IKE_PORT		500
 #define IKED_NATT_PORT		4500