Skip to content

[4.0] No TLS transfer encryption for database connections if localhost#26889

Merged
HLeithner merged 8 commits intojoomla:4.0-devfrom
richard67:4.0-dev-db-connection-encryptions-not-localhost
Nov 28, 2019
Merged

[4.0] No TLS transfer encryption for database connections if localhost#26889
HLeithner merged 8 commits intojoomla:4.0-devfrom
richard67:4.0-dev-db-connection-encryptions-not-localhost

Conversation

@richard67
Copy link
Member

@richard67 richard67 commented Nov 1, 2019

Pull Request for Issue # .

Follow-up to PR #26375 .

Summary of Changes

With PR #26375 the possibility to use encrypted database connections was added to the CMS, and with PR #26888 it would be added to the installation process, too, if that is accepted.

Again thanks to @andrepereiradasilva for the implementation of these new features.

Because database connections default to socket connections in case if the database server is 'localhost' (which is of course more performant that a TCP/IP connection like e.g. to '127.0.0.1' or '::1'), and encryption is not possible with socket connections, this PR here changes the showon condition for the related fields in the server section of the Global Configuration form so that they are hidden when database server is 'localhost', and it adds an onchange JS to clear the fields when changing database server from something else to 'localhost'.

The JS added by this PR has the advantage that it can be used in future for other onchange events, too, e.g. if we want to clear the FTP related fields when disabling the FTP layer.

Testing Instructions

  1. On an installation of current 4.0-dev or last nightly build, go to Global Configuration, tab "Server", section "Database".
  2. If field "Host" has a value different to "localhost", change it to "localhost".
  3. Select option "Two-way encryption" of field "Connection Encryption".
  4. Enter some values for the text fields related to database connection encryption as shown below in section "Actual result" in the screen shot.
    Result: See section "Actual result" below.
  5. Click the "Close" button in order to leave the form without saving the changes.
  6. Apply the patch of this PR e.g. with Patch Tester 3.0.0 Beta 4 or Patch Tester 4.0.0 Beta 2 (if the latter works for you).
  7. If not having used Patch Tester 4.0.0 Beta 2 with the CI option enabled:
    Run npm run build:js.
  8. Go again to Global Configuration, tab "Server", section "Database".
  9. If field "Host" has a value different to "localhost", change it to "localhost".
    Result: See screenshot 1 in section "Expected result" below.
  10. Change the host to something different to "localhost".
    Result: See screenshot 2 in section "Expected result" below.
  11. Select option "Two-way encryption" of field "Connection Encryption".
  12. Enter some values for the text fields related to database connection encryption.
    Result: See screenshot 3 in section "Expected result" below.
  13. Change the host back to "localhost".
    Result: The options for database connection encryption are hidden again.
  14. Change again the host to something different to "localhost".
    Result: See screenshot 4 in section "Expected result" below.
  15. Click the "Close" button in order to leave the form without saving the changes.

Expected result

  1. Database connection encryption related options are hidden if database host is "localhost".
    j4-db-encryption-localhost-after-patch_2
  2. The "Connection Encryption" field is shown if database host is different to "localhost".
    j4-db-encryption-localhost-after-patch_3
  3. If "Two-way encryption" is selected in field "Connection Encryption" you can enter values in the text fields shown in this case.
    j4-db-encryption-localhost-after-patch_1
  4. If the hostname has been changed to "localhost", the options for database connection encryption are reset.
    j4-db-encryption-localhost-after-patch_4

Actual result

When the database host (field "Host") is "localhost", fields related to database connection encryption are shown depending on the value selected in field "Connection Envrytion". If "Two-way encryption" is selected, you can enter values in the text fields shown in this case.

j4-db-encryption-localhost-before-patch

Documentation Changes Required

See PR #26375 .

@richard67
Copy link
Member Author

richard67 commented Nov 1, 2019

Potential testers please wait with testing until title does not contain "[WiP]" anymore and status on GitHub is not draft anymore.

@richard67 richard67 changed the title [4.0] [WiP] No tls transfer encryption for database connections if localhost [4.0] [WiP] No TLS transfer encryption for database connections if localhost Nov 1, 2019
@joomla-cms-bot joomla-cms-bot added the NPM Resource Changed This Pull Request can't be tested by Patchtester label Nov 17, 2019
@richard67 richard67 changed the title [4.0] [WiP] No TLS transfer encryption for database connections if localhost [4.0] No TLS transfer encryption for database connections if localhost Nov 17, 2019
@richard67 richard67 marked this pull request as ready for review November 17, 2019 15:49
@richard67 richard67 requested a review from wilsonge as a code owner November 17, 2019 15:49
@alikon
Copy link
Contributor

alikon commented Nov 24, 2019

I have tested this item ✅ successfully on 6f293c0


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

@SharkyKZ
Copy link
Contributor

I have tested this item ✅ successfully on ed138ce


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

@richard67
Copy link
Member Author

@alikon @SharkyKZ Thanks for testing. Not sure if we should take it RTC. There is a review by @wilsonge requested automatically because he is code owner. What do you think?

@SharkyKZ
Copy link
Contributor

RTC.


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 28, 2019
@HLeithner
Copy link
Member

I tested this with mariadb, this pr removes a set ca path which could be used for server verification mentioned in #26888

I merged this anyway and my 2 points from #26888 should be addressed soon.

Thanks for your work and tests.

@HLeithner HLeithner merged commit 49e5785 into joomla:4.0-dev Nov 28, 2019
@HLeithner HLeithner added this to the Joomla 4.0 milestone Nov 28, 2019
@richard67
Copy link
Member Author

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants