grommunio-{spam,ham}-run.sh: Insecure way of passing password to mysql(1)

[robert-scheck]

As of writing grommunio-spam-run.sh and grommunio-ham-run.sh parse /etc/gromox/mysql_adaptor.cfg and construct arguments to mysql – including the password (aside of this the current implementation might break with passwords containing spaces, semicolons or other characters interpreted by the bash).

https://mariadb.com/kb/en/mysql-command-line-client/ (more or less a web version of the man page) says:

Specifying a password on the command line should be considered insecure. You can use an option file to avoid giving the password on the command line.

The MySQL 8.0 Reference Manual goes into the details with § 6.1.2.1 End-User Guidelines for Password Security regarding -p:

This is convenient but insecure. On some systems, your password becomes visible to system status programs such as ps that may be invoked by other users to display command lines. MySQL clients typically overwrite the command-line password argument with zeros during their initialization sequence. However, there is still a brief interval during which the value is visible. Also, on some systems this overwriting strategy is ineffective and the password remains visible to ps. (SystemV Unix systems and perhaps others are subject to this problem.)

While I expect a system running grommunio to be sealed system (= not to be used by regular shell users), this can not be expected in general, nor does it seem to be required by the documentation. This insecure way of passing a password to mysql might be treated as security flaw, e.g. CWE-214: Invocation of Process Using Visible Sensitive Information. Note that I don't treat this as overly critical, because of the before mentioned behavior of MySQL clients.

My recommendation is to parse /etc/gromox/mysql_adaptor.cfg and to write a temporary configuration file (often known as .my.cnf) that could be passed using --defaults-file=<path to temporary configuration file> to mysql. Of course that temporary configuration file needs proper permissions and should be deleted after usage.

Interestingly, grommunio-index had the same issue, being fixed with grommunio/grommunio-index@4ebe9ad, thus a similar solution should also fit for grommunio-spam-run.sh and grommunio-ham-run.sh. By the way, I did not find any upstream for grommunio-spam-run.sh, even 629ab04 mentions one, however /usr/sbin/grommunio-spam-run.sh shipped by "grommunio-antispam" (RPM) package looks very similar. Is it possible that not all source code of the "grommunio-antispam" (RPM) package is currently publicly accessible (but still affected by the same weakness)?

[crpb]

Hey,

being fixed with grommunio/grommunio-index@4ebe9ad,

Yeah, i remember that issue and wanted to do that but it just didn't happen yet 🙈

By the way, I did not find any upstream for grommunio-spam-run.sh

I asked the support about it and they told me it would only exist in the (private afaik) OBS where the builds are happening..
And the publicly available stuff stuff isn't everything and not really up to date :-(.

even 629ab04 mentions one

thats just from my internal work repo i keep separate from/to i merge changes manually.

Is it possible that not all source code of the "grommunio-antispam" (RPM) package is currently publicly accessible (but still affected by the same weakness)?

No access as said before to my knowledge and yeah it has the same flaws.

grom-test-2:~ # rpm --query --file /usr/sbin/grommunio-spam-run.sh
grommunio-antispam-3.8.4-lp155.1.1.x86_64
grom-test-2:~ # cat /usr/sbin/grommunio-spam-run.sh
#!/bin/bash

# add "-d" to delete junk emails after they have been learned
if [ "$1" = "-d" ]
then
   DO_DEL=1
else
   DO_DEL=0
fi

MYSQL_CFG="/etc/gromox/mysql_adaptor.cfg"

if [ ! -e "${MYSQL_CFG}" ] ; then
  echo "MySQL configuration not found. (${MYSQL_CFG})"
  exit 1
fi

MYSQL_PARAMS="--skip-column-names --skip-line-numbers"
MYSQL_USERNAME=$(sed -ne 's/mysql_username\s*=\s*\(.*\)\s*/-u\1/pI' ${MYSQL_CFG})
MYSQL_PASSWORD=$(sed -ne 's/mysql_password\s*=\s*\(.*\)\s*/-p\1/pI' ${MYSQL_CFG})
MYSQL_DBNAME=$(sed -ne 's/mysql_dbname\s*=\s*\(.*\)\s*/\1/pI' ${MYSQL_CFG})
MYSQL_HOST=$(sed -ne 's/mysql_host\s*=\s*\(.*\)\s*/-h\1/pI' ${MYSQL_CFG})
MYSQL_QUERY="SELECT username,maildir from users where id <> 0 and maildir <> \"\";"
MYSQL_CMD=("mysql" "${MYSQL_PARAMS}" "${MYSQL_USERNAME:=-uroot}" "${MYSQL_PASSWORD:=}" "${MYSQL_HOST:=-hlocalhost}" "${MYSQL_DBNAME:=grommunio}")
SQLITE_QUERY='select message_id,mid_string from messages where parent_fid=0x17;'
# shellcheck disable=SC2068
if ${MYSQL_CMD[@]}<<<"exit"&>/dev/null; then
  echo "${MYSQL_QUERY}" | ${MYSQL_CMD[@]} | while read -r USERNAME MAILDIR ; do
    SPAMLIST=$(mktemp)
    echo "${SQLITE_QUERY}" | sqlite3 "${MAILDIR}/exmdb/exchange.sqlite3" -tabs -noheader > ${SPAMLIST}
    cat ${SPAMLIST} | while read -r MESSAGEID MIDSTRING; do
      echo "Learning spam for user ${USERNAME}" | systemd-cat -t grommunio-spam-run
      MSGFILE="$MAILDIR/eml/$MIDSTRING"
      if [[ ! -f "$MSGFILE" ]]; then
        gromox-exm2eml -u "${USERNAME}" "${MESSAGEID}" 2>/dev/null | rspamc learn_spam | systemd-cat -t grommunio-spam-run
      else
        rspamc learn_spam --header 'Learn-Type: bulk' "$MSGFILE" | systemd-cat -t grommunio-spam-run
      fi
      if [ ${DO_DEL} -eq 1 ]; then
        /usr/sbin/gromox-mbop -u "${USERNAME}" delmsg -f 0x17 "${MESSAGEID}" | systemd-cat -t grommunio-spam-run
      fi
    done
    rm -f "${SPAMLIST}"
  done
else
  echo "MySQL-Connection couldn't be established, please check your configuration." | systemd-cat -t grommunio-spam-run -p err
fi

I will send in an report to grommunio.

[crpb]

Please check out bd52b78 if i didn't miss anything.

[robert-scheck]

Please check out bd52b78 if i didn't miss anything.

Shall I check above one or bba96c3 (which seems to be force pushed)?

[crpb]

yep, the latest one 🙈

[robert-scheck]

yep, the latest one 🙈

Thanks for the clarification, I'll have a look to it in the next days.

[robert-scheck]

bba96c3

Yes, commit bba96c3 looks good. Will you ask upstream to inherit both scripts?

[crpb]

Yes, commit bba96c3 looks good. Will you ask upstream to inherit both scripts?

Thanks for looking over it and yeah i will send it in.

[crpb]

fyi, 6345741

 [client]
 user=${MYSQL_USERNAME}
-password=${MYSQL_PASSWORD}
+password='${MYSQL_PASSWORD}'
 host=${MYSQL_HOST}
 database=${MYSQL_DBNAME}
 CONFFILE

needed if you use e.g. # with a manually set password

which isn't a problem with the grommunio-setup defaults

randpw()
{
        < /dev/urandom tr -dc A-Za-z0-9 | head -c"${1:-16}"; echo;
}

still waiting for them to implement at least the --defaults-file= into upstream 🙊