netdev - Re: [RFC net-next] net: xfrm: support setting an output mark.

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: <20170807111623.GQ2631@secunet.com>
Date:   Mon, 7 Aug 2017 13:16:23 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Lorenzo Colitti <lorenzo@...gle.com>
CC:     <netdev@...r.kernel.org>, <davem@...emloft.net>,
        <jhs@...atatu.com>, <herbert@...dor.apana.org.au>,
        <nharold@...gle.com>, <misterikkit@...gle.com>
Subject: Re: [RFC net-next] net: xfrm: support setting an output mark.

On Mon, Aug 07, 2017 at 06:23:26PM +0900, Lorenzo Colitti wrote:
> On systems that use mark-based routing it may be necessary for
> routing lookups to use marks in order for packets to be routed
> correctly. An example of such a system is Android, which uses
> socket marks to route packets via different networks.
> 
> Currently, routing lookups in tunnel mode always use a mark of
> zero, making routing incorrect on such systems. This patch adds
> a new output_mark parameter to the SA properties.  The mark will
> be used in routing lookups, and if nonzero, will also set the
> mark of the packets emitted by the SA, so it can be matched by
> iptables.
> 
> Tested: https://android-review.googlesource.com/452776
> Signed-off-by: Lorenzo Colitti <lorenzo@...gle.com>
> ---
>  include/net/xfrm.h        |  9 ++++++---
>  include/uapi/linux/xfrm.h |  1 +
>  net/ipv4/xfrm4_policy.c   | 14 +++++++++-----
>  net/ipv6/xfrm6_policy.c   |  9 ++++++---
>  net/xfrm/xfrm_output.c    |  3 +++
>  net/xfrm/xfrm_policy.c    | 17 +++++++++--------
>  net/xfrm/xfrm_user.c      | 11 +++++++++++
>  7 files changed, 45 insertions(+), 19 deletions(-)
> 
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index afb4929d72..6b1ddb790d 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -163,6 +163,7 @@ struct xfrm_state {
>  		int		header_len;
>  		int		trailer_len;
>  		u32		extra_flags;
> +		u32		output_mark;
>  	} props;
>  
>  	struct xfrm_lifetime_cfg lft;
> @@ -296,10 +297,12 @@ struct xfrm_policy_afinfo {
>  	struct dst_entry	*(*dst_lookup)(struct net *net,
>  					       int tos, int oif,
>  					       const xfrm_address_t *saddr,
> -					       const xfrm_address_t *daddr);
> +					       const xfrm_address_t *daddr,
> +					       u32 mark);
>  	int			(*get_saddr)(struct net *net, int oif,
>  					     xfrm_address_t *saddr,
> -					     xfrm_address_t *daddr);
> +					     xfrm_address_t *daddr,
> +					     u32 mark);
>  	void			(*decode_session)(struct sk_buff *skb,
>  						  struct flowi *fl,
>  						  int reverse);
> @@ -1638,7 +1641,7 @@ static inline int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
>  struct dst_entry *__xfrm_dst_lookup(struct net *net, int tos, int oif,
>  				    const xfrm_address_t *saddr,
>  				    const xfrm_address_t *daddr,
> -				    int family);
> +				    int family, u32 mark);
>  
>  struct xfrm_policy *xfrm_policy_alloc(struct net *net, gfp_t gfp);
>  
> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
> index 2b384ff09f..5fe7370a2b 100644
> --- a/include/uapi/linux/xfrm.h
> +++ b/include/uapi/linux/xfrm.h
> @@ -304,6 +304,7 @@ enum xfrm_attr_type_t {
>  	XFRMA_ADDRESS_FILTER,	/* struct xfrm_address_filter */
>  	XFRMA_PAD,
>  	XFRMA_OFFLOAD_DEV,	/* struct xfrm_state_offload */
> +	XFRMA_OUTPUT_MARK,	/* __u32 */
>  	__XFRMA_MAX

Hm, why don't you use the existing xfrm_mark for this?
Having two different marks on one SA seems to be a bit odd.