Deescalate needed SQL privileges for MySQL / Mariadb #2089
Conversation
This commit drops the need to be able to use the system database "mysql" which is not accessible to "ordinary" db users, normally. At least in recent MySQL / MariaDB version it is not necessary to select a database to issue a "CREATE DATABASE" command, and it is also not necessary to first switch away from the selected data-base in order to drop it. Another approach could be to use the `information_schema` DB instead of the `mysql` as this would not require super-user privileges. With this patch it is possible to use MySQL/MariaDB as backend with more or less ordinary privileges for the the connecting user -- except that it needs to have CREATE privileges. This is bad from a security point of view. With MySQL / MariaDB this can be "fixed" to some extend by granting CREATE on a table-name wildcard, for example: GRANT CREATE ON `gnucash\_%`.* TO 'gnucash'@'%' IDENTIFIED BY '********'; IMHO, a better approach would be to at least optionally stop using DROP / CREATE DATABASE and instead simple drop all the tables of a given database which would achieve the same goal but with less privilege escalation. Signed-off-by: Claus-Justus Heine <himself@claus-justus-heine.de>
|
Related Bugzilla bug: https://bugs.gnucash.org/show_bug.cgi?id=799610 |
| template <DbType Type> bool | ||
| drop_database(dbi_conn conn, const UriStrings& uri) | ||
| { | ||
| // The user may have CREATE privileges even if it does not have access to the "root_db" |
There was a problem hiding this comment.
Some might find referring to humans with "it" offensive. Let's use "they" instead.
| drop_database(dbi_conn conn, const UriStrings& uri) | ||
| { | ||
| // The user may have CREATE privileges even if it does not have access to the "root_db" | ||
| const char *root_db; |
There was a problem hiding this comment.
root_db is used only in the DBI_PGSQL block so its decl might as well go there too.
| else if (Type == DbType::DBI_MYSQL) | ||
| { | ||
| root_db = "mysql"; | ||
| // There is no need to first switch to another DB | ||
| } | ||
| else |
There was a problem hiding this comment.
You might as well do
else if (Type != DbType::DBI_MYSQL)
{
PERR...
and change the comment to "Postgres can't drop a database it's connected to".
|
There are also ongoing comments in the bug-tracker, see https://bugs.gnucash.org/show_bug.cgi?id=799610#c2 In short: I do not agree about your comment concerning the target audience of GNC, IMHO it is always a good idea to keep access rights as low as possible, even if physically just the same person sits in front of the computer (see sudo etc.). If really one should disconnect from the to-be-deleted database, then we also have the Thank you also for pointing out the issues above, of course I can cure those. But as your comment https://bugs.gnucash.org/show_bug.cgi?id=799610#c1 was rather in the direction that the proposed change is not needed at all I would rather wait with this until the question is settled that a modified PR could be accepted -- or could perhaps just be replaced by using the unprivileged DB information_schema instead of the super-precious system DB mysql. |
|
Connecting to This isn't the part of bug 799610 that I'm skeptical about: It's the part about the DBA creating the database outside of GnuCash that I don't think would be an improvement. I imagine/hope that Oracle has fixed up the unusual defaults like the Swedish character set that made creating a database in the original MySQL rather error prone, but it's still true that there's a lot less friction if GnuCash can create it. |
I could perfectly live with the solution to use the information_schema table. BTW, I think "information_schema" is "standard" (see https://en.wikipedia.org/wiki/Information_schema) which just means that there is a chance that this then could also be used with other engines. Concerning MySQL google's KI claims that it has been around since 5.0 (released around 2007). MariaDB was forked later, around 2009. One could also try to select information_schema and if that does not work, use the current scheme of using the mysql database.
My impression was just that it is not needed to give the GNC-user super-user privileges. And yes: small business and private uses, the admin is the same biological person as the user (which is also my use-case), but still I would rather reduce my chances to shoot myself into my own foot. Thank you for your comments; I'll generate a new PR which just tries to use information_schema with fallback to use the "mysql" system database. |
| } | ||
| PairVec options; | ||
| options.push_back(std::make_pair("dbname", dbname)); | ||
| try | ||
| { | ||
| set_options(conn, options); | ||
| } | ||
| catch (std::runtime_error& err) | ||
| { | ||
| set_error (ERR_BACKEND_SERVER_ERR); | ||
| return false; | ||
| PairVec options; | ||
| options.push_back(std::make_pair("dbname", dbname)); | ||
| try | ||
| { | ||
| set_options(conn, options); | ||
| } | ||
| catch (std::runtime_error& err) | ||
| { | ||
| set_error (ERR_BACKEND_SERVER_ERR); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This might be overcome by switching to information_schema for both backends, but it occurs to me that it MySQL and Postgresql have different behaviors it would be cleaner to have separate specializations.
There was a problem hiding this comment.
I suppose it would be "constructive" if I would first provide a PR with information_schema. If that works then it would be a very tiny change -- just connect to the info-schema instead of the privileged root dbs.
Question would also be: for MySQL: would it be sufficient to support MySQL-server versions released after 2007, roughly 18 years ago?
For Postgres: I seldom use it, should check for the support of information_schema .. todo.
There are two scenarios for a to-be-written PR with info-schema:
- minimal: just replace the name of the root-db with "information_schema"
- with fallback: try "information_schema", if that fails fall back to the current state of the affair.
There was a problem hiding this comment.
MySQL-server versions released after 2007, roughly 18 years ago?
Sure. GnuCash didn't support MySQL until 2011 (2.4.0 introduced it and was released 20 December 2010). But I'd be comfortable with whatever was oldest in the stable versions of the main Linux distros (Arch, Debian/Ubuntu, Fedora, Gentoo, OpenSuSE, and Slackware) as of 2020. We just have to document what it is.
For Postgres: I seldom use it, should check for the support of information_schema .. todo.
Note that the CI runners aren't set up to do this but there are a couple of environment variables that if set with valid credentials will run the DBI backend tests against MySQL and Postgresql.
2 participants
This commit drops the need to be able to use the system database "mysql" which is not accessible to "ordinary" db users, normally.
At least in recent MySQL / MariaDB version it is not necessary to select a database to issue a "CREATE DATABASE" command, and it is also not necessary to first switch away from the selected data-base in order to drop it.
Another approach could be to use the
information_schemaDB instead of themysqlas this would not require super-user privileges.With this patch it is possible to use MySQL/MariaDB as backend with more or less ordinary privileges for the the connecting user -- except that it needs to have CREATE privileges. This is bad from a security point of view. With MySQL / MariaDB this can be "fixed" to some extend by granting CREATE on a table-name wildcard, for example:
GRANT CREATE ON
gnucash\_%.* TO 'gnucash'@'%' IDENTIFIED BY '********';IMHO, a better approach would be to at least optionally stop using DROP / CREATE DATABASE and instead simple drop all the tables of a given database which would achieve the same goal but with less privilege escalation.