[dev.icinga.com #2291] unknown macros are not replaced, and misleading to single dollar signs

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/2291

Created by mfriedrich on 2012-01-29 00:21:22 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2012-08-31 11:10:53 +00:00)
Target Version: 1.8
Last Update: 2012-08-31 11:10:53 +00:00 (in Redmine)

Icinga Version: 1.7.1
OS Version: Debian

---

if you are using macros which are not valid in the current situation, such as using $CONTACTEMAIL$ within a check command, or a service only macro while executing a host check/notification, this will put the macro as is on the shell.

this is then being interpreted as $CONTACTEMAIL (empty) and $ ... and then confusing the user that there was only a dollar sign as macro replacement.

-------- Original Message --------
Subject:    [Nagios-devel] Non existent macros are not replaced
Date:   Thu, 3 Mar 2011 17:39:48 +0100
From:   Matthieu Kermagoret 
Reply-To:   Nagios Developers List 
To:     Nagios Developers List 


Hi list,

When using a non-existent macros in a command (like for example
$SERVICEDESC$ in a host notification command), such macro is not
replaced. It is instead left as-is on the command line.

Such construct is interpreted by the shell as a variable (usually
empty) followed by a dollar sign which (usually) expands to a single
dollar and lead users to believe that Nagios replace non-existent
macros with a single dollar (see ticket #34
http://tracker.nagios.org/view.php?id=34).

I believe this behavior is not intuitive and should therefore be
modified to simply remove non-existent macros, as the attached patch
does.

What do you think about it ?

Best regards,

-- 
Matthieu KERMAGORET | Développeur

mkermagoret@merethis.com

MERETHIS est éditeur du logiciel Centreon.

http://tracker.nagios.org/view.php?id=34

Attachments

• nagios_invalid_macros_are_replaced_by_a_single_dollar.patch mfriedrich - 2012-01-29 00:21:22 +00:00

Changesets

2012-08-22 17:46:32 +00:00 by mfriedrich 02341c6

core: unknown macros are not replaced, and misleading to single dollar signs #2291 - MF

people report bugs, that check_bla -c $foo$ will lead into errors, like
the shell interpreting $foo as variable, but $ throws an error then.
others tell to put quotes around, especially having a password.

the correct way of putting dollar signs into command lines is to
properly escape them with another dollar sign (see the docs). all other
methods are not valid, and only circumvent the real problem.

other than that, the config could contain macros which are not valid for
the object executing the command. those macros were not replaced either
and therefore causing the commands to fail, leading to the infamous bug
reports too.

the best way to enforce this being fixed is logging a warning instead of
just leaving that on the shell. default will now remove the unknown
macro string from the macro buffer, leading to safety.

since this will possibly break all other existing setups, having their
own tricks and workarounds applied, we'll add the new icinga.cfg option
keep_unknown_macros, which can be re-enabled in order to revert to the
old, buggy behaviour.

it's up to each user which to chose, in favor of resolving a long
lasting bug, the default will change with 1.8.

refs #2291

2012-10-08 13:28:14 +00:00 by mfriedrich d5f9efd

tests: update #2291 with user macro test refs #2291

---

Relations:

• relates #2291
• duplicates #2291

[icinga-migration]

Updated by mfriedrich on 2012-01-29 00:39:04 +00:00

one major disadvantage removing the output - strings containing two dollar signs could be accidently interpreted as macro, i.e. passwords. so this must be evaluated if to be resolved differently, or just updating the documentation on possible errors.

another reason for removing it might be the case that all dollar signs need to be escaped by a second one, i.e. "$$foo$$bar" becomes "$foo$bar" afterwards.

-------- Original Message --------
Subject:    Re: [Nagios-devel] Non existent macros are not replaced
Date:   Thu, 03 Mar 2011 19:33:47 +0100
From:   Andreas Ericsson 
Reply-To:   Nagios Developers List 
To:     Nagios Developers List 
CC:     Jochen Bern 


On 03/03/2011 06:37 PM, Jochen Bern wrote:
> On 03/03/2011 05:54 PM, Andreas Ericsson wrote:
>> I think that would be quite stupid, as it would prevent users from
>> using shell variables in their commands, or multiple dollar-signs
>> for that matter.
> 
> Not quite, the handling of escaped dollar signs ("$$") is in a separate
> branch immediately preceding the "unknown macro name" branch. Hence, the
> properly escaped notation ("$$FOO$$BAR" in Nagios, becomes "$FOO$BAR" on
> shell level) would still be available.
> 

Yes, but how about a command_line such as this:

       $USER1$/check_something '$pas$word'

$pas$ is not a valid macro, so by the reasoning of the original poster
we should remove it, but that would mean there can *never* be two
dollar-signs that aren't part of macros inside anything where macros
get interpreted, and that would be a real limitation put in place to
prevent the minor discomfort of having to redo a botched job.

I just flat out refuse to accept patches with those effects.

> (I admit I wasn't aware that it's the *shell* which reduces "$FOO$" to
> "$", rather than Nagios.)
> 
> On one hand, trusting Nagios to leave "$FOO$" be (rather than properly
> configuring "$$FOO$$") is not supported.

It is. If the dollar-signs are within single-quotes the shell won't even
touch them and '$FOO$' gets passed verbatim to whatever command one's
trying to run.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.



-------- Original Message --------
Subject:    Re: [Nagios-devel] Non existent macros are not replaced
Date:   Thu, 03 Mar 2011 20:08:16 +0100
From:   Jochen Bern 
Reply-To:   Nagios Developers List 
To:     Nagios Developers List 


On 03/03/2011 07:33 PM, Andreas Ericsson wrote:
> On 03/03/2011 06:37 PM, Jochen Bern wrote:
>> Not quite, the handling of escaped dollar signs ("$$") is in a separate
>> branch immediately preceding the "unknown macro name" branch. Hence, the
>> properly escaped notation ("$$FOO$$BAR" in Nagios, becomes "$FOO$BAR" on
>> shell level) would still be available.
> 
> Yes, but how about a command_line such as this:
>        $USER1$/check_something '$pas$word'
> $pas$ is not a valid macro, so by the reasoning of the original poster
> we should remove it, but that would mean there can *never* be two
> dollar-signs that aren't part of macros inside anything where macros
> get interpreted

I'm not saying that you should follow his reasoning, but why you keep
ignoring the official escaping method?

"Also, if you want to pass a dollar sign ($) on the command line, you
have to escape it with another dollar sign."
-- http://nagios.sourceforge.net/docs/3_0/objectdefinitions.html#command

# grep -n 'pas.*word' ../var/objects.cache
612:    command_name    test_password
613:    command_line    /bin/echo '$$pas$$word'
35520:  check_command   test_password
# grep -n 'pas.*word' ../var/spool/status.dat
54898:  check_command=test_password
54924:  plugin_output=$pas$word

>> On one hand, trusting Nagios to leave "$FOO$" be (rather than properly
>> configuring "$$FOO$$") is not supported.
> 
> It is. If the dollar-signs are within single-quotes the shell won't even
> touch them and '$FOO$' gets passed verbatim to whatever command one's
> trying to run.

The above quote from the docs says it isn't ("you HAVE to"). But as I
said, *enforcing* that out of the blue sky is problematic, to say the least.

Kind regards,
                                J. Bern

[icinga-migration]

Updated by mfriedrich on 2012-04-03 07:14:32 +00:00

i'm still undecided if it makes sense to clean non-existant macros away, or maybe allow re-enabling with a config flag?

[icinga-migration]

Updated by mfriedrich on 2012-04-19 14:48:00 +00:00

• Target Version changed from 1.7 to 1.8

postpone it, and leave the discussion open.

[icinga-migration]

Updated by mfriedrich on 2012-07-06 17:45:11 +00:00

hm, that could be made a config option, allowing users to restore compatibility again (if they know what they do, as most the users having these errors, actually don't).

[icinga-migration]

Updated by idl0r on 2012-07-30 08:05:23 +00:00

A config option to let the user decide whether to strip away the non-existant macros would be perfect. +1 on that :P

[icinga-migration]

Updated by mfriedrich on 2012-08-09 15:02:38 +00:00

• Icinga Version set to 1
• OS Version set to Debian

dev decision:

• remove unused macros
• passwords containing $ signs need to be properly escaped
• log a warning instead, telling the user to fix it

[icinga-migration]

Updated by mfriedrich on 2012-08-22 17:37:09 +00:00

Aug 22 19:32:01 icinga-dev icinga: Warning: Skipping unknown macro '$UNKNOWNHOSTMACRO1$', removing it from output! Fix your config, or set keep_unknown_macros accordingly...


Aug 22 19:36:45 icinga-dev icinga: Warning: Skipping unknown macro '$UNKNOWSERVICENMACRO1$', removing it from output! Fix your config, or set keep_unknown_macros accordingly...

[icinga-migration]

Updated by mfriedrich on 2012-08-22 17:38:10 +00:00

# KEEP UNKNOWN MACROS
# This option can be used to keep unknown macros within the output.
# e.g. check_proc -C $foo$ will remain.
# This was the default in versions older than Icinga 1.8, but now
# the default is to remove those macros from the output, causing
# the shell to interpret $foo and leaving a single $ there. See
# #2291 for further information.
# Make sure to escape single dollar signs with another '$', as the
# docs describe. Other than that, enable this setting to revert to
# the old behaviour.

keep_unknown_macros=0

[icinga-migration]

Updated by mfriedrich on 2012-08-22 17:42:38 +00:00

this is a behavioural change, which should be marked in CHANGES section. people wanting the old behavior can revert that by setting

keep_unknown_macros=1

where the macro outputbuffer is kept as is. the default case will be to warn the user on unkbown macros.

that can be

• the wrong macro for the object (e.g. a host using a hostgroup macro)
• a not existing macro
• a nonescaped password

it possibly not work with the shell tricks for passwords, such as

'$mypassisquoted$fortheshell'

the correct way of defining that one will be

'$$mypassisquoted$$fortheshell'

in order to let the core know that there's actually a valid dolar sign. that method is described in the docs, but i believe people have probably circumvented that in various ways, so prepare for the clusterfuck either way.

but, as a matter of fact, leaving that on the shell for interpreting $foo and erroring out on $ this should be fixed either way.

[icinga-migration]

Updated by mfriedrich on 2012-08-22 17:43:35 +00:00

• Subject changed from non existant macros are not replaced, and misleading to single dollar signs to unknown macros are not replaced, and misleading to single dollar signs

[icinga-migration]

Updated by mfriedrich on 2012-08-22 17:54:10 +00:00

• Status changed from Assigned to 7
• Done % changed from 0 to 90

in dev/core - please test!

[icinga-migration]

Updated by mfriedrich on 2012-08-31 11:10:53 +00:00

• Status changed from 7 to Resolved
• Done % changed from 90 to 100