Summary: | Add TLS MySQL connection without mutual authentication | ||
---|---|---|---|
Product: | Koha | Reporter: | rschet |
Component: | Authentication | Assignee: | Lukas Koszyk <lukasz.koszyk> |
Status: | Needs documenting --- | QA Contact: | Thomas Klausner <domm> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | clemens.tubach, cornelius.amzar, david, dcook, domm, dpavlin, jonathan.druart, lucas, lukasz.koszyk, m.de.rooy, markus.john, michaela.sieber, raphael.straub |
Version: | Main | ||
Hardware: | All | ||
OS: | Linux | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38760 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39591 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39602 |
||
GIT URL: | Change sponsored?: | Sponsored | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: |
Database connections with TLS require client private keys and certificates for authentication, but MariaDB also supports authentication by user and password.
This enhancement allows omitting the TLS options for certificate based client authentication.
Information on configuring: https://wiki.koha-community.org/wiki/Securing_MariaDB_connections_in_Koha_using_a_separate_database_server
|
|
Version(s) released in: |
24.11.00
|
Circulation function: | |
Bug Depends on: | |||
Bug Blocks: | 39591 | ||
Attachments: |
Bug 36026: Use only configured TLS options for database connection
Bug 36026: Use only configured TLS options for database connection Bug 36026: Use only configured TLS options for database connection Bug 36026: Use only configured TLS options for database connection Bug 36026: Use only configured TLS options for database connection |
Description
rschet
2024-02-07 20:16:42 UTC
I just took a look at Koha/Database.pm and I see what you mean: my $tls = $config->get("tls"); if ($tls && $tls eq 'yes') { $dsn .= sprintf( ';mysql_ssl=1;mysql_ssl_client_key=%s;mysql_ssl_client_cert=%s;mysql_ssl_ca_file=%s', $config->get('key'), $config->get('cert'), $config->get('ca'), ); } It would take a bit of digging to figure out why it was set up this way originally, but it wouldn't be too hard to change this, so that the client authentication was optional/separate. My plate is a bit full at the moment, but if someone wanted to take this on... they'd need to make a Koha::Database::generate_dsn type function and unit test it with no tls, tls with no client auth, tls with client auth. If they make the $config object a parameter of the function, then it would be really easy to unit test. Ideally, someone would also do an integration test with a MySQL using SSL. It would be interesting to add that to the Jenkins CI actually. If no else does a patch, I could come back to it, but it might be a few weeks or longer. I think this is certainly a worthwhile one to do though. Great, thank you! -R Created attachment 172587 [details] [review] Bug 36026: Use only configured TLS options for database connection Database connections with TLS require client private keys and certificates for authentication but MariaDB also supports authentication by user and password. This patch allows omitting the TLS options for certificate based client authentication. To test: 1) Configure the database to support TLS connections. 2) Set "<tls>yes</tls>" in the config section in koha-conf.xml. 3) Run "koha-plack --reload <koha_instance>". 4) Open Koha's staff interface in the browser. 5) Observe an internal server error. 6) Apply patch. 7) Repeat step 3 and 4. 8) Observe the error is gone. 9) Sign off. Sponsored-by: Karlsruhe Institute of Technology (KIT) Created attachment 172822 [details] [review] Bug 36026: Use only configured TLS options for database connection Database connections with TLS require client private keys and certificates for authentication but MariaDB also supports authentication by user and password. This patch allows omitting the TLS options for certificate based client authentication. To test: 1) Configure the database to support TLS connections. 2) Set "<tls>yes</tls>" in the config section in koha-conf.xml. 3) Run "koha-plack --reload <koha_instance>". 4) Open Koha's staff interface in the browser. 5) Observe an internal server error. 6) Apply patch. 7) Repeat step 3 and 4. 8) Observe the error is gone. 9) Sign off. Sponsored-by: Karlsruhe Institute of Technology (KIT) Signed-off-by: Cornelius Amzar <cornelius.amzar@bsz-bw.de> Signed-off-by: Markus John <markus.john@bsz-bw.de> Tested connection, it works! Before, we were unable to connect to a remote mariadb server who requires TLS without having a key and cert pair for the client. With the patch we were able to connect to the server using a CA cert only but key and cert left blank. I tried to QA this patch according to the test plan, but failed: ``` DBI connect('database=koha_kohadev;host=db;port=3306;mysql_ssl=1','koha_kohadev',...) failed: SSL connection error: Enforcing SSL encryption is not supported ``` But I assume this failure is caused by the fact that KTD mysql does not have SSL configured. At least now the three SSL opts are not included in DSN with empty values: `mysql_ssl_client_key=;mysql_ssl_client_cert=;mysql_ssl_ca_file=` So I assume this works, but cannot properly test it without a TSL-enabled MySQL server. Is there a way to set this up in KTD? I asked Tomas and there seems to be no easy or ready-set-up way to test this with ktd. I know that KIT runs this in production and we also have tested the change on our servers thoroughly, but not in a development environment. Maybe we or KIT could share some more information about setting up the encrypted database connection in general? Maybe this could be helpful? https://peterkellner.net/2023/08/02/Enforcing-SSL-Connections-in-MySql-Using-Docker/ As proposed by David we will make the Koha::Database::generate_dsn type function and the unit test with no tls, tls with no client auth, tls with client auth. Created attachment 173381 [details] [review] Bug 36026: Use only configured TLS options for database connection Database connections with TLS require client private keys and certificates for authentication but MariaDB also supports authentication by user and password. This patch allows omitting the TLS options for certificate based client authentication. To test: 1) Apply patch. 2) Run "prove -v t/db_dependent/Koha/Database.t" to check if the new function generate_dsn returns correct DSNs. 3) Use KTD to verify that the TLS DB connection actually works: a) Open shell in database container "docker exec -it kohadev-db-1 /bin/bash" b) Create certificates and keys: mkdir -p /etc/mysql/ssl/{certs,private} openssl genrsa 4096 > /etc/mysql/ssl/private/ca-key.pem openssl req -new -x509 -nodes -days 3650 -key /etc/mysql/ssl/private/ca-key.pem \ -out /etc/mysql/ssl/certs/ca-cert.pem \ -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=MariaDB_CA" openssl req -newkey rsa:4096 -days 3650 -nodes -keyout /etc/mysql/ssl/private/server-key.pem \ -out /etc/mysql/ssl/certs/server-req.pem \ -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=db" openssl rsa -in /etc/mysql/ssl/private/server-key.pem -out /etc/mysql/ssl/private/server-key.pem openssl x509 -req -in /etc/mysql/ssl/certs/server-req.pem -days 3650 \ -CA /etc/mysql/ssl/certs/ca-cert.pem -CAkey /etc/mysql/ssl/private/ca-key.pem \ -set_serial 01 -out /etc/mysql/ssl/certs/server-cert.pem chown -Rv mysql:root /etc/mysql/ssl/ c) Configure the MariaDB server to require TLS: echo " [mariadb] ssl_ca=/etc/mysql/ssl/certs/ca-cert.pem ssl_cert=/etc/mysql/ssl/certs/server-cert.pem ssl_key=/etc/mysql/ssl/private/server-key.pem require_secure_transport = on " > /etc/mysql/mariadb.conf.d/50-ssl.cnf d) Restart MariaDB: kill 1 e) Switch to Koha container: ktd --shell f) Set "<tls>yes</tls>" in the <config> section in koha-conf.xml. g) In Koha/Database.pm: replace ";mysql_ssl=1" by ";mysql_ssl=1;mysql_ssl_optional=1" 4) Run restart_all. 5) Check that Koha's staff interface works. 6) Sign off. Sponsored-by: Karlsruhe Institute of Technology (KIT) Created attachment 173426 [details] [review] Bug 36026: Use only configured TLS options for database connection Database connections with TLS require client private keys and certificates for authentication but MariaDB also supports authentication by user and password. This patch allows omitting the TLS options for certificate based client authentication. To test: 1) Apply patch. 2) Run "prove -v t/db_dependent/Koha/Database.t" to check if the new function generate_dsn returns correct DSNs. 3) Use KTD to verify that the TLS DB connection actually works: a) Open shell in database container "docker exec -it kohadev-db-1 /bin/bash" b) Create certificates and keys: mkdir -p /etc/mysql/ssl/{certs,private} openssl genrsa 4096 > /etc/mysql/ssl/private/ca-key.pem openssl req -new -x509 -nodes -days 3650 -key /etc/mysql/ssl/private/ca-key.pem \ -out /etc/mysql/ssl/certs/ca-cert.pem \ -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=MariaDB_CA" openssl req -newkey rsa:4096 -days 3650 -nodes -keyout /etc/mysql/ssl/private/server-key.pem \ -out /etc/mysql/ssl/certs/server-req.pem \ -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=db" openssl rsa -in /etc/mysql/ssl/private/server-key.pem -out /etc/mysql/ssl/private/server-key.pem openssl x509 -req -in /etc/mysql/ssl/certs/server-req.pem -days 3650 \ -CA /etc/mysql/ssl/certs/ca-cert.pem -CAkey /etc/mysql/ssl/private/ca-key.pem \ -set_serial 01 -out /etc/mysql/ssl/certs/server-cert.pem chown -Rv mysql:root /etc/mysql/ssl/ c) Configure the MariaDB server to require TLS: echo " [mariadb] ssl_ca=/etc/mysql/ssl/certs/ca-cert.pem ssl_cert=/etc/mysql/ssl/certs/server-cert.pem ssl_key=/etc/mysql/ssl/private/server-key.pem require_secure_transport = on " > /etc/mysql/mariadb.conf.d/50-ssl.cnf d) Restart MariaDB: kill 1 e) Switch to Koha container: ktd --shell f) Set "<tls>yes</tls>" in the <config> section in koha-conf.xml. g) In Koha/Database.pm: replace ";mysql_ssl=1" by ";mysql_ssl=1;mysql_ssl_optional=1" 4) Run restart_all. 5) Check that Koha's staff interface works. 6) Sign off. Sponsored-by: Karlsruhe Institute of Technology (KIT) Signed-off-by: David Nind <david@davidnind.com> Thanks for providing the test plan! (This made it easy to sign off.) Could you also please add a release note? In addition, (once it passes QA) could you add a comment with instructions on how to set this up/what this means. Once it is pushed, I will add the instructions to either the Wiki or manual (probably the Wiki). Testing notes (using KTD): 1. Commands on one line to avoid any confusion for test plan step 3(b): mkdir -p /etc/mysql/ssl/{certs,private} openssl genrsa 4096 > /etc/mysql/ssl/private/ca-key.pem openssl req -new -x509 -nodes -days 3650 -key /etc/mysql/ssl/private/ca-key.pem -out /etc/mysql/ssl/certs/ca-cert.pem -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=MariaDB_CA" openssl req -newkey rsa:4096 -days 3650 -nodes -keyout /etc/mysql/ssl/private/server-key.pem -out /etc/mysql/ssl/certs/server-req.pem -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=db" openssl rsa -in /etc/mysql/ssl/private/server-key.pem -out /etc/mysql/ssl/private/server-key.pem openssl x509 -req -in /etc/mysql/ssl/certs/server-req.pem -days 3650 -CA /etc/mysql/ssl/certs/ca-cert.pem -CAkey /etc/mysql/ssl/private/ca-key.pem -set_serial 01 -out /etc/mysql/ssl/certs/server-cert.pem chown -Rv mysql:root /etc/mysql/ssl/ 2. For test plan step 3(f) - this is around line 265 (there are other <config> sections in the file). 3. For test plan step 3(g) - this is at line 209. (In reply to Lukas Koszyk from comment #10) > f) Set "<tls>yes</tls>" in the <config> section in koha-conf.xml. > > g) In Koha/Database.pm: > replace ";mysql_ssl=1" by ";mysql_ssl=1;mysql_ssl_optional=1" > > 4) Run restart_all. > > 5) Check that Koha's staff interface works. > > 6) Sign off. Could you please clarify this part of the test plan? We only set tls to yes, but do not fill the other stuff in koha-conf. We switch to ssl optional by changing the code we should be testing.. So we are actually testing something else? And signing off the original? And note: Setting mysql_ssl_optional to true disables strict SSL enforcement and makes SSL connection optional. This option opens security hole for man-in-the-middle attacks. => Not a good idea? (In reply to Marcel de Rooy from comment #13) > (In reply to Lukas Koszyk from comment #10) > > > f) Set "<tls>yes</tls>" in the <config> section in koha-conf.xml. > > > > g) In Koha/Database.pm: > > replace ";mysql_ssl=1" by ";mysql_ssl=1;mysql_ssl_optional=1" > > > > 4) Run restart_all. > > > > 5) Check that Koha's staff interface works. > > > > 6) Sign off. > > Could you please clarify this part of the test plan? > > We only set tls to yes, but do not fill the other stuff in koha-conf. We > switch to ssl optional by changing the code we should be testing.. So we are > actually testing something else? And signing off the original? > > And note: Setting mysql_ssl_optional to true disables strict SSL enforcement > and makes SSL connection optional. This option opens security hole for > man-in-the-middle attacks. > => Not a good idea? Yes, the mysql_ssl_optional parameter is not part of the patch to sign off. The additional parameter "mysql_ssl_optional=1" is added only to test in Koha-testing-docker. Without it we couldn't test in docker, probably there is a difference in the packages installed in docker. On our test system with MariaDB 10.6 on Ubuntu 22.04, the TLS connection works without this parameter. Also, I think mysql_ssl_optional should not be used on a production system. But even if this option is enabled for the client and if require_secure_transport is enabled on the MariaDB server then all connections attempted using insecure transport will be rejected. This test only checks if the TLS connection works. Additional parameters like ca certificate can be used (you need to copy the certificate first) but we didn't want to create too complicated configuration. Created attachment 174249 [details] [review] Bug 36026: Use only configured TLS options for database connection Database connections with TLS require client private keys and certificates for authentication but MariaDB also supports authentication by user and password. This patch allows omitting the TLS options for certificate based client authentication. To test: 1) Apply patch. 2) Run "prove -v t/db_dependent/Koha/Database.t" to check if the new function generate_dsn returns correct DSNs. 3) Use KTD to verify that the TLS DB connection actually works: a) Open shell in database container "docker exec -it kohadev-db-1 /bin/bash" b) Create certificates and keys: mkdir -p /etc/mysql/ssl/{certs,private} openssl genrsa 4096 > /etc/mysql/ssl/private/ca-key.pem openssl req -new -x509 -nodes -days 3650 -key /etc/mysql/ssl/private/ca-key.pem \ -out /etc/mysql/ssl/certs/ca-cert.pem \ -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=MariaDB_CA" openssl req -newkey rsa:4096 -days 3650 -nodes -keyout /etc/mysql/ssl/private/server-key.pem \ -out /etc/mysql/ssl/certs/server-req.pem \ -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=db" openssl rsa -in /etc/mysql/ssl/private/server-key.pem -out /etc/mysql/ssl/private/server-key.pem openssl x509 -req -in /etc/mysql/ssl/certs/server-req.pem -days 3650 \ -CA /etc/mysql/ssl/certs/ca-cert.pem -CAkey /etc/mysql/ssl/private/ca-key.pem \ -set_serial 01 -out /etc/mysql/ssl/certs/server-cert.pem chown -Rv mysql:root /etc/mysql/ssl/ c) Configure the MariaDB server to require TLS: echo " [mariadb] ssl_ca=/etc/mysql/ssl/certs/ca-cert.pem ssl_cert=/etc/mysql/ssl/certs/server-cert.pem ssl_key=/etc/mysql/ssl/private/server-key.pem require_secure_transport = on " > /etc/mysql/mariadb.conf.d/50-ssl.cnf d) Restart MariaDB: kill 1 e) Switch to Koha container: ktd --shell f) Set "<tls>yes</tls>" in the <config> section in koha-conf.xml. g) In Koha/Database.pm: replace ";mysql_ssl=1" by ";mysql_ssl=1;mysql_ssl_optional=1" 4) Run restart_all. 5) Check that Koha's staff interface works. 6) Sign off. Sponsored-by: Karlsruhe Institute of Technology (KIT) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Thomas Klausner <domm@plix.at> Thanks to the excellent test plan I could now easily generate certs/keys for MySQL and thus verify that everything works! The qa-script also worked, but it could not run the tests because either it failed due to the needed addition of `mysql_ssl_optional=1` or because the certs/keys where missing. But as the tests work when NOT including the workaround to actually try out the certs/keys; and as I manually checked that the DB connection works when using certs/keys, I declare this feature "Passed QA" :-) (In reply to Thomas Klausner from comment #16) > Thanks to the excellent test plan I could now easily generate certs/keys for > MySQL and thus verify that everything works! > > The qa-script also worked, but it could not run the tests because either it > failed due to the needed addition of `mysql_ssl_optional=1` or because the > certs/keys where missing. But as the tests work when NOT including the > workaround to actually try out the certs/keys; and as I manually checked > that the DB connection works when using certs/keys, I declare this feature > "Passed QA" :-) Did you see my comment actually before your declaration? :) (In reply to Marcel de Rooy from comment #17) > (In reply to Thomas Klausner from comment #16) > > Thanks to the excellent test plan I could now easily generate certs/keys for > > MySQL and thus verify that everything works! > > > > The qa-script also worked, but it could not run the tests because either it > > failed due to the needed addition of `mysql_ssl_optional=1` or because the > > certs/keys where missing. But as the tests work when NOT including the > > workaround to actually try out the certs/keys; and as I manually checked > > that the DB connection works when using certs/keys, I declare this feature > > "Passed QA" :-) > > Did you see my comment actually before your declaration? :) Did you check on Lukas reply? (In reply to Katrin Fischer from comment #18) > (In reply to Marcel de Rooy from comment #17) > > (In reply to Thomas Klausner from comment #16) > > > Thanks to the excellent test plan I could now easily generate certs/keys for > > > MySQL and thus verify that everything works! > > > > > > The qa-script also worked, but it could not run the tests because either it > > > failed due to the needed addition of `mysql_ssl_optional=1` or because the > > > certs/keys where missing. But as the tests work when NOT including the > > > workaround to actually try out the certs/keys; and as I manually checked > > > that the DB connection works when using certs/keys, I declare this feature > > > "Passed QA" :-) > > > > Did you see my comment actually before your declaration? :) > > Did you check on Lukas reply? Sure. I understand the reasoning behind it, but it does not change my former comment actually. Marcel, you pointed out some concerns with the patch and you say that Lukas' comment didn't solve the problem for you. What do you suggest to change? (In reply to Katrin Fischer from comment #20) > Marcel, you pointed out some concerns with the patch and you say that Lukas' > comment didn't solve the problem for you. What do you suggest to change? Hi Katrin. No thats not what I said. Please see comment13. This looks like a communication problem to me at this point and I try to resolve. Re-reading your comment doesn't give me any new clue. Could you try to re-phrase? (In reply to Katrin Fischer from comment #22) > This looks like a communication problem to me at this point and I try to > resolve. Re-reading your comment doesn't give me any new clue. Could you try > to re-phrase? We are not testing the actual code in the test plan. We are testing the optional flag which is actually not safe. The test plan should focus on testing the proposed changes, fully using TLS. Code looks good to me btw. (In reply to Marcel de Rooy from comment #23) > (In reply to Katrin Fischer from comment #22) > > This looks like a communication problem to me at this point and I try to > > resolve. Re-reading your comment doesn't give me any new clue. Could you try > > to re-phrase? > > We are not testing the actual code in the test plan. We are testing the > optional flag which is actually not safe. > The test plan should focus on testing the proposed changes, fully using TLS. > > Code looks good to me btw. So the issue with the test plan might be due to problems in making things work with koha-testing-docker. :( We have been testing this patch on a package installation both at KIT and BSZ for a while now. Could the other concern be moved to a separate bug since this patch didn't introduce the flag? (In reply to Katrin Fischer from comment #24) > (In reply to Marcel de Rooy from comment #23) > > (In reply to Katrin Fischer from comment #22) > > > This looks like a communication problem to me at this point and I try to > > > resolve. Re-reading your comment doesn't give me any new clue. Could you try > > > to re-phrase? > > > > We are not testing the actual code in the test plan. We are testing the > > optional flag which is actually not safe. > > The test plan should focus on testing the proposed changes, fully using TLS. > > > > Code looks good to me btw. > > So the issue with the test plan might be due to problems in making things > work with koha-testing-docker. :( We have been testing this patch on a > package installation both at KIT and BSZ for a while now. > > Could the other concern be moved to a separate bug since this patch didn't > introduce the flag? If this is thoroughly tested, I would recommend to record that on the commit message and remove/edit the current test plan which makes not much sense. Put some sign-off lines on it that really count. SUGGESTION * Re-added the BSZ sign-off lines since these got lost at some step. Code wasn't changed, so they are still valid. * Reverted to initial test plan that was not targetted at ktd testing Is this acceptable? ---- Bug 36026: Use only configured TLS options for database connection Database connections with TLS require client private keys and certificates for authentication but MariaDB also supports authentication by user and password. This patch allows omitting the TLS options for certificate based client authentication. This is not easily testable with a development installation. But the patch has been verified by different parties on production and testing systems installed with Debian packages. 0) Apply patch. 1) Configure the database to support TLS connections. 2) Set "<tls>yes</tls>" in the config section in koha-conf.xml. 3) Run "koha-plack --reload <koha_instance>". 4) Open Koha's staff interface in the browser. 5) Observe an internal server error. 6) Apply patch. 7) Repeat step 3 and 4. 8) Observe the error is gone. 9) Run "prove -v t/db_dependent/Koha/Database.t" Sponsored-by: Karlsruhe Institute of Technology (KIT) Signed-off-by: Cornelius Amzar <cornelius.amzar@bsz-bw.de> Signed-off-by: Markus John <markus.john@bsz-bw.de> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Thomas Klausner <domm@plix.at> Go for it (In reply to Marcel de Rooy from comment #27) > Go for it Thanks! Will fix commit message as suggested once I get there. Pushed for 24.11! Well done everyone, thank you! Enhancement, no 24.05.x backport. I have added a manual to the Koha Wiki on how to configure MariaDB and Koha on a separate database server to use secure connections: https://wiki.koha-community.org/wiki/Securing_MariaDB_connections_in_Koha_using_a_separate_database_server (In reply to Lukas Koszyk from comment #31) > I have added a manual to the Koha Wiki on how to configure MariaDB and Koha > on a separate database server to use secure connections: > > https://wiki.koha-community.org/wiki/ > Securing_MariaDB_connections_in_Koha_using_a_separate_database_server Thanks Lukas! (In reply to David Nind from comment #32) > (In reply to Lukas Koszyk from comment #31) > > I have added a manual to the Koha Wiki on how to configure MariaDB and Koha > > on a separate database server to use secure connections: > > > > https://wiki.koha-community.org/wiki/ > > Securing_MariaDB_connections_in_Koha_using_a_separate_database_server > > Thanks Lukas! Very nice, thank you! We just figured out that koha-create doesn't work still. So you will need to set up the encryption **after** setting up the instance. I've filed 2 bugs: - bug 38760 - the koha-create command has no options for setting up an encrypted connection from the start - Bug 39602 - Installer doesn't work with TLS MySQL connection without mutual authentication |