Subject: [pgadmin4][Patch]: RM #3122 - Backup not working on certificate (SSL) protected servers

From: Khushboo Vashi <[email-masked]>
Date: Wed, 14 Mar 2018 11:03:47 +0530

Hi,

Please find the attached patch to fix RM #3122 : Backup not working on
certificate (SSL) protected servers.

The attached patch fixes the issue in the following modules:

1. Backup
2. Restore
3. Import/Export
4. Maintenance

Thanks,
Khushboo
diff --git a/web/migrations/versions/12958d9f4970_.py b/web/migrations/versions/12958d9f4970_.py
new file mode 100644
index 0000000..aa2c5fd
--- /dev/null
+++ b/web/migrations/versions/12958d9f4970_.py
@@ -0,0 +1,33 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+"""
+Add SSL support for batch processes (RM #3122)
+
+Revision ID: 12958d9f4970
+Revises: 50aad68f99c2
+Create Date: 2018-03-13 21:21:25.455315
+
+"""
+from pgadmin.model import db
+
+# revision identifiers, used by Alembic.
+revision = '12958d9f4970'
+down_revision = '50aad68f99c2'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    db.engine.execute(
+        'ALTER TABLE process ADD COLUMN ssl TEXT'
+    )
+
+
+def downgrade():
+    pass
diff --git a/web/pgadmin/misc/bgprocess/processes.py b/web/pgadmin/misc/bgprocess/processes.py
index c456b4f..4a6e60a 100644
--- a/web/pgadmin/misc/bgprocess/processes.py
+++ b/web/pgadmin/misc/bgprocess/processes.py
@@ -19,7 +19,8 @@ from datetime import datetime
 from pickle import dumps, loads
 from subprocess import Popen
 
-from pgadmin.utils import IS_PY2, u, file_quote, fs_encoding
+from pgadmin.utils import IS_PY2, u, file_quote, fs_encoding, \
+    get_complete_file_path
 
 import pytz
 from dateutil import parser
@@ -61,13 +62,14 @@ class BatchProcess(object):
 
         self.id = self.desc = self.cmd = self.args = self.log_dir = \
             self.stdout = self.stderr = self.stime = self.etime = \
-            self.ecode = None
+            self.ecode = self.ssl = None
 
         if 'id' in kwargs:
             self._retrieve_process(kwargs['id'])
         else:
+            ssl = kwargs.get('ssl', None)
             self._create_process(
-                kwargs['desc'], kwargs['cmd'], kwargs['args']
+                kwargs['desc'], kwargs['cmd'], kwargs['args'], ssl
             )
 
     def _retrieve_process(self, _id):
@@ -110,8 +112,10 @@ class BatchProcess(object):
         self.etime = p.end_time
         # Exit code
         self.ecode = p.exit_code
+        # SSL
+        self.ssl = p.ssl
 
-    def _create_process(self, _desc, _cmd, _args):
+    def _create_process(self, _desc, _cmd, _args, ssl):
         ctime = get_current_time(format='%y%m%d%H%M%S%f')
         log_dir = os.path.join(
             config.SESSION_DB_PATH, 'process_logs'
@@ -163,6 +167,8 @@ class BatchProcess(object):
         self.etime = None
         # Exit code
         self.ecode = None
+        # SSL
+        self.ssl = dumps(ssl)
 
         # Arguments
         self.args = _args
@@ -199,7 +205,8 @@ class BatchProcess(object):
             if IS_PY2 and hasattr(args_val, 'decode') else args_val,
             logdir=log_dir,
             desc=tmp_desc,
-            user_id=current_user.id
+            user_id=current_user.id,
+            ssl=self.ssl
         )
         db.session.add(j)
         db.session.commit()
@@ -330,6 +337,20 @@ class BatchProcess(object):
         env['OUTDIR'] = self.log_dir
         env['PGA_BGP_FOREGROUND'] = "1"
 
+        # Set environment variable for SSL
+        ssl = loads(self.ssl)
+        if ssl and isinstance(ssl, dict):
+            ssl_mode = ssl.get('ssl_mode', None)
+            sslcert = ssl.get('sslcert', None)
+            sslkey = ssl.get('sslkey', None)
+            sslrootcert = ssl.get('sslrootcert', None)
+
+            if ssl_mode and sslcert and sslkey and sslrootcert:
+                env['PGSSLMODE'] = ssl_mode
+                env['PGSSLCERT'] = get_complete_file_path(sslcert)
+                env['PGSSLKEY'] = get_complete_file_path(sslkey)
+                env['PGSSLROOTCERT'] = get_complete_file_path(sslrootcert)
+
         if cb is not None:
             cb(env)
 
diff --git a/web/pgadmin/model/__init__.py b/web/pgadmin/model/__init__.py
index 11bc9f0..8a3f96f 100644
--- a/web/pgadmin/model/__init__.py
+++ b/web/pgadmin/model/__init__.py
@@ -238,6 +238,7 @@ class Process(db.Model):
     end_time = db.Column(db.String(), nullable=True)
     exit_code = db.Column(db.Integer(), nullable=True)
     acknowledge = db.Column(db.String(), nullable=True)
+    ssl = db.Column(db.String(), nullable=True)

 class Keys(db.Model):
diff --git a/web/pgadmin/tools/backup/__init__.py b/web/pgadmin/tools/backup/__init__.py
index 940ef5b..efb9767 100644
--- a/web/pgadmin/tools/backup/__init__.py
+++ b/web/pgadmin/tools/backup/__init__.py
@@ -300,6 +300,7 @@ def create_backup_job(sid):
         '--database',
         server.maintenance_db
     ]
+
     if 'role' in data and data['role']:
         args.append('--role')
         args.append(data['role'])
@@ -310,6 +311,16 @@ def create_backup_job(sid):
     if data['type'] == 'global':
         args.append('--globals-only')
 
+    # SSL
+    ssl = {}
+    if server.sslcert is not None and \
+            server.sslkey is not None and \
+            server.sslrootcert is not None:
+        ssl['ssl_mode'] = server.ssl_mode
+        ssl['sslcert'] = server.sslcert
+        ssl['sslkey'] = server.sslkey
+        ssl['sslrootcert'] = server.sslrootcert
+
     try:
         p = BatchProcess(
             desc=BackupMessage(
@@ -320,7 +331,7 @@ def create_backup_job(sid):
                 ) else data['file'],
                 *args
             ),
-            cmd=utility, args=args
+            cmd=utility, args=args, ssl=ssl
         )
         manager.export_password_env(p.id)
         p.start()
@@ -473,6 +484,16 @@ def create_backup_objects_job(sid):
 
     args.append(data['database'])
 
+    # SSL
+    ssl = {}
+    if server.sslcert is not None and \
+            server.sslkey is not None and \
+            server.sslrootcert is not None:
+        ssl['ssl_mode'] = server.ssl_mode
+        ssl['sslcert'] = server.sslcert
+        ssl['sslkey'] = server.sslkey
+        ssl['sslrootcert'] = server.sslrootcert
+
     try:
         p = BatchProcess(
             desc=BackupMessage(
@@ -483,7 +504,7 @@ def create_backup_objects_job(sid):
                 *args,
                 database=data['database']
             ),
-            cmd=utility, args=args
+            cmd=utility, args=args, ssl=ssl
         )
         manager.export_password_env(p.id)
         p.start()
diff --git a/web/pgadmin/tools/import_export/__init__.py b/web/pgadmin/tools/import_export/__init__.py
index 3f83fd4..0ad23f0 100644
--- a/web/pgadmin/tools/import_export/__init__.py
+++ b/web/pgadmin/tools/import_export/__init__.py
@@ -293,6 +293,16 @@ def create_import_export_job(sid):
 
     args = ['--command', query]
 
+    # SSL
+    ssl = {}
+    if server.sslcert is not None and \
+            server.sslkey is not None and \
+            server.sslrootcert is not None:
+        ssl['ssl_mode'] = server.ssl_mode
+        ssl['sslcert'] = server.sslcert
+        ssl['sslkey'] = server.sslkey
+        ssl['sslrootcert'] = server.sslrootcert
+
     try:
         p = BatchProcess(
             desc=IEMessage(
@@ -303,7 +313,8 @@ def create_import_export_job(sid):
                 storage_dir,
                 utility, *args
             ),
-            cmd=utility, args=args
+            cmd=utility, args=args,
+            ssl=ssl
         )
         manager.export_password_env(p.id)
 
diff --git a/web/pgadmin/tools/maintenance/__init__.py b/web/pgadmin/tools/maintenance/__init__.py
index 8416a20..cfdabf0 100644
--- a/web/pgadmin/tools/maintenance/__init__.py
+++ b/web/pgadmin/tools/maintenance/__init__.py
@@ -230,10 +230,21 @@ def create_maintenance_job(sid, did):
         '--command', query
     ]
 
+    # SSL
+    ssl = {}
+    if server.sslcert is not None and \
+            server.sslkey is not None and \
+            server.sslrootcert is not None:
+        ssl['ssl_mode'] = server.ssl_mode
+        ssl['sslcert'] = server.sslcert
+        ssl['sslkey'] = server.sslkey
+        ssl['sslrootcert'] = server.sslrootcert
+
     try:
         p = BatchProcess(
             desc=Message(sid, data, query),
-            cmd=utility, args=args
+            cmd=utility, args=args,
+            ssl=ssl
         )
         manager.export_password_env(p.id)
         p.start()
diff --git a/web/pgadmin/tools/restore/__init__.py b/web/pgadmin/tools/restore/__init__.py
index 6afa7b4..1865419 100644
--- a/web/pgadmin/tools/restore/__init__.py
+++ b/web/pgadmin/tools/restore/__init__.py
@@ -317,6 +317,16 @@ def create_restore_job(sid):
 
     args.append(fs_short_path(_file))
 
+    # SSL
+    ssl = {}
+    if server.sslcert is not None and \
+            server.sslkey is not None and \
+            server.sslrootcert is not None:
+        ssl['ssl_mode'] = server.ssl_mode
+        ssl['sslcert'] = server.sslcert
+        ssl['sslkey'] = server.sslkey
+        ssl['sslrootcert'] = server.sslrootcert
+
     try:
         p = BatchProcess(
             desc=RestoreMessage(
@@ -326,7 +336,8 @@ def create_restore_job(sid):
                 ) else data['file'],
                 *args
             ),
-            cmd=utility, args=args
+            cmd=utility, args=args,
+            ssl=ssl
         )
         manager.export_password_env(p.id)
         p.start()

---

From: Murtuza Zabuawala <[email-masked]>
Date: Wed, 14 Mar 2018 12:39:57 +0530

Hi Khushboo,

We can simplify this, we don't need to create any extra column,

Check: ../pgadmin4/web/pgadmin/tools/import_export/__init__.py +322 where
we are setting ENV variable we can create common utility function (let say
in ../tools/utils/__init__.py) which will set all required the environment
variables and then we will pass that function in p.start(..) method.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Murtuza Zabuawala <[email-masked]>
Date: Wed, 14 Mar 2018 12:48:49 +0530

../pgadmin4/web/pgadmin/tools/import_export/__init__.py +310 without your
patch applied.

---

From: Khushboo Vashi <[email-masked]>
Date: Wed, 14 Mar 2018 13:07:17 +0530

On Wed, Mar 14, 2018 at 12:48 PM, Murtuza Zabuawala <
[email-masked]> wrote:

>  ../pgadmin4/web/pgadmin/tools/import_export/__init__.py +310 without
> your patch applied.
>
> Yes, good point. Will update and send the patch.

>
> On Wed, Mar 14, 2018 at 12:39 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi Khushboo,
>>
>> Check: ../pgadmin4/web/pgadmin/tools/import_export/__init__.py +322
>> where we are setting ENV variable we can create common utility function
>> (let say in ../tools/utils/__init__.py) which will set all required the
>> environment variables and then we will pass that function in p.start(..)
>> method.
>>
>>
>> --
>> Regards,
>> Murtuza Zabuawala

---

From: Khushboo Vashi <[email-masked]>
Date: Wed, 14 Mar 2018 20:25:37 +0530

Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Wed, Mar 14, 2018 at 1:07 PM, Khushboo Vashi <
[email-masked]> wrote:

>
>
> On Wed, Mar 14, 2018 at 12:48 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
index c456b4f..cefb51a 100644

 import pytz
 from dateutil import parser
@@ -62,6 +63,7 @@ class BatchProcess(object):
         self.id = self.desc = self.cmd = self.args = self.log_dir = \
             self.stdout = self.stderr = self.stime = self.etime = \
             self.ecode = None
+        self.env = dict()
 
         if 'id' in kwargs:
             self._retrieve_process(kwargs['id'])
@@ -330,6 +332,9 @@ class BatchProcess(object):
         env['OUTDIR'] = self.log_dir
         env['PGA_BGP_FOREGROUND'] = "1"
 
+        if self.env:
+            env.update(self.env)
+
         if cb is not None:
             cb(env)
 
@@ -622,3 +627,18 @@ class BatchProcess(object):
             p.acknowledge = get_current_time()
 
         db.session.commit()
+
+    def set_env_variables(self, server, **kwargs):
+        """Set environment variables"""
+        if server and server.sslcert is not None and \
+                server.sslkey is not None and \
+                server.sslrootcert is not None:
+            # SSL environment variables
+            self.env['PGSSLMODE'] = server.ssl_mode
+            self.env['PGSSLCERT'] = get_complete_file_path(server.sslcert)
+            self.env['PGSSLKEY'] = get_complete_file_path(server.sslkey)
+            self.env['PGSSLROOTCERT'] = \
+                get_complete_file_path(server.sslrootcert)
+
+        if 'env' in kwargs:
+            self.env.update(kwargs['env'])
diff --git a/web/pgadmin/tools/backup/__init__.py b/web/pgadmin/tools/backup/__init__.py
index 940ef5b..8936e53 100644
--- a/web/pgadmin/tools/backup/__init__.py
+++ b/web/pgadmin/tools/backup/__init__.py
@@ -300,6 +300,7 @@ def create_backup_job(sid):
         '--database',
         server.maintenance_db
     ]
+
     if 'role' in data and data['role']:
         args.append('--role')
         args.append(data['role'])
@@ -323,6 +324,7 @@ def create_backup_job(sid):
             cmd=utility, args=args
         )
         manager.export_password_env(p.id)
+        p.set_env_variables(server)
         p.start()
         jid = p.id
     except Exception as e:
@@ -486,6 +488,7 @@ def create_backup_objects_job(sid):
             cmd=utility, args=args
         )
         manager.export_password_env(p.id)
+        p.set_env_variables(server)
         p.start()
         jid = p.id
     except Exception as e:
diff --git a/web/pgadmin/tools/import_export/__init__.py b/web/pgadmin/tools/import_export/__init__.py
index 3f83fd4..9690475 100644
--- a/web/pgadmin/tools/import_export/__init__.py
+++ b/web/pgadmin/tools/import_export/__init__.py
@@ -307,13 +307,13 @@ def create_import_export_job(sid):
         )
         manager.export_password_env(p.id)
 
-        def export_pg_env(env):
-            env['PGHOST'] = server.host
-            env['PGPORT'] = str(server.port)
-            env['PGUSER'] = server.username
-            env['PGDATABASE'] = data['database']
-
-        p.start(export_pg_env)
+        env = dict()
+        env['PGHOST'] = server.host
+        env['PGPORT'] = str(server.port)
+        env['PGUSER'] = server.username
+        env['PGDATABASE'] = data['database']
+        p.set_env_variables(server, env=env)
+        p.start()
         jid = p.id
     except Exception as e:
         current_app.logger.exception(e)
diff --git a/web/pgadmin/tools/maintenance/__init__.py b/web/pgadmin/tools/maintenance/__init__.py
index 8416a20..088922b 100644
--- a/web/pgadmin/tools/maintenance/__init__.py
+++ b/web/pgadmin/tools/maintenance/__init__.py
@@ -236,6 +236,7 @@ def create_maintenance_job(sid, did):
             cmd=utility, args=args
         )
         manager.export_password_env(p.id)
+        p.set_env_variables(server)
         p.start()
         jid = p.id
     except Exception as e:
diff --git a/web/pgadmin/tools/restore/__init__.py b/web/pgadmin/tools/restore/__init__.py
index 6afa7b4..db1d522 100644
--- a/web/pgadmin/tools/restore/__init__.py
+++ b/web/pgadmin/tools/restore/__init__.py
@@ -329,6 +329,7 @@ def create_restore_job(sid):
             cmd=utility, args=args
         )
         manager.export_password_env(p.id)
+        p.set_env_variables(server)
         p.start()
         jid = p.id
     except Exception as e:

---

From: Dave Page <[email-masked]>
Date: Wed, 14 Mar 2018 17:51:53 -0400

Hi

> Please find the attached updated patch.

Can you add a unit test for set_env_variables() please?

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Khushboo Vashi <[email-masked]>
Date: Thu, 15 Mar 2018 10:25:16 +0530

> Can you add a unit test for set_env_variables() please?
This function is a part of a BatchProcess class which is used by all
backup/restore/maintenance etc modules. So, when we write test cases for
this module this function will be covered.
I had already created RM for the same (Ref:
https://redmine.postgresql.org/issues/3206) and it is in the current sprint.

>>>>>> Please find the attached patch to fix RM #3122 : Backup not working
>>>>>> on certificate (SSL) protected servers.
>>>>>>

---

From: Dave Page <[email-masked]>
Date: Thu, 15 Mar 2018 07:37:11 -0400

Hi

On Thu, Mar 15, 2018 at 12:55 AM, Khushboo Vashi <
[email-masked]> wrote:

> I had already created RM for the same (Ref: https://redmine.postgresql.
> org/issues/3206) and it is in the current sprint.
>

OK, thanks. Patch applied.