Skip to content

Commit c2f6e31

Browse files
agnersagners
authored
Check password early on backup restore (#5519)
Introduce a validate password method which only peaks into the archive to validate the password before starting the actual restore process. This makes sure that a wrong password returns an error even when restoring the backup in background.
1 parent 61b3787 commit c2f6e31

File tree

4 files changed

+70
-16
lines changed

4 files changed

+70
-16
lines changed

‎supervisor/backups/backup.py

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -296,12 +296,13 @@ def new(
296296
if not compressed:
297297
self._data[ATTR_COMPRESSED] = False
298298

299-
def set_password(self, password: str) -> bool:
299+
def set_password(self, password: str | None) -> None:
300300
"""Set the password for an existing backup."""
301-
if not password:
302-
return False
303-
self._init_password(password)
304-
return True
301+
if password:
302+
self._init_password(password)
303+
else:
304+
self._key = None
305+
self._aes = None
305306

306307
def _init_password(self, password: str) -> None:
307308
"""Set password + init aes cipher."""
@@ -334,6 +335,48 @@ def _decrypt_data(self, data: str) -> str:
334335
data = padder.update(decrypt.update(b64decode(data))) + padder.finalize()
335336
return data.decode()
336337

338+
async def validate_password(self) -> bool:
339+
"""Validate backup password.
340+
341+
Returns false only when the password is known to be wrong.
342+
"""
343+
344+
def _validate_file() -> bool:
345+
ending = f".tar{'.gz' if self.compressed else ''}"
346+
347+
with tarfile.open(self.tarfile, "r:") as backup:
348+
test_tar_name = next(
349+
(
350+
entry.name
351+
for entry in backup.getmembers()
352+
if entry.name.endswith(ending)
353+
),
354+
None,
355+
)
356+
if not test_tar_name:
357+
_LOGGER.warning("No tar file found to validate password with")
358+
return True
359+
360+
test_tar_file = backup.extractfile(test_tar_name)
361+
try:
362+
with SecureTarFile(
363+
ending, # Not used
364+
gzip=self.compressed,
365+
key=self._key,
366+
mode="r",
367+
fileobj=test_tar_file,
368+
):
369+
# If we can read the tar file, the password is correct
370+
return True
371+
except tarfile.ReadError:
372+
_LOGGER.debug("Invalid password")
373+
return False
374+
except Exception: # pylint: disable=broad-exception-caught
375+
_LOGGER.exception("Unexpected error validating password")
376+
return True
377+
378+
return await self.sys_run_in_executor(_validate_file)
379+
337380
async def load(self):
338381
"""Read backup.json from tar file."""
339382
if not self.tarfile.is_file():

‎supervisor/backups/manager.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -687,10 +687,12 @@ async def do_restore_full(
687687
f"{backup.slug} is only a partial backup!", _LOGGER.error
688688
)
689689

690-
if backup.protected and not backup.set_password(password):
691-
raise BackupInvalidError(
692-
f"Invalid password for backup {backup.slug}", _LOGGER.error
693-
)
690+
if backup.protected:
691+
backup.set_password(password)
692+
if not await backup.validate_password():
693+
raise BackupInvalidError(
694+
f"Invalid password for backup {backup.slug}", _LOGGER.error
695+
)
694696

695697
if backup.supervisor_version > self.sys_supervisor.version:
696698
raise BackupInvalidError(
@@ -755,10 +757,12 @@ async def do_restore_partial(
755757
folder_list.remove(FOLDER_HOMEASSISTANT)
756758
homeassistant = True
757759

758-
if backup.protected and not backup.set_password(password):
759-
raise BackupInvalidError(
760-
f"Invalid password for backup {backup.slug}", _LOGGER.error
761-
)
760+
if backup.protected:
761+
backup.set_password(password)
762+
if not await backup.validate_password():
763+
raise BackupInvalidError(
764+
f"Invalid password for backup {backup.slug}", _LOGGER.error
765+
)
762766

763767
if backup.homeassistant is None and homeassistant:
764768
raise BackupInvalidError(

‎tests/api/test_backups.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ async def test_restore_immediate_errors(
478478

479479
with (
480480
patch.object(Backup, "protected", new=PropertyMock(return_value=True)),
481-
patch.object(Backup, "set_password", return_value=False),
481+
patch.object(Backup, "validate_password", return_value=False),
482482
):
483483
resp = await api_client.post(
484484
f"/backups/{mock_partial_backup.slug}/restore/partial",

‎tests/backups/test_manager.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ async def test_do_restore_full(coresys: CoreSys, full_backup_mock, install_addon
209209
manager = BackupManager(coresys)
210210

211211
backup_instance = full_backup_mock.return_value
212+
backup_instance.protected = False
212213
backup_instance.sys_addons = coresys.addons
213214
backup_instance.remove_delta_addons = partial(
214215
Backup.remove_delta_addons, backup_instance
@@ -241,6 +242,7 @@ async def test_do_restore_full_different_addon(
241242
manager = BackupManager(coresys)
242243

243244
backup_instance = full_backup_mock.return_value
245+
backup_instance.protected = False
244246
backup_instance.addon_list = ["differentslug"]
245247
backup_instance.sys_addons = coresys.addons
246248
backup_instance.remove_delta_addons = partial(
@@ -273,6 +275,7 @@ async def test_do_restore_partial_minimal(
273275
manager = BackupManager(coresys)
274276

275277
backup_instance = partial_backup_mock.return_value
278+
backup_instance.protected = False
276279
assert await manager.do_restore_partial(backup_instance, homeassistant=False)
277280

278281
backup_instance.restore_homeassistant.assert_not_called()
@@ -297,6 +300,7 @@ async def test_do_restore_partial_maximal(coresys: CoreSys, partial_backup_mock)
297300
manager = BackupManager(coresys)
298301

299302
backup_instance = partial_backup_mock.return_value
303+
backup_instance.protected = False
300304
assert await manager.do_restore_partial(
301305
backup_instance,
302306
addons=[TEST_ADDON_SLUG],
@@ -330,7 +334,7 @@ async def test_fail_invalid_full_backup(
330334

331335
backup_instance = full_backup_mock.return_value
332336
backup_instance.protected = True
333-
backup_instance.set_password.return_value = False
337+
backup_instance.validate_password = AsyncMock(return_value=False)
334338

335339
with pytest.raises(BackupInvalidError):
336340
await manager.do_restore_full(backup_instance)
@@ -359,7 +363,7 @@ async def test_fail_invalid_partial_backup(
359363

360364
backup_instance = partial_backup_mock.return_value
361365
backup_instance.protected = True
362-
backup_instance.set_password.return_value = False
366+
backup_instance.validate_password = AsyncMock(return_value=False)
363367

364368
with pytest.raises(BackupInvalidError):
365369
await manager.do_restore_partial(backup_instance)
@@ -407,6 +411,7 @@ async def test_restore_error(
407411
coresys.homeassistant.core.start = AsyncMock(return_value=None)
408412

409413
backup_instance = full_backup_mock.return_value
414+
backup_instance.protected = False
410415
backup_instance.restore_dockerconfig.side_effect = BackupError()
411416
with pytest.raises(BackupError):
412417
await coresys.backups.do_restore_full(backup_instance)
@@ -1818,6 +1823,7 @@ async def test_monitoring_after_full_restore(
18181823
manager = BackupManager(coresys)
18191824

18201825
backup_instance = full_backup_mock.return_value
1826+
backup_instance.protected = False
18211827
assert await manager.do_restore_full(backup_instance)
18221828

18231829
backup_instance.restore_addons.assert_called_once_with([TEST_ADDON_SLUG])
@@ -1835,6 +1841,7 @@ async def test_monitoring_after_partial_restore(
18351841
manager = BackupManager(coresys)
18361842

18371843
backup_instance = partial_backup_mock.return_value
1844+
backup_instance.protected = False
18381845
assert await manager.do_restore_partial(backup_instance, addons=[TEST_ADDON_SLUG])
18391846

18401847
backup_instance.restore_addons.assert_called_once_with([TEST_ADDON_SLUG])

0 commit comments

Comments
 (0)