child-sa: Pass default interface ID inherited from IKE_SA
authorTobias Brunner <tobias@strongswan.org>
Fri, 22 Mar 2019 16:39:47 +0000 (17:39 +0100)
committerTobias Brunner <tobias@strongswan.org>
Thu, 4 Apr 2019 07:36:38 +0000 (09:36 +0200)
Also pass optional arguments as struct.

src/libcharon/plugins/ha/ha_dispatcher.c
src/libcharon/sa/child_sa.c
src/libcharon/sa/child_sa.h
src/libcharon/sa/ikev1/tasks/quick_mode.c
src/libcharon/sa/ikev2/tasks/child_create.c
src/libcharon/sa/trap_manager.c

index 11f3bd9..ff75cb5 100644 (file)
@@ -743,10 +743,11 @@ static void process_child_add(private_ha_dispatcher_t *this,
                return;
        }
 
+       child_sa_create_t data = {
+               .encap = ike_sa->has_condition(ike_sa, COND_NAT_ANY),
+       };
        child_sa = child_sa_create(ike_sa->get_my_host(ike_sa),
-                                                          ike_sa->get_other_host(ike_sa), config, 0,
-                                                          ike_sa->has_condition(ike_sa, COND_NAT_ANY),
-                                                          0, 0, 0, 0);
+                                                          ike_sa->get_other_host(ike_sa), config, &data);
        child_sa->set_mode(child_sa, mode);
        child_sa->set_protocol(child_sa, PROTO_ESP);
        child_sa->set_ipcomp(child_sa, ipcomp);
index 40137d3..fc60b41 100644 (file)
@@ -1784,13 +1784,11 @@ static host_t* get_proxy_addr(child_cfg_t *config, host_t *ike, bool local)
        return host;
 }
 
-/**
- * Described in header.
+/*
+ * Described in header
  */
-child_sa_t * child_sa_create(host_t *me, host_t* other,
-                                                        child_cfg_t *config, uint32_t reqid, bool encap,
-                                                        uint32_t mark_in, uint32_t mark_out,
-                                                        uint32_t if_id_in, uint32_t if_id_out)
+child_sa_t *child_sa_create(host_t *me, host_t *other, child_cfg_t *config,
+                                                       child_sa_create_t *data)
 {
        private_child_sa_t *this;
        static refcount_t unique_id = 0, unique_mark = 0;
@@ -1839,7 +1837,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
                        .create_policy_enumerator = _create_policy_enumerator,
                        .destroy = _destroy,
                },
-               .encap = encap,
+               .encap = data->encap,
                .ipcomp = IPCOMP_NONE,
                .state = CHILD_CREATED,
                .my_ts = array_create(0, 0),
@@ -1852,8 +1850,8 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
                .unique_id = ref_get(&unique_id),
                .mark_in = config->get_mark(config, TRUE),
                .mark_out = config->get_mark(config, FALSE),
-               .if_id_in = config->get_if_id(config, TRUE),
-               .if_id_out = config->get_if_id(config, FALSE),
+               .if_id_in = config->get_if_id(config, TRUE) ?: data->if_id_in_def,
+               .if_id_out = config->get_if_id(config, FALSE) ?: data->if_id_out_def,
                .install_time = time_monotonic(NULL),
                .policies_fwd_out = config->has_option(config, OPT_FWD_OUT_POLICIES),
        );
@@ -1861,21 +1859,21 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
        this->config = config;
        config->get_ref(config);
 
-       if (mark_in)
+       if (data->mark_in)
        {
-               this->mark_in.value = mark_in;
+               this->mark_in.value = data->mark_in;
        }
-       if (mark_out)
+       if (data->mark_out)
        {
-               this->mark_out.value = mark_out;
+               this->mark_out.value = data->mark_out;
        }
-       if (if_id_in)
+       if (data->if_id_in)
        {
-               this->if_id_in = if_id_in;
+               this->if_id_in = data->if_id_in;
        }
-       if (if_id_out)
+       if (data->if_id_out)
        {
-               this->if_id_out = if_id_out;
+               this->if_id_out = data->if_id_out;
        }
 
        allocate_unique_if_ids(&this->if_id_in, &this->if_id_out);
@@ -1911,7 +1909,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
                 * replace the temporary SA on the kernel level. Rekeying such an SA
                 * requires an explicit reqid, as the cache currently knows the original
                 * selectors only for that reqid. */
-               this->reqid = reqid;
+               this->reqid = data->reqid;
        }
        else
        {
index 483dd15..c9b3f63 100644 (file)
@@ -26,6 +26,7 @@
 typedef enum child_sa_state_t child_sa_state_t;
 typedef enum child_sa_outbound_state_t child_sa_outbound_state_t;
 typedef struct child_sa_t child_sa_t;
+typedef struct child_sa_create_t child_sa_create_t;
 
 #include <library.h>
 #include <crypto/prf_plus.h>
@@ -513,22 +514,39 @@ struct child_sa_t {
 };
 
 /**
+ * Data passed to the constructor of a child_sa_t object.
+ */
+struct child_sa_create_t {
+       /** Optional reqid of old CHILD_SA when rekeying */
+       uint32_t reqid;
+       /** Optional inbound mark when rekeying */
+       uint32_t mark_in;
+       /** Optional outbound mark when rekeying */
+       uint32_t mark_out;
+       /** Optional inbound interface ID when rekeying */
+       uint32_t if_id_in;
+       /** Optional outbound interface ID when rekeying */
+       uint32_t if_id_out;
+       /** Optional default inbound interface ID, if neither if_id_in, nor config
+        * sets one */
+       uint32_t if_id_in_def;
+       /** Optional default outbound interface ID, if neither if_id_out, nor config
+        * sets one */
+       uint32_t if_id_out_def;
+       /** TRUE to enable UDP encapsulation (NAT traversal) */
+       bool encap;
+};
+
+/**
  * Constructor to create a child SA negotiated with IKE.
  *
  * @param me                           own address
  * @param other                                remote address
  * @param config                       config to use for this CHILD_SA
- * @param reqid                                reqid of old CHILD_SA when rekeying, 0 otherwise
- * @param encap                                TRUE to enable UDP encapsulation (NAT traversal)
- * @param mark_in                      explicit inbound mark value to use, 0 for config
- * @param mark_out                     explicit outbound mark value to use, 0 for config
- * @param if_id_in                     explicit inbound interface ID to use, 0 for config
- * @param if_id_out                    explicit outbound interface ID to use, 0 for config
+ * @param data                         data for this CHILD_SA
  * @return                                     child_sa_t object
  */
-child_sa_t * child_sa_create(host_t *me, host_t *other, child_cfg_t *config,
-                                                        uint32_t reqid, bool encap,
-                                                        uint32_t mark_in, uint32_t mark_out,
-                                                        uint32_t if_id_in, uint32_t if_id_out);
+child_sa_t *child_sa_create(host_t *me, host_t *other, child_cfg_t *config,
+                                                       child_sa_create_t *data);
 
 #endif /** CHILD_SA_H_ @}*/
index 59f049d..3309a5d 100644 (file)
@@ -151,29 +151,9 @@ struct private_quick_mode_t {
        uint64_t lifebytes;
 
        /**
-        * Reqid to use, 0 for auto-allocate
+        * Data collected to create the CHILD_SA
         */
-       uint32_t reqid;
-
-       /**
-        * Explicit inbound mark value to use, if any
-        */
-       uint32_t mark_in;
-
-       /**
-        * Explicit outbound mark value to use, if any
-        */
-       uint32_t mark_out;
-
-       /**
-        * Explicit inbound interface ID to use, if any
-        */
-       uint32_t if_id_in;
-
-       /**
-        * Explicit outbound interface ID to use, if any
-        */
-       uint32_t if_id_out;
+       child_sa_create_t child;
 
        /**
         * SPI of SA we rekey
@@ -196,11 +176,6 @@ struct private_quick_mode_t {
        protocol_id_t proto;
 
        /**
-        * Use UDP encapsulation
-        */
-       bool udp;
-
-       /**
         * Message ID of handled quick mode exchange
         */
        uint32_t mid;
@@ -637,7 +612,7 @@ static bool get_ts(private_quick_mode_t *this, message_t *message)
                tsr = traffic_selector_create_from_subnet(hsr->clone(hsr),
                                        hsr->get_family(hsr) == AF_INET ? 32 : 128, 0, 0, 65535);
        }
-       if (this->mode == MODE_TRANSPORT && this->udp &&
+       if (this->mode == MODE_TRANSPORT && this->child.encap &&
           (!tsi->is_host(tsi, hsi) || !tsr->is_host(tsr, hsr)))
        {       /* change TS in case of a NAT in transport mode */
                DBG2(DBG_IKE, "changing received traffic selectors %R=== %R due to NAT",
@@ -849,16 +824,19 @@ METHOD(task_t, build_i, status_t,
                        diffie_hellman_group_t group;
                        encap_t encap;
 
-                       this->udp = this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY);
                        this->mode = this->config->get_mode(this->config);
+                       this->child.if_id_in_def = this->ike_sa->get_if_id(this->ike_sa,
+                                                                                                                          TRUE);
+                       this->child.if_id_out_def = this->ike_sa->get_if_id(this->ike_sa,
+                                                                                                                               FALSE);
+                       this->child.encap = this->ike_sa->has_condition(this->ike_sa,
+                                                                                                                       COND_NAT_ANY);
                        this->child_sa = child_sa_create(
                                                                        this->ike_sa->get_my_host(this->ike_sa),
                                                                        this->ike_sa->get_other_host(this->ike_sa),
-                                                                       this->config, this->reqid, this->udp,
-                                                                       this->mark_in, this->mark_out,
-                                                                       this->if_id_in, this->if_id_out);
+                                                                       this->config, &this->child);
 
-                       if (this->udp && this->mode == MODE_TRANSPORT)
+                       if (this->child.encap && this->mode == MODE_TRANSPORT)
                        {
                                /* TODO-IKEv1: disable NAT-T for TRANSPORT mode by default? */
                                add_nat_oa_payloads(this, message);
@@ -925,7 +903,7 @@ METHOD(task_t, build_i, status_t,
                        }
 
                        get_lifetimes(this);
-                       encap = get_encap(this->ike_sa, this->udp);
+                       encap = get_encap(this->ike_sa, this->child.encap);
                        sa_payload = sa_payload_create_from_proposals_v1(list,
                                                                this->lifetime, this->lifebytes, AUTH_NONE,
                                                                this->mode, encap, this->cpi_i);
@@ -1037,7 +1015,7 @@ static void check_for_rekeyed_child(private_quick_mode_t *this, bool responder)
 
        name = this->config->get_name(this->config);
        enumerator = this->ike_sa->create_child_sa_enumerator(this->ike_sa);
-       while (this->reqid == 0 && enumerator->enumerate(enumerator, &child_sa))
+       while (!this->child.reqid && enumerator->enumerate(enumerator, &child_sa))
        {
                if (streq(child_sa->get_name(child_sa), name))
                {
@@ -1052,14 +1030,16 @@ static void check_for_rekeyed_child(private_quick_mode_t *this, bool responder)
                                                remote->equals(remote, other_ts) &&
                                                this->proposal->equals(this->proposal, proposal))
                                        {
-                                               this->reqid = child_sa->get_reqid(child_sa);
                                                this->rekey = child_sa->get_spi(child_sa, TRUE);
-                                               this->mark_in = child_sa->get_mark(child_sa,
-                                                                                                                       TRUE).value;
-                                               this->mark_out = child_sa->get_mark(child_sa,
-                                                                                                                       FALSE).value;
-                                               this->if_id_in = child_sa->get_if_id(child_sa, TRUE);
-                                               this->if_id_out = child_sa->get_if_id(child_sa, FALSE);
+                                               this->child.reqid = child_sa->get_reqid(child_sa);
+                                               this->child.mark_in = child_sa->get_mark(child_sa,
+                                                                                                                                TRUE).value;
+                                               this->child.mark_out = child_sa->get_mark(child_sa,
+                                                                                                                                 FALSE).value;
+                                               this->child.if_id_in = child_sa->get_if_id(child_sa,
+                                                                                                                                  TRUE);
+                                               this->child.if_id_out = child_sa->get_if_id(child_sa,
+                                                                                                                                       FALSE);
                                                child_sa->set_state(child_sa, CHILD_REKEYING);
                                                DBG1(DBG_IKE, "detected rekeying of CHILD_SA %s{%u}",
                                                         child_sa->get_name(child_sa),
@@ -1102,7 +1082,8 @@ METHOD(task_t, process_r, status_t,
                                return send_notify(this, INVALID_PAYLOAD_TYPE);
                        }
 
-                       this->mode = sa_payload->get_encap_mode(sa_payload, &this->udp);
+                       this->mode = sa_payload->get_encap_mode(sa_payload,
+                                                                                                       &this->child.encap);
 
                        if (!get_ts(this, message))
                        {
@@ -1193,13 +1174,14 @@ METHOD(task_t, process_r, status_t,
                        }
 
                        check_for_rekeyed_child(this, TRUE);
-
+                       this->child.if_id_in_def = this->ike_sa->get_if_id(this->ike_sa,
+                                                                                                                          TRUE);
+                       this->child.if_id_out_def = this->ike_sa->get_if_id(this->ike_sa,
+                                                                                                                               FALSE);
                        this->child_sa = child_sa_create(
                                                                        this->ike_sa->get_my_host(this->ike_sa),
                                                                        this->ike_sa->get_other_host(this->ike_sa),
-                                                                       this->config, this->reqid, this->udp,
-                                                                       this->mark_in, this->mark_out,
-                                                                       this->if_id_in, this->if_id_out);
+                                                                       this->config, &this->child);
 
                        tsi = linked_list_create_with_items(this->tsi, NULL);
                        tsr = linked_list_create_with_items(this->tsr, NULL);
@@ -1291,13 +1273,13 @@ METHOD(task_t, build_r, status_t,
                                }
                        }
 
-                       if (this->udp && this->mode == MODE_TRANSPORT)
+                       if (this->child.encap && this->mode == MODE_TRANSPORT)
                        {
                                /* TODO-IKEv1: disable NAT-T for TRANSPORT mode by default? */
                                add_nat_oa_payloads(this, message);
                        }
 
-                       encap = get_encap(this->ike_sa, this->udp);
+                       encap = get_encap(this->ike_sa, this->child.encap);
                        sa_payload = sa_payload_create_from_proposal_v1(this->proposal,
                                                                this->lifetime, this->lifebytes, AUTH_NONE,
                                                                this->mode, encap, this->cpi_r);
@@ -1422,21 +1404,21 @@ METHOD(quick_mode_t, get_mid, uint32_t,
 METHOD(quick_mode_t, use_reqid, void,
        private_quick_mode_t *this, uint32_t reqid)
 {
-       this->reqid = reqid;
+       this->child.reqid = reqid;
 }
 
 METHOD(quick_mode_t, use_marks, void,
        private_quick_mode_t *this, uint32_t in, uint32_t out)
 {
-       this->mark_in = in;
-       this->mark_out = out;
+       this->child.mark_in = in;
+       this->child.mark_out = out;
 }
 
 METHOD(quick_mode_t, use_if_ids, void,
        private_quick_mode_t *this, uint32_t in, uint32_t out)
 {
-       this->if_id_in = in;
-       this->if_id_out = out;
+       this->child.if_id_in = in;
+       this->child.if_id_out = out;
 }
 
 METHOD(quick_mode_t, rekey, void,
@@ -1467,10 +1449,7 @@ METHOD(task_t, migrate, void,
        this->dh = NULL;
        this->spi_i = 0;
        this->spi_r = 0;
-       this->mark_in = 0;
-       this->mark_out = 0;
-       this->if_id_in = 0;
-       this->if_id_out = 0;
+       this->child = (child_sa_create_t){};
 
        if (!this->initiator)
        {
index 340542b..d74013b 100644 (file)
@@ -169,29 +169,9 @@ struct private_child_create_t {
        uint16_t other_cpi;
 
        /**
-        * reqid to use if we are rekeying
+        * Data collected to create the CHILD_SA
         */
-       uint32_t reqid;
-
-       /**
-        * Explicit inbound mark value
-        */
-       uint32_t mark_in;
-
-       /**
-        * Explicit outbound mark value
-        */
-       uint32_t mark_out;
-
-       /**
-        * Explicit inbound interface ID to use, if any
-        */
-       uint32_t if_id_in;
-
-       /**
-        * Explicit outbound interface ID to use, if any
-        */
-       uint32_t if_id_out;
+       child_sa_create_t child;
 
        /**
         * CHILD_SA which gets established
@@ -227,7 +207,10 @@ static void schedule_delayed_retry(private_child_create_t *this)
        task = child_create_create(this->ike_sa,
                                                           this->config->get_ref(this->config), FALSE,
                                                           this->packet_tsi, this->packet_tsr);
-       task->use_reqid(task, this->reqid);
+       task->use_reqid(task, this->child.reqid);
+       task->use_marks(task, this->child.mark_in, this->child.mark_out);
+       task->use_if_ids(task, this->child.if_id_in, this->child.if_id_out);
+
        DBG1(DBG_IKE, "creating CHILD_SA failed, trying again in %d seconds",
                 retry);
        this->ike_sa->queue_task_delayed(this->ike_sa, (task_t*)task, retry);
@@ -1117,16 +1100,18 @@ METHOD(task_t, build_i, status_t,
                                                                                                  this->dh_group == MODP_NONE);
        this->mode = this->config->get_mode(this->config);
 
+       this->child.if_id_in_def = this->ike_sa->get_if_id(this->ike_sa, TRUE);
+       this->child.if_id_out_def = this->ike_sa->get_if_id(this->ike_sa, FALSE);
+       this->child.encap = this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY);
        this->child_sa = child_sa_create(this->ike_sa->get_my_host(this->ike_sa),
-                       this->ike_sa->get_other_host(this->ike_sa), this->config, this->reqid,
-                       this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY),
-                       this->mark_in, this->mark_out, this->if_id_in, this->if_id_out);
+                                                                        this->ike_sa->get_other_host(this->ike_sa),
+                                                                        this->config, &this->child);
 
-       if (this->reqid)
+       if (this->child.reqid)
        {
                DBG0(DBG_IKE, "establishing CHILD_SA %s{%d} reqid %d",
                         this->child_sa->get_name(this->child_sa),
-                        this->child_sa->get_unique_id(this->child_sa), this->reqid);
+                        this->child_sa->get_unique_id(this->child_sa), this->child.reqid);
        }
        else
        {
@@ -1402,10 +1387,12 @@ METHOD(task_t, build_r, status_t,
        }
        enumerator->destroy(enumerator);
 
+       this->child.if_id_in_def = this->ike_sa->get_if_id(this->ike_sa, TRUE);
+       this->child.if_id_out_def = this->ike_sa->get_if_id(this->ike_sa, FALSE);
+       this->child.encap = this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY);
        this->child_sa = child_sa_create(this->ike_sa->get_my_host(this->ike_sa),
-                       this->ike_sa->get_other_host(this->ike_sa), this->config, this->reqid,
-                       this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY),
-                       this->mark_in, this->mark_out, this->if_id_in, this->if_id_out);
+                                                                        this->ike_sa->get_other_host(this->ike_sa),
+                                                                        this->config, &this->child);
 
        if (this->ipcomp_received != IPCOMP_NONE)
        {
@@ -1670,21 +1657,21 @@ METHOD(task_t, process_i, status_t,
 METHOD(child_create_t, use_reqid, void,
        private_child_create_t *this, uint32_t reqid)
 {
-       this->reqid = reqid;
+       this->child.reqid = reqid;
 }
 
 METHOD(child_create_t, use_marks, void,
        private_child_create_t *this, uint32_t in, uint32_t out)
 {
-       this->mark_in = in;
-       this->mark_out = out;
+       this->child.mark_in = in;
+       this->child.mark_out = out;
 }
 
 METHOD(child_create_t, use_if_ids, void,
        private_child_create_t *this, uint32_t in, uint32_t out)
 {
-       this->if_id_in = in;
-       this->if_id_out = out;
+       this->child.if_id_in = in;
+       this->child.if_id_out = out;
 }
 
 METHOD(child_create_t, use_dh_group, void,
@@ -1762,12 +1749,8 @@ METHOD(task_t, migrate, void,
        this->ipcomp = IPCOMP_NONE;
        this->ipcomp_received = IPCOMP_NONE;
        this->other_cpi = 0;
-       this->reqid = 0;
-       this->mark_in = 0;
-       this->mark_out = 0;
-       this->if_id_in = 0;
-       this->if_id_out = 0;
        this->established = FALSE;
+       this->child = (child_sa_create_t){};
 }
 
 METHOD(task_t, destroy, void,
index 7acbb28..2bc531b 100644 (file)
@@ -293,7 +293,14 @@ METHOD(trap_manager_t, install, bool,
        this->lock->unlock(this->lock);
 
        /* create and route CHILD_SA */
-       child_sa = child_sa_create(me, other, child, 0, FALSE, 0, 0, 0, 0);
+       child_sa_create_t child_data = {
+               /* TODO: no reason to allocate unique interface IDs, there is currently
+                * no event to use them upon trap installation and we'd also have to
+                * pass them in a later initiate() call */
+               .if_id_in_def = peer->get_if_id(peer, TRUE),
+               .if_id_out_def = peer->get_if_id(peer, FALSE),
+       };
+       child_sa = child_sa_create(me, other, child, &child_data);
 
        list = linked_list_create_with_items(me, NULL);
        my_ts = child->get_traffic_selectors(child, TRUE, NULL, list, FALSE);