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

[3.x] Allow optional port numbers in remote database security check of installation

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Jun 10, 2020

Pull Request for Issue #29519 .

Summary of Changes

This Pull Request (PR) changes the special security check when using a remote database server to allow port numbers to be used in the host name.

The database drivers already seem to support that at least for hostnames and IPv4 addresses.

With IPv6 I'm not sure yet (the address should be enclosed in square brackets to distinguish the colon to separate the port from the colons in the IPv6 address).

Testing Instructions

Requirements

  • You need a clean, current staging or 3.9.19 or latest 3.9 nightly build.
  • You need a local database server, i.e. "localhost"/"127.0.0.1"/"::1".
  • You may test with any kind of database type which can be used.
    If you have MySQL or MariaDB, plese test both the "MySQLi" and the "MySQL (PDO)" type.
  • Please report back with which kind of database server and type you have tested.

Test Execution

  1. On a clean, current staging or 3.9.19 or latest 3.9 nightly build, apply the patch for this PR.

  2. Make a new installation.

  3. When coming to the database part, fill in correct data and use either "localhost", "127.0.0.1" or "::1" (the latter only if IPv6 works) as database host, together with the port number on which the database server works, which normally is 3306 for MySQL or MariaDB and 5432 for PostgreSQL, i.e. use as database host

  • for MySQL and MariaDB: "localhost:3306" or "127.0.0.1:3306" or "[::1]:3306"
  • for PostgreSQL: "localhost:5432" or "127.0.0.1:5432" or "[::1]:5432"
    or different ports if your servers are set up not to use the standard ports.
  1. Start the installation.
    Result: There is no extra security check using a temporary file, the installation works as usual when using a local database host.

  2. Clear the session cookie or close the browser window so the next test starts with a new session.

  3. Repeat the previous steps 1 to 4, i.e. make again a new installation using another empty database or creating another nerw one, but this time don't use a port number, and in case of IPv6 leave away the square brackets.
    Result: There is again no extra security check using a temporary file, the installation works.

  4. Clear the session cookie or close the browser window so the next test starts with a new session.

  5. Repeat step 6, i.e. make again a new installation using another empty database or creating another nerw one, but this time use something else than "localhost" or "127.0.0.1"or "::1", e.g. use the real computer name of that server and make sure it can be resolved to an IP address e.g. by adding it to the local hosts file ("c:\windows\system32\drivers\etc\hosts" on Windows or "/etc/hosts" on Linux). It's ok if this resolves to 127.0.0.1, too, it just needs a different name than the ones listed before. Use a port number like in the first installation.
    Result: This time there is extra security check using a temporary file, the installation works.

  6. Clear the session cookie or close the browser window so the next test starts with a new session.

  7. Repeat step 8, but this time don't use a port number.
    Result: Again there is extra security check using a temporary file, the installation works.

Expected result

No security check when using "localhost:1234", "127.0.0.1:1234" or "[::1]:1234" as database host, with "1234" being the port number on which that server works.

No security check when using "localhost", "127.0.0.1" or "::1" as database host.

Security check when using something else than "localhost", "127.0.0.1" or "::1" with or without port number as database host.

Actual result

Security check when using "localhost:1234", "127.0.0.1:1234" or "[::1]:1234" as database host, with "1234" being the port number on which that server works, as if it was a remote host.

No security check when using "localhost", "127.0.0.1" or "::1" as database host.

Security check when using something else than "localhost", "127.0.0.1" or "::1" with or without port number as database host.

Documentation Changes Required

Don't think so, but am not 100% sure.

@toivo
Copy link
Contributor

toivo commented Jun 11, 2020

I have tested this item successfully on 03676bd

Tested successfully in Joomla 3.9.20-dev from 11 May. Using IPv4, no PostgreSQL.
Environment: Wampserver 3.2.2 Apache 2.4.43a MySQL 8.0.20 MariaDB 10.4.13 PHP 7.4.7
MySQL: MySQLi, MySQL (PDO) localhost:3308, localhost
MariaDB: localhost, localhost:3306, databaseserver:3306, databaseserver


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29567.

@richard67
Copy link
Member Author

richard67 commented Jun 12, 2020

@Quy Could you test this one, too? It's does the same thing at the same time as #29568 does for J4 (only at a different place because the installation has been restructured in J4).

@Quy
Copy link

Quy commented Jun 12, 2020

I have tested this item successfully on 03676bd


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29567.

@Quy Quy removed the PR-staging label Jun 12, 2020
@Quy
Copy link

Quy commented Jun 12, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29567.

@joomla-cms-bot joomla-cms-bot added the RTC label Jun 12, 2020
@richard67
Copy link
Member Author

richard67 commented Jun 12, 2020

Thanks guys for testing.

@zero-24 zero-24 added this to the Joomla! 3.9.20 milestone Jun 13, 2020
@PhilETaylor

This comment was marked as disruptive content.

@PhilETaylor

This comment was marked as disruptive content.

@richard67
Copy link
Member Author

richard67 commented Jun 25, 2020

If Im right with a quick glance, this restricts the mysql port to 4 digits right ?

Wrong. It is "[1-9]{1}[0-9]{0,4}", which means at least one digit between 1 and 9 followed by zero to four digits from zero to nine, i.e. it can have 1 to 5 digts without leading zero. I did not limit it to high ports only, that's why I allow also less than 4 digits.

@PhilETaylor

This comment was marked as disruptive content.

@zero-24 zero-24 merged commit 2c42a30 into joomla:staging Jun 28, 2020
6 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC label Jun 28, 2020
@zero-24
Copy link
Member

zero-24 commented Jun 28, 2020

Merged thanks

@richard67 richard67 deleted the staging-installation-database-localhost-allow-ports branch Jun 28, 2020
Reconix pushed a commit to Reconix/joomla-cms that referenced this issue Aug 31, 2020
…f installation (joomla#29567)

* Allow ports for the remote database security check

* Fix my silly typo from the previous commit
Reconix added a commit to Reconix/joomla-cms that referenced this issue Aug 31, 2020
Reconix added a commit to Reconix/joomla-cms that referenced this issue Aug 31, 2020
Reconix added a commit to Reconix/joomla-cms that referenced this issue Aug 31, 2020
…security check of installation (joomla#29567)""

This reverts commit 4816d9a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants