Re: pkg and root privileges
- In reply to: niko.nastonen_a_icloud.com: "pkg and root privileges"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 28 Jul 2022 15:02:09 UTC
Did a little bugfixing and cleanup, now looks much better.
diff --git a/fetch.c b/fetch.c
index a310fbc..a6d1fb8 100644
--- a/fetch.c
+++ b/fetch.c
@@ -34,6 +34,7 @@
#include <ctype.h>
#include <fcntl.h>
#include <errno.h>
+#include <pwd.h>
#include <stdio.h>
#include <string.h>
#include <fetch.h>
@@ -48,6 +49,8 @@
#include "private/utils.h"
#include "private/fetch.h"
+extern void drop_privileges(void);
+
static struct fetcher {
const char *scheme;
int (*open)(struct pkg_repo *, struct url *, off_t *);
@@ -82,7 +85,6 @@ static struct fetcher {
},
};
-
int
pkg_fetch_file_tmp(struct pkg_repo *repo, const char *url, char *dest,
time_t t)
@@ -175,6 +177,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 +201,22 @@ 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:
+ drop_privileges();
+ break;
+ default:
+ while (waitpid(pid, &pstat, 0) == -1 && errno == EINTR)
+ ;
+
+ return (WEXITSTATUS(pstat));
+ }
+
pkg_debug(1, "Request to fetch %s", url);
if (repo != NULL &&
strncmp(URL_SCHEME_PREFIX, url, strlen(URL_SCHEME_PREFIX)) == 0) {
@@ -256,6 +276,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,6 +304,7 @@ 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 (write(dest, buf, r) != r) {
pkg_emit_errno("write", "");
@@ -354,5 +376,6 @@ cleanup:
/* restore original doc */
fetchFreeURL(u);
- return (retcode);
+ /* exit child */
+ exit(retcode);
}
> On 26. Jul 2022, at 19.15, niko.nastonen@icloud.com wrote:
>
> 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);