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

been verified; with & ok mikeb@

diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index 7ac47bf..66816a8 100644 (file)
--- 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 2cf82ec..302f379 100644 (file)
--- 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 e8dbf7b..a1daf47 100644 (file)
--- 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 96abaac..0cb3689 100644 (file)
--- 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 9292faf..5612729 100644 (file)
--- 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