Improve permission-error-reporting and raise open-file-limit - quark - quark we… | |
git clone git://git.suckless.org/quark | |
Log | |
Files | |
Refs | |
LICENSE | |
--- | |
commit e5db41118f5c9bfc27338a803d6d4eebec05cc1b | |
parent 7d26fc695d548b5a73305a97dce274a313e0f602 | |
Author: Laslo Hunhold <[email protected]> | |
Date: Sat, 16 Jan 2021 17:58:16 +0100 | |
Improve permission-error-reporting and raise open-file-limit | |
There was a small bug, namely that when quark was executed as a normal | |
user, the fork would fail because setrlimit() had made it impossible | |
beforehand (given it only controls thread-counts). Now the setrlimit() | |
for thread-count is done correctly after the fork. Additionally, | |
given we also open at least as many files as we have threads (and each | |
thread needs to keep multiple fd's open at the same time), we also now | |
set the open-file-limit properly. | |
Previously, when someone tried to execute quark as non-root, they would | |
get the error message | |
$ quark -p 5000 | |
quark: fork: Resource temporarily unavailable | |
$ | |
This was due to the aforementioned "bug", but even still, they would've | |
gotten an error message relating to a failed chroot. In either case, | |
it might've been a bit confusing, which is why it now shows a clear | |
error message on what's wrong and a possible "mitigation" (using | |
capabilities(7) as an alternative to setuid or root): | |
$ quark -p 5000 | |
quark: You need to run as root or have CAP_SYS_CHROOT set | |
$ | |
CAP_SYS_CHROOT alone is not sufficient to run quark, and it will print | |
further errors until all permissions are met, but I won't add a separate | |
error handling and logic just to appease with a cumulative | |
error-message. | |
When trying to bind to a privileged port, you get | |
$ quark -p 1000 | |
quark: You need to run as root or have CAP_NET_BIND_SERVICE set to | |
bind to privileged ports | |
$ | |
instead of | |
$ quark -p 1000 | |
quark: bind: Permission denied | |
$ | |
which is also a net-benefit. | |
In this context, this commit also improves the error-reporting when | |
someone tries 'dropping' to the root user or group by checking this | |
beforehand and not with a getuid() and getgid() later on. | |
Signed-off-by: Laslo Hunhold <[email protected]> | |
Diffstat: | |
M main.c | 70 +++++++++++++++++++++++------… | |
1 file changed, 53 insertions(+), 17 deletions(-) | |
--- | |
diff --git a/main.c b/main.c | |
@@ -586,12 +586,6 @@ main(int argc, char *argv[]) | |
} | |
} | |
- /* raise the process limit (2 + nthreads) */ | |
- rlim.rlim_cur = rlim.rlim_max = 2 + nthreads; | |
- if (setrlimit(RLIMIT_NPROC, &rlim) < 0) { | |
- die("setrlimit RLIMIT_NPROC:"); | |
- } | |
- | |
/* validate user and group */ | |
errno = 0; | |
if (!user || !(pwd = getpwnam(user))) { | |
@@ -609,6 +603,17 @@ main(int argc, char *argv[]) | |
handlesignals(sigcleanup); | |
+ /* set the fd-limit (3 initial + 4 per thread) */ | |
+ rlim.rlim_cur = rlim.rlim_max = 3 + 4 * (2 + nthreads); | |
+ if (setrlimit(RLIMIT_NOFILE, &rlim) < 0) { | |
+ if (errno == EPERM) { | |
+ die("You need to run as root or have " | |
+ "CAP_SYS_RESOURCE set"); | |
+ } else { | |
+ die("setrlimit:"); | |
+ } | |
+ } | |
+ | |
/* create a nonblocking listening socket for each thread */ | |
if (!(insock = reallocarray(insock, nthreads, sizeof(*insock)))) { | |
die("reallocarray:"); | |
@@ -640,6 +645,17 @@ main(int argc, char *argv[]) | |
die("signal: Failed to set SIG_IGN on SIGPIPE"); | |
} | |
+ /* set the thread limit (2 + nthreads) */ | |
+ rlim.rlim_cur = rlim.rlim_max = 2 + nthreads; | |
+ if (setrlimit(RLIMIT_NPROC, &rlim) < 0) { | |
+ if (errno == EPERM) { | |
+ die("You need to run as root or have " | |
+ "CAP_SYS_RESOURCE set"); | |
+ } else { | |
+ die("setrlimit:"); | |
+ } | |
+ } | |
+ | |
/* limit ourselves to reading the servedir and block further u… | |
eunveil(servedir, "r"); | |
eunveil(NULL, NULL); | |
@@ -649,18 +665,45 @@ main(int argc, char *argv[]) | |
die("chdir '%s':", servedir); | |
} | |
if (chroot(".") < 0) { | |
- die("chroot .:"); | |
+ if (errno == EPERM) { | |
+ die("You need to run as root or have " | |
+ "CAP_SYS_CHROOT set"); | |
+ } else { | |
+ die("chroot:"); | |
+ } | |
} | |
/* drop root */ | |
+ if (pwd->pw_uid == 0 || grp->gr_gid == 0) { | |
+ die("Won't run under root %s for hopefully obvious rea… | |
+ (pwd->pw_uid == 0) ? (grp->gr_gid == 0) ? | |
+ "user and group" : "user" : "group"); | |
+ } | |
+ | |
if (setgroups(1, &(grp->gr_gid)) < 0) { | |
- die("setgroups:"); | |
+ if (errno == EPERM) { | |
+ die("You need to run as root or have " | |
+ "CAP_SETGID set"); | |
+ } else { | |
+ die("setgroups:"); | |
+ } | |
} | |
if (setgid(grp->gr_gid) < 0) { | |
- die("setgid:"); | |
+ if (errno == EPERM) { | |
+ die("You need to run as root or have " | |
+ "CAP_SETGID set"); | |
+ } else { | |
+ die("setgid:"); | |
+ } | |
+ | |
} | |
if (setuid(pwd->pw_uid) < 0) { | |
- die("setuid:"); | |
+ if (errno == EPERM) { | |
+ die("You need to run as root or have " | |
+ "CAP_SETUID set"); | |
+ } else { | |
+ die("setuid:"); | |
+ } | |
} | |
if (udsname) { | |
@@ -669,13 +712,6 @@ main(int argc, char *argv[]) | |
epledge("stdio rpath proc inet", NULL); | |
} | |
- if (getuid() == 0) { | |
- die("Won't run as root user", argv0); | |
- } | |
- if (getgid() == 0) { | |
- die("Won't run as root group", argv0); | |
- } | |
- | |
/* accept incoming connections */ | |
handle_connections(insock, nthreads, nslots, &srv); | |