List:Commits« Previous MessageNext Message »
From:Georgi Kodinov Date:July 8 2011 4:23pm
Subject:Re: bzr commit into mysql-trunk branch (Georgi.Kodinov:3200) Bug#11747191
View as plain text  
Hello Serg, Nirbhay and Tatjana,

Thank you all for your reviews !
I hope I've addressed all of your remarks : moved the data members to the extension field,
fixed few no-ssl and yassl compilation errors updated all the test results affected and
got rid of the trailing spaces and typos.

Tatjana, 

on your remark on Master_SSL_Crl I think we need to keep it consistent with the rest of
the ssl replication options.
As for the unresolved SSL errors : I will re-test this on another box : it seems that my
mac is lacking these messages somehow.

The updated diff is attached.
Please check it and send me your review comments if you have them.

Attachment: [application/x-apple-msg-attachment] B11747191-trunk-3200.patch
--------------------- New commit message ---------------------------- committer: Georgi Kodinov <Georgi.Kodinov@stripped> branch nick: B11747191-trunk timestamp: Fri 2011-07-08 19:12:08 +0300 message: Bug #11747191: 31224: SUPPORT FOR SSL CERTIFICATE REVOCATION LISTS Added support for --ssl-crl and --ssl-crlpath to all client and server binaries that work with OpenSSL. You can specify none, one or both of the above. --ssl-crl takes a file path for a PEM encoded Certificate revocation lists. The relevant file is parsed and loaded into the X509 store of the SSL context. --ssl-crlpath takes a directory path. This directory must contain PEM encoded CRL (or other) files that are named by their hash value, .e.g. <hash_value>.r[0-9] See OpenSSL's X509_STORE_load_locations() for more details of the above. Note that if none of the --ssl-crl* options is specified no CRL checks will be performed, even if the -capath contains certificate revocation lists. Added Master_SSL_crl and Master_SSL_CRLPATH to CNANGE MASTER command. Added new columns Ssl_crl and Ssl_crlpath to mysql.slave_master_info system table. Reengineered mysql_ssl_set() in the C API into a number of mysql_options calls as follows (while keeping mysql_ssl_set()): mysql_ssl_set(mysql, key, cert, ca, capath, cipher) { mysql_options(mysql, MYSQL_OPT_SSL_KEY, key) mysql_options(mysql, MYSQL_OPT_SSL_CERT, cert) mysql_options(mysql, MYSQL_OPT_SSL_CA, ca) mysql_options(mysql, MYSQL_OPT_SSL_CAPATH, capath) mysql_options(mysql, MYSQL_OPT_SSL_CIPHER, cipher) } Added two new mysql_options that correspond to the command line calls : MYSQL_OPT_SSL_CRL and MYSQL_OPT_SSL_CRLPATH. Made sure these play nicely with the ABI by using the extension. Added tests and a set of cryptographic keys and crls to test the new options. --------------------------------------------------------------------------------- Best Regards, Joro On 07.07.2011, at 20:47, Sergei Golubchik wrote: > Hi, Nirbhay! > > On Jul 05, Nirbhay Choubey wrote: >> Hi Joro, >> >> Overall patch looks good, however I would suggest : >> >> 1) The code >> >> + mysql_options(&mysql, MYSQL_OPT_SSL_CRL, opt_ssl_crl); >> + mysql_options(&mysql, MYSQL_OPT_SSL_CRLPATH, opt_ssl_crlpath); >> >> is getting repeated in all the client programs and rpl_slave.cc. >> IMHO, we should add something like 'mysql_ssl_set_extended()' >> in client.c to make it call 'mysql_ssl_set()' and further add >> the two new introduced options. > > Note that extending mysql_options() is completely backward compatible. > Any client built with a newer library can later be perfectly used with > older libraries that users may happen to have on their computers. > mysql_options() will simply ignore unknown values (and return an error > that an application can check and do something). > > If you add a new function to a client library, an application will > simply not start with an older library (with an unhelpful message > "symbol _mysql_ssl_set_extended was not found"). > > On the other hand, the patch changes struct st_mysql_options. Which is > even worse - not only it is backward incompatible, it is both forward > and backward incompatible. An application will not work with either > older or newer version of the library. And there will be no error on > startup - it will start just fine and crash some time later. > > Generally for any incompatible change one needs to change the library > version. But in this case new options can simply be moved to struct > st_mysql_options_extention, which was created exactly for this purpose. Thanks for the rev > > Regards, > Sergei > -- Georgi Kodinov | Software Development Snr Manager | +359887700566 Oracle MySQL ul. Belgrad 12, office 34, Plovdiv, 4003, Bulgaria
Thread
bzr commit into mysql-trunk branch (Georgi.Kodinov:3200) Bug#11747191Georgi Kodinov19 Jun
  • Re: bzr commit into mysql-trunk branch (Georgi.Kodinov:3200)Bug#11747191Nirbhay Choubey6 Jul
    • Re: bzr commit into mysql-trunk branch (Georgi.Kodinov:3200)Bug#11747191Sergei Golubchik7 Jul
      • Re: bzr commit into mysql-trunk branch (Georgi.Kodinov:3200) Bug#11747191Georgi Kodinov10 Jul