Skip to content

Commit

Permalink
process: move POSIX credential accessors into node_credentials.cc
Browse files Browse the repository at this point in the history
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: #24961

PR-URL: #25066
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and addaleax committed Jan 14, 2019
1 parent 37ba201 commit e93dd4d
Show file tree
Hide file tree
Showing 17 changed files with 498 additions and 474 deletions.
26 changes: 20 additions & 6 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const {
internalModuleReadJSON,
internalModuleStat
} = internalBinding('fs');
const { safeGetenv } = internalBinding('util');
const { safeGetenv } = internalBinding('credentials');
const {
makeRequireFunction,
requireDepth,
Expand Down
73 changes: 35 additions & 38 deletions lib/internal/process/main_thread_only.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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.
Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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.
Expand Down Expand Up @@ -181,5 +177,6 @@ module.exports = {
setupStdio,
setupProcessMethods,
setupSignalHandlers,
setupChildProcessIpcChannel
setupChildProcessIpcChannel,
wrapPosixCredentialSetters
};
2 changes: 1 addition & 1 deletion lib/os.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,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',
Expand Down
11 changes: 0 additions & 11 deletions src/bootstrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
59 changes: 13 additions & 46 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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__)
}


Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/node_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
V(cares_wrap) \
V(config) \
V(contextify) \
V(credentials) \
V(domain) \
V(fs) \
V(fs_event_wrap) \
Expand Down
Loading

0 comments on commit e93dd4d

Please sign in to comment.