Opened 6 years ago
Closed 3 years 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)
Change History (5)
comment:1 Changed 6 years ago by
Changed 6 years ago by
Attachment: | ipv6-pool-max.patch added |
---|
comment:2 Changed 6 years ago by
Owner: | set to Gert Döring |
---|---|
Status: | new → accepted |
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:4 Changed 3 years ago by
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
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.
Some tests (this is openvpn on debian/stretch 2.4.0-6+deb9u1)