search

g4klx / ircDDBGateway

The ircDDB Gateway for D-Star
GNU General Public License v2.0
61 stars 41 forks source link

UDP Hole Punching bug #23

Closed w1bsb closed 5 years ago

w1bsb commented 5 years ago

There appears to be an issue in the UDP Hole Punching mechanism that causes ircddbgateway users to send hole punch requests to any users who key up on the network, whether or not the users who are receiving the hole punches are actually attempting to route to the user who is sending them. This happens automatically and neither the users sending the request nor the user keying up can see this. This was noticed while debugging some other ircDDB software that logs incoming port 40000 UDP requests.

I was able to track down the IP addresses of the incoming requests to users logged into QuadNet's ircDDB network using ircDDBGateway with the following version identifier: CIRCDDB:1.2.4 linux_ircDDBGateway-20180719

When a user keys up his hotspot/repeater and the ircDDB message is sent to other connected clients that the user has keyed up, ircddbgateway sends a UDP hole punching request to that client regardless of whether or not they are actually trying to route to them. The packets I was able to capture were 29 bytes in size, and had a data size of 1 byte. I can provide a packet capture if need be, but I'm fairly certain the issue lies in the UDP Hole Punching mechanism. Thanks!

Colby W1BSB

AndyTaylorTweet commented 5 years ago

Confirmed - just TX anywhere, as soon as anything is sent to the irc DDB, I get a pile of packets back from 20 or more hosts.

Since this is going out with Pi-Star v4, I have crippled the hole-punch code for the time being until I understand what the aim is and/or until the bug is resolved.

g4klx commented 5 years ago

I'll try and identify the commit id and revert it.

AndyTaylorTweet commented 5 years ago

In the short term I just commented out line 159 in: ircDDBGateway/Common/G2ProtocolHandler.cpp

// Commented to cripple "hole punch" code. // m_socket.write(buffer, 1, addr, G2_DV_PORT);

I'm not sure what the original intent is with this commit, you might want to branch it so that the originator can clean it up, I'll leave it to you make those choices.

g4klx commented 5 years ago

I think I've reverted the whole thing. Can you take a look and confirm please (or not).

AndyTaylorTweet commented 5 years ago

Well that seems to have reverted the behaviour, but also dropped the "-daemon" switch, and something happened to the logging. For the moment I'll leave the modified binary out there until things settle down a little bit.

Edit: I blame tired eyes - ignore this post totally....

g4klx commented 5 years ago

The daemon switch is still there, are you sure you're not running the GUI version which doesn't support the daemon switch?

AndyTaylorTweet commented 5 years ago

Cleaned my source tree and did it right this time, you can totally ignore my previous comment;

Problem still persists ---ircDDBGateway Log--- M: 2019-01-15 22:39:01: GATEWAY: NO9MB G 174.227.18.xx M: 2019-01-15 22:39:06: GATEWAY: SM6YEC G 81.225.126.xx M: 2019-01-15 22:39:09: GATEWAY: KG6HQD G 184.254.91.xx M: 2019-01-15 22:39:10: GATEWAY: DG1KWA G 84.132.186.xx M: 2019-01-15 22:39:10: GATEWAY: DC5LY G 87.140.1.xx M: 2019-01-15 22:39:13: USER: EA7OR EA7OR B EA7OR G 37.29.246.xx M: 2019-01-15 22:39:13: USER: EA7OR EA7OR B EA7OR G 37.29.246.xx

--firewall packet capture-- 22:39:01.140202 IP 192.168.11.106.40000 > 174.227.18.xx.40000: UDP, length 1 22:39:06.644059 IP 192.168.11.106.40000 > 81.225.126.xx.40000: UDP, length 1 22:39:09.144754 IP 192.168.11.106.40000 > 184.254.91.xx.40000: UDP, length 1 22:39:10.145803 IP 192.168.11.106.40000 > 84.132.186.xx.40000: UDP, length 1 22:39:10.145893 IP 192.168.11.106.40000 > 87.140.1.xx.40000: UDP, length 1 22:39:13.144212 IP 192.168.11.106.40000 > 37.29.246.xx.40000: UDP, length 1 22:39:13.144302 IP 192.168.11.106.40000 > 37.29.246.xx.40000: UDP, length 1

As you can see, for every host logged in the irc DDB, ircDDBGateway sends them a UDP/40000 packet.

g4klx commented 5 years ago

Grrrr. I need to roll back some other way. Bed time now.

Sent from Yahoo Mail for iPhone

On Tuesday, January 15, 2019, 22:47, Andy Taylor notifications@github.com wrote:

Cleaned my source tree and did it right this time, you can totally ignore my previous comment;

Problem still persists ---ircDDBGateway Log--- M: 2019-01-15 22:39:01: GATEWAY: NO9MB G 174.227.18.xx M: 2019-01-15 22:39:06: GATEWAY: SM6YEC G 81.225.126.xx M: 2019-01-15 22:39:09: GATEWAY: KG6HQD G 184.254.91.xx M: 2019-01-15 22:39:10: GATEWAY: DG1KWA G 84.132.186.xx M: 2019-01-15 22:39:10: GATEWAY: DC5LY G 87.140.1.xx M: 2019-01-15 22:39:13: USER: EA7OR EA7OR B EA7OR G 37.29.246.xx M: 2019-01-15 22:39:13: USER: EA7OR EA7OR B EA7OR G 37.29.246.xx

--firewall packet capture-- 22:39:01.140202 IP 192.168.11.106.40000 > 174.227.18.xx.40000: UDP, length 1 22:39:06.644059 IP 192.168.11.106.40000 > 81.225.126.xx.40000: UDP, length 1 22:39:09.144754 IP 192.168.11.106.40000 > 184.254.91.xx.40000: UDP, length 1 22:39:10.145803 IP 192.168.11.106.40000 > 84.132.186.xx.40000: UDP, length 1 22:39:10.145893 IP 192.168.11.106.40000 > 87.140.1.xx.40000: UDP, length 1 22:39:13.144212 IP 192.168.11.106.40000 > 37.29.246.xx.40000: UDP, length 1 22:39:13.144302 IP 192.168.11.106.40000 > 37.29.246.xx.40000: UDP, length 1

As you can see, for every host logged in the irc DDB, ircDDBGateway sends them a UDP/40000 packet.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub, or mute the thread.

AndyTaylorTweet commented 5 years ago

Pi-Star is patched - that should take the heat out of it for now while you work it out, this is only affecting our 4.0.0 users, so the audience is comparatively small at the moment.

g4klx commented 5 years ago

I've done a source code diff and apart from some unrelated changes (host files and DD-mode fix) the latest HEAD is nominally the same as the code just before Geoffrey started adding the hole punching.

w1bsb commented 5 years ago

It looks like the main portion of the code that does the hole punching (which Andy commented out) is still present in the current build, line 156 of Common/G2ProtocolHandler.cpp. I believe this is why Andy is still seeing the udp holes being punched in his packet trace. I went back through the commit history and it looks like the hole punching was added in pieces over several different pull requests/commits.

g4klx commented 5 years ago

It's finally gone.

AndyTaylorTweet commented 5 years ago

This looks good thank you, no more un-expected outbound packets.