Skip to content

Commit 934d827

Browse files
andrei-pavelandrei-pavel
committed
[#3256] check result of mysql_options
... and throw exception if result is unfortunate. ... and check exceptions thrown in unit tests because it is important now to distinguish that the old exceptions are thrown instead of the new.
1 parent 6e708a1 commit 934d827

File tree

2 files changed

+115
-22
lines changed

2 files changed

+115
-22
lines changed

‎src/lib/mysql/mysql_connection.cc‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -237,12 +237,30 @@ MySqlConnection::openDatabase() {
237237
// If TLS is enabled set it. If something should go wrong it will happen
238238
// later at the mysql_real_connect call.
239239
if (tls_) {
240-
mysql_options(mysql_, MYSQL_OPT_SSL_KEY, key_file);
241-
mysql_options(mysql_, MYSQL_OPT_SSL_CERT, cert_file);
242-
mysql_options(mysql_, MYSQL_OPT_SSL_CA, ca_file);
243-
mysql_options(mysql_, MYSQL_OPT_SSL_CAPATH, ca_dir);
244-
mysql_options(mysql_, MYSQL_OPT_SSL_CIPHER, cipher_list);
240+
result = mysql_options(mysql_, MYSQL_OPT_SSL_KEY, key_file);
241+
if (result != 0) {
242+
isc_throw(DbOpenError, "unable to set key: " << mysql_error(mysql_));
243+
}
245244

245+
result = mysql_options(mysql_, MYSQL_OPT_SSL_CERT, cert_file);
246+
if (result != 0) {
247+
isc_throw(DbOpenError, "unable to set certificate: " << mysql_error(mysql_));
248+
}
249+
250+
result = mysql_options(mysql_, MYSQL_OPT_SSL_CA, ca_file);
251+
if (result != 0) {
252+
isc_throw(DbOpenError, "unable to set CA: " << mysql_error(mysql_));
253+
}
254+
255+
result = mysql_options(mysql_, MYSQL_OPT_SSL_CAPATH, ca_dir);
256+
if (result != 0) {
257+
isc_throw(DbOpenError, "unable to set CA path: " << mysql_error(mysql_));
258+
}
259+
260+
result = mysql_options(mysql_, MYSQL_OPT_SSL_CIPHER, cipher_list);
261+
if (result != 0) {
262+
isc_throw(DbOpenError, "unable to set cipher: " << mysql_error(mysql_));
263+
}
246264
}
247265

248266
// Open the database.

‎src/lib/mysql/tests/mysql_connection_unittest.cc‎

Lines changed: 92 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -609,8 +609,10 @@ TEST_F(MySqlConnectionTest, transactions) {
609609
EXPECT_EQ("4", result[0][0]);
610610

611611
// Committing or rolling back a not started transaction is a coding error.
612-
EXPECT_THROW(conn_.commit(), isc::Unexpected);
613-
EXPECT_THROW(conn_.rollback(), isc::Unexpected);
612+
EXPECT_THROW_MSG(conn_.commit(), isc::Unexpected,
613+
"commit called for not started transaction - coding error");
614+
EXPECT_THROW_MSG(conn_.rollback(), isc::Unexpected,
615+
"rollback called for not started transaction - coding error");
614616
}
615617

616618
// Tests that invalid port value causes an error.
@@ -619,7 +621,8 @@ TEST_F(MySqlConnectionTest, portInvalid) {
619621
VALID_HOST_TCP, VALID_USER,
620622
VALID_PASSWORD, INVALID_PORT_1);
621623
MySqlConnection conn(DatabaseConnection::parse(conn_str));
622-
EXPECT_THROW(conn.openDatabase(), DbInvalidPort);
624+
EXPECT_THROW_MSG(conn.openDatabase(), DbInvalidPort,
625+
"port parameter (65536) must be an integer between 0 and 65535");
623626
}
624627

625628
// Tests that invalid timeout value type causes an error.
@@ -628,7 +631,8 @@ TEST_F(MySqlConnectionTest, connectionTimeoutInvalid) {
628631
VALID_HOST_TCP, VALID_USER,
629632
VALID_PASSWORD, INVALID_TIMEOUT_1);
630633
MySqlConnection conn(DatabaseConnection::parse(conn_str));
631-
EXPECT_THROW(conn.openDatabase(), DbInvalidTimeout);
634+
EXPECT_THROW_MSG(conn.openDatabase(), DbInvalidTimeout,
635+
"connect-timeout parameter (foo) must be an integer between 1 and 2147483647");
632636
}
633637

634638
// Tests that a negative connection timeout value causes an error.
@@ -637,7 +641,8 @@ TEST_F(MySqlConnectionTest, connectionTimeoutInvalid2) {
637641
VALID_HOST_TCP, VALID_USER,
638642
VALID_PASSWORD, INVALID_TIMEOUT_2);
639643
MySqlConnection conn(DatabaseConnection::parse(conn_str));
640-
EXPECT_THROW(conn.openDatabase(), DbInvalidTimeout);
644+
EXPECT_THROW_MSG(conn.openDatabase(), DbInvalidTimeout,
645+
"connect-timeout parameter (-17) must be an integer between 1 and 2147483647");
641646
}
642647

643648
// Tests that a zero connection timeout value causes an error.
@@ -646,7 +651,8 @@ TEST_F(MySqlConnectionTest, connectionTimeoutInvalid3) {
646651
VALID_HOST_TCP, VALID_USER,
647652
VALID_PASSWORD, INVALID_TIMEOUT_3);
648653
MySqlConnection conn(DatabaseConnection::parse(conn_str));
649-
EXPECT_THROW(conn.openDatabase(), DbInvalidTimeout);
654+
EXPECT_THROW_MSG(conn.openDatabase(), DbInvalidTimeout,
655+
"connect-timeout parameter (0) must be an integer between 1 and 2147483647");
650656
}
651657

652658
// Tests that an invalid read timeout causes an error.
@@ -655,7 +661,8 @@ TEST_F(MySqlConnectionTest, readTimeoutInvalid) {
655661
VALID_HOST_TCP, VALID_USER,
656662
VALID_PASSWORD, INVALID_READ_TIMEOUT_1);
657663
MySqlConnection conn(DatabaseConnection::parse(conn_str));
658-
EXPECT_THROW(conn.openDatabase(), DbInvalidTimeout);
664+
EXPECT_THROW_MSG(conn.openDatabase(), DbInvalidTimeout,
665+
"read-timeout parameter (bar) must be an integer between 0 and 2147483647");
659666
}
660667

661668
// Tests that an invalid write timeout causes an error.
@@ -664,7 +671,8 @@ TEST_F(MySqlConnectionTest, writeTimeoutInvalid) {
664671
VALID_HOST_TCP, VALID_USER,
665672
VALID_PASSWORD, INVALID_WRITE_TIMEOUT_1);
666673
MySqlConnection conn(DatabaseConnection::parse(conn_str));
667-
EXPECT_THROW(conn.openDatabase(), DbInvalidTimeout);
674+
EXPECT_THROW_MSG(conn.openDatabase(), DbInvalidTimeout,
675+
"write-timeout parameter (baz) must be an integer between 0 and 2147483647");
668676
}
669677

670678
#ifdef HAVE_MYSQL_GET_OPTION
@@ -875,7 +883,24 @@ TEST_F(MySqlSecureConnectionTest, TlsInvalidPassword) {
875883
VALID_CERT, VALID_KEY, VALID_CA,
876884
VALID_CIPHER);
877885
MySqlConnection conn(DatabaseConnection::parse(conn_str));
878-
EXPECT_THROW(conn.openDatabase(), DbOpenError);
886+
887+
try {
888+
conn.openDatabase();
889+
} catch (DbOpenError const& exception) {
890+
string const message(exception.what());
891+
vector<string> const expected{
892+
"TLS/SSL error: No or insufficient priorities were set.", // OpenSSL 1.1.1n
893+
"Access denied for user 'keatest_secure'",
894+
};
895+
for (string const& i : expected) {
896+
if (message.find(i) != string::npos) {
897+
return;
898+
}
899+
}
900+
ADD_FAILURE() << "Unexpected exception message '" << message << "'";
901+
} catch (exception const& exception) {
902+
ADD_FAILURE() << exception.what();
903+
}
879904
}
880905

881906
/// @brief Check the SSL/TLS protected connection requires crypto parameters.
@@ -885,7 +910,19 @@ TEST_F(MySqlSecureConnectionTest, TlsNoCrypto) {
885910
VALID_HOST_TCP, VALID_SECURE_USER,
886911
VALID_PASSWORD);
887912
MySqlConnection conn(DatabaseConnection::parse(conn_str));
888-
EXPECT_THROW(conn.openDatabase(), DbOpenError);
913+
914+
try {
915+
conn.openDatabase();
916+
} catch (DbOpenError const& exception) {
917+
string const message(exception.what());
918+
string const expected("Access denied for user 'keatest_secure'");
919+
if (message.find(expected) == string::npos) {
920+
ADD_FAILURE()
921+
<< "Expected exception message '" << expected << ".*', got '" << message << "'";
922+
}
923+
} catch (exception const& exception) {
924+
ADD_FAILURE() << exception.what();
925+
}
889926
}
890927

891928
/// @brief Check the SSL/TLS protected connection requires valid key.
@@ -897,7 +934,22 @@ TEST_F(MySqlSecureConnectionTest, TlsInvalidKey) {
897934
VALID_CERT, INVALID_KEY, VALID_CA,
898935
VALID_CIPHER);
899936
MySqlConnection conn(DatabaseConnection::parse(conn_str));
900-
EXPECT_THROW(conn.openDatabase(), DbOpenError);
937+
938+
try {
939+
conn.openDatabase();
940+
} catch (DbOpenError const& exception) {
941+
string const message(exception.what());
942+
vector<string> const expected{
943+
"TLS/SSL error: The certificate and the given key do not match.", // OpenSSL 1.1.1n
944+
"SSL connection error: Unable to get private key", // OpenSSL 3.0.2
945+
"TLS/SSL error: key values mismatch", // OpenSSL 3.3.0
946+
};
947+
if (!std::count(expected.begin(), expected.end(), message)) {
948+
ADD_FAILURE() << "Unexpected exception message '" << message << "'";
949+
}
950+
} catch (exception const& exception) {
951+
ADD_FAILURE() << exception.what();
952+
}
901953
}
902954

903955
/// @brief Check the SSL/TLS protected connection requires a key.
@@ -909,7 +961,24 @@ TEST_F(MySqlSecureConnectionTest, TlsNoKey) {
909961
VALID_CERT, 0, VALID_CA,
910962
VALID_CIPHER);
911963
MySqlConnection conn(DatabaseConnection::parse(conn_str));
912-
EXPECT_THROW(conn.openDatabase(), DbOpenError);
964+
965+
try {
966+
conn.openDatabase();
967+
} catch (DbOpenError const& exception) {
968+
string const message(exception.what());
969+
vector<string> const expected{
970+
"TLS/SSL error: The requested data were not available.", // OpenSSL 1.1.1n
971+
"TLS/SSL error: no start line", // OpenSSL 1.1.1w
972+
"SSL connection error: Unable to get private key", // OpenSSL 3.0.2
973+
"TLS/SSL error: no certificate assigned", // OpenSSL 3.0.9
974+
"TLS/SSL error: unsupported", // OpenSSL 3.3.0
975+
};
976+
if (!std::count(expected.begin(), expected.end(), message)) {
977+
ADD_FAILURE() << "Unexpected exception message '" << message << "'";
978+
}
979+
} catch (exception const& exception) {
980+
ADD_FAILURE() << exception.what();
981+
}
913982
}
914983

915984
/// @brief Check ensureSchemaVersion when schema is not created.
@@ -1016,11 +1085,17 @@ TEST_F(MySqlConnectionTest, toKeaAdminParameters) {
10161085
/// mysql_options() directly, omitting calls for those options for which the option value is NULL.
10171086
TEST_F(MySqlConnectionTest, mysqlOptions) {
10181087
MySqlHolder mysql;
1019-
mysql_options(mysql, MYSQL_OPT_SSL_KEY, nullptr);
1020-
mysql_options(mysql, MYSQL_OPT_SSL_CERT, nullptr);
1021-
mysql_options(mysql, MYSQL_OPT_SSL_CA, nullptr);
1022-
mysql_options(mysql, MYSQL_OPT_SSL_CAPATH, nullptr);
1023-
mysql_options(mysql, MYSQL_OPT_SSL_CIPHER, nullptr);
1088+
int result;
1089+
EXPECT_NO_THROW_LOG(result = mysql_options(mysql, MYSQL_OPT_SSL_KEY, nullptr));
1090+
EXPECT_EQ(0, result);
1091+
EXPECT_NO_THROW_LOG(result = mysql_options(mysql, MYSQL_OPT_SSL_CERT, nullptr));
1092+
EXPECT_EQ(0, result);
1093+
EXPECT_NO_THROW_LOG(result = mysql_options(mysql, MYSQL_OPT_SSL_CA, nullptr));
1094+
EXPECT_EQ(0, result);
1095+
EXPECT_NO_THROW_LOG(result = mysql_options(mysql, MYSQL_OPT_SSL_CAPATH, nullptr));
1096+
EXPECT_EQ(0, result);
1097+
EXPECT_NO_THROW_LOG(result = mysql_options(mysql, MYSQL_OPT_SSL_CIPHER, nullptr));
1098+
EXPECT_EQ(0, result);
10241099
}
10251100

10261101
} // namespace

0 commit comments

Comments
 (0)