Created attachment 76017 [details] [review] Use env variable instead of --password argument for mysqldump koha-dump currently passes a password as a CLI argument to mysqldump, which results in the following warning: mysqldump: [Warning] Using a password on the command line interface can be insecure. Depending on how a server is configured, this results in emails being sent to the administrator every time the cron job runs. It's trivial to change this to pass an environment variable instead, so as not to trigger the warning.
Created attachment 76018 [details] [review] Use env variable instead of --password argument for mysqldump (without spurious white space changes)
Created attachment 76019 [details] [review] Use env variable instead of --password argument for mysqldump
We should create a .my.cnf, but this is much better than passing the password in the options.
(In reply to Jonathan Druart from comment #3) > We should create a .my.cnf, but this is much better than passing the > password in the options. I agree. The environment variable is documented here: https://dev.mysql.com/doc/refman/5.5/en/environment-variables.html Let me attempt a counter-patch.
Created attachment 79052 [details] [review] Bug 20929: Create and Use CNF file Rather than have hostname, username, and passwords on the command line, visible with a ps, use a CNF file so that mysqldump knows them more securely. TEST PLAN ---------- 1) start kohadevbox 2) reset_all 3) sudo systemctl deamon-reload 4) git bz apply 20929 -- choose just this patch 5) restart_all 6) perl /home/vagrant/misc4dev/cp_debian_files.pl 7) sudo koha-dump kohadev 8) ls -la /var/spool/koha/kohadev -- there is a koha-dump.cnf file with rw owner only perms 9) sudo cat /var/spool/koha/kohadev/koha-dump.cnf -- the host, username, and password should match what are in the koha-conf.xml file. 10) zcat the backup to make sure the backup has meaningful data in it. BONUS TEST: Change the kohadev DB password, make the koha-conf.xml file match, confirm your kohadev still works, and that a retest has the new username/password in the koha-dump.cnf file.
My counter patch does NOT apply on top of the first patch. Choose one, sign it off. Obsolete both.
I prefer the simplicity of using ENV. If I was to use the .cnf file I would touch and chmod before putting information there just in case something goes wrong before the chmod.
Created attachment 87356 [details] [review] Bug 20929: Create and Use CNF file Rather than have hostname, username, and passwords on the command line, visible with a ps, use a CNF file so that mysqldump knows them more securely. TEST PLAN ---------- 1) start kohadevbox 2) reset_all 3) sudo systemctl deamon-reload 4) git bz apply 20929 -- choose just this patch 5) restart_all 6) perl /home/vagrant/misc4dev/cp_debian_files.pl 7) sudo koha-dump kohadev 8) in another terminal window: while true; do ls -lah /var/spool/koha/kohadev; sleep 1; done -- there is a koha-dump.cnf file with rw owner only perms in one of the ls runs 9) zcat the backup to make sure the backup has meaningful data in it. BONUS TEST: Change the kohadev DB password, make the koha-conf.xml file match, confirm your kohadev still works, and that a retest of koha-dump still works. Test plan mostly written by Mark Tompsett.
This counter-counter patch creates and chmod the file before the dump begins, and removes it when we are done with the file. It will be new each day, in case the password changes on the database. If the file exists for some unforeseen reason, the contents will be overwritten (and it'll still be deleted at the end). The only thing I can think of that might make this better, would be to put the temp .cnf file in /etc/koha/sites/<instance>/ instead of in /var/spool. Having sent this patch in, now I'm wondering if we shouldn't just create this config file per instance when we koha-create, and keep it in /etc/koha/sites/<instance> I think I'll do a counter patch for that approach as well, I don't like having the file with passwords even temporarily in a directory that could potentially have dodgy permissions (because people [no one I know though] do strange things with backups sometimes). I suppose if they've got in there, they've got the DB anyway so eh, not sure it matters buuuut... /etc/ will be safer and more persistent than /var/spool anyway.
Just noting that using ENV is decidedly insecure, and we should definitely not go about this that way: https://dev.mysql.com/doc/refman/5.5/en/password-security-user.html
(In reply to Liz Rea from comment #9) > The only thing I can think of that might make this better, would be to put > the temp .cnf file in /etc/koha/sites/<instance>/ instead of in /var/spool. > > Having sent this patch in, now I'm wondering if we shouldn't just create > this config file per instance when we koha-create, and keep it in > /etc/koha/sites/<instance> If you plan to work on that, the status should perhaps not be "Needs signoff"? :-)