Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

embedded mysql should listen by default on localhost #122

Open
jefimm opened this issue Jan 23, 2018 · 14 comments
Open

embedded mysql should listen by default on localhost #122

jefimm opened this issue Jan 23, 2018 · 14 comments

Comments

@jefimm
Copy link

jefimm commented Jan 23, 2018

The mysql opens ipv6 socket to the world, which is not secure and causes unnecessary warnings on MacOS.

from lsof output
IPv6 0x47e09a7bb638616f 0t0 TCP *:dyna-access (LISTEN)

Embedded mysql should listen on localhost only by default

@viliusl
Copy link
Collaborator

viliusl commented Jan 23, 2018

@jefimm yes, mysql does listen on * instead of explicit localhost mostly due to some issues I had/saw others having when Java on OSX in some occasions for localhost lookup returns you not 127.0.0.1, but instead your external interface. What happens is then mysqld is listening on 127.0.0.1 but java connection is being established on external IP and it fails like jruby/activerecord-jdbc-adapter#264

Now in wix-embedded-mysql I would rather have a less secure set-up, but with a higher chance of working than vice-versa. It's not a daemon that runs continuously, but just a thing with a short lifecycle. You can do a PR and add an option to force where mysqld should bind to.

@jefimm
Copy link
Author

jefimm commented Jan 23, 2018

If MysqldConfig would return the hostname to connect to (set to 127.0.0.1) it would be completely avoided

@viliusl
Copy link
Collaborator

viliusl commented Feb 6, 2018

@jefimm what I could do is to add option to MysqldConfig like:

MysqldConfig config = aMysqldConfig(v5_6_23)
  .withListenOn('*' || 'loclahost' || '127.0.0.1') // withHost(''), withBindTo('') - need to figure out the correct name
  .build();

Not sure if I would change default - current default '*' is most bulletproof, but whoever is worried about it could change it. Wdyt?

@jefimm
Copy link
Author

jefimm commented Feb 6, 2018

This would be a step in right direction
I would use .withListenOn('127.0.0.1') and use the same one for the jdbc url

@bobtiernay-okta
Copy link

Totally agree here. This is super annoying on Mac due to:

image

I don't want to disable my firewall, and it appears that each process is unique so it comes up all the time.

@bobtiernay-okta
Copy link

Is there a work around in the meantime?

@bobtiernay-okta
Copy link

@viliusl I don't mind having to set this via reflection, etc. if there is a workaround.

Your guidance is much appreciated. And thanks for such a great project!

@viliusl
Copy link
Collaborator

viliusl commented Apr 26, 2018

@bobtiernay-okta - I there was a suggestion for an extension of api to make it opt-in for now. PR's are welcome and it should be fairly simple addition. If it works as expected we could toggle it on by default in upcoming versions.

@bobtiernay-okta
Copy link

@viliusl I would be willing to take a look if you could point me to the right part of the code. Iirc, this is in my.conf. Not sure how to affect that from the code.

Thanks!

@viliusl
Copy link
Collaborator

viliusl commented Apr 26, 2018

@bobtiernay-okta so the way I see it there are several things that would need to change given

MysqldConfig config = aMysqldConfig(v5_6_23)
  .withListenOn('*' || 'loclahost' || '127.0.0.1')
  .build();

would be provided:

together with a test. test could be just reading server variable as in https://github.com/wix/wix-embedded-mysql/blob/master/wix-embedded-mysql/src/test/scala/com/wix/mysql/EmbeddedMysqlTest.scala#L30

hopefully I did not miss anything:)

@bobtiernay-okta
Copy link

Just a heads up, I was able to get this working without any changes:

    private EmbeddedMysql createMysqld() {
        MysqldConfig config = aMysqldConfig(v5_5_52)
                .withServerVariable("bind-address", "localhost")
                .build();

        return anEmbeddedMysql(config)
                .start();
    }

So perhaps we good already and just need to update the docs?

@viliusl
Copy link
Collaborator

viliusl commented Apr 28, 2018

@bobtiernay-okta :) once withServerVariable was added, you can actually tune most of stuff without adding explicit builder additions. Now I would rather add it to builder as there were at least 2 cases were users were bitten by current behavior. Adding to builder and maybe to defaults. You can leave it be of course and I will leave ticket open:)

@ddcprg
Copy link

ddcprg commented Nov 7, 2018

How did you manage to get around this?

https://github.com/wix/wix-embedded-mysql/blob/master/wix-embedded-mysql/src/main/java/com/wix/mysql/MysqlClient.java#L59

The client still uses localhost, I get the following exception when I try to start the embedded instance:

com.wix.mysql.exceptions.CommandFailedException: Command 'CREATE USER 'someuser'@'%' IDENTIFIED BY '';' on schema 'information_schema' failed with errCode '1' and output 'mysql: [Warning] Using a password on the command line interface can be insecure.
ERROR 2003 (HY000): Can't connect to MySQL server on 'localhost' (61)
'
        at com.wix.mysql.MysqlClient.execute(MysqlClient.java:74)
        at com.wix.mysql.MysqlClient.executeCommands(MysqlClient.java:49)
        at com.wix.mysql.EmbeddedMysql.<init>(EmbeddedMysql.java:48)
        at com.wix.mysql.EmbeddedMysql$Builder.start(EmbeddedMysql.java:169)

I can push a fix, should be a matter of fetching the bind address from the config. First add getters to com.wix.mysql.config.MysqldConfig.ServerVariable and then:

...
private void execute(String sql) {
...
  Process p = new ProcessBuilder(new String[]{
                    Paths.get(executable.getBaseDir().getAbsolutePath(), "bin", "mysql").toString(),
                    "--protocol=tcp",
                    format("--host=%s", getBindAddress()),
                    "--password=",
                    format("--default-character-set=%s", effectiveCharset.getCharset()),
                    format("--user=%s", SystemDefaults.USERNAME),
                    format("--port=%s", config.getPort()),
                    schemaName}).start();
....
}

private String getBindAddress() {
  for(ServerVariable sv : config.getServerVariables()) {
    if ("bind-address".equals(sv.getName()) && sv.getValue() != null) {
      return sv.getValue().toString();
    }
  }
  return "localhost"; 
}
...

I believe this only happens if the config contains a user

@atcollins06
Copy link

We used the workaround suggested by bobtiernay-okta above i.e. adding .withServerVariable("bind-address", "localhost") to the configuration and this worked as advertised to prevent the popups from happening on the Mac.

A side-effect seems to have cropped up though. Now, when the shutdown goes to terminate any mysqld instances, the spawned mysqladmin processes sometimes hang. It is intermittent and inconsistent how many processes hang. But, since we are running as part of our JUnit tests, this causes the tests to hang too.

We ultimately ended up writing our own shutdown hook to manually terminate all the leftover mysqld and mysqladmin processes. But, we'd like to avoid this if possible. Has anyone else seen this behavior and, if so, have you come up with a solution or better workaround? Interestingly, the code in stopUsingMysqldadmin in MysqldProcess has a comment saying that the code should include a timeout to wait on the mysqladmin process to return. Unfortunately, that has not been implemented yet so the mysqladmin process just hangs indefinitely.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants