| 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
| ||
|
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