codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.4k stars 1.9k forks source link

Bug: MySQLi Config "encrypt" values not being respected #7453

Closed DaneRainbird closed 1 year ago

DaneRainbird commented 1 year ago

PHP Version

8.2

CodeIgniter4 Version

4.3.2

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

No response

What happened?

While following the instructions for configuring a database connection, I have configured my env file to contain the following:

database.default.DBDriver = MySQLi
database.default.encrypt.ssl_ca = C:\xampp\perl\vendor\lib\Mozilla\CA\cacert.pem
database.default.encrypt.ssl_verify = true

However, it appears these are not being respected by the actual SSL connection, as seen in Database.php. Thus, checking in vendor/codeigniter4/framework/system/Database/MySQLi/Connection.php. it appears that the entire section for checking if is_array($this->encrypt) is skipped over due to not being an array.

I have "fixed" this by adding a simple line beforehand that manually sets the encrypt values:

$this->encrypt = [
    'ssl_key'    => '',
    'ssl_cert'   => '',
    'ssl_ca'     => 'C:\xampp\perl\vendor\lib\Mozilla\CA\cacert.pem',
    'ssl_capath' => '',
    'ssl_cipher' => '',
    'ssl_verify' => 'true'
];

Thus continuing the checks and ensuring I can make an encrypted connection.

Steps to Reproduce

  1. Set any of the encryption subvalues in your .env file, or in Database.php
  2. Go to vendor/codeigniter4/framework/system/Database/MySQLi/Connection.php
  3. Within the connection function, add a simple print_r($this->encrypt); and observe that it appears to be being interpreted as a boolean rather than a array.

Expected Output

The correct values to be passed to Database.php / Connection.php to allow for connecting to databases that enforce SSL connections.

Anything else?

No response

kenjis commented 1 year ago

This is not a bug. See #7311

kenjis commented 1 year ago

This is documented, but the current description is not sufficient? https://codeigniter4.github.io/CodeIgniter4/general/configuration.html#environment-variables-as-replacements-for-data

DaneRainbird commented 1 year ago

Hi @kenjis,

Thanks for that, I understand that the env files cannot create new entries - my main issue is thus with the documentation that specifically states what entries can be put into the env file:

https://codeigniter.com/user_guide/database/configuration.html#configuring-with-env-file

Specifically see the "encrypt" section of "Explanation of Values"

I'm happy to find another workaround by forcing the code elsewhere, but if these values are not respected, then should they be removed from the documentation?

Thanks!

kenjis commented 1 year ago

Environment variables can only be strings. And CI4's configuration class can only change scalar values by the environment variable.

If a property of a configuration class has a scalar value, you cannot change it to an array. If a property has an array value, you can change the value (scalar) of its elements, but you cannot add an element to the array.

This is not properly documented and is also difficult to understand. I think we need to improve the documentation.

If you really want to set the values only in the .env, describe the property values in JSON in the .env and add code to convert the JSON string to an array in the constructor of the Config class.

DaneRainbird commented 1 year ago

Thanks @kenjis,

Much appreciated on this one - I definitely think the documentation needs some work here.

In the meantime, I'm still not sure what's the best option for me to ensure that I enable SSL when talking to my web database, as short of modifying the /vendor/ directory, I don't appear to be able to enable ssl_verify as seen in Connection.php

Any ideas?

Thanks!

kenjis commented 1 year ago

Certainly, this SSL setting will vary depending on the environment, so it is impossible to set the Cert file location in the Config file.

database.default.encrypt = {"ssl_verify":true,"ssl_ca":"C:\\xampp\\perl\\vendor\\lib\\Mozilla\\CA\\cacert.pem"}
--- a/app/Config/Database.php
+++ b/app/Config/Database.php
@@ -80,5 +80,10 @@ class Database extends Config
         if (ENVIRONMENT === 'testing') {
             $this->defaultGroup = 'tests';
         }
+
+        $array = json_decode($this->default['encrypt'], true);
+        if (is_array($array)) {
+            $this->default['encrypt'] = $array;
+        }
     }
 }
kenjis commented 1 year ago

I sent a PR to improve the docs: #7454

DaneRainbird commented 1 year ago

Thank you so much on both counts for this, very much appreciated!