Send database passwords to MongoDB via anonymous pipe (#1013).

NEWS

@@ -3,6 +3,8 @@
  * #1003: In the Zabbix monitoring hook, support Zabbix 7.2's authentication changes.
  * #1009: Send database passwords to MariaDB and MySQL via anonymous pipe, which is more secure than
    using an environment variable.
+ * #1013: Send database passwords to MongoDB via anonymous pipe, which is more secure than using
+   "--password" on the command-line.
  * Add a "verify_tls" option to the Uptime Kuma monitoring hook for disabling TLS verification.
 
 1.9.12

borgmatic/hooks/data_source/mongodb.py

@@ -89,12 +89,36 @@ def dump_data_sources(
     return processes
 
 
+def make_password_config_file(password):
+    '''
+    Given a database password, write it as a MongoDB configuration file to an anonymous pipe and
+    return its filename. The idea is that this is a more secure way to transmit a password to
+    MongoDB than providing it directly on the command-line.
+
+    Do not use the returned value for multiple different command invocations. That will not work
+    because each pipe is "used up" once read.
+    '''
+    logger.debug('Writing MongoDB password to configuration file pipe')
+
+    read_file_descriptor, write_file_descriptor = os.pipe()
+    os.write(write_file_descriptor, f'password: {password}'.encode('utf-8'))
+    os.close(write_file_descriptor)
+
+    # This plus subprocess.Popen(..., close_fds=False) in execute.py is necessary for the database
+    # client child process to inherit the file descriptor.
+    os.set_inheritable(read_file_descriptor, True)
+
+    return f'/dev/fd/{read_file_descriptor}'
+
+
 def build_dump_command(database, config, dump_filename, dump_format):
     '''
     Return the mongodump command from a single database configuration.
     '''
     all_databases = database['name'] == 'all'
 
+    password = borgmatic.hooks.credential.parse.resolve_credential(database.get('password'), config)
+
     return (
         ('mongodump',)
         + (('--out', shlex.quote(dump_filename)) if dump_format == 'directory' else ())
@@ -112,18 +136,7 @@ def build_dump_command(database, config, dump_filename, dump_format):
             if 'username' in database
             else ()
         )
-        + (
-            (
-                '--password',
-                shlex.quote(
-                    borgmatic.hooks.credential.parse.resolve_credential(
-                        database['password'], config
-                    )
-                ),
-            )
-            if 'password' in database
-            else ()
-        )
+        + (('--config', make_password_config_file(password)) if password else ())
         + (
             ('--authenticationDatabase', shlex.quote(database['authentication_database']))
             if 'authentication_database' in database
@@ -251,7 +264,7 @@ def build_restore_command(extract_process, database, config, dump_filename, conn
     if username:
         command.extend(('--username', username))
     if password:
-        command.extend(('--password', password))
+        command.extend(('--config', make_password_config_file(password)))
     if 'authentication_database' in database:
         command.extend(('--authenticationDatabase', database['authentication_database']))
     if 'restore_options' in database:

tests/unit/hooks/data_source/test_mongodb.py

@@ -127,6 +127,9 @@ def test_dump_data_sources_runs_mongodump_with_username_and_password():
     flexmock(module.borgmatic.hooks.credential.parse).should_receive(
         'resolve_credential'
     ).replace_with(lambda value, config: value)
+    flexmock(module).should_receive('make_password_config_file').with_args('trustsome1').and_return(
+        '/dev/fd/99'
+    )
     flexmock(module.dump).should_receive('create_named_pipe_for_dump')
 
     flexmock(module).should_receive('execute_command').with_args(
@@ -134,8 +137,8 @@ def test_dump_data_sources_runs_mongodump_with_username_and_password():
             'mongodump',
             '--username',
             'mongo',
-            '--password',
-            'trustsome1',
+            '--config',
+            '/dev/fd/99',
             '--authenticationDatabase',
             'admin',
             '--db',
@@ -243,6 +246,22 @@ def test_dump_data_sources_runs_mongodumpall_for_all_databases():
     ) == [process]
 
 
+def test_make_password_config_file_writes_password_to_pipe():
+    read_file_descriptor = 99
+    write_file_descriptor = flexmock()
+
+    flexmock(module.os).should_receive('pipe').and_return(
+        (read_file_descriptor, write_file_descriptor)
+    )
+    flexmock(module.os).should_receive('write').with_args(
+        write_file_descriptor, b'password: trustsome1'
+    ).once()
+    flexmock(module.os).should_receive('close')
+    flexmock(module.os).should_receive('set_inheritable')
+
+    assert module.make_password_config_file('trustsome1') == '/dev/fd/99'
+
+
 def test_build_dump_command_with_username_injection_attack_gets_escaped():
     database = {'name': 'test', 'username': 'bob; naughty-command'}
     flexmock(module.borgmatic.hooks.credential.parse).should_receive(
@@ -345,6 +364,9 @@ def test_restore_data_source_dump_runs_mongorestore_with_username_and_password()
     flexmock(module.borgmatic.hooks.credential.parse).should_receive(
         'resolve_credential'
     ).replace_with(lambda value, config: value)
+    flexmock(module).should_receive('make_password_config_file').with_args('trustsome1').and_return(
+        '/dev/fd/99'
+    )
     flexmock(module).should_receive('execute_command_with_processes').with_args(
         [
             'mongorestore',
@@ -352,8 +374,8 @@ def test_restore_data_source_dump_runs_mongorestore_with_username_and_password()
             '--drop',
             '--username',
             'mongo',
-            '--password',
-            'trustsome1',
+            '--config',
+            '/dev/fd/99',
             '--authenticationDatabase',
             'admin',
         ],
@@ -399,6 +421,9 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_
     flexmock(module.borgmatic.hooks.credential.parse).should_receive(
         'resolve_credential'
     ).replace_with(lambda value, config: value)
+    flexmock(module).should_receive('make_password_config_file').with_args(
+        'clipassword'
+    ).and_return('/dev/fd/99')
     flexmock(module).should_receive('execute_command_with_processes').with_args(
         [
             'mongorestore',
@@ -410,8 +435,8 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_
             'cliport',
             '--username',
             'cliusername',
-            '--password',
-            'clipassword',
+            '--config',
+            '/dev/fd/99',
             '--authenticationDatabase',
             'admin',
         ],
@@ -457,6 +482,9 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_
     flexmock(module.borgmatic.hooks.credential.parse).should_receive(
         'resolve_credential'
     ).replace_with(lambda value, config: value)
+    flexmock(module).should_receive('make_password_config_file').with_args(
+        'restorepass'
+    ).and_return('/dev/fd/99')
     flexmock(module).should_receive('execute_command_with_processes').with_args(
         [
             'mongorestore',
@@ -468,8 +496,8 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_
             'restoreport',
             '--username',
             'restoreuser',
-            '--password',
-            'restorepass',
+            '--config',
+            '/dev/fd/99',
             '--authenticationDatabase',
             'admin',
         ],