[hackers] [quark] Improve permission-error-reporting and raise open-file-limit || Laslo Hunhold

From: <git_AT_suckless.org>
Date: Sat, 16 Jan 2021 18:08:33 +0100 (CET)

commit e5db41118f5c9bfc27338a803d6d4eebec05cc1b
Author: Laslo Hunhold <dev_AT_frign.de>
AuthorDate: Sat Jan 16 17:58:16 2021 +0100
Commit: Laslo Hunhold <dev_AT_frign.de>
CommitDate: Sat Jan 16 17:58:16 2021 +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 <dev_AT_frign.de>

diff --git a/main.c b/main.c
index e26cc77..2e8f3e7 100644
--- a/main.c
+++ b/main.c
_AT_@ -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))) {
_AT_@ -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:");
_AT_@ -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 unveils */
                 eunveil(servedir, "r");
                 eunveil(NULL, NULL);
_AT_@ -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 reasons",
+ (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) {
_AT_@ -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);
 
Received on Sat Jan 16 2021 - 18:08:33 CET