Skip to content

Commit f4065d8

Browse files
luipirluipir
authoredJun 11, 2018
Merge pull request #6647 from luipir/certs_not_removed_with_pki_postgis_on_win_port3
Certs not removed with pki postgis on win port3
2 parents 5567223 + 1743451 commit f4065d8

File tree

6 files changed

+378
-37
lines changed

6 files changed

+378
-37
lines changed
 

‎.ci/travis/linux/blacklist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,5 @@ PyQgsServerAccessControl
2929
PyQgsAuthManagerPKIPostgresTest
3030
PyQgsAuthManagerPasswordPostgresTest
3131
PyQgsAuthManagerOgrPostgresTest
32+
PyQgsDbManagerPostgis
3233

‎python/plugins/db_manager/db_plugins/postgis/connector.py

Lines changed: 33 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
from functools import cmp_to_key
2828

29-
from qgis.PyQt.QtCore import QRegExp
29+
from qgis.PyQt.QtCore import QRegExp, QFile
3030
from qgis.core import Qgis, QgsCredentials, QgsDataSourceUri
3131

3232
from ..connector import DBConnector
@@ -66,6 +66,7 @@ def __init__(self, uri):
6666
try:
6767
self.connection = psycopg2.connect(expandedConnInfo)
6868
except self.connection_error_types() as e:
69+
# get credentials if cached or asking to the user no more than 3 times
6970
err = str(e)
7071
uri = self.uri()
7172
conninfo = uri.connectionInfo(False)
@@ -88,44 +89,13 @@ def __init__(self, uri):
8889
except self.connection_error_types() as e:
8990
if i == 2:
9091
raise ConnectionError(e)
91-
9292
err = str(e)
9393
finally:
94-
# remove certs (if any) of the expanded connectionInfo
95-
expandedUri = QgsDataSourceUri(newExpandedConnInfo)
96-
97-
sslCertFile = expandedUri.param("sslcert")
98-
if sslCertFile:
99-
sslCertFile = sslCertFile.replace("'", "")
100-
os.remove(sslCertFile)
101-
102-
sslKeyFile = expandedUri.param("sslkey")
103-
if sslKeyFile:
104-
sslKeyFile = sslKeyFile.replace("'", "")
105-
os.remove(sslKeyFile)
106-
107-
sslCAFile = expandedUri.param("sslrootcert")
108-
if sslCAFile:
109-
sslCAFile = sslCAFile.replace("'", "")
110-
os.remove(sslCAFile)
94+
# clear certs for each time trying to connect
95+
self._clearSslTempCertsIfAny(newExpandedConnInfo)
11196
finally:
112-
# remove certs (if any) of the expanded connectionInfo
113-
expandedUri = QgsDataSourceUri(expandedConnInfo)
114-
115-
sslCertFile = expandedUri.param("sslcert")
116-
if sslCertFile:
117-
sslCertFile = sslCertFile.replace("'", "")
118-
os.remove(sslCertFile)
119-
120-
sslKeyFile = expandedUri.param("sslkey")
121-
if sslKeyFile:
122-
sslKeyFile = sslKeyFile.replace("'", "")
123-
os.remove(sslKeyFile)
124-
125-
sslCAFile = expandedUri.param("sslrootcert")
126-
if sslCAFile:
127-
sslCAFile = sslCAFile.replace("'", "")
128-
os.remove(sslCAFile)
97+
# clear certs of the first connection try
98+
self._clearSslTempCertsIfAny(expandedConnInfo)
12999

130100
self.connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
131101

@@ -141,6 +111,33 @@ def __init__(self, uri):
141111
def _connectionInfo(self):
142112
return str(self.uri().connectionInfo(True))
143113

114+
def _clearSslTempCertsIfAny(self, connectionInfo):
115+
# remove certs (if any) of the connectionInfo
116+
expandedUri = QgsDataSourceUri(connectionInfo)
117+
118+
def removeCert(certFile):
119+
certFile = certFile.replace("'", "")
120+
file = QFile(certFile)
121+
# set permission to allow removing on Win.
122+
# On linux and Mac if file is set with QFile::>ReadUser
123+
# does not create problem removing certs
124+
if not file.setPermissions(QFile.WriteOwner):
125+
raise Exception('Cannot change permissions on {}: error code: {}'.format(file.fileName(), file.error()))
126+
if not file.remove():
127+
raise Exception('Cannot remove {}: error code: {}'.format(file.fileName(), file.error()))
128+
129+
sslCertFile = expandedUri.param("sslcert")
130+
if sslCertFile:
131+
removeCert(sslCertFile)
132+
133+
sslKeyFile = expandedUri.param("sslkey")
134+
if sslKeyFile:
135+
removeCert(sslKeyFile)
136+
137+
sslCAFile = expandedUri.param("sslrootcert")
138+
if sslCAFile:
139+
removeCert(sslCAFile)
140+
144141
def _checkSpatial(self):
145142
""" check whether postgis_version is present in catalog """
146143
c = self._execute(None, u"SELECT COUNT(*) FROM pg_proc WHERE proname = 'postgis_version'")

‎src/providers/postgres/qgspostgresconn.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,26 @@ QgsPostgresConn::QgsPostgresConn( const QString &conninfo, bool readOnly, bool s
242242
{
243243
QString fileName = expandedUri.param( param );
244244
fileName.remove( QStringLiteral( "'" ) );
245-
QFile::remove( fileName );
245+
QFile file( fileName );
246+
// set minimal permission to allow removing on Win.
247+
// On linux and Mac if file is set with QFile::ReadUser
248+
// does not create problem removing certs
249+
if ( !file.setPermissions( QFile::WriteOwner ) )
250+
{
251+
QString errorMsg = tr( "Cannot set WriteOwner permission to cert: %0 to allow removing it" ).arg( file.fileName() );
252+
PQfinish();
253+
QgsMessageLog::logMessage( tr( "Client security failure" ) + '\n' + errorMsg, tr( "PostGIS" ) );
254+
mRef = 0;
255+
return;
256+
}
257+
if ( !file.remove() )
258+
{
259+
QString errorMsg = tr( "Cannot remove cert: %0" ).arg( file.fileName() );
260+
PQfinish();
261+
QgsMessageLog::logMessage( tr( "Client security failure" ) + '\n' + errorMsg, tr( "PostGIS" ) );
262+
mRef = 0;
263+
return;
264+
}
246265
}
247266
}
248267

‎tests/src/python/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ IF (ENABLE_PGTEST)
235235
ADD_PYTHON_TEST(PyQgsAuthManagerPKIPostgresTest test_authmanager_pki_postgres.py)
236236
ADD_PYTHON_TEST(PyQgsAuthManagerPasswordPostgresTest test_authmanager_password_postgres.py)
237237
ADD_PYTHON_TEST(PyQgsAuthManagerOgrPostgresTest test_authmanager_ogr_postgres.py)
238+
ADD_PYTHON_TEST(PyQgsDbManagerPostgis test_db_manager_postgis.py)
238239
ENDIF (ENABLE_PGTEST)
239240

240241
IF (ENABLE_MSSQLTEST)

‎tests/src/python/test_authmanager_pki_postgres.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import stat
3131
import subprocess
3232
import tempfile
33+
import glob
3334

3435
from shutil import rmtree
3536

@@ -44,6 +45,7 @@
4445
)
4546

4647
from qgis.PyQt.QtNetwork import QSslCertificate
48+
from qgis.PyQt.QtCore import QFile
4749

4850
from qgis.testing import (
4951
start_app,
@@ -234,6 +236,28 @@ def testInvalidAuthAccess(self):
234236
pg_layer = self._getPostGISLayer('testlayer_èé')
235237
self.assertFalse(pg_layer.isValid())
236238

239+
def testRemoveTemporaryCerts(self):
240+
"""
241+
Check that no temporary cert remain after connection with
242+
postgres provider
243+
"""
244+
def cleanTempPki():
245+
pkies = glob.glob(os.path.join(tempfile.gettempdir(), 'tmp*_{*}.pem'))
246+
for fn in pkies:
247+
f = QFile(fn)
248+
f.setPermissions(QFile.WriteOwner)
249+
f.remove()
250+
251+
# remove any temppki in temprorary path to check that no
252+
# other pki remain after connection
253+
cleanTempPki()
254+
# connect using postgres provider
255+
pg_layer = self._getPostGISLayer('testlayer_èé', authcfg=self.auth_config.id())
256+
self.assertTrue(pg_layer.isValid())
257+
# do test no certs remained
258+
pkies = glob.glob(os.path.join(tempfile.gettempdir(), 'tmp*_{*}.pem'))
259+
self.assertEqual(len(pkies), 0)
260+
237261

238262
if __name__ == '__main__':
239263
unittest.main()

0 commit comments

Comments
 (0)