Skip to contentSkip to conversation

do not set IPv6-only options on IPv4 addresses

PATCH Superseded Andrea Claudi 3 participants 7 messages Jun 24, 2019

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover do not set IPv6-only options on IPv4 addresses

'home', 'nodad' and 'mngtmpaddr' options are IPv6-only, but it is possible to set them on IPv4 addresses, too. This should not be possible.

Fix this adding a check on the protocol family before setting the flags, and exiting with invarg() on error.

Andrea Claudi (3):
ip address: do not set nodad option for IPv4 addresses ip address: do not set home option for IPv4 addresses ip address: do not set mngtmpaddr option for IPv4 addresses

 ip/ipaddress.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
Signature · 1 line
2.20.1
#1 lore raw

PATCH 1/3 ip address: do not set nodad option for IPv4 addresses

Duplicate Address Detection (RFC 4862) is available only for IPv6 addresses. As a consequence, 'nodad' option, turning it off, should be available only for IPv6, and is defined like that in the man page.

However it is possible to set nodad on IPv4 addresses, too:

$ ip link add dummy0 type dummy
$ ip -4 addr add 192.168.1.1 dev dummy0 nodad
$ ip a
1: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default qlen 1000
   link/ether 1a:6d:c6:96:ca:f8 brd ff:ff:ff:ff:ff:ff
   inet 192.168.1.1/32 scope global nodad dummy0
      valid_lft forever preferred_lft forever

Fix this adding a check on the protocol family before setting IFA_F_NODAD flag.

Fixes bac735c53a36d ("enabled to manipulate the flags of IFA_F_HOMEADDRESS or IFA_F_NODAD from ip.")
---
 ip/ipaddress.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
ip/ipaddress.c +4 -1
index 47e5be7462fe7..38356cc929e7b 100644
@@ -2250,7 +2250,10 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 		} else if (strcmp(*argv, "home") == 0) {
 			ifa_flags |= IFA_F_HOMEADDRESS;
 		} else if (strcmp(*argv, "nodad") == 0) {
-			ifa_flags |= IFA_F_NODAD;
+			if (req.ifa.ifa_family == AF_INET6)
+				ifa_flags |= IFA_F_NODAD;
+			else
+				invarg("nodad option can be set only for IPv6 addresses\n", *argv);
 		} else if (strcmp(*argv, "mngtmpaddr") == 0) {
 			ifa_flags |= IFA_F_MANAGETEMPADDR;
 		} else if (strcmp(*argv, "noprefixroute") == 0) {
Signature · 1 line
2.20.1
#3 lore raw

PATCH 3/3 ip address: do not set mngtmpaddr option for IPv4 addresses

In reply to Andrea Claudi

'mngtmpaddr' option make the kernel manage temporary addresses created from the specified one as template on behalf of Privacy Extensions (RFC3041). This option should be available only for IPv6 addresses, as correctly stated in the manpage.

However it is possible to set mngtmpaddr on IPv4 addresses, too:

$ ip link add dummy0 type dummy
$ ip -4 addr add 192.168.1.1 dev dummy0 mngtmpaddr
$ ip a
1: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default qlen 1000
   link/ether 1a:6d:c6:96:ca:f8 brd ff:ff:ff:ff:ff:ff
   inet 192.168.1.1/32 scope global mngtmpaddr dummy0
      valid_lft forever preferred_lft forever

Fix this adding a check on the protocol family before setting IFA_F_MANAGETEMPADDR flag.

Fixes 5b7e21c417bea ("add support for IFA_F_MANAGETEMPADDR")
---
 ip/ipaddress.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
ip/ipaddress.c +4 -1
index 0f59e0a40468c..06a9f904201c0 100644
@@ -2258,7 +2258,10 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 			else
 				invarg("nodad option can be set only for IPv6 addresses\n", *argv);
 		} else if (strcmp(*argv, "mngtmpaddr") == 0) {
-			ifa_flags |= IFA_F_MANAGETEMPADDR;
+			if (req.ifa.ifa_family == AF_INET6)
+				ifa_flags |= IFA_F_MANAGETEMPADDR;
+			else
+				invarg("mngtmpaddr option can be set only for IPv6 addresses\n", *argv);
 		} else if (strcmp(*argv, "noprefixroute") == 0) {
 			ifa_flags |= IFA_F_NOPREFIXROUTE;
 		} else if (strcmp(*argv, "autojoin") == 0) {
Signature · 1 line
2.20.1
#2 lore raw

PATCH 2/3 ip address: do not set home option for IPv4 addresses

In reply to Andrea Claudi

'home' option designates a IPv6 address as "home address" as defined in RFC 6275. This option should be available only for IPv6 addresses, as correctly stated in the manpage.

However it is possible to set home on IPv4 addresses, too:

$ ip link add dummy0 type dummy
$ ip -4 addr add 192.168.1.1 dev dummy0 home
$ ip a
1: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default qlen 1000
   link/ether 1a:6d:c6:96:ca:f8 brd ff:ff:ff:ff:ff:ff
   inet 192.168.1.1/32 scope global home dummy0
      valid_lft forever preferred_lft forever

Fix this adding a check on the protocol family before setting IFA_F_HOMEADDRESS flag.

Fixes bac735c53a36d ("enabled to manipulate the flags of IFA_F_HOMEADDRESS or IFA_F_NODAD from ip.")
---
 ip/ipaddress.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
ip/ipaddress.c +4 -1
index 38356cc929e7b..0f59e0a40468c 100644
@@ -2248,7 +2248,10 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 			if (set_lifetime(&preferred_lft, *argv))
 				invarg("preferred_lft value", *argv);
 		} else if (strcmp(*argv, "home") == 0) {
-			ifa_flags |= IFA_F_HOMEADDRESS;
+			if (req.ifa.ifa_family == AF_INET6)
+				ifa_flags |= IFA_F_HOMEADDRESS;
+			else
+				invarg("home option can be set only for IPv6 addresses\n", *argv);
 		} else if (strcmp(*argv, "nodad") == 0) {
 			if (req.ifa.ifa_family == AF_INET6)
 				ifa_flags |= IFA_F_NODAD;
Signature · 1 line
2.20.1
#4 lore raw
In reply to Andrea Claudi
Quoted 7 lines · On Mon, 24 Jun 2019 19:05:52 +0200 Andrea Claudi <[email protected]> wrote:

'home', 'nodad' and 'mngtmpaddr' options are IPv6-only, but it is possible to set them on IPv4 addresses, too. This should not be possible.

Fix this adding a check on the protocol family before setting the flags, and exiting with invarg() on error.

Quoted 8 lines

Andrea Claudi (3):
ip address: do not set nodad option for IPv4 addresses ip address: do not set home option for IPv4 addresses ip address: do not set mngtmpaddr option for IPv4 addresses

 ip/ipaddress.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Maybe this should be a warning, not a failure.
A little concerned that there will be some user with a scripted setup that this breaks.

#5 lore raw
Quoted 16 lines · Stephen Hemminger
Quoted 12 lines · On Mon, 24 Jun 2019 19:05:52 +0200 Andrea Claudi <[email protected]> wrote:

'home', 'nodad' and 'mngtmpaddr' options are IPv6-only, but it is possible to set them on IPv4 addresses, too. This should not be possible.

Fix this adding a check on the protocol family before setting the flags, and exiting with invarg() on error.

Andrea Claudi (3):
ip address: do not set nodad option for IPv4 addresses ip address: do not set home option for IPv4 addresses ip address: do not set mngtmpaddr option for IPv4 addresses

Quoted 7 lines
Quoted 3 lines
 ip/ipaddress.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Maybe this should be a warning, not a failure.
A little concerned that there will be some user with a scripted setup that this breaks.

Hi Stephen,
I think that if a script wrongly uses some of these flags on a IPv4 address, it most probably operates on an unexpected address, since everyone is aware that these flags are IPv6 only. In other words we are breaking a scripted setup that is already broken.
In this case it's probably worth exiting with error and give the author the chance to fix the script, otherwise the error can go unnoticed.

If you prefer, I can send a v2 with warnings instead of errors, just let me know.

Regards,
Andrea

#6 lore raw
Quoted 10 lines · Andrea Claudi

I think that if a script wrongly uses some of these flags on a IPv4 address, it most probably operates on an unexpected address, since everyone is aware that these flags are IPv6 only. In other words we are breaking a scripted setup that is already broken.
In this case it's probably worth exiting with error and give the author the chance to fix the script, otherwise the error can go unnoticed.

If you prefer, I can send a v2 with warnings instead of errors, just let me know.

Recent changes for strict mode have shown people do interesting things with scripts and like the silent "ignores". :-(