process: move POSIX credential accessors into node_credentials.cc

Expose the POSIX credential accessors through `internalBinding('credentials')` instead of setting them on the process or bootstrapper object from C++ directly. Also moves `SafeGetEnv` from `internalBinding('util')` to `internalBinding('credentials')` since it's closely related to the credentials. In the JS land, instead of wrapping the bindings then writing to the process object directly in main_thread_only.js, return the wrapped functions back to bootstrap/node.js where they get written to the process object conditionally for clarity. Refs: nodejs#24961 PR-URL: nodejs#25066 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
BridgeAR · Jan 16, 2019 · 39ff53a · 39ff53a
1 parent 9ca2237
commit 39ff53a
Show file tree

Hide file tree

Showing 17 changed files with 498 additions and 474 deletions.
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -23,8 +23,7 @@ const {
   _setupPromises, _chdir, _cpuUsage,
   _hrtime, _hrtimeBigInt,
   _memoryUsage, _rawDebug,
-  _umask, _initgroups, _setegid, _seteuid,
-  _setgid, _setuid, _setgroups,
+  _umask,
   _shouldAbortOnUncaughtToggle
 } = bootstrappers;
 const { internalBinding, NativeModule } = loaderExports;
@@ -73,13 +72,28 @@ function startup() {
   NativeModule.require('internal/process/warning').setup();
   NativeModule.require('internal/process/next_tick').setup(_setupNextTick,
                                                            _setupPromises);
+  const credentials = internalBinding('credentials');
+  if (credentials.implementsPosixCredentials) {
+    process.getuid = credentials.getuid;
+    process.geteuid = credentials.geteuid;
+    process.getgid = credentials.getgid;
+    process.getegid = credentials.getegid;
+    process.getgroups = credentials.getgroups;
+
+    if (isMainThread) {
+      const wrapped = mainThreadSetup.wrapPosixCredentialSetters(credentials);
+      process.initgroups = wrapped.initgroups;
+      process.setgroups = wrapped.setgroups;
+      process.setegid = wrapped.setegid;
+      process.seteuid = wrapped.seteuid;
+      process.setgid = wrapped.setgid;
+      process.setuid = wrapped.setuid;
+    }
+  }
 
   if (isMainThread) {
     mainThreadSetup.setupStdio();
-    mainThreadSetup.setupProcessMethods(
-      _chdir, _umask, _initgroups, _setegid, _seteuid,
-      _setgid, _setuid, _setgroups
-    );
+    mainThreadSetup.setupProcessMethods(_chdir, _umask);
   } else {
     workerThreadSetup.setupStdio();
   }

diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -34,7 +34,7 @@ const {
   internalModuleReadJSON,
   internalModuleStat
 } = internalBinding('fs');
-const { safeGetenv } = internalBinding('util');
+const { safeGetenv } = internalBinding('credentials');
 const {
   makeRequireFunction,
   requireDepth,

diff --git a/lib/internal/process/main_thread_only.js b/lib/internal/process/main_thread_only.js
@@ -29,13 +29,7 @@ function setupStdio() {
 
 // Non-POSIX platforms like Windows don't have certain methods.
 // Workers also lack these methods since they change process-global state.
-function setupProcessMethods(_chdir, _umask, _initgroups, _setegid,
-                             _seteuid, _setgid, _setuid, _setgroups) {
-  if (_setgid !== undefined) {
-    setupPosixMethods(_initgroups, _setegid, _seteuid,
-                      _setgid, _setuid, _setgroups);
-  }
-
+function setupProcessMethods(_chdir, _umask) {
   process.chdir = function chdir(directory) {
     validateString(directory, 'directory');
     return _chdir(directory);
@@ -51,10 +45,17 @@ function setupProcessMethods(_chdir, _umask, _initgroups, _setegid,
   };
 }
 
-function setupPosixMethods(_initgroups, _setegid, _seteuid,
-                           _setgid, _setuid, _setgroups) {
-
-  process.initgroups = function initgroups(user, extraGroup) {
+function wrapPosixCredentialSetters(credentials) {
+  const {
+    initgroups: _initgroups,
+    setgroups: _setgroups,
+    setegid: _setegid,
+    seteuid: _seteuid,
+    setgid: _setgid,
+    setuid: _setuid
+  } = credentials;
+
+  function initgroups(user, extraGroup) {
     validateId(user, 'user');
     validateId(extraGroup, 'extraGroup');
     // Result is 0 on success, 1 if user is unknown, 2 if group is unknown.
@@ -64,25 +65,9 @@ function setupPosixMethods(_initgroups, _setegid, _seteuid,
     } else if (result === 2) {
       throw new ERR_UNKNOWN_CREDENTIAL('Group', extraGroup);
     }
-  };
-
-  process.setegid = function setegid(id) {
-    return execId(id, 'Group', _setegid);
-  };
-
-  process.seteuid = function seteuid(id) {
-    return execId(id, 'User', _seteuid);
-  };
-
-  process.setgid = function setgid(id) {
-    return execId(id, 'Group', _setgid);
-  };
-
-  process.setuid = function setuid(id) {
-    return execId(id, 'User', _setuid);
-  };
+  }
 
-  process.setgroups = function setgroups(groups) {
+  function setgroups(groups) {
     if (!Array.isArray(groups)) {
       throw new ERR_INVALID_ARG_TYPE('groups', 'Array', groups);
     }
@@ -95,15 +80,17 @@ function setupPosixMethods(_initgroups, _setegid, _seteuid,
     if (result > 0) {
       throw new ERR_UNKNOWN_CREDENTIAL('Group', groups[result - 1]);
     }
-  };
+  }
 
-  function execId(id, type, method) {
-    validateId(id, 'id');
-    // Result is 0 on success, 1 if credential is unknown.
-    const result = method(id);
-    if (result === 1) {
-      throw new ERR_UNKNOWN_CREDENTIAL(type, id);
-    }
+  function wrapIdSetter(type, method) {
+    return function(id) {
+      validateId(id, 'id');
+      // Result is 0 on success, 1 if credential is unknown.
+      const result = method(id);
+      if (result === 1) {
+        throw new ERR_UNKNOWN_CREDENTIAL(type, id);
+      }
+    };
   }
 
   function validateId(id, name) {
@@ -113,6 +100,15 @@ function setupPosixMethods(_initgroups, _setegid, _seteuid,
       throw new ERR_INVALID_ARG_TYPE(name, ['number', 'string'], id);
     }
   }
+
+  return {
+    initgroups,
+    setgroups,
+    setegid: wrapIdSetter('Group', _setegid),
+    seteuid: wrapIdSetter('User', _seteuid),
+    setgid: wrapIdSetter('Group', _setgid),
+    setuid: wrapIdSetter('User', _setuid)
+  };
 }
 
 // Worker threads don't receive signals.
@@ -181,5 +177,6 @@ module.exports = {
   setupStdio,
   setupProcessMethods,
   setupSignalHandlers,
-  setupChildProcessIpcChannel
+  setupChildProcessIpcChannel,
+  wrapPosixCredentialSetters
 };
diff --git a/lib/os.js b/lib/os.js
@@ -21,7 +21,7 @@
 
 'use strict';
 
-const { safeGetenv } = internalBinding('util');
+const { safeGetenv } = internalBinding('credentials');
 const constants = internalBinding('constants').os;
 const { deprecate } = require('internal/util');
 const isWindows = process.platform === 'win32';

diff --git a/node.gyp b/node.gyp
@@ -351,6 +351,7 @@
         'src/node_config.cc',
         'src/node_constants.cc',
         'src/node_contextify.cc',
+        'src/node_credentials.cc',
         'src/node_domain.cc',
         'src/node_encoding.cc',
         'src/node_env_var.cc',

diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
@@ -147,17 +147,6 @@ void SetupBootstrapObject(Environment* env,
   BOOTSTRAP_METHOD(_rawDebug, RawDebug);
   BOOTSTRAP_METHOD(_umask, Umask);
 
-#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
-  if (env->is_main_thread()) {
-    BOOTSTRAP_METHOD(_initgroups, InitGroups);
-    BOOTSTRAP_METHOD(_setegid, SetEGid);
-    BOOTSTRAP_METHOD(_seteuid, SetEUid);
-    BOOTSTRAP_METHOD(_setgid, SetGid);
-    BOOTSTRAP_METHOD(_setuid, SetUid);
-    BOOTSTRAP_METHOD(_setgroups, SetGroups);
-  }
-#endif  // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__)
-
   Local<String> should_abort_on_uncaught_toggle =
       FIXED_ONE_BYTE_STRING(env->isolate(), "_shouldAbortOnUncaughtToggle");
   CHECK(bootstrapper->Set(env->context(),

diff --git a/src/env.cc b/src/env.cc
@@ -227,7 +227,7 @@ Environment::Environment(IsolateData* isolate_data,
   should_abort_on_uncaught_toggle_[0] = 1;
 
   std::string debug_cats;
-  SafeGetenv("NODE_DEBUG_NATIVE", &debug_cats);
+  credentials::SafeGetenv("NODE_DEBUG_NATIVE", &debug_cats);
   set_debug_categories(debug_cats, true);
 
   isolate()->GetHeapProfiler()->AddBuildEmbedderGraphCallback(

diff --git a/src/node.cc b/src/node.cc
@@ -100,12 +100,7 @@ typedef int mode_t;
 #else
 #include <pthread.h>
 #include <sys/resource.h>  // getrlimit, setrlimit
-#include <unistd.h>  // setuid, getuid
-#endif
-
-#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
-#include <pwd.h>  // getpwnam()
-#include <grp.h>  // getgrnam()
+#include <unistd.h>        // STDIN_FILENO, STDERR_FILENO
 #endif
 
 namespace node {
@@ -153,8 +148,6 @@ unsigned int reverted = 0;
 
 bool v8_initialized = false;
 
-bool linux_at_secure = false;
-
 // process-relative uptime base, initialized at start-up
 double prog_start_time;
 
@@ -504,27 +497,6 @@ const char* signo_string(int signo) {
   }
 }
 
-// Look up environment variable unless running as setuid root.
-bool SafeGetenv(const char* key, std::string* text) {
-#if !defined(__CloudABI__) && !defined(_WIN32)
-  if (linux_at_secure || getuid() != geteuid() || getgid() != getegid())
-    goto fail;
-#endif
-
-  {
-    Mutex::ScopedLock lock(environ_mutex);
-    if (const char* value = getenv(key)) {
-      *text = value;
-      return true;
-    }
-  }
-
-fail:
-  text->clear();
-  return false;
-}
-
-
 void* ArrayBufferAllocator::Allocate(size_t size) {
   if (zero_fill_field_ || per_process_opts->zero_fill_all_buffers)
     return UncheckedCalloc(size);
@@ -1165,14 +1137,6 @@ void SetupProcessObject(Environment* env,
   env->SetMethod(process, "dlopen", binding::DLOpen);
   env->SetMethod(process, "reallyExit", Exit);
   env->SetMethodNoSideEffect(process, "uptime", Uptime);
-
-#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
-  env->SetMethodNoSideEffect(process, "getuid", GetUid);
-  env->SetMethodNoSideEffect(process, "geteuid", GetEUid);
-  env->SetMethodNoSideEffect(process, "getgid", GetGid);
-  env->SetMethodNoSideEffect(process, "getegid", GetEGid);
-  env->SetMethodNoSideEffect(process, "getgroups", GetGroups);
-#endif  // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__)
 }
 
 
@@ -1633,37 +1597,40 @@ void Init(std::vector<std::string>* argv,
   {
     std::string text;
     default_env_options->pending_deprecation =
-        SafeGetenv("NODE_PENDING_DEPRECATION", &text) && text[0] == '1';
+        credentials::SafeGetenv("NODE_PENDING_DEPRECATION", &text) &&
+        text[0] == '1';
   }
 
   // Allow for environment set preserving symlinks.
   {
     std::string text;
     default_env_options->preserve_symlinks =
-        SafeGetenv("NODE_PRESERVE_SYMLINKS", &text) && text[0] == '1';
+        credentials::SafeGetenv("NODE_PRESERVE_SYMLINKS", &text) &&
+        text[0] == '1';
   }
 
   {
     std::string text;
     default_env_options->preserve_symlinks_main =
-        SafeGetenv("NODE_PRESERVE_SYMLINKS_MAIN", &text) && text[0] == '1';
+        credentials::SafeGetenv("NODE_PRESERVE_SYMLINKS_MAIN", &text) &&
+        text[0] == '1';
   }
 
   if (default_env_options->redirect_warnings.empty()) {
-    SafeGetenv("NODE_REDIRECT_WARNINGS",
-               &default_env_options->redirect_warnings);
+    credentials::SafeGetenv("NODE_REDIRECT_WARNINGS",
+                            &default_env_options->redirect_warnings);
   }
 
 #if HAVE_OPENSSL
   std::string* openssl_config = &per_process_opts->openssl_config;
   if (openssl_config->empty()) {
-    SafeGetenv("OPENSSL_CONF", openssl_config);
+    credentials::SafeGetenv("OPENSSL_CONF", openssl_config);
   }
 #endif
 
 #if !defined(NODE_WITHOUT_NODE_OPTIONS)
   std::string node_options;
-  if (SafeGetenv("NODE_OPTIONS", &node_options)) {
+  if (credentials::SafeGetenv("NODE_OPTIONS", &node_options)) {
     std::vector<std::string> env_argv;
     // [0] is expected to be the program name, fill it in from the real argv.
     env_argv.push_back(argv->at(0));
@@ -1695,7 +1662,7 @@ void Init(std::vector<std::string>* argv,
 #if defined(NODE_HAVE_I18N_SUPPORT)
   // If the parameter isn't given, use the env variable.
   if (per_process_opts->icu_data_dir.empty())
-    SafeGetenv("NODE_ICU_DATA", &per_process_opts->icu_data_dir);
+    credentials::SafeGetenv("NODE_ICU_DATA", &per_process_opts->icu_data_dir);
   // Initialize ICU.
   // If icu_data_dir is empty here, it will load the 'minimal' data.
   if (!i18n::InitializeICUDirectory(per_process_opts->icu_data_dir)) {
@@ -2103,7 +2070,7 @@ int Start(int argc, char** argv) {
 #if HAVE_OPENSSL
   {
     std::string extra_ca_certs;
-    if (SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
+    if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
       crypto::UseExtraCaCerts(extra_ca_certs);
   }
 #ifdef NODE_FIPS_MODE

diff --git a/src/node_binding.cc b/src/node_binding.cc
@@ -26,6 +26,7 @@
   V(cares_wrap)                                                                \
   V(config)                                                                    \
   V(contextify)                                                                \
+  V(credentials)                                                               \
   V(domain)                                                                    \
   V(fs)                                                                        \
   V(fs_event_wrap)                                                             \