From nobody Thu Jul 28 15:02:09 2022 X-Original-To: freebsd-pkg@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Ltv572Zmjz4X0MP for ; Thu, 28 Jul 2022 15:02:19 +0000 (UTC) (envelope-from niko.nastonen@icloud.com) Received: from pv50p00im-zteg10011501.me.com (pv50p00im-zteg10011501.me.com [17.58.6.42]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4Ltv560LGJz45gT for ; Thu, 28 Jul 2022 15:02:17 +0000 (UTC) (envelope-from niko.nastonen@icloud.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=1a1hai; t=1659020535; bh=n3nnKY7sBq4P0vtVCH7v636GN7KTL1Hb/G25vbb9q4c=; h=From:Content-Type:Mime-Version:Subject:Date:To:Message-Id; b=F8AgVi8oh6PcxOh8PCwy3n+DIQD34LyQOTW8HfMjLCTkRwZFRDgWmZ0xdBuE3JVls HkVAnhxuhlQjYzSAbM3k12VsoQH3wOcDLVAoN9ucRjHnWNXwavnftu1gz9QraVZHCf pbge61fTgLipMjCXfgDUvG93bnAgTOe4aBmkfsYhZD8cKB9cNxqxKkeqU/x6jabjwW RO+Ek7QHmPVK4rKKFWUIEYC4X7OtoTuw5sIaZskg1W802VozMGHjqmFTRQIyzGbhWW Al0JeBuQQW8XP33Q47aWNmsTN+ktmZW7HhawOLiE/ZyyJLvS6MZirnfmHlgqEPKred Egdzs19dfwVxw== Received: from smtpclient.apple (pv50p00im-dlb-asmtp-mailmevip.me.com [17.56.9.10]) by pv50p00im-zteg10011501.me.com (Postfix) with ESMTPSA id F1E5B2E05A5 for ; Thu, 28 Jul 2022 15:02:12 +0000 (UTC) From: niko.nastonen@icloud.com Content-Type: multipart/alternative; boundary="Apple-Mail=_43D110D9-E43D-4961-BD8D-30CE0BD81234" List-Id: Binary package management and package tools discussion List-Archive: https://lists.freebsd.org/archives/freebsd-pkg List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-pkg@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.100.31\)) Subject: Re: pkg and root privileges Date: Thu, 28 Jul 2022 18:02:09 +0300 References: <0320D2DB-F61B-4F8B-B80F-D7765860283E@icloud.com> To: "freebsd-pkg@freebsd.org" In-Reply-To: <0320D2DB-F61B-4F8B-B80F-D7765860283E@icloud.com> Message-Id: <00413BE8-BDF9-41B9-92DE-224A26A78CD4@icloud.com> X-Mailer: Apple Mail (2.3696.100.31) X-Proofpoint-ORIG-GUID: OlLQ8oAXtcymEEjPxwEE5rOqHIqQx2oV X-Proofpoint-GUID: OlLQ8oAXtcymEEjPxwEE5rOqHIqQx2oV X-Proofpoint-Virus-Version: =?UTF-8?Q?vendor=3Dfsecure_engine=3D1.1.170-22c6f66c430a71ce266a39bfe25bc?= =?UTF-8?Q?2903e8d5c8f:6.0.138,18.0.572,17.11.64.514.0000000_definitions?= =?UTF-8?Q?=3D2020-02-14=5F11:2020-02-14=5F02,2020-02-14=5F11,2022-02-23?= =?UTF-8?Q?=5F01_signatures=3D0?= X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 malwarescore=0 phishscore=0 clxscore=1015 adultscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000 definitions=main-2207280068 X-Rspamd-Queue-Id: 4Ltv560LGJz45gT X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org; dkim=pass header.d=icloud.com header.s=1a1hai header.b=F8AgVi8o; dmarc=pass (policy=quarantine) header.from=icloud.com; spf=pass (mx1.freebsd.org: domain of niko.nastonen@icloud.com designates 17.58.6.42 as permitted sender) smtp.mailfrom=niko.nastonen@icloud.com X-Spamd-Result: default: False [-6.60 / 15.00]; WHITELIST_SPF_DKIM(-3.00)[icloud.com:d:+,icloud.com:s:+]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_SHORT(-1.00)[-0.999]; DMARC_POLICY_ALLOW(-0.50)[icloud.com,quarantine]; MV_CASE(0.50)[]; R_SPF_ALLOW(-0.20)[+ip4:17.58.0.0/16]; R_DKIM_ALLOW(-0.20)[icloud.com:s=1a1hai]; RCVD_IN_DNSWL_LOW(-0.10)[17.58.6.42:from]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[freebsd-pkg@freebsd.org]; ASN(0.00)[asn:714, ipnet:17.58.0.0/20, country:US]; TO_DN_EQ_ADDR_ALL(0.00)[]; FROM_NO_DN(0.00)[]; RCPT_COUNT_ONE(0.00)[1]; MLMMJ_DEST(0.00)[freebsd-pkg@freebsd.org]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; FROM_EQ_ENVFROM(0.00)[]; DKIM_TRACE(0.00)[icloud.com:+]; FREEMAIL_FROM(0.00)[icloud.com]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:~]; MID_RHS_MATCH_FROM(0.00)[]; FREEMAIL_ENVFROM(0.00)[icloud.com]; DWL_DNSWL_NONE(0.00)[icloud.com:dkim]; RCVD_TLS_ALL(0.00)[] X-ThisMailContainsUnwantedMimeParts: N --Apple-Mail=_43D110D9-E43D-4961-BD8D-30CE0BD81234 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 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 #include #include +#include #include #include #include @@ -48,6 +49,8 @@ #include "private/utils.h" #include "private/fetch.h" =20 +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 { }, }; =20 - 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 =3D EPKG_OK; + int pstat; + pid_t pid; off_t sz =3D 0; size_t buflen =3D 0; size_t left =3D 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 */ =20 + pid =3D 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) =3D=3D -1 && errno =3D=3D = EINTR) + ; + + return (WEXITSTATUS(pstat)); + } + pkg_debug(1, "Request to fetch %s", url); if (repo !=3D NULL && strncmp(URL_SCHEME_PREFIX, url, = strlen(URL_SCHEME_PREFIX)) =3D=3D 0) { @@ -256,6 +276,7 @@ pkg_fetch_file_to_fd(struct pkg_repo *repo, const = char *url, int dest, break; } } + if (fetcher =3D=3D 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 =3D sizeof(buf); if (sz > 0) left =3D sz - done; + while ((r =3D fread(buf, 1, left < buflen ? left : buflen, = remote)) > 0) { if (write(dest, buf, r) !=3D r) { pkg_emit_errno("write", ""); @@ -354,5 +376,6 @@ cleanup: /* restore original doc */ fetchFreeURL(u); =20 - return (retcode); + /* exit child */ + exit(retcode); } > On 26. Jul 2022, at 19.15, niko.nastonen@icloud.com wrote: >=20 > Hi. >=20 > There was a recent discussion on the FreeBSD forum about security of = pkg and its ability to drop root privileges when fetching packages. >=20 > I couldn=E2=80=99t help but notice that there was a git commit >=20 > fcceab3f with comment "drop privileges when using libfetch=E2=80=9D >=20 > and another one >=20 > f3b0469e with comment "Stop dropping privileges when fetching as it = causes more issues than it solved=E2=80=9D. >=20 > Can I ask what kind of issues the first commit introduces and why pkg = still goes out to the internet unprotected? >=20 > 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=E2=80= =9D which makes fetch use nobody instead of root. >=20 > Feel free to modify it to match =E2=80=9Cthe real BSD hacker = standards, if applicable=E2=80=9D :-) >=20 >=20 >=20 > 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 > #include > #include > +#include > =20 > #include > #include > +#include > #include > +#include > +#include > #include > #include > #include > @@ -48,6 +52,10 @@ > #include "private/utils.h" > #include "private/fetch.h" > =20 > +void sig_handler(int signal); > +extern void drop_privileges(void); > +int stop =3D 0; > + > static struct fetcher { > const char *scheme; > int (*open)(struct pkg_repo *, struct url *, off_t *); > @@ -82,7 +90,6 @@ static struct fetcher { > }, > }; > =20 > - > 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); > } > =20 > +void sig_handler(int signal) > +{ > + if (signal =3D=3D SIGINT) > + stop =3D 1; > +} > + > + > #define URL_SCHEME_PREFIX "pkg+" > =20 > 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 =3D EPKG_OK; > + int pstat; > + pid_t pid; > off_t sz =3D 0; > size_t buflen =3D 0; > size_t left =3D 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 > */ > =20 > + pid =3D 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) !=3D 0) > + return (EPKG_FATAL); > + > + return (EPKG_OK); > + } > + > pkg_debug(1, "Request to fetch %s", url); > if (repo !=3D NULL && > strncmp(URL_SCHEME_PREFIX, url, = strlen(URL_SCHEME_PREFIX)) =3D=3D 0) { > @@ -256,6 +291,7 @@ pkg_fetch_file_to_fd(struct pkg_repo *repo, const = char *url, int dest, > break; > } > } > + > if (fetcher =3D=3D 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 =3D sizeof(buf); > if (sz > 0) > left =3D sz - done; > + > while ((r =3D fread(buf, 1, left < buflen ? left : buflen, = remote)) > 0) { > + > + if (stop) { > + retcode =3D EPKG_FATAL; > + goto cleanup; > + } > + > if (write(dest, buf, r) !=3D r) { > pkg_emit_errno("write", ""); > retcode =3D EPKG_FATAL; > @@ -351,6 +394,13 @@ cleanup: > futimes(dest, ftimes); > } > =20 > + if (strncmp(u->scheme, "ssh", 3) !=3D 0) { > + if (retcode =3D=3D EPKG_OK) > + exit(0); > + > + exit(EXIT_FAILURE); > + } > + > /* restore original doc */ > fetchFreeURL(u); --Apple-Mail=_43D110D9-E43D-4961-BD8D-30CE0BD81234 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 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 =3D = EPKG_OK;
+       int      =         pstat;
+       = pid_t            pid;
        off_t        =     sz =3D 0;
        = size_t           buflen =3D 0;
        size_t       =     left =3D 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 =3D = 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) =3D=3D -1 && errno =3D=3D = EINTR)
+                 =       ;
+
+     =           return = (WEXITSTATUS(pstat));
+       = }
+
        pkg_debug(1, = "Request to fetch %s", url);
        if = (repo !=3D NULL &&
        =         strncmp(URL_SCHEME_PREFIX, url, = strlen(URL_SCHEME_PREFIX)) =3D=3D 0) {
@@ -256,6 +276,7 @@ = pkg_fetch_file_to_fd(struct pkg_repo *repo, const char *url, int = dest,
                =         break;
        =         }
        = }
+
        if (fetcher =3D=3D = 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 =3D sizeof(buf);
        if (sz > 0)
                left = =3D sz - done;
+
        = while ((r =3D fread(buf, 1, left < buflen ? left : buflen, remote)) = > 0) {
              =   if (write(dest, buf, r) !=3D 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=E2=80=99t help but notice that there was a git = commit

fcceab3f with comment "drop = privileges when using libfetch=E2=80=9D

and another one

f3b0469e with comment "Stop dropping = privileges when fetching as it causes more issues than it = solved=E2=80=9D.

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=E2=80=9D which makes = fetch use nobody instead of root.

Feel free to modify it to match =E2=80=9C= the real BSD hacker standards, if applicable=E2=80=9D :-)



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 =3D 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 =3D=3D = SIGINT)
+           stop =3D 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 =3D = EPKG_OK;
+       int      =         pstat;
+       = pid_t            pid;
        off_t        =     sz =3D 0;
        = size_t           buflen =3D 0;
        size_t       =     left =3D 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 =3D = 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) !=3D = 0)
+                 =       return (EPKG_FATAL);
+
+               return = (EPKG_OK);
+       }
+
        pkg_debug(1, = "Request to fetch %s", url);
        if = (repo !=3D NULL &&
        =         strncmp(URL_SCHEME_PREFIX, url, = strlen(URL_SCHEME_PREFIX)) =3D=3D 0) {
@@ -256,6 +291,7 @@ = pkg_fetch_file_to_fd(struct pkg_repo *repo, const char *url, int = dest,
                =         break;
        =         }
        = }
+
        if (fetcher =3D=3D = 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 =3D sizeof(buf);
        if (sz > 0)
                left = =3D sz - done;
+
        = while ((r =3D fread(buf, 1, left < buflen ? left : buflen, remote)) = > 0) {
+
+         =       if (stop)  {
+         =               retcode =3D = EPKG_FATAL;
+             =           goto cleanup;
+               = }
+
              =   if (write(dest, buf, r) !=3D r) {
        =                 = pkg_emit_errno("write", "");
        =                 retcode =3D = EPKG_FATAL;
@@ -351,6 +394,13 @@ cleanup:
                = futimes(dest, ftimes);
        = }
 
+       if = (strncmp(u->scheme, "ssh", 3) !=3D 0) {
+     =           if (retcode =3D=3D EPKG_OK)
+                 =       exit(0);
+
+     =           exit(EXIT_FAILURE);
+       }
+
    =     /* restore original doc */
        = fetchFreeURL(u);

= --Apple-Mail=_43D110D9-E43D-4961-BD8D-30CE0BD81234--