From faa7c9af0047178e9b587d271b579b09217aa28e Mon Sep 17 00:00:00 2001 From: Bas Couwenberg Date: Tue, 27 Feb 2024 08:37:25 +0100 Subject: [PATCH 1/4] flake8: E275 missing whitespace after keyword. --- src/lib/director.py | 12 ++++++------ src/lib/logger.py | 2 +- src/lib/rsync.py | 8 ++++---- src/models/config.py | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/lib/director.py b/src/lib/director.py index a5668a2..83d4b32 100644 --- a/src/lib/director.py +++ b/src/lib/director.py @@ -23,7 +23,7 @@ def getJobArray(self, jobpath=None): jobArray = [] if jobpath is None: directory = config().jobconfigdirectory.rstrip('/') - if(os.path.exists(directory)): + if (os.path.exists(directory)): os.chdir(directory) for filename in glob.glob("*.job"): j = job(directory + "/" + filename) @@ -196,10 +196,10 @@ def backupRotate(self, job, moveCurrent=True): self._unlinkExpiredBackups(job) # Rotate backups - if(self._rotateBackups(job)): + if (self._rotateBackups(job)): latest = self._moveCurrentBackup(job) if latest: - if(self._updateLatestSymlink(job, latest)): + if (self._updateLatestSymlink(job, latest)): pass else: logger().error(("Error updating current symlink" @@ -339,10 +339,10 @@ def _moveLastBackupToCurrentBackup(self, job): def getWorkingDirectory(self): """Check in which folder we place the backup today""" ret = "daily" - if(int(datetime.datetime.today().strftime("%w")) == + if (int(datetime.datetime.today().strftime("%w")) == config().weeklybackup): ret = "weekly" - if(int(datetime.datetime.today().strftime("%d")) == + if (int(datetime.datetime.today().strftime("%d")) == config().monthlybackup): ret = "monthly" return ret @@ -383,7 +383,7 @@ def checkWorkingDirectory(self, workingDirectory): def processBackupStatus(self, job): job.backupstatus['hostname'] = job.hostname - if(job.ssh): + if (job.ssh): ssh = 'True' job.backupstatus['username'] = job.sshusername else: diff --git a/src/lib/logger.py b/src/lib/logger.py index b868946..4bbcf33 100644 --- a/src/lib/logger.py +++ b/src/lib/logger.py @@ -65,7 +65,7 @@ def __init__(self, logfile=None): # Create and remember instance logger.__instance = logger.__impl() - if(logfile): # pragma: no cover + if (logfile): # pragma: no cover logdirectory = os.path.dirname(logfile) if not os.path.exists(logdirectory): os.makedirs(logdirectory) diff --git a/src/lib/rsync.py b/src/lib/rsync.py index c56374f..4af8a83 100644 --- a/src/lib/rsync.py +++ b/src/lib/rsync.py @@ -103,13 +103,13 @@ def executeRsyncViaRsyncProtocol(self, job, latest): # Link files to the same inodes as last backup to save disk space # and boost backup performance - if(latest): + if (latest): latest = "--link-dest=%s" % latest else: latest = "" # Generate rsync CLI command and execute it - if(include): + if (include): password = "export RSYNC_PASSWORD=\"%s\"" % job.rsyncpassword rsyncCommand = "%s %s %s %s %s" % ( config().rsyncpath, options, latest, include, dir) @@ -139,13 +139,13 @@ def executeRsyncViaSshProtocol(self, job, latest): # Link files to the same inodes as last backup to save disk space # and boost backup performance - if(latest): + if (latest): latest = "--link-dest=%s" % latest else: latest = "" # Generate rsync CLI command and execute it - if(include): + if (include): command = "%s %s %s %s %s" % ( config().rsyncpath, options, latest, include, directory) logger().info("Executing rsync command (%s)" % command) diff --git a/src/models/config.py b/src/models/config.py index 8a128bb..a0adc0d 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -45,7 +45,7 @@ def __init__(self, mainconfigpath=None): # Create and remember instance config.__instance = config.__impl() - if(mainconfigpath): # pragma: no cover + if (mainconfigpath): # pragma: no cover self.mainconfigpath = mainconfigpath self.readConfig() self.init = False From 313356c3bb284e7e24a3a91fa6e0ff3ee70dd816 Mon Sep 17 00:00:00 2001 From: Bas Couwenberg Date: Tue, 27 Feb 2024 08:39:10 +0100 Subject: [PATCH 2/4] flake8: E501 line too long. --- src/lib/statusemail.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/statusemail.py b/src/lib/statusemail.py index aeb0c24..005888a 100644 --- a/src/lib/statusemail.py +++ b/src/lib/statusemail.py @@ -116,7 +116,8 @@ def getTextExceptionBody(self, exc): return template.render(exc=exc) def getOverallBackupState(self, jobs): - """Overall backup state = 'ok' unless there is at least one failed backup. + """Overall backup state = 'ok' unless there is at least one failed + backup. At the same time reorder the list so errors appear first.""" ret = "ok" good = [] From 835cb9f971d32017804ef78bdd74d33a5e1a3325 Mon Sep 17 00:00:00 2001 From: Bas Couwenberg Date: Wed, 6 Mar 2024 12:35:42 +0100 Subject: [PATCH 3/4] Restore pytest-cov options in setup.cfg. Were removed in 005f75ab590849caa47506842622025fdb535433. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index bf81b08..d363ed7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [tool:pytest] testpaths = tests -addopts = -v +addopts = -v --cov=src --cov-config=setup.cfg --cov-report=term-missing [coverage:run] branch = True From c2eb9aa24ab922ea416c6fbb02200077b807e146 Mon Sep 17 00:00:00 2001 From: Bas Couwenberg Date: Wed, 6 Mar 2024 14:02:49 +0100 Subject: [PATCH 4/4] Add sshdisabledalgs option to jobs to fix paramiko connect failure. Paramiko 2.9.0 added support for SHA-2 variants of RSA key verification algorithms. See: https://www.paramiko.org/changelog.html#2.9.0 This breaks connections to Debian squeeze hosts which don't support these nor server-sig-algs to help Paramilo make the right choice. Example of a failed connection: ``` DEBUG:paramiko.transport:Finalizing pubkey algorithm for key of type 'ssh-rsa' DEBUG:paramiko.transport:Our pubkey algorithm list: ['rsa-sha2-512', 'rsa-sha2-256', 'ssh-rsa'] DEBUG:paramiko.transport:Server did not send a server-sig-algs list; defaulting to our first preferred algo ('rsa-sha2-512') DEBUG:paramiko.transport:NOTE: you may use the 'disabled_algorithms' SSHClient/Transport init kwarg to disable that or other algorithms if your server does not support them! INFO:paramiko.transport:Authentication (publickey) failed. ``` This can be fixed by setting the following in the job configuration: ``` ssh_disabledalgs: pubkeys: - rsa-sha2-512 - rsa-sha2-256 ``` The connection then succeeds: ``` DEBUG:paramiko.transport:Finalizing pubkey algorithm for key of type 'ssh-rsa' DEBUG:paramiko.transport:Our pubkey algorithm list: ['ssh-rsa'] DEBUG:paramiko.transport:Server did not send a server-sig-algs list; defaulting to our first preferred algo ('ssh-rsa') DEBUG:paramiko.transport:NOTE: you may use the 'disabled_algorithms' SSHClient/Transport init kwarg to disable that or other algorithms if your server does not support them! INFO:paramiko.transport:Authentication (publickey) successful! ``` --- src/lib/command.py | 12 ++++- src/lib/rsync.py | 6 ++- src/models/job.py | 8 ++++ tests/etc/localhost.job | 4 ++ tests/etc/ssh-disabledalgs.job | 15 +++++++ tests/etc/ssh-no-disabledalgs.job | 5 +++ tests/test_command.py | 75 +++++++++++++++++++++++++++++-- tests/test_director.py | 72 +++++++++++++++++++++++++++-- tests/test_job.py | 19 ++++++++ tests/test_rsync.py | 36 ++++++++++++++- 10 files changed, 240 insertions(+), 12 deletions(-) create mode 100644 tests/etc/ssh-disabledalgs.job create mode 100644 tests/etc/ssh-no-disabledalgs.job diff --git a/src/lib/command.py b/src/lib/command.py index bdddde9..ab340ed 100644 --- a/src/lib/command.py +++ b/src/lib/command.py @@ -18,9 +18,13 @@ def checkRemoteHostViaSshProtocol(self, job, time.sleep(initial_wait) for x in range(retries): try: + kwargs = {} + if job.sshdisabledalgs: + kwargs['disabled_algorithms'] = job.sshdisabledalgs ssh.connect(job.hostname, username=job.sshusername, - key_filename=job.sshprivatekey) + key_filename=job.sshprivatekey, + **kwargs) logger().info(("Successfully connected to host" " via ssh protocol (%s)") % job.hostname) return True @@ -38,9 +42,13 @@ def executeRemoteCommand(self, job, command): ssh = paramiko.SSHClient() ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) try: + kwargs = {} + if job.sshdisabledalgs: + kwargs['disabled_algorithms'] = job.sshdisabledalgs ssh.connect(job.hostname, username=job.sshusername, - key_filename=job.sshprivatekey) + key_filename=job.sshprivatekey, + **kwargs) logger().info(("Successfully connected to host" " via ssh protocol (%s)") % job.hostname) diff --git a/src/lib/rsync.py b/src/lib/rsync.py index 4af8a83..dbd93a4 100644 --- a/src/lib/rsync.py +++ b/src/lib/rsync.py @@ -60,9 +60,13 @@ def checkRemoteHostViaSshProtocol(self, job, time.sleep(initial_wait) for x in range(retries): try: + kwargs = {} + if job.sshdisabledalgs: + kwargs['disabled_algorithms'] = job.sshdisabledalgs ssh.connect(job.hostname, username=job.sshusername, - key_filename=job.sshprivatekey) + key_filename=job.sshprivatekey, + **kwargs) logger().info(("Successfully connected to host" " via ssh protocol (%s)") % job.hostname) return True diff --git a/src/models/job.py b/src/models/job.py index 7607874..56968d5 100644 --- a/src/models/job.py +++ b/src/models/job.py @@ -17,6 +17,7 @@ def __init__(self, filepath=None): self.rsyncshare = None self.sshusername = None self.sshprivatekey = None + self.sshdisabledalgs = None self.port = None self.backupdir = None self.speedlimitkb = None @@ -112,6 +113,13 @@ def readJob(self): self.enabled = False return False + try: + self.sshdisabledalgs = jobconfig['ssh_disabledalgs'] + except Exception: + self.sshdisabledalgs = {} + logger().debug(("%s: No or invalid ssh_disabledalgs is set," + " using default") % self.filepath) + try: self.port = jobconfig['port'] except Exception: diff --git a/tests/etc/localhost.job b/tests/etc/localhost.job index d923b93..7f3b029 100644 --- a/tests/etc/localhost.job +++ b/tests/etc/localhost.job @@ -6,6 +6,10 @@ ssh: False ssh_sudo: False ssh_username: autorsyncbackup ssh_privatekey: /home/autorsyncbackup/.ssh/id_rsa +ssh_disabledalgs: + pubkeys: + - rsa-sha2-512 + - rsa-sha2-256 port: 10873 backupdir: /tmp speedlimitkb: 10000 diff --git a/tests/etc/ssh-disabledalgs.job b/tests/etc/ssh-disabledalgs.job new file mode 100644 index 0000000..0588b62 --- /dev/null +++ b/tests/etc/ssh-disabledalgs.job @@ -0,0 +1,15 @@ +hostname: localhost +ssh: True +ssh_sudo: False +ssh_username: autorsyncbackup +ssh_privatekey: /home/autorsyncbackup/.ssh/id_rsa +ssh_disabledalgs: + pubkeys: + - rsa-sha2-512 + - rsa-sha2-256 +backupdir: /tmp +include: #formerly fileset + - /etc +exclude: + - "*.bak" + - ".cache/*" diff --git a/tests/etc/ssh-no-disabledalgs.job b/tests/etc/ssh-no-disabledalgs.job new file mode 100644 index 0000000..2ca4acb --- /dev/null +++ b/tests/etc/ssh-no-disabledalgs.job @@ -0,0 +1,5 @@ +hostname: 'localhost' +ssh: true +ssh_sudo: true +ssh_username: 'autorsyncbackup' +ssh_privatekey: /home/autorsyncbackup/.ssh/id_rsa diff --git a/tests/test_command.py b/tests/test_command.py index 249fae3..d469ddd 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -9,7 +9,10 @@ def test_checkRemoteHostViaSshProtocol(monkeypatch): - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): return True monkeypatch.setattr(paramiko.SSHClient, 'connect', mock_connect) @@ -28,10 +31,36 @@ def mock_connect(self, hostname, username=None, key_filename=None): assert ret is True +def test_checkRemoteHostViaSshProtocol_no_disabledalgs(monkeypatch): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): + return True + + monkeypatch.setattr(paramiko.SSHClient, 'connect', mock_connect) + + path = os.path.join( + os.path.dirname(__file__), + 'etc/ssh-no-disabledalgs.job', + ) + + j = job(path) + + cmd = command() + + ret = cmd.checkRemoteHostViaSshProtocol(j) + + assert ret is True + + def test_checkRemoteHostViaSshProtocol_exception(monkeypatch, caplog): logger().debuglevel = 3 - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): raise IOError('Mock connection failed') monkeypatch.setattr(paramiko.SSHClient, 'connect', mock_connect) @@ -53,7 +82,10 @@ def mock_connect(self, hostname, username=None, key_filename=None): def test_executeRemoteCommand(monkeypatch): - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): return True def mock_exec_command(self, command): @@ -81,10 +113,45 @@ def mock_exec_command(self, command): assert 'Mock STDOUT' in stdout +def test_executeRemoteCommand_no_disabledalgs(monkeypatch): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): + return True + + def mock_exec_command(self, command): + stdin = io.StringIO('') + stdout = io.StringIO('Mock STDOUT\n0') + stderr = io.StringIO('') + + return stdin, stdout, stderr + + monkeypatch.setattr(paramiko.SSHClient, 'connect', mock_connect) + monkeypatch.setattr(paramiko.SSHClient, 'exec_command', mock_exec_command) + + path = os.path.join( + os.path.dirname(__file__), + 'etc/ssh-no-disabledalgs.job', + ) + + j = job(path) + + cmd = command() + + (status, stdout, stderr) = cmd.executeRemoteCommand(j, 'uptime') + + assert status == 0 + assert 'Mock STDOUT' in stdout + + def test_executeRemoteCommand_exception(monkeypatch, caplog): logger().debuglevel = 3 - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): raise IOError('Mock connection failed') monkeypatch.setattr(paramiko.SSHClient, 'connect', mock_connect) diff --git a/tests/test_director.py b/tests/test_director.py index a6d5b60..82f363f 100644 --- a/tests/test_director.py +++ b/tests/test_director.py @@ -150,7 +150,10 @@ def test_executeJobs_local(test_config, tmp_path): def test_executeJobs_remote(test_config, tmp_path, monkeypatch): - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): return True def mock_exec_command(self, command): @@ -280,7 +283,10 @@ def test_executeRsync_remote_before_fail( caplog, monkeypatch ): - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): return True def mock_exec_command(self, command): @@ -355,7 +361,10 @@ def test_executeRsync_remote_after_fail( caplog, monkeypatch ): - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): return True def mock_exec_command(self, command): @@ -703,6 +712,63 @@ def test_getBackupsSize(test_config, tmp_path): assert avg == float(backupstatus['rsync_literal_data']) +@pytest.mark.xfail(strict=False) +def test_getBackupsSize_no_daily(test_config, tmp_path): + config().jobspooldirectory = str(tmp_path) + + path = create_job(str(tmp_path)) + + j = job(path) + + d = director() + + d.checkBackupEnvironment(j) + + for interval in [ + 'weekly', + 'monthly', + ]: + subdir = time.strftime("%Y-%m-%d_%H-%M-%S_backup.0") + + interval_path = os.path.join( + j.backupdir, + j.hostname, + interval, + subdir, + ) + + os.makedirs(interval_path) + + if interval == 'daily': + symlink = os.path.join( + j.backupdir, + j.hostname, + 'latest', + ) + + os.symlink(interval_path, symlink) + + jrh = jobrunhistory(check=True) + + backupstatus = { + 'hostname': j.hostname, + 'startdatetime': time.time() - 1, + 'rsync_total_file_size': 1337, + 'rsync_literal_data': 42, + } + + hooks = [] + + jrh.insertJob(backupstatus, hooks) + + time.sleep(0.1) + + (size, avg) = d.getBackupsSize(j) + + assert size != 0 + assert avg == float(backupstatus['rsync_literal_data']) + + @pytest.mark.xfail(strict=False) def test_getBackupsSize_no_history(test_config, tmp_path): config().jobspooldirectory = str(tmp_path) diff --git a/tests/test_job.py b/tests/test_job.py index 08dfe7b..4822961 100644 --- a/tests/test_job.py +++ b/tests/test_job.py @@ -20,6 +20,7 @@ def test_job(): assert j.ssh_sudo is False assert j.sshusername == 'autorsyncbackup' assert j.sshprivatekey == '/home/autorsyncbackup/.ssh/id_rsa' + assert j.sshdisabledalgs == {'pubkeys': ['rsa-sha2-512', 'rsa-sha2-256']} assert j.rsyncusername == 'autorsyncbackup' assert j.rsyncpassword == 'fee-fi-fo-fum' assert j.port == 10873 @@ -66,6 +67,7 @@ def test_default_config(test_config): assert j.ssh_sudo is False assert j.sshusername is None assert j.sshprivatekey is None + assert j.sshdisabledalgs == {} assert j.rsyncusername == 'autorsyncbackup' assert j.rsyncpassword == 'fee-fi-fo-fum' assert j.rsyncshare == 'backup' @@ -145,6 +147,23 @@ def test_ssh_no_privatekey(): assert j.sshprivatekey is None +def test_ssh_no_disabledalgs(): + path = os.path.join( + os.path.dirname(__file__), + 'etc/ssh-no-disabledalgs.job', + ) + + j = job(path) + + assert j.enabled is False + assert j.hostname == 'localhost' + assert j.ssh is True + assert j.ssh_sudo is True + assert j.sshusername == 'autorsyncbackup' + assert j.sshprivatekey == '/home/autorsyncbackup/.ssh/id_rsa' + assert j.sshdisabledalgs == {} + + def test_ssh_no_port(): path = os.path.join( os.path.dirname(__file__), diff --git a/tests/test_rsync.py b/tests/test_rsync.py index 3c7c784..360c87c 100644 --- a/tests/test_rsync.py +++ b/tests/test_rsync.py @@ -53,7 +53,10 @@ def test_checkRemoteHost_rsync_fail(test_config, tmp_path): def test_checkRemoteHost_ssh(test_config, tmp_path, monkeypatch): - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): return True monkeypatch.setattr(paramiko.SSHClient, 'connect', mock_connect) @@ -75,8 +78,37 @@ def mock_connect(self, hostname, username=None, key_filename=None): assert j.backupstatus['rsync_backup_status'] == int(ret) +def test_checkRemoteHost_ssh_disabledalgs(test_config, tmp_path, monkeypatch): + def mock_connect( + self, hostname, + username=None, key_filename=None, disabled_algorithms=None, + ): + return True + + monkeypatch.setattr(paramiko.SSHClient, 'connect', mock_connect) + + config().jobspooldirectory = str(tmp_path) + + path = os.path.join( + os.path.dirname(__file__), + 'etc/ssh-disabledalgs.job', + ) + + j = job(path) + + r = rsync() + + ret = r.checkRemoteHost(j) + + assert ret is True + assert j.backupstatus['rsync_backup_status'] == int(ret) + + def test_checkRemoteHost_ssh_fail(test_config, tmp_path, monkeypatch): - def mock_connect(self, hostname, username=None, key_filename=None): + def mock_connect( + self, hostname, + username=None, key_filename=None, **kwargs, + ): raise IOError('Mock connection failed') monkeypatch.setattr(paramiko.SSHClient, 'connect', mock_connect)