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

Only accept dns requests going to internal IPs #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

james-otten
Copy link

Problem

Because /ip/dns/allow-remote-requests=yes and the default firewall rule for port 53 is to allow any traffic (for mesh users to use their omni as a cache) if an omni is given a public IP and no firewall rules are changed, it will respond to DNS requests from the internet. This was being abused in some cases.

Solution

To make this less error prone, the default firewall rule for port 53 is made to be more restrictive. This is fine in the default case without a public IP, and now volunteers giving out new public IPs don't need to think about DNS.

@@ -138,7 +138,7 @@ add address=23.158.16.0/24 list=meshaddr

/ip firewall filter
add action=accept chain=input protocol=icmp
add action=accept chain=input dst-port=53 protocol=udp
add action=accept chain=input dst-port=53 protocol=udp dst-address=10.0.0.0/8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use the meshaddr address list instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It includes 2 public mesh IP blocks which I think we want to deny just in case something from one of them gets assigned to an omni. This applies to just the input filter, and not the forward filter, so users can still access DNS servers at public mesh IPs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm of the same mind as Andrew here. We should be using src-address-list=meshaddr similar to the "drop input if not from meshaddr" line a few lines down. It is a little odd to have a dst-address here for the input, and actually this would break the use-case that Andrew and I were talking about a few days ago where we would have a public IP routed to the member's router and they reply to the public IP on the router.

The reason for this allow line even though we have an input drop line is to allow wlan2 users to access dns despite not being allowed to access the router's management interface. So, another option is to just add in-bridge-port=wlan2 on this line instead of your suggestion, which would resolve that problem, and everyone else would be covered by the "drop input if not from meshaddr".

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I misread at fist thinking about dest not src. Updated to src-address-list=meshaddr

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.

3 participants