Skip to content

Commit 321e296

Browse files
joyeecheungjoyeecheung
committed
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 <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 74a1dfb commit 321e296

17 files changed

Lines changed: 498 additions & 474 deletions

‎lib/internal/bootstrap/node.js‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ const {
2323
_setupPromises, _chdir, _cpuUsage,
2424
_hrtime, _hrtimeBigInt,
2525
_memoryUsage, _rawDebug,
26-
_umask, _initgroups, _setegid, _seteuid,
27-
_setgid, _setuid, _setgroups,
26+
_umask,
2827
_shouldAbortOnUncaughtToggle
2928
} = bootstrappers;
3029
const { internalBinding, NativeModule } = loaderExports;
@@ -72,13 +71,28 @@ function startup() {
7271
NativeModule.require('internal/process/warning').setup();
7372
NativeModule.require('internal/process/next_tick').setup(_setupNextTick,
7473
_setupPromises);
74+
const credentials = internalBinding('credentials');
75+
if (credentials.implementsPosixCredentials) {
76+
process.getuid = credentials.getuid;
77+
process.geteuid = credentials.geteuid;
78+
process.getgid = credentials.getgid;
79+
process.getegid = credentials.getegid;
80+
process.getgroups = credentials.getgroups;
81+
82+
if (isMainThread) {
83+
const wrapped = mainThreadSetup.wrapPosixCredentialSetters(credentials);
84+
process.initgroups = wrapped.initgroups;
85+
process.setgroups = wrapped.setgroups;
86+
process.setegid = wrapped.setegid;
87+
process.seteuid = wrapped.seteuid;
88+
process.setgid = wrapped.setgid;
89+
process.setuid = wrapped.setuid;
90+
}
91+
}
7592

7693
if (isMainThread) {
7794
mainThreadSetup.setupStdio();
78-
mainThreadSetup.setupProcessMethods(
79-
_chdir, _umask, _initgroups, _setegid, _seteuid,
80-
_setgid, _setuid, _setgroups
81-
);
95+
mainThreadSetup.setupProcessMethods(_chdir, _umask);
8296
} else {
8397
workerThreadSetup.setupStdio();
8498
}

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const {
3434
internalModuleReadJSON,
3535
internalModuleStat
3636
} = internalBinding('fs');
37-
const { safeGetenv } = internalBinding('util');
37+
const { safeGetenv } = internalBinding('credentials');
3838
const {
3939
makeRequireFunction,
4040
requireDepth,

‎lib/internal/process/main_thread_only.js‎

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,7 @@ function setupStdio() {
2929

3030
// Non-POSIX platforms like Windows don't have certain methods.
3131
// Workers also lack these methods since they change process-global state.
32-
function setupProcessMethods(_chdir, _umask, _initgroups, _setegid,
33-
_seteuid, _setgid, _setuid, _setgroups) {
34-
if (_setgid !== undefined) {
35-
setupPosixMethods(_initgroups, _setegid, _seteuid,
36-
_setgid, _setuid, _setgroups);
37-
}
38-
32+
function setupProcessMethods(_chdir, _umask) {
3933
process.chdir = function chdir(directory) {
4034
validateString(directory, 'directory');
4135
return _chdir(directory);
@@ -51,10 +45,17 @@ function setupProcessMethods(_chdir, _umask, _initgroups, _setegid,
5145
};
5246
}
5347

54-
function setupPosixMethods(_initgroups, _setegid, _seteuid,
55-
_setgid, _setuid, _setgroups) {
56-
57-
process.initgroups = function initgroups(user, extraGroup) {
48+
function wrapPosixCredentialSetters(credentials) {
49+
const {
50+
initgroups: _initgroups,
51+
setgroups: _setgroups,
52+
setegid: _setegid,
53+
seteuid: _seteuid,
54+
setgid: _setgid,
55+
setuid: _setuid
56+
} = credentials;
57+
58+
function initgroups(user, extraGroup) {
5859
validateId(user, 'user');
5960
validateId(extraGroup, 'extraGroup');
6061
// Result is 0 on success, 1 if user is unknown, 2 if group is unknown.
@@ -64,25 +65,9 @@ function setupPosixMethods(_initgroups, _setegid, _seteuid,
6465
} else if (result === 2) {
6566
throw new ERR_UNKNOWN_CREDENTIAL('Group', extraGroup);
6667
}
67-
};
68-
69-
process.setegid = function setegid(id) {
70-
return execId(id, 'Group', _setegid);
71-
};
72-
73-
process.seteuid = function seteuid(id) {
74-
return execId(id, 'User', _seteuid);
75-
};
76-
77-
process.setgid = function setgid(id) {
78-
return execId(id, 'Group', _setgid);
79-
};
80-
81-
process.setuid = function setuid(id) {
82-
return execId(id, 'User', _setuid);
83-
};
68+
}
8469

85-
process.setgroups = function setgroups(groups) {
70+
function setgroups(groups) {
8671
if (!Array.isArray(groups)) {
8772
throw new ERR_INVALID_ARG_TYPE('groups', 'Array', groups);
8873
}
@@ -95,15 +80,17 @@ function setupPosixMethods(_initgroups, _setegid, _seteuid,
9580
if (result > 0) {
9681
throw new ERR_UNKNOWN_CREDENTIAL('Group', groups[result - 1]);
9782
}
98-
};
83+
}
9984

100-
function execId(id, type, method) {
101-
validateId(id, 'id');
102-
// Result is 0 on success, 1 if credential is unknown.
103-
const result = method(id);
104-
if (result === 1) {
105-
throw new ERR_UNKNOWN_CREDENTIAL(type, id);
106-
}
85+
function wrapIdSetter(type, method) {
86+
return function(id) {
87+
validateId(id, 'id');
88+
// Result is 0 on success, 1 if credential is unknown.
89+
const result = method(id);
90+
if (result === 1) {
91+
throw new ERR_UNKNOWN_CREDENTIAL(type, id);
92+
}
93+
};
10794
}
10895

10996
function validateId(id, name) {
@@ -113,6 +100,15 @@ function setupPosixMethods(_initgroups, _setegid, _seteuid,
113100
throw new ERR_INVALID_ARG_TYPE(name, ['number', 'string'], id);
114101
}
115102
}
103+
104+
return {
105+
initgroups,
106+
setgroups,
107+
setegid: wrapIdSetter('Group', _setegid),
108+
seteuid: wrapIdSetter('User', _seteuid),
109+
setgid: wrapIdSetter('Group', _setgid),
110+
setuid: wrapIdSetter('User', _setuid)
111+
};
116112
}
117113

118114
// Worker threads don't receive signals.
@@ -181,5 +177,6 @@ module.exports = {
181177
setupStdio,
182178
setupProcessMethods,
183179
setupSignalHandlers,
184-
setupChildProcessIpcChannel
180+
setupChildProcessIpcChannel,
181+
wrapPosixCredentialSetters
185182
};

‎lib/os.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
'use strict';
2323

24-
const { safeGetenv } = internalBinding('util');
24+
const { safeGetenv } = internalBinding('credentials');
2525
const constants = internalBinding('constants').os;
2626
const { deprecate } = require('internal/util');
2727
const isWindows = process.platform === 'win32';

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@
350350
'src/node_config.cc',
351351
'src/node_constants.cc',
352352
'src/node_contextify.cc',
353+
'src/node_credentials.cc',
353354
'src/node_domain.cc',
354355
'src/node_encoding.cc',
355356
'src/node_env_var.cc',

‎src/bootstrapper.cc‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,17 +147,6 @@ void SetupBootstrapObject(Environment* env,
147147
BOOTSTRAP_METHOD(_rawDebug, RawDebug);
148148
BOOTSTRAP_METHOD(_umask, Umask);
149149

150-
#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
151-
if (env->is_main_thread()) {
152-
BOOTSTRAP_METHOD(_initgroups, InitGroups);
153-
BOOTSTRAP_METHOD(_setegid, SetEGid);
154-
BOOTSTRAP_METHOD(_seteuid, SetEUid);
155-
BOOTSTRAP_METHOD(_setgid, SetGid);
156-
BOOTSTRAP_METHOD(_setuid, SetUid);
157-
BOOTSTRAP_METHOD(_setgroups, SetGroups);
158-
}
159-
#endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__)
160-
161150
Local<String> should_abort_on_uncaught_toggle =
162151
FIXED_ONE_BYTE_STRING(env->isolate(), "_shouldAbortOnUncaughtToggle");
163152
CHECK(bootstrapper->Set(env->context(),

‎src/env.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ Environment::Environment(IsolateData* isolate_data,
225225
should_abort_on_uncaught_toggle_[0] = 1;
226226

227227
std::string debug_cats;
228-
SafeGetenv("NODE_DEBUG_NATIVE", &debug_cats);
228+
credentials::SafeGetenv("NODE_DEBUG_NATIVE", &debug_cats);
229229
set_debug_categories(debug_cats, true);
230230

231231
isolate()->GetHeapProfiler()->AddBuildEmbedderGraphCallback(

‎src/node.cc‎

Lines changed: 13 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,7 @@ typedef int mode_t;
100100
#else
101101
#include <pthread.h>
102102
#include <sys/resource.h> // getrlimit, setrlimit
103-
#include <unistd.h> // setuid, getuid
104-
#endif
105-
106-
#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
107-
#include <pwd.h> // getpwnam()
108-
#include <grp.h> // getgrnam()
103+
#include <unistd.h> // STDIN_FILENO, STDERR_FILENO
109104
#endif
110105

111106
namespace node {
@@ -153,8 +148,6 @@ unsigned int reverted = 0;
153148

154149
bool v8_initialized = false;
155150

156-
bool linux_at_secure = false;
157-
158151
// process-relative uptime base, initialized at start-up
159152
double prog_start_time;
160153

@@ -501,27 +494,6 @@ const char* signo_string(int signo) {
501494
}
502495
}
503496

504-
// Look up environment variable unless running as setuid root.
505-
bool SafeGetenv(const char* key, std::string* text) {
506-
#if !defined(__CloudABI__) && !defined(_WIN32)
507-
if (linux_at_secure || getuid() != geteuid() || getgid() != getegid())
508-
goto fail;
509-
#endif
510-
511-
{
512-
Mutex::ScopedLock lock(environ_mutex);
513-
if (const char* value = getenv(key)) {
514-
*text = value;
515-
return true;
516-
}
517-
}
518-
519-
fail:
520-
text->clear();
521-
return false;
522-
}
523-
524-
525497
void* ArrayBufferAllocator::Allocate(size_t size) {
526498
if (zero_fill_field_ || per_process_opts->zero_fill_all_buffers)
527499
return UncheckedCalloc(size);
@@ -1157,14 +1129,6 @@ void SetupProcessObject(Environment* env,
11571129
env->SetMethod(process, "dlopen", binding::DLOpen);
11581130
env->SetMethod(process, "reallyExit", Exit);
11591131
env->SetMethodNoSideEffect(process, "uptime", Uptime);
1160-
1161-
#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
1162-
env->SetMethodNoSideEffect(process, "getuid", GetUid);
1163-
env->SetMethodNoSideEffect(process, "geteuid", GetEUid);
1164-
env->SetMethodNoSideEffect(process, "getgid", GetGid);
1165-
env->SetMethodNoSideEffect(process, "getegid", GetEGid);
1166-
env->SetMethodNoSideEffect(process, "getgroups", GetGroups);
1167-
#endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__)
11681132
}
11691133

11701134

@@ -1625,37 +1589,40 @@ void Init(std::vector<std::string>* argv,
16251589
{
16261590
std::string text;
16271591
default_env_options->pending_deprecation =
1628-
SafeGetenv("NODE_PENDING_DEPRECATION", &text) && text[0] == '1';
1592+
credentials::SafeGetenv("NODE_PENDING_DEPRECATION", &text) &&
1593+
text[0] == '1';
16291594
}
16301595

16311596
// Allow for environment set preserving symlinks.
16321597
{
16331598
std::string text;
16341599
default_env_options->preserve_symlinks =
1635-
SafeGetenv("NODE_PRESERVE_SYMLINKS", &text) && text[0] == '1';
1600+
credentials::SafeGetenv("NODE_PRESERVE_SYMLINKS", &text) &&
1601+
text[0] == '1';
16361602
}
16371603

16381604
{
16391605
std::string text;
16401606
default_env_options->preserve_symlinks_main =
1641-
SafeGetenv("NODE_PRESERVE_SYMLINKS_MAIN", &text) && text[0] == '1';
1607+
credentials::SafeGetenv("NODE_PRESERVE_SYMLINKS_MAIN", &text) &&
1608+
text[0] == '1';
16421609
}
16431610

16441611
if (default_env_options->redirect_warnings.empty()) {
1645-
SafeGetenv("NODE_REDIRECT_WARNINGS",
1646-
&default_env_options->redirect_warnings);
1612+
credentials::SafeGetenv("NODE_REDIRECT_WARNINGS",
1613+
&default_env_options->redirect_warnings);
16471614
}
16481615

16491616
#if HAVE_OPENSSL
16501617
std::string* openssl_config = &per_process_opts->openssl_config;
16511618
if (openssl_config->empty()) {
1652-
SafeGetenv("OPENSSL_CONF", openssl_config);
1619+
credentials::SafeGetenv("OPENSSL_CONF", openssl_config);
16531620
}
16541621
#endif
16551622

16561623
#if !defined(NODE_WITHOUT_NODE_OPTIONS)
16571624
std::string node_options;
1658-
if (SafeGetenv("NODE_OPTIONS", &node_options)) {
1625+
if (credentials::SafeGetenv("NODE_OPTIONS", &node_options)) {
16591626
std::vector<std::string> env_argv;
16601627
// [0] is expected to be the program name, fill it in from the real argv.
16611628
env_argv.push_back(argv->at(0));
@@ -1687,7 +1654,7 @@ void Init(std::vector<std::string>* argv,
16871654
#if defined(NODE_HAVE_I18N_SUPPORT)
16881655
// If the parameter isn't given, use the env variable.
16891656
if (per_process_opts->icu_data_dir.empty())
1690-
SafeGetenv("NODE_ICU_DATA", &per_process_opts->icu_data_dir);
1657+
credentials::SafeGetenv("NODE_ICU_DATA", &per_process_opts->icu_data_dir);
16911658
// Initialize ICU.
16921659
// If icu_data_dir is empty here, it will load the 'minimal' data.
16931660
if (!i18n::InitializeICUDirectory(per_process_opts->icu_data_dir)) {
@@ -2095,7 +2062,7 @@ int Start(int argc, char** argv) {
20952062
#if HAVE_OPENSSL
20962063
{
20972064
std::string extra_ca_certs;
2098-
if (SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
2065+
if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
20992066
crypto::UseExtraCaCerts(extra_ca_certs);
21002067
}
21012068
#ifdef NODE_FIPS_MODE

‎src/node_binding.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
V(cares_wrap) \
3131
V(config) \
3232
V(contextify) \
33+
V(credentials) \
3334
V(domain) \
3435
V(fs) \
3536
V(fs_event_wrap) \

0 commit comments

Comments
 (0)