From: Thomas Markwalder <tmark@isc.org>
Date: Thu, 31 May 2018 14:36:26 +0000 (-0400)
Subject: [5629] PostgreSQL Lease back end now properly validates schema version
X-Git-Tag: trac5631_base~7^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28e33a3c2dcf25c6d3d00fe703ce8366eea41018;p=thirdparty%2Fkea.git

[5629] PostgreSQL Lease back end now properly validates schema version

src/lib/dhcpsrv/pgsql_lease_mgr.cc
    PgSqlLeaseMgr::PgSqlLeaseMgr() - now validates schema version before
    preparing SQL statements

    PgSqlLeaseMgr::getVersion() - no longers uses pre-prepared statement
    or formal statement execution error checking
---

diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h
index 497ed2104c..38615124c4 100644
--- a/src/lib/dhcpsrv/mysql_lease_mgr.h
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.h
@@ -475,7 +475,7 @@ public:
     ///
     /// 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 does not rely on a pre-prepared statement or
+    /// is correct. Thus it must not rely on a pre-prepared statement or
     /// formal statement execution error checking.
     ///
     /// @return Version number as a pair of unsigned integers.  "first" is the
diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc
index 1929bae8ed..f976c5eeb3 100644
--- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc
@@ -202,11 +202,6 @@ PgSqlTaggedStatement tagged_statements[] = {
       "ORDER BY expire "
       "LIMIT $3"},
 
-    // GET_VERSION
-    { 0, { OID_NONE },
-      "get_version",
-      "SELECT version, minor FROM schema_version"},
-
     // INSERT_LEASE4
     { 10, { OID_INT8, OID_BYTEA, OID_BYTEA, OID_INT8, OID_TIMESTAMP, OID_INT8,
             OID_BOOL, OID_BOOL, OID_VARCHAR, OID_INT8 },
@@ -1000,16 +995,6 @@ PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters)
     : LeaseMgr(), exchange4_(new PgSqlLease4Exchange()),
     exchange6_(new PgSqlLease6Exchange()), conn_(parameters) {
     conn_.openDatabase();
-    int i = 0;
-    for( ; tagged_statements[i].text != NULL ; ++i) {
-        conn_.prepareStatement(tagged_statements[i]);
-    }
-
-    // Just in case somebody foo-barred things
-    if (i != NUM_STATEMENTS) {
-        isc_throw(DbOpenError, "Number of statements prepared: " << i
-                  << " does not match expected count:" << NUM_STATEMENTS);
-    }
 
     pair<uint32_t, uint32_t> code_version(PG_SCHEMA_VERSION_MAJOR, PG_SCHEMA_VERSION_MINOR);
     pair<uint32_t, uint32_t> db_version = getVersion();
@@ -1020,6 +1005,17 @@ PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters)
                       << " found version:  " << db_version.first << "."
                       << db_version.second);
     }
+
+    int i = 0;
+    for( ; tagged_statements[i].text != NULL ; ++i) {
+        conn_.prepareStatement(tagged_statements[i]);
+    }
+
+    // Just in case somebody foo-barred things
+    if (i != NUM_STATEMENTS) {
+        isc_throw(DbOpenError, "Number of statements prepared: " << i
+                  << " does not match expected count:" << NUM_STATEMENTS);
+    }
 }
 
 PgSqlLeaseMgr::~PgSqlLeaseMgr() {
@@ -1681,8 +1677,12 @@ PgSqlLeaseMgr::getVersion() const {
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
               DHCPSRV_PGSQL_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_));
+    }
 
     istringstream tmp;
     uint32_t version;
diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h
index e36abf02c3..79353dfcdc 100644
--- a/src/lib/dhcpsrv/pgsql_lease_mgr.h
+++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h
@@ -442,6 +442,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 as a pair of unsigned integers.  "first" is the
     ///         major version number, "second" the minor number.
     ///
@@ -486,7 +491,6 @@ public:
         GET_LEASE6_DUID_IAID_SUBID, // Get lease6 by DUID, IAID and subnet ID
         GET_LEASE6_SUBID,           // Get IPv6 leases by subnet ID
         GET_LEASE6_EXPIRE,          // Get expired lease6
-        GET_VERSION,                // Obtain version number
         INSERT_LEASE4,              // Add entry to lease4 table
         INSERT_LEASE6,              // Add entry to lease6 table
         UPDATE_LEASE4,              // Update a Lease4 entry