ipsec-types: Restrict the use of %unique and other keywords when parsing marks
authorMartin Willi <martin@strongswan.org>
Mon, 14 May 2018 11:42:53 +0000 (13:42 +0200)
committerTobias Brunner <tobias@strongswan.org>
Fri, 31 Aug 2018 10:26:40 +0000 (12:26 +0200)
%unique (and the upcoming %same key) are usable in specific contexts only.
To restrict the user from using it in other places where it does not get the
expected results, reject such keywords unless explicitly allowed.

src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c
src/libcharon/plugins/socket_default/socket_default_socket.c
src/libcharon/plugins/vici/vici_config.c
src/libstrongswan/ipsec/ipsec_types.c
src/libstrongswan/ipsec/ipsec_types.h
src/libstrongswan/tests/suites/test_utils.c
src/starter/confread.c

index 3ef3dc7..c9a76ba 100644 (file)
@@ -2925,7 +2925,7 @@ static status_t manage_rule(private_kernel_netlink_net_t *this, int nlmsg_type,
                        msg->rtm_flags |= FIB_RULE_INVERT;
                        fwmark++;
                }
-               if (mark_from_string(fwmark, &mark))
+               if (mark_from_string(fwmark, MARK_OP_NONE, &mark))
                {
                        chunk = chunk_from_thing(mark.value);
                        netlink_add_attribute(hdr, FRA_FWMARK, chunk, sizeof(request));
index 57e0929..68e5a7a 100644 (file)
@@ -745,7 +745,7 @@ static int open_socket(private_socket_default_socket_t *this,
 
                fwmark = lib->settings->get_str(lib->settings,
                                                        "%s.plugins.socket-default.fwmark", NULL, lib->ns);
-               if (fwmark && mark_from_string(fwmark, &mark))
+               if (fwmark && mark_from_string(fwmark, MARK_OP_NONE, &mark))
                {
                        if (setsockopt(skt, SOL_SOCKET, SO_MARK, &mark.value,
                                                   sizeof(mark.value)) < 0)
index 05fa8c5..98f6826 100644 (file)
@@ -1181,7 +1181,7 @@ CALLBACK(parse_mark, bool,
        {
                return FALSE;
        }
-       return mark_from_string(buf, out);
+       return mark_from_string(buf, MARK_OP_UNIQUE, out);
 }
 
 /**
index d231bb3..7eed156 100644 (file)
@@ -69,7 +69,7 @@ bool ipsec_sa_cfg_equals(ipsec_sa_cfg_t *a, ipsec_sa_cfg_t *b)
 /*
  * See header
  */
-bool mark_from_string(const char *value, mark_t *mark)
+bool mark_from_string(const char *value, mark_op_t ops, mark_t *mark)
 {
        char *endptr;
 
@@ -79,6 +79,11 @@ bool mark_from_string(const char *value, mark_t *mark)
        }
        if (strcasepfx(value, "%unique"))
        {
+               if (!(ops & MARK_OP_UNIQUE))
+               {
+                       DBG1(DBG_APP, "unexpected use of %%unique mark", value);
+                       return FALSE;
+               }
                endptr = (char*)value + strlen("%unique");
                if (strcasepfx(endptr, "-dir"))
                {
index bd5545e..5b17351 100644 (file)
@@ -28,6 +28,7 @@ typedef enum policy_priority_t policy_priority_t;
 typedef enum ipcomp_transform_t ipcomp_transform_t;
 typedef enum hw_offload_t hw_offload_t;
 typedef enum dscp_copy_t dscp_copy_t;
+typedef enum mark_op_t mark_op_t;
 typedef struct ipsec_sa_cfg_t ipsec_sa_cfg_t;
 typedef struct lifetime_cfg_t lifetime_cfg_t;
 typedef struct mark_t mark_t;
@@ -217,12 +218,23 @@ struct mark_t {
 #define MARK_IS_UNIQUE(m) ((m) == MARK_UNIQUE || (m) == MARK_UNIQUE_DIR)
 
 /**
+ * Special mark operations to accept when parsing marks.
+ */
+enum mark_op_t {
+       /** none of the following */
+       MARK_OP_NONE = 0,
+       /** %unique and %unique-dir */
+       MARK_OP_UNIQUE = (1<<0),
+};
+
+/**
  * Try to parse a mark_t from the given string of the form mark[/mask].
  *
  * @param value                string to parse
+ * @param ops          operations to accept
  * @param mark         mark to fill
  * @return                     TRUE if parsing was successful
  */
-bool mark_from_string(const char *value, mark_t *mark);
+bool mark_from_string(const char *value, mark_op_t ops, mark_t *mark);
 
 #endif /** IPSEC_TYPES_H_ @}*/
index 00f000a..5a854f3 100644 (file)
@@ -860,47 +860,69 @@ END_TEST
 static struct {
        char *s;
        bool ok;
+       mark_op_t ops;
        mark_t m;
 } mark_data[] = {
-       {NULL,                  FALSE, { 0 }},
-       {"",                    TRUE,  { 0, 0xffffffff }},
-       {"/",                   TRUE,  { 0, 0 }},
-       {"42",                  TRUE,  { 42, 0xffffffff }},
-       {"0x42",                TRUE,  { 0x42, 0xffffffff }},
-       {"x",                   FALSE, { 0 }},
-       {"42/",                 TRUE,  { 0, 0 }},
-       {"42/0",                TRUE,  { 0, 0 }},
-       {"42/x",                FALSE, { 0 }},
-       {"42/42",               TRUE,  { 42, 42 }},
-       {"42/0xff",             TRUE,  { 42, 0xff }},
-       {"0x42/0xff",   TRUE,  { 0x42, 0xff }},
-       {"/0xff",               TRUE,  { 0, 0xff }},
-       {"/x",                  FALSE, { 0 }},
-       {"x/x",                 FALSE, { 0 }},
-       {"0xfffffff0/0x0000ffff",       TRUE,  { 0x0000fff0, 0x0000ffff }},
-       {"%unique",                                     TRUE,  { MARK_UNIQUE, 0xffffffff }},
-       {"%unique/",                            TRUE,  { MARK_UNIQUE, 0 }},
-       {"%unique/0x0000ffff",          TRUE,  { MARK_UNIQUE, 0x0000ffff }},
-       {"%unique/0xffffffff",          TRUE,  { MARK_UNIQUE, 0xffffffff }},
-       {"%unique0xffffffffff",         FALSE, { 0, 0 }},
-       {"0xffffffff/0x0000ffff",       TRUE,  { MARK_UNIQUE, 0x0000ffff }},
-       {"0xffffffff/0xffffffff",       TRUE,  { MARK_UNIQUE, 0xffffffff }},
-       {"%unique-dir",                         TRUE,  { MARK_UNIQUE_DIR, 0xffffffff }},
-       {"%unique-dir/",                        TRUE,  { MARK_UNIQUE_DIR, 0 }},
-       {"%unique-dir/0x0000ffff",      TRUE,  { MARK_UNIQUE_DIR, 0x0000ffff }},
-       {"%unique-dir/0xffffffff",      TRUE,  { MARK_UNIQUE_DIR, 0xffffffff }},
-       {"%unique-dir0xffffffff",       FALSE, { 0, 0 }},
-       {"0xfffffffe/0x0000ffff",       TRUE,  { MARK_UNIQUE_DIR, 0x0000ffff }},
-       {"0xfffffffe/0xffffffff",       TRUE,  { MARK_UNIQUE_DIR, 0xffffffff }},
-       {"%unique-/0xffffffff",         FALSE, { 0, 0 }},
-       {"%unique-foo/0xffffffff",      FALSE, { 0, 0 }},
+       {NULL,                  FALSE,  MARK_OP_NONE, { 0 }},
+       {"",                    TRUE,   MARK_OP_NONE, { 0, 0xffffffff }},
+       {"/",                   TRUE,   MARK_OP_NONE, { 0, 0 }},
+       {"42",                  TRUE,   MARK_OP_NONE, { 42, 0xffffffff }},
+       {"0x42",                TRUE,   MARK_OP_NONE, { 0x42, 0xffffffff }},
+       {"x",                   FALSE,  MARK_OP_NONE, { 0 }},
+       {"42/",                 TRUE,   MARK_OP_NONE, { 0, 0 }},
+       {"42/0",                TRUE,   MARK_OP_NONE, { 0, 0 }},
+       {"42/x",                FALSE,  MARK_OP_NONE, { 0 }},
+       {"42/42",               TRUE,   MARK_OP_NONE, { 42, 42 }},
+       {"42/0xff",             TRUE,   MARK_OP_NONE, { 42, 0xff }},
+       {"0x42/0xff",   TRUE,   MARK_OP_NONE, { 0x42, 0xff }},
+       {"/0xff",               TRUE,   MARK_OP_NONE, { 0, 0xff }},
+       {"/x",                  FALSE,  MARK_OP_NONE, { 0 }},
+       {"x/x",                 FALSE,  MARK_OP_NONE, { 0 }},
+       {"0xfffffff0/0x0000ffff",       TRUE,   MARK_OP_UNIQUE,
+               { 0x0000fff0, 0x0000ffff }},
+       {"%unique",                                     TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE, 0xffffffff }},
+       {"%unique/",                            TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE, 0 }},
+       {"%unique",                                     FALSE,  MARK_OP_NONE,
+               { 0, 0 }},
+       {"%unique/0x0000ffff",          TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE, 0x0000ffff }},
+       {"%unique/0xffffffff",          TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE, 0xffffffff }},
+       {"%unique0xffffffffff",         FALSE,  MARK_OP_UNIQUE,
+               { 0, 0 }},
+       {"0xffffffff/0x0000ffff",       TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE, 0x0000ffff }},
+       {"0xffffffff/0xffffffff",       TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE, 0xffffffff }},
+       {"%unique-dir",                         TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE_DIR, 0xffffffff }},
+       {"%unique-dir/",                        TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE_DIR, 0 }},
+       {"%unique-dir",                         FALSE,  MARK_OP_NONE,
+               { 0, 0 }},
+       {"%unique-dir/0x0000ffff",      TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE_DIR, 0x0000ffff }},
+       {"%unique-dir/0xffffffff",      TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE_DIR, 0xffffffff }},
+       {"%unique-dir0xffffffff",       FALSE,  MARK_OP_UNIQUE,
+               { 0, 0 }},
+       {"0xfffffffe/0x0000ffff",       TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE_DIR, 0x0000ffff }},
+       {"0xfffffffe/0xffffffff",       TRUE,   MARK_OP_UNIQUE,
+               { MARK_UNIQUE_DIR, 0xffffffff }},
+       {"%unique-/0xffffffff",         FALSE,  MARK_OP_UNIQUE,
+               { 0, 0 }},
+       {"%unique-foo/0xffffffff",      FALSE,  MARK_OP_UNIQUE,
+               { 0, 0 }},
 };
 
 START_TEST(test_mark_from_string)
 {
        mark_t mark;
 
-       if (mark_from_string(mark_data[_i].s, &mark))
+       if (mark_from_string(mark_data[_i].s, mark_data[_i].ops, &mark))
        {
                ck_assert_int_eq(mark.value, mark_data[_i].m.value);
                ck_assert_int_eq(mark.mask, mark_data[_i].m.mask);
index 345d0b6..407ef5e 100644 (file)
@@ -444,7 +444,7 @@ static void handle_keyword(kw_token_t token, starter_conn_t *conn, char *key,
                        KW_SA_OPTION_FLAG("yes", "no", SA_OPTION_COMPRESS)
                        break;
                case KW_MARK:
-                       if (!mark_from_string(value, &conn->mark_in))
+                       if (!mark_from_string(value, MARK_OP_UNIQUE, &conn->mark_in))
                        {
                                cfg->err++;
                                break;
@@ -452,13 +452,13 @@ static void handle_keyword(kw_token_t token, starter_conn_t *conn, char *key,
                        conn->mark_out = conn->mark_in;
                        break;
                case KW_MARK_IN:
-                       if (!mark_from_string(value, &conn->mark_in))
+                       if (!mark_from_string(value, MARK_OP_UNIQUE, &conn->mark_in))
                        {
                                cfg->err++;
                        }
                        break;
                case KW_MARK_OUT:
-                       if (!mark_from_string(value, &conn->mark_out))
+                       if (!mark_from_string(value, MARK_OP_UNIQUE, &conn->mark_out))
                        {
                                cfg->err++;
                        }