MEDIUM: init: prevent process and thread creation at runtime
Some concerns are regularly raised about the risk to inherit some Lua
files which make use of a fork (e.g. via os.execute()) as well as
whether or not some of bugs we fix might or not be exploitable to run
some code. Given that haproxy is event-driven, any foreground activity
completely stops processing and is easy to detect, but background
activity is a different story. A Lua script could very well discretely
fork a sub-process connecting to a remote location and taking commands,
and some injected code could also try to hide its activity by creating
a process or a thread without blocking the rest of the processing. While
such activities should be extremely limited when run in an empty chroot
without any permission, it would be better to get a higher assurance
they cannot happen.
This patch introduces something very simple: it limits the number of
processes and threads to zero in the workers after the last thread was
created. By doing so, it effectively instructs the system to fail on
any fork() or clone() syscall. Thus any undesired activity has to happen
in the foreground and is way easier to detect.
This will obviously break external checks (whose concept is already
totally insecure), and for this reason a new option
"insecure-fork-wanted" was added to disable this protection, and it
is suggested in the fork() error report from the checks. It is
obviously recommended not to use it and to reconsider the reasons
leading to it being enabled in the first place.
If for any reason we fail to disable forks, we still start because it
could be imaginable that some operating systems refuse to set this
limit to zero, but in this case we emit a warning, that may or may not
be reported since we're after the fork point. Ideally over the long
term it should be conditionned by strict-limits and cause a hard fail.
diff --git a/src/cfgparse-global.c b/src/cfgparse-global.c
index dd37559..c083a05 100644
--- a/src/cfgparse-global.c
+++ b/src/cfgparse-global.c
@@ -94,6 +94,14 @@
else
global.tune.options |= GTUNE_SET_DUMPABLE;
}
+ else if (!strcmp(args[0], "insecure-fork-wanted")) { /* "no insecure-fork-wanted" or "insecure-fork-wanted" */
+ if (alertif_too_many_args(0, file, linenum, args, &err_code))
+ goto out;
+ if (kwm == KWM_NO)
+ global.tune.options &= ~GTUNE_INSECURE_FORK;
+ else
+ global.tune.options |= GTUNE_INSECURE_FORK;
+ }
else if (!strcmp(args[0], "nosplice")) {
if (alertif_too_many_args(0, file, linenum, args, &err_code))
goto out;
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 2e200e8..fdc19f4 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -2160,10 +2160,11 @@
if (kwm != KWM_STD && strcmp(args[0], "option") != 0 &&
strcmp(args[0], "log") != 0 && strcmp(args[0], "busy-polling") != 0 &&
- strcmp(args[0], "set-dumpable") != 0 && strcmp(args[0], "strict-limits") != 0) {
+ strcmp(args[0], "set-dumpable") != 0 && strcmp(args[0], "strict-limits") != 0 &&
+ strcmp(args[0], "insecure-fork-wanted") != 0) {
ha_alert("parsing [%s:%d]: negation/default currently "
"supported only for options, log, busy-polling, "
- "set-dumpable and strict-limits.\n", file, linenum);
+ "set-dumpable, strict-limits, and insecure-fork-wanted.\n", file, linenum);
err_code |= ERR_ALERT | ERR_FATAL;
}
@@ -2549,6 +2550,11 @@
curproxy->id, "option external-check");
cfgerr++;
}
+ if (!(global.tune.options & GTUNE_INSECURE_FORK)) {
+ ha_warning("Proxy '%s' : 'insecure-fork-wanted' not enabled in the global section, '%s' will likely fail.\n",
+ curproxy->id, "option external-check");
+ err_code |= ERR_WARN;
+ }
}
if (curproxy->email_alert.set) {
diff --git a/src/checks.c b/src/checks.c
index 247caf1..909bd52 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -2012,7 +2012,9 @@
pid = fork();
if (pid < 0) {
- ha_alert("Failed to fork process for external health check: %s. Aborting.\n",
+ ha_alert("Failed to fork process for external health check%s: %s. Aborting.\n",
+ (global.tune.options & GTUNE_INSECURE_FORK) ?
+ "" : " (likely caused by missing 'insecure-fork-wanted')",
strerror(errno));
set_server_check_status(check, HCHK_STATUS_SOCKERR, strerror(errno));
goto out;
diff --git a/src/haproxy.c b/src/haproxy.c
index b03d0ad..7ba3ae1 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2749,6 +2749,23 @@
pthread_mutex_unlock(&init_mutex);
#endif
+#if defined(RLIMIT_NPROC)
+ /* all threads have started, it's now time to prevent any new thread
+ * or process from starting. Obviously we do this in workers only. We
+ * can't hard-fail on this one as it really is implementation dependent
+ * though we're interested in feedback, hence the warning.
+ */
+ if (!(global.tune.options & GTUNE_INSECURE_FORK) && !master) {
+ struct rlimit limit = { .rlim_cur = 0, .rlim_max = 0 };
+ static int warn_fail;
+
+ if (setrlimit(RLIMIT_NPROC, &limit) == -1 && !_HA_ATOMIC_XADD(&warn_fail, 1)) {
+ ha_warning("Failed to disable forks, please report to developers with detailed "
+ "information about your operating system. You can silence this warning "
+ "by adding 'insecure-fork-wanted' in the 'global' section.\n");
+ }
+ }
+#endif
run_poll_loop();
list_for_each_entry(ptdf, &per_thread_deinit_list, list)