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

Wrong scope used with unbound and multiple interfaces #886

Closed
GrantRoot1970 opened this issue Jan 31, 2024 · 1 comment
Closed

Wrong scope used with unbound and multiple interfaces #886

GrantRoot1970 opened this issue Jan 31, 2024 · 1 comment

Comments

@GrantRoot1970
Copy link

The wrong scope may be used when the host has multiple interfaces and the container is set to unbound (INSTANCE_IP: 0.0.0.0 and INSTANCE_LISTEN: 0.0.0.0).

This is because the Instance.IP is used to determine the scope rather than using the IP of the interface that received the request (see the following patch). This bug may be present in other functions, but I did not review any others.

PATCH

--- scopes.go.bug       2024-01-30 05:49:52.000000000 -0500
+++ scopes.go   2024-01-30 18:59:15.000000000 -0500
@@ -99,6 +99,21 @@
        // To prioritise requests from a DHCP relay being matched correctly, give their subnet
        // match a 1 bit more priority
        const dhcpRelayBias = 1
+       // Use the instance ip unless the the interface is not bound
+       ip := extconfig.Get().Instance.IP
+       if req.oob != nil {
+               if ief, err := net.InterfaceByIndex(req.oob.IfIndex); err == nil {
+                       if addrs, err := ief.Addrs(); err == nil {
+                               for _, addr := range addrs {
+                                       if ipv4Addr := addr.(*net.IPNet).IP.To4(); ipv4Addr != nil {
+                                               ip = ipv4Addr.String()
+                                               req.log.Debug("Unbound interface found", zap.String("ifname",  ief.Name), zap.String("ip",  ip))
+                                               break
+                                       }
+                               }
+                       }
+               }
+       }
        for _, scope := range r.scopes {
                // Check based on gateway IP (highest priority)
                gatewayMatchBits := scope.match(req.GatewayIPAddr)
@@ -106,12 +121,12 @@
                        req.log.Debug("selected scope based on cidr match (gateway IP)", zap.String("scope", scope.Name))
                        match = scope
                        longestBits = gatewayMatchBits + dhcpRelayBias
                // Handle local broadcast, check with the instance's listening IP
                // Only consider local scopes if we don't have a match already
-               localMatchBits := scope.match(net.ParseIP(extconfig.Get().Instance.IP))
+               localMatchBits := scope.match(net.ParseIP(ip))
                if localMatchBits > -1 && localMatchBits > longestBits {
-                       req.log.Debug("selected scope based on cidr match (instance IP)", zap.String("scope", scope.Name))
+                       req.log.Debug("selected scope based on cidr match (instance/interface IP)", zap.String("scope", scope.Name))
                        match = scope
                        longestBits = localMatchBits
                }
@BeryJu BeryJu closed this as completed in 8b4c11b Jan 31, 2024
@BeryJu
Copy link
Owner

BeryJu commented Jan 31, 2024

@GrantRoot1970 thanks a lot for reporting these issues and providing fixes too!

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

No branches or pull requests

2 participants