yugabyte / yugabyte-db

YugabyteDB - the cloud native distributed SQL database for mission-critical applications.
https://www.yugabyte.com
Other
8.97k stars 1.07k forks source link

[YSQL] CREATE ROLE WITH LOGIN PASSWORD logs unredacted password on error #13401

Open jameshartig opened 2 years ago

jameshartig commented 2 years ago

Jira Link: DB-3032

Description

Running yb in Docker like:

docker run --rm -it --name yb yugabytedb/yugabyte:2.12.9.0-b3 bash -c "bin/yugabyted start --tserver_flags 'ysql_log_statement=ddl' --daemon=false"

If you run the following command (yes, twice):

docker exec -it yb ysqlsh -c "CREATE ROLE test WITH LOGIN PASSWORD 'foobar';"
docker exec -it yb ysqlsh -c "CREATE ROLE test WITH LOGIN PASSWORD 'foobar';"

then in the logs you'll see:

docker exec -it yb bash -c "cat /root/var/logs/tserver/postgresql-* | grep 'CREATE ROLE'"

2022-07-22 13:47:48.883 UTC [286] LOG:  statement: CREATE ROLE test WITH LOGIN PASSWORD <REDACTED>
2022-07-22 13:48:18.244 UTC [330] LOG:  statement: CREATE ROLE test WITH LOGIN PASSWORD <REDACTED>
2022-07-22 13:48:18.251 UTC [330] STATEMENT:  CREATE ROLE test WITH LOGIN PASSWORD 'foobar';

You shouldn't see foobar in the logs.

tedyu commented 2 years ago

Revelation of original password was due to elog() unconditionally logging the SQL statement for the second attempt:

    if (OidIsValid(get_role_oid(stmt->role, true)))
        ereport(ERROR,
                (errcode(ERRCODE_DUPLICATE_OBJECT),
                 errmsg("role \"%s\" already exists",
                        stmt->role)));

If we want to avoid this scenario, we should decide which SQL statements need to be redacted. From quick check, there are 81 places where ERRCODE_DUPLICATE_OBJECT is raised.

One option is to redact SQL statement with this pattern PASSWORD 'xx'

tedyu commented 2 years ago

Looks like there have been proposals in PG community for redacting the password, all of which got rejected.

Suggestion is to keep server log secure.