@gwaggott opened this Pull Request on November 14th 2016 Contributor

Hi,

I require an SSL connection to MySQL in AWS RDS, so I'm creating a new pull request from the changes first proposed in #8049.

Can you let me know what further work is required to make it suitable for merging.

Regards,
Geoff

@mattab commented on December 27th 2016 Owner

Hi Geoff,

Thanks for the Pull request! It looks really good so far. Ideally we would be running the test on Travis CI. Maybe you could investigate how we could enable ssl connections on Travis CI so the integration test could run?

@mattab commented on January 21st 2017 Owner

Hi @gwaggott
This PR would be very useful to have. Maybe you could find some time to create an automated test on CI/Travis using the MySQL SSL connection?

@mattab commented on April 26th 2017 Owner

Thank you for this proposed pull request.

Because it was last updated more than one month ago, it is our policy to close pull requests opened for a long time without updates. If you would like to continue work on the pull request, please simply ping us to have it re-opened (after you have pushed a new commit).

We hope you understand this and we look forward to seeing an update from you on this pull request or another one!

Thanks.

@tsteur commented on January 25th 2018 Owner

I think this PR would be actually good to merge and can't really "break" anything and users confirm it works. Be good to have SSL support for mysql?

@cah-andrewfitzgerald commented on January 25th 2018

copy-pasting my response from #7039


We're currently running 2.16.1 in a Docker container.
To confirm the PR works, I:
* downloaded the 2.16.1 release from builds.matomo.org
* manually copied the PR changes to the local 2.16.1 release
* updated the dockerfile to point to the local release instead of pulling from builds.matomo.org
* updated the config in our dockerfile to use the new SSL config options
* deployed the docker container and successfully connected to an AWS RDS MySQL instance which is configured to only allow SSL connections
* verified that piwik had no error messages and was able to successfully query the database
@Fitzoh commented on February 3rd 2018 Contributor

@tsteur/@mattab any updates on this one?

@mattab commented on February 4th 2018 Owner

As @tsteur mentions, in this particular case of a security improvement, it's still better to have the feature un-tested (but confirmed as working by a few users) rather than not have it at all.

We'll definitely take a look at it (Assigned it to 3.4.0 for now)

@cah-andrewfitzgerald commented on February 12th 2018

So I need to backtrack a little bit on my initial statement about this working.

It's a little difficult for me to troubleshoot, as I'm not particularly familiar with the piwik/matomo internals, and we're working off an older version (2.16.1), but is it possible that piwik/matomo is creating multiple mysql connections, where one is using ssl for the db connection and the other isn't?

We're observing the following behavior:

The core of piwik/matomo works fine. dashboards load, I can view visitor logs, etc.

A couple widgets that are dynamically updated are failing to load, and the root cause appears to be a mysql auth issue. These are items like the real time visitor count. The only relevant logs I'm able to find in our dockerized container are from nginx logs:

2018/02/12 02:53:58 [error] 71<a href='/71'>#71</a>: *6857 FastCGI sent in stderr: "PHP message: Error in Piwik (tracker): SQLSTATE[28000] [1045] Access denied for user 'mysqluser'@'10.some.ip' (using password: YES)" while reading response header from upstream, client: some.ip, server: *.myurl.com, request: "GET /piwik.php?e_c=somelongquerystream HTTP/1.1", upstream: "fastcgi://unix:/var/run/php5-fpm.sock:", host: "analytics.myhost.com", referrer: "https://myapplication.com/"
@cah-andrewfitzgerald commented on February 16th 2018

Figured it out, Tracker does in fact set up a separate database connection.
I've got a patch for the Tracker PDO mysql adapter, will work on getting a PR open for it.

@mattab commented on March 20th 2018 Owner

Hi @cah-andrewfitzgerald
We're planning to review the PR very soon. Would you maybe be able to open the PR for the tracker SSL support so we can review at the same time?

@cah-andrewfitzgerald commented on March 20th 2018

Sorry for the delay @mattab, I've got a reminder set to take a look after work today

@Fitzoh commented on March 21st 2018 Contributor

@mattab see #12631 for tracker PDO changes

@diosmosis commented on March 22nd 2018 Member

@mattab tested locally & tweaked the PR a bit (included a new option so it'll work w/ self-signed certificates (for testing)). good to merge

@diosmosis commented on March 22nd 2018 Member

Created an issue for writing the FAQ: https://github.com/matomo-org/matomo/issues/12636

@Fitzoh commented on March 22nd 2018 Contributor

Probably an unnecessary reminder, but this should not be released without merging #12631 (as Tracker requests will fail if the db requires SSL).

@diosmosis commented on March 25th 2018 Member

@mattab can we merge this PR?

This Pull Request was closed on April 2nd 2018
Powered by GitHub Issue Mirror