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

Add mysqli connection flags to mysqli_real_connect #53

Closed
4 tasks done
michalbundyra opened this issue Jan 16, 2020 · 7 comments
Closed
4 tasks done

Add mysqli connection flags to mysqli_real_connect #53

michalbundyra opened this issue Jan 16, 2020 · 7 comments

Comments

@michalbundyra
Copy link
Member

The Mysqli driver doesn't allow setting of flags passed into mysqli_real_connect.

  • Are you fixing a bug?
    • Detail how the bug is invoked currently.

In some instances (like MySQL and PHP on Azure) MYSQLI_SSL_CLIENT needs to be set.

'params' => [
    'host' => 'xxxx.mysql.database.azure.com',
    'port' => 3306,
    'user' => 'xxxx@xxxxx',
    'password' => 'xxxxx',
    'dbname' => 'xxxx',
    'charset' => 'utf8',
    'use_ssl' => true,
    'ssl_ca' => 'xxxxx/certs/BaltimoreCyberTrustRoot.crt.pem',
    'driver_options' => [ ]
]
  • Detail the original, incorrect behavior.

This calls $this->resource->ssl_set($clientKey, $clientCert, $caCert, $caPath, $cipher); to configure the SSL options, but no flag is set on mysqli_real_connect. Connection to Azure subsequently fails with 'SSL connection is required'

  • Detail the new, expected behavior.

Driver should allow connecting as illustrated in azure documentation at https://docs.microsoft.com/en-us/azure/mysql/howto-configure-ssl .

Added 'flags' config key to driver_options to support the setting of flags into mysqli_real_connect. This lets users supply MYSQLI_CLIENT_SSL and other connection flags as indicated on http://www.php.net/manual/en/mysqli.real-connect.php

'params' => [
    'host' => 'xxxx.mysql.database.azure.com',
    'port' => 3306,
    'user' => 'xxxx@xxxxx',
    'password' => 'xxxxx',
    'dbname' => 'xxxx',
    'charset' => 'utf8',
    'use_ssl' => true,
    'ssl_ca' => 'xxxxx/certs/BaltimoreCyberTrustRoot.crt.pem',
    'driver_options' => [ 
        'flags' => MYSQLI_CLIENT_SSL 
    ]
]

Originally posted by @kris-sum at zendframework/zend-db#314

@michalbundyra
Copy link
Member Author

Is anyone available to review this? @ezimuel @weierophinney ?


Originally posted by @kris-sum at zendframework/zend-db#314 (comment)

@michalbundyra
Copy link
Member Author

@kris-sum We need test case for this change.


Originally posted by @michalbundyra at zendframework/zend-db#314 (comment)

@michalbundyra
Copy link
Member Author

@webimpress Any test case would require setting up SSL on the MySQL end - I can't see a way I can do that with the current integration test setup?


Originally posted by @kris-sum at zendframework/zend-db#314 (comment)

@michalbundyra
Copy link
Member Author

@kris-sum mock resource, pass it into the constructor and check if calls are with proper values/flags. Seems to be doable.


Originally posted by @michalbundyra at zendframework/zend-db#314 (comment)

@michalbundyra
Copy link
Member Author

@kris-sum can you provide the unit test as suggested by @webimpress ? Thanks.


Originally posted by @ezimuel at zendframework/zend-db#314 (comment)

@demiankatz
Copy link
Contributor

@webimpress, @michalbundyra, as @kris-sum pointed out above, PR #169 should resolve this issue. What needs to be done to get that PR merged? Happy to help in any way I can, as this fix would be very helpful to have!

@weierophinney
Copy link
Member

This package is considered feature-complete, and is now in security-only maintenance mode, following a decision by the Technical Steering Committee.
If you have a security issue, please follow our security reporting guidelines.
If you wish to take on the role of maintainer, please nominate yourself

If you are looking for an actively maintained package alternative, we recommend:

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

No branches or pull requests

3 participants