diff --git a/firewall/interception.go b/firewall/interception.go index 5fa1eb80..025a6c55 100644 --- a/firewall/interception.go +++ b/firewall/interception.go @@ -144,6 +144,10 @@ func fastTrackedPermit(pkt packet.Packet) (handled bool) { // Always permit ICMP. log.Debugf("filter: fast-track accepting ICMP: %s", pkt) + + // If the packet was submitted to the listener, we must not do a + // permanent accept, because then we won't see any future packets of that + // connection and thus cannot continue to submit them. if submitted { _ = pkt.Accept() } else { @@ -157,6 +161,10 @@ func fastTrackedPermit(pkt packet.Packet) (handled bool) { // Always permit ICMPv6. log.Debugf("filter: fast-track accepting ICMPv6: %s", pkt) + + // If the packet was submitted to the listener, we must not do a + // permanent accept, because then we won't see any future packets of that + // connection and thus cannot continue to submit them. if submitted { _ = pkt.Accept() } else { diff --git a/netenv/icmp_listener.go b/netenv/icmp_listener.go index 6b8de2e5..229802b7 100644 --- a/netenv/icmp_listener.go +++ b/netenv/icmp_listener.go @@ -9,12 +9,25 @@ import ( "github.com/safing/portmaster/network/packet" ) +/* +This ICMP listening system is a simple system for components to listen to ICMP +packets via the firewall. + +The main use case for this is to receive ICMP packets that are not always +delivered correctly, or need special permissions and or sockets to receive +them. This is the case when doing a traceroute. + +In order to keep it simple, the system is only designed to be used by one +"user" at at time. Further calls to ListenToICMP will wait for the previous +operation to complete. +*/ + var ( // listenICMPLock locks the ICMP listening system for one user at a time. listenICMPLock sync.Mutex - // listenICMPEnabled defines whether or not the firewall should send ICMP - // packets through this interface. + // listenICMPEnabled defines whether or not the firewall should submit ICMP + // packets to this interface. listenICMPEnabled = abool.New() // listenICMPInput is created for every use of the ICMP listenting system. @@ -22,28 +35,35 @@ var ( listenICMPInputLock sync.Mutex ) +// ListenToICMP returns a new channel for listenting to icmp packets. Please +// note that any icmp packet will be passed and filtering must be done on +// the side of the caller. The caller must call the returned done function when +// done with the listener. func ListenToICMP() (packets chan packet.Packet, done func()) { // Lock for single use. listenICMPLock.Lock() // Create new input channel. listenICMPInputLock.Lock() - listenICMPInput = make(chan packet.Packet, 10) + listenICMPInput = make(chan packet.Packet, 100) listenICMPEnabled.Set() listenICMPInputLock.Unlock() return listenICMPInput, func() { + // Release for someone else to use. + defer listenICMPLock.Unlock() + // Close input channel. listenICMPInputLock.Lock() listenICMPEnabled.UnSet() close(listenICMPInput) listenICMPInputLock.Unlock() - - // Release for someone else to use. - listenICMPLock.Unlock() } } +// SubmitPacketToICMPListener checks if an ICMP packet should be submitted to +// the listener. If so, it is submitted right away. The function returns +// whether or not the packet should be submitted, not if it was successful. func SubmitPacketToICMPListener(pkt packet.Packet) (submitted bool) { // Hot path. if !listenICMPEnabled.IsSet() { @@ -71,8 +91,6 @@ func submitPacketToICMPListenerSlow(pkt packet.Packet) { return } - log.Criticalf("netenv: recvd ICMP packet: %s", pkt) - // Send to channel, if possible. select { case listenICMPInput <- pkt: diff --git a/netenv/location.go b/netenv/location.go index f913306c..b46728a7 100644 --- a/netenv/location.go +++ b/netenv/location.go @@ -66,6 +66,24 @@ type DeviceLocation struct { SourceAccuracy int } +// IsMoreAccurateThan checks if the device location is more accurate than the +// given one. +func (dl *DeviceLocation) IsMoreAccurateThan(other *DeviceLocation) bool { + switch { + case dl.SourceAccuracy > other.SourceAccuracy: + // Higher accuracy is better. + return true + case dl.ASN != 0 && other.ASN == 0: + // Having an ASN is better than having none. + return true + case dl.Country == "" && other.Country != "": + // Having a Country is better than having none. + return true + } + + return false +} + type DeviceLocationSource string const ( @@ -106,28 +124,6 @@ func SetInternetLocation(ip net.IP, source DeviceLocationSource) (ok bool) { SourceAccuracy: source.Accuracy(), } - locationsLock.Lock() - defer locationsLock.Unlock() - - // Add to locations, if better. - key := loc.IP.String() - existing, ok := locations.All[key] - if ok && existing.SourceAccuracy > loc.SourceAccuracy { - // Existing entry is better. - // Return true, because the IP address is part of the locations. - return true - } - locations.All[key] = loc - - // Find best location. - var best *DeviceLocation - for _, dl := range locations.All { - if best == nil || dl.SourceAccuracy > best.SourceAccuracy { - best = dl - } - } - locations.Best = best - // Get geoip information, but continue if it fails. geoLoc, err := geoip.GetLocation(ip) if err != nil { @@ -139,6 +135,28 @@ func SetInternetLocation(ip net.IP, source DeviceLocationSource) (ok bool) { loc.ASOrg = geoLoc.AutonomousSystemOrganization } + locationsLock.Lock() + defer locationsLock.Unlock() + + // Add to locations, if better. + key := loc.IP.String() + existing, ok := locations.All[key] + if ok && existing.IsMoreAccurateThan(loc) { + // Existing entry is more accurate, abort adding. + // Return true, because the IP address is already part of the locations. + return true + } + locations.All[key] = loc + + // Find best location. + best := loc + for _, dl := range locations.All { + if dl.IsMoreAccurateThan(best) { + best = dl + } + } + locations.Best = best + return true } diff --git a/netenv/network-change.go b/netenv/network-change.go index 19a56085..642b534d 100644 --- a/netenv/network-change.go +++ b/netenv/network-change.go @@ -13,16 +13,16 @@ import ( ) var ( - networkChangeCheckTrigger = make(chan struct{}, 1) - networkChangedFlagController = utils.NewFlagController() + networkChangeCheckTrigger = make(chan struct{}, 1) + networkChangedBroadcastFlag = utils.NewBroadcastFlag() ) func GetNetworkChangedFlag() *utils.Flag { - return networkChangedFlagController.NewFlag() + return networkChangedBroadcastFlag.NewFlag() } func notifyOfNetworkChange() { - networkChangedFlagController.NotifyAndReset() + networkChangedBroadcastFlag.NotifyAndReset() module.TriggerEvent(NetworkChangedEvent, nil) } diff --git a/network/packet/parse.go b/network/packet/parse.go index c84a1964..2b978097 100644 --- a/network/packet/parse.go +++ b/network/packet/parse.go @@ -144,8 +144,8 @@ func Parse(packetData []byte, pktBase *Base) (err error) { } pktBase.layers = packet - if packet.TransportLayer() != nil { - pktBase.layer5Data = packet.TransportLayer().LayerPayload() + if transport := packet.TransportLayer(); transport != nil { + pktBase.layer5Data = transport.LayerPayload() } return nil } diff --git a/process/profile.go b/process/profile.go index 37cfcbd5..e55873a2 100644 --- a/process/profile.go +++ b/process/profile.go @@ -58,8 +58,13 @@ func (p *Process) GetProfile(ctx context.Context) (changed bool, err error) { // Check if this is the system resolver. switch runtime.GOOS { case "windows": - if (p.Path == `C:\Windows\System32\svchost.exe` || p.Path == `C:\Windows\system32\svchost.exe`) && - (strings.Contains(p.SpecialDetail, "Dnscache") || strings.Contains(p.CmdLine, "-s Dnscache")) { + // Depending on the OS version System32 may be capitalized or not. + if (p.Path == `C:\Windows\System32\svchost.exe` || + p.Path == `C:\Windows\system32\svchost.exe`) && + // This comes from the windows tasklist command and should be pretty consistent. + (strings.Contains(p.SpecialDetail, "Dnscache") || + // As an alternative in case of failure, we try to match the svchost.exe service parameter. + strings.Contains(p.CmdLine, "-s Dnscache")) { profileID = profile.SystemResolverProfileID } case "linux":