get rid of redundant {csa,flow}_{src,dst}id pointers, so we don't need

to update it on rekey (fixes use-after-free); ok mikeb@

diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index a1665f0..0e240f6 100644 (file)
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: iked.h,v 1.80 2014/05/09 06:29:46 markus Exp $        */
+/*     $OpenBSD: iked.h,v 1.81 2014/05/09 06:37:24 markus Exp $        */
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -146,9 +146,6 @@ struct iked_flow {
        u_int8_t                         flow_ipproto;
        u_int8_t                         flow_type;
 
-       struct iked_id                  *flow_srcid;
-       struct iked_id                  *flow_dstid;
-
        struct iked_addr                *flow_local;    /* outer source */
        struct iked_addr                *flow_peer;     /* outer dest */
        struct iked_sa                  *flow_ikesa;    /* parent SA */
@@ -179,9 +176,6 @@ struct iked_childsa {
        struct ibuf                     *csa_integrkey; /* auth key */
        u_int16_t                        csa_integrid;  /* auth xform id */
 
-       struct iked_id                  *csa_srcid;
-       struct iked_id                  *csa_dstid;
-
        struct iked_addr                *csa_local;     /* outer source */
        struct iked_addr                *csa_peer;      /* outer dest */
        struct iked_sa                  *csa_ikesa;     /* parent SA */
@@ -417,6 +411,8 @@ struct iked_sa {
        struct iked_id                   sa_rid;        /* responder id */
        struct iked_id                   sa_icert;      /* initiator cert */
        struct iked_id                   sa_rcert;      /* responder cert */
+#define IKESA_SRCID(x) ((x)->sa_hdr.sh_initiator ? &(x)->sa_iid : &(x)->sa_rid)
+#define IKESA_DSTID(x) ((x)->sa_hdr.sh_initiator ? &(x)->sa_rid : &(x)->sa_iid)
 
        char                            *sa_eapid;      /* EAP identity */
        struct iked_id                   sa_eap;        /* EAP challenge */

diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index 37feb43..d5953cb 100644 (file)
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ikev2.c,v 1.111 2014/05/09 06:29:46 markus Exp $      */
+/*     $OpenBSD: ikev2.c,v 1.112 2014/05/09 06:37:24 markus Exp $      */
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -4171,7 +4171,6 @@ ikev2_childsa_negotiate(struct iked *env, struct iked_sa *sa,
        struct iked_transform   *xform, *encrxf = NULL, *integrxf = NULL;
        struct iked_childsa     *csa, *csb;
        struct iked_flow        *flow, *saflow, *flowa, *flowb;
-       struct iked_id          *peerid, *localid;
        struct ibuf             *keymat = NULL, *seed = NULL, *dhsecret = NULL;
        struct group            *group;
        u_int32_t                spi = 0;
@@ -4182,15 +4181,7 @@ ikev2_childsa_negotiate(struct iked *env, struct iked_sa *sa,
        if (!sa_stateok(sa, IKEV2_STATE_VALID))
                return (-1);
 
-       if (sa->sa_hdr.sh_initiator) {
-               peerid = &sa->sa_rid;
-               localid = &sa->sa_iid;
-       } else {
-               peerid = &sa->sa_iid;
-               localid = &sa->sa_rid;
-       }
-
-       if (ikev2_sa_tag(sa, peerid) == -1)
+       if (ikev2_sa_tag(sa, IKESA_DSTID(sa)) == -1)
                return (-1);
 
        /* We need to determine the key material length first */
@@ -4289,8 +4280,6 @@ ikev2_childsa_negotiate(struct iked *env, struct iked_sa *sa,
                        memcpy(flowa, flow, sizeof(*flow));
                        flowa->flow_dir = IPSP_DIRECTION_OUT;
                        flowa->flow_saproto = prop->prop_protoid;
-                       flowa->flow_srcid = localid;
-                       flowa->flow_dstid = peerid;
                        flowa->flow_local = &sa->sa_local;
                        flowa->flow_peer = &sa->sa_peer;
                        flowa->flow_ikesa = sa;
@@ -4330,8 +4319,6 @@ ikev2_childsa_negotiate(struct iked *env, struct iked_sa *sa,
 
                csa->csa_saproto = prop->prop_protoid;
                csa->csa_ikesa = sa;
-               csa->csa_srcid = localid;
-               csa->csa_dstid = peerid;
                csa->csa_spi.spi_protoid = prop->prop_protoid;
                csa->csa_esn = esn;
 
@@ -4481,13 +4468,6 @@ ikev2_ipcomp_enable(struct iked *env, struct iked_sa *sa)
        /* install IPCOMP SAs */
        csa->csa_ikesa = sa;
        csa->csa_saproto = IKEV2_SAPROTO_IPCOMP;
-       if (sa->sa_hdr.sh_initiator) {
-               csa->csa_dstid = &sa->sa_rid;
-               csa->csa_srcid = &sa->sa_iid;
-       } else {
-               csa->csa_dstid = &sa->sa_iid;
-               csa->csa_srcid = &sa->sa_rid;
-       }
        csa->csa_spi.spi_size = 2;
        csa->csa_spi.spi = sa->sa_cpi_out;
        csa->csa_peerspi = sa->sa_cpi_in;
@@ -4557,13 +4537,6 @@ ikev2_ipcomp_enable(struct iked *env, struct iked_sa *sa)
        /* setup ESP flows for gateways */
        flowa->flow_dir = IPSP_DIRECTION_OUT;
        flowa->flow_saproto = IKEV2_SAPROTO_ESP;
-       if (sa->sa_hdr.sh_initiator) {
-               flowa->flow_dstid = &sa->sa_rid;
-               flowa->flow_srcid = &sa->sa_iid;
-       } else {
-               flowa->flow_dstid = &sa->sa_iid;
-               flowa->flow_srcid = &sa->sa_rid;
-       }
        flowa->flow_local = &sa->sa_local;
        flowa->flow_peer = &sa->sa_peer;
        memcpy(&flowa->flow_src, &sa->sa_local, sizeof(sa->sa_local));

diff --git a/sbin/iked/pfkey.c b/sbin/iked/pfkey.c
index afe2d60..b92138c 100644 (file)
--- a/sbin/iked/pfkey.c
+++ b/sbin/iked/pfkey.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pfkey.c,v 1.36 2014/05/09 06:29:46 markus Exp $       */
+/*     $OpenBSD: pfkey.c,v 1.37 2014/05/09 06:37:24 markus Exp $       */
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -312,11 +312,11 @@ pfkey_flow(int sd, u_int8_t satype, u_int8_t action, struct iked_flow *flow)
                    (sizeof(sa_peer) + ROUNDUP(speer.ss_len)) / 8;
 
                /* local id */
-               sa_srcid = pfkey_id2ident(flow->flow_srcid,
+               sa_srcid = pfkey_id2ident(IKESA_SRCID(flow->flow_ikesa),
                    SADB_EXT_IDENTITY_SRC);
 
                /* peer id */
-               sa_dstid = pfkey_id2ident(flow->flow_dstid,
+               sa_dstid = pfkey_id2ident(IKESA_DSTID(flow->flow_ikesa),
                    SADB_EXT_IDENTITY_DST);
        }
 
@@ -578,11 +578,18 @@ pfkey_sa(int sd, u_int8_t satype, u_int8_t action, struct iked_childsa *sa)
        if (satype == SADB_X_SATYPE_IPCOMP)
                sadb.sadb_sa_encrypt = SADB_X_CALG_DEFLATE;
 
-       /* local id */
-       sa_srcid = pfkey_id2ident(sa->csa_srcid, SADB_EXT_IDENTITY_SRC);
-
-       /* peer id */
-       sa_dstid = pfkey_id2ident(sa->csa_dstid, SADB_EXT_IDENTITY_DST);
+       /* Note that we need to swap the IDs for incoming SAs (SADB_UPDATE) */
+       if (action != SADB_UPDATE) {
+               sa_srcid = pfkey_id2ident(
+                   IKESA_SRCID(sa->csa_ikesa), SADB_EXT_IDENTITY_SRC);
+               sa_dstid = pfkey_id2ident(
+                   IKESA_DSTID(sa->csa_ikesa), SADB_EXT_IDENTITY_DST);
+       } else {
+               sa_srcid = pfkey_id2ident(
+                   IKESA_DSTID(sa->csa_ikesa), SADB_EXT_IDENTITY_SRC);
+               sa_dstid = pfkey_id2ident(
+                   IKESA_SRCID(sa->csa_ikesa), SADB_EXT_IDENTITY_DST);
+       }
 
        tag = sa->csa_ikesa->sa_tag;
        if (tag != NULL && *tag != '\0') {