| Message ID | 20200304022459.6433-1-phil@nwl.cc |
|---|---|
| State | Superseded |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [iptables] connlabel: Allow numeric labels even if connlabel.conf exists | expand |
Phil Sutter <phil@nwl.cc> wrote: > Existing code is a bit quirky: If no connlabel.conf was found, the local > function connlabel_value_parse() is called which tries to interpret > given label as a number. If the config exists though, > nfct_labelmap_get_bit() is called instead which doesn't care about > "undefined" connlabel names. So unless installed connlabel.conf contains > entries for all possible numeric labels, rules added by users may stop > working if a connlabel.conf is created. Fix this by falling back to > connlabel_value_parse() function also if connlabel_open() returned 0 but > nfct_labelmap_get_bit() returned an error. Acked-by: Florian Westphal <fw@strlen.de>
Hi, On Wed, Mar 04, 2020 at 09:16:51AM +0100, Florian Westphal wrote: > Phil Sutter <phil@nwl.cc> wrote: > > Existing code is a bit quirky: If no connlabel.conf was found, the local > > function connlabel_value_parse() is called which tries to interpret > > given label as a number. If the config exists though, > > nfct_labelmap_get_bit() is called instead which doesn't care about > > "undefined" connlabel names. So unless installed connlabel.conf contains > > entries for all possible numeric labels, rules added by users may stop > > working if a connlabel.conf is created. Fix this by falling back to > > connlabel_value_parse() function also if connlabel_open() returned 0 but > > nfct_labelmap_get_bit() returned an error. > > Acked-by: Florian Westphal <fw@strlen.de> When checking whether documentation needs an update, I stumbled upon the following sentences: "Instead of a name (which will be translated to a number, see EXAMPLE below), a number may be used instead. Using a number always overrides connlabel.conf." So actually I should change the code to try numeric parsing first and only then fall back to nfct_labelmap_get_bit(). Commit 51340f7b6a110 ("extensions: libxt_connlabel: use libnetfilter_conntrack") broke this in 2013. I'll send a v2. Thanks, Phil
diff --git a/extensions/libxt_connlabel.c b/extensions/libxt_connlabel.c index 5a01fe7237bd8..1fc92f42cd969 100644 --- a/extensions/libxt_connlabel.c +++ b/extensions/libxt_connlabel.c @@ -71,7 +71,7 @@ static void connlabel_mt_parse(struct xt_option_call *cb) { struct xt_connlabel_mtinfo *info = cb->data; bool have_labelmap = !connlabel_open(); - int tmp; + int tmp = -1; xtables_option_parse(cb); @@ -79,7 +79,7 @@ static void connlabel_mt_parse(struct xt_option_call *cb) case O_LABEL: if (have_labelmap) tmp = nfct_labelmap_get_bit(map, cb->arg); - else + if (tmp < 0) tmp = connlabel_value_parse(cb->arg); if (tmp < 0)
Existing code is a bit quirky: If no connlabel.conf was found, the local function connlabel_value_parse() is called which tries to interpret given label as a number. If the config exists though, nfct_labelmap_get_bit() is called instead which doesn't care about "undefined" connlabel names. So unless installed connlabel.conf contains entries for all possible numeric labels, rules added by users may stop working if a connlabel.conf is created. Fix this by falling back to connlabel_value_parse() function also if connlabel_open() returned 0 but nfct_labelmap_get_bit() returned an error. Fixes: 3a3bb480a738a ("extensions: connlabel: Fallback on missing connlabel.conf") Signed-off-by: Phil Sutter <phil@nwl.cc> --- extensions/libxt_connlabel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)