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] [day] [month] [year] [list]
Date:   Wed, 17 Apr 2019 15:19:19 -0600
From:   David Ahern <dsahern@...il.com>
To:     Kristian Evensen <kristian.evensen@...il.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH iproute2-next] ip fou: Support binding FOU ports

On 4/9/19 1:14 PM, Kristian Evensen wrote:
> diff --git a/include/uapi/linux/fou.h b/include/uapi/linux/fou.h
> index bf022c63..9f915118 100644
> --- a/include/uapi/linux/fou.h
> +++ b/include/uapi/linux/fou.h
> @@ -16,6 +16,12 @@ enum {
>  	FOU_ATTR_IPPROTO,			/* u8 */
>  	FOU_ATTR_TYPE,				/* u8 */
>  	FOU_ATTR_REMCSUM_NOPARTIAL,		/* flag */
> +	FOU_ATTR_LOCAL_V4,			/* u32 */
> +	FOU_ATTR_LOCAL_V6,			/* in6_addr */
> +	FOU_ATTR_PEER_V4,			/* u32 */
> +	FOU_ATTR_PEER_V6,			/* in6_addr */
> +	FOU_ATTR_PEER_PORT,			/* u16 */
> +	FOU_ATTR_IFINDEX,			/* s32 */
>  
>  	__FOU_ATTR_MAX,
>  };

I just updated the kernel headers, so you can drop the above.

> diff --git a/ip/ipfou.c b/ip/ipfou.c
> index 346522dd..8bc62d78 100644
> --- a/ip/ipfou.c
> +++ b/ip/ipfou.c
> @@ -28,11 +28,17 @@ static void usage(void)
>  {
>  	fprintf(stderr,
>  		"Usage: ip fou add port PORT { ipproto PROTO  | gue } [ -6 ]\n"
> -		"       ip fou del port PORT [ -6 ]\n"
> +		"		   [ local IFADDR ] [ peer IFADDR ]\n"
> +		"		   [ peer_port PORT ] [ index IDX ]\n"
> +		"       ip fou del port PORT [ -6 ] [ local IFADDR ]\n"
> +		"		   [ peer IFADDR ] [ peer_port PORT ]\n"
> +		"		   [ index IDX ]\n"

iproute2 allows users to specify a device by name; it is not very user
friendly to have to run 'ip li sh' to lookup a device to get the index
to run an 'ip' command.

>  		"       ip fou show\n"
>  		"\n"
>  		"Where: PROTO { ipproto-name | 1..255 }\n"
> -		"       PORT { 1..65535 }\n");
> +		"       PORT { 1..65535 }\n"
> +		"       IFADDR { addr }\n"
> +		"       IDX { interface index }\n");
>  
>  	exit(-1);
>  }
> @@ -48,12 +54,14 @@ static int genl_family = -1;
>  static int fou_parse_opt(int argc, char **argv, struct nlmsghdr *n,
>  			 bool adding)
>  {
> -	__u16 port;
> -	int port_set = 0;
> +	__u16 port, peer_port;
> +	int port_set = 0, peer_port_set = 0;
>  	__u8 ipproto, type;
>  	bool gue_set = false;
>  	int ipproto_set = 0;
>  	__u8 family = AF_INET;
> +	const char *local = NULL, *peer = NULL;
> +	int index, index_set = 0;

iproute2 follows the kernel; please order your new ones in reverse xmas
tree (ie., they all go to the top).

>  
>  	while (argc > 0) {
>  		if (!matches(*argv, "port")) {
> @@ -77,6 +85,26 @@ static int fou_parse_opt(int argc, char **argv, struct nlmsghdr *n,
>  			gue_set = true;
>  		} else if (!matches(*argv, "-6")) {
>  			family = AF_INET6;
> +		} else if (!matches(*argv, "local")) {
> +			NEXT_ARG();
> +
> +			local = *argv;
> +		} else if (!matches(*argv, "peer")) {
> +			NEXT_ARG();
> +
> +			peer = *argv;
> +		} else if (!matches(*argv, "peer_port")) {
> +			NEXT_ARG();
> +
> +			if (get_be16(&peer_port, *argv, 0) || peer_port == 0)
> +				invarg("invalid peer port", *argv);
> +			peer_port_set = 1;

since port == 0 is not valid, then peer_port_set is not needed; just use
'if (peer_port)'

> +		} else if (!matches(*argv, "index")) {
> +			NEXT_ARG();
> +
> +			if (get_s32(&index, *argv, 0) || index == 0)
> +				invarg("invalid interface index", *argv);
> +			index_set = 1;
>  		} else {
>  			fprintf(stderr
>  				, "fou: unknown command \"%s\"?\n", *argv);
> @@ -101,6 +129,11 @@ static int fou_parse_opt(int argc, char **argv, struct nlmsghdr *n,
>  		return -1;
>  	}
>  
> +	if ((peer_port_set && !peer) || (peer && !peer_port_set)) {
> +		fprintf(stderr, "fou: both peer and peer port must be set\n");
> +		return -1;
> +	}
> +
>  	type = gue_set ? FOU_ENCAP_GUE : FOU_ENCAP_DIRECT;
>  
>  	addattr16(n, 1024, FOU_ATTR_PORT, port);
> @@ -110,6 +143,32 @@ static int fou_parse_opt(int argc, char **argv, struct nlmsghdr *n,
>  	if (ipproto_set)
>  		addattr8(n, 1024, FOU_ATTR_IPPROTO, ipproto);
>  
> +	if (local) {
> +		inet_prefix local_addr;
> +		__u8 attr_type = family == AF_INET ? FOU_ATTR_LOCAL_V4 :
> +						     FOU_ATTR_LOCAL_V6;
> +
> +		get_addr(&local_addr, local, family);
> +		addattr_l(n, 1024, attr_type, &local_addr.data,
> +			  local_addr.bytelen);
> +	}
> +
> +	if (peer) {
> +		inet_prefix peer_addr;
> +		__u8 attr_type = family == AF_INET ? FOU_ATTR_PEER_V4 :
> +						     FOU_ATTR_PEER_V6;
> +
> +		get_addr(&peer_addr, peer, family);
> +		addattr_l(n, 1024, attr_type, &peer_addr.data,
> +			  peer_addr.bytelen);
> +
> +		if (peer_port_set)
> +			addattr16(n, 1024, FOU_ATTR_PEER_PORT, peer_port);

what ensures that peer and local are valid IPv4 or IPv6 addresses as
determined by the family? yes, get_addr calls exit(), but it should not
- it should be returning 1 instead. Check the return code on get_addr
and handle the error.


> +	}
> +
> +	if (index_set)
> +		addattr32(n, 1024, FOU_ATTR_IFINDEX, index);
> +
>  	return 0;
>  }
>  
> @@ -142,6 +201,8 @@ static int print_fou_mapping(struct nlmsghdr *n, void *arg)
>  	struct genlmsghdr *ghdr;
>  	struct rtattr *tb[FOU_ATTR_MAX + 1];
>  	int len = n->nlmsg_len;
> +	__u8 family = AF_INET, local_attr_type, peer_attr_type, byte_len;
> +	__u8 empty_buf[16] = {0};

reverse xmas tree.


Powered by blists - more mailing lists