[8.x](backport #41581) [packetbeat] Expire source port mappings.

[mergify]

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

---

This is an automatic backport of pull request #41581 done by Mergify.

[botelastic]

This pull request doesn't have a Team:<team> label.