phpbu icon indicating copy to clipboard operation
phpbu copied to clipboard

Wrong MySQL username/password doesn't generate error

Open byalextran opened this issue 6 years ago • 18 comments

After getting my first configuration properly backing up files, I added email logging for errors only. In order to test the settings, I entered wrong MySQL credentials expecting an error email. However, instead of that, phpbu created a "successful" backup. Instead of the actual SQL dump, it was a zero-byte file.

Basically, I believe an error should be triggered in this case.

byalextran avatar Apr 12 '19 03:04 byalextran

I will have a look at this asap, in the meantime you can add a check, checking the size of the backup, if you have a zero byte file the check will fail.

sebastianfeldmann avatar Apr 12 '19 07:04 sebastianfeldmann

The problem here is that for efficiency if possible I pipe the mysqldump output directly to the compression like bzip2.

The default behavior of piping is that just the last exit code is relevant. In order to change this I have to `set -o pipefail;' so if any command in the pipeline fails the whole pipeline fails.

The thing is that not all shells support the pipefail feature. I would say the most common do, like bash, fish and zsh. But I'm still not sure if I should add it 🤔

sebastianfeldmann avatar Apr 12 '19 09:04 sebastianfeldmann

oh hey, i see you looked into it some more and determined pipefail would fail gracefully with shells that don't support it. awesome!

thanks for the quick response/fix.

byalextran avatar Apr 12 '19 15:04 byalextran

I just released version 5.2.0 with the new pipefail feature

sebastianfeldmann avatar Apr 12 '19 23:04 sebastianfeldmann

Thanks for reporting the issue :)

sebastianfeldmann avatar Apr 12 '19 23:04 sebastianfeldmann

so i just downloaded 5.2 and get this error running it with a configuration file that was working in 5.1.11.

phpbu 5.2.0 by Sebastian Feldmann and contributors.

  Warning - The configuration file did not pass validation!
  The following problems have been detected:

  Line 10:
  - Element 'crypt': This element is not expected. Expected is ( source ).

  Line 31:
  - Element 'crypt': This element is not expected. Expected is ( source ).

  Line 52:
  - Element 'crypt': This element is not expected. Expected is ( source ).

Time: 1 second, Memory: 6.00MB

Exception 'RuntimeException' with message 'Command failed:
  exit-code: 2
  message:   sh: 1: set: Illegal option -o pipefail

'
in phar:///path/to/phpbu.phar/lib/sf-cli/Command/Runner/Simple.php:57

Exception 'RuntimeException' with message 'Command failed:
  exit-code: 2
  message:   sh: 1: set: Illegal option -o pipefail

'
in phar:///path/to/phpbu.phar/lib/sf-cli/Command/Runner/Simple.php:57

Exception 'RuntimeException' with message 'Command failed:
  exit-code: 2
  message:   sh: 1: set: Illegal option -o pipefail

'
in phar:///path/to/phpbu.phar/lib/sf-cli/Command/Runner/Simple.php:57

FAILURE!
Backups: 3, failed Checks: 0, failed Crypts: 0, failed Syncs: 0, failed Cleanups: 0.

byalextran avatar Apr 13 '19 01:04 byalextran

if it helps, i'm using phpbu on a digitalocean machine running ubuntu 18.04.

byalextran avatar Apr 13 '19 01:04 byalextran

Can you paste you configuration. Off course without any passwords you might have in there

sebastianfeldmann avatar Apr 13 '19 07:04 sebastianfeldmann

These warnings...

Warning - The configuration file did not pass validation!
  The following problems have been detected:

  Line 10:
  - Element 'crypt': This element is not expected. Expected is ( source ).

  Line 31:
  - Element 'crypt': This element is not expected. Expected is ( source ).

  Line 52:
  - Element 'crypt': This element is not expected. Expected is ( source ).

...are a new 5.2 feature that validates your configuration against the xsd schema. It looks like you have a structure error in your configuration file.

sebastianfeldmann avatar Apr 13 '19 08:04 sebastianfeldmann

I deactivated the pipefail stuff in version 5.2.1 for now, seems my local testing didn't reproduce in the shell jungle wilderness as well as I expected.

sebastianfeldmann avatar Apr 13 '19 08:04 sebastianfeldmann

<?xml version="1.0" encoding="UTF-8"?>
<phpbu xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="http://schema.phpbu.de/5.1/phpbu.xsd">
  <adapters>
    <adapter type="dotenv" name="ENV">
      <option name="file" value=".env"/>
    </adapter>
  </adapters>
  <backups>
    <backup name="REDACTED-db">
      <crypt type="openssl">
        <option name="password" value="adapter:ENV:PHPBU_ENCRYPTION_KEY"/>
        <option name="algorithm" value="aes-256-cbc"/>
      </crypt>
      <source type="mysqldump">
        <option name="user" value="REDACTED"/>
        <option name="password" value="adapter:ENV:ATO_DB_PASSWORD"/>
      </source>
      <target dirname="backups/databases" filename="ato-%Y-%m-%d_%H%i.sql" compress="bzip2"/>
      <cleanup type="quantity">
        <option name="amount" value="7"/>
      </cleanup>
      <sync type="dropbox">
        <option name="token" value="adapter:ENV:DROPBOX_SECRET_KEY"/>
        <option name="path" value="databases"/>
        <option name="cleanup.type" value="quantity"/>
        <option name="cleanup.amount" value="7"/>
      </sync>
      <check type="SizeDiffPreviousPercent" value="10"/>
    </backup>
    <backup name="REDACTED-db">
      <crypt type="openssl">
        <option name="password" value="adapter:ENV:PHPBU_ENCRYPTION_KEY"/>
        <option name="algorithm" value="aes-256-cbc"/>
      </crypt>
      <source type="mysqldump">
        <option name="user" value="REDACTED"/>
        <option name="password" value="adapter:ENV:OAP_DB_PASSWORD"/>
      </source>
      <target dirname="backups/databases" filename="oap-%Y-%m-%d_%H%i.sql" compress="bzip2"/>
      <cleanup type="quantity">
        <option name="amount" value="7"/>
      </cleanup>
      <sync type="dropbox">
        <option name="token" value="adapter:ENV:DROPBOX_SECRET_KEY"/>
        <option name="path" value="databases"/>
        <option name="cleanup.type" value="quantity"/>
        <option name="cleanup.amount" value="7"/>
      </sync>
      <check type="SizeDiffPreviousPercent" value="10"/>
    </backup>
    <backup name="REDACTED-db">
      <crypt type="openssl">
        <option name="password" value="adapter:ENV:PHPBU_ENCRYPTION_KEY"/>
        <option name="algorithm" value="aes-256-cbc"/>
      </crypt>
      <source type="mysqldump">
        <option name="user" value="REDACTED"/>
        <option name="password" value="RSH_DB_PASSWORD"/>
      </source>
      <target dirname="backups/databases" filename="rsh-%Y-%m-%d_%H%i.sql" compress="bzip2"/>
      <cleanup type="quantity">
        <option name="amount" value="7"/>
      </cleanup>
      <sync type="dropbox">
        <option name="token" value="adapter:ENV:DROPBOX_SECRET_KEY"/>
        <option name="path" value="databases"/>
        <option name="cleanup.type" value="quantity"/>
        <option name="cleanup.amount" value="7"/>
      </sync>
      <check type="SizeDiffPreviousPercent" value="10"/>
    </backup>
  </backups>
  <logging>
    <log type="mail">
      <option name="sendOnlyOnError" value="true"/>
      <option name="transport" value="mail"/>
      <option name="recipients" value="REDACTED"/>
      <option name="sender.name" value="phpbu"/>
      <option name="sender.mail" value="REDACTED"/>
    </log>
  </logging>
</phpbu>

byalextran avatar Apr 13 '19 12:04 byalextran

It looks like you are using Bourne-Shell (sh) to execute phpbu. sh doesn't support set -e. Since you are for sure not the only one I think it's best to leave this feature deactivated by default and maybe add a way to activate it. Or maybe I try to detect if it works somehow and use it automatically if it is available. For know you should check if the backup has at least 1MB or something.

<check type="size" value="1M" />

sebastianfeldmann avatar Apr 13 '19 21:04 sebastianfeldmann

my server uses bash.

$ echo $0
-bash

although, i do see the log referencing sh. that's curious. is the way you're invoking commands forcing sh instead of the server's default?

also, after playing around more, it appears as if the schema definition requires elements in a specific order. i moved elements around to match the schema order and it validates properly.

but i would suggest updating the schema using xs:all instead of xs:sequence so that order doesn't matter.

byalextran avatar Apr 13 '19 21:04 byalextran

Changing the xsd to not force an order sounds reasonable :)

In order of execution, I'm not forcing anything special. I would expect php to call the default shell of the user it runs under.

sebastianfeldmann avatar Apr 13 '19 23:04 sebastianfeldmann

i dug in a little more.

it appears dash (which doesn't support pipefail) is the default shell on digitalocean servers. but for whatever reason, bash is the default shell when i log into my server.

however, when executing commands from php, sh is the shell used (which is symlinked to dash).

$ php -a
Interactive shell

php > echo `echo $0`;
sh
php >

given all that, i don't know what the best solution is for this.

is there a way to detect which shells are available and tell php to use one that supports pipefail and exclude that option if no supported shells are available?

FWIW, i think it's worth exploring a solution. login typos are fairly common. and if a user isn't vigilant about verifying their backup, they might be stuck with worthless backups when they need a valid backup the most.

pointing out login errors would also increase the authority of phpbu (i.e. when phpbu says a backup succeeded, you can be confident it actually did.)

anyway, your check solutions are good enough for me to move forward. but if you've got extra brain space, i think this is worth exploring more.

byalextran avatar Apr 16 '19 03:04 byalextran

You are absolutely right, I think the best way to do this is either trying to detect if pipefail is available and put out a warning if it is not, or defaulting to pipefail active with the option to deactivate it manually.

sebastianfeldmann avatar Apr 16 '19 05:04 sebastianfeldmann

What about trying to connect to database before making a backup, perform simple select, like SELECT NOW() and fail if we have invalid credentials?

Koc avatar Aug 19 '19 23:08 Koc

That could be done, but it wouldn't solve the problem completely. SELECT NOW() does not need the same permissions as mysqldump. You can connect to the DB and everything works but mysqldump still fails due to wrong permissions :/

sebastianfeldmann avatar Aug 20 '19 06:08 sebastianfeldmann