From c98e48cf0ecc9563166f2e7d009462c01fc3fb6e Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 18 Aug 2016 16:11:34 +0200 Subject: [PATCH 1/4] child-cfg: Add setting that controls whether outbound FWD policies are installed --- src/libcharon/config/child_cfg.c | 14 ++++++++++++++ src/libcharon/config/child_cfg.h | 10 ++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/libcharon/config/child_cfg.c b/src/libcharon/config/child_cfg.c index 76d7f2c58..6a9c342f4 100644 --- a/src/libcharon/config/child_cfg.c +++ b/src/libcharon/config/child_cfg.c @@ -153,6 +153,11 @@ struct private_child_cfg_t { */ bool install_policy; + /** + * Install outbound FWD policies + */ + bool fwd_out_policy; + /** * anti-replay window size */ @@ -564,6 +569,12 @@ METHOD(child_cfg_t, install_policy, bool, return this->install_policy; } +METHOD(child_cfg_t, install_fwd_out_policy, bool, + private_child_cfg_t *this) +{ + return this->fwd_out_policy; +} + #define LT_PART_EQUALS(a, b) ({ a.life == b.life && a.rekey == b.rekey && a.jitter == b.jitter; }) #define LIFETIME_EQUALS(a, b) ({ LT_PART_EQUALS(a.time, b.time) && LT_PART_EQUALS(a.bytes, b.bytes) && LT_PART_EQUALS(a.packets, b.packets); }) @@ -613,6 +624,7 @@ METHOD(child_cfg_t, equals, bool, this->replay_window == other->replay_window && this->proxy_mode == other->proxy_mode && this->install_policy == other->install_policy && + this->fwd_out_policy == other->fwd_out_policy && streq(this->updown, other->updown) && streq(this->interface, other->interface); } @@ -673,6 +685,7 @@ child_cfg_t *child_cfg_create(char *name, child_cfg_create_t *data) .set_replay_window = _set_replay_window, .use_proxy_mode = _use_proxy_mode, .install_policy = _install_policy, + .install_fwd_out_policy = _install_fwd_out_policy, .equals = _equals, .get_ref = _get_ref, .destroy = _destroy, @@ -695,6 +708,7 @@ child_cfg_t *child_cfg_create(char *name, child_cfg_create_t *data) .manual_prio = data->priority, .interface = strdupnull(data->interface), .install_policy = !data->suppress_policies, + .fwd_out_policy = data->fwd_out_policies, .refcount = 1, .proposals = linked_list_create(), .my_ts = linked_list_create(), diff --git a/src/libcharon/config/child_cfg.h b/src/libcharon/config/child_cfg.h index e736b2737..b85bfd9bc 100644 --- a/src/libcharon/config/child_cfg.h +++ b/src/libcharon/config/child_cfg.h @@ -283,6 +283,14 @@ struct child_cfg_t { */ bool (*install_policy)(child_cfg_t *this); + /** + * Check whether outbound FWD IPsec policies should be installed. + * + * @return TRUE, if outbound FWD policies should be installed + * FALSE, otherwise + */ + bool (*install_fwd_out_policy)(child_cfg_t *this); + /** * Check if two child_cfg objects are equal. * @@ -346,6 +354,8 @@ struct child_cfg_create_t { bool hostaccess; /** Don't install IPsec policies */ bool suppress_policies; + /** Install outbound FWD IPsec policies to bypass drop policies */ + bool fwd_out_policies; }; /** From 50721a61d8cd5d6cad4f8cc308b51fa299808243 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 18 Aug 2016 16:22:51 +0200 Subject: [PATCH 2/4] vici: Make installation of outbound FWD policies configurable --- src/libcharon/plugins/vici/vici_config.c | 54 +++++++++++++----------- src/swanctl/swanctl.opt | 7 +++ 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c index daefcaae5..224a51923 100644 --- a/src/libcharon/plugins/vici/vici_config.c +++ b/src/libcharon/plugins/vici/vici_config.c @@ -437,6 +437,7 @@ typedef struct { linked_list_t *remote_ts; uint32_t replay_window; bool policies; + bool policies_fwd_out; child_cfg_create_t cfg; } child_data_t; @@ -462,6 +463,7 @@ static void log_child_data(child_data_t *data, char *name) DBG2(DBG_CFG, " ipcomp = %u", cfg->ipcomp); DBG2(DBG_CFG, " mode = %N", ipsec_mode_names, cfg->mode); DBG2(DBG_CFG, " policies = %u", data->policies); + DBG2(DBG_CFG, " policies_fwd_out = %u", data->policies_fwd_out); if (data->replay_window != REPLAY_UNDEFINED) { DBG2(DBG_CFG, " replay_window = %u", data->replay_window); @@ -1330,31 +1332,32 @@ CALLBACK(child_kv, bool, child_data_t *child, vici_message_t *message, char *name, chunk_t value) { parse_rule_t rules[] = { - { "updown", parse_string, &child->cfg.updown }, - { "hostaccess", parse_bool, &child->cfg.hostaccess }, - { "mode", parse_mode, &child->cfg.mode }, - { "policies", parse_bool, &child->policies }, - { "replay_window", parse_uint32, &child->replay_window }, - { "rekey_time", parse_time, &child->cfg.lifetime.time.rekey }, - { "life_time", parse_time, &child->cfg.lifetime.time.life }, - { "rand_time", parse_time, &child->cfg.lifetime.time.jitter }, - { "rekey_bytes", parse_bytes, &child->cfg.lifetime.bytes.rekey }, - { "life_bytes", parse_bytes, &child->cfg.lifetime.bytes.life }, - { "rand_bytes", parse_bytes, &child->cfg.lifetime.bytes.jitter }, - { "rekey_packets", parse_uint64, &child->cfg.lifetime.packets.rekey }, - { "life_packets", parse_uint64, &child->cfg.lifetime.packets.life }, - { "rand_packets", parse_uint64, &child->cfg.lifetime.packets.jitter }, - { "dpd_action", parse_action, &child->cfg.dpd_action }, - { "start_action", parse_action, &child->cfg.start_action }, - { "close_action", parse_action, &child->cfg.close_action }, - { "ipcomp", parse_bool, &child->cfg.ipcomp }, - { "inactivity", parse_time, &child->cfg.inactivity }, - { "reqid", parse_uint32, &child->cfg.reqid }, - { "mark_in", parse_mark, &child->cfg.mark_in }, - { "mark_out", parse_mark, &child->cfg.mark_out }, - { "tfc_padding", parse_tfc, &child->cfg.tfc }, - { "priority", parse_uint32, &child->cfg.priority }, - { "interface", parse_string, &child->cfg.interface }, + { "updown", parse_string, &child->cfg.updown }, + { "hostaccess", parse_bool, &child->cfg.hostaccess }, + { "mode", parse_mode, &child->cfg.mode }, + { "policies", parse_bool, &child->policies }, + { "policies_fwd_out", parse_bool, &child->policies_fwd_out }, + { "replay_window", parse_uint32, &child->replay_window }, + { "rekey_time", parse_time, &child->cfg.lifetime.time.rekey }, + { "life_time", parse_time, &child->cfg.lifetime.time.life }, + { "rand_time", parse_time, &child->cfg.lifetime.time.jitter }, + { "rekey_bytes", parse_bytes, &child->cfg.lifetime.bytes.rekey }, + { "life_bytes", parse_bytes, &child->cfg.lifetime.bytes.life }, + { "rand_bytes", parse_bytes, &child->cfg.lifetime.bytes.jitter }, + { "rekey_packets", parse_uint64, &child->cfg.lifetime.packets.rekey }, + { "life_packets", parse_uint64, &child->cfg.lifetime.packets.life }, + { "rand_packets", parse_uint64, &child->cfg.lifetime.packets.jitter }, + { "dpd_action", parse_action, &child->cfg.dpd_action }, + { "start_action", parse_action, &child->cfg.start_action }, + { "close_action", parse_action, &child->cfg.close_action }, + { "ipcomp", parse_bool, &child->cfg.ipcomp }, + { "inactivity", parse_time, &child->cfg.inactivity }, + { "reqid", parse_uint32, &child->cfg.reqid }, + { "mark_in", parse_mark, &child->cfg.mark_in }, + { "mark_out", parse_mark, &child->cfg.mark_out }, + { "tfc_padding", parse_tfc, &child->cfg.tfc }, + { "priority", parse_uint32, &child->cfg.priority }, + { "interface", parse_string, &child->cfg.interface }, }; return parse_rules(rules, countof(rules), name, value, @@ -1537,6 +1540,7 @@ CALLBACK(children_sn, bool, } } child.cfg.suppress_policies = !child.policies; + child.cfg.fwd_out_policies = child.policies_fwd_out; check_lifetimes(&child.cfg.lifetime); diff --git a/src/swanctl/swanctl.opt b/src/swanctl/swanctl.opt index 15cbc6cfc..2a4f5a789 100644 --- a/src/swanctl/swanctl.opt +++ b/src/swanctl/swanctl.opt @@ -659,6 +659,13 @@ connections..children..policies = yes Whether to install IPsec policies or not. Disabling this can be useful in some scenarios e.g. MIPv6, where policies are not managed by the IKE daemon. +connections..children..policies_fwd_out = no + Whether to install outbound FWD IPsec policies or not. + + Whether to install outbound FWD IPsec policies or not. Enabling this is + required in case there is a drop policy that would match and block forwarded + traffic for this CHILD_SA. + connections..children..dpd_action = clear Action to perform on DPD timeout (_clear_, _trap_ or _restart_). From 62636da4f3bb8bf237ab6e1531629a450fd48f7b Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 18 Aug 2016 17:09:15 +0200 Subject: [PATCH 3/4] testing: Enable outbound FWD policies in swanctl/manual-prio scenario --- .../manual-prio/hosts/moon/etc/swanctl/swanctl.conf | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/testing/tests/swanctl/manual-prio/hosts/moon/etc/swanctl/swanctl.conf b/testing/tests/swanctl/manual-prio/hosts/moon/etc/swanctl/swanctl.conf index 53883f79d..5fefdcdd2 100755 --- a/testing/tests/swanctl/manual-prio/hosts/moon/etc/swanctl/swanctl.conf +++ b/testing/tests/swanctl/manual-prio/hosts/moon/etc/swanctl/swanctl.conf @@ -16,7 +16,8 @@ connections { local_ts = 10.1.0.0/16 priority = 2 interface = eth0 - + policies_fwd_out = yes + esp_proposals = aes128gcm128-modp3072 } } @@ -32,15 +33,15 @@ connections { local_ts = 0.0.0.0/0 remote_ts = 0.0.0.0/0 interface = eth0 - priority = 4 + priority = 4 - mode = drop + mode = drop start_action = trap } pass-ssh-in { local_ts = 0.0.0.0/0[tcp/ssh] remote_ts = 0.0.0.0/0[tcp] - priority = 1 + priority = 1 mode = pass start_action = trap @@ -61,6 +62,6 @@ connections { mode = pass start_action = trap } - } + } } } From ad1b53454e782e6f310fd3dba25936ffc22ed321 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 18 Aug 2016 15:09:08 +0200 Subject: [PATCH 4/4] child-sa: Only install outbound FWD policies if explicitly configured They are only required if drop policies would otherwise prevent forwarding traffic. This reduces the number of policies and avoids conflicts e.g. with SPD hash thresholds. --- src/libcharon/sa/child_sa.c | 41 ++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 8a405d93c..e4364de12 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -111,11 +111,16 @@ struct private_child_sa_t { */ bool static_reqid; - /* + /** * Unique CHILD_SA identifier */ uint32_t unique_id; + /** + * Whether FWD policieis in the outbound direction should be installed + */ + bool policies_fwd_out; + /** * inbound mark used for this child_sa */ @@ -931,15 +936,19 @@ static status_t install_policies_internal(private_child_sa_t *this, * policies of two SAs we install them with reduced priority. As they * basically act as bypass policies for drop policies we use a higher * priority than is used for them. */ - out_id.dir = POLICY_FWD; - other_sa->reqid = 0; - if (priority == POLICY_PRIORITY_DEFAULT) + if (this->policies_fwd_out) { - out_policy.prio = POLICY_PRIORITY_ROUTED; + out_id.dir = POLICY_FWD; + other_sa->reqid = 0; + if (priority == POLICY_PRIORITY_DEFAULT) + { + out_policy.prio = POLICY_PRIORITY_ROUTED; + } + status |= charon->kernel->add_policy(charon->kernel, &out_id, + &out_policy); + /* reset the reqid for any other further policies */ + other_sa->reqid = this->reqid; } - status |= charon->kernel->add_policy(charon->kernel, &out_id, &out_policy); - /* reset the reqid for any other further policies */ - other_sa->reqid = this->reqid; } return status; } @@ -988,14 +997,17 @@ static void del_policies_internal(private_child_sa_t *this, in_id.dir = POLICY_FWD; charon->kernel->del_policy(charon->kernel, &in_id, &in_policy); - out_id.dir = POLICY_FWD; - other_sa->reqid = 0; - if (priority == POLICY_PRIORITY_DEFAULT) + if (this->policies_fwd_out) { - out_policy.prio = POLICY_PRIORITY_ROUTED; + out_id.dir = POLICY_FWD; + other_sa->reqid = 0; + if (priority == POLICY_PRIORITY_DEFAULT) + { + out_policy.prio = POLICY_PRIORITY_ROUTED; + } + charon->kernel->del_policy(charon->kernel, &out_id, &out_policy); + other_sa->reqid = this->reqid; } - charon->kernel->del_policy(charon->kernel, &out_id, &out_policy); - other_sa->reqid = this->reqid; } } @@ -1443,6 +1455,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, .mark_in = config->get_mark(config, TRUE), .mark_out = config->get_mark(config, FALSE), .install_time = time_monotonic(NULL), + .policies_fwd_out = config->install_fwd_out_policy(config), ); this->config = config;