From: Francis Dupont Date: Mon, 20 Dec 2021 15:30:39 +0000 (+0100) Subject: [#34] Addressed almost all comments X-Git-Tag: Kea-2.1.2~149 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=875dbd68e5b79f2708362aab6a2f538468c10e80;p=thirdparty%2Fkea.git [#34] Addressed almost all comments --- diff --git a/ChangeLog b/ChangeLog index 93111f3357..0c6a607102 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,8 +4,8 @@ database parameters are "cert-file", "key-file", "trust-anchor" and "cipher-list". The negotiated cipher name is logged so the MySQL service configuration can - be checked... PostgreSQL accepts the same parameters - but they only trigger on a call to the OpenSSL generic + be checked. PostgreSQL accepts the same parameters + but they only trigger the call to the OpenSSL generic initialization in the Pq C-API. (Gitlab #34) diff --git a/doc/devel/unit-tests.dox b/doc/devel/unit-tests.dox index e843c8c69f..7bb6032138 100644 --- a/doc/devel/unit-tests.dox +++ b/doc/devel/unit-tests.dox @@ -201,7 +201,7 @@ anything e.g. `DEBUG=true`. `unset DEBUG` to remove this behavior. section in the Kea Administrator Reference Manual). - @subsection mysqlUnitTestsILS MySQL Database with SSL/TLS + @subsection mysqlUnitTestsTLS MySQL Database with SSL/TLS Usually MySQL is compiled with SSL/TLS support using OpenSSL. This is easy to verify using the: diff --git a/doc/examples/kea4/all-keys.json b/doc/examples/kea4/all-keys.json index ef624eb613..cff480b595 100644 --- a/doc/examples/kea4/all-keys.json +++ b/doc/examples/kea4/all-keys.json @@ -327,9 +327,9 @@ "cert-file": "my-cert", // Private key file name. - "key-file": "my key", + "key-file": "my-key", - // Cipher list (see the OpenSSL ciohers command manual). + // Cipher list (see the OpenSSL ciphers command manual). "cipher-list": "AES" }, { diff --git a/doc/examples/kea6/all-keys.json b/doc/examples/kea6/all-keys.json index 02ecb3f242..8d45fbc070 100644 --- a/doc/examples/kea6/all-keys.json +++ b/doc/examples/kea6/all-keys.json @@ -287,9 +287,9 @@ "cert-file": "my-cert", // Private key file name. - "key-file": "my key", + "key-file": "my-key", - // Cipher list (see the OpenSSL ciohers command manual). + // Cipher list (see the OpenSSL ciphers command manual). "cipher-list": "AES" }, { diff --git a/doc/sphinx/arm/admin.rst b/doc/sphinx/arm/admin.rst index 0ed60e90d1..82d454d496 100644 --- a/doc/sphinx/arm/admin.rst +++ b/doc/sphinx/arm/admin.rst @@ -589,12 +589,12 @@ PostgreSQL without OpenSSL support ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Usually the PostgreSQL database client library is built with the OpenSSL -support but in the case it was not Kea can be configured to handle this -case: +support but Kea can be configured to handle the case where it is not +supported: .. code-block:: console - $ ./configure [other-options] --disable-pgsql-ssl + $ ./configure [other-options] --disable-pgsql-ssl .. _cql-database: diff --git a/doc/sphinx/arm/database-connectivity.rst b/doc/sphinx/arm/database-connectivity.rst index 5a4cdd5df7..186b55e8a7 100644 --- a/doc/sphinx/arm/database-connectivity.rst +++ b/doc/sphinx/arm/database-connectivity.rst @@ -75,7 +75,7 @@ Currently the support for each database is: - MySQL supports the whole set, additional configuration must be done in the MySQL local setup, for instance certificate revocation list, - choice of a specific TLS version, mutual authentication, ... + choice of a specific TLS version, mutual authentication, etc. When a TLS connection was required but the actual connection is in clear text an error log is emitted. diff --git a/src/lib/database/testutils/schema.cc b/src/lib/database/testutils/schema.cc index 951c8a861a..1edc48209c 100644 --- a/src/lib/database/testutils/schema.cc +++ b/src/lib/database/testutils/schema.cc @@ -43,9 +43,9 @@ const char* VALID_CIPHER = "cipher-list=AES"; string connectionString(const char* type, const char* name, const char* host, const char* user, const char* password, - const char* timeout, const char* readonly_db, - const char* cert_file, const char* key_file, - const char* trust_anchor, const char* cipher) { + const char* timeout, const char* readonly_db, + const char* cert_file, const char* key_file, + const char* trust_anchor, const char* cipher) { const string space = " "; string result = ""; @@ -136,6 +136,6 @@ softWipeEnabled() { return (true); } -}; -}; -}; +} +} +} diff --git a/src/lib/database/testutils/schema.h b/src/lib/database/testutils/schema.h index 7dce7cbb72..41139ea38f 100644 --- a/src/lib/database/testutils/schema.h +++ b/src/lib/database/testutils/schema.h @@ -55,10 +55,10 @@ std::string connectionString(const char* type, const char* name = 0, const char* host = 0, const char* user = 0, const char* password = 0, const char* timeout = 0, const char* readonly_db = 0, - const char* cert_file = 0, - const char* key_file = 0, - const char* trust_anchor = 0, - const char* cipher = 0); + const char* cert_file = 0, + const char* key_file = 0, + const char* trust_anchor = 0, + const char* cipher = 0); /// @brief Determines if wiping only the data between tests is enabled /// @@ -66,8 +66,8 @@ std::string connectionString(const char* type, const char* name = 0, /// defined as "true" or if it is not present. bool softWipeEnabled(); -}; -}; -}; +} +} +} #endif diff --git a/src/lib/dhcpsrv/benchmarks/Makefile.am b/src/lib/dhcpsrv/benchmarks/Makefile.am index 362c923b11..b2b15bf4fb 100644 --- a/src/lib/dhcpsrv/benchmarks/Makefile.am +++ b/src/lib/dhcpsrv/benchmarks/Makefile.am @@ -83,6 +83,7 @@ run_benchmarks_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la if HAVE_MYSQL run_benchmarks_LDADD += $(top_builddir)/src/lib/mysql/testutils/libmysqltest.la +run_benchmarks_LDADD += $(top_builddir)/src/lib/mysql/libkea-mysql.la endif if HAVE_PGSQL diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index daa0b9e8d4..5eb64944a4 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -1218,7 +1218,7 @@ This message is issued only once. % DHCPSRV_PGSQL_NO_TLS_SUPPORT Attempt to configure TLS (unsupported for PostgreSQL): %1 This error message is printed when TLS support was required in the Kea -configuration: Kea was built with this feature was disabled for PostgreSQL. +configuration: Kea was built with this feature disabled for PostgreSQL. The parameters of the connection are logged. % DHCPSRV_PGSQL_ROLLBACK rolling back PostgreSQL database diff --git a/src/lib/mysql/mysql_connection.h b/src/lib/mysql/mysql_connection.h index bac7220530..2328fbf3fb 100644 --- a/src/lib/mysql/mysql_connection.h +++ b/src/lib/mysql/mysql_connection.h @@ -704,7 +704,7 @@ public: return (tls_); } - /// @brief Get the TLS cipher + /// @brief Get the TLS cipher. /// /// This method is used to check if required TLS was setup. std::string getTlsCipher() { diff --git a/src/lib/mysql/tests/mysql_connection_unittest.cc b/src/lib/mysql/tests/mysql_connection_unittest.cc index eb83ed2f13..c8106fe5ff 100644 --- a/src/lib/mysql/tests/mysql_connection_unittest.cc +++ b/src/lib/mysql/tests/mysql_connection_unittest.cc @@ -658,7 +658,7 @@ TEST_F(MySqlSchemaTest, checkVersion) { EXPECT_EQ(MYSQL_SCHEMA_VERSION_MINOR, version.second); } -/// @brief Test fixture class for secure connection. +/// @brief Test fixture class for secure connection. class MySqlSecureConnectionTest : public ::testing::Test { public: @@ -666,7 +666,7 @@ public: bool hasMySQLTls() { std::string tls = getMySQLTlsEnv(); if (tls.empty()) { - tls = getMySQLTlsEnv(); + tls = getMySQLTlsServer(); } return (tls == "YES"); } @@ -687,7 +687,7 @@ TEST_F(MySqlSecureConnectionTest, getMySQLTls) { try { std::cout << "getMySQLTlsServer returns '" << getMySQLTlsServer() << "'\n"; } catch (const isc::Exception& ex) { - std::cerr << "getMySQLTlsServer fails with " << ex.what() << "\n"; + std::cerr << "getMySQLTlsServer fails with " << ex.what() << "\n"; } } diff --git a/src/lib/pgsql/pgsql_connection.h b/src/lib/pgsql/pgsql_connection.h index 4135d2951e..8cc7c212fa 100644 --- a/src/lib/pgsql/pgsql_connection.h +++ b/src/lib/pgsql/pgsql_connection.h @@ -203,7 +203,7 @@ public: /// @brief Function invoked to process fetched row. typedef std::function ConsumeResultRowFun; - /// @brief Emit once the TLS support warning. + /// @brief Emit the TLS support warning only once. static bool warned_about_tls; /// @brief Constructor diff --git a/src/lib/util/file_utilities.h b/src/lib/util/file_utilities.h index dc925a1400..f1b6275c6d 100644 --- a/src/lib/util/file_utilities.h +++ b/src/lib/util/file_utilities.h @@ -23,7 +23,7 @@ std::string getContent(const std::string& file_name); /// @brief Is a directory predicate. /// /// @param name The file or directory name. -/// @return True if the name points to a directory, false if not including +/// @return True if the name points to a directory, false otherwise including /// if the pointed location does not exist. bool isDir(const std::string& name);