From abd47fc14e338c984ed01a5f79be9a91a797d5de Mon Sep 17 00:00:00 2001 From: Edward Shornock Date: Thu, 18 Jun 2020 01:16:34 +0300 Subject: [PATCH 1/7] Add SSL support to PostgreSQL hooks --- borgmatic/config/schema.yaml | 28 +++++++++++++++++++++++++ borgmatic/hooks/postgresql.py | 27 ++++++++++++++++++++++-- tests/unit/hooks/test_postgresql.py | 32 ++++++++++++++--------------- 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index cdd54eae..210148bb 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -556,6 +556,34 @@ map: documentation for details. Note that format is ignored when the database name is "all". example: directory + sslmode: + type: str + enum: ['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full'] + desc: | + SSL mode to use to connect to the database server. One of "disable", "allow", "prefer", + "require", "verify-ca" or "verify-full". Defaults to "disable". See + https://www.postgresql.org/docs/current/libpq-ssl.html for details. + example: disable + sslcert: + type: str + desc: | + Path to a client certificate. + example: "/root/.postgresql/postgresql.crt" + sslkey: + type: str + desc: | + Path to a private client key. + example: "/root/.postgresql/postgresql.key" + sslrootcert: + type: str + desc: | + Path to a root certificate containing a list of trusted certificate authorities. + example: "/root/.postgresql/root.crt" + sslcrl: + type: str + desc: | + Path to a certificate revocation list. + example: "/root/.postgresql/root.crl" options: type: str desc: | diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index db90025e..c9a4fdaa 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -56,7 +56,19 @@ def dump_databases(databases, log_prefix, location_config, dry_run): # format in a particular, a named destination is required, and redirection doesn't work. + (('>', dump_filename) if dump_format != 'directory' else ()) ) - extra_environment = {'PGPASSWORD': database['password']} if 'password' in database else None + extra_environment = dict() + if 'password' in database: + extra_environment['PGPASSWORD'] = database['password'] + extra_environment['PGSSLMODE'] = database['sslmode'] if 'sslmode' in database else 'disable' + if 'sslcert' in database: + extra_environment['PGSSLCERT'] = database['sslcert'] + if 'sslkey' in database: + extra_environment['PGSSLKEY'] = database['sslkey'] + if 'sslrootcert' in database: + extra_environment['PGSSLROOTCERT'] = database['sslrootcert'] + if 'sslcrl' in database: + extra_environment['PGSSLCRL'] = database['sslcrl'] + logger.debug( '{}: Dumping PostgreSQL database {} to {}{}'.format( @@ -141,7 +153,18 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, + (('--username', database['username']) if 'username' in database else ()) + (() if extract_process else (dump_filename,)) ) - extra_environment = {'PGPASSWORD': database['password']} if 'password' in database else None + extra_environment = dict() + if 'password' in database: + extra_environment['PGPASSWORD'] = database['password'] + extra_environment['PGSSLMODE'] = database['sslmode'] if 'sslmode' in database else 'disable' + if 'sslcert' in database: + extra_environment['PGSSLCERT'] = database['sslcert'] + if 'sslkey' in database: + extra_environment['PGSSLKEY'] = database['sslkey'] + if 'sslrootcert' in database: + extra_environment['PGSSLROOTCERT'] = database['sslrootcert'] + if 'sslcrl' in database: + extra_environment['PGSSLCRL'] = database['sslcrl'] logger.debug( '{}: Restoring PostgreSQL database {}{}'.format(log_prefix, database['name'], dry_run_label) diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 0f3d70dc..5b92a872 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -29,7 +29,7 @@ def test_dump_databases_runs_pg_dump_for_each_database(): 'databases/localhost/{}'.format(name), ), shell=True, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, run_to_completion=False, ).and_return(process).once() @@ -74,7 +74,7 @@ def test_dump_databases_runs_pg_dump_with_hostname_and_port(): 'databases/database.example.org/foo', ), shell=True, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, run_to_completion=False, ).and_return(process).once() @@ -105,7 +105,7 @@ def test_dump_databases_runs_pg_dump_with_username_and_password(): 'databases/localhost/foo', ), shell=True, - extra_environment={'PGPASSWORD': 'trustsome1'}, + extra_environment={'PGPASSWORD': 'trustsome1', 'PGSSLMODE': 'disable'}, run_to_completion=False, ).and_return(process).once() @@ -135,7 +135,7 @@ def test_dump_databases_runs_pg_dump_with_directory_format(): 'foo', ), shell=True, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, run_to_completion=False, ).and_return(process).once() @@ -165,7 +165,7 @@ def test_dump_databases_runs_pg_dump_with_options(): 'databases/localhost/foo', ), shell=True, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, run_to_completion=False, ).and_return(process).once() @@ -184,7 +184,7 @@ def test_dump_databases_runs_pg_dumpall_for_all_databases(): flexmock(module).should_receive('execute_command').with_args( ('pg_dumpall', '--no-password', '--clean', '--if-exists', '>', 'databases/localhost/all'), shell=True, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, run_to_completion=False, ).and_return(process).once() @@ -210,12 +210,12 @@ def test_restore_database_dump_runs_pg_restore(): processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, borg_local_path='borg', ).once() flexmock(module).should_receive('execute_command').with_args( ('psql', '--no-password', '--quiet', '--dbname', 'foo', '--command', 'ANALYZE'), - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, ).once() module.restore_database_dump( @@ -260,7 +260,7 @@ def test_restore_database_dump_runs_pg_restore_with_hostname_and_port(): processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, borg_local_path='borg', ).once() flexmock(module).should_receive('execute_command').with_args( @@ -277,7 +277,7 @@ def test_restore_database_dump_runs_pg_restore_with_hostname_and_port(): '--command', 'ANALYZE', ), - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, ).once() module.restore_database_dump( @@ -306,7 +306,7 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password(): processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - extra_environment={'PGPASSWORD': 'trustsome1'}, + extra_environment={'PGPASSWORD': 'trustsome1', 'PGSSLMODE': 'disable'}, borg_local_path='borg', ).once() flexmock(module).should_receive('execute_command').with_args( @@ -321,7 +321,7 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password(): '--command', 'ANALYZE', ), - extra_environment={'PGPASSWORD': 'trustsome1'}, + extra_environment={'PGPASSWORD': 'trustsome1', 'PGSSLMODE': 'disable'}, ).once() module.restore_database_dump( @@ -340,11 +340,11 @@ def test_restore_database_dump_runs_psql_for_all_database_dump(): processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, borg_local_path='borg', ).once() flexmock(module).should_receive('execute_command').with_args( - ('psql', '--no-password', '--quiet', '--command', 'ANALYZE'), extra_environment=None + ('psql', '--no-password', '--quiet', '--command', 'ANALYZE'), extra_environment={'PGSSLMODE': 'disable'} ).once() module.restore_database_dump( @@ -383,12 +383,12 @@ def test_restore_database_dump_without_extract_process_restores_from_disk(): processes=[], output_log_level=logging.DEBUG, input_file=None, - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, borg_local_path='borg', ).once() flexmock(module).should_receive('execute_command').with_args( ('psql', '--no-password', '--quiet', '--dbname', 'foo', '--command', 'ANALYZE'), - extra_environment=None, + extra_environment={'PGSSLMODE': 'disable'}, ).once() module.restore_database_dump( From 33113890f592843da82515202d5a2dafd0e4b82d Mon Sep 17 00:00:00 2001 From: Edward Shornock Date: Fri, 19 Jun 2020 12:30:26 +0300 Subject: [PATCH 2/7] Reduce duplication with a common function --- borgmatic/hooks/postgresql.py | 46 ++++++++++++++++------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index c9a4fdaa..765d92ae 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -15,6 +15,25 @@ def make_dump_path(location_config): # pragma: no cover ) +def make_extra_environment(database): + ''' + Make the extra_environment dict from the given database configuration. + ''' + extra = dict() + if 'password' in database: + extra['PGPASSWORD'] = database['password'] + extra['PGSSLMODE'] = database.get('sslmode', 'disable') + if 'sslcert' in database: + extra['PGSSLCERT'] = database['sslcert'] + if 'sslkey' in database: + extra['PGSSLKEY'] = database['sslkey'] + if 'sslrootcert' in database: + extra['PGSSLROOTCERT'] = database['sslrootcert'] + if 'sslcrl' in database: + extra['PGSSLCRL'] = database['sslcrl'] + return extra + + def dump_databases(databases, log_prefix, location_config, dry_run): ''' Dump the given PostgreSQL databases to a named pipe. The databases are supplied as a sequence of @@ -56,19 +75,7 @@ def dump_databases(databases, log_prefix, location_config, dry_run): # format in a particular, a named destination is required, and redirection doesn't work. + (('>', dump_filename) if dump_format != 'directory' else ()) ) - extra_environment = dict() - if 'password' in database: - extra_environment['PGPASSWORD'] = database['password'] - extra_environment['PGSSLMODE'] = database['sslmode'] if 'sslmode' in database else 'disable' - if 'sslcert' in database: - extra_environment['PGSSLCERT'] = database['sslcert'] - if 'sslkey' in database: - extra_environment['PGSSLKEY'] = database['sslkey'] - if 'sslrootcert' in database: - extra_environment['PGSSLROOTCERT'] = database['sslrootcert'] - if 'sslcrl' in database: - extra_environment['PGSSLCRL'] = database['sslcrl'] - + extra_environment = make_extra_environment(database) logger.debug( '{}: Dumping PostgreSQL database {} to {}{}'.format( @@ -153,18 +160,7 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, + (('--username', database['username']) if 'username' in database else ()) + (() if extract_process else (dump_filename,)) ) - extra_environment = dict() - if 'password' in database: - extra_environment['PGPASSWORD'] = database['password'] - extra_environment['PGSSLMODE'] = database['sslmode'] if 'sslmode' in database else 'disable' - if 'sslcert' in database: - extra_environment['PGSSLCERT'] = database['sslcert'] - if 'sslkey' in database: - extra_environment['PGSSLKEY'] = database['sslkey'] - if 'sslrootcert' in database: - extra_environment['PGSSLROOTCERT'] = database['sslrootcert'] - if 'sslcrl' in database: - extra_environment['PGSSLCRL'] = database['sslcrl'] + extra_environment = make_extra_environment(database) logger.debug( '{}: Restoring PostgreSQL database {}{}'.format(log_prefix, database['name'], dry_run_label) From a16fed88874f0acdb474c160b8f79e9d6833dcb1 Mon Sep 17 00:00:00 2001 From: Edward Shornock Date: Fri, 19 Jun 2020 12:46:27 +0300 Subject: [PATCH 3/7] Rename PostgreSQL SSL config variables e.g. s/sslmode/ssl_mode/g to conform with borgmatic naming conventions. --- borgmatic/config/schema.yaml | 10 +++++----- borgmatic/hooks/postgresql.py | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 210148bb..bddd6fb6 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -556,7 +556,7 @@ map: documentation for details. Note that format is ignored when the database name is "all". example: directory - sslmode: + ssl_mode: type: str enum: ['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full'] desc: | @@ -564,22 +564,22 @@ map: "require", "verify-ca" or "verify-full". Defaults to "disable". See https://www.postgresql.org/docs/current/libpq-ssl.html for details. example: disable - sslcert: + ssl_cert: type: str desc: | Path to a client certificate. example: "/root/.postgresql/postgresql.crt" - sslkey: + ssl_key: type: str desc: | Path to a private client key. example: "/root/.postgresql/postgresql.key" - sslrootcert: + ssl_root_cert: type: str desc: | Path to a root certificate containing a list of trusted certificate authorities. example: "/root/.postgresql/root.crt" - sslcrl: + ssl_crl: type: str desc: | Path to a certificate revocation list. diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index 765d92ae..f5660901 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -22,15 +22,15 @@ def make_extra_environment(database): extra = dict() if 'password' in database: extra['PGPASSWORD'] = database['password'] - extra['PGSSLMODE'] = database.get('sslmode', 'disable') - if 'sslcert' in database: - extra['PGSSLCERT'] = database['sslcert'] - if 'sslkey' in database: - extra['PGSSLKEY'] = database['sslkey'] - if 'sslrootcert' in database: - extra['PGSSLROOTCERT'] = database['sslrootcert'] - if 'sslcrl' in database: - extra['PGSSLCRL'] = database['sslcrl'] + extra['PGSSLMODE'] = database.get('ssl_mode', 'disable') + if 'ssl_cert' in database: + extra['PGSSLCERT'] = database['ssl_cert'] + if 'ssl_key' in database: + extra['PGSSLKEY'] = database['ssl_key'] + if 'ssl_root_cert' in database: + extra['PGSSLROOTCERT'] = database['ssl_root_cert'] + if 'ssl_crl' in database: + extra['PGSSLCRL'] = database['ssl_crl'] return extra From 463a133a63feedb3fbad2cc36bba3b939b604ced Mon Sep 17 00:00:00 2001 From: Edward Shornock Date: Fri, 19 Jun 2020 13:06:22 +0300 Subject: [PATCH 4/7] Ensure schema lines are less than 80 characters in length --- borgmatic/config/schema.yaml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index bddd6fb6..79f14dd6 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -558,11 +558,13 @@ map: example: directory ssl_mode: type: str - enum: ['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full'] + enum: ['disable', 'allow', 'prefer', + 'require', 'verify-ca', 'verify-full'] desc: | - SSL mode to use to connect to the database server. One of "disable", "allow", "prefer", - "require", "verify-ca" or "verify-full". Defaults to "disable". See - https://www.postgresql.org/docs/current/libpq-ssl.html for details. + SSL mode to use to connect to the database + server. One of "disable", "allow", "prefer", + "require", "verify-ca" or "verify-full". + Defaults to "disable". example: disable ssl_cert: type: str @@ -577,7 +579,8 @@ map: ssl_root_cert: type: str desc: | - Path to a root certificate containing a list of trusted certificate authorities. + Path to a root certificate containing a list of + trusted certificate authorities. example: "/root/.postgresql/root.crt" ssl_crl: type: str From 8fb830099f2b727f41ecf36510d183f4429f9bfc Mon Sep 17 00:00:00 2001 From: Edward Shornock Date: Fri, 19 Jun 2020 13:06:52 +0300 Subject: [PATCH 5/7] Re-add the ilbpq-ssl documentation URL to the schema It's been moved from describing `ssl_mode` to the general postgresql_database description key. --- borgmatic/config/schema.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 79f14dd6..7205ad49 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -601,7 +601,8 @@ map: database dumps are added to your source directories at runtime, backed up, and removed afterwards. Requires pg_dump/pg_dumpall/pg_restore commands. See - https://www.postgresql.org/docs/current/app-pgdump.html for + https://www.postgresql.org/docs/current/app-pgdump.html and + https://www.postgresql.org/docs/current/libpq-ssl.html for details. mysql_databases: seq: From d2d92b1f1a154d20c965aa0f1008e7484762080e Mon Sep 17 00:00:00 2001 From: Edward Shornock Date: Fri, 19 Jun 2020 16:26:48 +0300 Subject: [PATCH 6/7] Add tests for the PostgreSQL SSL options --- tests/unit/hooks/test_postgresql.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 5b92a872..8ad9dd3e 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -112,6 +112,27 @@ def test_dump_databases_runs_pg_dump_with_username_and_password(): assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == [process] +def test_make_extra_environment(): + database = { + 'name': 'foo', + 'ssl_mode': 'require', + 'ssl_cert': 'cert.crt', + 'ssl_key': 'key.key', + 'ssl_root_cert': 'root.crt', + 'ssl_crl': 'crl.crl', + } + expected = { + 'PGSSLMODE': 'require', + 'PGSSLCERT': 'cert.crt', + 'PGSSLKEY': 'key.key', + 'PGSSLROOTCERT': 'root.crt', + 'PGSSLCRL': 'crl.crl', + } + + extra_env = module.make_extra_environment(database) + assert extra_env == expected + + def test_dump_databases_runs_pg_dump_with_directory_format(): databases = [{'name': 'foo', 'format': 'directory'}] process = flexmock() @@ -151,6 +172,8 @@ def test_dump_databases_runs_pg_dump_with_options(): ) flexmock(module.dump).should_receive('create_named_pipe_for_dump') + flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) + flexmock(module).should_receive('execute_command').with_args( ( 'pg_dump', @@ -344,7 +367,8 @@ def test_restore_database_dump_runs_psql_for_all_database_dump(): borg_local_path='borg', ).once() flexmock(module).should_receive('execute_command').with_args( - ('psql', '--no-password', '--quiet', '--command', 'ANALYZE'), extra_environment={'PGSSLMODE': 'disable'} + ('psql', '--no-password', '--quiet', '--command', 'ANALYZE'), + extra_environment={'PGSSLMODE': 'disable'}, ).once() module.restore_database_dump( From 01db676d685e934e10106a2205dfee63ca3b1b8f Mon Sep 17 00:00:00 2001 From: Edward Shornock Date: Sat, 20 Jun 2020 15:11:57 +0300 Subject: [PATCH 7/7] Change the example for the ssl_mode parameter --- borgmatic/config/schema.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 7205ad49..f1b8228f 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -565,7 +565,7 @@ map: server. One of "disable", "allow", "prefer", "require", "verify-ca" or "verify-full". Defaults to "disable". - example: disable + example: require ssl_cert: type: str desc: |