LC_NUMERIC set twice, and the wrong value used

[cederlys]

Summary

Icinga 2 tries to enforce LC_NUMERIC=C in ProcessSpawnImpl() in lib/base/process.cpp, but it fails if LC_NUMERIC is already present in the environment. LC_NUMERIC=C is appended to the environment, without checking if it already exists. You might end up with two LC_NUMERIC variables. In my testing, getenv(3) on Ubuntu 18.04 returns the first value it finds, so the override made by Icinga is ignored.

Expected Behavior

Check commands should always see exactly one LC_NUMERIC environment variable, and it should be set to C.

Current Behavior

If LC_NUMERIC already exists in the environment when Icinga is started, check commands will see two LC_NUMERIC variables, and will typically not use the value that Icinga put into the environment.

Possible Solution

Change the way the environment is built up, so that it never adds the same variable twice.

Note: this probably also affects environment variables set via extraEnvironment. They should override the inherited environment, but unless I misread the code the inherited environment will take precedence.

The easiest way to fix this would be to just reverse the order in which the environment is built up, so that LC_NUMERIC is set first, then the extraEnvironment, and finally the inherited environment. But the GNU C Library says:

The order in which strings appear in the environment is not significant, but the same name must not appear more than once.

So I think a more advanced implementation is needed. Perhaps put all the values into an std::map so that the final LC_NUMERIC setting would override any pre-existing variable of that name?

Steps to Reproduce (for bugs)

1. Ensure LC_NUMERIC=sv_SE.utf-8 is set in the environment before Icinga is started
2. Compile the adhoc.c program (see below) and save the result as /usr/local/bin/adhoc
3. Add the following check-command and service (please adjust the hostname from dua to suite your environment):

object CheckCommand "adhoc" {
  command = [ "/usr/local/bin/adhoc", ]
}

object Service "dua adhoc" {
  display_name = "adhoc"
  host_name = "dua"
  check_command = "adhoc"
}

3. Wait until icinga has run the command.
4. Inspect the resuling /tmp/adhoc.out file. My file looks like this (I've removed a few irrelevant environment variables):

   LC_TIME=sv_SE.UTF-8\0
   LC_MONETARY=sv_SE.UTF-8\0
   NOTIFY_SOCKET=/run/systemd/notify\0
   PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\0
   LANG=en_US.UTF-8\0
   PWD=/\0
   LC_NUMERIC=sv_SE.UTF-8\0
   LC_PAPER=sv_SE.UTF-8\0
   ICINGA2_ERROR_LOG=/var/log/icinga2/error.log\0
   LC_NUMERIC=C\0
   And now the getenv result: sv_SE.UTF-8

As you can see, LC_NUMERIC is included twice, and getenv("LC_NUMERIC") picks the wrong one.

Context

Background: I'm using the check_ntp_time plugin via the built-in ntp_time check-command. In my environment, LC_NUMERIC is set to sv_SE.utf-8. If I run the check_ntp_time plugin with that value, it predictably fails, since it parses the "-w 0.5" argument as 0 seconds, as you can see here:

# Fails: 0.5 parsed as 0.
$ LC_NUMERIC=sv_SE.utf-8 /usr/lib/nagios/plugins/check_ntp_time -H ntp.opera.com -w 0.5 -c 1.0
NTP WARNING: Offset 0,001195788383 secs|offset=0,001196s;0,000000;1,000000;
# Works: sv_SE.utf-8 expects decimal comma:
$ LC_NUMERIC=sv_SE.utf-8 /usr/lib/nagios/plugins/check_ntp_time -H ntp.opera.com -w 0,5 -c 1,0
NTP OK: Offset -0,0004163980484 secs|offset=-0,000416s;0,500000;1,000000;
# Works: The C locale properly parses 0.5.
$ LC_NUMERIC=C /usr/lib/nagios/plugins/check_ntp_time -H ntp.opera.com -w 0.5 -c 1.0
NTP OK: Offset 0.001442342997 secs|offset=0.001442s;0.500000;1.000000;

Because of this issue, my ntp_time check is constantly in a warning state.

Workaround: I could change the warning threshold to 1 and the critical threshold to 2, but I want to be notified long before the clock is that much out of sync.

Your Environment

• Version used (icinga2 --version):

  
  icinga2 - The Icinga 2 network monitoring daemon (version: r2.11.0-1)

Copyright (c) 2012-2019 Icinga GmbH (https://icinga.com/) License GPLv2+: GNU GPL version 2 or later http://gnu.org/licenses/gpl2.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law.

System information: Platform: Ubuntu Platform version: 18.04.3 LTS (Bionic Beaver) Kernel: Linux Kernel version: 4.15.0-64-generic Architecture: x86_64

Build information: Compiler: GNU 8.3.0 Build host: runner-LTrJQZ9N-project-298-concurrent-0

Application information:

General paths: Config directory: /etc/icinga2 Data directory: /var/lib/icinga2 Log directory: /var/log/icinga2 Cache directory: /var/cache/icinga2 Spool directory: /var/spool/icinga2 Run directory: /run/icinga2

Old paths (deprecated): Installation root: /usr Sysconf directory: /etc Run directory (base): /run Local state directory: /var

Internal paths: Package data directory: /usr/share/icinga2 State path: /var/lib/icinga2/icinga2.state Modified attributes path: /var/lib/icinga2/modified-attributes.conf Objects path: /var/cache/icinga2/icinga2.debug Vars path: /var/cache/icinga2/icinga2.vars PID path: /run/icinga2/icinga2.pid

* Operating System and version: Ubuntu 18.04
* Enabled features (`icinga2 feature list`): api checker mainlog notification
* Icinga Web 2 version and modules (System - About): not relevant
* Config validation (`icinga2 daemon -C`): not relevant
* If you run multiple Icinga 2 instances, the `zones.conf` file (or `icinga2 object list --type Endpoint` and `icinga2 object list --type Zone`) from all affected nodes. not relevant

## Test program

This is the adhoc.c program. It prints the environment to stdout, and also to the `/tmp/adhoc.out` file.  The file also contains the result of `getenv("LC_NUMERIC")`.

define _GNU_SOURCE

include

include

include

int main() { FILE *fp = fopen("/tmp/adhoc.out", "w"); char *e = environ; while (e) { char c = e; while (c) { if (c > 32 && c < 127 && c != '\') { fputc(c, fp); putchar(c); } else { fprintf(fp, "\x%02x", c); printf("\x%02x", c); } c++; } printf("\0\n"); fprintf(fp, "\0\n"); e++; } fprintf(fp, "And now the getenv result: %s\n", getenv("LC_NUMERIC")); fclose(fp); }

[dnsmichi]

Note: setenv() has an override parameter, but this function is not necessarily thread-safe. The best implementation would be to check **environmen whether the extra argument exists and replace that inside envp.

[htriem]

@Al2Klimov