fix initgroups issue in process library

https://github.com/haskell/process/pull/148
2019-07-19 03:02:09 +00:00 · 2019-07-19 03:02:09 +00:00 · bbd41d24d2
parent 5ee2271b2b
commit bbd41d24d2
3 changed files with 128 additions and 0 deletions
--- a/3e0812fe9d3f4712638a1c4c49bf2b2a7dc4311b.patch
+++ b/3e0812fe9d3f4712638a1c4c49bf2b2a7dc4311b.patch
@ -0,0 +1,92 @@
+From 3e0812fe9d3f4712638a1c4c49bf2b2a7dc4311b Mon Sep 17 00:00:00 2001
+From: Ben Gamari <ben@smart-cactus.org>
+Date: Mon, 1 Jul 2019 11:03:33 -0400
+Subject: [PATCH] Call initgroups before setuid
+
+Previously we would fail to call initgroups before setuid'ing. This
+meant that our groups we not be reset to reflect those our new user
+belongs to. Fix this.
+---
+ cbits/runProcess.c   | 32 +++++++++++++++++++++++++++++---
+ include/runProcess.h |  4 ++++
+ 2 files changed, 33 insertions(+), 3 deletions(-)
+
+diff --git a/cbits/runProcess.c b/cbits/runProcess.c
+index 10794bc..84d5fd4 100644
+--- a/cbits/runProcess.c
+++ b/cbits/runProcess.c
+@@ -33,6 +33,10 @@ static long max_fd = 0;
+ extern void blockUserSignals(void);
+ extern void unblockUserSignals(void);
+ 
+// These are arbitrarily chosen -- JP
+#define forkSetgidFailed 124
+#define forkSetuidFailed 125
+
+ // See #1593.  The convention for the exit code when
+ // exec() fails seems to be 127 (gleened from C's
+ // system()), but there's no equivalent convention for
+@@ -40,9 +44,8 @@ extern void unblockUserSignals(void);
+ #define forkChdirFailed 126
+ #define forkExecFailed  127
+ 
+-// These are arbitrarily chosen -- JP
+-#define forkSetgidFailed 124
+-#define forkSetuidFailed 125
+#define forkGetpwuidFailed 128
+#define forkInitgroupsFailed 129
+ 
+ __attribute__((__noreturn__))
+ static void childFailed(int pipe, int failCode) {
+@@ -182,6 +185,23 @@ runInteractiveProcess (char *const args[],
+         }
+ 
+         if ( childUser) {
+            // Using setuid properly first requires that we initgroups.
+            // However, to do this we must know the username of the user we are
+            // switching to.
+            struct passwd pw;
+            struct passwd *res = NULL;
+            int buf_len = sysconf(_SC_GETPW_R_SIZE_MAX);
+            char *buf = malloc(buf_len);
+            gid_t suppl_gid = childGroup ? *childGroup : getgid();
+            if ( getpwuid_r(*childUser, &pw, buf, buf_len, &res) != 0) {
+                childFailed(forkCommunicationFds[1], forkGetpwuidFailed);
+            }
+            if ( res == NULL ) {
+                childFailed(forkCommunicationFds[1], forkGetpwuidFailed);
+            }
+            if ( initgroups(res->pw_name, suppl_gid) != 0) {
+                childFailed(forkCommunicationFds[1], forkInitgroupsFailed);
+            }
+             if ( setuid( *childUser) != 0) {
+                 // ERROR
+                 childFailed(forkCommunicationFds[1], forkSetuidFailed);
+@@ -330,6 +350,12 @@ runInteractiveProcess (char *const args[],
+         case forkSetuidFailed:
+             *failed_doing = "runInteractiveProcess: setuid";
+             break;
+        case forkGetpwuidFailed:
+            *failed_doing = "runInteractiveProcess: getpwuid";
+            break;
+        case forkInitgroupsFailed:
+            *failed_doing = "runInteractiveProcess: initgroups";
+            break;
+         default:
+             *failed_doing = "runInteractiveProcess: unknown";
+             break;
+diff --git a/include/runProcess.h b/include/runProcess.h
+index 3807389..dff3905 100644
+--- a/include/runProcess.h
+++ b/include/runProcess.h
+@@ -21,6 +21,10 @@
+ 
+ #include <unistd.h>
+ #include <sys/types.h>
+#if !(defined(_MSC_VER) || defined(__MINGW32__) || defined(_WIN32))
+#include <pwd.h>
+#include <grp.h>
+#endif
+ 
+ #ifdef HAVE_FCNTL_H
+ #include <fcntl.h>
--- a/73ea41b3622e2e578d928f7513941aac9d873279.patch
+++ b/73ea41b3622e2e578d928f7513941aac9d873279.patch
@ -0,0 +1,24 @@
+From 73ea41b3622e2e578d928f7513941aac9d873279 Mon Sep 17 00:00:00 2001
+From: Ben Gamari <ben@smart-cactus.org>
+Date: Mon, 1 Jul 2019 11:02:45 -0400
+Subject: [PATCH] Fix incorrect case fallthrough
+
+The error message lookup logic would fallthrough from the
+forkSetuidFailed case into the default case, meaning that the error
+message of the former would never be returned.
+---
+ cbits/runProcess.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/cbits/runProcess.c b/cbits/runProcess.c
+index c621158..10794bc 100644
+--- a/cbits/runProcess.c
+++ b/cbits/runProcess.c
+@@ -329,6 +329,7 @@ runInteractiveProcess (char *const args[],
+             break;
+         case forkSetuidFailed:
+             *failed_doing = "runInteractiveProcess: setuid";
+            break;
+         default:
+             *failed_doing = "runInteractiveProcess: unknown";
+             break;
--- a/ghc.spec
+++ b/ghc.spec
@ -60,6 +60,10 @@ Patch2:  ghc-Cabal-install-PATH-warning.patch
 # https://phabricator.haskell.org/rGHC4eebc8016f68719e1ccdf460754a97d1f4d6ef05
 Patch6: ghc-8.6.3-sphinx-1.8.patch

+# https://github.com/haskell/process/pull/148
+Patch10: https://github.com/haskell/process/commit/73ea41b3622e2e578d928f7513941aac9d873279.patch
+Patch11: https://github.com/haskell/process/commit/3e0812fe9d3f4712638a1c4c49bf2b2a7dc4311b.patch
+
 # Arch dependent patches

 # arm
@ -304,6 +308,12 @@ packages to be automatically installed too.

 rm -r libffi-tarballs

+(
+cd libraries/process
+%patch10 -p1 -b .orig10
+%patch11 -p1 -b .orig11
+)
+
 %ifarch armv7hl
 %patch12 -p1 -b .orig
 %endif
@ -665,6 +675,8 @@ make test
 - https://downloads.haskell.org/~ghc/8.6.5/docs/html/users_guide/8.6.3-notes.html
 - https://downloads.haskell.org/~ghc/8.6.5/docs/html/users_guide/8.6.4-notes.html
 - https://downloads.haskell.org/~ghc/8.6.5/docs/html/users_guide/8.6.5-notes.html
+- fix process library initgroups issue
+  (https://github.com/haskell/process/pull/148)
 - enable s390x with unregisterized workaround for 8.4 (#1648537)
 - also re-enable ppc64 with bigendian patch for containers (#1651448)