pkg and root privileges

Hi.

There was a recent discussion on the FreeBSD forum about security of pkg and its ability to drop root privileges when fetching packages.

I couldn’t help but notice that there was a git commit

fcceab3f with comment "drop privileges when using libfetch”

and another one

f3b0469e with comment "Stop dropping privileges when fetching as it causes more issues than it solved”.

Can I ask what kind of issues the first commit introduces and why pkg still goes out to the internet unprotected?

In case the issues are already solved by later commits, let me present a silly patch (mostly copied from fcceab3f) for branch "release-1.18” which makes fetch use nobody instead of root.

Feel free to modify it to match “the real BSD hacker standards, if applicable” :-)



diff --git a/libpkg/fetch.c b/libpkg/fetch.c
index a310fbc3..c8e02f5b 100644
--- a/libpkg/fetch.c
+++ b/libpkg/fetch.c
@@ -30,10 +30,14 @@
 #include <sys/wait.h>
 #include <sys/socket.h>
 #include <sys/time.h>
+#include <sys/types.h>
 
 #include <ctype.h>
 #include <fcntl.h>
+#include <err.h>
 #include <errno.h>
+#include <pwd.h>
+#include <signal.h>
 #include <stdio.h>
 #include <string.h>
 #include <fetch.h>
@@ -48,6 +52,10 @@
 #include "private/utils.h"
 #include "private/fetch.h"
 
+void sig_handler(int signal);
+extern void drop_privileges(void);
+int stop = 0;
+
 static struct fetcher {
        const char *scheme;
        int (*open)(struct pkg_repo *, struct url *, off_t *);
@@ -82,7 +90,6 @@ static struct fetcher {
        },
 };
 
-
 int
 pkg_fetch_file_tmp(struct pkg_repo *repo, const char *url, char *dest,
        time_t t)
@@ -160,6 +167,13 @@ pkg_fetch_file(struct pkg_repo *repo, const char *url, char *dest, time_t t,
        return (retcode);
 }
 
+void sig_handler(int signal)
+{
+    if (signal == SIGINT)
+           stop = 1;
+}
+
+
 #define URL_SCHEME_PREFIX      "pkg+"
 
 int
@@ -175,6 +189,8 @@ pkg_fetch_file_to_fd(struct pkg_repo *repo, const char *url, int dest,
        off_t            r;
        char             buf[8192];
        int              retcode = EPKG_OK;
+       int              pstat;
+       pid_t            pid;
        off_t            sz = 0;
        size_t           buflen = 0;
        size_t           left = 0;
@@ -197,6 +213,25 @@ pkg_fetch_file_to_fd(struct pkg_repo *repo, const char *url, int dest,
         * Error if using plain http://, https:// etc with SRV
         */
 
+       pid = fork();
+
+       switch (pid) {
+       case -1:
+               pkg_emit_error("Unable to fork");
+               return (EPKG_FATAL);
+       case 0:
+               sigset(SIGINT, sig_handler);
+               drop_privileges();
+               break;
+       default:
+               waitpid(pid, &pstat, 0);
+
+               if (WEXITSTATUS(pstat) != 0)
+                       return (EPKG_FATAL);
+
+               return (EPKG_OK);
+       }
+
        pkg_debug(1, "Request to fetch %s", url);
        if (repo != NULL &&
                strncmp(URL_SCHEME_PREFIX, url, strlen(URL_SCHEME_PREFIX)) == 0) {
@@ -256,6 +291,7 @@ pkg_fetch_file_to_fd(struct pkg_repo *repo, const char *url, int dest,
                        break;
                }
        }
+
        if (fetcher == NULL) {
                pkg_emit_error("Unknown scheme: %s", u->scheme);
                return (EPKG_FATAL);
@@ -283,7 +319,14 @@ pkg_fetch_file_to_fd(struct pkg_repo *repo, const char *url, int dest,
        left = sizeof(buf);
        if (sz > 0)
                left = sz - done;
+
        while ((r = fread(buf, 1, left < buflen ? left : buflen, remote)) > 0) {
+
+               if (stop)  {
+                       retcode = EPKG_FATAL;
+                       goto cleanup;
+               }
+
                if (write(dest, buf, r) != r) {
                        pkg_emit_errno("write", "");
                        retcode = EPKG_FATAL;
@@ -351,6 +394,13 @@ cleanup:
                futimes(dest, ftimes);
        }
 
+       if (strncmp(u->scheme, "ssh", 3) != 0) {
+               if (retcode == EPKG_OK)
+                       exit(0);
+
+               exit(EXIT_FAILURE);
+       }
+
        /* restore original doc */
        fetchFreeURL(u);