From 3a4e989317a7c662b5319c0f3e3643104cc4e724 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Thu, 31 May 2018 11:51:19 -0400 Subject: [PATCH] [5629] MySQL and PostgreSQL host backends now verify schema version src/lib/dhcpsrv/mysql_host_data_source.cc MySqlHostDataSourceImpl::getVersion() - new function to fetch version accessible to impl constructor MySqlHostDataSourceImpl() - added schema validation after connecting MySqlHostDataSource::getVersion() - new uses impl method src/lib/dhcpsrv/pgsql_host_data_source.cc PgSqlHostDataSourceImpl()) - added schema validation after connecting PgSqlHostDataSourceImpl::getVersion() - no longer uses pre-prepared statement --- src/lib/dhcpsrv/mysql_host_data_source.cc | 141 ++++++++++++++-------- src/lib/dhcpsrv/mysql_host_data_source.h | 3 +- src/lib/dhcpsrv/mysql_lease_mgr.cc | 28 +++-- src/lib/dhcpsrv/pgsql_host_data_source.cc | 30 +++-- src/lib/dhcpsrv/pgsql_host_data_source.h | 5 + src/lib/dhcpsrv/pgsql_lease_mgr.cc | 2 +- 6 files changed, 128 insertions(+), 81 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 6d2c2d032a..c4dfae044b 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -1906,7 +1906,6 @@ public: GET_HOST_SUBID_ADDR, // Gets host by IPv4 SubnetID and IPv4 address GET_HOST_PREFIX, // Gets host by IPv6 prefix GET_HOST_SUBID6_ADDR, // Gets host by IPv6 SubnetID and IPv6 prefix - GET_VERSION, // Obtain version number INSERT_HOST, // Insert new host to collection INSERT_V6_RESRV, // Insert v6 reservation INSERT_V4_OPTION, // Insert DHCPv4 option @@ -1933,6 +1932,21 @@ public: /// @brief Destructor. ~MySqlHostDataSourceImpl(); + /// @brief Returns backend version. + /// + /// The method is called by the constructor after opening the database + /// but prior to preparing SQL statements, to verify that the schema version + /// is correct. Thus it must not rely on a pre-prepared statement or + /// formal statement execution error checking. + // + /// @return Version number stored in the database, as a pair of unsigned + /// integers. "first" is the major version number, "second" the + /// minor number. + /// + /// @throw isc::dhcp::DbOperationError An operation on the open database + /// has failed. + std::pair getVersion() const; + /// @brief Executes statements which inserts a row into one of the tables. /// /// @param stindex Index of a statement being executed. @@ -2232,10 +2246,6 @@ TaggedStatementArray tagged_statements = { { "WHERE h.dhcp6_subnet_id = ? AND r.address = ? " "ORDER BY h.host_id, o.option_id, r.reservation_id"}, - // Retrieves MySQL schema version. - {MySqlHostDataSourceImpl::GET_VERSION, - "SELECT version, minor FROM schema_version"}, - // Inserts a host into the 'hosts' table. {MySqlHostDataSourceImpl::INSERT_HOST, "INSERT INTO hosts(host_id, dhcp_identifier, dhcp_identifier_type, " @@ -2293,6 +2303,17 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) // Open the database. conn_.openDatabase(); + // Test schema version before we try to prepare statements. + std::pair code_version(MYSQL_SCHEMA_VERSION_MAJOR, + MYSQL_SCHEMA_VERSION_MINOR); + std::pair db_version = getVersion(); + if (code_version != db_version) { + isc_throw(DbOpenError, "MySQL schema version mismatch: need version: " + << code_version.first << "." << code_version.second + << " found version: " << db_version.first << "." + << db_version.second); + } + // Enable autocommit. In case transaction is explicitly used, this // setting will be overwritten for the transaction. However, there are // cases when lack of autocommit could cause transactions to hang @@ -2341,6 +2362,67 @@ MySqlHostDataSourceImpl::~MySqlHostDataSourceImpl() { // closed in the destructor of the mysql_ member variable. } +std::pair +MySqlHostDataSourceImpl::getVersion() const { + LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, + DHCPSRV_MYSQL_HOST_DB_GET_VERSION); + + // Allocate a new statement. + MYSQL_STMT *stmt = mysql_stmt_init(conn_.mysql_); + if (stmt == NULL) { + isc_throw(DbOperationError, "unable to allocate MySQL prepared " + "statement structure, reason: " << mysql_error(conn_.mysql_)); + } + + // Prepare the statement from SQL text. + const char* version_sql = "SELECT version, minor FROM schema_version"; + int status = mysql_stmt_prepare(stmt, version_sql, strlen(version_sql)); + if (status != 0) { + isc_throw(DbOperationError, "unable to prepare MySQL statement <" + << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + } + + // Execute the prepared statement. + if (mysql_stmt_execute(stmt) != 0) { + isc_throw(DbOperationError, "cannot execute schema version query <" + << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + } + + // Bind the output of the statement to the appropriate variables. + MYSQL_BIND bind[2]; + memset(bind, 0, sizeof(bind)); + + uint32_t major; + bind[0].buffer_type = MYSQL_TYPE_LONG; + bind[0].is_unsigned = 1; + bind[0].buffer = &major; + bind[0].buffer_length = sizeof(major); + + uint32_t minor; + bind[1].buffer_type = MYSQL_TYPE_LONG; + bind[1].is_unsigned = 1; + bind[1].buffer = &minor; + bind[1].buffer_length = sizeof(minor); + + if (mysql_stmt_bind_result(stmt, bind)) { + isc_throw(DbOperationError, "unable to bind result set for <" + << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + } + + // Fetch the data. + if (mysql_stmt_fetch(stmt)) { + mysql_stmt_close(stmt); + isc_throw(DbOperationError, "unable to bind result set for <" + << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + } + + // Discard the statement and its resources + mysql_stmt_close(stmt); + + return (std::make_pair(major, minor)); +} + + void MySqlHostDataSourceImpl::addStatement(StatementIndex stindex, std::vector& bind) { @@ -2977,54 +3059,7 @@ std::string MySqlHostDataSource::getDescription() const { } std::pair MySqlHostDataSource::getVersion() const { - const MySqlHostDataSourceImpl::StatementIndex stindex = - MySqlHostDataSourceImpl::GET_VERSION; - - LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, - DHCPSRV_MYSQL_HOST_DB_GET_VERSION); - - uint32_t major; // Major version number - uint32_t minor; // Minor version number - - // Execute the prepared statement - int status = mysql_stmt_execute(impl_->conn_.statements_[stindex]); - if (status != 0) { - isc_throw(DbOperationError, "unable to execute <" - << impl_->conn_.text_statements_[stindex] - << "> - reason: " << mysql_error(impl_->conn_.mysql_)); - } - - // Bind the output of the statement to the appropriate variables. - MYSQL_BIND bind[2]; - memset(bind, 0, sizeof(bind)); - - bind[0].buffer_type = MYSQL_TYPE_LONG; - bind[0].is_unsigned = 1; - bind[0].buffer = &major; - bind[0].buffer_length = sizeof(major); - - bind[1].buffer_type = MYSQL_TYPE_LONG; - bind[1].is_unsigned = 1; - bind[1].buffer = &minor; - bind[1].buffer_length = sizeof(minor); - - status = mysql_stmt_bind_result(impl_->conn_.statements_[stindex], bind); - if (status != 0) { - isc_throw(DbOperationError, "unable to bind result set: " - << mysql_error(impl_->conn_.mysql_)); - } - - // Fetch the data and set up the "release" object to release associated - // resources when this method exits then retrieve the data. - // mysql_stmt_fetch return value other than 0 means error occurrence. - MySqlFreeResult fetch_release(impl_->conn_.statements_[stindex]); - status = mysql_stmt_fetch(impl_->conn_.statements_[stindex]); - if (status != 0) { - isc_throw(DbOperationError, "unable to obtain result set: " - << mysql_error(impl_->conn_.mysql_)); - } - - return (std::make_pair(major, minor)); + return(impl_->getVersion()); } void diff --git a/src/lib/dhcpsrv/mysql_host_data_source.h b/src/lib/dhcpsrv/mysql_host_data_source.h index 8476284aa3..874a9ac163 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.h +++ b/src/lib/dhcpsrv/mysql_host_data_source.h @@ -49,7 +49,8 @@ public: /// concerned with the database. /// /// @throw isc::dhcp::NoDatabaseName Mandatory database name not given - /// @throw isc::dhcp::DbOpenError Error opening the database + /// @throw isc::dhcp::DbOpenError Error opening the database or the + /// schema version is invalid. /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. MySqlHostDataSource(const DatabaseConnection::ParameterMap& parameters); diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 4367a389e5..93593e9054 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1500,13 +1500,15 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) conn_.openDatabase(); // Test schema version before we try to prepare statements. - std::pair version = getVersion(); - if (version.first != MYSQL_SCHEMA_VERSION_MAJOR || - version.second != MYSQL_SCHEMA_VERSION_MINOR) { - isc_throw(DbOpenError, "MySQL schema version is: " - << version.first << "." << version.second << ", need version: " - << MYSQL_SCHEMA_VERSION_MAJOR << "." - << MYSQL_SCHEMA_VERSION_MINOR); + std::pair code_version(MYSQL_SCHEMA_VERSION_MAJOR, + MYSQL_SCHEMA_VERSION_MINOR); + std::pair db_version = getVersion(); + if (code_version != db_version) { + isc_throw(DbOpenError, + "MySQL schema version mismatch: need version: " + << code_version.first << "." << code_version.second + << " found version: " << db_version.first << "." + << db_version.second); } // Enable autocommit. To avoid a flush to disk on every commit, the global @@ -2400,8 +2402,8 @@ MySqlLeaseMgr::getVersion() const { // Execute the prepared statement. if (mysql_stmt_execute(stmt) != 0) { - isc_throw(DbOperationError, "cannot execute schema version query:" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")"); + isc_throw(DbOperationError, "cannot execute schema version query <" + << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); } // Bind the output of the statement to the appropriate variables. @@ -2421,15 +2423,15 @@ MySqlLeaseMgr::getVersion() const { bind[1].buffer_length = sizeof(minor); if (mysql_stmt_bind_result(stmt, bind)) { - isc_throw(DbOperationError, "unable to bind result set for:" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")"); + isc_throw(DbOperationError, "unable to bind result set for <" + << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); } // Fetch the data. if (mysql_stmt_fetch(stmt)) { mysql_stmt_close(stmt); - isc_throw(DbOperationError, "unable to bind result set for:" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")"); + isc_throw(DbOperationError, "unable to bind result set for <" + << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); } // Discard the statement and its resources diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index ae632ef22b..91a207efbf 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -1259,7 +1259,6 @@ public: GET_HOST_SUBID_ADDR, // Gets host by IPv4 SubnetID and IPv4 address GET_HOST_PREFIX, // Gets host by IPv6 prefix GET_HOST_SUBID6_ADDR, // Gets host by IPv6 SubnetID and IPv6 prefix - GET_VERSION, // Obtain version number INSERT_HOST, // Insert new host to collection INSERT_V6_RESRV, // Insert v6 reservation INSERT_V4_HOST_OPTION, // Insert DHCPv4 option @@ -1606,14 +1605,6 @@ TaggedStatementArray tagged_statements = { { "ORDER BY h.host_id, o.option_id, r.reservation_id" }, - // PgSqlHostDataSourceImpl::GET_VERSION - // Retrieves PgSQL schema version. - {0, - { OID_NONE }, - "get_version", - "SELECT version, minor FROM schema_version" - }, - // PgSqlHostDataSourceImpl::INSERT_HOST // Inserts a host into the 'hosts' table. Returns the inserted host id. {12, @@ -1709,6 +1700,16 @@ PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters) // Open the database. conn_.openDatabase(); + pair code_version(PG_SCHEMA_VERSION_MAJOR, PG_SCHEMA_VERSION_MINOR); + pair db_version = getVersion(); + if (code_version != db_version) { + isc_throw(DbOpenError, + "PostgreSQL schema version mismatch: need version: " + << code_version.first << "." << code_version.second + << " found version: " << db_version.first << "." + << db_version.second); + } + conn_.prepareStatements(tagged_statements.begin(), tagged_statements.begin() + WRITE_STMTS_BEGIN); @@ -1896,15 +1897,18 @@ getHost(const SubnetID& subnet_id, std::pair PgSqlHostDataSourceImpl::getVersion() const { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_HOST_DB_GET_VERSION); - - PgSqlResult r(PQexecPrepared(conn_, "get_version", 0, NULL, NULL, NULL, 0)); - conn_.checkStatementError(r, tagged_statements[GET_VERSION]); + const char* version_sql = "SELECT version, minor FROM schema_version;"; + PgSqlResult r(PQexec(conn_, version_sql)); + if(PQresultStatus(r) != PGRES_TUPLES_OK) { + isc_throw(DbOperationError, "unable to execute PostgreSQL statement <" + << version_sql << ">, reason: " << PQerrorMessage(conn_)); + } uint32_t version; PgSqlExchange::getColumnValue(r, 0, 0, version); uint32_t minor; - PgSqlExchange::getColumnValue(r, 0, 0, minor); + PgSqlExchange::getColumnValue(r, 0, 1, minor); return (std::make_pair(version, minor)); } diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.h b/src/lib/dhcpsrv/pgsql_host_data_source.h index 5a270b449c..384ff49b70 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.h +++ b/src/lib/dhcpsrv/pgsql_host_data_source.h @@ -317,6 +317,11 @@ public: /// @brief Returns backend version. /// + /// The method is called by the constructor after opening the database + /// but prior to preparing SQL statements, to verify that the schema version + /// is correct. Thus it must not rely on a pre-prepared statement or + /// formal statement execution error checking. + /// /// @return Version number stored in the database, as a pair of unsigned /// integers. "first" is the major version number, "second" the /// minor number. diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index f976c5eeb3..7e368db778 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -1680,7 +1680,7 @@ PgSqlLeaseMgr::getVersion() const { const char* version_sql = "SELECT version, minor FROM schema_version;"; PgSqlResult r(PQexec(conn_, version_sql)); if(PQresultStatus(r) != PGRES_TUPLES_OK) { - isc_throw(DbOperationError, "unable to execute PostgreSQL statement: " + isc_throw(DbOperationError, "unable to execute PostgreSQL statement <" << version_sql << ", reason: " << PQerrorMessage(conn_)); } -- 2.47.2