[packetbeat] Expire source port mappings.

[haesbaert]

port->pid mappings were only overwritten, never expired, the overwriting mechanism has some issues:

• It only overwrites if it manages to find the new pid, so it misses short lived processes.
• It only refreshes the mapping of said port, if a packet arriving on another port misses the lookup (otherwise the original port is found and returned). Meaning, once all ports are used at least once, the cache is filled and never mutated again.

The observable effect is that the user will see wrong process correlations to older/long lived processes, imagine the follwing:

• Long lived process makes short lived TCP connection from src_port S.
• Years later, a short lived process makes a TCP connection to somewhere else, but from the same src_port S. It hits the cache, since it had a mapping for S, so packetbeat incorrectly correlates the new short-lived process connection, with the old long lived process.

Related to a very long SDH, where a more in depth explanation of the bug can be found here, with a program to reproduce it.

• https://github.com/elastic/sdh-beats/issues/4604#issuecomment-2459969325
• https://github.com/elastic/sdh-beats/issues/4604#issuecomment-2460829030

The solution is to discard mappings that are "old enough", with a hardcoded window of 10 seconds, so as long as the port is not re-used in this window, we are fine.

This also makes sure the cache never becomes "immutable", since mappings will invariably get old, forcing a refresh.

It's a very conservative approach as I don't want to introduce other bugs by redesigning it, work is on the way to change how the cache works in linux anyway.

While here, I've noticed the locking was also wrong, we were doing the lookup unlocked, and also having to relock in case we have to update the mapping, so change this to grab the lock once and only once, interleaving is baad.

Proposed commit message

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have made corresponding change to the default configuration files
  - [ ] I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Test

The following program can be used to demonstrate the bug:

#define _GNU_SOURCE
#include <err.h>
#include <errno.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
#include <unistd.h>

#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>

#define M		"all your mappings are belong to us"
#define msleep(_x)	usleep((uint64_t)_x * 1000ULL)
#define nitems(_a)	(sizeof((_a)) / sizeof((_a)[0]))

static void
usage(void)
{
	fprintf(stderr, "usage: %s host port\n", program_invocation_short_name);

	exit(1);
}

int
do_connect(int bport, struct sockaddr_in *sin)
{
	int			fd;
	struct sockaddr_in	bsin;

	if ((fd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
		err(1, "socket");
	bzero(&bsin, sizeof(bsin));
	bsin.sin_family = AF_INET;
	bsin.sin_addr.s_addr = INADDR_ANY;
	bsin.sin_port = htons(bport);
	if (bind(fd, (struct sockaddr *)&bsin, sizeof(bsin)) == -1) {
		warn("bind: %d", bport);
		close(fd);
		return (-1);
	}
	if (connect(fd, sin, sizeof(*sin)) == -1) {
		warn("connect: %d", bport);
		close(fd);
		return (-1);
	}
	if (write(fd, M, strlen(M)) != strlen(M)) {
		warn("write: %d", bport);
		close(fd);
		return (-1);
	}

	return (fd);
}

int
main(int argc, char *argv[])
{
	struct sockaddr_in	 sin;
	int			 i, bport, fds[5000];

	if (argc != 3)
		usage();

	bzero(&sin, sizeof(sin));
	sin.sin_family = AF_INET;
	if (inet_aton(argv[1], &sin.sin_addr) != 1)
		err(1, "inet_aton: %s", argv[1]);
	/* no strtonum in leen0x */
	if (atoi(argv[2]) < 1024 || atoi(argv[2]) >= 65536)
		errx(1, "bad port %s", argv[2]);
	sin.sin_port = htons(atoi(argv[2]));

	bport = 1024;
	while (bport < 65536) {
		for (i = 0; i < (int)nitems(fds) && bport < 65536; i++) {
			fds[i] = do_connect(bport, &sin);
			if (fds[i] != -1) {
				printf("%d\n", bport);
				msleep(1);
			}
			bport++;
		}
		sleep(10);
		for (i = 0; i < (int)nitems(fds); i++) {
			if (fds[i] != -1) {
				close(fds[i]);
				fds[i] = -1;
			}
		}
	}
	
	return (0);
}

Build and run with:

$ cc -o all_your_mappings_are_belong_to_us all_your_mappings_are_belong_to_us.c  -Wall && ./all_your_mappings_are_belong_to_us 192.168.1.50 12345

It will do one connection per source port to the specified address (192.168.1.50:12345) and send some bytes, to make it easier, 192.168.1.50 should be in another machine than packetbeat, you can then run tcpbench, by yours truly, or any other service that will accept tcp connections and eat some bytes:

$ git clone https://github.com/bluhm/tcpbench-portable && cd tcpbench-portable && make && ./tcpbench -s4

After running all_your_mappings_are_belong_to_us, if you do a tcp connection to any other port, packetbeat will incorrectly assume it belongs to all_your_mappings_are_belong_to_us, see screenshots

Tested on 8.14.3 and main.

Screenshots

The circled in red thing is a wget to google.com, yet it things it's from all_your_mappings_are_belong_to_us.

After the fix, the mappings correctly show wget

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @haesbaert? 🙏.
For such, you'll need to label your PR with:

• The upcoming major version of the Elastic Stack
• The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit

[mergify]

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

[elasticmachine]

Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform)

[haesbaert]

cc @marc-gr @Tacklebox

[haesbaert]

The fix naturally results in more /proc parsing, basically at least once per 10 seconds for every connection, before it was basically "at every new connection that doesn't have a map already". The other conservative approach would be to do an O(n) expiration of every map every X seconds, while this would reduce the amount of /proc parsing on the positive case, it wouldn't change anything for the negative (if it misses the cache it tries to rebuild it anyway).

[nicholasberlin]

Some nitpicks, but LGTM

[nicholasberlin]

Should add this comment above line 250, also.

[nicholasberlin]

Suggested change

	// it's old enough. When we the first time here, our caller
	// it's old enough. The first time here, our caller