From: markus Date: Tue, 29 Apr 2014 11:51:13 +0000 (+0000) Subject: make sure the state machine only advances if the AUTH payload has X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=4a986ab9d15eddc43a079516af68ef4fc9e7189c;p=openbsd 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 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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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