| 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
| ||
|
Message-ID: <66ee8de2744e9_35dcb129411@willemb.c.googlers.com.notmuch>
Date: Sat, 21 Sep 2024 05:12:02 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Tiago Lam <tiagolam@...udflare.com>,
"David S. Miller" <davem@...emloft.net>,
David Ahern <dsahern@...nel.org>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>,
Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...ichev.me>,
Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>,
Mykola Lysenko <mykolal@...com>,
Shuah Khan <shuah@...nel.org>
Cc: netdev@...r.kernel.org,
linux-kernel@...r.kernel.org,
bpf@...r.kernel.org,
linux-kselftest@...r.kernel.org,
Jakub Sitnicki <jakub@...udflare.com>,
Tiago Lam <tiagolam@...udflare.com>,
kernel-team@...udflare.com
Subject: Re: [RFC PATCH v2 1/3] ipv4: Support setting src port in sendmsg().
Tiago Lam wrote:
> sendmsg() doesn't currently allow users to set the src port from which
> egress traffic should be sent from. This is possible if a user wants to
> configure the src address from which egress traffic should be sent from
> - with the IP_PKTINFO ancillary message, a user is currently able to
> specify a source address to egress from when calling sendmsg().
> However, this still requires the user to set the IP_TRANSPARENT flag
> using setsockopt(), which happens to require special privileges in the
> case of IPv4.
>
> To support users setting the src port for egress traffic when using
> sendmsg(), this patch extends the ancillary messages supported by
> sendmsg() to support the IP_ORIGDSTADDR ancillary message, reusing the
> same cmsg and struct used in recvmsg() - which already supports
> specifying a port.
>
> Additionally, to avoid having to have special configurations, such as
> IP_TRANSPARENT, this patch allows egress traffic that's been configured
> using (the newly added) IP_ORIGDSTADDR to proceed if there's an ingress
> socket lookup (sk_lookup) that matches that traffic - by performing a
> reserve sk_lookup. Thus, if the sk_lookup reverse call returns a socket
> that matches the egress socket, we also let the egress traffic through -
> following the principle of, allowing return traffic to proceed if
> ingress traffic is allowed in. In case no match is found in the reverse
> sk_lookup, traffic falls back to the regular egress path.
>
> This reverse lookup is only performed in case an sk_lookup ebpf program
> is attached and the source address and/or port for the return traffic
> have been modified using the (newly added) IP_ORIGDSTADDR in sendmsg.
>
> Suggested-by: Jakub Sitnicki <jakub@...udflare.com>
> Signed-off-by: Tiago Lam <tiagolam@...udflare.com>
> ---
> include/net/ip.h | 1 +
> net/ipv4/ip_sockglue.c | 11 +++++++++++
> net/ipv4/udp.c | 35 ++++++++++++++++++++++++++++++++++-
> 3 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/ip.h b/include/net/ip.h
> index c5606cadb1a5..e5753abd7247 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -75,6 +75,7 @@ static inline unsigned int ip_hdrlen(const struct sk_buff *skb)
> struct ipcm_cookie {
> struct sockcm_cookie sockc;
> __be32 addr;
> + __be16 port;
> int oif;
> struct ip_options_rcu *opt;
> __u8 protocol;
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index cf377377b52d..6e55bd25b5f7 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -297,6 +297,17 @@ int ip_cmsg_send(struct sock *sk, struct msghdr *msg, struct ipcm_cookie *ipc,
> ipc->addr = info->ipi_spec_dst.s_addr;
> break;
> }
> + case IP_ORIGDSTADDR:
Should this just be IP_SRCADDR?
> + {
> + struct sockaddr_in *dst_addr;
> +
> + if (cmsg->cmsg_len != CMSG_LEN(sizeof(struct sockaddr_in)))
> + return -EINVAL;
> + dst_addr = (struct sockaddr_in *)CMSG_DATA(cmsg);
> + ipc->port = dst_addr->sin_port;
> + ipc->addr = dst_addr->sin_addr.s_addr;
> + break;
> + }
> case IP_TTL:
> if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)))
> return -EINVAL;
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 49c622e743e8..208cee40c0ec 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1060,6 +1060,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> DECLARE_SOCKADDR(struct sockaddr_in *, usin, msg->msg_name);
> struct flowi4 fl4_stack;
> struct flowi4 *fl4;
> + __u8 flow_flags = inet_sk_flowi_flags(sk);
> int ulen = len;
> struct ipcm_cookie ipc;
> struct rtable *rt = NULL;
> @@ -1179,6 +1180,39 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> }
> }
>
> + /* If we're egressing with a different source address and/or port, we
> + * perform a reverse socket lookup. The rationale behind this is that
> + * we can allow return UDP traffic that has ingressed through sk_lookup
> + * to also egress correctly. In case this the reverse lookup fails.
> + *
> + * The lookup is performed if either source address and/or port
> + * changed, and neither is "0".
> + */
> + if (static_branch_unlikely(&bpf_sk_lookup_enabled) &&
> + !connected &&
> + (ipc.port && ipc.addr) &&
> + (inet->inet_saddr != ipc.addr || inet->inet_sport != ipc.port)) {
> + struct sock *sk_egress;
> +
> + bpf_sk_lookup_run_v4(sock_net(sk), IPPROTO_UDP,
> + daddr, dport, ipc.addr, ntohs(ipc.port),
> + 1, &sk_egress);
Does this need to use a bpf helper rather than a normal route lookup
function?
I don't know this func, but the sk is returned without a reference
taken?
> + if (IS_ERR_OR_NULL(sk_egress) || sk_egress != sk) {
> + net_info_ratelimited("No reverse socket lookup match for local addr %pI4:%d remote addr %pI4:%d\n",
> + &ipc.addr, ntohs(ipc.port), &daddr,
> + ntohs(dport));
No need for logging to the kernel log when syscalls can just return an
error.
> + } else {
> + /* Override the source port to use with the one we got
> + * in cmsg, and tell routing to let us use a non-local
> + * address. Otherwise route lookups will fail with
> + * non-local source address when IP_TRANSPARENT isn't
> + * set.
> + */
> + inet->inet_sport = ipc.port;
> + flow_flags |= FLOWI_FLAG_ANYSRC;
> + }
> + }
> +
> saddr = ipc.addr;
> ipc.addr = faddr = daddr;
>
> @@ -1223,7 +1257,6 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>
> if (!rt) {
> struct net *net = sock_net(sk);
> - __u8 flow_flags = inet_sk_flowi_flags(sk);
>
> fl4 = &fl4_stack;
>
>
> --
> 2.34.1
>
Powered by blists - more mailing lists