* [PATCH] src: proto: Add missing packet type @ 2016-03-01 12:39 Shivani Bhardwaj 2016-03-01 13:17 ` Florian Westphal 0 siblings, 1 reply; 5+ messages in thread From: Shivani Bhardwaj @ 2016-03-01 12:39 UTC (permalink / raw) To: netfilter-devel Add missing packet type "invalid" for DCCP. Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> --- src/proto.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/proto.c b/src/proto.c index 0ed98ed..4d049f5 100644 --- a/src/proto.c +++ b/src/proto.c @@ -443,6 +443,7 @@ static const struct symbol_table dccp_pkttype_tbl = { SYMBOL("reset", DCCP_PKT_RESET), SYMBOL("sync", DCCP_PKT_SYNC), SYMBOL("syncack", DCCP_PKT_SYNCACK), + SYMBOL("invalid", DCCP_PKT_INVALID), SYMBOL_LIST_END }, }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] src: proto: Add missing packet type 2016-03-01 12:39 [PATCH] src: proto: Add missing packet type Shivani Bhardwaj @ 2016-03-01 13:17 ` Florian Westphal 2016-03-01 13:51 ` Shivani Bhardwaj 0 siblings, 1 reply; 5+ messages in thread From: Florian Westphal @ 2016-03-01 13:17 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: netfilter-devel Shivani Bhardwaj <shivanib134@gmail.com> wrote: > Add missing packet type "invalid" for DCCP. > > Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> > --- > src/proto.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/proto.c b/src/proto.c > index 0ed98ed..4d049f5 100644 > --- a/src/proto.c > +++ b/src/proto.c > @@ -443,6 +443,7 @@ static const struct symbol_table dccp_pkttype_tbl = { > SYMBOL("reset", DCCP_PKT_RESET), > SYMBOL("sync", DCCP_PKT_SYNC), > SYMBOL("syncack", DCCP_PKT_SYNCACK), > + SYMBOL("invalid", DCCP_PKT_INVALID), I don't think this is a good idea -- when user asks to match 'invalid' then this will check type == DCCP_PKT_INVALID; however the correct way would be to ask for type '> synack' (i.e., outside of the range of types specified). ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] src: proto: Add missing packet type 2016-03-01 13:17 ` Florian Westphal @ 2016-03-01 13:51 ` Shivani Bhardwaj 2016-03-01 14:01 ` Florian Westphal 0 siblings, 1 reply; 5+ messages in thread From: Shivani Bhardwaj @ 2016-03-01 13:51 UTC (permalink / raw) To: Florian Westphal; +Cc: Netfilter Development Mailing list On Tue, Mar 1, 2016 at 6:47 PM, Florian Westphal <fw@strlen.de> wrote: > Shivani Bhardwaj <shivanib134@gmail.com> wrote: >> Add missing packet type "invalid" for DCCP. >> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> >> --- >> src/proto.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/proto.c b/src/proto.c >> index 0ed98ed..4d049f5 100644 >> --- a/src/proto.c >> +++ b/src/proto.c >> @@ -443,6 +443,7 @@ static const struct symbol_table dccp_pkttype_tbl = { >> SYMBOL("reset", DCCP_PKT_RESET), >> SYMBOL("sync", DCCP_PKT_SYNC), >> SYMBOL("syncack", DCCP_PKT_SYNCACK), >> + SYMBOL("invalid", DCCP_PKT_INVALID), > > I don't think this is a good idea -- when user asks to match > 'invalid' then this will check type == DCCP_PKT_INVALID; however > the correct way would be to ask for type '> synack' (i.e., > outside of the range of types specified). OK. So, should this be removed from the enum dccp_pkt_type as well? And, if following iptables rules, one tries to get code corresponding to dccp type invalid using iptables-translate utility, then the corresponding nft rule should be "dccp type gt syncack". Please correct me if I'm wrong here. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] src: proto: Add missing packet type 2016-03-01 13:51 ` Shivani Bhardwaj @ 2016-03-01 14:01 ` Florian Westphal 2016-03-03 17:58 ` Pablo Neira Ayuso 0 siblings, 1 reply; 5+ messages in thread From: Florian Westphal @ 2016-03-01 14:01 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: Florian Westphal, Netfilter Development Mailing list Shivani Bhardwaj <shivanib134@gmail.com> wrote: > On Tue, Mar 1, 2016 at 6:47 PM, Florian Westphal <fw@strlen.de> wrote: > > Shivani Bhardwaj <shivanib134@gmail.com> wrote: > >> Add missing packet type "invalid" for DCCP. > >> > >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> > >> --- > >> src/proto.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/src/proto.c b/src/proto.c > >> index 0ed98ed..4d049f5 100644 > >> --- a/src/proto.c > >> +++ b/src/proto.c > >> @@ -443,6 +443,7 @@ static const struct symbol_table dccp_pkttype_tbl = { > >> SYMBOL("reset", DCCP_PKT_RESET), > >> SYMBOL("sync", DCCP_PKT_SYNC), > >> SYMBOL("syncack", DCCP_PKT_SYNCACK), > >> + SYMBOL("invalid", DCCP_PKT_INVALID), > > > > I don't think this is a good idea -- when user asks to match > > 'invalid' then this will check type == DCCP_PKT_INVALID; however > > the correct way would be to ask for type '> synack' (i.e., > > outside of the range of types specified). > > OK. So, should this be removed from the enum dccp_pkt_type as well? Yes, but we cannot do this anymore for compatibility reasons :-( > And, if following iptables rules, one tries to get code corresponding > to dccp type invalid using iptables-translate utility, then the > corresponding nft rule should be "dccp type gt syncack". Please > correct me if I'm wrong here. Good question. Pablo, I think -m dccp is buggy here, it should not have allowed INVALID match in the first place. Looking at kernel part it doesn't seem to special-case this and will just check for type == INVALID. But such type isn't defined in dccp protocol, it just looks like implementation helper so DCCP stack can do simpler sanity tests. So I suggest to not support translation of INVALID to nft and to document the INVALID shortoming in iptables-extensions manpage. Any comments? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] src: proto: Add missing packet type 2016-03-01 14:01 ` Florian Westphal @ 2016-03-03 17:58 ` Pablo Neira Ayuso 0 siblings, 0 replies; 5+ messages in thread From: Pablo Neira Ayuso @ 2016-03-03 17:58 UTC (permalink / raw) To: Florian Westphal; +Cc: Shivani Bhardwaj, Netfilter Development Mailing list On Tue, Mar 01, 2016 at 03:01:06PM +0100, Florian Westphal wrote: > Shivani Bhardwaj <shivanib134@gmail.com> wrote: > > On Tue, Mar 1, 2016 at 6:47 PM, Florian Westphal <fw@strlen.de> wrote: > > > Shivani Bhardwaj <shivanib134@gmail.com> wrote: > > >> Add missing packet type "invalid" for DCCP. > > >> > > >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> > > >> --- > > >> src/proto.c | 1 + > > >> 1 file changed, 1 insertion(+) > > >> > > >> diff --git a/src/proto.c b/src/proto.c > > >> index 0ed98ed..4d049f5 100644 > > >> --- a/src/proto.c > > >> +++ b/src/proto.c > > >> @@ -443,6 +443,7 @@ static const struct symbol_table dccp_pkttype_tbl = { > > >> SYMBOL("reset", DCCP_PKT_RESET), > > >> SYMBOL("sync", DCCP_PKT_SYNC), > > >> SYMBOL("syncack", DCCP_PKT_SYNCACK), > > >> + SYMBOL("invalid", DCCP_PKT_INVALID), > > > > > > I don't think this is a good idea -- when user asks to match > > > 'invalid' then this will check type == DCCP_PKT_INVALID; however > > > the correct way would be to ask for type '> synack' (i.e., > > > outside of the range of types specified). > > > > OK. So, should this be removed from the enum dccp_pkt_type as well? > > Yes, but we cannot do this anymore for compatibility reasons :-( > > > And, if following iptables rules, one tries to get code corresponding > > to dccp type invalid using iptables-translate utility, then the > > corresponding nft rule should be "dccp type gt syncack". Please > > correct me if I'm wrong here. > > Good question. > Pablo, I think -m dccp is buggy here, it should not have allowed > INVALID match in the first place. Right. > Looking at kernel part it doesn't seem to special-case this > and will just check for type == INVALID. But such type isn't > defined in dccp protocol, it just looks like implementation > helper so DCCP stack can do simpler sanity tests. > > So I suggest to not support translation of INVALID to nft and > to document the INVALID shortoming in iptables-extensions manpage. > > Any comments? That's fine, we can return 0 (no translation available) in that case. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-03-03 17:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-01 12:39 [PATCH] src: proto: Add missing packet type Shivani Bhardwaj 2016-03-01 13:17 ` Florian Westphal 2016-03-01 13:51 ` Shivani Bhardwaj 2016-03-01 14:01 ` Florian Westphal 2016-03-03 17:58 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).