830476 - patch stun-server to accept public ip addresses on the command line

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Ralph Giles (:rillian)]

We're using the stun-server package to run our public stun servers for WebRTC.

Since these are running on AWS, then are behind a NAT, and return the internal addresses as candidates. These are on the 10.x.x.x subnet and will usually be culled as unrouted by the ice engine, but we shouldn't advertise them to avoid confusion.

This involves patching the stun-server code to accept the public addresses as additional command-line switches and list only those for the Source address entries.

Comment 1

[Wesley Dawson [:whd]]

Hi. A cursory check of the internet yields this:

http://www.voip-info.org/wiki/view/Vovida.org+STUN+server
http://www.powerpbx.ru/blog/wp-content/uploads/stund.patch

I will attempt to patch an epel rpm and see how it goes.

Comment 2

[Wesley Dawson [:whd]]

RPM with patch at:

https://people.mozilla.com/~wdawson/stun-server-0.96-4.amzn1.x86_64.rpm

I am setting up a box to test this on.

Comment 3

[Ralph Giles (:rillian)]

Thanks. Can you please attach the patch?

NB :ekr says we don't need the second NAT-characterization IP address for ICE, so you can just map a single public IP to the vm, Avoiding the complexity of a private subnet.

Comment 4

[Wesley Dawson [:whd]]

Attachment: Patch to fix NAT issues

Comment 5

[Wesley Dawson [:whd]]

Ok, getting this from my test box:

$ stun-client -v 1 ec2-54-245-6-22.us-west-2.compute.amazonaws.com 1

STUN client version 0.96
running test number 1
running test number 1
Opened port 22814 with fd 3
Encoding stun message: 
Encoding ChangeRequest: 0

About to send msg of len 28 to 54.245.6.22:3478
Got a response
Received stun message: 88 bytes
MappedAddress = 63.245.220.240:41305
SourceAddress = 54.245.6.22:3478
ChangedAddress = 0.0.0.0:3479
XorMappedAddress = 63.245.220.240:41305
ServerName = Vovida.org 0.96
         ok=1
         id=1:160:51:124:166:55:26:0:236:214:243:57:11:68:97:108
         mappedAddr=63.245.220.240:41305
         changedAddr=0.0.0.0:3479


Return value is 0x000000

:rillian can you confirm that looks right?

Comment 6

[Ralph Giles (:rillian)]

Looks good. Thanks!

> ServerName = Vovida.org 0.96

We should patch the servername to indicate it's our custom build. Append "Mozilla" and a git commit id?

Comment 7

[Wesley Dawson [:whd]]

Currently our provisioning scripts, spec files and patches all live here:

https://github.com/whd/stun-aws-svc/

(aside: perhaps this should merge with https://github.com/mozilla/stun-vm/ ?)

If I understand correctly, you want to dynamically insert into the source at build time the current git commit id of the spec file repository. If we do this, we should put the rpm spec file and patch into its own repository, and we should use a Makefile to build the rpm within a build subdirectory of the repository so that the spec can use "git log --pretty=format:'%h' -n 1" in a sed call or similar to insert the commit id.

Does this sound correct, or did you have something else in mind? Another possibility is to use the rpm version number as the identifier (e.g. Mozilla 5svc.amz1) for which an rpm macro already exists, since every new build of the rpm will require bumping this number in the spec file.

Comment 8

[Mark Mayo [:mmayo]]

+1 on "mozilla.org 5svc.amz1" format.

Comment 9

[Ralph Giles (:rillian)]

(In reply to Wesley Dawson [:whd] from comment #7)

> https://github.com/whd/stun-aws-svc/
> 
> (aside: perhaps this should merge with https://github.com/mozilla/stun-vm/ ?)

Yes please. Send me a pull request, or push it yourself.

> Another
> possibility is to use the rpm version number as the identifier (e.g. Mozilla
> 5svc.amz1) for which an rpm macro already exists, since every new build of
> the rpm will require bumping this number in the spec file.

That's fine. I just wanted to make sure there there was some kind of version identifier there.

Comment 10

[Ralph Giles (:rillian)]

I merged it myself.

https://github.com/mozilla/stun-vm/commit/f55c3270c2a4b90b97b3fea7b2d30f1bee8202ba
https://github.com/mozilla/stun-vm/commit/acfdac9e940ecc6e5150262e0465f7dfeb6662b6

Comment 11

[Wesley Dawson [:whd]]

Ok, I updated the rpm spec with the version identifier insertion stuff. I'm sending a pull request with the changes.

Comment 12

[Ralph Giles (:rillian)]

Merged. I think we're done here.

Thanks for the quick turn-around!