private-etc breaks with 'net none' and 'dns=foo'

[glitsj16]

During testing of the recent private-etc refactoring (mostly in 5d0822c) I think there's a bug under specific conditions. I'll keep digging to try to pinpoint it as accurate as I can, but the below might already be a reproducer.

Relates to #5610 (see this comment).

This works as expected:

$ firejail --net=none --ignore=dns --private-etc=@tls-ca,java*
[glitsj16@lab ~]$ pwd
/home/glitsj16

These (and similar) variations however keeps failing for me:

$ firejail --net=none --private-etc=@tls-ca,java*
Error mount: fs_etc.c:142 fs_resolvconf: No such file or directory
Error: proc 79088 cannot sync with peer: unexpected EOF
Peer 79089 unexpectedly exited with status 1

$ firejail --net=none --private-etc=java*
Error mount: fs_etc.c:142 fs_resolvconf: No such file or directory
Error: proc 79088 cannot sync with peer: unexpected EOF
Peer 79089 unexpectedly exited with status 1

Note that the actual value of private-etc doesn't matter. When dns is there it will alway fail in my tests.

Current code logic in fs_etc.c always tries to create a new /etc/resolv.conf, even when networking is disabled via --net=none:

firejail/src/firejail/fs_etc.c

Lines 113 to 121 in 45a641d

	void fs_resolvconf(void) {
	if (arg_debug)
	printf("Creating a new /etc/resolv.conf file\n");
	FILE *fp = fopen(RUN_RESOLVCONF_FILE, "wxe");
	if (!fp) {
	fprintf(stderr, "Error: cannot create /etc/resolv.conf file\n");
	exit(1);
	}

If I understand the relevant code there, I think it would make sense to create /etc/resolv.conf in a more conditional way, something like:

[...]
 void fs_resolvconf(void) {
	if (arg_nonetwork)
		if (arg_debug)
			printf("Network disabled via --net=none. Skip creating new /etc/resolv.conf file\n");
		return;
 	if (arg_debug)
 		printf("Creating a new /etc/resolv.conf file\n");
 	FILE *fp = fopen(RUN_RESOLVCONF_FILE, "wxe");
[...]

Can anyone reproduce this? Thoughts on how to fix this in a safer way (if indeed this is a bug)?

UPDATE: I've made a small patch that seems to work for me. Here it is if anyone wants to test it:

--- a/src/firejail/fs_etc.c
+++ b/src/firejail/fs_etc.c
@@ -111,6 +111,10 @@
 }
 
 void fs_resolvconf(void) {
+	if (arg_nonetwork)
+		if (arg_debug)
+			printf("arg_nonetwork found (--net=none). Skip creating new /etc/resolv.conf file\n");
+		return;
 	if (arg_debug)
 		printf("Creating a new /etc/resolv.conf file\n");
 	FILE *fp = fopen(RUN_RESOLVCONF_FILE, "wxe");

Do note that this needs to go on top of current git master.

[glitsj16]

UPDATED PATCH (fixed misleading indentation):

--- a/src/firejail/fs_etc.c
+++ b/src/firejail/fs_etc.c
@@ -111,6 +111,11 @@
 }
 
 void fs_resolvconf(void) {
+	if (arg_nonetwork) {
+		if (arg_debug)
+			printf("arg_nonetwork found (--net=none). Skip /etc/resolv.conf file creation\n");
+		return;
+	}
 	if (arg_debug)
 		printf("Creating a new /etc/resolv.conf file\n");
 	FILE *fp = fopen(RUN_RESOLVCONF_FILE, "wxe");

[kmk3]

@glitsj16

It looks like #5737 was supposed to fix this.

Does the issue still happen?

[glitsj16]

@kmk3

It looks like #5737 was supposed to fix this.
Does the issue still happen?

This issue is fixed. Closing.