Lost OpenVPN connection latest snap
-
@stilez
the problem doesn't exist in ip2long32() … the problem exists in long2ip32()see:
-
https://github.com/pfsense/pfsense/blob/master/src/etc/inc/openvpn.inc#L932 <– call to openvpn_get_interface_ip() that returned empty
-
https://github.com/pfsense/pfsense/blob/master/src/etc/inc/openvpn.inc#L1706-L1711 <–- function that i recreated in my console debugging
$baselong was/is correct.
$ip1 & ip2 on lines 1708-1709 of openvpn.inc were emty after your commit and thus the function, returned empty. this caused the openvpn client conf to become invalid.i redid your script to show you the differences in long2ip32 (i ran it in /usr/local/www through browser):
// included in case any dependencies, though there shouldn't be any require_once('util.inc'); require_once('openvpn.inc'); function long2ip32_a($ip) { return long2ip($ip & 0xFFFFFFFF); // return ((is_int($ip) && ($ip >> 32) == 0) ? long2ip($ip & 0xFFFFFFFF) : ''); } function long2ip32_b($ip) { // return long2ip($ip & 0xFFFFFFFF); return ((is_int($ip) && ($ip >> 32) == 0) ? long2ip($ip & 0xFFFFFFFF) : ''); } echo "test HEPER's snippet and input data using long2ip32() version A "; $baselong = ip2long32('192.168.222.0') & ip2long('255.255.255.248'); $ip1 = long2ip32_a($baselong + 1); $ip2 = long2ip32_a($baselong + 2); echo "baselong: [{$baselong}] "; echo "ip1: [{$ip1}] "; echo "ip2: [{$ip2}] "; echo " test HEPER's snippet and input data using long2ip32() version B "; $baselong = ip2long32('192.168.222.0') & ip2long('255.255.255.248'); $ip1 = long2ip32_b($baselong + 1); $ip2 = long2ip32_b($baselong + 2); echo "baselong: [{$baselong}] "; echo "ip1: [{$ip1}] "; echo "ip2: [{$ip2}] "; ?>output:
test HEPER's snippet and input data using long2ip32() version A baselong: [-1062674944] ip1: [192.168.222.1] ip2: [192.168.222.2] test HEPER's snippet and input data using long2ip32() version B baselong: [-1062674944] ip1: [] ip2: []I notice in your testing, you are running on 64bit(=positive longs). I'm experiencing this on 32bit and perhaps, thats where your version of long2ip32() fails & original works ?
-
-
Interesting. I run the exact snippet you gave (zero modification) in the GUI diagnostic -> command prompt shell, and got this:
test HEPER's snippet and input data using long2ip32() version A
baselong: [3232292352]
ip1: [192.168.222.1]
ip2: [192.168.222.2]test HEPER's snippet and input data using long2ip32() version B
baselong: [3232292352]
ip1: [192.168.222.1]
ip2: [192.168.222.2]Hard to see how it's an x32/x64 issue, but I'm inclined to say it must be.
As I don't have x32 around here, can you check out for me what you get with the following modification:function long2ip32_b($ip) { echo "DEBUG:"; $data = array ($ip, is_int($ip), $ip >> 32, (($ip >> 32) == 0), (is_int($ip) && ($ip >> 32) == 0)); var_dump($data); return ((is_int($ip) && ($ip >> 32) == 0) ? long2ip($ip & 0xFFFFFFFF) : ''); }On x64 I get:
[ 0] => int(3232292354)
[1] => bool(true)
[2] => int(0)
[3] => bool(true)
[4] => bool(true) -
DEBUGarray(5) {
[ 0] => int(-1062674943)
[1] => bool(true)
[2] => int(-1062674943)
[3] => bool(false)
[4] => bool(false) }DEBUG:array(5) {
[ 0] => int(-1062674942)
[1] => bool(true)
[2] => int(-1062674942)
[3] => bool(false)
[4] => bool(false) } -
So the shift right operator works differently in x32 and x64. >>32 should shift right by 32 bits and should clear any 32 bit value, and therefore confirm if the input has any data in bits >=33 for validity. But on x32 it's not shifting at all. That's…. unexpected, but easily fixed. That'll be it.
Update:
http://php.net/manual/en/language.operators.bitwise.php small print confirms: "Shifting integers by values greater than or equal to the system long integer width results in undefined behavior. In other words, don't shift more than 31 bits on a 32-bit system, and don't shift more than 63 bits on a 64-bit system".
How unsatisfactory. But easily fixed. Replace shift >>32 by an equivalent test. Might need you to verify the result of a few possible variants, on x32.I'm just cautious about x32 systems that might nonetheless handle integers >32 bits. In that case you wouldn't be able to rely on assuming (it's an integer) -> (it's a valid x32 value),, so the function would still need a test whether the input is within 2^32 limits. The easiest test is that if you shift out the rightmost 32 bits, do you have zeros left. But if that won't work on x32 then alternatives are easy.
Other tests would probably need to check MAXINT or system bitness instead (ie "SYSTEM INT is 32 bits wide OR $ip >>32 == 0"), because both logical AND, and directly testing the value of the integer, might share the same issues as >>32 does on x32.
What happens if the B version is replaced by this, which isn't so elegant but is functionally equivalent? Does it work for you then?
function long2ip32_b($ip) { // replace >> 32 by >> 16 twice, due to >>32 issues on x32 platforms echo "DEBUG:"; $data = array ($ip, is_int($ip), ($ip >> 16) >> 16, ((($ip >> 16) >> 16) == 0), (is_int($ip) && (($ip >> 16) >> 16) == 0)); var_dump($data); return ((is_int($ip) && (($ip >> 16) >> 16) == 0) ? long2ip($ip & 0xFFFFFFFF) : ''); } -
DEBUG:array(5) {
[ 0] => int(-1062674943)
[1] => bool(true)
[2] => int(-1)
[3] => bool(false)
[4] => bool(false) }DEBUG:array(5) {
[ 0] => int(-1062674942)
[1] => bool(true)
[2] => int(-1)
[3] => bool(false)
[4] => bool(false) }baselong: [-1062674944]
ip1: []
ip2: []so no, doesn't work
-
Intriguing. I wonder how on earth, a right bit shift ends up with a 1-filled value (-1 binary), when every other shift operation zero fills. Never mind. MAX_INT should be robust and bitwise NOT should automatically adapt to the system integer size. Can you try again with these two versions:
function long2ip32_b($ip) { // ">> 32" only works when INT SIZE > 4 bytes, but if <=4 bytes it's automatically within IPv4 limits anyway echo "DEBUG:"; $data = array ($ip, is_int($ip), PHP_INT_SIZE, ($ip >> 32), (($ip >> 32) == 0), (is_int($ip) && (PHP_INT_SIZE <= 4 || ($ip >> 32) == 0))); var_dump($data); return ((is_int($ip) && (PHP_INT_SIZE <= 4 || ($ip >> 32) == 0)) ? long2ip($ip & 0xFFFFFFFF) : ''); }and:
function long2ip32_b($ip) { echo "DEBUG:"; $data = array ($ip, is_int($ip), ($ip & ~0xFFFFFFFF), (($ip & ~0xFFFFFFFF) == 0), (is_int($ip) && ($ip & ~0xFFFFFFFF) == 0)); var_dump($data); return ((is_int($ip) && ($ip & ~0xFFFFFFFF) == 0) ? long2ip($ip & 0xFFFFFFFF) : ''); }One, probably both, should have a chance of working
-
both seem to work (called them B & C)
test HEPER's snippet and input data using long2ip32() version A baselong: [-1062674944] ip1: [192.168.222.1] ip2: [192.168.222.2] test HEPER's snippet and input data using long2ip32() version B DEBUG: array(6) { [0] => int(-1062674943) [1] => bool(true) [2] => int(4) [3] => int(-1062674943) [4] => bool(false) [5] => bool(true) } DEBUG: array(6) { [0] => int(-1062674942) [1] => bool(true) [2] => int(4) [3] => int(-1062674942) [4] => bool(false) [5] => bool(true) } baselong: [-1062674944] ip1: [192.168.222.1] ip2: [192.168.222.2] test HEPER's snippet and input data using long2ip32() version C DEBUG: array(5) { [0] => int(-1062674943) [1] => bool(true) [2] => int(0) [3] => bool(true) [4] => bool(true) } DEBUG: array(5) { [0] => int(-1062674942) [1] => bool(true) [2] => int(0) [3] => bool(true) [4] => bool(true) } baselong: [-1062674944] ip1: [192.168.222.1] ip2: [192.168.222.2] -
PR#2317 submitted with the above fix that seems to now work properly on x32 and x64: https://github.com/pfsense/pfsense/pull/2317
Thanks for the heads up and help bugfixing it!
-
thats what we are here for ;)
-
PR has been merged. Thanks for the detective work. Seems PHP always has a new quirk up its sleeve :)