Do not leak the GALAXY11_PASSWORD value into the log file

[jsmeix]

• Type: Bug Fix

• Impact: Critical

• Reference to related issue (URL):
  Do not log BACKUP_PROG_CRYPT_KEY value (issue 2155) #2156

• How was this pull request tested?
  I cannot test it because I do not have the needed backup software

• Brief description of the changes in this pull request:

In
verify/GALAXY11/default/420_login_to_galaxy_and_setup_environment.sh
run commands that deal with GALAXY11_PASSWORD
in a confidential way via

{ confidential_command ; } 2>/dev/null

to not leak the GALAXY11_PASSWORD value into the log file

[jsmeix]

verify/GALAXY11/default/420_login_to_galaxy_and_setup_environment.sh
is the only place where GALAXY11_PASSWORD is used according to

# find usr/sbin/rear usr/share/rear -type f | xargs grep 'GALAXY11_PASSWORD' | grep -v ': *#'

usr/share/rear/conf/default.conf:
{ GALAXY11_PASSWORD=${GALAXY11_PASSWORD:-} ; } 2>/dev/null

usr/share/rear/verify/GALAXY11/default/420_login_to_galaxy_and_setup_environment.sh:
    if [ -n "$GALAXY11_USER" ] && [ -n "$GALAXY11_PASSWORD" ]; then
            qlogin -u "${GALAXY11_USER}" -clp "${GALAXY11_PASSWORD}" || \
                    Error "Could not logon to Commvault CommServe with credentials from GALAXY11_USER ($GALAXY11_USER) and GALAXY11_PASSWORD. Check the log file."
            LogPrint "CommVault client logged in with credentials from GALAXY11_USER ($GALAXY11_USER) and GALAXY11_PASSWORD"

[codefritzel]

@jsmeix
I tested your stand and it worked with Commvault (Galaxy11)

a comparison
e.g. GALAXY11_USER="my_commvault_user" and GALAXY11_PASSWORD="my_commvault_secret"

actual master version:

grep -n "my_commvault_secret" rear-myserver.log
2682:++ '[' -n 'my_commvault_secret' ']'
2683:++ qlogin -u 'my_commvault_user' -clp 'my_commvault_secret'

grep -n "qlogin" rear-myserver.log
2683:++ qlogin -u 'my_commvault_user' -clp 'my_commvault_secret'

your version:

grep -n "my_commvault_secret" rear-myserver-new.log
[empty]

grep -n "qlogin" rear-myserver-new.log
[empty]

In my opinion, it is very good to hide the credentials so that they are not uploaded by mistake.

Otherwise, in some places it may be useful to see them. But by default this should be disabled. This could be activated by an additional CMD line parameter (see 2967).

[jsmeix]

@codefritzel
thank you so much for testing it and
for your comment what your preferred default behaviour is!

[jsmeix]

@rear/contributors
I would like to merge it today afternoon
unless there are objections.

I know that with

{ qlogin -u "${GALAXY11_USER}" -clp "${GALAXY11_PASSWORD}" ; } 2>/dev/null

there are no longer any 'qlogin' stderr messages in the log
which could make it harder to find the root cause
when 'qlogin' fails - depending on what messages 'qlogin'
shows on stdout or stderr when it fails.

I think it is sufficient to see that 'qlogin' fails
(which is made clear here via the Error exit)
and then it is the user's task to debug on his own
why exactly 'qlogin' fails with his credentials
on his system in his environment.
At least I would never ever go so far to debug issues
where I would need to know secret user data
for further debugging.

In general user data is sacrosanct so
secret user data is topmost sacrosanct
which means some reasonable level of protection
from accidentally publishing secret user data
outweighs drawbacks in debugging specific cases.

[schlomo]

You could add an error messages saying that qlogin failed and should be retried on the console to see the error

[schlomo]

Fine with it. FYI, we wanted to switch this over the use UserInput to gather the login credentials so that --non-interactive would also catch it

[jsmeix]

A side note FYI:

My recent
4a3865a
shows an interesting bash coding subtleness:

# Using "if COMMAND ; then ... ; else echo COMMAND failed with $? ; fi" is mandatory
# because "if ! COMMAND ; then echo COMMAND failed with $? ..." shows wrong $? because '!' results $?=0

How bash behaves with $? versus $PIPESTATUS:

# true ; echo $?
0

# false ; echo $?
1

# ! true ; echo $?
1

# ! false ; echo $?
0

# true ; echo $PIPESTATUS
0

# false ; echo $PIPESTATUS
1

# ! true ; echo $PIPESTATUS
0

# ! false ; echo $PIPESTATUS
1

This is because '!' is a reserverd word in bash
so it behaves like a command i.e. it sets $?
so one has to avoid '!' like

# if ! cat qqqq ; then echo failed with $? ; else echo worked with $? ; fi
cat: qqqq: No such file or directory
failed with 0

# if cat qqqq ; then echo worked with $? ; else echo failed with $? ; fi
cat: qqqq: No such file or directory
failed with 1

Excerpts from "man bash" (GNU bash version 4.4.23)

A pipeline is a sequence of one or more commands ...

The return status of a pipeline is the exit status
of the last command, unless ...

If the reserved word ! precedes a pipeline,
the exit status of that pipeline is the logical negation
of the exit status as described above.

PIPESTATUS
An array variable ... containing a list of exit status values
from the processes in the most-recently-executed foreground
pipeline (which may contain only a single command).

[jsmeix]

As an addedum to make the code easier to read
in particular to avoid needless nested braces { ... {...} ... {...} ... }
I removed superfluous braces {...} around variable names via
07462c3