ipv6: do not disable temp_address when reaching max_address

PATCH In review Ding Tianhong 2 participants 9 messages Aug 14, 2013 – Aug 16, 2013

Subsystems (3)

Networking Network drivers IPv4/IPv6

lore

Share & tools

lore

Ding Tianhong

author

2013-08-14 09:07 UTC #0 lore raw

PATCH v4 1/3 ipv6: do not disable temp_address when reaching max_address

A LAN user can remotely disable temporary address which may lead to privacy violatins and information disclosure.

The reason is that the linux kernel uses the 'ipv6.max_addresses' option to specify how many ipv6 addresses and interface may have. The 'ipv6.regen_max_retry' (default value 3) option specifies how many times the kernel will try to create a new address.

But the kernel is not distinguish between the event of reaching max_addresses for an interface and failing to generate a new address. the kernel disable the temporary address after regenerate a new address 'regen_max_retry' times.

According RFC4941 3.3.7:

---------------------------------------

If DAD indicates the address is already in use,
the node must generate a new randomized interface
identifier as described in section 3.2 above, and
repeat the previous steps as appropriate up to
TEMP_IDGEN_RETRIES times.

If after TEMP_IDGEN_RETRIES consecutive attempts no non-unique address was generated, the node must log a system error and must not attempt to generate temporary address for that interface.

------------------------------------------

RFC4941 3.3.7 specifies that disabling the temp_address must happen upon the address is already in use, not reach the max_address, So we have to check the return err and distinguish the correct retry path.

This fixes CVE-2013-0343

Signed-off-by Ding Tianhong <[email protected]>

Tested-by Wang Weidong <[email protected]>

Cc David S. Miller <[email protected]>

Cc Sergei Shtylyov <[email protected]>

Cc Eric Dumazet <[email protected]>

---
 net/ipv6/addrconf.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

net/ipv6/addrconf.c +22 -4

index da4241c..7b55464 100644
@@ -1134,10 +1134,28 @@ retry:
 	if (IS_ERR_OR_NULL(ift)) {
 		in6_ifa_put(ifp);
 		in6_dev_put(idev);
-		pr_info("%s: retry temporary address regeneration\n", __func__);
-		tmpaddr = &addr;
-		write_lock(&idev->lock);
-		goto retry;
+
+		/* According RFC4941 3.3.7:
+		 * If DAD indicates the address is already in use,
+		 * the node must generate a new randomized interface
+		 * identifier as described in section 3.2 above, and
+		 * repeat the previous steps as appropriate up to
+		 * TEMP_IDGEN_RETRIES times.
+		 * If after TEMP_IDGEN_RETRIES consecutive attempts no
+		 * non-unique address was generated, the node must log
+		 * a system error and must not attempt to generate
+		 * temporary address for that interface.
+		 * So we have to check the return err and distinguish
+		 * the correct retry path.
+		 */
+		if (PTR_ERR(ift) == -EEXIST) {
+			pr_info("%s: retry temporary address regeneration\n", __func__);
+			tmpaddr = &addr;
+			write_lock(&idev->lock);
+			goto retry;
+		}
+		/* do not retry if the err code is not -EEXIST */
+		goto out;
 	}
 
 	spin_lock_bh(&ift->lock);

Signature · 1 line

1.8.2.1

Hannes Frederic Sowa

2013-08-14 10:15 UTC #1 lore raw

Quoted 28 lines · Ding Tianhong

A LAN user can remotely disable temporary address which may lead to privacy violatins and information disclosure.
The reason is that the linux kernel uses the 'ipv6.max_addresses' option to specify how many ipv6 addresses and interface may have. The 'ipv6.regen_max_retry' (default value 3) option specifies how many times the kernel will try to create a new address.
But the kernel is not distinguish between the event of reaching max_addresses for an interface and failing to generate a new address. the kernel disable the temporary address after regenerate a new address 'regen_max_retry' times.
According RFC4941 3.3.7:
---------------------------------------
If DAD indicates the address is already in use,
the node must generate a new randomized interface
identifier as described in section 3.2 above, and
repeat the previous steps as appropriate up to
TEMP_IDGEN_RETRIES times.
If after TEMP_IDGEN_RETRIES consecutive attempts no non-unique address was generated, the node must log a system error and must not attempt to generate temporary address for that interface.

Quoted 7 lines

------------------------------------------
RFC4941 3.3.7 specifies that disabling the temp_address must happen upon the address is already in use, not reach the max_address, So we have to check the return err and distinguish the correct retry path.
This fixes CVE-2013-0343

I don't think this patch fixes CVE-2013-0343.

Quoted 27 lines

Signed-off-by Ding Tianhong <[email protected]>

Tested-by Wang Weidong <[email protected]>

Cc David S. Miller <[email protected]>

Cc Sergei Shtylyov <[email protected]>

Cc Eric Dumazet <[email protected]>

---
 net/ipv6/addrconf.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

net/ipv6/addrconf.c +5 -4

index da4241c..7b55464 100644
@@ -1134,10 +1134,28 @@ retry:
 	if (IS_ERR_OR_NULL(ift)) {
 		in6_ifa_put(ifp);
 		in6_dev_put(idev);
-		pr_info("%s: retry temporary address regeneration\n", __func__);
-		tmpaddr = &addr;
-		write_lock(&idev->lock);
-		goto retry;
+
+		/* According RFC4941 3.3.7:
+		 * If DAD indicates the address is already in use,
+		 * the node must generate a new randomized interface
+		 * identifier as described in section 3.2 above, and

net/ipv6/addrconf.c retry: · line 1151

Quoted 10 lines

net/ipv6/addrconf.c +10 -0

+		 * repeat the previous steps as appropriate up to
+		 * TEMP_IDGEN_RETRIES times.
+		 * If after TEMP_IDGEN_RETRIES consecutive attempts no
+		 * non-unique address was generated, the node must log
+		 * a system error and must not attempt to generate
+		 * temporary address for that interface.
+		 * So we have to check the return err and distinguish
+		 * the correct retry path.
+		 */
+		if (PTR_ERR(ift) == -EEXIST) {

-EEXIST is not the same as "ipv6 address is is already used on the subnet". I really don't see the point here. IMHO this breaks the intended regeneration logic.

I fear a fix of CVE-2013-0343 will be a bit more complicated. ;) I give it a thought.

Greetings,

Hannes

Ding Tianhong

author

2013-08-15 00:43 UTC #2 lore raw

Quoted 81 lines · Hannes Frederic Sowa

Quoted 28 lines · Ding Tianhong
A LAN user can remotely disable temporary address which may lead to privacy violatins and information disclosure.
The reason is that the linux kernel uses the 'ipv6.max_addresses' option to specify how many ipv6 addresses and interface may have. The 'ipv6.regen_max_retry' (default value 3) option specifies how many times the kernel will try to create a new address.
But the kernel is not distinguish between the event of reaching max_addresses for an interface and failing to generate a new address. the kernel disable the temporary address after regenerate a new address 'regen_max_retry' times.
According RFC4941 3.3.7:
---------------------------------------
If DAD indicates the address is already in use,
the node must generate a new randomized interface
identifier as described in section 3.2 above, and
repeat the previous steps as appropriate up to
TEMP_IDGEN_RETRIES times.
If after TEMP_IDGEN_RETRIES consecutive attempts no non-unique address was generated, the node must log a system error and must not attempt to generate temporary address for that interface.
Quoted 7 lines
------------------------------------------
RFC4941 3.3.7 specifies that disabling the temp_address must happen upon the address is already in use, not reach the max_address, So we have to check the return err and distinguish the correct retry path.
This fixes CVE-2013-0343
I don't think this patch fixes CVE-2013-0343.
Quoted 27 lines
Signed-off-by Ding Tianhong <[email protected]>
Tested-by Wang Weidong <[email protected]>
Cc David S. Miller <[email protected]>
Cc Sergei Shtylyov <[email protected]>
Cc Eric Dumazet <[email protected]>
---
 net/ipv6/addrconf.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)
net/ipv6/addrconf.c +5 -4
index da4241c..7b55464 100644
@@ -1134,10 +1134,28 @@ retry:
 	if (IS_ERR_OR_NULL(ift)) {
 		in6_ifa_put(ifp);
 		in6_dev_put(idev);
-		pr_info("%s: retry temporary address regeneration\n", __func__);
-		tmpaddr = &addr;
-		write_lock(&idev->lock);
-		goto retry;
+
+		/* According RFC4941 3.3.7:
+		 * If DAD indicates the address is already in use,
+		 * the node must generate a new randomized interface
+		 * identifier as described in section 3.2 above, and
net/ipv6/addrconf.c retry: · line 1151
Quoted 10 lines
net/ipv6/addrconf.c +10 -0
+		 * repeat the previous steps as appropriate up to
+		 * TEMP_IDGEN_RETRIES times.
+		 * If after TEMP_IDGEN_RETRIES consecutive attempts no
+		 * non-unique address was generated, the node must log
+		 * a system error and must not attempt to generate
+		 * temporary address for that interface.
+		 * So we have to check the return err and distinguish
+		 * the correct retry path.
+		 */
+		if (PTR_ERR(ift) == -EEXIST) {
-EEXIST is not the same as "ipv6 address is is already used on the subnet". I really don't see the point here. IMHO this breaks the intended regeneration logic.

Quoted 8 lines

I fear a fix of CVE-2013-0343 will be a bit more complicated. ;) I give it a thought.
Greetings,
Hannes

ok, thanks for your feedback, I'll waiting you for more information to fix the problem.:)

Quoted 2 lines

.

Hannes Frederic Sowa

2013-08-15 01:24 UTC #3 lore raw

Quoted 85 lines · Ding Tianhong

Quoted 84 lines · Hannes Frederic Sowa
Quoted 28 lines · Ding Tianhong
A LAN user can remotely disable temporary address which may lead to privacy violatins and information disclosure.
The reason is that the linux kernel uses the 'ipv6.max_addresses' option to specify how many ipv6 addresses and interface may have. The 'ipv6.regen_max_retry' (default value 3) option specifies how many times the kernel will try to create a new address.
But the kernel is not distinguish between the event of reaching max_addresses for an interface and failing to generate a new address. the kernel disable the temporary address after regenerate a new address 'regen_max_retry' times.
According RFC4941 3.3.7:
---------------------------------------
If DAD indicates the address is already in use,
the node must generate a new randomized interface
identifier as described in section 3.2 above, and
repeat the previous steps as appropriate up to
TEMP_IDGEN_RETRIES times.
If after TEMP_IDGEN_RETRIES consecutive attempts no non-unique address was generated, the node must log a system error and must not attempt to generate temporary address for that interface.
Quoted 7 lines
------------------------------------------
RFC4941 3.3.7 specifies that disabling the temp_address must happen upon the address is already in use, not reach the max_address, So we have to check the return err and distinguish the correct retry path.
This fixes CVE-2013-0343
I don't think this patch fixes CVE-2013-0343.
Quoted 27 lines
Signed-off-by Ding Tianhong <[email protected]>
Tested-by Wang Weidong <[email protected]>
Cc David S. Miller <[email protected]>
Cc Sergei Shtylyov <[email protected]>
Cc Eric Dumazet <[email protected]>
---
 net/ipv6/addrconf.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)
net/ipv6/addrconf.c +5 -4
index da4241c..7b55464 100644
@@ -1134,10 +1134,28 @@ retry:
 	if (IS_ERR_OR_NULL(ift)) {
 		in6_ifa_put(ifp);
 		in6_dev_put(idev);
-		pr_info("%s: retry temporary address regeneration\n", __func__);
-		tmpaddr = &addr;
-		write_lock(&idev->lock);
-		goto retry;
+
+		/* According RFC4941 3.3.7:
+		 * If DAD indicates the address is already in use,
+		 * the node must generate a new randomized interface
+		 * identifier as described in section 3.2 above, and
net/ipv6/addrconf.c retry: · line 1151
Quoted 10 lines
net/ipv6/addrconf.c +10 -0
+		 * repeat the previous steps as appropriate up to
+		 * TEMP_IDGEN_RETRIES times.
+		 * If after TEMP_IDGEN_RETRIES consecutive attempts no
+		 * non-unique address was generated, the node must log
+		 * a system error and must not attempt to generate
+		 * temporary address for that interface.
+		 * So we have to check the return err and distinguish
+		 * the correct retry path.
+		 */
+		if (PTR_ERR(ift) == -EEXIST) {
-EEXIST is not the same as "ipv6 address is is already used on the subnet". I really don't see the point here. IMHO this breaks the intended regeneration logic.
I fear a fix of CVE-2013-0343 will be a bit more complicated. ;) I give it a thought.

Quoted 6 lines

Quoted 5 lines
Greetings,
Hannes
ok, thanks for your feedback, I'll waiting you for more information to fix the problem.:)

[added George Kargiotakis and P J P to Cc and full quote]

I wonder if the easiest solution would be to just drop the max_addresses limit from ipv6_create_tempaddr. max_addresses protects the kernel from installing an unlimited amount of addresses on an interface which gets flooded by RAs. Because we have a direct relation between interface address to temp address, I don't see that we would create the possiblity of DoS.

Sure, an audit and testing is needed.

Greetings,

Hannes

Ding Tianhong

author

2013-08-15 02:17 UTC #4 lore raw

Quoted 101 lines · Hannes Frederic Sowa

Quoted 85 lines · Ding Tianhong
Quoted 84 lines · Hannes Frederic Sowa
Quoted 28 lines · Ding Tianhong
A LAN user can remotely disable temporary address which may lead to privacy violatins and information disclosure.
The reason is that the linux kernel uses the 'ipv6.max_addresses' option to specify how many ipv6 addresses and interface may have. The 'ipv6.regen_max_retry' (default value 3) option specifies how many times the kernel will try to create a new address.
But the kernel is not distinguish between the event of reaching max_addresses for an interface and failing to generate a new address. the kernel disable the temporary address after regenerate a new address 'regen_max_retry' times.
According RFC4941 3.3.7:
---------------------------------------
If DAD indicates the address is already in use,
the node must generate a new randomized interface
identifier as described in section 3.2 above, and
repeat the previous steps as appropriate up to
TEMP_IDGEN_RETRIES times.
If after TEMP_IDGEN_RETRIES consecutive attempts no non-unique address was generated, the node must log a system error and must not attempt to generate temporary address for that interface.
Quoted 7 lines
------------------------------------------
RFC4941 3.3.7 specifies that disabling the temp_address must happen upon the address is already in use, not reach the max_address, So we have to check the return err and distinguish the correct retry path.
This fixes CVE-2013-0343
I don't think this patch fixes CVE-2013-0343.
Quoted 27 lines
Signed-off-by Ding Tianhong <[email protected]>
Tested-by Wang Weidong <[email protected]>
Cc David S. Miller <[email protected]>
Cc Sergei Shtylyov <[email protected]>
Cc Eric Dumazet <[email protected]>
---
 net/ipv6/addrconf.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)
net/ipv6/addrconf.c +5 -4
index da4241c..7b55464 100644
@@ -1134,10 +1134,28 @@ retry:
 	if (IS_ERR_OR_NULL(ift)) {
 		in6_ifa_put(ifp);
 		in6_dev_put(idev);
-		pr_info("%s: retry temporary address regeneration\n", __func__);
-		tmpaddr = &addr;
-		write_lock(&idev->lock);
-		goto retry;
+
+		/* According RFC4941 3.3.7:
+		 * If DAD indicates the address is already in use,
+		 * the node must generate a new randomized interface
+		 * identifier as described in section 3.2 above, and
net/ipv6/addrconf.c retry: · line 1151
Quoted 10 lines
net/ipv6/addrconf.c +10 -0
+		 * repeat the previous steps as appropriate up to
+		 * TEMP_IDGEN_RETRIES times.
+		 * If after TEMP_IDGEN_RETRIES consecutive attempts no
+		 * non-unique address was generated, the node must log
+		 * a system error and must not attempt to generate
+		 * temporary address for that interface.
+		 * So we have to check the return err and distinguish
+		 * the correct retry path.
+		 */
+		if (PTR_ERR(ift) == -EEXIST) {
-EEXIST is not the same as "ipv6 address is is already used on the subnet". I really don't see the point here. IMHO this breaks the intended regeneration logic.
I fear a fix of CVE-2013-0343 will be a bit more complicated. ;) I give it a thought.
Quoted 6 lines
Quoted 5 lines
Greetings,
Hannes
ok, thanks for your feedback, I'll waiting you for more information to fix the problem.:)
[added George Kargiotakis and P J P to Cc and full quote]
I wonder if the easiest solution would be to just drop the max_addresses limit from ipv6_create_tempaddr. max_addresses protects the kernel from installing an unlimited amount of addresses on an interface which gets flooded by RAs. Because we have a direct relation between interface address to temp address, I don't see that we would create the possiblity of DoS.

Quoted 6 lines

Sure, an audit and testing is needed.
Greetings,
Hannes

I am afraid that if remove the max limit from the ipv6_create_tempaddr, the tool flood_route26 attack will create huge address to the temp_list, it will be a huge list, may it destroy something or not?

Best regards
Ding Tianhong

Quoted 7 lines

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
.

Hannes Frederic Sowa

2013-08-15 17:36 UTC #5 lore raw

Quoted 2 lines · Ding Tianhong

I am afraid that if remove the max limit from the ipv6_create_tempaddr, the tool flood_route26 attack will create huge address to the temp_list, it will be a huge list, may it destroy something or not?

I just tested, no it does not. Because it will only create a temporary address for each prefix received, which still is limited by max_addresses. But of course, more review is needed here. Maybe there is still a possibility to DoS?

Now we have to check why these addresses don't go out of tentative state.

Greetings,

Hannes

Hannes Frederic Sowa

2013-08-15 19:07 UTC #6 lore raw

Quoted 1 line · Hannes Frederic Sowa

Now we have to check why these addresses don't go out of tentative state.

Just looked at it. flood_router26 just emits pretty high values for RetransTime:

21:05:52.050159 IP6 (hlim 255, next-header ICMPv6 (58) payload length: 192) fe80::c:2a47:1360:1101 > ff02::1: [icmp6 sum ok] ICMP6, router advertisement, length 192

        hop limit 255, Flags [none], pref high, router lifetime 65535s, reachable time 16384000ms, retrans time 1966080ms
          mtu option (5), length 8 (1):  1500
            0x0000:  0000 0000 05dc

We are completly in spec here. Just DAD needs a lot of time to finish.

Greetings,

Hannes

Ding Tianhong

author

2013-08-16 00:49 UTC #7 lore raw

Quoted 11 lines · Hannes Frederic Sowa

Quoted 1 line · Hannes Frederic Sowa
Now we have to check why these addresses don't go out of tentative state.
Just looked at it. flood_router26 just emits pretty high values for RetransTime:
21:05:52.050159 IP6 (hlim 255, next-header ICMPv6 (58) payload length: 192) fe80::c:2a47:1360:1101 > ff02::1: [icmp6 sum ok] ICMP6, router advertisement, length 192
        hop limit 255, Flags [none], pref high, router lifetime 65535s, reachable time 16384000ms, retrans time 1966080ms
          mtu option (5), length 8 (1):  1500
            0x0000:  0000 0000 05dc

Quoted 7 lines

We are completly in spec here. Just DAD needs a lot of time to finish.
Greetings,
Hannes

Great job, more and more close to the truth, let's find a better way to finish it.

Quoted 2 lines

.

Hannes Frederic Sowa

2013-08-16 11:07 UTC #8 lore raw

Quoted 12 lines · Ding Tianhong

Quoted 11 lines · Hannes Frederic Sowa
Quoted 1 line · Hannes Frederic Sowa
Now we have to check why these addresses don't go out of tentative state.
Just looked at it. flood_router26 just emits pretty high values for RetransTime:
21:05:52.050159 IP6 (hlim 255, next-header ICMPv6 (58) payload length: 192) fe80::c:2a47:1360:1101 > ff02::1: [icmp6 sum ok] ICMP6, router advertisement, length 192
        hop limit 255, Flags [none], pref high, router lifetime 65535s, reachable time 16384000ms, retrans time 1966080ms
          mtu option (5), length 8 (1):  1500
            0x0000:  0000 0000 05dc

Quoted 8 lines

Quoted 7 lines
We are completly in spec here. Just DAD needs a lot of time to finish.
Greetings,
Hannes
Great job, more and more close to the truth, let's find a better way to finish it.

Thanks! :)

I just sent out a patch with the removal of the max_addresses check. Please have a look!

I tested it with flood_router26 and modified it to also change the retrans_time, because it affects in which interval addresses get regenerated. We always were on the safe side.

Greetings,

Hannes