Fix possible log cred leak (#442)

* Fix possible log cred leak

[#184798214]

* Fix possible log leak of CF password

On 2023-03-27 we discovered that new stemcells were using OS audit tools to
log every command run on a VM. This was causing CF authentication commands
of the form `cf auth --client-credentials USERNAME PASSWORD` to be logged
in plaintext to syslog.

Internal shell commands like setting variables are not logged by OS audit
tools, so this commit changes our auth from passing a password as an
argument to passing a password as an environment variable

when a binary is executed on linux the audit system will log the
execution into syslog. This log includes the arguments provided to the
binary call. In this script these arguments contain the admin creds for
the created service broker.

To avoid this, switch to use echo (a bash builtin) and
<() process substition which in turn provides file descriptors. This can
then be used with cf curl via the -d param which accepts a path via @
annotation.

The cf cli currently doesn't support sourcing values from the environment
or config files for this call. Even once it supports it, we cannot be
totally sure that our release is running with the required cf cli
version available. We assume that the cf cli is present on the VM that
this errand is executed on, but we do not provide it via our release.

This implementation uses cf curl instead.
This way the data can be provided through means that will not log
cleartext credentials into syslog.

Co-authored-by: Gareth Smith <sgareth@vmware.com>

* remove var export

some work for avoiding this cred leak has already been done in 013342d

---------

Co-authored-by: Gareth Smith <sgareth@vmware.com>

Author: nouseforaname

jobs/nfsbrokerpush/templates/deploy.sh.erb

@@ -125,9 +125,23 @@ function push_app() {
     fi
   popd > /dev/null
 }
-
 function register_service() {
-  cf create-service-broker $SERVICE_BROKER_NAME $USERNAME $PASSWORD $APP_URL || cf update-service-broker $SERVICE_BROKER_NAME $USERNAME $PASSWORD $APP_URL
+  # We want to avoid providing creds as commandline params to binaries to avoid leaking creds via autitd logs. The below `cf curl` replaces the (create|update)-service-broker commands.
+  # cf create-service-broker $SERVICE_BROKER_NAME $USERNAME $PASSWORD $APP_URL || cf update-service-broker $SERVICE_BROKER_NAME $USERNAME $PASSWORD $APP_URL
+  if ! cf curl "/v2/service_brokers?q=name:$SERVICE_BROKER_NAME" | grep $SERVICE_BROKER_NAME; then
+    echo "Creating service broker - name:${SERVICE_BROKER_NAME} host:${APP_URL}"
+    echo "{\"auth_username\":\"${USERNAME}\",\"auth_password\":\"${PASSWORD}\",\"broker_url\":\"${APP_URL}\", \"name\": \"${SERVICE_BROKER_NAME}\"}" > ./data
+    cf curl --fail \
+      -X POST "/v2/service_brokers" \
+      -d @./data
+    rm data
+  else
+    echo "Updating service broker - name:${SERVICE_BROKER_NAME} host:${APP_URL}"
+    broker_guid="$(cf curl /v2/service_brokers?q=name:$SERVICE_BROKER_NAME | grep \"guid\" | cut -f4 -d\")"
+    cf curl --fail \
+      -X PUT "/v2/service_brokers/${broker_guid}" \
+      -d @<(echo "{\"auth_username\":\"${USERNAME}\",\"auth_password\":\"${PASSWORD}\",\"broker_url\":\"${APP_URL}\"}") > /dev/null
+  fi
 }
 
 function clean_up() {

jobs/nfsbrokerpush/templates/start.sh.erb

@@ -24,6 +24,7 @@
     raise 'missing credhub URL'
   end
 %>
+
 bin/nfsbroker --listenAddr="0.0.0.0:$PORT" \
               --servicesConfig="./services.json" \
               --credhubURL="<%= credhub_url %>" \

spec/jobs/nfsbrokerpush/start_sh_spec.rb

@@ -41,6 +41,8 @@
 
         expect(tpl_output).to include("bin/nfsbroker --listenAddr=\"0.0.0.0:$PORT\"")
         expect(tpl_output).to include("--credhubURL=\"https://some-credhub-url:4321\"")
+        expect(tpl_output).not_to include("--uaaClientID=\"client-id\"")
+        expect(tpl_output).not_to include("--uaaClientSecret=\"client-secret\"")
         expect(tpl_output).to include("--servicesConfig=\"./services.json\"")
         expect(tpl_output).to include("--logLevel=\"some-log-level\"")
         expect(tpl_output).to include("--timeFormat=\"some-log-time-format\"")
@@ -60,10 +62,12 @@
         }
       end
 
-      it 'includes the credhub flags in the script' do
+      it 'includes the unsensitive credhub flags in the script' do
         tpl_output = template.render(manifest_properties, consumes: credhub_link)
 
         expect(tpl_output).to include("--credhubURL=\"https://some-credhub-url:4321\"")
+        expect(tpl_output).not_to include("--uaaClientID=\"some-uaa-client-id\"")
+        expect(tpl_output).not_to include("--uaaClientSecret=\"some-uaa-client-secret\"")
         expect(tpl_output).to include("--storeID=\"nfsbroker\"")
       end