ipsec-types: Restrict the use of %unique and other keywords when parsing marks

%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.

diff --git a/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c b/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c
index 3ef3dc712fa04f1667e3796705a9e30730203e20..c9a76ba01af46deb864ebd3c66864e8bd2178bdf 100644 (file)
--- a/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c
+++ b/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c
@@ -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));

diff --git a/src/libcharon/plugins/socket_default/socket_default_socket.c b/src/libcharon/plugins/socket_default/socket_default_socket.c
index 57e092968b608c2e1d8b9b8176b919a1c6229e5d..68e5a7a0e9090881daa2499e13ef3e6846fef544 100644 (file)
--- a/src/libcharon/plugins/socket_default/socket_default_socket.c
+++ b/src/libcharon/plugins/socket_default/socket_default_socket.c
@@ -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)

diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c
index 05fa8c5ca92db6dcfa781b645ee5f7914b12dcf6..98f68264c29337819a78c8b8f27b1aff81235635 100644 (file)
--- a/src/libcharon/plugins/vici/vici_config.c
+++ b/src/libcharon/plugins/vici/vici_config.c
@@ -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);
 }
 
 /**

diff --git a/src/libstrongswan/ipsec/ipsec_types.c b/src/libstrongswan/ipsec/ipsec_types.c
index d231bb3a450b2a9400a5e5569359e2284d0c565d..7eed1566a88d7c00b59786a857bfecd2c146615f 100644 (file)
--- a/src/libstrongswan/ipsec/ipsec_types.c
+++ b/src/libstrongswan/ipsec/ipsec_types.c
@@ -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"))
                {

diff --git a/src/libstrongswan/ipsec/ipsec_types.h b/src/libstrongswan/ipsec/ipsec_types.h
index bd5545e221bc07a47db73aa879882b360baa25ee..5b1735178a984e873e1a672bf4b133ef7b0f03b2 100644 (file)
--- a/src/libstrongswan/ipsec/ipsec_types.h
+++ b/src/libstrongswan/ipsec/ipsec_types.h
@@ -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;
@@ -216,13 +217,24 @@ struct mark_t {
 #define MARK_UNIQUE_DIR (0xFFFFFFFE)
 #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_ @}*/

diff --git a/src/libstrongswan/tests/suites/test_utils.c b/src/libstrongswan/tests/suites/test_utils.c
index 00f000a6a0b79b31d7f1e1c462ff86bb8487ba30..5a854f3f675fbc54288623ad3a8bf75c352a6e6d 100644 (file)
--- a/src/libstrongswan/tests/suites/test_utils.c
+++ b/src/libstrongswan/tests/suites/test_utils.c
@@ -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);

diff --git a/src/starter/confread.c b/src/starter/confread.c
index 345d0b60bfe65b915afcfc9867daaaa075a09aae..407ef5e13bc25e8186d24d4bc3b434944456596e 100644 (file)
--- a/src/starter/confread.c
+++ b/src/starter/confread.c
@@ -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++;
                        }