linux-kernel - Re: [PATCH net-next 8/9] openvswitch: Allow matching on conntrack label

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150731132004.GA20471@breakpoint.cc>
Date:	Fri, 31 Jul 2015 15:20:04 +0200
From:	Florian Westphal <fw@...len.de>
To:	Joe Stringer <joestringer@...ira.com>
Cc:	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	pablo@...filter.org, kaber@...sh.net, jpettit@...ira.com,
	pshelar@...ira.com, azhou@...ira.com, jesse@...ira.com,
	fwestpha@...hat.com, hannes@...hat.com, tgraf@...ronetworks.com
Subject: Re: [PATCH net-next 8/9] openvswitch: Allow matching on conntrack
 label

Joe Stringer <joestringer@...ira.com> wrote:
> Allow matching and setting the conntrack label field. As with ct_mark,
> this is populated by executing the ct() action, and is a writable field.
> The set_field() action may be used to modify the label, which will take
> effect on the most recent conntrack entry.
> 
> E.g.: actions:ct(zone=1),set_field(1->ct_label)
> 
> This will perform conntrack lookup in zone 1, then modify the label for
> that entry. The conntrack entry itself must be committed using the
> "commit" flag in the conntrack action flags for this change to persist.
> 
> Signed-off-by: Joe Stringer <joestringer@...ira.com>

> +/* Load connlabel and ensure it supports 128-bit labels */
> +static struct xt_match *load_connlabel(struct net *net)
> +{
> +#ifdef CONFIG_NF_CONNTRACK_LABELS
> +	struct xt_match *match;
> +	struct xt_mtchk_param mtpar;
> +	struct xt_connlabel_mtinfo info;
> +	int err = -EINVAL;
> +
> +	match = xt_request_find_match(NFPROTO_UNSPEC, "connlabel", 0);
> +	if (IS_ERR(match)) {
> +		match = NULL;
> +		goto exit;
> +	}
> +
> +	info.bit = sizeof(struct ovs_key_ct_label) * 8 - 1;
> +	info.options = 0;
> +
> +	mtpar.net	= net;
> +	mtpar.table	= match->table;
> +	mtpar.entryinfo = NULL;
> +	mtpar.match	= match;
> +	mtpar.matchinfo = &info;
> +	mtpar.hook_mask = BIT(NF_INET_PRE_ROUTING);
> +	mtpar.family	= NFPROTO_IPV4;
> +
> +	err = xt_check_match(&mtpar, XT_ALIGN(match->matchsize), match->proto,
> +			     0);

Yummy :-)

Rather than adding a dependency on xtables I think a better option would
be to move the

par->net->ct.labels_used++;
words = BITS_TO_LONGS(info->bit+1);
if (words > par->net->ct.label_words)
	par->net->ct.label_words = words;

parts from the checkentry/destroy hooks of xt_connlabel into
nf_conntrack_labels.c so that you don't need this mtpar stunt above
anymore (and I'd like to add ctlabel set support for nft at one point
so I'd also need to move that out of xt_label).

You can move that out of this series and submit that to nf-devel as
separate patch if you want.

> +	    ovs_ct_verify(OVS_KEY_ATTR_CT_LABEL)) {
> +		const struct ovs_key_ct_label *cl;
> +
> +		cl = nla_data(a[OVS_KEY_ATTR_CT_LABEL]);
> +		SW_FLOW_KEY_MEMCPY(match, ct.label, cl->ct_label,
> +				   sizeof(*cl), is_mask);
> +		*attrs &= ~(1ULL << OVS_KEY_ATTR_CT_LABEL);
> +	}

So you're using labels as arbitrary 128 bit identifier, right?

Nothing wrong with that, just asking.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/