Skip to content

[4.0] TLS encryption for database connections: Correct language texts and showon for fields#27320

Closed
richard67 wants to merge 6 commits intojoomla:4.0-devfrom
richard67:4.0-dev-tls-encrypt-db-connections-mod-1
Closed

[4.0] TLS encryption for database connections: Correct language texts and showon for fields#27320
richard67 wants to merge 6 commits intojoomla:4.0-devfrom
richard67:4.0-dev-tls-encrypt-db-connections-mod-1

Conversation

@richard67
Copy link
Member

@richard67 richard67 commented Dec 19, 2019

Pull Request for #26888 (comment).

Summary of Changes

This PR fixes 2 issues mentioned in comment #26888 (comment) in Global Configuration, section Database Configuration:

  • For field "Connection Encryption", change option texts for 1-way and 2-way from "... encryption" to "... authentication" so it is more clear that "1-way" and "2-way" refers to authentication.
    See e.g. here for an explanation: https://mariadb.com/kb/en/library/securing-connections-for-client-and-server/.
    1-way means that only the database server is authenticated with a certificate, and 2-way means database server and client are authenticated.
  • Change showon condition for the fields "Path to CA File" and "Path to CA folder" so the fields are shown in case if server certificate verification is switched on and authentication is 1-way or 2-way, and not to show them when server certificate verification is switched off.
    Right now the fields are only shown if authentication is 2-way, regardless of the server certificate verification. This is wrong because the CA certificate is only used by the client to verify the server certificate.
    What is not changed by this PR is that field "Path to CA folder" is not shown if database type is PostgreSQL.

Testing Instructions

Please wait with testing. I will make some changes in the next day or make a new PR.

Code review, or:

  1. On an installation of current 4.0-dev or last nightly build without this PR applied, go to Global Configuration, tab "Server", section "Database".
  2. If field "Host" has value "localhost", change it to something different, e.g. "127.0.0.1", so the fields for the database connection encryption options are shown. For the reasson of this see PR [4.0] No TLS transfer encryption for database connections if localhost #26889 .
  3. Select value "One-way encryption" for field "Connection Encryption", toggle value of field "Verify Server Certificate" and watch if additional fields are displayed after changes.
    Result: No additional fields are displayed after the value of field "Verify Server Certificate" has been changed.
  4. Select value "Two-way encryption" for field "Connection Encryption" and check which additional fields are displayed.
    Result: Additional fields are displayed after the value of field "Connection Encryption" has been changed to "Two-way encryption".
  5. Change the database type several times so you have checked all 3 types "MySQLi", "MySQL (PDO)" and "PostgreSQL (PDO)", or at least all types which are available in your testing environment, and for each type toggle the value of field "Verify Server Certificate".
    Result: When the value of field "Connection Encryption" is "Two-way encryption", fields "Path to Private Key File", "Path to Certificate File" and "Path to CA File" are shown in any case, and 2 more fields "Path to CA Folder" and "Supported Cipher Suite" are only shown for the MySQL types but not for PostreSQL (PDO), all this regardless of the value of field "Verify Server Certificate".
  6. Close Global Configuration without saving changes.
  7. Apply the patch for this PR.
  8. Go again to Global Configuration, tab "Server", section "Database", and change value of field "Host" to something else than "localhost", e.g. "127.0.0.1".
  9. Select value "One-way authentication" for field "Connection Encryption".
  10. Change the database type several times so you have checked all 3 types "MySQLi", "MySQL (PDO)" and "PostgreSQL (PDO)", or at least all types which are available in your testing environment, and for each type toggle the value of field "Verify Server Certificate".
    Result: When field "Verify Server Certificate" has value "Yes", field "Path to CA File" is shown, and if a MySQL database type is used, also field "Path to CA Folder". When field "Verify Server Certificate" has value "No", these fields are not shown.
  11. Select value "Two-way authentication" for field "Connection Encryption", and repeat step 10.
    Result: Additional fields "Path to Private Key File", "Path to Certificate File" and in case of a MySQL database type also field "Supported Cipher Suite" is shown. For fields "Path to CA File" and "Path to CA Folder" behavior is still the same as in step 10.
  12. Close Global Configuration without saving changes.

Expected result

Fields "Path to CA File" and "Path to CA Folder" are only shown if "Yes" is chosen for field "Verify Server Certificate" and field "Connection Encryption" has values "One-way authentication" or "Two-way authentication".

The texts misleading texts "One-way encryption" and "Two-way encryption" have been changed to "One-way authentication" or "Two-way authentication".

Actual result

Fields "Path to CA File" and "Path to CA Folder" are only shown if "Two-way encryption" is chosen for field "Connection Encryption", regardless of the value of field "Verify Server Certificate".

The texts "One-way encryption" and "Two-way encryption" are misleading because the connection will be encrypted with both of these values, it is authentication which is done either one or two way.

Documentation Changes Required

None as far as I know, because as far as I know there is no documentation yet for the database connection encryption option in Global Configuration.

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Dec 19, 2019
COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE_VALUE_NONE="Default (server controlled)"
COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE_VALUE_ONE_WAY="One-way encryption"
COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE_VALUE_TWO_WAY="Two-way encryption"
COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE_VALUE_ONE_WAY="One-way authentication"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as this is a new string and you are changing the value to authentication then it would be good to update the constant as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How you mean to change it? COM_CONFIG_FIELD_DATABASE_AUTHENTICATION_ENABLE_VALUE_ONE_WAY would be wrong, because all the fields with names starting with COM_CONFIG_FIELD_DATABASE_ENCRYPTION belong to the same feature, which is well described with that name prefix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or how else do you mean it should be changed? I don't understand yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that was what I was suggesting

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well COM_CONFIG_FIELD_DATABASE_AUTHENTICATION_ENABLE_VALUE_ONE_WAY would be wrong, because it reads as if it would be related to authentication to database in general, but it is only relevant for connection encryption.

What could make sense would be to change the texts COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE... to COM_CONFIG_FIELD_DATABASE_ENCRYPTION_MODE..., or something like that, that would fit more to the values ("Default (server controlled)", "One-way authentication" and "Two-way authentication". The "ENABLE" is a bit misleading and has historically grown when that feadutre was developed by @andrepereiradasilva .

Should I change them to COM_CONFIG_FIELD_DATABASE_ENCRYPTION_MODE...?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brianteeman Please check my comment above. To make it perfect I could change COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE_LABEL to COM_CONFIG_FIELD_DATABASE_ENCRYPTION_MODE_LABEL, COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE_VALUE_NONE to COM_CONFIG_FIELD_DATABASE_ENCRYPTION_MODE_VALUE_DEFAULT, COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE_VALUE_ONE_WAY to COM_CONFIG_FIELD_DATABASE_ENCRYPTION_MODE_VALUE_ONE_WAY and COM_CONFIG_FIELD_DATABASE_ENCRYPTION_ENABLE_VALUE_TWO_WAY to COM_CONFIG_FIELD_DATABASE_ENCRYPTION_MODE_VALUE_TWO_WAY.

Let me know your opinion and what you suggest, but your original suggestion was not good in my opinion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we leave text names like they are.

@richard67 richard67 requested a review from HLeithner December 19, 2019 18:06
@richard67
Copy link
Member Author

Please wait with testing. I will make some changes in the next day or make a new PR. I'll report back here when all is ready.

@richard67
Copy link
Member Author

Closing in favour of PR #27351 .

@richard67 richard67 closed this Dec 27, 2019
@richard67 richard67 deleted the 4.0-dev-tls-encrypt-db-connections-mod-1 branch December 27, 2019 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants