From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arturo Borrero Gonzalez Subject: [libnftables PATCH v2] ct: fix key and dir requirements Date: Wed, 15 Jan 2014 19:20:22 +0100 Message-ID: <20140115181939.25239.19905.stgit@nfdev.cica.es> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: pablo@netfilter.org To: netfilter-devel@vger.kernel.org Return-path: Received: from smtp3.cica.es ([150.214.5.190]:58573 "EHLO smtp.cica.es" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752127AbaAOSUd (ORCPT ); Wed, 15 Jan 2014 13:20:33 -0500 Sender: netfilter-devel-owner@vger.kernel.org List-ID: Follow linux/net/netfilter/nft_ct.c to adjust key and dir attributes. The dir attribute is needed only when using certaing keys, and prohibited with others. Key is always mandatory. Previous to this patch, using XML/JSON to manage this expr led to some undefined and erroneous behaviours. While at it, update tests files in order to pass nft-parsing-test. Signed-off-by: Arturo Borrero Gonzalez --- v2: fixed wrong logic in the XML parser. Added support for NFT_CT_L3PROTOCOL src/expr/ct.c | 97 +++++++++++++++++++++++++++------------ tests/xmlfiles/24-rule-ct.xml | 2 - tests/xmlfiles/37-rule-real.xml | 2 - tests/xmlfiles/39-rule-real.xml | 2 - tests/xmlfiles/50-rule-real.xml | 2 - tests/xmlfiles/51-rule-real.xml | 2 - tests/xmlfiles/52-rule-real.xml | 2 - tests/xmlfiles/53-rule-real.xml | 2 - tests/xmlfiles/54-rule-real.xml | 2 - tests/xmlfiles/55-rule-real.xml | 2 - tests/xmlfiles/56-rule-real.xml | 2 - tests/xmlfiles/57-rule-real.xml | 2 - 12 files changed, 78 insertions(+), 41 deletions(-) diff --git a/src/expr/ct.c b/src/expr/ct.c index e960134..e74c36c 100644 --- a/src/expr/ct.c +++ b/src/expr/ct.c @@ -179,6 +179,28 @@ static inline int str2ctkey(const char *ctkey) return -1; } +static bool ctkey_req_dir(int ctkey) +{ + switch (ctkey) { + case NFT_CT_STATE: + case NFT_CT_DIRECTION: + case NFT_CT_STATUS: + case NFT_CT_MARK: + case NFT_CT_SECMARK: + case NFT_CT_EXPIRATION: + case NFT_CT_HELPER: + return false; + case NFT_CT_L3PROTOCOL: + case NFT_CT_SRC: + case NFT_CT_DST: + case NFT_CT_PROTOCOL: + case NFT_CT_PROTO_SRC: + case NFT_CT_PROTO_DST: + default: + return true; + } +} + static int nft_rule_expr_ct_json_parse(struct nft_rule_expr *e, json_t *root, struct nft_parse_err *err) { @@ -193,22 +215,19 @@ static int nft_rule_expr_ct_json_parse(struct nft_rule_expr *e, json_t *root, nft_rule_expr_set_u32(e, NFT_EXPR_CT_DREG, reg); - if (nft_jansson_node_exist(root, "key")) { - key_str = nft_jansson_parse_str(root, "key", err); - if (key_str == NULL) - return -1; - - key = str2ctkey(key_str); - if (key < 0) - goto err; + key_str = nft_jansson_parse_str(root, "key", err); + if (key_str == NULL) + return -1; - nft_rule_expr_set_u32(e, NFT_EXPR_CT_KEY, key); + key = str2ctkey(key_str); + if (key < 0) + goto err; - } + nft_rule_expr_set_u32(e, NFT_EXPR_CT_KEY, key); - if (nft_jansson_node_exist(root, "dir")) { - if (nft_jansson_parse_val(root, "dir", NFT_TYPE_U8, &dir, - err) < 0) + if (ctkey_req_dir(key)) { + if (nft_jansson_parse_val(root, "dir", NFT_TYPE_U8, + &dir, err) < 0) return -1; if (dir != IP_CT_DIR_ORIGINAL && dir != IP_CT_DIR_REPLY) @@ -257,15 +276,18 @@ static int nft_rule_expr_ct_xml_parse(struct nft_rule_expr *e, mxml_node_t *tree ct->key = key; e->flags |= (1 << NFT_EXPR_CT_KEY); - if (nft_mxml_num_parse(tree, "dir", MXML_DESCEND_FIRST, BASE_DEC, - &dir, NFT_TYPE_U8, NFT_XML_MAND, err) != 0) - return -1; + if (ctkey_req_dir(key)) { + if (nft_mxml_num_parse(tree, "dir", MXML_DESCEND_FIRST, + BASE_DEC, &dir, NFT_TYPE_U8, + NFT_XML_MAND, err) != 0) + return -1; - if (dir != IP_CT_DIR_ORIGINAL && dir != IP_CT_DIR_REPLY) - goto err; + if (dir != IP_CT_DIR_ORIGINAL && dir != IP_CT_DIR_REPLY) + goto err; - ct->dir = dir; - e->flags |= (1 << NFT_EXPR_CT_DIR); + ct->dir = dir; + e->flags |= (1 << NFT_EXPR_CT_DIR); + } return 0; err: @@ -286,19 +308,37 @@ nft_expr_ct_snprintf_json(char *buf, size_t size, struct nft_rule_expr *e) ret = snprintf(buf, len, "\"dreg\":%u", ct->dreg); SNPRINTF_BUFFER_SIZE(ret, size, len, offset); - if (e->flags & (1 << NFT_EXPR_CT_KEY)) { - ret = snprintf(buf+offset, len, ",\"key\":\"%s\"", - ctkey2str(ct->key)); - SNPRINTF_BUFFER_SIZE(ret, size, len, offset); - } + ret = snprintf(buf+offset, len, ",\"key\":\"%s\"", + ctkey2str(ct->key)); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); - if (e->flags & (1 << NFT_EXPR_CT_DIR)) { + if (ctkey_req_dir(ct->key) && (e->flags & (1 << NFT_EXPR_CT_DIR))) { ret = snprintf(buf+offset, len, ",\"dir\":%u", ct->dir); SNPRINTF_BUFFER_SIZE(ret, size, len, offset); } return offset; +} + +static int +nft_expr_ct_snprintf_xml(char *buf, size_t size, struct nft_rule_expr *e) +{ + int ret, len = size, offset = 0; + struct nft_expr_ct *ct = nft_expr_data(e); + + ret = snprintf(buf, len, "%u", ct->dreg); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + + ret = snprintf(buf+offset, len, "%s", + ctkey2str(ct->key)); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + if (ctkey_req_dir(ct->key) && (e->flags & (1 << NFT_EXPR_CT_DIR))) { + ret = snprintf(buf+offset, len, "%u", ct->dir); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + } + + return offset; } static int @@ -312,10 +352,7 @@ nft_rule_expr_ct_snprintf(char *buf, size_t len, uint32_t type, return snprintf(buf, len, "load %s => reg %u dir %u ", ctkey2str(ct->key), ct->dreg, ct->dir); case NFT_OUTPUT_XML: - return snprintf(buf, len, "%u" - "%s" - "%u", - ct->dreg, ctkey2str(ct->key), ct->dir); + return nft_expr_ct_snprintf_xml(buf, len, e); case NFT_OUTPUT_JSON: return nft_expr_ct_snprintf_json(buf, len, e); default: diff --git a/tests/xmlfiles/24-rule-ct.xml b/tests/xmlfiles/24-rule-ct.xml index b3f4ad8..7890da0 100644 --- a/tests/xmlfiles/24-rule-ct.xml +++ b/tests/xmlfiles/24-rule-ct.xml @@ -1 +1 @@ -ipfilter
INPUT1001state0 +ipfilter
INPUT1001state diff --git a/tests/xmlfiles/37-rule-real.xml b/tests/xmlfiles/37-rule-real.xml index 89b06b8..8eca025 100644 --- a/tests/xmlfiles/37-rule-real.xml +++ b/tests/xmlfiles/37-rule-real.xml @@ -1 +1 @@ -ipfilter
INPUT251iifname1eq160x000000000x000000000x650000000x00306874191network1eq10x00000006122transport1eq20x000016001state011440x0000000a40x000000001neq40x0000000000testprefix100 +ipfilter
INPUT251iifname1eq160x000000000x000000000x650000000x00306874191network1eq10x00000006122transport1eq20x000016001state11440x0000000a40x000000001neq40x0000000000testprefix100 diff --git a/tests/xmlfiles/39-rule-real.xml b/tests/xmlfiles/39-rule-real.xml index a307a2e..07e9a84 100644 --- a/tests/xmlfiles/39-rule-real.xml +++ b/tests/xmlfiles/39-rule-real.xml @@ -1 +1 @@ -ip6filter
test311iifname1eq160x000000000x000000000x6f6200000x0030646e1oifname1eq160x000000000x620000000x31646e6f0x0037322e1816network1eq160xc09a002a0x2700cac10x000000000x50010000161network1eq10x00000011122transport1eq20x000035001status01eq40x0000000100dns_drop2000drop +ip6filter
test311iifname1eq160x000000000x000000000x6f6200000x0030646e1oifname1eq160x000000000x620000000x31646e6f0x0037322e1816network1eq160xc09a002a0x2700cac10x000000000x50010000161network1eq10x00000011122transport1eq20x000035001status1eq40x0000000100dns_drop2000drop diff --git a/tests/xmlfiles/50-rule-real.xml b/tests/xmlfiles/50-rule-real.xml index d15eff4..8977c5d 100644 --- a/tests/xmlfiles/50-rule-real.xml +++ b/tests/xmlfiles/50-rule-real.xml @@ -1 +1 @@ -ipfilter
output121state011440x0000000a40x000000001neq40x000000005511407 +ipfilter
output121state11440x0000000a40x000000001neq40x000000005511407 diff --git a/tests/xmlfiles/51-rule-real.xml b/tests/xmlfiles/51-rule-real.xml index 471cd2b..9cc5dae 100644 --- a/tests/xmlfiles/51-rule-real.xml +++ b/tests/xmlfiles/51-rule-real.xml @@ -1 +1 @@ -ipfilter
output131direction01eq10x000000005160 +ipfilter
output131direction1eq10x000000005160 diff --git a/tests/xmlfiles/52-rule-real.xml b/tests/xmlfiles/52-rule-real.xml index 61a1269..55ff00b 100644 --- a/tests/xmlfiles/52-rule-real.xml +++ b/tests/xmlfiles/52-rule-real.xml @@ -1 +1 @@ -ipfilter
output141direction01eq10x000000015011247 +ipfilter
output141direction1eq10x000000015011247 diff --git a/tests/xmlfiles/53-rule-real.xml b/tests/xmlfiles/53-rule-real.xml index d835639..c0c10e9 100644 --- a/tests/xmlfiles/53-rule-real.xml +++ b/tests/xmlfiles/53-rule-real.xml @@ -1 +1 @@ -ipfilter
output151status01eq40x0000000100 +ipfilter
output151status1eq40x0000000100 diff --git a/tests/xmlfiles/54-rule-real.xml b/tests/xmlfiles/54-rule-real.xml index ed27e56..ecb596f 100644 --- a/tests/xmlfiles/54-rule-real.xml +++ b/tests/xmlfiles/54-rule-real.xml @@ -1 +1 @@ -ipfilter
output161mark01eq40x0000006400 +ipfilter
output161l3protocol11eq40x0000006400 diff --git a/tests/xmlfiles/55-rule-real.xml b/tests/xmlfiles/55-rule-real.xml index 2d2bf7f..b7e6606 100644 --- a/tests/xmlfiles/55-rule-real.xml +++ b/tests/xmlfiles/55-rule-real.xml @@ -1 +1 @@ -ipfilter
output171secmark01eq40x000000005511407 +ipfilter
output171secmark1eq40x000000005511407 diff --git a/tests/xmlfiles/56-rule-real.xml b/tests/xmlfiles/56-rule-real.xml index 4596689..0fc23cc 100644 --- a/tests/xmlfiles/56-rule-real.xml +++ b/tests/xmlfiles/56-rule-real.xml @@ -1 +1 @@ -ipfilter
output181expiration01eq40x0000001e00 +ipfilter
output181expiration1eq40x0000001e00 diff --git a/tests/xmlfiles/57-rule-real.xml b/tests/xmlfiles/57-rule-real.xml index 6a2ad52..971dfb5 100644 --- a/tests/xmlfiles/57-rule-real.xml +++ b/tests/xmlfiles/57-rule-real.xml @@ -1 +1 @@ -ipfilter
output191helper01eq40x0070746600 +ipfilter
output191helper1eq40x0070746600