The sss_nss_mc_destroy_ctx() function will close the TCP socket of the daemon process

[answer9030]

The main logic of the daemon process is as follows:
1.process A calls the getpwnam() function, and then fork process B；
2.process B close all file descriptors and performs daemonlize，while process B create a TCP socket；
3.process B forks process C and monitors the status of process C. Process C exits and then forks process C again；
4.process C calls the getpwnam() function；

Under the following conditions, the TCP socket inherited by process C from process B is closed:
1.The SSSD service started and shut down again;
2.The passwd option in the /etc/nsswitch.conf file prioritizes using sss；
3.Start daemon process;
4.Add a user using the useradd command;
5.Kill process C and allow B to fork process C again;
6.The TCP socket inherited by process C from process B is closed;

The analysis process is as follows:
1.When starting the SSSD service, /var/lib/sss/mc/passwd file will be created；
2.process A calling the getpwnam() function resulted in dynamic library libnss_sss.so.2 's global variable pw_mc_ctx->fd is 3;
3.process B close all file descriptors and create a TCP socket, TCP socket occupies file descriptor 3, pw_mc_ctx->fd is also 3;
4.the useradd command caused the /var/lib/sss/mc/passwd file to be deleted;
5.when process C calls the getpwnam() function again, it will trigger the call to sss_nss_mc_destroy_ctx() function to close file descriptor number 3;
6.At this point, the file descriptor number 3 is TCP socket inherited from process B;

The daemon process code is as follows

#define _GNU_SOURCE
#include <sched.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <sys/wait.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <pwd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>


static int enter_daemon(void)
{
	int pid = fork();
	if(pid < 0) {
		fprintf(stderr, "fork failure:%s\n", strerror(errno));
		return pid;
	}
	else if(pid == 0) {
		//the chile process close all file descriptors
		for(int fd = 3; fd <= 32; ++fd) {
			close(fd);
		}
		
		setgid(getpid());
		return 0;
	}
	else {
		//parent process exits
		fprintf(stdout, "original process %d goto exiting...\n", getpid());
		exit(EXIT_SUCCESS);
	}

	return 0;
}

static int tcp_socket_init(void)
{
	int ret;
	int fd = socket(AF_INET, SOCK_STREAM, 0);
	if(fd < 0) {
		fprintf(stderr, "socket failure:%s\n", strerror(errno));
		return fd;
	}

	struct sockaddr_in listen_addr = {
		.sin_family = AF_INET,
		.sin_port = ntohs(6666),
		.sin_addr = {
			.s_addr = 0,
		},
	};
	ret = bind(fd, (struct sockaddr*)&listen_addr, sizeof(listen_addr));
	if(ret < 0) {
		fprintf(stderr, "bind failure:%s\n", strerror(errno));
		close(fd);
		return ret;
	}

	ret = listen(fd, 1024);
	if(ret < 0) {
		fprintf(stderr, "listen failure:%s\n", strerror(errno));
		close(fd);
		return ret;
	}

	return fd;
}

void loop()
{
	while(1)
	{}
}

int main(int argc, char *argv[])
{
	//libnss_sss.so will open /var/lib/sss/mc/passwd and will occupy a fd when calling getpwnam()
	getpwnam("any");

	if(enter_daemon() < 0) {
		return -1;
	}

	printf("parent process is: %d\n", getpid());
	//create a tcp socket
	int net_fd = tcp_socket_init();

	while(1) {
		int pid = fork();
		if(pid < 0) {
			fprintf(stderr, "fork failure:%s\n", strerror(errno));
			return 0;
		}
		else if(pid == 0) {
			//child process call getpwnam()
			printf("child process is: %d\n", getpid());
			getpwnam("any");
			
			loop();
		}
		else {
			//parent process monitors child process
			int child_state;
			pid_t child = waitpid(pid, &child_state, 0);
			if(child < 0) {
				fprintf(stderr, "waitpid failure:%s\n", strerror(errno));
			}
			else {
				fprintf(stdout, "Test finish,%d quit...\n", pid);
			}
		}
	}

	return 0;
}

[alexey-tikhonov]

for(int fd = 3; fd <= 32; ++fd) {

This code ruins internal status of libnss_sss.so
Besides memory-cache FDs (passwd, groups, initgroups, SID), there are also UNIX socket to talk to 'sssd_nss'.
And who knows what else in other libs.

[answer9030]

for(int fd = 3; fd <= 32; ++fd) {

but this is the common way of writing daemon process.

[alexey-tikhonov]

Common thing is to close fd's 0, 1 and 2. Everything else depends on.

In particular, calling getpwnam() before first fork() is already questionable.

[answer9030]

The specifications I found are Close all open file descriptors except standard input, output, and error (i.e. the first three file descriptors 0, 1, 2). This ensures that no accidentally passed file descriptor stays around in the daemon process. from https://man.archlinux.org/man/daemon.7

Now some applications are written according to this specification, which causes conflicts

[alexey-tikhonov]

What would be your proposal?

To make 'libnss_sss' completely stateless?

[answer9030]

I'm not sure if can change it to stateless.
If possible, it's best to change it to stateless

[answer9030]

Why not close "/var/lib/sss/mc/*" fd after mmap?

[pbrezina]

The specifications I found are Close all open file descriptors except standard input, output, and error (i.e. the first three file descriptors 0, 1, 2). This ensures that no accidentally passed file descriptor stays around in the daemon process. from https://man.archlinux.org/man/daemon.7

Now some applications are written according to this specification, which causes conflicts

Hi, I think it means that you should close all file descriptors that you have opened. You should not tinker with libraries and fds you don't know. How did you come up with number 32?

How can I use your code to reproduce the issue?

[answer9030]

The specifications I found are Close all open file descriptors except standard input, output, and error (i.e. the first three file descriptors 0, 1, 2). This ensures that no accidentally passed file descriptor stays around in the daemon process. from https://man.archlinux.org/man/daemon.7
Now some applications are written according to this specification, which causes conflicts

Hi, I think it means that you should close all file descriptors that you have opened. You should not tinker with libraries and fds you don't know. How did you come up with number 32?

How can I use your code to reproduce the issue?

Under the following conditions, the TCP socket inherited by process C from process B is closed:
1.The SSSD service started and shut down again;
2.The passwd option in the /etc/nsswitch.conf file prioritizes using sss；
3.Start daemon process;
4.Add a user using the useradd command;
5.Kill process C and allow B to fork process C again;
6.The TCP socket inherited by process C from process B is closed;

[pbrezina]

I did some research following on my previous comments. Some forum posts (e.g. on stackoverflow) suggest to close all file descriptors in a for loop, like you do, but only as a simple solution to clean up after poor application. You should not, by all means, close valid file descriptors that will be used by the fork - in this case SSSD's fds, but it can be even fd that you opened. You should only close fds that you are not going to use anymore.

for loop itself, is a bad practice. The parent process should correctly clean up for itself (close fds that are no longer needed), additional 32 is just a made up number. There can be much more file descriptors opened, see ulimit -n.

To reiterate: You should only close file descriptors that your application opened and the child process is not going to use.

[answer9030]

SSSD's fds is not visible to the application, and the application believes that 3~32 fds will no longer be used, so it has been closed.

[alexey-tikhonov]

I'm reopening ticket due to real example:
https://github.com/TigerVNC/tigervnc/blob/effd854bfd19654fa67ff3d39514a91a246b8ae6/unix/xserver/hw/vnc/xvnc.c#L369

While I think this is a bug in TigerVNC, it seems we have to deal with this somehow.

[alexey-tikhonov]

Pushed PR: #7085

• master
  • 2bcfb7f - SSS_CLIENT: check if reponder socket was hijacked
  • 0344c41 - SSS_CLIENT: check if mem-cache fd was hijacked
  • 958a5e2 - SSS_CLIENT: MC: in case mem-cache file validation fails,
• sssd-2-9
  • abb146e - SSS_CLIENT: check if reponder socket was hijacked
  • a186224 - SSS_CLIENT: check if mem-cache fd was hijacked
  • 160738e - SSS_CLIENT: MC: in case mem-cache file validation fails,