Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
ghost opened this issue Jan 16, 2020 · 3 comments
Open

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

ghost opened this issue Jan 16, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@ghost
Copy link

ghost commented Jan 16, 2020

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
Copy link
Owner

alxhotel commented Feb 5, 2020

Hi @TheBrainEscaped,

Super thank you for the feedback ❤️

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 :)

@alxhotel alxhotel added the bug Something isn't working label Feb 5, 2020
@ghost
Copy link
Author

ghost commented Feb 7, 2020

I can confirm that with [email protected] the issue mentioned above is gone the way of the Dodo bird.

Thanks for the update!

@ghost
Copy link
Author

ghost commented Feb 16, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@alxhotel