From 58dfb11786825b6cfd8a3ea36bacbc343c37aa03 Mon Sep 17 00:00:00 2001
From: Paul Lorenz <paul.lorenz@netfoundry.io>
Date: Tue, 15 Jun 2021 16:49:11 -0400
Subject: [PATCH] Fix address remove from loopback on shutdown. Fixes GH-670

---
 tunnel/intercept/svcpoll.go             |  2 ++
 tunnel/intercept/tproxy/tproxy_linux.go | 42 +++++++++++++------------
 tunnel/router/router_linux.go           |  3 ++
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/tunnel/intercept/svcpoll.go b/tunnel/intercept/svcpoll.go
index e7c3414c4..ab6dcc2c8 100644
--- a/tunnel/intercept/svcpoll.go
+++ b/tunnel/intercept/svcpoll.go
@@ -346,11 +346,13 @@ type AddressTracker interface {
 type addrTracker map[string]int
 
 func (self addrTracker) AddAddress(addr string) {
+	logrus.Debugf("adding %v from address tracker: %+v", addr, self)
 	useCnt := self[addr]
 	self[addr] = useCnt + 1
 }
 
 func (self addrTracker) RemoveAddress(addr string) bool {
+	logrus.Debugf("trying to remove %v from address tracker: %+v", addr, self)
 	useCnt := self[addr]
 	if useCnt <= 1 {
 		delete(self, addr)
diff --git a/tunnel/intercept/tproxy/tproxy_linux.go b/tunnel/intercept/tproxy/tproxy_linux.go
index 1f779e4ea..cccc1723d 100644
--- a/tunnel/intercept/tproxy/tproxy_linux.go
+++ b/tunnel/intercept/tproxy/tproxy_linux.go
@@ -30,6 +30,7 @@ import (
 	"github.com/openziti/foundation/util/info"
 	"github.com/openziti/foundation/util/mempool"
 	"github.com/openziti/foundation/util/stringz"
+	"github.com/openziti/sdk-golang/ziti/edge/impl"
 	cmap "github.com/orcaman/concurrent-map"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -331,20 +332,16 @@ func getOriginalDest(oob []byte) (*net.UDPAddr, error) {
 }
 
 func deleteIptablesChain(ipt *iptables.IPTables, table, srcChain, dstChain string) {
-	log := pfxlog.Logger()
-	err := ipt.Delete(table, srcChain, []string{"-j", dstChain}...)
-	if err != nil {
-		log.Errorf("failed to unlink chain %s: %v", dstChain, err)
+	if err := ipt.Delete(table, srcChain, []string{"-j", dstChain}...); err != nil {
+		logrus.Errorf("failed to unlink chain %s: %v", dstChain, err)
 	}
 
-	err = ipt.ClearChain(table, dstChain)
-	if err != nil {
-		log.Errorf("failed to clear chain %s, %v", dstChain, err)
+	if err := ipt.ClearChain(table, dstChain); err != nil {
+		logrus.Errorf("failed to clear chain %s, %v", dstChain, err)
 	}
 
-	err = ipt.DeleteChain(table, dstChain)
-	if err != nil {
-		log.Errorf("failed to delete chain %s: %v", dstChain, err)
+	if err := ipt.DeleteChain(table, dstChain); err != nil {
+		logrus.Errorf("failed to delete chain %s: %v", dstChain, err)
 	}
 }
 
@@ -422,6 +419,7 @@ func (self *tProxy) addInterceptAddr(interceptAddr *intercept.InterceptAddress,
 		return fmt.Errorf("failed to add local route: %v", err)
 	}
 	tracker.AddAddress(ipNet.String())
+	self.addresses = append(self.addresses, interceptAddr)
 
 	interceptAddr.TproxySpec = []string{
 		"-m", "comment", "--comment", service.Name,
@@ -458,32 +456,36 @@ func (self *tProxy) addInterceptAddr(interceptAddr *intercept.InterceptAddress,
 }
 
 func (self *tProxy) StopIntercepting(tracker intercept.AddressTracker) error {
-	// keep track of routes used by all intercepts. use a map to avoid duplicates
-	routes := map[string]*net.IPNet{}
+	var errors []error
+
 	for _, addr := range self.addresses {
 		err := self.interceptor.ipt.Delete(mangleTable, dstChain, addr.TproxySpec...)
 		if err != nil {
-			return fmt.Errorf("failed to remove iptables rule for service %s: %v", self.service.Name, err)
+			errors = append(errors, err)
+			logrus.Errorf("failed to remove iptables rule for service %s: %v", self.service.Name, err)
 		}
 		if self.interceptor.lanIf != "" {
 			err = self.interceptor.ipt.Delete(filterTable, dstChain, addr.AcceptSpec...)
 			if err != nil {
-				return fmt.Errorf("failed to remove iptables rule for service %s: %v", self.service.Name, err)
+				errors = append(errors, err)
+				logrus.Errorf("failed to remove iptables rule for service %s: %v", self.service.Name, err)
 			}
 		}
-		ipn := addr.IpNet()
-		routes[ipn.String()] = ipn
-	}
-	for _, ipNet := range routes {
+
+		ipNet := addr.IpNet()
 		if tracker.RemoveAddress(ipNet.String()) {
 			err := router.RemoveLocalAddress(ipNet, "lo")
 			if err != nil {
-				return fmt.Errorf("failed to remove route for service %s: %v", self.service.Name, err)
+				errors = append(errors, err)
+				logrus.Errorf("failed to remove route for service %s: %v", self.service.Name, err)
 			}
 		}
 	}
 
-	return nil
+	if len(errors) == 0 {
+		return nil
+	}
+	return impl.MultipleErrors(errors)
 }
 
 type IPPortAddr interface {
diff --git a/tunnel/router/router_linux.go b/tunnel/router/router_linux.go
index 4047f8dfe..53d0d2b50 100644
--- a/tunnel/router/router_linux.go
+++ b/tunnel/router/router_linux.go
@@ -22,6 +22,7 @@ import (
 	"github.com/mdlayher/netlink"
 	"github.com/mdlayher/netlink/nlenc"
 	"github.com/michaelquigley/pfxlog"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 	"net"
 	"os"
@@ -29,10 +30,12 @@ import (
 
 // Add an address (or prefix) to the specified network interface.
 func AddLocalAddress(prefix *net.IPNet, ifName string) error {
+	logrus.Debugf("adding local address '%v' to interface %v", prefix.String(), ifName)
 	return nlAddrReq(prefix, nil, ifName, unix.RTM_NEWADDR)
 }
 
 func RemoveLocalAddress(prefix *net.IPNet, ifName string) error {
+	logrus.Debugf("removing local address '%v' from interface %v", prefix.String(), ifName)
 	return nlAddrReq(prefix, nil, ifName, unix.RTM_DELADDR)
 }