make sure the state machine only advances if the AUTH payload has

been verified; with & ok mikeb@

5 files changed, 55 insertions, 31 deletions

diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index 7ac47bf0392..66816a8ab1e 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: iked.h,v 1.72 2014/04/22 12:00:03 reyk Exp $	*/
+/*	$OpenBSD: iked.h,v 1.73 2014/04/29 11:51:13 markus Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -324,15 +324,17 @@ struct iked_id {
 };
 
 #define IKED_REQ_CERT		0x01	/* get local certificate (if required) */
-#define IKED_REQ_VALID		0x02	/* validate the peer cert */
+#define IKED_REQ_CERTVALID	0x02	/* validated the peer cert */
 #define IKED_REQ_AUTH		0x04	/* AUTH payload */
-#define IKED_REQ_SA		0x08	/* SA available */
-#define IKED_REQ_CHILDSA	0x10	/* Child SA initiated */
-#define IKED_REQ_INF		0x20	/* Informational exchange initiated */
-#define IKED_REQ_DELETE		0x40	/* Rekeying continuation */
+#define IKED_REQ_AUTHVALID	0x08	/* AUTH payload has been verified */
+#define IKED_REQ_SA		0x10	/* SA available */
+#define IKED_REQ_EAPVALID	0x20	/* EAP payload has been verified */
+#define IKED_REQ_CHILDSA	0x40	/* Child SA initiated */
+#define IKED_REQ_INF		0x80	/* Informational exchange initiated */
+#define IKED_REQ_DELETE		0x100	/* Rekeying continuation */
 
 #define IKED_REQ_BITS	\
-    "\10\01CERT\02VALID\03AUTH\04SA"
+    "\20\01CERT\02CERTVALID\03AUTH\04AUTHVALID\05SA\06EAP"
 
 TAILQ_HEAD(iked_msgqueue, iked_message);
 
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index 2cf82ec29d9..302f3794620 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ikev2.c,v 1.101 2014/04/28 11:21:02 reyk Exp $	*/
+/*	$OpenBSD: ikev2.c,v 1.102 2014/04/29 11:51:13 markus Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -238,7 +238,7 @@ ikev2_dispatch_cert(int fd, struct privsep_proc *p, struct imsg *imsg)
 
 		if (imsg->hdr.type == IMSG_CERTVALID) {
 			log_debug("%s: peer certificate is valid", __func__);
-			sa_stateflags(sa, IKED_REQ_VALID);
+			sa_stateflags(sa, IKED_REQ_CERTVALID);
 			sa_state(env, sa, IKEV2_STATE_VALID);
 		} else {
 			log_warnx("%s: peer certificate is invalid", __func__);
@@ -296,6 +296,11 @@ ikev2_dispatch_cert(int fd, struct privsep_proc *p, struct imsg *imsg)
 			log_debug("%s: invalid auth reply", __func__);
 			break;
 		}
+		if (sa_stateok(sa, IKEV2_STATE_VALID)) {
+			log_warnx("%s: ignoring AUTH in state %s", __func__,
+			    print_map(sa->sa_state, ikev2_state_map));
+			break;
+		}
 
 		log_debug("%s: AUTH type %d len %zu", __func__, type, len);
 
@@ -314,6 +319,8 @@ ikev2_dispatch_cert(int fd, struct privsep_proc *p, struct imsg *imsg)
 		}
 
 		sa_stateflags(sa, IKED_REQ_AUTH);
+		/* Switch in case we already have certvalid or authvalid */
+		sa_state(env, sa, IKEV2_STATE_VALID);
 
 		if (ikev2_ike_auth(env, sa, NULL) != 0)
 			log_debug("%s: failed to send ike auth", __func__);
@@ -523,15 +530,17 @@ ikev2_ike_auth(struct iked *env, struct iked_sa *sa,
 		memcpy(id, &msg->msg_id, sizeof(*id));
 		bzero(&msg->msg_id, sizeof(msg->msg_id));
 
-		if ((authmsg = ikev2_msg_auth(env, sa,
-		    !sa->sa_hdr.sh_initiator)) == NULL) {
-			log_debug("%s: failed to get response "
-			    "auth data", __func__);
-			return (-1);
-		}
+		if (!sa->sa_hdr.sh_initiator) {
+			if ((authmsg = ikev2_msg_auth(env, sa,
+			    !sa->sa_hdr.sh_initiator)) == NULL) {
+				log_debug("%s: failed to get response "
+				    "auth data", __func__);
+				return (-1);
+			}
 
-		ca_setauth(env, sa, authmsg, PROC_CERT);
-		ibuf_release(authmsg);
+			ca_setauth(env, sa, authmsg, PROC_CERT);
+			ibuf_release(authmsg);
+		}
 	}
 
 	if (msg->msg_cert.id_type) {
@@ -578,8 +587,10 @@ ikev2_ike_auth(struct iked *env, struct iked_sa *sa,
 		    authmsg);
 		ibuf_release(authmsg);
 
-		if (ret != 0)
-			goto done;
+		if (ret != 0) {
+			log_debug("%s: ikev2_msg_authverify failed", __func__);
+			return (-1);
+		}
 
 		if (sa->sa_eapmsk != NULL) {
 			if ((authmsg = ikev2_msg_auth(env, sa,
@@ -598,6 +609,10 @@ ikev2_ike_auth(struct iked *env, struct iked_sa *sa,
 				return (-1);
 			}
 
+			/* ikev2_msg_authverify verified AUTH */
+			sa_stateflags(sa, IKED_REQ_AUTHVALID);
+			sa_stateflags(sa, IKED_REQ_EAPVALID);
+
 			sa_state(env, sa, IKEV2_STATE_EAP_VALID);
 		}
 	}
@@ -611,7 +626,6 @@ ikev2_ike_auth(struct iked *env, struct iked_sa *sa,
 		} else
 			sa_stateflags(sa, IKED_REQ_SA);
 	}
-
  done:
 	if (sa->sa_hdr.sh_initiator) {
 		if (sa_stateok(sa, IKEV2_STATE_AUTH_SUCCESS))
diff --git a/sbin/iked/ikev2.h b/sbin/iked/ikev2.h
index e8dbf7b5d76..a1daf478d94 100644
--- a/sbin/iked/ikev2.h
+++ b/sbin/iked/ikev2.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ikev2.h,v 1.13 2014/02/14 09:00:03 markus Exp $	*/
+/*	$OpenBSD: ikev2.h,v 1.14 2014/04/29 11:51:13 markus Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -34,7 +34,7 @@
 #define IKEV2_STATE_EAP			3	/* EAP requested */
 #define IKEV2_STATE_AUTH_REQUEST	4	/* auth received */
 #define IKEV2_STATE_AUTH_SUCCESS	5	/* authenticated */
-#define IKEV2_STATE_VALID		6	/* validated peer certs */
+#define IKEV2_STATE_VALID		6	/* authenticated AND validated certs */
 #define IKEV2_STATE_EAP_VALID		7	/* EAP validated */
 #define IKEV2_STATE_ESTABLISHED		8	/* active IKE SA */
 #define IKEV2_STATE_CLOSED		9	/* delete this SA */
diff --git a/sbin/iked/ikev2_msg.c b/sbin/iked/ikev2_msg.c
index 96abaaca9d0..0cb36891d4b 100644
--- a/sbin/iked/ikev2_msg.c
+++ b/sbin/iked/ikev2_msg.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ikev2_msg.c,v 1.31 2014/04/22 12:00:03 reyk Exp $	*/
+/*	$OpenBSD: ikev2_msg.c,v 1.32 2014/04/29 11:51:13 markus Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -756,6 +756,7 @@ ikev2_msg_authverify(struct iked *env, struct iked_sa *sa,
 	if ((ret = dsa_verify_final(dsa, buf, len)) == 0) {
 		log_debug("%s: authentication successful", __func__);
 		sa_state(env, sa, IKEV2_STATE_AUTH_SUCCESS);
+		sa_stateflags(sa, IKED_REQ_AUTHVALID);
 
 		if (!sa->sa_policy->pol_auth.auth_eap &&
 		    auth->auth_method == IKEV2_AUTH_SHARED_KEY_MIC)
diff --git a/sbin/iked/policy.c b/sbin/iked/policy.c
index 9292faf849a..561272931d4 100644
--- a/sbin/iked/policy.c
+++ b/sbin/iked/policy.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: policy.c,v 1.31 2014/02/21 20:52:38 markus Exp $	*/
+/*	$OpenBSD: policy.c,v 1.32 2014/04/29 11:51:13 markus Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -222,11 +222,17 @@ sa_state(struct iked *env, struct iked_sa *sa, int state)
 {
 	const char		*a;
 	const char		*b;
+	int 			ostate = sa->sa_state;
 
-	a = print_map(sa->sa_state, ikev2_state_map);
+	a = print_map(ostate, ikev2_state_map);
 	b = print_map(state, ikev2_state_map);
 
-	if (state > sa->sa_state) {
+	sa->sa_state = state;
+	if (ostate != IKEV2_STATE_INIT &&
+	    !sa_stateok(sa, state)) {
+		log_debug("%s: cannot switch: %s -> %s", __func__, a, b);
+		sa->sa_state = ostate;
+	} else if (ostate != sa->sa_state) {
 		switch (state) {
 		case IKEV2_STATE_ESTABLISHED:
 		case IKEV2_STATE_CLOSED:
@@ -244,7 +250,6 @@ sa_state(struct iked *env, struct iked_sa *sa, int state)
 		}
 	}
 
-	sa->sa_state = state;
 }
 
 void
@@ -280,6 +285,7 @@ sa_stateok(struct iked_sa *sa, int state)
 
 	if (state == IKEV2_STATE_SA_INIT ||
 	    state == IKEV2_STATE_VALID ||
+	    state == IKEV2_STATE_EAP_VALID ||
 	    state == IKEV2_STATE_EAP) {
 		log_debug("%s: %s flags 0x%02x, require 0x%02x %s", __func__,
 		    print_map(state, ikev2_state_map),
@@ -315,17 +321,18 @@ sa_new(struct iked *env, u_int64_t ispi, u_int64_t rspi,
 	else
 		pol = sa->sa_policy;
 
-	sa->sa_statevalid = IKED_REQ_AUTH|IKED_REQ_SA;
+	sa->sa_statevalid = IKED_REQ_AUTH|IKED_REQ_AUTHVALID|IKED_REQ_SA;
 	if (pol != NULL && pol->pol_auth.auth_eap) {
-		sa->sa_statevalid |= IKED_REQ_CERT;
+		sa->sa_statevalid |= IKED_REQ_CERT|IKED_REQ_EAPVALID;
 	} else if (pol != NULL && pol->pol_auth.auth_method !=
 	    IKEV2_AUTH_SHARED_KEY_MIC) {
-		sa->sa_statevalid |= IKED_REQ_VALID|IKED_REQ_CERT;
+		sa->sa_statevalid |= IKED_REQ_CERTVALID|IKED_REQ_CERT;
 	}
 
 	if (initiator) {
 		localid = &sa->sa_iid;
-		diff = IKED_REQ_VALID|IKED_REQ_SA;
+		diff = IKED_REQ_CERTVALID|IKED_REQ_AUTHVALID|IKED_REQ_SA|
+		    IKED_REQ_EAPVALID;
 		sa->sa_stateinit = sa->sa_statevalid & ~diff;
 		sa->sa_statevalid = sa->sa_statevalid & diff;
 	} else