| Message ID | 20160301123912.GA6905@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Pablo Neira |
| Headers | show |
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). -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 }, };
Add missing packet type "invalid" for DCCP. Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> --- src/proto.c | 1 + 1 file changed, 1 insertion(+)