Merge pull request #1290 from safing/fix/connection-state-handling

Fix connection state handling and improve DNS request attribution
This commit is contained in:
Daniel Hovie
2023-08-18 17:00:07 +02:00
committed by GitHub
16 changed files with 187 additions and 89 deletions

View File

@@ -116,18 +116,24 @@ func BandwidthStatsWorker(ctx context.Context, collectInterval time.Duration, ba
// reportBandwidth reports the bandwidth to the given updates channel.
func reportBandwidth(ctx context.Context, objs bpfObjects, bandwidthUpdates chan *packet.BandwidthUpdate) {
var (
skKey bpfSkKey
skInfo bpfSkInfo
updated int
skipped int
)
iter := objs.bpfMaps.PmBandwidthMap.Iterate()
var skKey bpfSkKey
var skInfo bpfSkInfo
for iter.Next(&skKey, &skInfo) {
// Check if already reported.
if skInfo.Reported >= 1 {
skipped++
continue
}
// Mark as reported and update the map.
skInfo.Reported = 1
if err := objs.bpfMaps.PmBandwidthMap.Put(&skKey, &skInfo); err != nil {
log.Debugf("ebpf: failed to update map: %s", err)
if err := objs.bpfMaps.PmBandwidthMap.Update(&skKey, &skInfo, ebpf.UpdateExist); err != nil {
log.Debugf("ebpf: failed to mark bandwidth map entry as reported: %s", err)
}
connID := packet.CreateConnectionID(
@@ -144,10 +150,11 @@ func reportBandwidth(ctx context.Context, objs bpfObjects, bandwidthUpdates chan
}
select {
case bandwidthUpdates <- update:
updated++
case <-ctx.Done():
return
default:
log.Warning("ebpf: bandwidth update queue is full, skipping rest of batch")
log.Warningf("ebpf: bandwidth update queue is full (updated=%d, skipped=%d), skipping rest of batch", updated, skipped)
return
}
}

View File

@@ -19,8 +19,6 @@ import (
"github.com/safing/portmaster/network"
"github.com/safing/portmaster/network/netutils"
"github.com/safing/portmaster/network/packet"
"github.com/safing/portmaster/network/state"
"github.com/safing/portmaster/process"
"github.com/safing/portmaster/profile"
"github.com/safing/portmaster/profile/endpoints"
)
@@ -31,9 +29,6 @@ type deciderFn func(context.Context, *network.Connection, *profile.LayeredProfil
var defaultDeciders = []deciderFn{
checkPortmasterConnection,
// TODO: This is currently very slow.
// Find a way to improve performance using the eBPF data.
// checkSelfCommunication,
checkIfBroadcastReply,
checkConnectionType,
checkConnectionScope,
@@ -164,42 +159,6 @@ func checkPortmasterConnection(ctx context.Context, conn *network.Connection, _
return true
}
// checkSelfCommunication checks if the process is communicating with itself.
func checkSelfCommunication(ctx context.Context, conn *network.Connection, _ *profile.LayeredProfile, pkt packet.Packet) bool {
// check if process is communicating with itself
if pkt != nil {
// TODO: evaluate the case where different IPs in the 127/8 net are used.
pktInfo := pkt.Info()
if conn.Process().Pid >= 0 && pktInfo.Src.Equal(pktInfo.Dst) {
// get PID
otherPid, _, err := state.Lookup(&packet.Info{
Inbound: !pktInfo.Inbound, // we want to know the process on the other end
Version: pktInfo.Version,
Protocol: pktInfo.Protocol,
Src: pktInfo.Src,
SrcPort: pktInfo.SrcPort,
Dst: pktInfo.Dst,
DstPort: pktInfo.DstPort,
}, true)
if err != nil {
log.Tracer(ctx).Debugf("filter: failed to find local peer process PID: %s", err)
} else {
// get primary process
otherProcess, err := process.GetOrFindProcess(ctx, otherPid)
if err != nil {
log.Tracer(ctx).Debugf("filter: failed to find load local peer process with PID %d: %s", otherPid, err)
} else if otherProcess.Path == conn.Process().Path {
conn.Accept("process internal connection", noReasonOptionKey)
conn.Internal = true
return true
}
}
}
}
return false
}
func checkIfBroadcastReply(ctx context.Context, conn *network.Connection, _ *profile.LayeredProfile, _ packet.Packet) bool {
// Only check inbound connections.
if !conn.Inbound {

View File

@@ -23,6 +23,7 @@ import (
"github.com/safing/portmaster/network/netutils"
"github.com/safing/portmaster/network/packet"
"github.com/safing/portmaster/network/reference"
"github.com/safing/portmaster/process"
"github.com/safing/spn/access"
)
@@ -140,12 +141,12 @@ func handlePacket(pkt packet.Packet) {
}
// fastTrackedPermit quickly permits certain network critical or internal connections.
func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bool) {
func fastTrackedPermit(conn *network.Connection, pkt packet.Packet) (verdict network.Verdict, permanent bool) {
meta := pkt.Info()
// Check if packed was already fast-tracked by the OS integration.
if pkt.FastTrackedByIntegration() {
log.Debugf("filter: fast-tracked by OS integration: %s", pkt)
log.Tracer(pkt.Ctx()).Debugf("filter: fast-tracked by OS integration: %s", pkt)
return network.VerdictAccept, true
}
@@ -159,7 +160,7 @@ func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bo
// Eg. dig: https://gitlab.isc.org/isc-projects/bind9/-/issues/1140
if meta.SrcPort == meta.DstPort &&
meta.Src.Equal(meta.Dst) {
log.Debugf("filter: fast-track network self-check: %s", pkt)
log.Tracer(pkt.Ctx()).Debugf("filter: fast-track network self-check: %s", pkt)
return network.VerdictAccept, true
}
@@ -169,7 +170,7 @@ func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bo
// Load packet data.
err := pkt.LoadPacketData()
if err != nil {
log.Debugf("filter: failed to load ICMP packet data: %s", err)
log.Tracer(pkt.Ctx()).Debugf("filter: failed to load ICMP packet data: %s", err)
return network.VerdictAccept, true
}
@@ -179,7 +180,7 @@ func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bo
// 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.
log.Debugf("filter: fast-track tracing ICMP/v6: %s", pkt)
log.Tracer(pkt.Ctx()).Debugf("filter: fast-track tracing ICMP/v6: %s", pkt)
return network.VerdictAccept, false
}
@@ -202,7 +203,7 @@ func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bo
}
// Permit all ICMP/v6 packets that are not echo requests or replies.
log.Debugf("filter: fast-track accepting ICMP/v6: %s", pkt)
log.Tracer(pkt.Ctx()).Debugf("filter: fast-track accepting ICMP/v6: %s", pkt)
return network.VerdictAccept, true
case packet.UDP, packet.TCP:
@@ -224,7 +225,7 @@ func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bo
}
// Log and permit.
log.Debugf("filter: fast-track accepting DHCP: %s", pkt)
log.Tracer(pkt.Ctx()).Debugf("filter: fast-track accepting DHCP: %s", pkt)
return network.VerdictAccept, true
case apiPort:
@@ -249,14 +250,14 @@ func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bo
isMe, err := netenv.IsMyIP(meta.Src)
switch {
case err != nil:
log.Debugf("filter: failed to check if %s is own IP for fast-track: %s", meta.Src, err)
log.Tracer(pkt.Ctx()).Debugf("filter: failed to check if %s is own IP for fast-track: %s", meta.Src, err)
return network.VerdictUndecided, false
case !isMe:
return network.VerdictUndecided, false
}
// Log and permit.
log.Debugf("filter: fast-track accepting api connection: %s", pkt)
log.Tracer(pkt.Ctx()).Debugf("filter: fast-track accepting api connection: %s", pkt)
return network.VerdictAccept, true
case 53:
@@ -277,15 +278,24 @@ func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bo
isMe, err := netenv.IsMyIP(meta.Src)
switch {
case err != nil:
log.Debugf("filter: failed to check if %s is own IP for fast-track: %s", meta.Src, err)
log.Tracer(pkt.Ctx()).Debugf("filter: failed to check if %s is own IP for fast-track: %s", meta.Src, err)
return network.VerdictUndecided, false
case !isMe:
return network.VerdictUndecided, false
}
// Log and permit.
log.Debugf("filter: fast-track accepting local dns: %s", pkt)
return network.VerdictAccept, true
log.Tracer(pkt.Ctx()).Debugf("filter: fast-track accepting local dns: %s", pkt)
// Add to DNS request connections to attribute DNS request if outgoing.
if pkt.IsOutbound() {
// Assign PID from packet directly, as processing stops after fast-track.
conn.PID = pkt.Info().PID
network.SaveDNSRequestConnection(conn, pkt)
}
// Accept local DNS, but only make permanent if we have the PID too.
return network.VerdictAccept, conn.PID != process.UndefinedProcessID
}
case compat.SystemIntegrationCheckProtocol:
@@ -299,7 +309,7 @@ func fastTrackedPermit(pkt packet.Packet) (verdict network.Verdict, permanent bo
}
func fastTrackHandler(conn *network.Connection, pkt packet.Packet) {
fastTrackedVerdict, permanent := fastTrackedPermit(pkt)
fastTrackedVerdict, permanent := fastTrackedPermit(conn, pkt)
if fastTrackedVerdict != network.VerdictUndecided {
// Set verdict on connection.
conn.Verdict.Active = fastTrackedVerdict
@@ -375,6 +385,10 @@ func filterHandler(conn *network.Connection, pkt packet.Packet) {
conn.SetVerdict(network.VerdictRerouteToNameserver, "redirecting rogue dns query", "", nil)
conn.Internal = true
log.Tracer(pkt.Ctx()).Infof("filter: redirecting dns query %s to Portmaster", conn)
// Add to DNS request connections to attribute DNS request.
network.SaveDNSRequestConnection(conn, pkt)
// End directly, as no other processing is necessary.
conn.StopFirewallHandler()
finalizeVerdict(conn)

View File

@@ -122,17 +122,13 @@ func (m *module) prepare() error {
var body struct {
ProfileIDs []string `json:"profileIDs"`
}
dec := json.NewDecoder(ar.Body)
dec.DisallowUnknownFields()
if err := dec.Decode(&body); err != nil {
return "", err
if err := json.Unmarshal(ar.InputData, &body); err != nil {
return "", fmt.Errorf("failed to decode parameters in body: %w", err)
}
if len(body.ProfileIDs) == 0 {
if err := m.mng.store.RemoveAllHistoryData(ar.Context()); err != nil {
return "", err
return "", fmt.Errorf("failed to remove all history: %w", err)
}
} else {
merr := new(multierror.Error)

View File

@@ -48,6 +48,7 @@ func cleanConnections() (activePIDs map[int]struct{}) {
_ = module.RunMicroTask("clean connections", 0, func(ctx context.Context) error {
now := time.Now().UTC()
nowUnix := now.Unix()
ignoreNewer := nowUnix - 1
deleteOlderThan := now.Add(-DeleteConnsAfterEndedThreshold).Unix()
deleteIncompleteOlderThan := now.Add(-DeleteIncompleteConnsAfterStartedThreshold).Unix()
@@ -57,6 +58,8 @@ func cleanConnections() (activePIDs map[int]struct{}) {
// delete inactive connections
switch {
case conn.Started >= ignoreNewer:
// Skip very fresh connections to evade edge cases.
case !conn.DataIsComplete():
// Step 0: delete old incomplete connections
if conn.Started < deleteIncompleteOlderThan {
@@ -76,6 +79,7 @@ func cleanConnections() (activePIDs map[int]struct{}) {
Dst: conn.Entity.IP,
DstPort: conn.Entity.Port,
PID: process.UndefinedProcessID,
SeenAt: time.Unix(conn.Started, 0), // State tables will be updated if older than this.
}, now)
// Step 2: mark as ended
@@ -118,6 +122,9 @@ func cleanConnections() (activePIDs map[int]struct{}) {
conn.Unlock()
}
// rerouted dns requests
cleanDNSRequestConnections()
return nil
})

View File

@@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"net"
"strings"
"sync"
"time"
@@ -293,18 +292,22 @@ func NewConnectionFromDNSRequest(ctx context.Context, fqdn string, cnames []stri
}
// Check if the dns request connection was reported with process info.
dnsRequestConnID := pi.CreateConnectionID()
// Cut the destination, as the dns request may have been redirected and we
// don't know the original destination.
dnsRequestConnIDPrefix, ok := strings.CutSuffix(dnsRequestConnID, "<nil>-0")
if !ok {
log.Tracer(ctx).Warningf("network: unexpected connection ID for finding dns requests connection: %s", dnsRequestConnID)
}
// Find matching dns request connection.
dnsRequestConn, ok := conns.findByPrefix(dnsRequestConnIDPrefix)
if ok && dnsRequestConn.PID != process.UndefinedProcessID {
log.Tracer(ctx).Debugf("network: found matching dns request connection %s", dnsRequestConn)
var proc *process.Process
dnsRequestConn, ok := GetDNSRequestConnection(pi)
switch {
case !ok:
// No dns request connection found.
case dnsRequestConn.PID < 0:
// Process is not identified or is special.
case dnsRequestConn.Ended > 0 && dnsRequestConn.Ended < time.Now().Unix()-3:
// Connection has already ended (too long ago).
log.Tracer(ctx).Debugf("network: found ended dns request connection %s for dns request for %s", dnsRequestConn, fqdn)
default:
log.Tracer(ctx).Debugf("network: found matching dns request connection %s", dnsRequestConn.String())
// Inherit PID.
pi.PID = dnsRequestConn.PID
// Inherit process struct itself, as the PID may already be re-used.
proc = dnsRequestConn.process
}
// Find process by remote IP/Port.
@@ -316,7 +319,9 @@ func NewConnectionFromDNSRequest(ctx context.Context, fqdn string, cnames []stri
}
// Get process and profile with PID.
proc, _ := process.GetProcessWithProfile(ctx, pi.PID)
if proc == nil {
proc, _ = process.GetProcessWithProfile(ctx, pi.PID)
}
timestamp := time.Now().Unix()
dnsConn := &Connection{
@@ -1017,6 +1022,8 @@ func (conn *Connection) SetInspectorData(newInspectorData map[uint8]interface{})
// String returns a string representation of conn.
func (conn *Connection) String() string {
switch {
case conn.process == nil || conn.Entity == nil:
return conn.ID
case conn.Inbound:
return fmt.Sprintf("%s <- %s", conn.process, conn.Entity.IP)
case conn.Entity.Domain != "":

View File

@@ -40,7 +40,7 @@ func (cs *connectionStore) get(id string) (*Connection, bool) {
// findByPrefix returns the first connection where the key matches the given prefix.
// If the prefix matches multiple entries, the result is not deterministic.
func (cs *connectionStore) findByPrefix(prefix string) (*Connection, bool) {
func (cs *connectionStore) findByPrefix(prefix string) (*Connection, bool) { //nolint:unused
cs.rw.RLock()
defer cs.rw.RUnlock()

View File

@@ -12,12 +12,16 @@ import (
"github.com/safing/portbase/log"
"github.com/safing/portmaster/nameserver/nsutil"
"github.com/safing/portmaster/network/packet"
"github.com/safing/portmaster/process"
"github.com/safing/portmaster/resolver"
)
var (
openDNSRequests = make(map[string]*Connection) // key: <pid>/fqdn
dnsRequestConnections = make(map[string]*Connection) // key: <protocol>-<local ip>-<local port>
dnsRequestConnectionsLock sync.RWMutex
openDNSRequests = make(map[string]*Connection) // key: <pid>/<fqdn>
openDNSRequestsLock sync.Mutex
supportedDomainToIPRecordTypes = []uint16{
@@ -38,6 +42,82 @@ const (
openDNSRequestLimit = 3 * time.Second
)
func getDNSRequestConnectionKey(packetInfo *packet.Info) (id string, ok bool) {
// We only support protocols with ports.
if packetInfo.SrcPort == 0 {
return "", false
}
return fmt.Sprintf("%d-%s-%d", packetInfo.Protocol, packetInfo.Src, packetInfo.SrcPort), true
}
// SaveDNSRequestConnection saves a dns request connection for later retrieval.
func SaveDNSRequestConnection(conn *Connection, pkt packet.Packet) {
// Check connection.
if conn.PID == process.UndefinedProcessID {
log.Tracer(pkt.Ctx()).Tracef("network: not saving dns request connection because the PID is undefined")
return
}
// Create key.
key, ok := getDNSRequestConnectionKey(pkt.Info())
if !ok {
log.Tracer(pkt.Ctx()).Debugf("network: not saving dns request connection %s because the protocol is not supported", pkt)
return
}
// Add or update DNS request connection.
log.Tracer(pkt.Ctx()).Tracef("network: saving %s with PID %d as dns request connection for fast DNS request attribution", pkt, conn.PID)
dnsRequestConnectionsLock.Lock()
defer dnsRequestConnectionsLock.Unlock()
dnsRequestConnections[key] = conn
}
// GetDNSRequestConnection returns a saved dns request connection.
func GetDNSRequestConnection(packetInfo *packet.Info) (conn *Connection, ok bool) {
// Make key.
key, ok := getDNSRequestConnectionKey(packetInfo)
if !ok {
return nil, false
}
// Get and return
dnsRequestConnectionsLock.RLock()
defer dnsRequestConnectionsLock.RUnlock()
conn, ok = dnsRequestConnections[key]
return
}
// deleteDNSRequestConnection removes a connection from the dns request connections.
func deleteDNSRequestConnection(packetInfo *packet.Info) { //nolint:unused,deadcode
dnsRequestConnectionsLock.Lock()
defer dnsRequestConnectionsLock.Unlock()
key, ok := getDNSRequestConnectionKey(packetInfo)
if ok {
delete(dnsRequestConnections, key)
}
}
// cleanDNSRequestConnections deletes old DNS request connections.
func cleanDNSRequestConnections() {
deleteOlderThan := time.Now().Unix() - 3
dnsRequestConnectionsLock.Lock()
defer dnsRequestConnectionsLock.Unlock()
for key, conn := range dnsRequestConnections {
conn.Lock()
if conn.Ended > 0 && conn.Ended < deleteOlderThan {
delete(dnsRequestConnections, key)
}
conn.Unlock()
}
}
// IsSupportDNSRecordType returns whether the given DSN RR type is supported
// by the network package, as in the requests are specially handled and can be
// "merged" into the resulting connection.

View File

@@ -35,6 +35,11 @@ func Exists(pktInfo *packet.Info, now time.Time) (exists bool) {
}
func (table *tcpTable) exists(pktInfo *packet.Info) (exists bool) {
// Update tables if older than the connection that is checked.
if table.lastUpdateAt.Load() < pktInfo.SeenAt.UnixNano() {
table.updateTables(table.updateIter.Load())
}
table.lock.RLock()
defer table.lock.RUnlock()
@@ -57,6 +62,11 @@ func (table *tcpTable) exists(pktInfo *packet.Info) (exists bool) {
}
func (table *udpTable) exists(pktInfo *packet.Info, now time.Time) (exists bool) {
// Update tables if older than the connection that is checked.
if table.lastUpdateAt.Load() < pktInfo.SeenAt.UnixNano() {
table.updateTables(table.updateIter.Load())
}
table.lock.RLock()
defer table.lock.RUnlock()

View File

@@ -76,7 +76,7 @@ func (table *tcpTable) lookup(pktInfo *packet.Info, fast bool) (
// Search for the socket until found.
for i := 1; i <= lookupTries; i++ {
// Get or update tables.
if i == 1 {
if i == 1 && pktInfo.SeenAt.UnixNano() >= table.lastUpdateAt.Load() {
connections, listeners, updateIter = table.getCurrentTables()
} else {
connections, listeners, updateIter = table.updateTables(updateIter)
@@ -179,7 +179,7 @@ func (table *udpTable) lookup(pktInfo *packet.Info, fast bool) (
// Search for the socket until found.
for i := 1; i <= lookupTries; i++ {
// Get or update tables.
if i == 1 {
if i == 1 && pktInfo.SeenAt.UnixNano() >= table.lastUpdateAt.Load() {
binds, updateIter = table.getCurrentTables()
} else {
binds, updateIter = table.updateTables(updateIter)

View File

@@ -17,9 +17,12 @@ type tcpTable struct {
connections []*socket.ConnectionInfo
listeners []*socket.BindInfo
updateIter atomic.Uint64
lock sync.RWMutex
updateIter atomic.Uint64
// lastUpdateAt stores the time when the tables where last updated as unix nanoseconds.
lastUpdateAt atomic.Int64
fetchingLock sync.Mutex
fetchingInProgress bool
fetchingDoneSignal chan struct{}
@@ -133,6 +136,7 @@ func (table *tcpTable) updateTables(previousUpdateIter uint64) (
table.connections = connections
table.listeners = listeners
table.updateIter.Add(1)
table.lastUpdateAt.Store(time.Now().UnixNano())
// Return new tables immediately.
return table.connections, table.listeners, table.updateIter.Load()

View File

@@ -17,9 +17,12 @@ import (
type udpTable struct {
version int
binds []*socket.BindInfo
binds []*socket.BindInfo
lock sync.RWMutex
updateIter atomic.Uint64
lock sync.RWMutex
// lastUpdateAt stores the time when the tables where last updated as unix nanoseconds.
lastUpdateAt atomic.Int64
fetchingLock sync.Mutex
fetchingInProgress bool
@@ -152,6 +155,7 @@ func (table *udpTable) updateTables(previousUpdateIter uint64) (
defer table.lock.Unlock()
table.binds = binds
table.updateIter.Add(1)
table.lastUpdateAt.Store(time.Now().UnixNano())
// Return new tables immediately.
return table.binds, table.updateIter.Load()

View File

@@ -112,6 +112,16 @@ func (p *Process) IsIdentified() bool {
}
}
// IsLocal returns whether the process has been identified as a local process.
func (p *Process) IsLocal() bool {
// Check if process exists.
if p == nil {
return false
}
return p.Pid >= 0
}
// Equal returns if the two processes are both identified and have the same PID.
func (p *Process) Equal(other *Process) bool {
return p.IsIdentified() && other.IsIdentified() && p.Pid == other.Pid

View File

@@ -295,11 +295,11 @@ func specialProfileNeedsReset(profile *Profile) bool {
switch profile.ID {
case SystemResolverProfileID:
return canBeUpgraded(profile, "12.8.2023") // FIXME: set one day after stable release date.
return canBeUpgraded(profile, "22.8.2023")
case PortmasterProfileID:
return canBeUpgraded(profile, "12.8.2023") // FIXME: set one day after stable release date.
return canBeUpgraded(profile, "22.8.2023")
case PortmasterAppProfileID:
return canBeUpgraded(profile, "12.8.2023") // FIXME: set one day after stable release date.
return canBeUpgraded(profile, "22.8.2023")
default:
// Not a special profile or no upgrade available yet.
return false