Bug #17112

Persistence config files "insecure" backup get emptied in some situations

Added by intrigeri 2019-10-02 06:31:20 . Updated 2020-02-06 16:43:45 .

Status:
Resolved
Priority:
Elevated
Assignee:
Category:
Persistence
Target version:
Tails_4.3
Start date:
Due date:
% Done:

100%

Feature Branch:
bugfix/17112-dont-empty-persistence-config-backup
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

As we can see on Bug #10976#note-73 and Bug #10976#note-66, if the permissions/ownership of the root directory of the persistent filesystem are wrong, live-additional-software.conf.insecure_disabled ends up being empty, which is unfortunate as it’s meant to be a backup that the user can later restore by hand.

I believe that’s because every call to disable_and_create_empty_persistence_conf_file in live-persist will rename the current live-additional-software.conf to live-additional-software.conf.insecure_disabled. So if the user immediately fixes the problem, the first time it happens, then we’re good. But if they reboot and unlock their persistence again, the (non-empty) backup gets overwritten by live-persist which replaces it with the new, empty config file it created during last boot.

Solution: ensure disable_and_create_empty_persistence_conf_file does not replace a non-empty .insecure_disabled backup file with an empty one.

Priority >> normal because it makes it harder to recover from the already painful Bug #10976.

Note: I don’t understand why, in Cody’s report, the same problem did not happen for persistence.conf. There might be another problem on top of this one. But this should not block it from fixing the problem I’ve already understood here :)

Subtasks

Related issues

Related to Tails - Bug #10976: persistence.conf lost, recoverable by reconfiguring In Progress 2019-02-12
Related to Tails - Bug #17116: Improve lost persistence.conf instructions on known issues page Resolved
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed

History

#1 Updated by intrigeri 2019-10-02 06:31:36

  • related to Bug #10976: persistence.conf lost, recoverable by reconfiguring added

#2 Updated by intrigeri 2019-10-02 06:31:46

#3 Updated by cbrownstein 2019-10-02 18:43:04

  • related to Bug #17116: Improve lost persistence.conf instructions on known issues page added

#4 Updated by intrigeri 2020-01-28 12:31:29

  • Status changed from Confirmed to In Progress

Applied in changeset commit:tails|ee6ff8d1dce58b5447c6f939c4158a1d86a76a09.

#5 Updated by intrigeri 2020-01-28 12:32:03

  • Assignee set to intrigeri
  • Target version set to Tails_4.3
  • Feature Branch set to bugfix/17112-dont-empty-persistence-config-backup

#6 Updated by intrigeri 2020-02-04 08:56:35

  • Status changed from In Progress to Needs Validation
  • Assignee deleted (intrigeri)

No regression spotted by Jenkins but that does not tell much: we have no test coverage for this code path.

I’ve tested this manually:

  1. Install an image built from this branch on a (virtual) USB stick
  2. Reboot, set up persistence, enable Additional Software
  3. Reboot, unlock persistence, install a package
  4. Reboot, unlock persistence, confirm that the additional package is installed
  5. Set unsafe permissions: chmod 777 /live/persistence/TailsData_unlocked
  6. Reboot, unlock persistence, confirm that:
    1. live-additional-software.conf was backup’ed and replaced by a new, empty file
    2. the persistent volume root directory still has unsafe access rights
    3. no persistent data directory was bind-mounted
  7. Reboot, unlock persistence, confirm that:
    1. the live-additional-software.conf and persistence.conf non-empty backup files are still there, and their mtime matches the previous reboot when they got created initially (while without the fix brought by this branch, they would have been replaced by empty files, which is no good)
    2. the persistent volume root directory still has unsafe access rights
    3. no persistent data directory was bind-mounted

#7 Updated by anonym 2020-02-05 15:18:44

  • Assignee set to anonym

#8 Updated by anonym 2020-02-06 16:43:29

intrigeri wrote:
> No regression spotted by Jenkins

Confirmed, including a local run.

> but that does not tell much: we have no test coverage for this code path.

Indeed. :)

> I’ve tested this manually:
>
> # Install an image built from this branch on a (virtual) USB stick
> # Reboot, set up persistence, enable Additional Software
> # Reboot, unlock persistence, install a package
> # Reboot, unlock persistence, confirm that the additional package is installed
> # Set unsafe permissions: chmod 777 /live/persistence/TailsData_unlocked
> # Reboot, unlock persistence, confirm that:
> ## live-additional-software.conf was backup’ed and replaced by a new, empty file
> ## the persistent volume root directory still has unsafe access rights
> ## no persistent data directory was bind-mounted
> # Reboot, unlock persistence, confirm that:
> ## the live-additional-software.conf and persistence.conf non-empty backup files are still there, and their mtime matches the previous reboot when they got created initially (while without the fix brought by this branch, they would have been replaced by empty files, which is no good)
> ## the persistent volume root directory still has unsafe access rights
> ## no persistent data directory was bind-mounted

I confirm both that this test makes sense, and that it works for me too.

And the code makes sense (TIL: test -s checks for existence of non-empty files). Merged!

#9 Updated by anonym 2020-02-06 16:43:45

  • Status changed from Needs Validation to Resolved
  • Assignee deleted (anonym)
  • % Done changed from 0 to 100