Question about cache

[themighty1]

@gustavo-iniguez-goya ,

https://github.com/evilsocket/opensnitch/blob/75a08245a966768b602ddf678a94afcd09cc5ad3/daemon/procmon/cache.go#L92

if len(pidsCache) > maxCachedPids {
  	  pidsCache = nil
}

Is there a reason why we zero out the cache instead of just dropping the last entries?

[themighty1]

nvm, I had the wrong understanding of what the code does.

[themighty1]

@gustavo-iniguez-goya , still struggling to understand why here
https://github.com/evilsocket/opensnitch/blob/75a08245a966768b602ddf678a94afcd09cc5ad3/daemon/procmon/parse.go#L47

we call

cleanUpCaches()

(which zeroes out all caches)
and then immediately proceed to search in the (now empty) cache with

getPidByInodeFromCache(inodeKey)

?

[gustavo-iniguez-goya]

The unique reason to call cleanUpCaches() is that it's unfinished. As you said, we should delete last entries, or better, last PIDs without network activity for the last n minutes, like we do here:

https://github.com/evilsocket/opensnitch/blob/75a08245a966768b602ddf678a94afcd09cc5ad3/daemon/procmon/audit/client.go#L203
https://github.com/evilsocket/opensnitch/blob/75a08245a966768b602ddf678a94afcd09cc5ad3/daemon/procmon/audit/client.go#L143

The reason to do it asynchronously is that I realized that we lost a lot of time checking for old events.

[themighty1]

Ok, please confirm what I understood from your response.
When you say "unfinished" you mean that at some point you wanted to add an InodeCache for the proc monitor method but later you realized that it was not efficient.
So getPidByInodeFromCache() will never find anything because the cache is empty, correct?

[gustavo-iniguez-goya]

When you say "unfinished" you mean that at some point you wanted to add an InodeCache for the proc monitor method but later you realized that it was not efficient.

Sorry, I meant that I should have added a better mechanism to clean up the cache, rather than delete all the cache entries.

So getPidByInodeFromCache() will never find anything because the cache is empty, correct?

Yes, if we reach to the point to have to clean up the cache yes.

Regarding the inodesCache, I think that it would be sufficient to use srcPort+srcIp+dstIp+dstPort, and maybe we'd get more hits at that point.

[themighty1]

Thanks, it makes sense now.
Now onto another cache: PID cache.
Currently, we check every /proc/PID/fd/* serially.
What are your thoughts about spanning (in advance) as many threads as there are fd/ entries and checking all the fds parallelly. That should give us a constant 10-20 microsecs lookup time.
Have you explored this idea? Are there any downsides to it?

[gustavo-iniguez-goya]

What are your thoughts about spanning (in advance) as many threads as there are fd/ entries and checking all the fds parallelly. That should give us a constant 10-20 microsecs lookup time.

do you mean on startup? I have no idea, I haven't explored that idea. But I guess that having so many threads would cause some penalty on the system. Bear in mind, that besides /proc/<PID>/fd also exists /proc/<PID>/task/<TID>/fd/ . And still, we'd need to get the inode of the connection.

We'd need a way to hook a function just after a socket is created, well using eBPF, or by using kprobes.

A few weeks ago I played with this idea, replacing the kprobe do_execve https://github.com/evilsocket/opensnitch/blob/master/daemon/procmon/watcher.go#L21 by security_socket_connect . That give us only the PIDs that create network activity, instead of every process spawned on the system (which reduces the number of PIDs saved, and improves performance).

I have no idea how to configure this kprobe to dump the fields of skb_buff, which would dump the details of the connections (https://elixir.bootlin.com/linux/v4.0/source/security/security.c#L1186). But, by monitoring the subEvent sock/inet_sock_set_state, we can monitor when a process sends a SYN:

# echo 1 > /sys/kernel/debug/tracing/events/sock/inet_sock_set_state/enable
# cat /sys/kernel/debug/tracing/trace_pipe
telnet-13732 [000] .... 1148057.045628: inet_sock_set_state: family=AF_INET protocol=IPPROTO_TCP sport=0 dport=23 saddr=192.168.1.102 daddr=1.1.1.1 saddrv6=::ffff:192.168.1.102 daddrv6=::ffff:1.1.1.1 oldstate=TCP_CLOSE newstate=TCP_SYN_SENT

Unfortunately, the source port in the majority of ocassions is 0 (sport=0 dport=23), so we can't use it to match against the connections that we capture via NFQUEUE. I have a local branch with these changes, just in case some day I can reuse for whatever.

[themighty1]

Thanks for sharing your observations.

The idea with parallel threads which I had in mind was to have as many threads running as there are /proc/PID/fd entries for the most internet-active process.
We tell each thread to wait for a signal and upon a signal the first thread does readlink(/proc/PID/fd/1), the second does readlink(/proc/PID/fd/2) and so on.
But having run some benchmarks, I see that it is not efficient to have threads. go only spends 4usec in a single readlink() call. Usually a process has a few hundred fds in proc.
With threads, there is a prohibitive cost in synchronizing the threads which runs into 100s of usecs.