From 32e5c49234ce4af2ef375e3f1750fdb90f813905 Mon Sep 17 00:00:00 2001 From: Eyal Birger Date: Fri, 28 Jul 2017 12:18:52 +0300 Subject: [PATCH] child-sa: Allow requesting different unique marks for in/out When requiring unique flags for CHILD_SAs, allow the configuration to request different marks for each direction by using the %unique-dir keyword. This is useful when different marks are desired for each direction but the number of peers is not predefined. An example use case is when implementing a site-to-site route-based VPN without VTI devices. A use of 0.0.0.0/0 - 0.0.0.0/0 traffic selectors with identical in/out marks results in outbound traffic being wrongfully matched against the 'fwd' policy - for which the underlay 'template' does not match - and dropped. Using different marks for each direction avoids this issue as the 'fwd' policy uses the 'in' mark will not match outbound traffic. Closes strongswan/strongswan#78. --- man/ipsec.conf.5.in | 5 ++++- src/libcharon/sa/child_sa.c | 29 +++++++++++++++++++++------ src/libstrongswan/ipsec/ipsec_types.c | 15 +++++++++++++- src/libstrongswan/ipsec/ipsec_types.h | 4 +++- src/swanctl/swanctl.opt | 8 ++++++-- 5 files changed, 50 insertions(+), 11 deletions(-) diff --git a/man/ipsec.conf.5.in b/man/ipsec.conf.5.in index fef44ae21..69aeba8cb 100644 --- a/man/ipsec.conf.5.in +++ b/man/ipsec.conf.5.in @@ -1037,7 +1037,10 @@ mask of .B 0xffffffff is assumed. The special value .B %unique -assigns a unique value to each newly created IPsec SA. +assigns a unique value to each newly created IPsec SA. To additionally +make the mark unique for each IPsec SA direction (in/out) the special value +.B %unique-dir +may be used. .TP .BR mark_in " = [/]" sets an XFRM mark in the inbound IPsec SA and diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 422526774..4133d9182 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -1745,7 +1745,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, { private_child_sa_t *this; static refcount_t unique_id = 0, unique_mark = 0; - refcount_t mark; + refcount_t mark = 0; INIT(this, .public = { @@ -1818,16 +1818,33 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, { this->mark_out.value = mark_out; } - if (this->mark_in.value == MARK_UNIQUE || - this->mark_out.value == MARK_UNIQUE) + + if (MARK_IS_UNIQUE(this->mark_in.value) || + MARK_IS_UNIQUE(this->mark_out.value)) { - mark = ref_get(&unique_mark); - if (this->mark_in.value == MARK_UNIQUE) + bool unique_dir; + + unique_dir = this->mark_in.value == MARK_UNIQUE_DIR || + this->mark_out.value == MARK_UNIQUE_DIR; + + if (!unique_dir) { + mark = ref_get(&unique_mark); + } + if (MARK_IS_UNIQUE(this->mark_in.value)) + { + if (unique_dir) + { + mark = ref_get(&unique_mark); + } this->mark_in.value = mark; } - if (this->mark_out.value == MARK_UNIQUE) + if (MARK_IS_UNIQUE(this->mark_out.value)) { + if (unique_dir) + { + mark = ref_get(&unique_mark); + } this->mark_out.value = mark; } } diff --git a/src/libstrongswan/ipsec/ipsec_types.c b/src/libstrongswan/ipsec/ipsec_types.c index a52a1eb51..68c3935b9 100644 --- a/src/libstrongswan/ipsec/ipsec_types.c +++ b/src/libstrongswan/ipsec/ipsec_types.c @@ -66,8 +66,21 @@ bool mark_from_string(const char *value, mark_t *mark) } if (strcasepfx(value, "%unique")) { - mark->value = MARK_UNIQUE; endptr = (char*)value + strlen("%unique"); + if (strcasepfx(endptr, "-dir")) + { + mark->value = MARK_UNIQUE_DIR; + endptr += strlen("-dir"); + } + else if (!*endptr || *endptr == '/') + { + mark->value = MARK_UNIQUE; + } + else + { + DBG1(DBG_APP, "invalid mark value: %s", value); + return FALSE; + } } else { diff --git a/src/libstrongswan/ipsec/ipsec_types.h b/src/libstrongswan/ipsec/ipsec_types.h index c93d95562..1db78ba6f 100644 --- a/src/libstrongswan/ipsec/ipsec_types.h +++ b/src/libstrongswan/ipsec/ipsec_types.h @@ -178,9 +178,11 @@ struct mark_t { }; /** - * Special mark value that uses a unique mark for each CHILD_SA + * Special mark value that uses a unique mark for each CHILD_SA (and direction) */ #define MARK_UNIQUE (0xFFFFFFFF) +#define MARK_UNIQUE_DIR (0xFFFFFFFE) +#define MARK_IS_UNIQUE(m) ((m) == MARK_UNIQUE || (m) == MARK_UNIQUE_DIR) /** * Try to parse a mark_t from the given string of the form mark[/mask]. diff --git a/src/swanctl/swanctl.opt b/src/swanctl/swanctl.opt index 634271d26..d0a0d21dd 100644 --- a/src/swanctl/swanctl.opt +++ b/src/swanctl/swanctl.opt @@ -870,7 +870,9 @@ connections..children..mark_in = 0/0x00000000 Netfilter mark and mask for input traffic. On Linux Netfilter may require marks on each packet to match an SA having that option set. This allows Netfilter rules to select specific tunnels for incoming traffic. The - special value _%unique_ sets a unique mark on each CHILD_SA instance. + special value _%unique_ sets a unique mark on each CHILD_SA instance, + beyond that the value _%unique-dir_ assigns a different unique mark for each + CHILD_SA direction (in/out). An additional mask may be appended to the mark, separated by _/_. The default mask if omitted is 0xffffffff. @@ -881,7 +883,9 @@ connections..children..mark_out = 0/0x00000000 Netfilter mark and mask for output traffic. On Linux Netfilter may require marks on each packet to match a policy having that option set. This allows Netfilter rules to select specific tunnels for outgoing traffic. The - special value _%unique_ sets a unique mark on each CHILD_SA instance. + special value _%unique_ sets a unique mark on each CHILD_SA instance, + beyond that the value _%unique-dir_ assigns a different unique mark for each + CHILD_SA direction (in/out). An additional mask may be appended to the mark, separated by _/_. The default mask if omitted is 0xffffffff.