$SECRET_OUTPUT_DEV'¶

Labels: enhancement, fixed / solved / done

jsmeix opened issue at 2023-08-02 09:09:¶

Type: Enhancement
Impact: Low
Reference to related issue (URL):

See
https://github.com/rear/rear/issues/2967
and
https://github.com/rear/rear/pull/3006
therein in particular
https://github.com/rear/rear/pull/3006#issuecomment-1590775360

How was this pull request tested?

See
https://github.com/rear/rear/pull/3006#issuecomment-1578565338

Brief description of the changes in this pull request:

Use

{ SECRET COMMAND ; } 2>>/dev/$SECRET_OUTPUT_DEV

instead of

{ SECRET COMMAND ; } 2>/dev/null

Reasoning:
See
https://github.com/rear/rear/issues/2967#issuecomment-1562732484
that reads (excerpt):

the

{ ... ; } 2>>/dev/$SECRET_OUTPUT_DEV

syntax makes it clear which redirections are
explicitly meant to hide secrets to distinguish them
from usual unwanted output discard via '2>/dev/null'

jsmeix commented at 2023-08-02 11:23:¶

This is not a critical issue
because before there was already 2>/dev/null
so nothing leaked into the log.

jsmeix commented at 2023-08-02 11:57:¶

While I searched the scripts for '2>/dev/null'
I found it in lib/authtoken-functions.sh

# grep '2>/dev/null' usr/share/rear/lib/authtoken-functions.sh
    length=$(base64 -d <<< "${rawchunk:0:8}" 2>/dev/null | tr -d '\0' 2>/dev/null)
    lenb64=$(base64 <<< $(printf '%05d' $lenwr) 2>/dev/null)
            data=$(dd if="$1" bs=$2 count=1 iflag=skip_bytes skip=${3:-0} status=none 2>/dev/null | sed 's/\x00/\x1a/g' 2>/dev/null)
    ciphertext=$(tr -d '\r\n' <<< "$ciphertext" 2>/dev/null)
        plaintext=$(systemd-creds --with-key=tpm2 --name="${tmp%:*}" decrypt - - <<< "$1" 2>/dev/null); ret=$?
        plaintext=$(openssl aes-256-cbc -d -a -A -pbkdf2 -pass pass:"$2" <<< "$1" 2>/dev/null); ret=$?
    garbage=$(dd if="${3:-/dev/urandom}" bs=$1 count=1 status=none 2>/dev/null | base64 --wrap=0 2>/dev/null)

I am not at all familiar with that code
but I think some of them leak secrets,
for example like

# ciphertext="SECRET"

# ( set -x ; ciphertext=$(tr -d '\r\n' <<< "$ciphertext" 2>/dev/null) )
++ tr -d '\r\n'
+ ciphertext=SECRET

which I assume should better be like

# ciphertext="SECRET"

# set -x

# { ciphertext=$( tr -d '\r\n' <<< "$ciphertext" ) ; } 2>/dev/null

# set +x
+ set +x

# echo $ciphertext
SECRET

jsmeix commented at 2023-08-03 07:04:¶

@rear/contributors
I would merge it today afternoon
unless objections appear.

pcahyna commented at 2023-08-24 12:46:¶

@jsmeix when starting the rescue system, I now get plenty of messages:

/usr/share/rear/conf/default.conf: line 751: /dev/: Is a directory 
/usr/share/rear/conf/default.conf: line 786: /dev/: Is a directory 
/usr/share/rear/conf/default.conf: line 874: /dev/: Is a directory 
/usr/share/rear/conf/default.conf: line 1445: /dev/: Is a directory 
/usr/share/rear/conf/default.conf: line 1924: /dev/: Is a directory 
/usr/share/rear/conf/default.conf: line 1994: /dev/: Is a directory 
/usr/share/rear/conf/default.conf: line 2272: /dev/: Is a directory 
/usr/share/rear/conf/default.conf: line 2957: /dev/: Is a directory 
/usr/share/rear/conf/default.conf: line 2991: /dev/: Is a directory

Verifying md5sums of the files in the Relax-and-Recover rescue system

md5sums are OK


Configuring Relax-and-Recover rescue system

Line 751 is:
https://github.com/rear/rear/pull/3034/files#diff-00613392f8fca2a4c8c6ffb1eb59e66f4e1504eeeb14cf6b5a13458310a8227eL751

The problem comes from here:
https://github.com/rear/rear/blob/0b069f1f923cb9e1a9f7a97efb007b8d97067ab2/usr/share/rear/skel/default/etc/scripts/system-setup#L76

where default.conf is being sourced outside of /usr/sbin/rear and thus does not have SECRET_OUTPUT_DEV set.

jsmeix commented at 2023-09-04 14:10:¶

Oops!

@pcahyna
thank you so much for reporting that new bug!

It is even a serious bug in theory because it does not set the variable

# { var='value' ; } 2>>/dev/$SECRET_OUTPUT_DEV
bash: /dev/: Is a directory

# echo "'$var'"
''

but I think - fortunately - it is not serious in practice
because none of the variables that are set with SECRET_OUTPUT_DEV
are needed by the ReaR recovery system startup scripts
in etc/scripts/system-setup.d/*.sh
I think OPAL_PBA_DEBUG_PASSWORD and OPAL_PBA_TKNKEY
are used in etc/scripts/unlock-opal-disks
which is not a ReaR recovery system startup script.

[Export of Github issue for rear/rear.]

#3034 PR merged: Use '2>>/dev/$SECRET_OUTPUT_DEV'¶