From f75fc7d1624a9f65ac629e8a59de26c018299f5c Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 7 Nov 2019 16:13:22 +0100 Subject: [PATCH] Clean up linter errors --- firewall/firewall.go | 104 ++++++++++-------- firewall/inspection/inspection.go | 5 +- firewall/interception/nfqueue/doc.go | 2 +- firewall/interception/nfqueue/nfqueue.go | 83 +++++++------- firewall/interception/nfqueue/packet.go | 23 +++- firewall/interception/nfqueue_linux.go | 19 +--- firewall/interception/windowskext/handler.go | 1 + firewall/interception/windowskext/kext.go | 1 + firewall/interception/windowskext/packet.go | 1 + .../windowskext/test/endian/main.go | 26 ----- firewall/master.go | 5 +- firewall/ports.go | 4 +- firewall/prompt.go | 24 ++-- intel/resolve.go | 1 + intel/resolver.go | 2 + nameserver/nameserver.go | 4 +- nameserver/only/nameserver.go | 4 +- network/clean.go | 13 ++- network/communication.go | 6 +- network/database.go | 2 +- network/environment/main.go | 2 + network/environment/main_test.go | 35 ++++++ network/geoip/location.go | 2 +- network/geoip/module.go | 13 +-- network/link.go | 64 +++++++---- pmctl/logs.go | 5 +- pmctl/main.go | 7 +- pmctl/run.go | 11 +- pmctl/service.go | 6 +- process/database.go | 3 +- process/matching.go | 35 ++---- process/proc/processfinder.go | 2 +- process/proc/sockets.go | 13 --- process/process.go | 74 ++++++++----- process/process_linux.go | 18 +-- profile/defaults.go | 2 +- profile/endpoints.go | 9 +- profile/endpoints_test.go | 12 +- profile/fingerprint.go | 4 +- profile/profile.go | 16 +-- profile/set.go | 4 - profile/set_test.go | 19 ++-- profile/specialprofiles.go | 10 +- profile/updates.go | 2 +- status/database.go | 4 + status/module.go | 12 +- ui/serve.go | 1 - updates/export.go | 6 +- updates/main.go | 4 +- updates/upgrader.go | 11 +- 50 files changed, 402 insertions(+), 334 deletions(-) delete mode 100644 firewall/interception/windowskext/test/endian/main.go create mode 100644 network/environment/main_test.go diff --git a/firewall/firewall.go b/firewall/firewall.go index 83b1840d..2bb49af5 100644 --- a/firewall/firewall.go +++ b/firewall/firewall.go @@ -3,7 +3,6 @@ package firewall import ( "context" "fmt" - "net" "os" "sync/atomic" "time" @@ -21,26 +20,28 @@ import ( ) var ( + module *modules.Module + // localNet net.IPNet - localhost net.IP - dnsServer net.IPNet + // localhost net.IP + // dnsServer net.IPNet packetsAccepted *uint64 packetsBlocked *uint64 packetsDropped *uint64 - localNet4 *net.IPNet + // localNet4 *net.IPNet - localhost4 = net.IPv4(127, 0, 0, 1) - localhost6 = net.IPv6loopback + // 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") + // tunnelNet4 *net.IPNet + // tunnelNet6 *net.IPNet + // tunnelEntry4 = net.IPv4(127, 0, 0, 17) + // tunnelEntry6 = net.ParseIP("fd17::17") ) func init() { - modules.Register("firewall", prep, start, stop, "core", "network", "nameserver", "profile", "updates") + module = modules.Register("firewall", prep, start, stop, "core", "network", "nameserver", "profile", "updates") } func prep() (err error) { @@ -55,21 +56,21 @@ func prep() (err error) { 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("firewall: failed to parse cidr 127.0.0.0/24: %s", 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("firewall: 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("firewall: failed to parse cidr 127.17.0.0/16: %s", err) - } - _, tunnelNet6, err = net.ParseCIDR("fd17::/64") - if err != nil { - return fmt.Errorf("firewall: failed to parse cidr fd17::/64: %s", err) - } + // _, tunnelNet4, err = net.ParseCIDR("127.17.0.0/16") + // if err != nil { + // return fmt.Errorf("firewall: failed to parse cidr 127.17.0.0/16: %s", err) + // } + // _, tunnelNet6, err = net.ParseCIDR("fd17::/64") + // if err != nil { + // return fmt.Errorf("firewall: failed to parse cidr fd17::/64: %s", err) + // } var pA uint64 packetsAccepted = &pA @@ -83,9 +84,21 @@ func prep() (err error) { func start() error { startAPIAuth() - go statLogger() - go run() - go portsInUseCleaner() + + module.StartWorker("stat logger", func(ctx context.Context) error { + statLogger() + return nil + }) + + module.StartWorker("packet handler", func(ctx context.Context) error { + run() + return nil + }) + + module.StartWorker("ports state cleaner", func(ctx context.Context) error { + portsInUseCleaner() + return nil + }) return interception.Start() } @@ -106,7 +119,7 @@ func handlePacket(pkt packet.Packet) { // allow local dns if (pkt.Info().DstPort == 53 || pkt.Info().SrcPort == 53) && pkt.Info().Src.Equal(pkt.Info().Dst) { log.Debugf("accepting local dns: %s", pkt) - pkt.PermanentAccept() + _ = pkt.PermanentAccept() return } @@ -114,7 +127,7 @@ func handlePacket(pkt packet.Packet) { if apiPortSet { if (pkt.Info().DstPort == apiPort || pkt.Info().SrcPort == apiPort) && pkt.Info().Src.Equal(pkt.Info().Dst) { log.Debugf("accepting api connection: %s", pkt) - pkt.PermanentAccept() + _ = pkt.PermanentAccept() return } } @@ -130,20 +143,20 @@ func handlePacket(pkt packet.Packet) { switch pkt.Info().Protocol { case packet.ICMP: log.Debugf("accepting ICMP: %s", pkt) - pkt.PermanentAccept() + _ = pkt.PermanentAccept() return case packet.ICMPv6: log.Debugf("accepting ICMPv6: %s", pkt) - pkt.PermanentAccept() + _ = pkt.PermanentAccept() return case packet.IGMP: log.Debugf("accepting IGMP: %s", pkt) - pkt.PermanentAccept() + _ = pkt.PermanentAccept() return case packet.UDP: if pkt.Info().DstPort == 67 || pkt.Info().DstPort == 68 { log.Debugf("accepting DHCP: %s", pkt) - pkt.PermanentAccept() + _ = pkt.PermanentAccept() return } // TODO: Howto handle NetBios? @@ -310,39 +323,44 @@ func issueVerdict(pkt packet.Packet, link *network.Link, verdict network.Verdict verdict = link.Verdict } + var err error switch verdict { case network.VerdictAccept: atomic.AddUint64(packetsAccepted, 1) if link.VerdictPermanent { - pkt.PermanentAccept() + err = pkt.PermanentAccept() } else { - pkt.Accept() + err = pkt.Accept() } case network.VerdictBlock: atomic.AddUint64(packetsBlocked, 1) if link.VerdictPermanent { - pkt.PermanentBlock() + err = pkt.PermanentBlock() } else { - pkt.Block() + err = pkt.Block() } case network.VerdictDrop: atomic.AddUint64(packetsDropped, 1) if link.VerdictPermanent { - pkt.PermanentDrop() + err = pkt.PermanentDrop() } else { - pkt.Drop() + err = pkt.Drop() } case network.VerdictRerouteToNameserver: - pkt.RerouteToNameserver() + err = pkt.RerouteToNameserver() case network.VerdictRerouteToTunnel: - pkt.RerouteToTunnel() + err = pkt.RerouteToTunnel() default: atomic.AddUint64(packetsDropped, 1) - pkt.Drop() + err = pkt.Drop() } link.Unlock() + if err != nil { + log.Warningf("firewall: failed to apply verdict to pkt %s: %s", pkt, err) + } + log.Tracer(pkt.Ctx()).Infof("firewall: %s %s", link.Verdict, link) } diff --git a/firewall/inspection/inspection.go b/firewall/inspection/inspection.go index cce18084..70ef4e06 100644 --- a/firewall/inspection/inspection.go +++ b/firewall/inspection/inspection.go @@ -7,6 +7,7 @@ import ( "github.com/safing/portmaster/network/packet" ) +//nolint:golint,stylecheck // FIXME const ( DO_NOTHING uint8 = iota BLOCK_PACKET @@ -25,6 +26,7 @@ var ( inspectorsLock sync.Mutex ) +// RegisterInspector registers a traffic inspector. func RegisterInspector(name string, inspector inspectorFn, inspectVerdict network.Verdict) (index int) { inspectorsLock.Lock() defer inspectorsLock.Unlock() @@ -35,13 +37,14 @@ func RegisterInspector(name string, inspector inspectorFn, inspectVerdict networ return } +// RunInspectors runs all the applicable inspectors on the given packet. func RunInspectors(pkt packet.Packet, link *network.Link) (network.Verdict, bool) { // inspectorsLock.Lock() // defer inspectorsLock.Unlock() activeInspectors := link.GetActiveInspectors() if activeInspectors == nil { - activeInspectors = make([]bool, len(inspectors), len(inspectors)) + activeInspectors = make([]bool, len(inspectors)) link.SetActiveInspectors(activeInspectors) } diff --git a/firewall/interception/nfqueue/doc.go b/firewall/interception/nfqueue/doc.go index c6f78c8b..13be9112 100644 --- a/firewall/interception/nfqueue/doc.go +++ b/firewall/interception/nfqueue/doc.go @@ -1,2 +1,2 @@ -// Package nfqueue provides network interception capabilites on linux via iptables nfqueue. +// Package nfqueue provides network interception capabilities on linux via iptables nfqueue. package nfqueue diff --git a/firewall/interception/nfqueue/nfqueue.go b/firewall/interception/nfqueue/nfqueue.go index da4b0575..fc238744 100644 --- a/firewall/interception/nfqueue/nfqueue.go +++ b/firewall/interception/nfqueue/nfqueue.go @@ -27,6 +27,8 @@ func init() { queues = make(map[uint16]*NFQueue) } +// NFQueue holds a Linux NFQ Handle and associated information. +//nolint:maligned // FIXME type NFQueue struct { DefaultVerdict uint32 Timeout time.Duration @@ -41,6 +43,7 @@ type NFQueue struct { Packets chan packet.Packet } +// NewNFQueue initializes a new netfilter queue. func NewNFQueue(qid uint16) (nfq *NFQueue, err error) { if os.Geteuid() != 0 { return nil, errors.New("must be root to intercept packets") @@ -61,96 +64,98 @@ func NewNFQueue(qid uint16) (nfq *NFQueue, err error) { return nfq, nil } -func (this *NFQueue) init() error { +func (nfq *NFQueue) init() error { var err error - if this.h, err = C.nfq_open(); err != nil || this.h == nil { + if nfq.h, err = C.nfq_open(); err != nil || nfq.h == nil { return fmt.Errorf("could not open nfqueue: %s", err) } - //if this.qh, err = C.nfq_create_queue(this.h, qid, C.get_cb(), unsafe.Pointer(nfq)); err != nil || this.qh == nil { + //if nfq.qh, err = C.nfq_create_queue(nfq.h, qid, C.get_cb(), unsafe.Pointer(nfq)); err != nil || nfq.qh == nil { - this.Packets = make(chan packet.Packet, 1) + nfq.Packets = make(chan packet.Packet, 1) - if C.nfq_unbind_pf(this.h, C.AF_INET) < 0 { - this.Destroy() + if C.nfq_unbind_pf(nfq.h, C.AF_INET) < 0 { + nfq.Destroy() return errors.New("nfq_unbind_pf(AF_INET) failed, are you root?") } - if C.nfq_unbind_pf(this.h, C.AF_INET6) < 0 { - this.Destroy() + if C.nfq_unbind_pf(nfq.h, C.AF_INET6) < 0 { + nfq.Destroy() return errors.New("nfq_unbind_pf(AF_INET6) failed") } - if C.nfq_bind_pf(this.h, C.AF_INET) < 0 { - this.Destroy() + if C.nfq_bind_pf(nfq.h, C.AF_INET) < 0 { + nfq.Destroy() return errors.New("nfq_bind_pf(AF_INET) failed") } - if C.nfq_bind_pf(this.h, C.AF_INET6) < 0 { - this.Destroy() + if C.nfq_bind_pf(nfq.h, C.AF_INET6) < 0 { + nfq.Destroy() return errors.New("nfq_bind_pf(AF_INET6) failed") } - if this.qh, err = C.create_queue(this.h, C.uint16_t(this.qid)); err != nil || this.qh == nil { - C.nfq_close(this.h) + if nfq.qh, err = C.create_queue(nfq.h, C.uint16_t(nfq.qid)); err != nil || nfq.qh == nil { + C.nfq_close(nfq.h) return fmt.Errorf("could not create queue: %s", err) } - this.fd = int(C.nfq_fd(this.h)) + nfq.fd = int(C.nfq_fd(nfq.h)) - if C.nfq_set_mode(this.qh, C.NFQNL_COPY_PACKET, 0xffff) < 0 { - this.Destroy() + if C.nfq_set_mode(nfq.qh, C.NFQNL_COPY_PACKET, 0xffff) < 0 { + nfq.Destroy() return errors.New("nfq_set_mode(NFQNL_COPY_PACKET) failed") } - if C.nfq_set_queue_maxlen(this.qh, 1024*8) < 0 { - this.Destroy() + if C.nfq_set_queue_maxlen(nfq.qh, 1024*8) < 0 { + nfq.Destroy() return errors.New("nfq_set_queue_maxlen(1024 * 8) failed") } return nil } -func (this *NFQueue) Destroy() { - this.lk.Lock() - defer this.lk.Unlock() +// Destroy closes all the nfqueues. +func (nfq *NFQueue) Destroy() { + nfq.lk.Lock() + defer nfq.lk.Unlock() - if this.fd != 0 && this.Valid() { - syscall.Close(this.fd) + if nfq.fd != 0 && nfq.Valid() { + syscall.Close(nfq.fd) } - if this.qh != nil { - C.nfq_destroy_queue(this.qh) - this.qh = nil + if nfq.qh != nil { + C.nfq_destroy_queue(nfq.qh) + nfq.qh = nil } - if this.h != nil { - C.nfq_close(this.h) - this.h = nil + if nfq.h != nil { + C.nfq_close(nfq.h) + nfq.h = nil } // TODO: don't close, we're exiting anyway - // if this.Packets != nil { - // close(this.Packets) + // if nfq.Packets != nil { + // close(nfq.Packets) // } } -func (this *NFQueue) Valid() bool { - return this.h != nil && this.qh != nil +// Valid returns whether the NFQueue is still valid. +func (nfq *NFQueue) Valid() bool { + return nfq.h != nil && nfq.qh != nil } //export go_nfq_callback func go_nfq_callback(id uint32, hwproto uint16, hook uint8, mark *uint32, version, protocol, tos, ttl uint8, saddr, daddr unsafe.Pointer, - sport, dport, checksum uint16, payload_len uint32, payload, data unsafe.Pointer) (v uint32) { + sport, dport, checksum uint16, payloadLen uint32, payload, data unsafe.Pointer) (v uint32) { qidptr := (*uint16)(data) - qid := uint16(*qidptr) + qid := *qidptr // nfq := (*NFQueue)(nfqptr) ipVersion := packet.IPVersion(version) ipsz := C.int(ipVersion.ByteSize()) - bs := C.GoBytes(payload, (C.int)(payload_len)) + bs := C.GoBytes(payload, (C.int)(payloadLen)) verdict := make(chan uint32, 1) pkt := Packet{ - QueueId: qid, - Id: id, + QueueID: qid, + ID: id, HWProtocol: hwproto, Hook: hook, Mark: *mark, diff --git a/firewall/interception/nfqueue/packet.go b/firewall/interception/nfqueue/packet.go index ea873ddc..35ffa091 100644 --- a/firewall/interception/nfqueue/packet.go +++ b/firewall/interception/nfqueue/packet.go @@ -1,15 +1,18 @@ package nfqueue import ( - "fmt" + "errors" "github.com/safing/portmaster/network/packet" ) +// NFQ Errors var ( - ErrVerdictSentOrTimedOut error = fmt.Errorf("The verdict was already sent or timed out.") + ErrVerdictSentOrTimedOut = errors.New("the verdict was already sent or timed out") ) +// NFQ Packet Constants +//nolint:golint,stylecheck // FIXME const ( NFQ_DROP uint32 = 0 // discarded the packet NFQ_ACCEPT uint32 = 1 // the packet passes, continue iterations @@ -19,11 +22,12 @@ const ( NFQ_STOP uint32 = 5 // accept, but don't continue iterations ) +// Packet represents a packet with a NFQ reference. type Packet struct { packet.Base - QueueId uint16 - Id uint32 + QueueID uint16 + ID uint32 HWProtocol uint16 Hook uint8 Mark uint32 @@ -35,9 +39,10 @@ type Packet struct { // func (pkt *Packet) String() string { // return fmt.Sprintf("", -// pkt.QueueId, pkt.Id, pkt.Protocol, pkt.Src, pkt.SrcPort, pkt.Dst, pkt.DstPort, pkt.Mark, pkt.Checksum, pkt.Tos, pkt.TTL) +// pkt.QueueID, pkt.Id, pkt.Protocol, pkt.Src, pkt.SrcPort, pkt.Dst, pkt.DstPort, pkt.Mark, pkt.Checksum, pkt.Tos, pkt.TTL) // } +//nolint:unparam // FIXME func (pkt *Packet) setVerdict(v uint32) (err error) { defer func() { if x := recover(); x != nil { @@ -68,41 +73,49 @@ func (pkt *Packet) setVerdict(v uint32) (err error) { // return pkt.setVerdict(NFQ_DROP) // } +// Accept implements the packet interface. func (pkt *Packet) Accept() error { pkt.Mark = 1700 return pkt.setVerdict(NFQ_ACCEPT) } +// Block implements the packet interface. func (pkt *Packet) Block() error { pkt.Mark = 1701 return pkt.setVerdict(NFQ_ACCEPT) } +// Drop implements the packet interface. func (pkt *Packet) Drop() error { pkt.Mark = 1702 return pkt.setVerdict(NFQ_ACCEPT) } +// PermanentAccept implements the packet interface. func (pkt *Packet) PermanentAccept() error { pkt.Mark = 1710 return pkt.setVerdict(NFQ_ACCEPT) } +// PermanentBlock implements the packet interface. func (pkt *Packet) PermanentBlock() error { pkt.Mark = 1711 return pkt.setVerdict(NFQ_ACCEPT) } +// PermanentDrop implements the packet interface. func (pkt *Packet) PermanentDrop() error { pkt.Mark = 1712 return pkt.setVerdict(NFQ_ACCEPT) } +// RerouteToNameserver implements the packet interface. func (pkt *Packet) RerouteToNameserver() error { pkt.Mark = 1799 return pkt.setVerdict(NFQ_ACCEPT) } +// RerouteToTunnel implements the packet interface. func (pkt *Packet) RerouteToTunnel() error { pkt.Mark = 1717 return pkt.setVerdict(NFQ_ACCEPT) diff --git a/firewall/interception/nfqueue_linux.go b/firewall/interception/nfqueue_linux.go index 17e1fb2a..48cebadc 100644 --- a/firewall/interception/nfqueue_linux.go +++ b/firewall/interception/nfqueue_linux.go @@ -247,28 +247,28 @@ func StartNfqueueInterception() (err error) { err = activateNfqueueFirewall() if err != nil { - Stop() + _ = Stop() return fmt.Errorf("could not initialize nfqueue: %s", err) } out4Queue, err = nfqueue.NewNFQueue(17040) if err != nil { - Stop() + _ = Stop() return fmt.Errorf("interception: failed to create nfqueue(IPv4, in): %s", err) } in4Queue, err = nfqueue.NewNFQueue(17140) if err != nil { - Stop() + _ = Stop() return fmt.Errorf("interception: failed to create nfqueue(IPv4, in): %s", err) } out6Queue, err = nfqueue.NewNFQueue(17060) if err != nil { - Stop() + _ = Stop() return fmt.Errorf("interception: failed to create nfqueue(IPv4, in): %s", err) } in6Queue, err = nfqueue.NewNFQueue(17160) if err != nil { - Stop() + _ = Stop() return fmt.Errorf("interception: failed to create nfqueue(IPv4, in): %s", err) } @@ -321,12 +321,3 @@ func handleInterception() { } } } - -func stringInSlice(slice []string, value string) bool { - for _, entry := range slice { - if value == entry { - return true - } - } - return false -} diff --git a/firewall/interception/windowskext/handler.go b/firewall/interception/windowskext/handler.go index 4419f157..f33cf025 100644 --- a/firewall/interception/windowskext/handler.go +++ b/firewall/interception/windowskext/handler.go @@ -1,4 +1,5 @@ // +build windows + package windowskext import ( diff --git a/firewall/interception/windowskext/kext.go b/firewall/interception/windowskext/kext.go index 96800b06..aabd8fd1 100644 --- a/firewall/interception/windowskext/kext.go +++ b/firewall/interception/windowskext/kext.go @@ -1,4 +1,5 @@ // +build windows + package windowskext import ( diff --git a/firewall/interception/windowskext/packet.go b/firewall/interception/windowskext/packet.go index 98cf0985..cb43d598 100644 --- a/firewall/interception/windowskext/packet.go +++ b/firewall/interception/windowskext/packet.go @@ -1,4 +1,5 @@ // +build windows + package windowskext import ( diff --git a/firewall/interception/windowskext/test/endian/main.go b/firewall/interception/windowskext/test/endian/main.go deleted file mode 100644 index 22116b78..00000000 --- a/firewall/interception/windowskext/test/endian/main.go +++ /dev/null @@ -1,26 +0,0 @@ -package main - -import ( - "fmt" - "unsafe" -) - -const integerSize int = int(unsafe.Sizeof(0)) - -func isBigEndian() bool { - var i int = 0x1 - bs := (*[integerSize]byte)(unsafe.Pointer(&i)) - if bs[0] == 0 { - return true - } else { - return false - } -} - -func main() { - if isBigEndian() { - fmt.Println("System is Big Endian (Network Byte Order): uint16 0x1234 is 0x1234 in memory") - } else { - fmt.Println("System is Little Endian (Host Byte Order): uint16 0x1234 is 0x3412 in memory") - } -} diff --git a/firewall/master.go b/firewall/master.go index f4519f90..15040764 100644 --- a/firewall/master.go +++ b/firewall/master.go @@ -139,6 +139,7 @@ func DecideOnCommunicationAfterIntel(comm *network.Communication, fqdn string, r } // FilterDNSResponse filters a dns response according to the application profile and settings. +//nolint:gocognit // FIXME func FilterDNSResponse(comm *network.Communication, q *intel.Query, rrCache *intel.RRCache) *intel.RRCache { // do not modify own queries - this should not happen anyway if comm.Process().Pid == os.Getpid() { @@ -497,7 +498,7 @@ func checkRelation(comm *network.Communication, fqdn string) (related bool) { // TODO: add #AI - pathElements := strings.Split(comm.Process().Path, "/") // FIXME: path seperator + pathElements := strings.Split(comm.Process().Path, "/") // FIXME: path separator // only look at the last two path segments if len(pathElements) > 2 { pathElements = pathElements[len(pathElements)-2:] @@ -537,5 +538,5 @@ matchLoop: log.Infof("firewall: permitting communication %s, match to domain was found: %s is related to %s", comm, domainElement, processElement) comm.Accept(fmt.Sprintf("domain is related to process: %s is related to %s", domainElement, processElement)) } - return + return related } diff --git a/firewall/ports.go b/firewall/ports.go index 4271a924..b6b13189 100644 --- a/firewall/ports.go +++ b/firewall/ports.go @@ -80,10 +80,10 @@ func cleanPortsInUse() { portsInUseLock.Lock() defer portsInUseLock.Unlock() - threshhold := time.Now().Add(-cleanTimeout) + threshold := time.Now().Add(-cleanTimeout) for port, status := range portsInUse { - if status.lastSeen.Before(threshhold) { + if status.lastSeen.Before(threshold) { delete(portsInUse, port) } } diff --git a/firewall/prompt.go b/firewall/prompt.go index d50fe3ad..a6cd160f 100644 --- a/firewall/prompt.go +++ b/firewall/prompt.go @@ -1,6 +1,7 @@ package firewall import ( + "context" "fmt" "time" @@ -24,6 +25,11 @@ const ( denyServingIP = "deny-serving-ip" ) +var ( + mtSaveProfile = "save profile" +) + +//nolint:gocognit // FIXME func prompt(comm *network.Communication, link *network.Link, pkt packet.Packet, fqdn string) { nTTL := time.Duration(promptTimeout()) * time.Second @@ -66,11 +72,11 @@ func prompt(comm *network.Communication, link *network.Link, pkt packet.Packet, case comm.Direction: // incoming n.Message = fmt.Sprintf("Application %s wants to accept connections from %s (on %d/%d)", comm.Process(), pkt.Info().RemoteIP(), pkt.Info().Protocol, pkt.Info().LocalPort()) n.AvailableActions = []*notifications.Action{ - ¬ifications.Action{ + { ID: permitServingIP, Text: "Permit", }, - ¬ifications.Action{ + { ID: denyServingIP, Text: "Deny", }, @@ -78,11 +84,11 @@ func prompt(comm *network.Communication, link *network.Link, pkt packet.Packet, case fqdn == "": // direct connection n.Message = fmt.Sprintf("Application %s wants to connect to %s (on %d/%d)", comm.Process(), pkt.Info().RemoteIP(), pkt.Info().Protocol, pkt.Info().RemotePort()) n.AvailableActions = []*notifications.Action{ - ¬ifications.Action{ + { ID: permitIP, Text: "Permit", }, - ¬ifications.Action{ + { ID: denyIP, Text: "Deny", }, @@ -94,15 +100,15 @@ func prompt(comm *network.Communication, link *network.Link, pkt packet.Packet, n.Message = fmt.Sprintf("Application %s wants to connect to %s", comm.Process(), comm.Domain) } n.AvailableActions = []*notifications.Action{ - ¬ifications.Action{ + { ID: permitDomainAll, Text: "Permit all", }, - ¬ifications.Action{ + { ID: permitDomainDistinct, Text: "Permit", }, - ¬ifications.Action{ + { ID: denyDomainDistinct, Text: "Deny", }, @@ -182,7 +188,9 @@ func prompt(comm *network.Communication, link *network.Link, pkt packet.Packet, } // save! - go userProfile.Save("") + module.StartMicroTask(&mtSaveProfile, func(ctx context.Context) error { + return userProfile.Save("") + }) case <-n.Expired(): if link != nil { diff --git a/intel/resolve.go b/intel/resolve.go index b3a9d0eb..e8c4dddc 100644 --- a/intel/resolve.go +++ b/intel/resolve.go @@ -37,6 +37,7 @@ var ( ErrNoCompliance = fmt.Errorf("%w: no compliant resolvers for this query", ErrBlocked) ) +// Query describes a dns query. type Query struct { FQDN string QType dns.Type diff --git a/intel/resolver.go b/intel/resolver.go index a0d74a77..f475b7f5 100644 --- a/intel/resolver.go +++ b/intel/resolver.go @@ -11,6 +11,7 @@ import ( "github.com/safing/portmaster/network/environment" ) +// DNS Resolver Attributes const ( ServerTypeDNS = "dns" ServerTypeTCP = "tcp" @@ -85,6 +86,7 @@ func (brc *BasicResolverConn) LastFail() time.Time { return brc.lastFail } +// Query executes the given query against the resolver. func (brc *BasicResolverConn) Query(ctx context.Context, q *Query) (*RRCache, error) { // convenience resolver := brc.resolver diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index 00799984..e2caf375 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -25,7 +25,7 @@ var ( mtDNSRequest = "dns request" listenAddress = "0.0.0.0:53" - IPv4Localhost = net.IPv4(127, 0, 0, 1) + ipv4Localhost = net.IPv4(127, 0, 0, 1) localhostRRs []dns.RR ) @@ -135,7 +135,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, query *dns.Msg) er log.Warningf("nameserver: could not get remote address of request for %s%s, ignoring", q.FQDN, q.QType) return nil } - if !remoteAddr.IP.Equal(IPv4Localhost) { + if !remoteAddr.IP.Equal(ipv4Localhost) { // if request is not coming from 127.0.0.1, check if it's really local localAddr, ok := w.RemoteAddr().(*net.UDPAddr) diff --git a/nameserver/only/nameserver.go b/nameserver/only/nameserver.go index 1a392bf3..24920477 100644 --- a/nameserver/only/nameserver.go +++ b/nameserver/only/nameserver.go @@ -22,7 +22,7 @@ var ( mtDNSRequest = "dns request" listenAddress = "127.0.0.1:53" - IPv4Localhost = net.IPv4(127, 0, 0, 1) + ipv4Localhost = net.IPv4(127, 0, 0, 1) localhostRRs []dns.RR ) @@ -127,7 +127,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, query *dns.Msg) er log.Warningf("nameserver: could not get remote address of request for %s%s, ignoring", q.FQDN, q.QType) return nil } - if !remoteAddr.IP.Equal(IPv4Localhost) { + if !remoteAddr.IP.Equal(ipv4Localhost) { // if request is not coming from 127.0.0.1, check if it's really local localAddr, ok := w.RemoteAddr().(*net.UDPAddr) diff --git a/network/clean.go b/network/clean.go index b4e5560c..fd41d82f 100644 --- a/network/clean.go +++ b/network/clean.go @@ -1,6 +1,7 @@ package network import ( + "context" "time" "github.com/safing/portbase/log" @@ -11,7 +12,8 @@ var ( cleanerTickDuration = 10 * time.Second deleteLinksAfterEndedThreshold = 5 * time.Minute deleteCommsWithoutLinksThreshhold = 3 * time.Minute - lastEstablishedUpdateThreshold = 30 * time.Second + + mtSaveLink = "save network link" ) func cleaner() { @@ -68,12 +70,17 @@ func cleanLinks() (activeComms map[string]struct{}) { link.Ended = now link.Unlock() log.Tracef("network.clean: marked %s as ended", link.DatabaseKey()) - go link.save() + // save + linkToSave := link + module.StartMicroTask(&mtSaveLink, func(ctx context.Context) error { + linkToSave.saveAndLog() + return nil + }) } } - return + return activeComms } func cleanComms(activeLinks map[string]struct{}) (activeComms map[string]struct{}) { diff --git a/network/communication.go b/network/communication.go index cedd26f6..d2324ba8 100644 --- a/network/communication.go +++ b/network/communication.go @@ -18,6 +18,7 @@ import ( ) // Communication describes a logical connection between a process and a domain. +//nolint:maligned // TODO: fix alignment type Communication struct { record.Base sync.Mutex @@ -288,7 +289,10 @@ func (comm *Communication) SaveIfNeeded() { comm.Unlock() if save { - comm.save() + err := comm.save() + if err != nil { + log.Warningf("network: failed to save comm %s: %s", comm, err) + } } } diff --git a/network/database.go b/network/database.go index 72dbb7c3..9c31200b 100644 --- a/network/database.go +++ b/network/database.go @@ -32,7 +32,7 @@ type StorageInterface struct { func (s *StorageInterface) Get(key string) (record.Record, error) { splitted := strings.Split(key, "/") - switch splitted[0] { + switch splitted[0] { //nolint:gocritic // TODO: implement full key space case "tree": switch len(splitted) { case 2: diff --git a/network/environment/main.go b/network/environment/main.go index 31ee8d61..7a18d0db 100644 --- a/network/environment/main.go +++ b/network/environment/main.go @@ -15,12 +15,14 @@ var ( module *modules.Module ) +// InitSubModule initializes module specific things with the given module. Intended to be used as part of the "network" module. func InitSubModule(m *modules.Module) { module = m module.RegisterEvent(networkChangedEvent) module.RegisterEvent(onlineStatusChangedEvent) } +// StartSubModule starts module specific things with the given module. Intended to be used as part of the "network" module. func StartSubModule() error { if module == nil { return errors.New("not initialized") diff --git a/network/environment/main_test.go b/network/environment/main_test.go new file mode 100644 index 00000000..010c1e11 --- /dev/null +++ b/network/environment/main_test.go @@ -0,0 +1,35 @@ +package environment + +import ( + "os" + "testing" + + "github.com/safing/portbase/modules" + "github.com/safing/portmaster/core" +) + +func TestMain(m *testing.M) { + // setup + tmpDir, err := core.InitForTesting() + if err != nil { + panic(err) + } + + // setup package + netModule := modules.Register("network", nil, nil, nil, "core") + InitSubModule(netModule) + err = StartSubModule() + if err != nil { + panic(err) + } + + // run tests + rv := m.Run() + + // teardown + core.StopTesting() + _ = os.RemoveAll(tmpDir) + + // exit with test run return value + os.Exit(rv) +} diff --git a/network/geoip/location.go b/network/geoip/location.go index 8c341ed4..92fd5b70 100644 --- a/network/geoip/location.go +++ b/network/geoip/location.go @@ -95,7 +95,7 @@ func (l *Location) EstimateNetworkProximity(to *Location) (proximity int) { } } - return //nolint:nakedreturn + return //nolint:nakedret } // PrimitiveNetworkProximity calculates the numerical distance between two IP addresses. Returns a proximity value between 0 (far away) and 100 (nearby). diff --git a/network/geoip/module.go b/network/geoip/module.go index 05b33a91..4f1efa56 100644 --- a/network/geoip/module.go +++ b/network/geoip/module.go @@ -3,7 +3,6 @@ package geoip import ( "context" "fmt" - "time" "github.com/safing/portbase/modules" ) @@ -22,22 +21,12 @@ func start() error { return fmt.Errorf("goeip: failed to load databases: %s", err) } - module.RegisterEventHook( + return module.RegisterEventHook( "updates", "resource update", "upgrade databases", upgradeDatabases, ) - - // TODO: replace with update subscription - module.NewTask("update databases", func(ctx context.Context, task *modules.Task) { - - dbFileLock.Lock() - defer dbFileLock.Unlock() - - }).Repeat(10 * time.Minute).MaxDelay(1 * time.Hour) - - return nil } func upgradeDatabases(_ context.Context, _ interface{}) error { diff --git a/network/link.go b/network/link.go index dcc52db3..82f02a6f 100644 --- a/network/link.go +++ b/network/link.go @@ -1,6 +1,7 @@ package network import ( + "context" "errors" "fmt" "sync" @@ -14,11 +15,8 @@ import ( // FirewallHandler defines the function signature for a firewall handle function type FirewallHandler func(pkt packet.Packet, link *Link) -var ( - linkTimeout = 10 * time.Minute -) - // Link describes a distinct physical connection (e.g. TCP connection) - like an instance - of a Connection. +//nolint:maligned // TODO: fix alignment type Link struct { record.Base sync.Mutex @@ -75,7 +73,13 @@ func (link *Link) SetFirewallHandler(handler FirewallHandler) { if link.firewallHandler == nil { link.firewallHandler = handler link.pktQueue = make(chan packet.Packet, 1000) - go link.packetHandler() + + // start handling + module.StartWorker("", func(ctx context.Context) error { + link.packetHandler() + return nil + }) + return } link.firewallHandler = handler @@ -98,8 +102,13 @@ func (link *Link) HandlePacket(pkt packet.Packet) { link.pktQueue <- pkt return } + log.Warningf("network: link %s does not have a firewallHandler, dropping packet", link) - pkt.Drop() + + err := pkt.Drop() + if err != nil { + log.Warningf("network: failed to drop packet %s: %s", pkt, err) + } } // Accept accepts the link and adds the given reason. @@ -195,41 +204,48 @@ func (link *Link) ApplyVerdict(pkt packet.Packet) { link.Lock() defer link.Unlock() + var err error + if link.VerdictPermanent { switch link.Verdict { case VerdictAccept: - pkt.PermanentAccept() + err = pkt.PermanentAccept() case VerdictBlock: - pkt.PermanentBlock() + err = pkt.PermanentBlock() case VerdictDrop: - pkt.PermanentDrop() + err = pkt.PermanentDrop() case VerdictRerouteToNameserver: - pkt.RerouteToNameserver() + err = pkt.RerouteToNameserver() case VerdictRerouteToTunnel: - pkt.RerouteToTunnel() + err = pkt.RerouteToTunnel() default: - pkt.Drop() + err = pkt.Drop() } } else { switch link.Verdict { case VerdictAccept: - pkt.Accept() + err = pkt.Accept() case VerdictBlock: - pkt.Block() + err = pkt.Block() case VerdictDrop: - pkt.Drop() + err = pkt.Drop() case VerdictRerouteToNameserver: - pkt.RerouteToNameserver() + err = pkt.RerouteToNameserver() case VerdictRerouteToTunnel: - pkt.RerouteToTunnel() + err = pkt.RerouteToTunnel() default: - pkt.Drop() + err = pkt.Drop() } } + + if err != nil { + log.Warningf("network: failed to apply link verdict to packet %s: %s", pkt, err) + } } // SaveWhenFinished marks the Link for saving after all current actions are finished. func (link *Link) SaveWhenFinished() { + // FIXME: check if we should lock here link.saveWhenFinished = true } @@ -243,11 +259,19 @@ func (link *Link) SaveIfNeeded() { link.Unlock() if save { - link.save() + link.saveAndLog() } } -// Save saves the link object in the storage and propagates the change. +// saveAndLog saves the link object in the storage and propagates the change. It does not return an error, but logs it. +func (link *Link) saveAndLog() { + err := link.save() + if err != nil { + log.Warningf("network: failed to save link %s: %s", link, err) + } +} + +// save saves the link object in the storage and propagates the change. func (link *Link) save() error { // update link link.Lock() diff --git a/pmctl/logs.go b/pmctl/logs.go index e08eda71..7ee1576d 100644 --- a/pmctl/logs.go +++ b/pmctl/logs.go @@ -85,6 +85,7 @@ func initControlLogFile() *os.File { return initializeLogFile(logFilePath, "control/portmaster-control", info.Version()) } +//nolint:deadcode,unused // false positive on linux, currently used by windows only func logControlError(cErr error) { // check if error present if cErr == nil { @@ -110,6 +111,7 @@ func logControlError(cErr error) { errorFile.Close() } +//nolint:deadcode,unused // TODO func logControlStack() { // check logging dir logFileBasePath := filepath.Join(logsRoot.Path, "fstree", "control") @@ -126,10 +128,11 @@ func logControlStack() { } // write error and close - pprof.Lookup("goroutine").WriteTo(errorFile, 1) + _ = pprof.Lookup("goroutine").WriteTo(errorFile, 2) errorFile.Close() } +//nolint:deadcode,unused // false positive on linux, currently used by windows only func runAndLogControlError(wrappedFunc func(cmd *cobra.Command, args []string) error) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { err := wrappedFunc(cmd, args) diff --git a/pmctl/main.go b/pmctl/main.go index 4e095712..2c99fe2f 100644 --- a/pmctl/main.go +++ b/pmctl/main.go @@ -65,8 +65,8 @@ func init() { rootCmd.PersistentFlags().StringVar(&dataDir, "data", "", "set data directory") rootCmd.PersistentFlags().StringVar(&databaseDir, "db", "", "alias to --data (deprecated)") - rootCmd.MarkPersistentFlagDirname("data") - rootCmd.MarkPersistentFlagDirname("db") + _ = rootCmd.MarkPersistentFlagDirname("data") + _ = rootCmd.MarkPersistentFlagDirname("db") rootCmd.Flags().BoolVar(&showFullVersion, "version", false, "print version") rootCmd.Flags().BoolVar(&showShortVersion, "ver", false, "print version number only") } @@ -85,11 +85,10 @@ func main() { // }() // catch interrupt for clean shutdown - signalCh := make(chan os.Signal) + signalCh := make(chan os.Signal, 2) signal.Notify( signalCh, os.Interrupt, - os.Kill, syscall.SIGHUP, syscall.SIGINT, syscall.SIGTERM, diff --git a/pmctl/run.go b/pmctl/run.go index eab7893c..1f9ffcfc 100644 --- a/pmctl/run.go +++ b/pmctl/run.go @@ -124,7 +124,13 @@ func run(cmd *cobra.Command, opts *Options) (err error) { if pid != 0 { return fmt.Errorf("another instance of Portmaster Core is already running: PID %d", pid) } - defer deleteInstanceLock(opts.ShortIdentifier) + defer func() { + err := deleteInstanceLock(opts.ShortIdentifier) + if err != nil { + log.Printf("failed to delete instance lock: %s\n", err) + } + }() + } // notify service after some time @@ -192,6 +198,7 @@ func run(cmd *cobra.Command, opts *Options) (err error) { } } +// nolint:gocyclo,gocognit // TODO: simplify func execute(opts *Options, args []string) (cont bool, err error) { file, err := registry.GetFile(platform(opts.Identifier)) if err != nil { @@ -236,7 +243,7 @@ func execute(opts *Options, args []string) (cont bool, err error) { } // create command - exc := exec.Command(file.Path(), args...) + exc := exec.Command(file.Path(), args...) //nolint:gosec // everything is okay if !runningInConsole && opts.AllowHidingWindow { // Windows only: diff --git a/pmctl/service.go b/pmctl/service.go index 4a9130ca..cec7fa34 100644 --- a/pmctl/service.go +++ b/pmctl/service.go @@ -8,8 +8,9 @@ var ( startupComplete = make(chan struct{}) // signal that the start procedure completed (is never closed, just signaled once) shuttingDown = make(chan struct{}) // signal that we are shutting down (will be closed, may not be closed directly, use initiateShutdown) shutdownInitiated = false // not to be used directly - shutdownError error // may not be read or written to directly - shutdownLock sync.Mutex + //nolint:deadcode,unused // false positive on linux, currently used by windows only + shutdownError error // may not be read or written to directly + shutdownLock sync.Mutex ) func initiateShutdown(err error) { @@ -23,6 +24,7 @@ func initiateShutdown(err error) { } } +//nolint:deadcode,unused // false positive on linux, currently used by windows only func getShutdownError() error { shutdownLock.Lock() defer shutdownLock.Unlock() diff --git a/process/database.go b/process/database.go index 72b70967..d86f79b2 100644 --- a/process/database.go +++ b/process/database.go @@ -24,8 +24,7 @@ var ( dbController *database.Controller dbControllerFlag = abool.NewBool(false) - deleteProcessesThreshold = 15 * time.Minute - lastEstablishedUpdateThreshold = 30 * time.Second + deleteProcessesThreshold = 15 * time.Minute ) // GetProcessFromStorage returns a process from the internal storage. diff --git a/process/matching.go b/process/matching.go index bf5db2ad..88c020c6 100644 --- a/process/matching.go +++ b/process/matching.go @@ -33,16 +33,14 @@ func (p *Process) FindProfiles(ctx context.Context) error { } var userProfile *profile.Profile - for r := range it.Next { - it.Cancel() - userProfile, err = profile.EnsureProfile(r) - if err != nil { - return err - } - break - } - if it.Err() != nil { - return it.Err() + // get first result + r := <-it.Next + // cancel immediately + it.Cancel() + // ensure its a profile + userProfile, err = profile.EnsureProfile(r) + if err != nil { + return err } // create new profile if it does not exist. @@ -54,7 +52,7 @@ func (p *Process) FindProfiles(ctx context.Context) error { } if userProfile.MarkUsed() { - userProfile.Save(profile.UserNamespace) + _ = userProfile.Save(profile.UserNamespace) } // Stamp @@ -74,17 +72,7 @@ func (p *Process) FindProfiles(ctx context.Context) error { return nil } -func selectProfile(p *Process, profs []*profile.Profile) (selectedProfile *profile.Profile) { - var highestScore int - for _, prof := range profs { - score := matchProfile(p, prof) - if score > highestScore { - selectedProfile = prof - } - } - return -} - +//nolint:deadcode,unused // FIXME func matchProfile(p *Process, prof *profile.Profile) (score int) { for _, fp := range prof.Fingerprints { score += matchFingerprint(p, fp) @@ -92,6 +80,7 @@ func matchProfile(p *Process, prof *profile.Profile) (score int) { return } +//nolint:deadcode,unused // FIXME func matchFingerprint(p *Process, fp *profile.Fingerprint) (score int) { if !fp.MatchesOS() { return 0 @@ -100,8 +89,8 @@ func matchFingerprint(p *Process, fp *profile.Fingerprint) (score int) { switch fp.Type { case "full_path": if p.Path == fp.Value { + return profile.GetFingerprintWeight(fp.Type) } - return profile.GetFingerprintWeight(fp.Type) case "partial_path": // FIXME: if full_path matches, do not match partial paths return profile.GetFingerprintWeight(fp.Type) diff --git a/process/proc/processfinder.go b/process/proc/processfinder.go index 2daa324e..32f25136 100644 --- a/process/proc/processfinder.go +++ b/process/proc/processfinder.go @@ -155,7 +155,7 @@ func readDirNames(dir string) (names []string) { defer file.Close() names, err = file.Readdirnames(0) if err != nil { - log.Warningf("process: could not get entries from direcotry %s: %s", dir, err) + log.Warningf("process: could not get entries from directory %s: %s", dir, err) return []string{} } return diff --git a/process/proc/sockets.go b/process/proc/sockets.go index 38700a40..49027119 100644 --- a/process/proc/sockets.go +++ b/process/proc/sockets.go @@ -47,19 +47,6 @@ const ( UDP6Data = "/proc/net/udp6" ICMP4Data = "/proc/net/icmp" ICMP6Data = "/proc/net/icmp6" - - TCP_ESTABLISHED = iota + 1 - TCP_SYN_SENT - TCP_SYN_RECV - TCP_FIN_WAIT1 - TCP_FIN_WAIT2 - TCP_TIME_WAIT - TCP_CLOSE - TCP_CLOSE_WAIT - TCP_LAST_ACK - TCP_LISTEN - TCP_CLOSING - TCP_NEW_SYN_RECV ) var ( diff --git a/process/process.go b/process/process.go index 6d18474f..fa145586 100644 --- a/process/process.go +++ b/process/process.go @@ -16,7 +16,7 @@ import ( ) var ( - dupReqMap = make(map[int]*sync.Mutex) + dupReqMap = make(map[int]*sync.WaitGroup) dupReqLock sync.Mutex ) @@ -61,7 +61,7 @@ func (p *Process) ProfileSet() *profile.Set { return p.profileSet } -// Strings returns a string represenation of process. +// Strings returns a string representation of process. func (p *Process) String() string { p.Lock() defer p.Unlock() @@ -79,7 +79,7 @@ func (p *Process) AddCommunication() { // check if we should save save := false - if p.LastCommEstablished < time.Now().Add(-3*time.Second).Unix() { + if p.LastCommEstablished == 0 || p.LastCommEstablished < time.Now().Add(-3*time.Second).Unix() { save = true } @@ -206,6 +206,43 @@ func GetOrFindProcess(ctx context.Context, pid int) (*Process, error) { return p, nil } +func deduplicateRequest(ctx context.Context, pid int) (finishRequest func()) { + dupReqLock.Lock() + defer dupReqLock.Unlock() + + // get duplicate request waitgroup + wg, requestActive := dupReqMap[pid] + + // someone else is already on it! + if requestActive { + // log that we are waiting + log.Tracer(ctx).Tracef("intel: waiting for duplicate request for PID %d to complete", pid) + // wait + wg.Wait() + // done! + return nil + } + + // we are currently the only one doing a request for this + + // create new waitgroup + wg = new(sync.WaitGroup) + // add worker (us!) + wg.Add(1) + // add to registry + dupReqMap[pid] = wg + + // return function to mark request as finished + return func() { + dupReqLock.Lock() + defer dupReqLock.Unlock() + // mark request as done + wg.Done() + // delete from registry + delete(dupReqMap, pid) + } +} + func loadProcess(ctx context.Context, pid int) (*Process, error) { if pid == -1 { return UnknownProcess, nil @@ -219,35 +256,20 @@ func loadProcess(ctx context.Context, pid int) (*Process, error) { return process, nil } - // dedup requests - dupReqLock.Lock() - mutex, requestActive := dupReqMap[pid] - if !requestActive { - mutex = new(sync.Mutex) - mutex.Lock() - dupReqMap[pid] = mutex - dupReqLock.Unlock() - } else { - dupReqLock.Unlock() - log.Tracer(ctx).Tracef("process: waiting for duplicate request for PID %d to complete", pid) - mutex.Lock() - // wait until duplicate request is finished, then fetch current Process and return - mutex.Unlock() + // dedupe! + markRequestFinished := deduplicateRequest(ctx, pid) + if markRequestFinished == nil { + // we waited for another request, recheck the storage! process, ok = GetProcessFromStorage(pid) if ok { return process, nil } - return nil, fmt.Errorf("previous request for process with PID %d failed", pid) + // if cache is still empty, go ahead + } else { + // we are the first! + defer markRequestFinished() } - // lock request for this pid - defer func() { - dupReqLock.Lock() - delete(dupReqMap, pid) - dupReqLock.Unlock() - mutex.Unlock() - }() - // create new process new := &Process{ Pid: pid, diff --git a/process/process_linux.go b/process/process_linux.go index cc871dad..44a21098 100644 --- a/process/process_linux.go +++ b/process/process_linux.go @@ -1,26 +1,26 @@ package process // IsUser returns whether the process is run by a normal user. -func (m *Process) IsUser() bool { - return m.UserID >= 1000 +func (p *Process) IsUser() bool { + return p.UserID >= 1000 } // IsAdmin returns whether the process is run by an admin user. -func (m *Process) IsAdmin() bool { - return m.UserID >= 0 +func (p *Process) IsAdmin() bool { + return p.UserID >= 0 } // IsSystem returns whether the process is run by the operating system. -func (m *Process) IsSystem() bool { - return m.UserID == 0 +func (p *Process) IsSystem() bool { + return p.UserID == 0 } // IsKernel returns whether the process is the Kernel. -func (m *Process) IsKernel() bool { - return m.Pid == 0 +func (p *Process) IsKernel() bool { + return p.Pid == 0 } // specialOSInit does special OS specific Process initialization. -func (m *Process) specialOSInit() { +func (p *Process) specialOSInit() { } diff --git a/profile/defaults.go b/profile/defaults.go index 272ed7e3..20ad11f8 100644 --- a/profile/defaults.go +++ b/profile/defaults.go @@ -29,7 +29,7 @@ func makeDefaultFallbackProfile() *Profile { Related: status.SecurityLevelDynamic, }, ServiceEndpoints: []*EndpointPermission{ - &EndpointPermission{ + { Type: EptAny, Protocol: 0, StartPort: 0, diff --git a/profile/endpoints.go b/profile/endpoints.go index 8e584c5c..bd8c5d88 100644 --- a/profile/endpoints.go +++ b/profile/endpoints.go @@ -15,8 +15,8 @@ type Endpoints []*EndpointPermission // EndpointPermission holds a decision about an endpoint. type EndpointPermission struct { - Type EPType Value string + Type EPType Protocol uint8 StartPort uint16 @@ -55,10 +55,7 @@ const ( // IsSet returns whether the Endpoints object is "set". func (e Endpoints) IsSet() bool { - if len(e) > 0 { - return true - } - return false + return len(e) > 0 } // CheckDomain checks the if the given endpoint matches a EndpointPermission in the list. @@ -246,7 +243,7 @@ func (ep EndpointPermission) MatchesIP(domain string, ip net.IP, protocol uint8, } func (e Endpoints) String() string { - var s []string + s := make([]string, 0, len(e)) for _, entry := range e { s = append(s, entry.String()) } diff --git a/profile/endpoints_test.go b/profile/endpoints_test.go index 18044dda..b89db27f 100644 --- a/profile/endpoints_test.go +++ b/profile/endpoints_test.go @@ -155,15 +155,14 @@ func TestEndpointMatching(t *testing.T) { } func TestEPString(t *testing.T) { - var endpoints Endpoints - endpoints = []*EndpointPermission{ - &EndpointPermission{ + var endpoints Endpoints = []*EndpointPermission{ + { Type: EptDomain, Value: "example.com", Protocol: 6, Permit: true, }, - &EndpointPermission{ + { Type: EptIPv4, Value: "1.1.1.1", Protocol: 17, // TCP @@ -171,7 +170,7 @@ func TestEPString(t *testing.T) { EndPort: 53, Permit: false, }, - &EndpointPermission{ + { Type: EptDomain, Value: "example.org", Permit: false, @@ -181,8 +180,7 @@ func TestEPString(t *testing.T) { t.Errorf("unexpected result: %s", endpoints.String()) } - var noEndpoints Endpoints - noEndpoints = []*EndpointPermission{} + var noEndpoints Endpoints = []*EndpointPermission{} if noEndpoints.String() != "[]" { t.Errorf("unexpected result: %s", noEndpoints.String()) } diff --git a/profile/fingerprint.go b/profile/fingerprint.go index 3d4b6dc9..9fe38a99 100644 --- a/profile/fingerprint.go +++ b/profile/fingerprint.go @@ -36,7 +36,7 @@ func GetFingerprintWeight(fpType string) (weight int) { } // AddFingerprint adds the given fingerprint to the profile. -func (p *Profile) AddFingerprint(fp *Fingerprint) { +func (profile *Profile) AddFingerprint(fp *Fingerprint) { if fp.OS == "" { fp.OS = osIdentifier } @@ -44,5 +44,5 @@ func (p *Profile) AddFingerprint(fp *Fingerprint) { fp.LastUsed = time.Now().Unix() } - p.Fingerprints = append(p.Fingerprints, fp) + profile.Fingerprints = append(profile.Fingerprints, fp) } diff --git a/profile/profile.go b/profile/profile.go index f1bda3c9..a25ec1cd 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -58,8 +58,8 @@ func New() *Profile { } // MakeProfileKey creates the correct key for a profile with the given namespace and ID. -func MakeProfileKey(namespace, ID string) string { - return fmt.Sprintf("core:profiles/%s/%s", namespace, ID) +func MakeProfileKey(namespace, id string) string { + return fmt.Sprintf("core:profiles/%s/%s", namespace, id) } // Save saves the profile to the database @@ -98,17 +98,17 @@ func (profile *Profile) DetailedString() string { } // GetUserProfile loads a profile from the database. -func GetUserProfile(ID string) (*Profile, error) { - return getProfile(UserNamespace, ID) +func GetUserProfile(id string) (*Profile, error) { + return getProfile(UserNamespace, id) } // GetStampProfile loads a profile from the database. -func GetStampProfile(ID string) (*Profile, error) { - return getProfile(StampNamespace, ID) +func GetStampProfile(id string) (*Profile, error) { + return getProfile(StampNamespace, id) } -func getProfile(namespace, ID string) (*Profile, error) { - r, err := profileDB.Get(MakeProfileKey(namespace, ID)) +func getProfile(namespace, id string) (*Profile, error) { + r, err := profileDB.Get(MakeProfileKey(namespace, id)) if err != nil { return nil, err } diff --git a/profile/set.go b/profile/set.go index 5afa8f53..57c908e3 100644 --- a/profile/set.go +++ b/profile/set.go @@ -8,10 +8,6 @@ import ( "github.com/safing/portmaster/status" ) -var ( - emptyFlags = Flags{} -) - // Set handles Profile chaining. type Set struct { sync.Mutex diff --git a/profile/set_test.go b/profile/set_test.go index 07b5a4ce..8662b4d6 100644 --- a/profile/set_test.go +++ b/profile/set_test.go @@ -1,3 +1,4 @@ +//nolint:unparam package profile import ( @@ -30,25 +31,25 @@ func init() { Independent: status.SecurityLevelFortress, }, Endpoints: []*EndpointPermission{ - &EndpointPermission{ + { Type: EptDomain, Value: "good.bad.example.com.", Permit: true, Created: time.Now().Unix(), }, - &EndpointPermission{ + { Type: EptDomain, Value: "*bad.example.com.", Permit: false, Created: time.Now().Unix(), }, - &EndpointPermission{ + { Type: EptDomain, Value: "example.com.", Permit: true, Created: time.Now().Unix(), }, - &EndpointPermission{ + { Type: EptAny, Permit: true, Protocol: 6, @@ -67,13 +68,13 @@ func init() { // Internet: status.SecurityLevelsAll, // }, Endpoints: []*EndpointPermission{ - &EndpointPermission{ + { Type: EptDomain, Value: "*bad2.example.com.", Permit: false, Created: time.Now().Unix(), }, - &EndpointPermission{ + { Type: EptAny, Permit: true, Protocol: 6, @@ -83,7 +84,7 @@ func init() { }, }, ServiceEndpoints: []*EndpointPermission{ - &EndpointPermission{ + { Type: EptAny, Permit: true, Protocol: 17, @@ -91,7 +92,7 @@ func init() { EndPort: 12347, Created: time.Now().Unix(), }, - &EndpointPermission{ // default deny + { // default deny Type: EptAny, Permit: false, Created: time.Now().Unix(), @@ -173,6 +174,6 @@ func TestProfileSet(t *testing.T) { } func getLineNumberOfCaller(levels int) int { - _, _, line, _ := runtime.Caller(levels + 1) + _, _, line, _ := runtime.Caller(levels + 1) //nolint:dogsled return line } diff --git a/profile/specialprofiles.go b/profile/specialprofiles.go index f5c8c346..4903f74c 100644 --- a/profile/specialprofiles.go +++ b/profile/specialprofiles.go @@ -24,7 +24,7 @@ func initSpecialProfiles() (err error) { return err } globalProfile = makeDefaultGlobalProfile() - globalProfile.Save(SpecialNamespace) + _ = globalProfile.Save(SpecialNamespace) } fallbackProfile, err = getSpecialProfile("fallback") @@ -34,15 +34,15 @@ func initSpecialProfiles() (err error) { } fallbackProfile = makeDefaultFallbackProfile() ensureServiceEndpointsDenyAll(fallbackProfile) - fallbackProfile.Save(SpecialNamespace) + _ = fallbackProfile.Save(SpecialNamespace) } ensureServiceEndpointsDenyAll(fallbackProfile) return nil } -func getSpecialProfile(ID string) (*Profile, error) { - return getProfile(SpecialNamespace, ID) +func getSpecialProfile(id string) (*Profile, error) { + return getProfile(SpecialNamespace, id) } func ensureServiceEndpointsDenyAll(p *Profile) (changed bool) { @@ -52,7 +52,7 @@ func ensureServiceEndpointsDenyAll(p *Profile) (changed bool) { ep.Protocol == 0 && ep.StartPort == 0 && ep.EndPort == 0 && - ep.Permit == false { + !ep.Permit { return false } } diff --git a/profile/updates.go b/profile/updates.go index 36d57f9b..3f350db0 100644 --- a/profile/updates.go +++ b/profile/updates.go @@ -52,7 +52,7 @@ func updateListener(sub *database.Subscription) { profile.Unlock() if profileChanged { - profile.Save(SpecialNamespace) + _ = profile.Save(SpecialNamespace) continue } diff --git a/status/database.go b/status/database.go index b0d7b291..94240729 100644 --- a/status/database.go +++ b/status/database.go @@ -48,3 +48,7 @@ func initStatusHook() (err error) { hook, err = database.RegisterHook(query.New(statusDBKey), &statusHook{}) return err } + +func stopStatusHook() error { + return hook.Cancel() +} diff --git a/status/module.go b/status/module.go index a41147fc..ca788701 100644 --- a/status/module.go +++ b/status/module.go @@ -9,10 +9,6 @@ import ( _ "github.com/safing/portmaster/core" ) -var ( - shutdownSignal = make(chan struct{}) -) - func init() { modules.Register("status", nil, start, stop, "core") } @@ -57,11 +53,5 @@ func start() error { } func stop() error { - select { - case <-shutdownSignal: - // already closed - default: - close(shutdownSignal) - } - return nil + return stopStatusHook() } diff --git a/ui/serve.go b/ui/serve.go index ee92edbf..96bb34b9 100644 --- a/ui/serve.go +++ b/ui/serve.go @@ -129,7 +129,6 @@ func ServeFileFromBundle(w http.ResponseWriter, r *http.Request, bundleName stri } readCloser.Close() - return } // RedirectToBase redirects the requests to the control app diff --git a/updates/export.go b/updates/export.go index ea331147..8d027ff5 100644 --- a/updates/export.go +++ b/updates/export.go @@ -49,21 +49,19 @@ func initVersionExport() (err error) { return err } - module.RegisterEventHook( + return module.RegisterEventHook( "updates", eventVersionUpdate, "export version status", export, ) - return nil } func stopVersionExport() error { return versionExportHook.Cancel() } -var exportMicroTaskName = "update version status" - +// export is an event hook func export(_ context.Context, _ interface{}) error { // populate versionExport.lock.Lock() diff --git a/updates/main.go b/updates/main.go index 85f8bde5..a4092277 100644 --- a/updates/main.go +++ b/updates/main.go @@ -100,9 +100,7 @@ func start() error { }).Repeat(24 * time.Hour).MaxDelay(1 * time.Hour).Schedule(time.Now().Add(10 * time.Second)) // react to upgrades - initUpgrader() - - return nil + return initUpgrader() } func stop() error { diff --git a/updates/upgrader.go b/updates/upgrader.go index 06694a3d..fff7855c 100644 --- a/updates/upgrader.go +++ b/updates/upgrader.go @@ -28,16 +28,15 @@ const ( ) var ( - upgraderActive = abool.NewBool(false) - dontUpgradeBefore = time.Now().Add(5 * time.Minute) - pmCtrlUpdate *updater.File - pmCoreUpdate *updater.File + upgraderActive = abool.NewBool(false) + pmCtrlUpdate *updater.File + pmCoreUpdate *updater.File rawVersionRegex = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+b?\*?$`) ) -func initUpgrader() { - module.RegisterEventHook( +func initUpgrader() error { + return module.RegisterEventHook( "updates", eventResourceUpdate, "run upgrades",