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

Allow the use of IPv4 address mask when built with IPv6 support but disabled at runtime #714

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danielbodart
Copy link

So the default binaries on OpenWRT have IPv6 support built in but if you use the runtime config option
ipv6_disable to disable IPv6 you still can't set listening_ip to an address mask like 10.0.0.1/24.

This tiny change corrects that issue and allows the IP address mask to be supplied.

I changed the flow slightly so that the syslog warning did not have to be duplicated.

I couldn't see a test for this code path but I did run a full build

@miniupnp miniupnp self-assigned this Mar 12, 2024
@miniupnp
Copy link
Owner

@danielbodart there is an issue : there is no guarantee that SETFLAG(IPV6DISABLEDMASK); is called before the call to
parselanaddr() !

@danielbodart
Copy link
Author

I assume you mean SETFLAG(IPV6DISABLEDMASK); is not called before this code is run? As I would imagine the code is deterministic not probabilistic!

@miniupnp
Copy link
Owner

I assume you mean SETFLAG(IPV6DISABLEDMASK); is not called before this code is run? As I would imagine the code is deterministic not probabilistic!

the code is deterministic. But the user input is not known in advance.

ipv6_disable=yes
listening_ip=192.168.1.0/24

would be OK, but not the other way around.

@danielbodart
Copy link
Author

Hmmmm, interesting is that a conscious trade-off or just some accidental complexity of the current implementation?

Also is there a reason to not support the subnet masks for IP6? Seems odd not to be able to specify which IPv6 subnet you want to enable this for. On my pretty standard OpenWRT setup I have 3 different IPv6 subnets on the same interface, 1 is public, 1 is link local and 1 is global but not routable. Now obviously most of the time if I have a public ipv6 address I'm going to want to turn off UPnP as it serves no purpose but I could imagine a more complex scenario where someone only wanted UPnP enabled on the private IPv6 address and not on the internet routable IPv6 subnet

@miniupnp
Copy link
Owner

Hmmmm, interesting is that a conscious trade-off or just some accidental complexity of the current implementation?

Just have a look a the code : the configuration file is parsed top-down.
There is some work needed to make it work differently.

Also is there a reason to not support the subnet masks for IP6? Seems odd not to be able to specify which IPv6 subnet you want to enable this for. On my pretty standard OpenWRT setup I have 3 different IPv6 subnets on the same interface, 1 is public, 1 is link local and 1 is global but not routable. Now obviously most of the time if I have a public ipv6 address I'm going to want to turn off UPnP as it serves no purpose but I could imagine a more complex scenario where someone only wanted UPnP enabled on the private IPv6 address and not on the internet routable IPv6 subnet

I remember that UPnP doesn't work well with link local addresses.

I don't really get why you have the public IP on the same interface as your non routable one...
Why is UPnP IGD needed in such a setup ?

@miniupnp
Copy link
Owner

well I'm stupid. UPnP IGD WANIPv6FirewallControl supposes that you use only routable adresses.

@Headcrabed
Copy link

Any update on this?

@miniupnp
Copy link
Owner

Any update on this?

I haven't had time to change the parsing of the config file yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants