search

alxhotel / nat-api

:left_right_arrow: Fast port mapping with UPnP and NAT-PMP
MIT License
23 stars 12 forks source link

Dual TCP/UDP port mapping does not work as expected #4

Open ghost opened 4 years ago

ghost commented 4 years ago

nat-api version: 0.1.2

c:\>systeminfo | findstr /B /C:"OS Name" /C:"OS Version"
OS Name:                   Microsoft Windows 10 Pro
OS Version:                10.0.18363 N/A Build 18363

c:\>node --version
v12.13.1
const NatAPI = require('nat-api');

const client = new NatAPI({autoUpdate: false});

Test 1:

client.map(10000,function (err) {
    if (err) return console.log('Error', err);
    console.log('Port mapped!');
});

Maps port 10000 UDP and bombs for TCP with:

dgram.js:839
    throw new ERR_SOCKET_DGRAM_NOT_RUNNING();
    ^

Error [ERR_SOCKET_DGRAM_NOT_RUNNING]: Not running
    at healthCheck (dgram.js:839:11)
    at Socket.send (dgram.js:609:3)
    at Client._next (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:230:15)
    at Client.request (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:155:8)
    at Client.portMapping (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:182:8)
    at NatAPI._pmpMap (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:316:21)
    at NatAPI._map (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:229:12)
    at C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:78:14
    at C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:233:9
    at Client.<anonymous> (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:342:7) {
  code: 'ERR_SOCKET_DGRAM_NOT_RUNNING'
}

Test 2 (with NULL protocol in options):

client.map({ publicPort: 10000, privatePort: 10000, ttl: 1200 , protocol: null},function (err) {
    if (err) return console.log('Error', err);
    console.log('Port mapped!');
});

Maps port 10000 UDP and bombs for TCP with:

dgram.js:839
    throw new ERR_SOCKET_DGRAM_NOT_RUNNING();
    ^

Error [ERR_SOCKET_DGRAM_NOT_RUNNING]: Not running
    at healthCheck (dgram.js:839:11)
    at Socket.send (dgram.js:609:3)
    at Client._next (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:230:15)
    at Client.request (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:155:8)
    at Client.portMapping (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:182:8)
    at NatAPI._pmpMap (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:316:21)
    at NatAPI._map (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:229:12)
    at C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:78:14
    at C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:233:9
    at Client.<anonymous> (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-api\index.js:342:7) {
  code: 'ERR_SOCKET_DGRAM_NOT_RUNNING'
}

Test 3:

client.map({ publicPort: 10000, privatePort: 10000, ttl: 1200 , protocol: 'TCP'},function (err) {
    if (err) return console.log('Error', err);
    console.log('Port mapped!');
});

Works: Port mapped!

Test 4:

client.map({ publicPort: 10000, privatePort: 10000, ttl: 1200 , protocol: 'UDP'},function (err) {
    if (err) return console.log('Error', err);
    console.log('Port mapped!');
});

Works: Port mapped!

Test 5:

client.map({ publicPort: 10000, privatePort: 10000, ttl: 1200 , protocol: 'TCP'},function (err) {
    if (err) return console.log('Error', err);
    console.log('Port mapped!');
});

client.map({ publicPort: 10000, privatePort: 10000, ttl: 1200 , protocol: 'UDP'},function (err) {
    if (err) return console.log('Error', err);
    console.log('Port mapped!');
});

TCP port mapped, UDP port bombs:

Port mapped!
dgram.js:839
    throw new ERR_SOCKET_DGRAM_NOT_RUNNING();
    ^

Error [ERR_SOCKET_DGRAM_NOT_RUNNING]: Not running
    at healthCheck (dgram.js:839:11)
    at Socket.send (dgram.js:609:3)
    at Client._next (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:230:15)
    at cb (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:284:10)
    at Client.onmessage (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:335:5)
    at Socket.<anonymous> (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:371:32)
    at Socket.emit (events.js:210:5)
    at UDP.onMessage [as onmessage] (dgram.js:861:8) {
  code: 'ERR_SOCKET_DGRAM_NOT_RUNNING'
}

Test 6:

client.map({ publicPort: 10000, privatePort: 10000, ttl: 1200 , protocol: 'UDP'},function (err) {
    if (err) return console.log('Error', err);
    console.log('Port mapped!');
});

client.map({ publicPort: 10000, privatePort: 10000, ttl: 1200 , protocol: 'TCP'},function (err) {
    if (err) return console.log('Error', err);
    console.log('Port mapped!');
});

UDP port maps, TCP port bombs:

Port mapped!
dgram.js:839
    throw new ERR_SOCKET_DGRAM_NOT_RUNNING();
    ^

Error [ERR_SOCKET_DGRAM_NOT_RUNNING]: Not running
    at healthCheck (dgram.js:839:11)
    at Socket.send (dgram.js:609:3)
    at Client._next (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:230:15)
    at cb (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:284:10)
    at Client.onmessage (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:335:5)
    at Socket.<anonymous> (C:\Users\Cos\Desktop\test\node-upnp-test\node_modules\nat-pmp\index.js:371:32)
    at Socket.emit (events.js:210:5)
    at UDP.onMessage [as onmessage] (dgram.js:861:8) {
  code: 'ERR_SOCKET_DGRAM_NOT_RUNNING'
}

Conclusion: dual TCP/UDP port mapping does not work as expected.

alxhotel commented 4 years ago

Hi @TheBrainEscaped,

Super thank you for the feedback :heart:

I haven't yet be able to reproduce the bug. However, it looks like the problem is related to PMP.

So I have added an option on the last version (0.1.3) of nat-api to force PMP. This means that if you update your version you shouldn't have any more problems because it is not using PMP by default.

Please check it out and let me know how it went :)

ghost commented 4 years ago

I can confirm that with nat-api@0.1.3 the issue mentioned above is gone the way of the Dodo bird.

Thanks for the update!

ghost commented 4 years ago

Let me take that back.

I distinctively remember creating a quick test project on Windows 10 and testing some of the above code against 0.1.3 and had my ports appearing on the router's "UPNP "page without issues.

However, testing on a Linux distribution I started running into issues again so I decided to re-test on Windows and managed to replicate the issues encountered on Linux.

This code

const clientTCP = new NatAPI({ autoUpdate: true, enablePMP: false });
clientTCP.map({ publicPort: 10000, privatePort: 10000, ttl: 1200, protocol: 'TCP', description: 'Nat-Api TCP' },function (err) {
    if (err) {
        console.log('Error', err);
    } else {
        console.log('TCP port mapped!');
    }
});

const clientUDP = new NatAPI({ autoUpdate: true, enablePMP: false });
clientUDP.map({ publicPort: 10000, privatePort: 10000, ttl: 1200, protocol: 'UDP', description: 'Nat-Api UDP' },function (err) {
    if (err) {
        console.log('Error', err);
    } else {
        console.log('UDP port mapped!');
    }
});

leads to

Error Error: UPnP port mapping failed
    at C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\index.js:226:25
    at C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\index.js:293:16
    at C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\lib\upnp\index.js:24:23
    at Timeout._onTimeout (C:\Users\Cos\Desktop\nat-test-2\node_modules\nat-api\lib\upnp\index.js:193:7)
    at listOnTimeout (internal/timers.js:531:17)
    at processTimers (internal/timers.js:475:7)
Error Error: UPnP port mapping failed
    at C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\index.js:226:25
    at C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\index.js:293:16
    at C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\lib\upnp\index.js:24:23
    at Timeout._onTimeout (C:\Users\Cos\Desktop\nat-test-2\node_modules\nat-api\lib\upnp\index.js:193:7)
    at listOnTimeout (internal/timers.js:531:17)
    at processTimers (internal/timers.js:475:7)

and no ports mapped.

I have come to realize that my router only supports NAT-PMP, therefore the exceptions above are unavoidable due to nat-api attemtping UPNP mapping by default. To test this assumption, I went ahead and commented out all UPNP related code and re-tested with

const NatAPI = require('nat-api');

const clientTCP = new NatAPI({ autoUpdate: true, enablePMP: true });
clientTCP.map({ publicPort: 10000, privatePort: 10000, ttl: 1200, protocol: 'TCP', description: 'Nat-Api TCP' },function (err) {
    if (err) {
        console.log('Error', err);
    } else {
        console.log('TCP port mapped!');
    }
});

const clientUDP = new NatAPI({ autoUpdate: true, enablePMP: true });
clientUDP.map({ publicPort: 10000, privatePort: 10000, ttl: 1200, protocol: 'UDP', description: 'Nat-Api UDP' },function (err) {
    if (err) {
        console.log('Error', err);
    } else {
        console.log('UDP port mapped!');
    }
});

The above maps both ports, however it bombs with

C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\lib\pmp\index.js:257
    parsed.vers = msg.readUInt8(0)
                      ^

TypeError: Cannot read property 'readUInt8' of undefined
    at Client.onMessage (C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\lib\pmp\index.js:257:23)
    at Socket.<anonymous> (C:\Users\XXX\Desktop\nat-test-2\node_modules\nat-api\lib\pmp\index.js:50:42)
    at Socket.emit (events.js:210:5)
    at UDP.onMessage [as onmessage] (dgram.js:861:8)

The culprit, line 257 in lib/pmp/index.js : parsed.vers = msg.readUInt8(0). Apparently msg is undefined. Adding if (!msg) return; after line 231 ( if (this._queue.length === 0) return) in lib/pmp/index.js fixes it: no more exceptions. Bear in mind that this is possible by commenting-out the UPNP mapping code.

Would it be possible to have the option to toggle the mapping type between UPNP and NAT-PMP ? Something along the lines of:

const clientUPNP = new NatAPI({ autoUpdate: true, mappingType: 'UPNP' });

const clientPMP = new NatAPI({ autoUpdate: true, mappingType: 'PMP' });

And of course if mappingType is left out it should probably default to UPNP.