New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not log BACKUP_PROG_CRYPT_KEY value (issue 2155) #2156
Do not log BACKUP_PROG_CRYPT_KEY value (issue 2155) #2156
Conversation
My very first usage of BACKUP_PROG_CRYPT_ENABLED Because in this case the whole backup command pipe Here my whole backup.log for "rear -D mkbackup":
The only line that is not caused by
i.e. the The recover.backup.tar.gz.*.restore.log looks a bit better
i.e. tons of |
I think I can improve it so that only the actually confidential command E.g. on my SLES11 system
and on my openSUSE Leap 15.0 system
I use the subshell here only to encapsulate the |
…ation how to run things confidential
…_PROG_CRYPT_KEY assignment lines
…log tells intentionally nothing what was actually done
…sh of files that are not affected
Now all works well for me |
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, cf. #2156
Type: Bug Fix
Impact: Critical
This is a critical security issue only if BACKUP_PROG_CRYPT_ENABLED
with a BACKUP_PROG_CRYPT_KEY is used.
Reference to related issue (URL):
User's secret BACKUP_PROG_CRYPT_KEY value is shown in backup.log #2155
therein in particular
User's secret BACKUP_PROG_CRYPT_KEY value is shown in backup.log #2155 (comment)
How was this pull request tested?
This is an early submit so that you can have an early look
that is not yet tested at all by me because I never used
BACKUP_PROG_CRYPT_ENABLED myself.
Brief description of the changes in this pull request:
Separate the special case when BACKUP_PROG_CRYPT_ENABLED
with a BACKUP_PROG_CRYPT_KEY is used
from the usual case when it is not used.
Only when BACKUP_PROG_CRYPT_ENABLED with
a BACKUP_PROG_CRYPT_KEY is used do special stuff
(in particular redirect stderr to /dev/null for certain commands)
to avoid that the BACKUP_PROG_CRYPT_KEY value appears
in a log file in particular when
set -x
is set.I think it is more important to not leak out user secrets into a log file
than having stderr error messages when a confidential command fails.
That separation caused "mostly duplicated" code parts, i.e. code parts
that are almost same - except the differences that are related to
the "BACKUP_PROG_CRYPT_ENABLED" case but I have no better idea
how to redirect stderr to /dev/null versus not redirect stderr to /dev/null
for the two cases in a clean way without making the code even more
complicated and oversophisticated than it already is.
By the way I also corrected the spelling typo
from
debugscripts mode
todebugscript mode
to match the spelling in "man rear" that reads