uaa_health_check() in bin/ctl is flaky

[aeijdenberg]

What version of the credhub server you are using?

2.1.2

If you were attempting to accomplish a task, what was it you were attempting to do?

Deploy via bosh with uaa

What did you expect to happen?

Not get errors in stderr

What was the actual behavior?

/var/vcap/sys/log/credhub/credhub.stderr.log shows:

[2018-12-06 01:42:18+0000] /var/vcap/jobs/credhub/bin/ctl: line 15: [: too many arguments

Please confirm where necessary:

• I have included a log output
• My log includes an error message
• I have included steps for reproduction

Detail

/var/vcap/jobs/credhub/bin/ctl contains this code:

uaa_health_check() {
    status=`curl --max-time 5 --connect-timeout 2 -k --silent https://uaa.example.com/healthz`
    if [ $? -ne 0 ]; then
      echo "Could not reach the UAA server"
      exit 1
    fi

    if [ $status != "ok" ]; then
      exit 1
    fi
    echo "Successfully connected to UAA, continuing startup"
}

I'm guessing that under some circumstances the call the /healthz is returning a value with spaces in it, e.g. not ready.

So for example if you ran:

status="not ready"
[ $status != "ok" ]

This will reproduce the error:

-bash: [: too many arguments

Wrapping in double-quotes, e.g. [ "$status" != "ok" ] will fix this specific example.

It's not clear to me whether there isn't a latent vulnerability associated with taking arbitrary output from the UAA /healthz check and passing to the test in this manner, but I wasn't able to find a way to exploit it.

In any case I think there are at least 3 issues:

1. That this error didn't result in the script actually failing. Perhaps consider adding set -e or similar so that errors actually cause failure instead of failing silently.
2. Quoting or otherwise treating correctly the value of $status would allow this check to proceed as designed.
3. The -k in the curl command skips TLS verification. Given the trouble we have to go to in order to pass CA certs to begin with, there doesn't seem any obvious good reason for doing this.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/162468758

The labels on this github issue will be updated when the story is started.

[mdelillo]

@aeijdenberg Thanks for reporting this, it definitely looks like a bug. We'll get a fix out soon.