Opened 3 years ago

Closed 4 weeks ago

#920 closed Bug / Defect (fixed)

Specifying an IPv6 pool with a mask between 96 and 112 results in a pool bigger than IFCONFIG_POOL_MAX

Reported by: znerol Owned by: Gert Döring
Priority: major Milestone:
Component: Generic / unclassified Version: OpenVPN git master branch (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

pool.h:

#define IFCONFIG_POOL_MAX         65536

pool.c:

struct ifconfig_pool *
ifconfig_pool_init(int type, in_addr_t start, in_addr_t end,
                   const bool duplicate_cn,
                   const bool ipv6_pool, const struct in6_addr ipv6_base,
                   const int ipv6_netbits )
{

[...]

        pool->size_ipv6 = ipv6_netbits>96 ? ( 1<<(128-ipv6_netbits) )
                          : IFCONFIG_POOL_MAX;

        msg( D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: (IPv4) size=%d, size_ipv6=%d, netbits=%d, base_ipv6=%s",
             pool->size, pool->size_ipv6, ipv6_netbits,
             print_in6_addr( pool->base_ipv6, 0, &gc ));

[...]

}

The intention behind this piece of code is likely to choose a pool size which is never bigger than IFCONFIG_POOL_MAX. Regrettably 128-96 is 32 while the IFCONFIG_POOL_MAX is 216, so we better should check for ipv6_netbits>112 (128-16) instead.

Attachments (1)

ipv6-pool-max.patch (584 bytes) - added by znerol 3 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 3 years ago by znerol

Some tests (this is openvpn on debian/stretch 2.4.0-6+deb9u1)

server-ipv6 2001:DB8::/96
Sun Jul 23 00:08:22 2017 us=664226 IFCONFIG POOL IPv6: (IPv4) size=62, size_ipv6=65536, netbits=96, base_ipv6=2001:db8::1000

server-ipv6 2001:DB8::/97
Sun Jul 23 00:09:01 2017 us=636993 IFCONFIG POOL IPv6: (IPv4) size=62, size_ipv6=-2147483648, netbits=97, base_ipv6=2001:db8::1000

server-ipv6 2001:DB8::/98
Sun Jul 23 00:09:26 2017 us=108708 IFCONFIG POOL IPv6: (IPv4) size=62, size_ipv6=1073741824, netbits=98, base_ipv6=2001:db8::1000

server-ipv6 2001:DB8::/99
Sun Jul 23 00:09:50 2017 us=136158 IFCONFIG POOL IPv6: (IPv4) size=62, size_ipv6=536870912, netbits=99, base_ipv6=2001:db8::1000

Changed 3 years ago by znerol

Attachment: ipv6-pool-max.patch added

comment:2 Changed 3 years ago by Gert Döring

Owner: set to Gert Döring
Status: newaccepted

That whole calculation is a bit misleading, as there is no true "ipv6 pool" anyway (the code uses the IPv4 pool, and then takes the offset for the IPv4 address from the start of the pool to calculate the IPv6 address).

I need to review the code, but I think all that check is used for is "ensure we do not overrun the IPv6 pool range" - so the calculation mainly needs to make sure that there is no integer overflow.

So, arguably your patch is right, but maybe things need a bit more thorough review.

comment:3 Changed 3 years ago by tincantech

Watching

comment:4 Changed 4 weeks ago by Gert Döring

Resolution: fixed
Status: acceptedclosed

So, returning here after a long time.

This has been fixed "in passing" as part of the IPv6-only patchset (trac #208) - we lifted the "pools must be /112 or bigger" restriction (up to /124 is now allowed) and since IPv6 pools can be "standalone" with no v4, size handling had to be fixed.

For master, this is in

commit 81d66a1f14d4be3282dd648ecc2049658e3a65ed
Author: Antonio Quartulli <a@…>
Date: Sat May 30 02:05:54 2020 +0200

pool: prevent IPv6 pools to be larger than 216 addresses

For 2.4, the size fix is backported in

commit fc0297143494e0a0f08564d90dbb210669d0abf5
Author: Antonio Quartulli <a@…>
Date: Sat May 30 02:05:54 2020 +0200

pool: prevent IPv6 pools to be larger than 216 addresses

Apologies for not giving @znerol credit here - I had totally forgotten that this patch was in trac, and I'm only now rediscovering it.

Note: See TracTickets for help on using tickets.