MySQL must not use /tmp

Thank you for taking the time to report this bug and helping to make Ubuntu better. The issue that you reported is one that should be reproducible with the live environment of the Desktop CD of the development release - Karmic Koala. It would help us greatly if you could test with it so we can work on getting it fixed in the next release of Ubuntu. You can find out more about the development release at http://www.ubuntu.com/testing/. Thanks again and we appreciate your help.

The default my.cnf still lists /tmp as temp directory

Please use In Progress if you're actually working on providing a fix and assign yourself to the bug.

Bug #578922 discusses security implications of having MySQL use /tmp as its temporary directory, and I have redirected that part of the discussion of that bug here. Basically, if MySQL can write to a world-readable directory, then an SQL injection in a web application could write out a file to later be included in that web application for arbitrary code execution. If you are going to move the temporary directory, would it be possible to either make that directory 700 or 750 and if not set the mysql umask to 077 or 027?

I like it. The FILE permission has caused me quite a bit of pain in the past, and quite a few shops just run mysql with 'ALL PRIVILEGES ON *.*' for their app users because they don't want to deal with grants or don't understand.

The only issue would be that if users actually depend on the ability to export files from mysql in this way, they will have to grant the reading processes access to the mysql group, or change /var/tmp/mysql to an alternate group (the files are always created 666).

I just tested this and it works fine without the user-tmp abstraction, by setting tmpdir=/var/tmp/mysql and making sure the directory exists in the upstart script.

mysql> select * into outfile '/var/tmp/user.txt' from mysql.user;
ERROR 1 (HY000): Can't create/write to file '/var/tmp/user.txt' (Errcode: 13)
mysql> select * into outfile '/var/tmp/mysql/user.txt' from mysql.user;
Query OK, 5 rows affected (0.00 sec)

mysql> select * into outfile '/tmp/user.txt' from mysql.user;
ERROR 1 (HY000): Can't create/write to file '/tmp/user.txt' (Errcode: 13)
mysql>

I'm hesitant to break the FILE privilege's basic assumptions, but at the same time, I'd rather restrict that functionality and close a door for common exploits.

Can anyone else comment on that?

Alright, absent comment I've erred on the side of security. Merge proposal updated to reflect the apparmor/upstart changes necessary to lock down tmpdir to /var/tmp/mysql.

One thing that I'm seeing that may need to be documented, is that this breaks mysql-testsuite in its default configuration.

Because we can no longer let /usr/sbin/mysqld write to /tmp at will, the test suite won't start without some coaxing. This works

sudo -u mysql /usr/lib/mysql-test/mysql-test-run.pl --vardir=/var/tmp/mysql

Or, if we add this to the apparmor profile:

  # for the testing suite
  owner @{HOME}/tmp/** rwkl,
  owner @{HOME}/tmp/ rw,

Then we can start it as any user with just

/usr/lib/mysql-test/mysql-test-run.pl --vardir=$HOME/tmp

The mysql user runs with $HOME == /nonexistent, so this should be safe.

Thanks for your work on this Clint; it is much appreciated.

I'd prefer not to adjust the profile for the test suite. @{HOME} in an AppArmor profile does not expand to the process' uid's HOME, but the value of the @{HOME} variable as set in /etc/apparmor.d/tunables/home. As such, this expands to:
  owner /home/you/tmp/...
  owner /home/me/tmp/...
  owner /home/her/tmp/...
  ...

While with 'owner' match, it should generally be ok since /home/you/tmp shouldn't be owned by the mysql user, it does open an avenue of attack for people running mysqld as themselves and is IMHO unnecessary.

As for documenting, the best course IMO is patch /usr/lib/mysql-test/mysql-test-run.pl itself to first do a quick test to see if --vardir is writable, and if not, give a helpful message about AppArmor possibly blocking it, suggest to use --vardir=/var/tmp/mysql instead, and exit with error.

We should of course also adjust the test script in lp:qa-regression-testing to use --vardir=/var/tmp/mysql, since it is now using the testsuite.

I ran the mysql-testsuite with the version of the package in the merge proposal.

Attaching the results. Looks good to go, though the test suite must be run with both vardir and tmpdir set to directories underneath /var/tmp/mysql, because the mysqld that is spawned must write files somewhere. This is the command I used:

cd /usr/lib/mysql-testsuite && sudo -u mysql mkdir -p /var/tmp/mysql/test && sudo -u mysql /usr/lib/mysql-testsuite/mysql-test-run.pl --vardir=/var/tmp/mysql/test --tmpdir=/var/tmp/mysql/test/tmp2

Thanks, Clint! I've fixed up the test script in lp:qa-regression-testing to take all this into account.

I just had a thought, should we check for un-applied data files in /tmp when upgrading, and move them to /var/tmp/mysql ? If somebody upgrades an active slave, that could be a huge problem.

MySQL would be stopped and restarted, would that take care of the /tmp backlog ?
If not, then it could indeed be an issue.

Moving files from /tmp to /var/tmp/mysql would need extra care (to not inject rogue files). Maybe asking MySQL to commit /tmp backlog before upgrade (if that makes sense) is the safest ?

From http://dev.mysql.com/doc/refman/5.1/en/temporary-files.html

"A replication slave needs some of its temporary files to survive a machine restart so that it can replicate temporary tables or LOAD DATA INFILE operations. If files in the temporary file directory are lost when the server restarts, replication fails."

Because LOAD DATA INFILE checks for the killed flag while it is loading rows, it may stop mid-transaction, and leave the file in tmpdir. When the slave is started again, the file will still be there and the LOAD DATA INFILE command re-issued.

What I don't know is whether the binlog embeds @tmpdir/filename or /tmp/filename ... if its the former, then this is a problem, if its the latter, then we actually don't need to do it.

Assuming @tmpdir, its tricky but I think we can figure this out..

--preinst--
if there is a master.info in datadir (must be present on all slaves)
  figure out what old_tmpdir is
  stop mysql
  if $old_tmpdir != $new_tmpdir
    if find $old_tmpdir -user mysql -type f ; then
      LOW priority debconf question to ask whether to move the files to new tmpdir, with answer == Y?

--postinst--
if debconf answer to move files from old tmpdir was Y
  mv files to new tmpdir
start mysql

Another, simpler approach would be to just check for a running master in the preinst, and issue a warning advising the user to run 'SLAVE STOP' and ensure all files are cleared out from tmpdir before continuing.

One mitigating factor about this is that if we don't do anything, the slave will fail to start after the upgrade with a missing file in tmpdir, so the admin can resolve it fairly easily by manually moving the file at that time. Given that the changelog mentions the tmpdir change, and refers to this bug report, this isn't the worst thing in the world.

I have found at least two instances where the change to restrict mysqld from writing to dirs outside /var/tmp/mysql breaks automated build tests that spawn a mysqld instance to run tests.

Given that, we can either

a) Modify build tests to copy /usr/sbin/mysqld to the local build dir to run the tests

or

b) relax this requirement to allow /tmp access again

What about this one:
c) adjust the automated build tests to use /var/tmp/mysql instead of /tmp

Hi Jamie, I did think of that, though I'm glad you brought it up as I should have mentioned it.

The build portion of a package must be runnable as a non-root user.

The whole point of using /var/tmp/mysql and not the user-tmp abstraction is that it is only accessible by the mysql user. So we can't mkdir or write files or anything as a non-root non-mysql user in that directory. We can't rely on any privilege elevation of any kind in the build step.

It shouldn't be hard to fix all of the affected builds. We might even want to provide a package just for testing that puts mysqld somewhere else.

The two places that I know of that run /usr/sbin/mysqld for their own tests (php5, and the version of libdbi-drivers in debian) run it as part of the build step.

Digging through the package listings, I only see two others that build-depend on mysql-server:

akonadi

and

digikam

I haven't reviewed their build process to see why they depend on mysql-server, but I think thats a reasonably sure way of knowing that those are the only packages affected.

I'm slightly confused. These build tests are using an installed, live mysql server and create temporary files somewhere in such a way that mysqld needs to read them?

If that is the case, then maybe we could add to the mysqld profile something like:
  owner /var/tmp/mysql/** rwkl,
  owner /var/tmp/mysql/* rw,
  /var/tmp/mysql-buildtests/** r,
  /var/tmp/mysql-buildtests/ r,

Then do:
# mkdir -m 0770 /var/tmp/mysql
# chown mysql:mysql
# mkdir -m 1113 /var/tmp/mysql-buildtests
# chown mysql:mysql /var/tmp/mysql-buildtests

This should allow any user to write to anything in /var/tmp/mysql-buildtests, after which testsuites can invoke mysqld with the proper arguments for tmpdir being /var/tmp/mysql and reading specific files in /var/tmp/mysql-buildtests. The idea is that under normal circumstances, mysqld would ignore /var/tmp/mysql-buildtests/, but in the face of an attack both DAC and AppArmor prevent writing to /var/tmp/mysql-buildtests. We use the weird '1113' permissions on /var/tmp/mysql-buildtests to create a sticky directory to allow 'other' to create files in the directory, but mysql can only read from this directory. DAC prevents regular users from reading /var/tmp/mysql. This should mitigate bug #578922 while allowing for test suites to run.

It would be great if others could review my suggestion.

err... this:
# chown mysql:mysql

should have been:
# chown mysql:mysql /var/tmp/mysql

No! These build tests start their own private mysqld as the user running the build step, not as mysql.

Further, that mysqld needs to be able to write to the directory during these tests, so the readonly restriction would prevent that.

To be fair, if they build-depend on mysql-server-core, and not mysql-server, then the apparmor profile won't get loaded on cleanroom builds, and so there is no need for any of this.

But its fairly simply to rebase mysqld and get it to run outside the apparmor profile for private build tests.

Here is my proposed fix for the PHP issue for instance:

https://code.edge.launchpad.net/~clint-fewbar/ubuntu/maverick/php5/fix-mysql-tests-apparmor/+merge/35562

/var/tmp/mysql-buildtests was intended to be the location for mysql to read from, but still use --tmpdir /var/tmp/mysql. Based on your comment, this isn't going to work anyway because of the permissions on /var/tmp/mysql and running as non-mysql. Additionally, in discussing this with mdeslaur, I've come to the conclusion that adjusting the profile for the testsuite is the wrong approach anyway since it does cover the chained attack of sql injection -> file -> include in php, but does not cover the other way around. Therefore, I don't see any other option than to use a mysqld not confined by AppArmor for these private build tests.

This said, it is vitally important that the QRT test-mysql.py is run before upload.

Ok, did you want me to post the results of that here, or was that more of an admonition that they should be run in general?

From what I see in the code base there, they run as root so /var/tmp/mysql should be fine.

Sorry for not being clear. This was just strong encouragement that we need to do it in general so that we can catch profiling errors before users see them.

Proposed branch fails to build, so back to the drawing board :)

So, now that some time has passed and I've been able to think about this some, I think we should relax the apparmor profile back to the regulr tmpdir abstraction.

YES it would be more secure to be able to say you can't create files even if you want to because apparmor is restricting users.

But its going to break *a lot* of peoples' applications. LOAD DATA INFILE is a really terrible way to get data into mysql (at least use LOCAL!) but people still do it. SELECT INTO OUTFILE is also a really flawed way to extract data from mysql, but people do it anyway.

The added problem of having to change all test suites that start their own mysqld instance, just so they can create their isolated test database in the system tmpdir, raises a red flag. These test suites should strive to run things as close to reality as possible. If we copy mysqld out of its normal location, we're getting around *all* of the apparmor protections, and so we're bypassing some other things that might go wrong when run against the real mysqld.

So, how about we just move tmpdir to /var/tmp/mysql, but leave the apparmor profile as is?

I think moving tmpdir to /var/tmp/mysql is fine (though remember /var/tmp is not automatically cleaned). This directory would then not be sticky and lack world writes? Would it also be 0700? If it is 0755 then mysql's umask should be 077.