Summary | SQL SSL support |
Queue | Horde Framework Packages |
Queue Version | FRAMEWORK_3 |
Type | Enhancement |
State | Resolved |
Priority | 1. Low |
Owners | chuck (at) horde (dot) org |
Requester | BryanRJ (at) gmail (dot) com |
Created | 02/11/2009 (5986 days ago) |
Due | |
Updated | 02/25/2009 (5972 days ago) |
Assigned | 02/12/2009 (5985 days ago) |
Resolved | 02/25/2009 (5972 days ago) |
Milestone | 3.3.4 |
Patch | No |
State ⇒ Resolved
http://cvs.horde.org/diff.php/framework/Alarm/Alarm/sql.php?rt=horde&r1=1.18&r2=1.19&ty=u
http://cvs.horde.org/diff.php/framework/Auth/Auth/sql.php?rt=horde&r1=1.109&r2=1.110&ty=u
http://cvs.horde.org/diff.php/framework/Cache/Cache/sql.php?rt=horde&r1=1.23&r2=1.24&ty=u
http://cvs.horde.org/diff.php/framework/DataTree/DataTree/sql.php?rt=horde&r1=1.247&r2=1.248&ty=u
http://cvs.horde.org/diff.php/framework/Group/Group/sql.php?rt=horde&r1=1.11&r2=1.12&ty=u
http://cvs.horde.org/diff.php/framework/Horde/Horde/Config.php?rt=horde&r1=1.147&r2=1.148&ty=u
http://cvs.horde.org/diff.php/framework/Lock/Lock/sql.php?rt=horde&r1=1.16&r2=1.17&ty=u
http://cvs.horde.org/diff.php/framework/Perms/Perms/sql.php?rt=horde&r1=1.9&r2=1.10&ty=u
http://cvs.horde.org/diff.php/framework/Prefs/Prefs/sql.php?rt=horde&r1=1.130&r2=1.131&ty=u
New Attachment: ssl2.patch
hard to apply to CVS (as opposed to a packaged release). It also
fails to apply in most cases for me if i supply the paths manually.
Can you please do this patch against a CVS HEAD/git checkout
(http://horde.org/source/)? Also, you seem to be introducing tabs and
whitespace inconsistencies - if those could be ironed out it would
speed up applying
(http://horde.org/horde/docs/?f=CODING_STANDARDS.html). Thanks!
hard to apply to CVS (as opposed to a packaged release). It also fails
to apply in most cases for me if i supply the paths manually. Can you
please do this patch against a CVS HEAD/git checkout
(http://horde.org/source/)? Also, you seem to be introducing tabs and
whitespace inconsistencies - if those could be ironed out it would
speed up applying
(http://horde.org/horde/docs/?f=CODING_STANDARDS.html). Thanks!
New Attachment: ssl.patch
situation is thus: if a user wants to use a persistent connection for
the RW database and NOT use one for the RO database, they set the
"persistent" flag in the RW options and set no such option in the RO.
Attached: updated patch that uses the merged array where appropriate.
situation is thus: if a user wants to use a persistent connection for
the RW database and NOT use one for the RO database, they set the
"persistent" flag in the RW options and set no such option in the RO.
Example code:
<?php
$foo = array('foo'=>true, 'bar'=>'baz');
$bazza = array('bar'=>'yar');
var_dump(array_merge($foo,$bazza));
?>
This code prints:
array(2) {
["foo"]=>
bool(true)
["bar"]=>
string(3) "yar"
}
Note that "foo" is still set to true. The analogy to the current
situation is thus: if a user wants to use a persistent connection for
the RW database and NOT use one for the RO database, they set the
"persistent" flag in the RW options and set no such option in the RO.
So you call array_merge($rwopts,$roopts) and get an array which has
"persistent" nonempty, because it was nonempty in $rwopts... Get it?
should be just "$params".
that issue?
since the $params array is not correct anyhow and so using it won't
change anything.
Imp/Turba/Kronolith/etc? Pretty much every application also needs it.
Thanks!
State ⇒ Feedback
reads appears to be broken in all of these places - array_merge is
called to merge the original parameters with the read-only DB ones.
However, both the persistent connections option and this new SSL one
check !empty, so in the case where an SSL connection is desired for
the original database but not for the new one you will get incorrect
behavior.
should override a true key in the original one just fine.
should be just "$params".
Imp/Turba/Kronolith/etc? Pretty much every application also needs it.
Thanks!
Perhaps this is a topic for another bug, but the handling of split
reads appears to be broken in all of these places - array_merge is
called to merge the original parameters with the read-only DB ones.
However, both the persistent connections option and this new SSL one
check !empty, so in the case where an SSL connection is desired for
the original database but not for the new one you will get incorrect
behavior.
Also, the references to "$this->_params" inside the splitreads block
should be just "$params".
How do I go about getting this same change made to
Imp/Turba/Kronolith/etc? Pretty much every application also needs it.
State ⇒ Resolved
Milestone ⇒ 3.3.4
http://cvs.horde.org/diff.php/framework/Alarm/Alarm/sql.php?rt=horde&r1=1.17&r2=1.18&ty=u
http://cvs.horde.org/diff.php/framework/Auth/Auth/sql.php?rt=horde&r1=1.108&r2=1.109&ty=u
http://cvs.horde.org/diff.php/framework/Cache/Cache/sql.php?rt=horde&r1=1.22&r2=1.23&ty=u
http://cvs.horde.org/diff.php/framework/DataTree/DataTree/sql.php?rt=horde&r1=1.246&r2=1.247&ty=u
http://cvs.horde.org/diff.php/framework/Group/Group/sql.php?rt=horde&r1=1.10&r2=1.11&ty=u
http://cvs.horde.org/diff.php/framework/Horde/Horde/Config.php?rt=horde&r1=1.146&r2=1.147&ty=u
http://cvs.horde.org/diff.php/framework/Lock/Lock/sql.php?rt=horde&r1=1.15&r2=1.16&ty=u
http://cvs.horde.org/diff.php/framework/Perms/Perms/sql.php?rt=horde&r1=1.8&r2=1.9&ty=u
http://cvs.horde.org/diff.php/framework/Prefs/Prefs/sql.php?rt=horde&r1=1.129&r2=1.130&ty=u
http://cvs.horde.org/diff.php/framework/SessionHandler/SessionHandler/sql.php?rt=horde&r1=1.49&r2=1.50&ty=u
http://cvs.horde.org/diff.php/framework/Token/Token/sql.php?rt=horde&r1=1.43&r2=1.44&ty=u
http://cvs.horde.org/diff.php/framework/VFS/lib/VFS/sql.php?rt=horde&r1=1.5&r2=1.6&ty=u
http://cvs.horde.org/diff.php/framework/VFS/lib/VFS/sql_file.php?rt=horde&r1=1.1&r2=1.2&ty=u
http://cvs.horde.org/diff.php/horde/docs/CHANGES?rt=horde&r1=1.1204&r2=1.1205&ty=u
Assigned to Chuck Hagenbuch
State ⇒ Assigned
(http://git.horde.org/co.php/framework/Db/lib/Horde/Db/Adapter/Mysqli.php?r=9115f57f8f3da190a9f9f05b0db55135f51ac4d7&rt=horde-git) has SSL support in the mysqli
adapter
already.
Is this only relevant for mysqli?
didn't realize that your Rdo code wasn't used - I saw calls to
connect, so I rewrote them to accept additional arguments. This code
in theory applies to all SQL versions, but I'm unsure what PEAR DB
does with the ssl=>true setting with backends other than mysqli.
(http://git.horde.org/co.php/framework/Db/lib/Horde/Db/Adapter/Mysqli.php?r=9115f57f8f3da190a9f9f05b0db55135f51ac4d7&rt=horde-git) has SSL support in the mysqli adapter
already.
Is this only relevant for mysqli?
New Attachment: ssl
provide a patch?
State ⇒ Feedback
provide a patch?
Priority ⇒ 1. Low
State ⇒ New
Patch ⇒ No
Milestone ⇒
Summary ⇒ SQL SSL support
Type ⇒ Enhancement
Queue ⇒ Horde Framework Packages
This is an easy feature to implement: mysqli and PEAR DB both support
it already. All that needs to be done is changing
/lib/Horde/Rdo/Adapter/Mysqli.php on line 171 to use mysqli_init,
mysqli_ssl_set, and mysqli_real_connect (there is already a comment
there to that effect) and then going through every call to DB::connect
(such as the one on nag/lib/Driver/sql.php around line 590) in all
horde applications and changing two things:
a) add a "ca" to _params
b) add "ssl"=>true in the options array passed to DB::connect
So, steps to implement this:
1) Add the user interface to accept a SSL CA against which to verify
the server (and, if you like, a client cert/key to use) and a checkbox
for enabling SSL
2) Modify the MySQLi Rdo adaptor to use these options
3) Modify each call to DB::connect to use these options
I completed these steps myself and verified that horde and its
applications can now access a database where I gave the horde user
"grant all privileges ... require ssl" permissions. Previously the
database connection failed. All told, this work took twenty minutes,
although I hardcoded my CA cert instead of actually adding an option
to conf.php.