NagiosEnterprises / ncpa

Nagios Cross-Platform Agent
Other
177 stars 95 forks source link

NCPA strips slashes from anything passed to a plugin #942

Open ChristopherSchultz opened 1 year ago

ChristopherSchultz commented 1 year ago

I don't think issue #303 was ever actually fixed.

When running check_ncpa.py recursively I get this:

$ /usr/local/nagios/libexec/check_ncpa.py -H host.example.com -M plugins/check_ncpa.py \
    -t mytoken -a '-H 10.0.0.1 -M plugins/check_yum -t mytoken'
UNKNOWN: Unable to run check on node without check method. Requested 'plugins' node.

I have a simple echo.sh script in plugins/ which just dumps each argument on a separate line. Repeating the above with my script instead of check_ncpa.py gives me:

$ /usr/local/nagios/libexec/check_ncpa.py -H host.example.com -M plugins/echo.sh \
    -t mytoken -a '-H 10.0.0.1 -M plugins/check_yum -t mytoken'
arg: -H
arg: 10.0.0.1
arg: -M
arg: plugins
arg: check_yum
arg: -t
arg: mytoken

Note that plugins/check_yum has become plugins and check_yum separately.

I've tried a series of hacks to get the / to come through, and was unable to do so (I'm removing extraneous output):

$ /usr/local/nagios/libexec/check_ncpa.py -H host.example.com -M plugins/echo.sh \
    -t mytoken -a '-H 10.0.0.1 -M plugins\/check_yum -t mytoken'
arg: plugins check_yum
$ /usr/local/nagios/libexec/check_ncpa.py -H host.example.com -M plugins/echo.sh \
    -t mytoken -a '-H 10.0.0.1 -M plugins%2Fcheck_yum -t mytoken'
arg: plugins%2Fcheck_yum
$ /usr/local/nagios/libexec/check_ncpa.py -H host.example.com -M plugins/echo.sh -t mytoken \
    -a '-H 10.0.0.1 -M plugins%252Fcheck_yum -t mytoken'
arg: plugins%252Fcheck_yum

This appears to be related to how Flask performs URL handling. I have read https://github.com/pallets/flask/issues/900 and it meanders a bit and blames things on Apache httpd and other things which are not in play with NCPA server. But it seems like the logic for Flask is that it decodes the URL before splitting on / to separate-out parameters. I can see that check_ncpa.py is properly-encoding the slashes in the URL being sent to the server, but the server is mangling the path segments prematurely. I'm not sure if this is something that "should be" fixed in Flask or in NCPA but maybe there is a way to get NCPA to work around this.

The only way I was able to make this work was to create a hack script to wrap check_ncpa.py on the server by performing my own unescaping:

#!/bin/bash
# check_ncpa converts / arguments to spaces when inside -a
# so we'll convert literal %2F into /
args=""
while [ "$#" -gt 0 ] ; do
  args="$args $( echo $1 | sed -e 's/%2[fF]/\//g' )"
  shift
done

#echo "Running ncpa with arguments: $args"

$( dirname "$0" )/check_ncpa.py $args

Now I can run my check like this:

$ /usr/local/nagios/libexec/check_ncpa.py -H host.example.com -M plugins/ncpa.sh \
    -t mytoken -a '-H 10.0.0.1 -M plugins%2Fcheck_yum -t mytoken'
OK - System is updated.

This hack works, but it's ugly and has some security issues due to the way the arguments are handled as strings but I'm at my wit's end trying to make NCPA work recursively.

Is there a better way to do this that I'm just not seeing? If not, it would be great if NCPA server can solve this problem.

ccztux commented 1 year ago

I had a similar issue #937 and i have fixed it in this version of check_ncpa.py.

I have tested your issue with the above linked version of check_ncpa.py and with this version the error doesnt occur.

Can you test it with that version?

Tests with the fixed version of check_ncpa.py:

Argument -a doenst work:

./check_ncpa.py -H cl01 -t mytoken -M 'plugins/check_ncpa.py' -a '-H cl02 -M plugins/check_test.sh -t mytoken'
UNKNOWN: Unable to run check on node without check method. Requested 'plugins' node.

New Argument -r works:

./check_ncpa.py -H cl01 -t mytoken -M 'plugins/check_ncpa.py' -r '-H cl02 -M plugins/check_test.sh -t mytoken'
hi
ccztux commented 1 year ago

@corynorell It would be nice if -a could be replaced by -r. Not sure if this is possible for compatibility reasons.

corynorell commented 1 year ago

@ccztux - just accepted your PR (#938) - thank you for doing that.

For now, we'll keep the -a flag around for the reason you mentioned: compatibility with systems that may be using said flag.

@ChristopherSchultz If you would like, you could drop @ccztux's version of check_ncpa.py in place on your system and see if it is an acceptable solution for you. Otherwise we will have a follow-up release with the aforementioned changes included sometime in the near future; though I can't promise a date.

I'll keep this issue open for a while until we hear back from you.

ChristopherSchultz commented 1 year ago

I'm sorry, I'm just now coming back to this issue way later than expected. I'll give it a try.

ChristopherSchultz commented 1 year ago

@corynorell and @ccztux On first inspection, this appears to work as expected. We'll try to put it through its paces.