Skip to content
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

[issue] entrypoint script does not escape password properly #183

Closed
Celmor opened this issue Jul 15, 2018 · 14 comments · Fixed by #356
Closed

[issue] entrypoint script does not escape password properly #183

Celmor opened this issue Jul 15, 2018 · 14 comments · Fixed by #356
Labels

Comments

@Celmor
Copy link

Celmor commented Jul 15, 2018

Type Version/Name
MARIADB_VERSION 1:10.3.8+maria~jessie
Docker Image mariadb:10.3.8@sha256:f2085c2176ba6294cf73033b344a420faa2ddae1b97b6795c101552e86284ba3
Docker Hosts Kernel Version 4.17.0-2-MANJARO
Docker Server Version 18.05.0-ce

Description

I've generated my password for MYSQL_ROOT_PASSWORD env var with keepass with all character ranges enabled. The container exits with

ERROR 1064 (42000) at line 6: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'Oe(]xI=<&(') ;
GRANT ALL ON *.* TO 'root'@'localhost' WITH GRANT OPTION ;
CREATE' at line 1

Full log
where 'Oe(]xI=<&( was part of my MYSQL_ROOT_PASSWORD.

Steps to reproduce

Run sudo docker run --rm -e "MYSQL_ROOT_PASSWORD='Oe(]xI=<&(" mariadb:10.3.8@sha256:f2085c2176ba6294cf73033b344a420faa2ddae1b97b6795c101552e86284ba3

@yosifkit
Copy link
Contributor

We don't escape any password characters since we didn't want to implement the equivalent of mysql_real_escape_string in bash and didn't want to burden the image with an install of a library/language just for this purpose.

https://github.com/docker-library/mariadb/blob/8818681187395ecf968ab61dfd8d04d458c356dc/docker-entrypoint.sh#L130

@Celmor
Copy link
Author

Celmor commented Jul 16, 2018

@yosifkit Then the fact passwords won't be escaped (and have to be chosen appropriately) should be documented on the Docker Hub page.

@tianon
Copy link
Contributor

tianon commented Sep 5, 2018

This is essentially a duplicate of docker-library/mysql#338.

@ltangvald what're your thoughts? For postgres, we recently applied docker-library/postgres#489, which allowed us to pass variables directly to psql so that the user input could be properly escaped by psql itself -- does there already exist any similar method in mysql for implementing something like that which I'm not familiar with (and can't seem to find)? 🙏 (Maybe something like that on the roadmap for the future? 😅)

@ltangvald
Copy link

@ltangvald
Copy link

ltangvald commented Sep 6, 2018

   %q     ARGUMENT is printed in a format that  can  be  reused  as  shell
          input, escaping non-printable characters with the proposed POSIX
          $'' syntax.

This escapes certain symbols that don't need to be escaped in MySQL, such as $, but MySQL handles that, so if the root password is set to 'mypa$s', printf %q will output 'mypa\$s', but the password will still be stored as 'mypa$s'

@tianon
Copy link
Contributor

tianon commented Sep 12, 2018

So, I don't think that'll work properly given that it outputs $'...' syntax sometimes:

$ printf '%q\n' "$(echo -e 'a \r b \n c \\ d '\'' e " f \x1a g')"
$'a \r b \n c \\ d \' e " f \032 g'

(Which I don't think is valid in MySQL, right?)

@ltangvald
Copy link

Does that work in postgres? i.e. setting the password to

"$(echo -e 'a \r b \n c \ d ''' e " f \x1a g')"

@yosifkit
Copy link
Contributor

I got postgres to set the password without complaint but I can't get psql to authenticate using it in PGPASSWORD, so I am not sure which part failed (initial setting the password or using the password).

$ docker run -d -e POSTGRES_PASSWORD="$(echo -e 'a b \n c \\ d '\'' e " f \x1a g')" postgres:10

@ltangvald
Copy link

Right, the same happens with our deb packages (that use printf %q). I'm not sure we should try to support everything, but random password generators often output some special characters that need escaping.

@grooverdan
Copy link
Member

grooverdan commented Mar 5, 2021

Actually escaping isn't that bad. Only \ and ' need to be quoted and ensure the session (unlike my lazy test) SQL_MODE doesn't include NO_BACKSLASH_ESCAPES.

ref: https://mariadb.com/kb/en/string-literals/

#!/bin/bash
set -eo pipefail
shopt -s nullglob

m="$HOME/repos/build-mariadb-server-10.5/client/mariadb -S /tmp/build-mariadb-server-10.5.sock"

$m -e "SET GLOBAL SQL_MODE = REPLACE(@@SQL_MODE, 'NO_BACKSLASH_ESCAPES', '');"
while [ 1 ]
do
	REAL_PASSWORD=$(pwgen   --numerals   --capitalize  --symbols --secure 32 1)
	#read -r -d '' -N 30 REAL_PASSWORD < /dev/urandom
	# SQL escaping \
	p=${REAL_PASSWORD//\\/\\\\}
	# SQL escaping '
	MARIADB_ROOT_PASSWORD=${p//\'/\\\'}
        $m <<- EOSQL || true
                      SET PASSWORD FOR 'root'@'localhost' = PASSWORD('${MARIADB_ROOT_PASSWORD}');
EOSQL
	echo "pass=${MARIADB_ROOT_PASSWORD}"
        $m  -u root -p"${REAL_PASSWORD}" -Be 'select 1' > /dev/null
done

This cover all the pwgen characters that I saw after a while. Using /dev/urandom (the commented line) does however fail eventually on some character(s?) that I haven't got around to identifying (might even be a bug) - --default-character-set=binary didn't help.

If this is introduced now is this going to throw compatibility problems pre-escaping their password?

As a safe route, since no release has been done with 8e0175f I could start escaping the MARIADB_*PASSWORD variables only on images going forwards and leave MYSQL_*PASSWORD as is.

Any objection or other factors I should consider?

@mariadb-RoelVandePaar
Copy link

mariadb-RoelVandePaar commented Mar 8, 2021

Improved script. To use:

  1. If you have an initial root pwd for your setup, set PREV_PWD to it.
  2. Also change 'm' def line to match your setup.
#!/bin/bash
set -eo pipefail
shopt -s nullglob
set +H

m="/test/MD060321-mariadb-10.6.0-linux-x86_64-dbg/bin/mariadb -uroot -S/test/MD060321-mariadb-10.6.0-linux-x86_64-dbg/socket.sock --binary-mode test"

$m -e "SET GLOBAL SQL_MODE = REPLACE(@@SQL_MODE, 'NO_BACKSLASH_ESCAPES', '');"
PREV_PWD=''
FIRST_RUN=1
# Random entropy init
RANDOM=$(date +%s%N | cut -b10-19)
while true; do
  #REAL_PASSWORD=$(pwgen   --numerals   --capitalize  --symbols --secure 32 1)
  read -r -d '' -N 200 REAL_PASSWORD < /dev/urandom
  echo "REAL_PASSWORD=${REAL_PASSWORD}"; sync
  # SQL escaping \
  p=${REAL_PASSWORD//\\/\\\\}
  # SQL escaping '
  MARIADB_ROOT_PASSWORD=${p//\'/\\\'}
  echo "MARIADB_ROOT_PASSWORD=${MARIADB_ROOT_PASSWORD}"; sync
  if [ ${FIRST_RUN} -eq 1 -a -z "${PREV_PWD}" ]; then
    $m <<- EOSQL || exit 1
     SET PASSWORD FOR 'root'@'localhost' = PASSWORD('${MARIADB_ROOT_PASSWORD}');
EOSQL
  else
    $m -p"${PREV_PWD}" <<- EOSQL || exit 1
     SET PASSWORD FOR 'root'@'localhost' = PASSWORD('${MARIADB_ROOT_PASSWORD}');
EOSQL
  fi
  FIRST_RUN=0
  echo "pass=${MARIADB_ROOT_PASSWORD}"; sync
  $m -p"${REAL_PASSWORD}" -Be 'SELECT 1;' > /dev/null
  PREV_PWD="${REAL_PASSWORD}"
done

If I remove ' --binary-mode' an issue happens within about a minute. Otherwise, it happily keeps running.

@mariadb-RoelVandePaar
Copy link

Also adding a write-to-file script for long runs.

#!/bin/bash
set -eo pipefail
shopt -s nullglob
set +H

m="/test/MD060321-mariadb-10.6.0-linux-x86_64-dbg/bin/mariadb -uroot -S/test/MD060321-mariadb-10.6.0-linux-x86_64-dbg/socket.sock --binary-mode test"

$m -e "SET GLOBAL SQL_MODE = REPLACE(@@SQL_MODE, 'NO_BACKSLASH_ESCAPES', '');"
PREV_PWD=''
FIRST_RUN=1
# Random entropy init
RANDOM=$(date +%s%N | cut -b10-19)
while true; do
  #REAL_PASSWORD=$(pwgen   --numerals   --capitalize  --symbols --secure 32 1)
  read -r -d '' -N 200 REAL_PASSWORD < /dev/urandom
  #echo "REAL_PASSWORD=${REAL_PASSWORD}"; sync 
  if [ -r pwd_done.txt ]; then
    mv pwd_done.txt pwd_done.prev
  fi
  echo "|||||${REAL_PASSWORD}|||||" > pwd_done.txt; sync
  # SQL escaping \
  p=${REAL_PASSWORD//\\/\\\\}
  # SQL escaping '
  MARIADB_ROOT_PASSWORD=${p//\'/\\\'}
  #echo "MARIADB_ROOT_PASSWORD=${MARIADB_ROOT_PASSWORD}"; sync
  if [ ${FIRST_RUN} -eq 1 -a -z "${PREV_PWD}" ]; then
    $m <<- EOSQL || exit 1
     SET PASSWORD FOR 'root'@'localhost' = PASSWORD('${MARIADB_ROOT_PASSWORD}');
EOSQL
  else
    $m -p"${PREV_PWD}" <<- EOSQL || exit 1
     SET PASSWORD FOR 'root'@'localhost' = PASSWORD('${MARIADB_ROOT_PASSWORD}');
EOSQL
  fi
  FIRST_RUN=0
  #echo "pass=${MARIADB_ROOT_PASSWORD}"; sync
  $m -p"${REAL_PASSWORD}" -Be 'SELECT 1;' > /dev/null
  PREV_PWD="${REAL_PASSWORD}"
done

@mariadb-RoelVandePaar
Copy link

mariadb-RoelVandePaar commented Mar 9, 2021

An overnight run did not locate any issues using the script from my last comment.
I tested Community Server 10.6, not Docker. Feel free to run some testing against Docker setup using the same also.

grooverdan added a commit to grooverdan/mariadb-docker that referenced this issue Mar 12, 2021
To correctly SQL escape passwords, escaping \ first is
required. Then we need to escape ' in the password to
prevent it being treated as a end of SQL statement quote.

All escaping needs to use \, so we cannot be in
NO_BACKSLASH_ESCAPES sql_mode otherwise no escaping will
work.

As an added complication when using the config file within
the entrypoint, for waiting until MariaDB can initialize,
a password mechanism a different escaping applies
(https://mariadb.com/kb/en/configuring-mariadb-with-option-files/).

The printf %q is close, however it provides a much
richer escape of non-printable characters than what can be
read in the configuration file. As such the password
complexity of the root password is limited to escaping
\n, \r, \t, \b, \s, \", \', and \\ while for a user
MARIADB_PASSWORD anything, including positively crazy strings
such as \0 will work.

Closes MariaDB#183
@grooverdan
Copy link
Member

Thanks for testing @mariadb-RoelVandePaar.

For our users #356, contains what has passed my testing so far. If you'd like to test this and provide feedback this would be most useful.

grooverdan added a commit to grooverdan/mariadb-docker that referenced this issue Mar 16, 2021
To correctly SQL escape passwords, escaping \ first is
required. Then we need to escape ' in the password to
prevent it being treated as a end of SQL statement quote.

All escaping needs to use \, so we cannot be in
NO_BACKSLASH_ESCAPES sql_mode otherwise no escaping will
work.

Getting bash to escape the root password for its
internal uses and in a form that was recognized
by MariaDB reading the configuration file was
becoming exceptionally complicated, and unsuccessful.

As such we use the MYSQL_PWD environment variable
to contain the password when needed.

Occasionally the socket wasn't ready on a clean install
by the time docker_process_sql was called so allow
up to 5 seconds just in case.

example logs of this:
2021-03-16 00:41:21+00:00 [Note] [Entrypoint]: Waiting for server startup
2021-03-16  0:41:21 140680894076608 [Note] mysqld (mysqld 10.2.37-MariaDB-1:10.2.37+maria~bionic) starting as process 111 ...
2021-03-16 00:41:21+00:00 [Note] [Entrypoint]: Temporary server started.
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Mutexes and rw_locks use GCC atomic builtins
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Uses event mutexes
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Compressed tables use zlib 1.2.11
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Using Linux native AIO
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Number of pools: 1
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Using SSE2 crc32 instructions
ERROR 2002 (HY000): Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2)

Closes MariaDB#183
grooverdan added a commit that referenced this issue Apr 27, 2021
To correctly SQL escape passwords, escaping \ first is
required. Then we need to escape ' in the password to
prevent it being treated as a end of SQL statement quote.

All escaping needs to use \, so we cannot be in
NO_BACKSLASH_ESCAPES sql_mode otherwise no escaping will
work.

Getting bash to escape the root password for its
internal uses and in a form that was recognized
by MariaDB reading the configuration file was
becoming exceptionally complicated, and unsuccessful.

As such we use the MYSQL_PWD environment variable
to contain the password when needed.

Occasionally the socket wasn't ready on a clean install
by the time docker_process_sql was called so allow
up to 5 seconds just in case.

example logs of this:
2021-03-16 00:41:21+00:00 [Note] [Entrypoint]: Waiting for server startup
2021-03-16  0:41:21 140680894076608 [Note] mysqld (mysqld 10.2.37-MariaDB-1:10.2.37+maria~bionic) starting as process 111 ...
2021-03-16 00:41:21+00:00 [Note] [Entrypoint]: Temporary server started.
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Mutexes and rw_locks use GCC atomic builtins
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Uses event mutexes
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Compressed tables use zlib 1.2.11
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Using Linux native AIO
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Number of pools: 1
2021-03-16  0:41:21 140680894076608 [Note] InnoDB: Using SSE2 crc32 instructions
ERROR 2002 (HY000): Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2)

Closes #183
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

7 participants