netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next v1] net/ipv6: allow any source address for sendmsg pktinfo with ip_nonlocal_bind
@ 2018-07-25 11:19 Vincent Bernat
  2018-07-29 19:28 ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Vincent Bernat @ 2018-07-25 11:19 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	Tom Herbert
  Cc: Vincent Bernat

When freebind feature is set of an IPv6 socket, any source address can
be used when sending UDP datagrams using IPv6 PKTINFO ancillary
message. Global non-local bind feature was added in commit
35a256fee52c ("ipv6: Nonlocal bind") for IPv6. This commit also allows
IPv6 source address spoofing when non-local bind feature is enabled.

Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
 net/ipv6/datagram.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 201306b9b5ea..c46936563b15 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -800,7 +800,8 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 
 			if (addr_type != IPV6_ADDR_ANY) {
 				int strict = __ipv6_addr_src_scope(addr_type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
-				if (!(inet_sk(sk)->freebind || inet_sk(sk)->transparent) &&
+				if (!(net->ipv6.sysctl.ip_nonlocal_bind ||
+				      inet_sk(sk)->freebind || inet_sk(sk)->transparent) &&
 				    !ipv6_chk_addr_and_flags(net, &src_info->ipi6_addr,
 							     dev, !strict, 0,
 							     IFA_F_TENTATIVE) &&
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [net-next v1] net/ipv6: allow any source address for sendmsg pktinfo with ip_nonlocal_bind
  2018-07-25 11:19 [net-next v1] net/ipv6: allow any source address for sendmsg pktinfo with ip_nonlocal_bind Vincent Bernat
@ 2018-07-29 19:28 ` David Miller
  2018-07-30  6:08   ` Vincent Bernat
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2018-07-29 19:28 UTC (permalink / raw)
  To: vincent; +Cc: kuznet, yoshfuji, netdev, tom

From: Vincent Bernat <vincent@bernat.im>
Date: Wed, 25 Jul 2018 13:19:13 +0200

> When freebind feature is set of an IPv6 socket, any source address can
> be used when sending UDP datagrams using IPv6 PKTINFO ancillary
> message. Global non-local bind feature was added in commit
> 35a256fee52c ("ipv6: Nonlocal bind") for IPv6. This commit also allows
> IPv6 source address spoofing when non-local bind feature is enabled.
> 
> Signed-off-by: Vincent Bernat <vincent@bernat.im>

This definitely seems to make sense.  And is consistent with the other
tests involving freebind and transparent.

This test involving ip_nonlocal_bind, freeebind, and transparent happens
in several locations.  Perhaps we should add a helper function for this?

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [net-next v1] net/ipv6: allow any source address for sendmsg pktinfo with ip_nonlocal_bind
  2018-07-29 19:28 ` David Miller
@ 2018-07-30  6:08   ` Vincent Bernat
  2018-07-30 16:08     ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Vincent Bernat @ 2018-07-30  6:08 UTC (permalink / raw)
  To: David Miller; +Cc: kuznet, yoshfuji, netdev, tom

 ❦ 29 juillet 2018 12:28 -0700, David Miller <davem@davemloft.net> :

>> When freebind feature is set of an IPv6 socket, any source address can
>> be used when sending UDP datagrams using IPv6 PKTINFO ancillary
>> message. Global non-local bind feature was added in commit
>> 35a256fee52c ("ipv6: Nonlocal bind") for IPv6. This commit also allows
>> IPv6 source address spoofing when non-local bind feature is enabled.
>> 
>> Signed-off-by: Vincent Bernat <vincent@bernat.im>
>
> This definitely seems to make sense.  And is consistent with the other
> tests involving freebind and transparent.
>
> This test involving ip_nonlocal_bind, freeebind, and transparent happens
> in several locations.  Perhaps we should add a helper function for
> this?

Yes, I can do that. Should I also include one for SCTP?
-- 
"Elves and Dragons!" I says to him.  "Cabbages and potatoes are better
for you and me."
		-- J. R. R. Tolkien

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [net-next v1] net/ipv6: allow any source address for sendmsg pktinfo with ip_nonlocal_bind
  2018-07-30  6:08   ` Vincent Bernat
@ 2018-07-30 16:08     ` David Miller
  2018-07-31 19:18       ` [PATCH net-next v1] net: add helpers checking if socket can be bound to nonlocal address Vincent Bernat
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2018-07-30 16:08 UTC (permalink / raw)
  To: vincent; +Cc: kuznet, yoshfuji, netdev, tom

From: Vincent Bernat <vincent@bernat.im>
Date: Mon, 30 Jul 2018 08:08:12 +0200

>  ❦ 29 juillet 2018 12:28 -0700, David Miller <davem@davemloft.net> :
> 
>>> When freebind feature is set of an IPv6 socket, any source address can
>>> be used when sending UDP datagrams using IPv6 PKTINFO ancillary
>>> message. Global non-local bind feature was added in commit
>>> 35a256fee52c ("ipv6: Nonlocal bind") for IPv6. This commit also allows
>>> IPv6 source address spoofing when non-local bind feature is enabled.
>>> 
>>> Signed-off-by: Vincent Bernat <vincent@bernat.im>
>>
>> This definitely seems to make sense.  And is consistent with the other
>> tests involving freebind and transparent.
>>
>> This test involving ip_nonlocal_bind, freeebind, and transparent happens
>> in several locations.  Perhaps we should add a helper function for
>> this?
> 
> Yes, I can do that. Should I also include one for SCTP?

If the helper for SCTP needs to be different and thus will only be
used in one place, probably not.  The whole idea is to remove
duplicated code, and prevent someone in the future from forgetting
to test all three values in these situations.

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net-next v1] net: add helpers checking if socket can be bound to nonlocal address
  2018-07-30 16:08     ` David Miller
@ 2018-07-31 19:18       ` Vincent Bernat
  2018-08-01 16:50         ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Vincent Bernat @ 2018-07-31 19:18 UTC (permalink / raw)
  To: David Miller, kuznet, yoshfuji, netdev, tom; +Cc: Vincent Bernat

The construction "net->ipv4.sysctl_ip_nonlocal_bind || inet->freebind
|| inet->transparent" is present three times and its IPv6 counterpart
is also present three times. We introduce two small helpers to
characterize these tests uniformly.

Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
 include/net/inet_sock.h | 8 ++++++++
 include/net/ipv6.h      | 7 +++++++
 net/ipv4/af_inet.c      | 3 +--
 net/ipv4/ping.c         | 6 ++----
 net/ipv6/af_inet6.c     | 6 ++----
 net/ipv6/datagram.c     | 3 +--
 6 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 314be484c696..e03b93360f33 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -359,4 +359,12 @@ static inline bool inet_get_convert_csum(struct sock *sk)
 	return !!inet_sk(sk)->convert_csum;
 }
 
+
+static inline bool inet_can_nonlocal_bind(struct net *net,
+					  struct inet_sock *inet)
+{
+	return net->ipv4.sysctl_ip_nonlocal_bind ||
+		inet->freebind || inet->transparent;
+}
+
 #endif	/* _INET_SOCK_H */
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index a44509f4e985..82deb684ba73 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -766,6 +766,13 @@ static inline int ip6_sk_dst_hoplimit(struct ipv6_pinfo *np, struct flowi6 *fl6,
 	return hlimit;
 }
 
+static inline bool ipv6_can_nonlocal_bind(struct net *net,
+					  struct inet_sock *inet)
+{
+	return net->ipv6.sysctl.ip_nonlocal_bind ||
+		inet->freebind || inet->transparent;
+}
+
 /* copy IPv6 saddr & daddr to flow_keys, possibly using 64bit load/store
  * Equivalent to :	flow->v6addrs.src = iph->saddr;
  *			flow->v6addrs.dst = iph->daddr;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index f2a0a3bab6b5..ee707b91d1a7 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -486,8 +486,7 @@ int __inet_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
 	 *  is temporarily down)
 	 */
 	err = -EADDRNOTAVAIL;
-	if (!net->ipv4.sysctl_ip_nonlocal_bind &&
-	    !(inet->freebind || inet->transparent) &&
+	if (!inet_can_nonlocal_bind(net, inet) &&
 	    addr->sin_addr.s_addr != htonl(INADDR_ANY) &&
 	    chk_addr_ret != RTN_LOCAL &&
 	    chk_addr_ret != RTN_MULTICAST &&
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index b54c964ad925..8d7aaf118a30 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -320,8 +320,7 @@ static int ping_check_bind_addr(struct sock *sk, struct inet_sock *isk,
 		if (addr->sin_addr.s_addr == htonl(INADDR_ANY))
 			chk_addr_ret = RTN_LOCAL;
 
-		if ((net->ipv4.sysctl_ip_nonlocal_bind == 0 &&
-		    isk->freebind == 0 && isk->transparent == 0 &&
+		if ((!inet_can_nonlocal_bind(net, isk) &&
 		     chk_addr_ret != RTN_LOCAL) ||
 		    chk_addr_ret == RTN_MULTICAST ||
 		    chk_addr_ret == RTN_BROADCAST)
@@ -361,8 +360,7 @@ static int ping_check_bind_addr(struct sock *sk, struct inet_sock *isk,
 						    scoped);
 		rcu_read_unlock();
 
-		if (!(net->ipv6.sysctl.ip_nonlocal_bind ||
-		      isk->freebind || isk->transparent || has_addr ||
+		if (!(ipv6_can_nonlocal_bind(net, isk) || has_addr ||
 		      addr_type == IPV6_ADDR_ANY))
 			return -EADDRNOTAVAIL;
 
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index c9535354149f..020f6e14a7af 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -322,8 +322,7 @@ static int __inet6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
 		/* Reproduce AF_INET checks to make the bindings consistent */
 		v4addr = addr->sin6_addr.s6_addr32[3];
 		chk_addr_ret = inet_addr_type(net, v4addr);
-		if (!net->ipv4.sysctl_ip_nonlocal_bind &&
-		    !(inet->freebind || inet->transparent) &&
+		if (!inet_can_nonlocal_bind(net, inet) &&
 		    v4addr != htonl(INADDR_ANY) &&
 		    chk_addr_ret != RTN_LOCAL &&
 		    chk_addr_ret != RTN_MULTICAST &&
@@ -362,8 +361,7 @@ static int __inet6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
 			 */
 			v4addr = LOOPBACK4_IPV6;
 			if (!(addr_type & IPV6_ADDR_MULTICAST))	{
-				if (!net->ipv6.sysctl.ip_nonlocal_bind &&
-				    !(inet->freebind || inet->transparent) &&
+				if (!ipv6_can_nonlocal_bind(net, inet) &&
 				    !ipv6_chk_addr(net, &addr->sin6_addr,
 						   dev, 0)) {
 					err = -EADDRNOTAVAIL;
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index c46936563b15..b0123af1492d 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -800,8 +800,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 
 			if (addr_type != IPV6_ADDR_ANY) {
 				int strict = __ipv6_addr_src_scope(addr_type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
-				if (!(net->ipv6.sysctl.ip_nonlocal_bind ||
-				      inet_sk(sk)->freebind || inet_sk(sk)->transparent) &&
+				if (!ipv6_can_nonlocal_bind(net, inet_sk(sk)) &&
 				    !ipv6_chk_addr_and_flags(net, &src_info->ipi6_addr,
 							     dev, !strict, 0,
 							     IFA_F_TENTATIVE) &&
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v1] net: add helpers checking if socket can be bound to nonlocal address
  2018-07-31 19:18       ` [PATCH net-next v1] net: add helpers checking if socket can be bound to nonlocal address Vincent Bernat
@ 2018-08-01 16:50         ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-08-01 16:50 UTC (permalink / raw)
  To: vincent; +Cc: kuznet, yoshfuji, netdev, tom

From: Vincent Bernat <vincent@bernat.im>
Date: Tue, 31 Jul 2018 21:18:11 +0200

> The construction "net->ipv4.sysctl_ip_nonlocal_bind || inet->freebind
> || inet->transparent" is present three times and its IPv6 counterpart
> is also present three times. We introduce two small helpers to
> characterize these tests uniformly.
> 
> Signed-off-by: Vincent Bernat <vincent@bernat.im>

Looks great, thanks for doing this.

Applied.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-08-01 19:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-25 11:19 [net-next v1] net/ipv6: allow any source address for sendmsg pktinfo with ip_nonlocal_bind Vincent Bernat
2018-07-29 19:28 ` David Miller
2018-07-30  6:08   ` Vincent Bernat
2018-07-30 16:08     ` David Miller
2018-07-31 19:18       ` [PATCH net-next v1] net: add helpers checking if socket can be bound to nonlocal address Vincent Bernat
2018-08-01 16:50         ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).