From fafa76984d62b6126ba61b2be2383bcb0225ea10 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 22 Mar 2019 17:39:47 +0100 Subject: [PATCH] child-sa: Pass default interface ID inherited from IKE_SA Also pass optional arguments as struct. --- src/libcharon/plugins/ha/ha_dispatcher.c | 7 +- src/libcharon/sa/child_sa.c | 34 ++++---- src/libcharon/sa/child_sa.h | 38 ++++++--- src/libcharon/sa/ikev1/tasks/quick_mode.c | 95 ++++++++------------- src/libcharon/sa/ikev2/tasks/child_create.c | 65 ++++++-------- src/libcharon/sa/trap_manager.c | 9 +- 6 files changed, 117 insertions(+), 131 deletions(-) diff --git a/src/libcharon/plugins/ha/ha_dispatcher.c b/src/libcharon/plugins/ha/ha_dispatcher.c index 11f3bd914c..ff75cb5c1a 100644 --- a/src/libcharon/plugins/ha/ha_dispatcher.c +++ b/src/libcharon/plugins/ha/ha_dispatcher.c @@ -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); diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 40137d3c20..fc60b413f1 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -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 { diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index 483dd1512f..c9b3f63e2b 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -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 #include @@ -512,23 +513,40 @@ struct child_sa_t { void (*destroy) (child_sa_t *this); }; +/** + * 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_ @}*/ diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index 59f049dbdd..3309a5ddc1 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c @@ -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 @@ -195,11 +175,6 @@ struct private_quick_mode_t { */ protocol_id_t proto; - /** - * Use UDP encapsulation - */ - bool udp; - /** * Message ID of handled quick mode exchange */ @@ -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) { diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 340542b19c..d74013b8ff 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -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, diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c index 7acbb28c21..2bc531b387 100644 --- a/src/libcharon/sa/trap_manager.c +++ b/src/libcharon/sa/trap_manager.c @@ -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); -- 2.47.2