Merge pull request #133 from safing/fix/networking-integration-fasttracking

Fix and improve fast tracked permits in firewall and network integration

firewall/interception.go

@@ -2,11 +2,12 @@ package firewall
 
 import (
 	"context"
-	"net"
 	"os"
 	"sync/atomic"
 	"time"
 
+	"github.com/safing/portmaster/netenv"
+
 	"github.com/safing/portbase/log"
 	"github.com/safing/portbase/modules"
 	"github.com/safing/portmaster/firewall/inspection"
@@ -24,23 +25,10 @@ import (
 var (
 	interceptionModule *modules.Module
 
-	// localNet        net.IPNet
+	packetsAccepted = new(uint64)
-	// localhost net.IP
+	packetsBlocked  = new(uint64)
-	// dnsServer       net.IPNet
+	packetsDropped  = new(uint64)
-	packetsAccepted *uint64
+	packetsFailed   = new(uint64)
-	packetsBlocked  *uint64
-	packetsDropped  *uint64
-	packetsFailed   *uint64
-
-	// localNet4 *net.IPNet
-
-	localhost4 = net.IPv4(127, 0, 0, 1)
-	// localhost6 = net.IPv6loopback
-
-	// tunnelNet4 *net.IPNet
-	// tunnelNet6 *net.IPNet
-	// tunnelEntry4 = net.IPv4(127, 0, 0, 17)
-	// tunnelEntry6 = net.ParseIP("fd17::17")
 )
 
 func init() {
@@ -50,33 +38,7 @@ func init() {
 }
 
 func interceptionPrep() (err error) {
-	err = prepAPIAuth()
+	return prepAPIAuth()
-	if err != nil {
-		return err
-	}
-
-	// _, localNet4, err = net.ParseCIDR("127.0.0.0/24")
-	// // Yes, this would normally be 127.0.0.0/8
-	// // TODO: figure out any side effects
-	// if err != nil {
-	// 	return fmt.Errorf("filter: failed to parse cidr 127.0.0.0/24: %s", err)
-	// }
-
-	// _, tunnelNet4, err = net.ParseCIDR("127.17.0.0/16")
-	// if err != nil {
-	// 	return fmt.Errorf("filter: failed to parse cidr 127.17.0.0/16: %s", err)
-	// }
-	// _, tunnelNet6, err = net.ParseCIDR("fd17::/64")
-	// if err != nil {
-	// 	return fmt.Errorf("filter: failed to parse cidr fd17::/64: %s", err)
-	// }
-
-	packetsAccepted = new(uint64)
-	packetsBlocked = new(uint64)
-	packetsDropped = new(uint64)
-	packetsFailed = new(uint64)
-
-	return nil
 }
 
 func interceptionStart() error {
@@ -94,80 +56,10 @@ func interceptionStop() error {
 }
 
 func handlePacket(pkt packet.Packet) {
-
+	if fastTrackedPermit(pkt) {
-	// allow localhost, for now
-	// if pkt.Info().Src.Equal(pkt.Info().Dst) {
-	// 	log.Debugf("accepting localhost communication: %s", pkt)
-	// 	pkt.PermanentAccept()
-	// 	return
-	// }
-
-	// for UX and performance
-	meta := pkt.Info()
-
-	// allow local dns
-	if (meta.DstPort == 53 || meta.SrcPort == 53) &&
-		(meta.Src.Equal(meta.Dst) || // Windows redirects back to same interface
-			meta.Src.Equal(localhost4) || // Linux sometimes does 127.0.0.1->127.0.0.53
-			meta.Dst.Equal(localhost4)) {
-		log.Debugf("accepting local dns: %s", pkt)
-		_ = pkt.PermanentAccept()
 		return
 	}
 
-	// allow api access, if address was parsed successfully
-	if apiPortSet {
-		if (meta.DstPort == apiPort || meta.SrcPort == apiPort) && meta.Src.Equal(meta.Dst) {
-			log.Debugf("accepting api connection: %s", pkt)
-			_ = pkt.PermanentAccept()
-			return
-		}
-	}
-
-	// // redirect dns (if we know that it's not our own request)
-	// if pkt.IsOutbound() && intel.RemoteIsActiveNameserver(pkt) {
-	// 	log.Debugf("redirecting dns: %s", pkt)
-	// 	pkt.RedirToNameserver()
-	// }
-
-	// allow ICMP and DHCP
-	// TODO: actually handle these
-	switch meta.Protocol {
-	case packet.ICMP:
-		log.Debugf("accepting ICMP: %s", pkt)
-		_ = pkt.PermanentAccept()
-		return
-	case packet.ICMPv6:
-		log.Debugf("accepting ICMPv6: %s", pkt)
-		_ = pkt.PermanentAccept()
-		return
-	case packet.UDP:
-		if meta.DstPort == 67 || meta.DstPort == 68 {
-			log.Debugf("accepting DHCP: %s", pkt)
-			_ = pkt.PermanentAccept()
-			return
-		}
-		// TODO: Howto handle NetBios?
-	}
-
-	// log.Debugf("filter: pkt %s has ID %s", pkt, pkt.GetLinkID())
-
-	// use this to time how long it takes process packet
-	// timed := time.Now()
-	// defer log.Tracef("filter: took %s to process packet %s", time.Now().Sub(timed).String(), pkt)
-
-	// check if packet is destined for tunnel
-	// switch pkt.IPVersion() {
-	// case packet.IPv4:
-	// 	if TunnelNet4 != nil && TunnelNet4.Contains(meta.Dst) {
-	// 		tunnelHandler(pkt)
-	// 	}
-	// case packet.IPv6:
-	// 	if TunnelNet6 != nil && TunnelNet6.Contains(meta.Dst) {
-	// 		tunnelHandler(pkt)
-	// 	}
-	// }
-
 	traceCtx, tracer := log.AddTracer(context.Background())
 	if tracer != nil {
 		pkt.SetCtx(traceCtx)
@@ -188,6 +80,90 @@ func handlePacket(pkt packet.Packet) {
 	conn.HandlePacket(pkt)
 }
 
+// fastTrackedPermit quickly permits certain network criticial or internal connections.
+func fastTrackedPermit(pkt packet.Packet) (handled bool) {
+	meta := pkt.Info()
+
+	switch meta.Protocol {
+	case packet.ICMP:
+		// Always permit ICMP.
+		log.Debugf("accepting ICMP: %s", pkt)
+		_ = pkt.PermanentAccept()
+		return true
+
+	case packet.ICMPv6:
+		// Always permit ICMPv6.
+		log.Debugf("accepting ICMPv6: %s", pkt)
+		_ = pkt.PermanentAccept()
+		return true
+
+	case packet.UDP, packet.TCP:
+		switch meta.DstPort {
+
+		case 67, 68, 546, 547:
+			// Always allow DHCP, DHCPv6.
+
+			// DHCP and DHCPv6 must be UDP.
+			if meta.Protocol != packet.UDP {
+				return false
+			}
+
+			// DHCP is only valid in local network scopes.
+			switch netutils.ClassifyIP(meta.Dst) {
+			case netutils.HostLocal, netutils.LinkLocal, netutils.SiteLocal, netutils.LocalMulticast:
+			default:
+				return false
+			}
+
+			// Log and permit.
+			log.Debugf("accepting DHCP: %s", pkt)
+			_ = pkt.PermanentAccept()
+			return true
+
+		case apiPort:
+			// Always allow direct access to the Portmaster API.
+
+			// Check if the api port is even set.
+			if !apiPortSet {
+				return false
+			}
+
+			// Portmaster API must be TCP
+			if meta.Protocol != packet.TCP {
+				return false
+			}
+
+			fallthrough
+		case 53:
+			// Always allow direct local access to own services.
+			// DNS is both UDP and TCP.
+
+			// Only allow to own IPs.
+			dstIsMe, err := netenv.IsMyIP(meta.Dst)
+			if err != nil {
+				log.Warningf("filter: failed to check if IP is local: %s", err)
+			}
+			if !dstIsMe {
+				return false
+			}
+
+			// Log and permit.
+			switch meta.DstPort {
+			case 53:
+				log.Debugf("accepting local dns: %s", pkt)
+			case apiPort:
+				log.Debugf("accepting api connection: %s", pkt)
+			default:
+				return false
+			}
+			_ = pkt.PermanentAccept()
+			return true
+		}
+	}
+
+	return false
+}
+
 func initialHandler(conn *network.Connection, pkt packet.Packet) {
 	log.Tracer(pkt.Ctx()).Trace("filter: [initial handler]")
 

firewall/interception/nfqueue_linux.go

@@ -115,10 +115,10 @@ func init() {
 		"mangle INPUT -j C171",
 		"filter OUTPUT -j C17",
 		"filter INPUT -j C17",
-		"nat OUTPUT -m mark --mark 1799 -p udp -j DNAT --to [fd17::17]:53",
+		"nat OUTPUT -m mark --mark 1799 -p udp -j DNAT --to [::1]:53",
-		"nat OUTPUT -m mark --mark 1717 -p tcp -j DNAT --to [fd17::17]:717",
+		"nat OUTPUT -m mark --mark 1717 -p tcp -j DNAT --to [::1]:717",
-		"nat OUTPUT -m mark --mark 1717 -p udp -j DNAT --to [fd17::17]:717",
+		"nat OUTPUT -m mark --mark 1717 -p udp -j DNAT --to [::1]:717",
-		// "nat OUTPUT -m mark --mark 1717 ! -p tcp ! -p udp -j DNAT --to [fd17::17]",
+		// "nat OUTPUT -m mark --mark 1717 ! -p tcp ! -p udp -j DNAT --to [::1]",
 	}
 
 	// Reverse because we'd like to insert in a loop

netenv/addresses.go

@@ -51,48 +51,68 @@ func GetAssignedGlobalAddresses() (ipv4 []net.IP, ipv6 []net.IP, err error) {
 }
 
 var (
-	myIPs     []net.IP
+	myNetworks     []*net.IPNet
-	myIPsLock sync.Mutex
+	myNetworksLock sync.Mutex
 )
 
-// IsMyIP returns whether the given IP is currently configured on the local host.
+// IsMyIP returns whether the given unicast IP is currently configured on the local host.
+// Broadcast or multicast addresses will never match, even if valid in in use.
 func IsMyIP(ip net.IP) (yes bool, err error) {
-	if netutils.IPIsLocalhost(ip) {
+	// Check for IPs that don't need extra checks.
+	switch netutils.ClassifyIP(ip) {
+	case netutils.HostLocal:
 		return true, nil
+	case netutils.LocalMulticast, netutils.GlobalMulticast:
+		return false, nil
 	}
 
-	myIPsLock.Lock()
+	myNetworksLock.Lock()
-	defer myIPsLock.Unlock()
+	defer myNetworksLock.Unlock()
 
-	// check
+	// Check for match.
-	for _, myIP := range myIPs {
+	if mine, matched := checkIfMyIP(ip); matched {
-		if ip.Equal(myIP) {
+		return mine, nil
-			return true, nil
-		}
 	}
 
-	// refresh IPs
+	// Refresh assigned networks.
-	myAddrs, err := net.InterfaceAddrs()
+	interfaceNetworks, err := net.InterfaceAddrs()
 	if err != nil {
 		return false, fmt.Errorf("failed to refresh interface addresses: %s", err)
 	}
-	myIPs = make([]net.IP, 0, len(myAddrs))
+	myNetworks = make([]*net.IPNet, 0, len(interfaceNetworks))
-	for _, addr := range myAddrs {
+	for _, ifNet := range interfaceNetworks {
-		netAddr, ok := addr.(*net.IPNet)
+		net, ok := ifNet.(*net.IPNet)
 		if !ok {
-			log.Warningf("netenv: interface address of unexpected type %T", addr)
+			log.Warningf("netenv: interface network of unexpected type %T", ifNet)
 			continue
 		}
 
-		myIPs = append(myIPs, netAddr.IP)
+		myNetworks = append(myNetworks, net)
 	}
 
-	// check again
+	// Check for match again.
-	for _, myIP := range myIPs {
+	if mine, matched := checkIfMyIP(ip); matched {
-		if ip.Equal(myIP) {
+		return mine, nil
-			return true, nil
 	}
-	}
-
 	return false, nil
 }
+
+func checkIfMyIP(ip net.IP) (mine bool, matched bool) {
+	// Check against assigned IPs.
+	for _, myNet := range myNetworks {
+		if ip.Equal(myNet.IP) {
+			return true, true
+		}
+	}
+
+	// Check for other IPs in range and broadcast addresses.
+	// Do this in a second loop, as an IP will match in
+	// most cases and network matching is more expensive.
+	for _, myNet := range myNetworks {
+		if myNet.Contains(ip) {
+			return false, true
+		}
+	}
+
+	return false, false
+}