Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong MySQL username/password doesn't generate error #180

Open
byalextran opened this issue Apr 12, 2019 · 18 comments
Open

Wrong MySQL username/password doesn't generate error #180

byalextran opened this issue Apr 12, 2019 · 18 comments
Assignees

Comments

@byalextran
Copy link

@byalextran byalextran commented Apr 12, 2019

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.

@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 12, 2019

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
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 12, 2019

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 added a commit that referenced this issue Apr 12, 2019
In order to determine if a command in a command pipeline failed the
'set -o pipefail' option has to be set. Sadly not all shells support it.
But the most public ones do and since nothing breaks if it is not
available we should be fine using it.

Issue #180
@byalextran
Copy link
Author

@byalextran byalextran commented Apr 12, 2019

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 byalextran closed this Apr 12, 2019
@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 12, 2019

I just released version 5.2.0 with the new pipefail feature

@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 12, 2019

Thanks for reporting the issue :)

@byalextran
Copy link
Author

@byalextran byalextran commented Apr 13, 2019

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 byalextran reopened this Apr 13, 2019
@byalextran
Copy link
Author

@byalextran byalextran commented Apr 13, 2019

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

@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 13, 2019

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

@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 13, 2019

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
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 13, 2019

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.

@byalextran
Copy link
Author

@byalextran byalextran commented Apr 13, 2019

<?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>

@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 13, 2019

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" />

@byalextran
Copy link
Author

@byalextran byalextran commented Apr 13, 2019

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.

@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 13, 2019

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.

@byalextran
Copy link
Author

@byalextran byalextran commented Apr 16, 2019

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.

@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Apr 16, 2019

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.

@Koc
Copy link

@Koc Koc commented Aug 19, 2019

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

@sebastianfeldmann
Copy link
Owner

@sebastianfeldmann sebastianfeldmann commented Aug 20, 2019

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 sebastianfeldmann removed this from the 6.0 milestone Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants