Skip to content

Commit c9ed842

Browse files
committed
src: add SafeCapGetenv function
This commit suggests adding a function named SafeCapGetenv which takes into consideration if a capability has been set on the executable, and if so allows the reading of the environment variables. This is a separate function so that it does not affect and JavaScript methods that currently use SafeGetenv. The motivation for this change is a use-case where Node is run in a container, and the is a requirement to be able to listen to ports below 1024. This is done by setting the capability of cap_net_bin_service. In addition there is a need to set the environment variable `NODE_EXTRA_CA_CERTS`. But currently this environment variable will not be read when the capability has been set on the executable.
1 parent bfa6e37 commit c9ed842

3 files changed

Lines changed: 40 additions & 1 deletion

File tree

src/node.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ InitializationResult InitializeOncePerProcess(int argc, char** argv) {
10101010
#if HAVE_OPENSSL
10111011
{
10121012
std::string extra_ca_certs;
1013-
if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
1013+
if (credentials::SafeCapGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
10141014
crypto::UseExtraCaCerts(extra_ca_certs);
10151015
}
10161016
// In the case of FIPS builds we should make sure

src/node_credentials.cc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,44 @@ bool linux_at_secure = false;
3333

3434
namespace credentials {
3535

36+
// Look up environment variable and allow if linux capabilities have been set
37+
// but not setuid. setuid is assumed if the real/effective user/group differ.
38+
// We also check for the off chance that the process is running as root and
39+
// also has capabilities set (in which case the user/group ids would be the
40+
// same).
41+
bool SafeCapGetenv(const char* key, std::string* text) {
42+
#if !defined(__CloudABI__) && !defined(_WIN32)
43+
if (per_process::linux_at_secure && (getuid() != geteuid() ||
44+
getgid() != getegid()) || getuid() == 0) {
45+
goto fail;
46+
}
47+
#endif
48+
49+
{
50+
Mutex::ScopedLock lock(per_process::env_var_mutex);
51+
52+
size_t init_sz = 256;
53+
MaybeStackBuffer<char, 256> val;
54+
int ret = uv_os_getenv(key, *val, &init_sz);
55+
56+
if (ret == UV_ENOBUFS) {
57+
// Buffer is not large enough, reallocate to the updated init_sz
58+
// and fetch env value again.
59+
val.AllocateSufficientStorage(init_sz);
60+
ret = uv_os_getenv(key, *val, &init_sz);
61+
}
62+
63+
if (ret >= 0) { // Env key value fetch success.
64+
*text = *val;
65+
return true;
66+
}
67+
}
68+
69+
fail:
70+
text->clear();
71+
return false;
72+
}
73+
3674
// Look up environment variable unless running as setuid root.
3775
bool SafeGetenv(const char* key, std::string* text, Environment* env) {
3876
#if !defined(__CloudABI__) && !defined(_WIN32)

src/node_internals.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ class ThreadPoolWork {
292292

293293
namespace credentials {
294294
bool SafeGetenv(const char* key, std::string* text, Environment* env = nullptr);
295+
bool SafeCapGetenv(const char* key, std::string* text);
295296
} // namespace credentials
296297

297298
void DefineZlibConstants(v8::Local<v8::Object> target);

0 commit comments

Comments
 (0)