check_ps_unauth_users causes nhc to crash

[thedavidwhiteside]

It seems that because check_ps_unauth_users returns an error string containing a ', it causes a parsing error in nhc. Below includes a snippet of my nhc.conf and the nhc log that demonstrates the issue.

$ grep check_ps_unauth /etc/nhc.conf * || check_ps_unauth_users log syslog kill die

$ tail /var/log/nhc.log

/usr/sbin/nhc: eval: line 54: syntax error near unexpected token `('
/usr/sbin/nhc: eval: line 54: `/usr/libexec/nhc/node-mark-offline 'n10000' 'check_ps_unauth_users:  david's "mono" process is unauthorized. (PID 777)''

I have a few ideas of a possible fix, one is to filter the note to just a set of valid characters and another is to let bash escape it by using a subprocess (). I just wanted to bring this to your attention and see what your opinion is for the best way to resolve this.

/usr/sbin/nhc - filtering the note

<         eval $OFFLINE_NODE "'$HOSTNAME'" "'$*'"

---
>         MARK_OFFLINE_NOTE="$*"
>         eval $OFFLINE_NODE "'$HOSTNAME'" "'${MARK_OFFLINE_NOTE//[^0-9a-zA-Z :_()\-]}'"

/usr/sbin/nhc - bash subprocess, which escapes the command arguments

<         eval $OFFLINE_NODE "'$HOSTNAME'" "'$*'"

---
>         ($OFFLINE_NODE "$HOSTNAME" "$*")

[mej]

I'm not yet sure what the right answer is here. The simplest fix is to change the message in check_ps_unauth_users() so that it doesn't have the contraction in it, but it seems like maybe this is a larger problem since users' own checks might trigger the same issue.

My gut instinct is always to avoid subshells, but that does seem like the cleanest solution at first glance. So I'm a little torn to be perfectly honest. I am going to have to play with this a little bit more. I think I may have another idea or two myself, but I need to try them and make sure I'm not completely smoking crack.

Definitely thanks for pointing this out!

[thedavidwhiteside]

I had almost forgotten about this issue until today we are running into it again. I am planning on going with a bash subprocess for our hotfix, did you sort out any other alternate solutions?

[mej]

After a substantial amount of time spent on research and experimentation for the past few weeks, and after trying several different possible alternatives, I've come to the inescapable conclusion that the best approach is, in fact, the subshell syntax originally suggested above.

What I didn't realize at first is that the use of eval actually creates a subshell as well, so preferring it over a parenthesized subshell expression was ultimately self-defeating. I also tried numerous filtering/quoting techniques to try and talk BASH into doing the heavy lifting, but in light of what I've now learned, the simplest and safest method for doing exactly that is the subshell syntax.

This problem will be fixed in #131 once merged, which I plan to do right away. Given the simplicity of the changes and how easily one could back them out if necessary, I don't see much reason for delaying the merge on this one! :-)

[mej]

Fixed in #131.