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.

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

Differentiate between IPv4 and IPv6 NAT-1-1 addresses #2423

Merged
merged 1 commit into from
Nov 9, 2020
Merged

Conversation

lminiero
Copy link
Member

@lminiero lminiero commented Nov 6, 2020

As the title says, this new PR tries to add some awareness in Janus about the nature of the provided address(es) for NAT-1-1 mapping purposes. At the moment, when you specify one or more addresses, they're blindly set for all candidates, independently of the protocol family: this means that if you set, for instance, 1.2.3.4, this would be set for IPv6 candidates as well, which is clearly wrong. This new patch corrects that, and ensures that a NAT-1-1 address is only enforced if it's the same family as the candidate it's being applied on.

There's no change in how you configure the feature, it works exactly the same way: it's Janus that will look at the address(es) you pass, and figures out if they're IPv4 or IPv6 to handle them accordingly.

I tested this in several scenarios and it seems to be working, but of course I encourage you to test this carefully, especially if you're using the NAT-1-1 feature a lot (e.g., deploying on AWS, Azure, or other cloud providers). If I don't get any feedback, I plan to merge this relatively soon.

@alexamirante
Copy link
Member

I tested it and works as expected. 👍

@lminiero
Copy link
Member Author

lminiero commented Nov 9, 2020

Merging.

@lminiero lminiero merged commit a3b840f into master Nov 9, 2020
@lminiero lminiero deleted the nat11-v6 branch November 9, 2020 09:20
PauKerr pushed a commit to caffeinetv/janus-gateway that referenced this pull request Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants