Security issue MYSQL_PASSWORD as ENV variable

[mazoo]

Thanks for your great work, especially for this article.

Although we have carefully crafted this example it may contain bugs, security issues or other problems that we were not aware of at the time.

Here is what we do:
Using the swarm mode of Docker, we create Docker Secrets.

Instead of

MYSQL_PASSWORD | MySQL password | secure

we're using:

MYSQL_PASSWORD | MySQL password | secure or /run/secrets/MYSQL_PASSWORD

The place, where we need it, we do something like:

$password = $is_dev ?  $_ENV['MYSQL_PASSWORD'] : trim(file_get_contents($_ENV['MYSQL_PASSWORD'])),

So, if $is_dev, we can use it as plaintext, otherwise we get the output from our docker secret.

[ju5t]

Fair point.

It's only used when setting the database credentials here.
https://github.com/sensson/docker-magento2/blob/master/entrypoint.sh#L41

We could probably replace this with something like:

if [ -f "${MYSQL_PASSWORD}"]; then
  _MYSQL_PASSWORD=$(<"$MYSQL_PASSWORD")
else
  _MYSQL_PASSWORD=$MYSQL_PASSWORD
fi

CMD_CONFIG="${CMD_MAGENTO} setup:config:set --db-host="${MYSQL_HOSTNAME}" \
            --db-name="${MYSQL_DATABASE}" --db-user="${MYSQL_USERNAME}" \
            --db-password="${_MYSQL_PASSWORD}" --key="${CRYPTO_KEY}""

It would still require you to pass on the information through an environment variable.

I'm not 100% sure how it affects running copies as CMD_CONFIG runs on every start. I'm not a Magento guru by any means but it could be that the configuration is overridden by this. The actual password wouldn't be in ENV though.

Would that help?

[mazoo]

IMHO passing secrets through ENV variables is not state of the art.
Could you imagine something like this:

if [ ! -z "$MYSQL_PASSWORD_FILE" -a -z "$MYSQL_PASSWORD" ]; then
  MYSQL_PASSWORD=$(cat $MYSQL_PASSWORD_FILE)
fi

MYSQL_PASSWORD_FILEbeing a Docker secret, for example /run/secrets/MYSQL_PASSWORD.

See also here.

[ju5t]

In your example you said that you used the following code:

$password = $is_dev ?  $_ENV['MYSQL_PASSWORD'] : trim(file_get_contents($_ENV['MYSQL_PASSWORD'])),

which is why I made the entrypoint.sh example to check if MYSQL_PASSWORD is a file and use its contents, so it wouldn't interfere with what you have already.

Feel free to open a PR for whatever solution you feel is best. You can introduce a new variable or reuse the existing one as the example I posted, I don't have a strong opinion about either.