Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NODE_EXTRA_CA_CERTS is ignored after adding capability to node process to run on port below 1024 #22081

Closed
dooman87 opened this issue Aug 2, 2018 · 5 comments

Comments

@dooman87
Copy link

dooman87 commented Aug 2, 2018

  • Version: node:8.11-stretch
  • Platform: docker image (Linux fb58461d8443 4.9.93-linuxkit-aufs deps: update openssl to 1.0.1j #1 SMP Wed Jun 6 16:55:56 UTC 2018 x86_64 GNU/Linux)
  • Subsystem: TLS/HTTPS

We needed to add self signed root CA to nodejs, so we did it with using NODE_EXTRA_CA_CERTS environment variable. We are also running nodejs HTTP servers on port 80 and 443 and using following command to allow non-root user to do that:

setcap 'cap_net_bind_service=+ep' /usr/local/bin/node

With that command in place though NODE_EXTRA_CA_CERTS was being ignored. It started working once we removed capability from node executable. I guess that node is not reading file as well as we didn't see warning in case NODE_EXTRA_CA_CERTS pointed to non-existing file.

Reproducible configuration can be found in this repo - https://github.com/dooman87/nodejs-setcap-bug.

To run: docker-compose up

Once you run it you will see an error: ERROR self signed certificate
If you comment line 12 in Dockerfile and run it again then you will see successful response from nginx.

@dooman87 dooman87 changed the title NODE_EXTRA_CA_CERTS is ignore after adding capability to node process to run on port below 1024 NODE_EXTRA_CA_CERTS is ignored after adding capability to node process to run on port below 1024 Aug 2, 2018
@mscdex
Copy link
Contributor

mscdex commented Aug 2, 2018

This is a bit unrelated, but FWIW doing setcap 'cap_net_bind_service=+ep' /usr/local/bin/node is a bad idea as it affects all scripts executed under node. Some alternatives to setcap:

  • Have a firewall rule that rewrites the lower-numbered port to a higher-numbered port
  • Use authbind to limit the access to specific lower-numbered ports to specific users
  • Utilize sudoers and dropping privileges after binding in your script.

@dooman87
Copy link
Author

dooman87 commented Aug 2, 2018

Thanks @mscdex. We solved a problem by using nginx and route ports. It was very hard though to find where the problem is.

@bnoordhuis
Copy link
Member

Thanks for the bug report. The behavior you describe is an intentional security measure. When node.js runs with elevated privileges (setuid root or capabilities), it ignores the NODE_EXTRA_CA_CERTS environment variable.

targos pushed a commit that referenced this issue Oct 24, 2018
Fixes: #22081

PR-URL: #23770
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 26, 2018
Fixes: #22081

PR-URL: #23770
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
rvagg pushed a commit that referenced this issue Nov 28, 2018
Fixes: #22081

PR-URL: #23770
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 29, 2018
Fixes: #22081

PR-URL: #23770
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Toilal added a commit to inetum-orleans/generator-docker-devbox that referenced this issue Jun 19, 2019
@pavelhoral
Copy link
Contributor

The behavior you describe is an intentional security measure.

@bnoordhuis I tried to search for some additional information about this but without any success. Are there any resources people can read or maybe a reference to a related source code? Thank you.

@bnoordhuis
Copy link
Member

@pavelhoral See https://linux.die.net/man/3/secure_getenv - that's basically what Node.js does, except cross-platform:

// Look up environment variable unless running as setuid root.
bool SafeGetenv(const char* key, std::string* text, Environment* env) {
#if !defined(__CloudABI__) && !defined(_WIN32)
if (per_process::linux_at_secure || getuid() != geteuid() ||
getgid() != getegid())
goto fail;
#endif
if (env != nullptr) {
HandleScope handle_scope(env->isolate());
TryCatch ignore_errors(env->isolate());
MaybeLocal<String> value = env->env_vars()->Get(
env->isolate(),
String::NewFromUtf8(env->isolate(), key, NewStringType::kNormal)
.ToLocalChecked());
if (value.IsEmpty()) goto fail;
String::Utf8Value utf8_value(env->isolate(), value.ToLocalChecked());
if (*utf8_value == nullptr) goto fail;
*text = std::string(*utf8_value, utf8_value.length());
return true;
}
{
Mutex::ScopedLock lock(per_process::env_var_mutex);
if (const char* value = getenv(key)) {
*text = value;
return true;
}
}
fail:
text->clear();
return false;
}

node/src/node_main.cc

Lines 114 to 124 in 7bb4f95

#if defined(__linux__)
char** envp = environ;
while (*envp++ != nullptr) {}
Elf_auxv_t* auxv = reinterpret_cast<Elf_auxv_t*>(envp);
for (; auxv->a_type != AT_NULL; auxv++) {
if (auxv->a_type == AT_SECURE) {
node::per_process::linux_at_secure = auxv->a_un.a_val;
break;
}
}
#endif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants