* [PATCH 0/6] conntrack: fixes for handling unknown protocols
@ 2022-06-23 17:49 Mikhail Sennikovsky
2022-06-23 17:49 ` [PATCH 1/6] tests/conntrack: ct create for " Mikhail Sennikovsky
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Mikhail Sennikovsky @ 2022-06-23 17:49 UTC (permalink / raw)
To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky
Hi Pablo & all,
Here is a set of fixes and test cases for handling ct entries of
protocols unknown to the conntrack tool.
The six commits are organized in a way: a commit with a testcase
showing an issue, followed by a commit fixing the issue.
Comments/suggestions are very welcome as always.
Regards,
Mikhail
Mikhail Sennikovsky (6):
tests/conntrack: ct create for unknown protocols
conntrack: set reply l4 proto for unknown protocol
tests/conntrack: invalid protocol values
conntrack: fix protocol number parsing
tests/conntrack: ct -o save for unknown protocols
conntrack: fix -o save dump for unknown protocols
extensions/libct_proto_unknown.c | 11 ++++++++++
src/conntrack.c | 33 ++++++++++++++++++++++++++---
tests/conntrack/testsuite/00create | 32 ++++++++++++++++++++++++++++
tests/conntrack/testsuite/09dumpopt | 26 +++++++++++++++++++++++
4 files changed, 99 insertions(+), 3 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/6] tests/conntrack: ct create for unknown protocols
2022-06-23 17:49 [PATCH 0/6] conntrack: fixes for handling unknown protocols Mikhail Sennikovsky
@ 2022-06-23 17:49 ` Mikhail Sennikovsky
2022-06-23 17:49 ` [PATCH 2/6] conntrack: set reply l4 proto for unknown protocol Mikhail Sennikovsky
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Mikhail Sennikovsky @ 2022-06-23 17:49 UTC (permalink / raw)
To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky
Testcases covering creation of ct entries of unknown L4 protocols,
which does not work properly at the moment.
Fix included in the next commit.
Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
tests/conntrack/testsuite/00create | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/tests/conntrack/testsuite/00create b/tests/conntrack/testsuite/00create
index 911e711..9962e23 100644
--- a/tests/conntrack/testsuite/00create
+++ b/tests/conntrack/testsuite/00create
@@ -34,3 +34,30 @@
-I -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
# delete icmp ping request entry
-D -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# Test protocols unknown by the conntrack tool
+# IGMP
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# Create again - should fail
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; BAD
+# repeat using protocol name instead of the value, should fail as well
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p igmp ; BAD
+# delete
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# delete again should fail
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; BAD
+# create using protocol name instead of the value
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p igmp ; OK
+# update
+-U -t 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# delete
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# delete again should fail
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p igmp ; BAD
+# take some protocol that is not normally not in /etc/protocols
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# update
+-U -t 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# delete
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# delete again
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; BAD
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/6] conntrack: set reply l4 proto for unknown protocol
2022-06-23 17:49 [PATCH 0/6] conntrack: fixes for handling unknown protocols Mikhail Sennikovsky
2022-06-23 17:49 ` [PATCH 1/6] tests/conntrack: ct create for " Mikhail Sennikovsky
@ 2022-06-23 17:49 ` Mikhail Sennikovsky
2022-06-23 19:30 ` Pablo Neira Ayuso
2022-06-23 17:49 ` [PATCH 3/6] tests/conntrack: invalid protocol values Mikhail Sennikovsky
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Mikhail Sennikovsky @ 2022-06-23 17:49 UTC (permalink / raw)
To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky
Withouth reply l4 protocol being set consistently the mnl_cb_run
(in fact the kernel) would return EINVAL.
Make sure the reply l4 protocol is set properly for unknown
protocols.
Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
extensions/libct_proto_unknown.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/extensions/libct_proto_unknown.c b/extensions/libct_proto_unknown.c
index 2a47704..992b1ed 100644
--- a/extensions/libct_proto_unknown.c
+++ b/extensions/libct_proto_unknown.c
@@ -21,10 +21,21 @@ static void help(void)
fprintf(stdout, " no options (unsupported)\n");
}
+static void final_check(unsigned int flags,
+ unsigned int cmd,
+ struct nf_conntrack *ct)
+{
+ if (nfct_attr_is_set(ct, ATTR_REPL_L3PROTO) &&
+ nfct_attr_is_set(ct, ATTR_L4PROTO) &&
+ !nfct_attr_is_set(ct, ATTR_REPL_L4PROTO))
+ nfct_set_attr_u8(ct, ATTR_REPL_L4PROTO, nfct_get_attr_u8(ct, ATTR_L4PROTO));
+}
+
struct ctproto_handler ct_proto_unknown = {
.name = "unknown",
.help = help,
.opts = opts,
+ .final_check = final_check,
.version = VERSION,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/6] tests/conntrack: invalid protocol values
2022-06-23 17:49 [PATCH 0/6] conntrack: fixes for handling unknown protocols Mikhail Sennikovsky
2022-06-23 17:49 ` [PATCH 1/6] tests/conntrack: ct create for " Mikhail Sennikovsky
2022-06-23 17:49 ` [PATCH 2/6] conntrack: set reply l4 proto for unknown protocol Mikhail Sennikovsky
@ 2022-06-23 17:49 ` Mikhail Sennikovsky
2022-06-23 19:29 ` Pablo Neira Ayuso
2022-06-23 17:49 ` [PATCH 4/6] conntrack: fix protocol number parsing Mikhail Sennikovsky
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Mikhail Sennikovsky @ 2022-06-23 17:49 UTC (permalink / raw)
To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky
Testcases covering passing invalid protocol values via -p parameter.
* -p 256 should fail
* -p foo should fail
which does not work properly at the moment.
Fix included in the next commit.
Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
tests/conntrack/testsuite/00create | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tests/conntrack/testsuite/00create b/tests/conntrack/testsuite/00create
index 9962e23..9fb3a0b 100644
--- a/tests/conntrack/testsuite/00create
+++ b/tests/conntrack/testsuite/00create
@@ -61,3 +61,8 @@
-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
# delete again
-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; BAD
+# Invalid protocol values
+# 256 should fail
+-I -t 10 -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p 256 ; BAD
+# take some invalid protocol name
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p foo ; BAD
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/6] conntrack: fix protocol number parsing
2022-06-23 17:49 [PATCH 0/6] conntrack: fixes for handling unknown protocols Mikhail Sennikovsky
` (2 preceding siblings ...)
2022-06-23 17:49 ` [PATCH 3/6] tests/conntrack: invalid protocol values Mikhail Sennikovsky
@ 2022-06-23 17:49 ` Mikhail Sennikovsky
2022-06-23 19:29 ` Pablo Neira Ayuso
2022-06-23 17:49 ` [PATCH 5/6] tests/conntrack: ct -o save for unknown protocols Mikhail Sennikovsky
2022-06-23 17:50 ` [PATCH 6/6] conntrack: fix -o save dump " Mikhail Sennikovsky
5 siblings, 1 reply; 12+ messages in thread
From: Mikhail Sennikovsky @ 2022-06-23 17:49 UTC (permalink / raw)
To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky
Before this commit it was possible to successfully create a ct entry
passing -p 256 and -p some_nonsense.
In both cases an entry with the protocol=0 would be created.
Do not allow invalid protocol values to -p option.
Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
src/conntrack.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/conntrack.c b/src/conntrack.c
index 500e736..dca7da6 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -882,6 +882,24 @@ static int ct_save_snprintf(char *buf, size_t len,
extern struct ctproto_handler ct_proto_unknown;
+static int parse_proto_num(const char *str)
+{
+ char *endptr;
+ long val;
+
+ errno = 0;
+ val = strtol(str, &endptr, 0);
+ if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) ||
+ (errno != 0 && val == 0) ||
+ endptr == str ||
+ *endptr != '\0' ||
+ val >= IPPROTO_MAX) {
+ return -1;
+ }
+
+ return val;
+}
+
static struct ctproto_handler *findproto(char *name, int *pnum)
{
struct ctproto_handler *cur;
@@ -901,8 +919,8 @@ static struct ctproto_handler *findproto(char *name, int *pnum)
return &ct_proto_unknown;
}
/* using a protocol number? */
- protonum = atoi(name);
- if (protonum >= 0 && protonum <= IPPROTO_MAX) {
+ protonum = parse_proto_num(name);
+ if (protonum >= 0) {
/* try lookup by number, perhaps this protocol is supported */
list_for_each_entry(cur, &proto_list, head) {
if (cur->protonum == protonum) {
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/6] tests/conntrack: ct -o save for unknown protocols
2022-06-23 17:49 [PATCH 0/6] conntrack: fixes for handling unknown protocols Mikhail Sennikovsky
` (3 preceding siblings ...)
2022-06-23 17:49 ` [PATCH 4/6] conntrack: fix protocol number parsing Mikhail Sennikovsky
@ 2022-06-23 17:49 ` Mikhail Sennikovsky
2022-06-23 19:28 ` Pablo Neira Ayuso
2022-06-23 17:50 ` [PATCH 6/6] conntrack: fix -o save dump " Mikhail Sennikovsky
5 siblings, 1 reply; 12+ messages in thread
From: Mikhail Sennikovsky @ 2022-06-23 17:49 UTC (permalink / raw)
To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky
Testcases covering dumping in -o save formate of ct entries of
L4 protocols unknown to the conntrack tool,
which does not work properly at the moment.
Fix included in the next commit.
Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
tests/conntrack/testsuite/09dumpopt | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/tests/conntrack/testsuite/09dumpopt b/tests/conntrack/testsuite/09dumpopt
index 447590b..c1e0e6e 100644
--- a/tests/conntrack/testsuite/09dumpopt
+++ b/tests/conntrack/testsuite/09dumpopt
@@ -145,3 +145,29 @@
-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
# clean up after yourself
-D -w 10 ; OK
+# Cover protocols unknown to the conntrack tool
+# Create a conntrack entries
+# IGMP
+-I -w 10 -t 59 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ;
+# Some fency protocol
+-I -w 10 -t 59 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ;
+# Some fency protocol with IPv6
+-I -w 10 -t 59 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ;
+-R - ; OK
+# copy to zone 11
+-L -w 10 -o save ; |s/-w 10/-w 11/g
+-R - ; OK
+# Delete stuff in zone 10, should succeed
+# IGMP
+-D -w 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# Some fency protocol
+-D -w 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# Some fency protocol with IPv6
+-D -w 10 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ; OK
+# Delete stuff in zone 11, should succeed
+# IGMP
+-D -w 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# Some fency protocol
+-D -w 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# Some fency protocol with IPv6
+-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ; OK
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 6/6] conntrack: fix -o save dump for unknown protocols
2022-06-23 17:49 [PATCH 0/6] conntrack: fixes for handling unknown protocols Mikhail Sennikovsky
` (4 preceding siblings ...)
2022-06-23 17:49 ` [PATCH 5/6] tests/conntrack: ct -o save for unknown protocols Mikhail Sennikovsky
@ 2022-06-23 17:50 ` Mikhail Sennikovsky
2022-06-23 19:27 ` Pablo Neira Ayuso
5 siblings, 1 reply; 12+ messages in thread
From: Mikhail Sennikovsky @ 2022-06-23 17:50 UTC (permalink / raw)
To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky
Make sure the protocol (-p) option is included in the -o save
ct entry dumps for L4 protocols unknown to the conntrack tool
Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
src/conntrack.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/conntrack.c b/src/conntrack.c
index dca7da6..f8a228f 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -870,9 +870,18 @@ static int ct_save_snprintf(char *buf, size_t len,
ret = ct_snprintf_opts(buf + offset, len, ct, cur->print_opts);
BUFFER_SIZE(ret, size, len, offset);
- break;
+ goto done_proto4;
}
+ /**
+ * Do not use getprotobynumber here to ensure
+ * "-o save" data incompatibility between hosts having
+ * different /etc/protocols contents
+ */
+ ret = snprintf(buf + offset, len, "-p %d ", l4proto);
+ BUFFER_SIZE(ret, size, len, offset);
+
+done_proto4:
/* skip trailing space, if any */
for (; size && buf[size-1] == ' '; --size)
buf[size-1] = '\0';
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 6/6] conntrack: fix -o save dump for unknown protocols
2022-06-23 17:50 ` [PATCH 6/6] conntrack: fix -o save dump " Mikhail Sennikovsky
@ 2022-06-23 19:27 ` Pablo Neira Ayuso
0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2022-06-23 19:27 UTC (permalink / raw)
To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky
On Thu, Jun 23, 2022 at 07:50:00PM +0200, Mikhail Sennikovsky wrote:
> Make sure the protocol (-p) option is included in the -o save
> ct entry dumps for L4 protocols unknown to the conntrack tool
>
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
> src/conntrack.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/conntrack.c b/src/conntrack.c
> index dca7da6..f8a228f 100644
> --- a/src/conntrack.c
> +++ b/src/conntrack.c
> @@ -870,9 +870,18 @@ static int ct_save_snprintf(char *buf, size_t len,
>
> ret = ct_snprintf_opts(buf + offset, len, ct, cur->print_opts);
> BUFFER_SIZE(ret, size, len, offset);
> - break;
> + goto done_proto4;
I'd suggest:
l4proto_set = true;
so you can remove this goto.
> }
>
> + /**
> + * Do not use getprotobynumber here to ensure
> + * "-o save" data incompatibility between hosts having
> + * different /etc/protocols contents
> + */
No need for this comment, explain this in the commit message, git
annotate will help to find the reason for this.
> + ret = snprintf(buf + offset, len, "-p %d ", l4proto);
> + BUFFER_SIZE(ret, size, len, offset);
if (!l4proto_set) {
ret = snprintf(buf + offset, len, "-p %d ", l4proto);
BUFFER_SIZE(ret, size, len, offset);
}
> +
> +done_proto4:
> /* skip trailing space, if any */
> for (; size && buf[size-1] == ' '; --size)
> buf[size-1] = '\0';
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] tests/conntrack: ct -o save for unknown protocols
2022-06-23 17:49 ` [PATCH 5/6] tests/conntrack: ct -o save for unknown protocols Mikhail Sennikovsky
@ 2022-06-23 19:28 ` Pablo Neira Ayuso
0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2022-06-23 19:28 UTC (permalink / raw)
To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky
On Thu, Jun 23, 2022 at 07:49:59PM +0200, Mikhail Sennikovsky wrote:
> Testcases covering dumping in -o save formate of ct entries of
> L4 protocols unknown to the conntrack tool,
> which does not work properly at the moment.
> Fix included in the next commit.
Could you collapse the test to the relevant patch (next commit as
description suggests)? I find this easier to find when looking at git
annotate.
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
> tests/conntrack/testsuite/09dumpopt | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/tests/conntrack/testsuite/09dumpopt b/tests/conntrack/testsuite/09dumpopt
> index 447590b..c1e0e6e 100644
> --- a/tests/conntrack/testsuite/09dumpopt
> +++ b/tests/conntrack/testsuite/09dumpopt
> @@ -145,3 +145,29 @@
> -D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
> # clean up after yourself
> -D -w 10 ; OK
> +# Cover protocols unknown to the conntrack tool
> +# Create a conntrack entries
> +# IGMP
> +-I -w 10 -t 59 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ;
> +# Some fency protocol
> +-I -w 10 -t 59 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ;
> +# Some fency protocol with IPv6
> +-I -w 10 -t 59 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ;
> +-R - ; OK
> +# copy to zone 11
> +-L -w 10 -o save ; |s/-w 10/-w 11/g
> +-R - ; OK
> +# Delete stuff in zone 10, should succeed
> +# IGMP
> +-D -w 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
> +# Some fency protocol
> +-D -w 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
> +# Some fency protocol with IPv6
> +-D -w 10 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ; OK
> +# Delete stuff in zone 11, should succeed
> +# IGMP
> +-D -w 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
> +# Some fency protocol
> +-D -w 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
> +# Some fency protocol with IPv6
> +-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ; OK
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/6] conntrack: fix protocol number parsing
2022-06-23 17:49 ` [PATCH 4/6] conntrack: fix protocol number parsing Mikhail Sennikovsky
@ 2022-06-23 19:29 ` Pablo Neira Ayuso
0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2022-06-23 19:29 UTC (permalink / raw)
To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky
On Thu, Jun 23, 2022 at 07:49:58PM +0200, Mikhail Sennikovsky wrote:
> Before this commit it was possible to successfully create a ct entry
> passing -p 256 and -p some_nonsense.
> In both cases an entry with the protocol=0 would be created.
>
> Do not allow invalid protocol values to -p option.
>
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
> src/conntrack.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/src/conntrack.c b/src/conntrack.c
> index 500e736..dca7da6 100644
> --- a/src/conntrack.c
> +++ b/src/conntrack.c
> @@ -882,6 +882,24 @@ static int ct_save_snprintf(char *buf, size_t len,
>
> extern struct ctproto_handler ct_proto_unknown;
>
> +static int parse_proto_num(const char *str)
> +{
> + char *endptr;
> + long val;
> +
> + errno = 0;
> + val = strtol(str, &endptr, 0);
> + if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) ||
> + (errno != 0 && val == 0) ||
> + endptr == str ||
> + *endptr != '\0' ||
> + val >= IPPROTO_MAX) {
There might be a more simple way to do error reporting for strtoul?
> + return -1;
> + }
> +
> + return val;
> +}
> +
> static struct ctproto_handler *findproto(char *name, int *pnum)
> {
> struct ctproto_handler *cur;
> @@ -901,8 +919,8 @@ static struct ctproto_handler *findproto(char *name, int *pnum)
> return &ct_proto_unknown;
> }
> /* using a protocol number? */
> - protonum = atoi(name);
> - if (protonum >= 0 && protonum <= IPPROTO_MAX) {
> + protonum = parse_proto_num(name);
> + if (protonum >= 0) {
> /* try lookup by number, perhaps this protocol is supported */
> list_for_each_entry(cur, &proto_list, head) {
> if (cur->protonum == protonum) {
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/6] tests/conntrack: invalid protocol values
2022-06-23 17:49 ` [PATCH 3/6] tests/conntrack: invalid protocol values Mikhail Sennikovsky
@ 2022-06-23 19:29 ` Pablo Neira Ayuso
0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2022-06-23 19:29 UTC (permalink / raw)
To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky
On Thu, Jun 23, 2022 at 07:49:57PM +0200, Mikhail Sennikovsky wrote:
> Testcases covering passing invalid protocol values via -p parameter.
> * -p 256 should fail
> * -p foo should fail
> which does not work properly at the moment.
> Fix included in the next commit.
Please, collapse this to next patch 4/6 in v2.
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
> tests/conntrack/testsuite/00create | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tests/conntrack/testsuite/00create b/tests/conntrack/testsuite/00create
> index 9962e23..9fb3a0b 100644
> --- a/tests/conntrack/testsuite/00create
> +++ b/tests/conntrack/testsuite/00create
> @@ -61,3 +61,8 @@
> -D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
> # delete again
> -D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; BAD
> +# Invalid protocol values
> +# 256 should fail
> +-I -t 10 -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p 256 ; BAD
> +# take some invalid protocol name
> +-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p foo ; BAD
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/6] conntrack: set reply l4 proto for unknown protocol
2022-06-23 17:49 ` [PATCH 2/6] conntrack: set reply l4 proto for unknown protocol Mikhail Sennikovsky
@ 2022-06-23 19:30 ` Pablo Neira Ayuso
0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2022-06-23 19:30 UTC (permalink / raw)
To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky
On Thu, Jun 23, 2022 at 07:49:56PM +0200, Mikhail Sennikovsky wrote:
> Withouth reply l4 protocol being set consistently the mnl_cb_run
> (in fact the kernel) would return EINVAL.
>
> Make sure the reply l4 protocol is set properly for unknown
> protocols.
>
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
> extensions/libct_proto_unknown.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/extensions/libct_proto_unknown.c b/extensions/libct_proto_unknown.c
> index 2a47704..992b1ed 100644
> --- a/extensions/libct_proto_unknown.c
> +++ b/extensions/libct_proto_unknown.c
> @@ -21,10 +21,21 @@ static void help(void)
> fprintf(stdout, " no options (unsupported)\n");
> }
>
> +static void final_check(unsigned int flags,
> + unsigned int cmd,
> + struct nf_conntrack *ct)
> +{
> + if (nfct_attr_is_set(ct, ATTR_REPL_L3PROTO) &&
> + nfct_attr_is_set(ct, ATTR_L4PROTO) &&
> + !nfct_attr_is_set(ct, ATTR_REPL_L4PROTO))
> + nfct_set_attr_u8(ct, ATTR_REPL_L4PROTO, nfct_get_attr_u8(ct, ATTR_L4PROTO));
> +}
> +
> struct ctproto_handler ct_proto_unknown = {
> .name = "unknown",
> .help = help,
> .opts = opts,
> + .final_check = final_check,
missing indent to align it with other C99 initializers (coding style nitpick)
.final_check = final_check,
> .version = VERSION,
> };
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-06-23 19:40 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-23 17:49 [PATCH 0/6] conntrack: fixes for handling unknown protocols Mikhail Sennikovsky
2022-06-23 17:49 ` [PATCH 1/6] tests/conntrack: ct create for " Mikhail Sennikovsky
2022-06-23 17:49 ` [PATCH 2/6] conntrack: set reply l4 proto for unknown protocol Mikhail Sennikovsky
2022-06-23 19:30 ` Pablo Neira Ayuso
2022-06-23 17:49 ` [PATCH 3/6] tests/conntrack: invalid protocol values Mikhail Sennikovsky
2022-06-23 19:29 ` Pablo Neira Ayuso
2022-06-23 17:49 ` [PATCH 4/6] conntrack: fix protocol number parsing Mikhail Sennikovsky
2022-06-23 19:29 ` Pablo Neira Ayuso
2022-06-23 17:49 ` [PATCH 5/6] tests/conntrack: ct -o save for unknown protocols Mikhail Sennikovsky
2022-06-23 19:28 ` Pablo Neira Ayuso
2022-06-23 17:50 ` [PATCH 6/6] conntrack: fix -o save dump " Mikhail Sennikovsky
2022-06-23 19:27 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.