Bug #10431

Icedove profile migration broke read-only persistence

Added by alant 2015-10-27 07:51:04 . Updated 2015-11-03 11:28:14 .

Status:
Resolved
Priority:
Elevated
Assignee:
Category:
Persistence
Target version:
Tails_1.7
Start date:
2015-10-27
Due date:
% Done:

100%

Feature Branch:
bugfix/10431-fix-read-only-persistence
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Email Client
Deliverable for:
244

Description

During 1.7-rc1 testing, we found that RO persistence works if there is no “Claws mail” preset but fails if there is one.

This is likely due to the following code in config/chroot_local-includes/usr/local/sbin/live-persist that is logically wrong:

       if [ "$PERSISTENCE_READONLY" = true ]
       then
               warning "Persistence configuration needs to be migrated, but read only was selected; please retry in read-write mode"
       fi
       echo "$NEW_PRESET       source=$NEW_PRESET_SOURCE" \
               >> "$CONFIG" \
               || error "Failed to make $NEW_PRESET: $?"
       warning "Successfully made $NEW_PRESET persistent"
       fi
}

The code issues a warning, but the continues to try to do impossible things. If we want to error() we should to it immediately in the if-statement above. We also should notify users when this happens, since few will read syslog.

By the way, this code is very poorly indented, which may be the cause of introducing this error.

Subtasks

History

#1 Updated by kytv 2015-10-29 12:43:33

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 30
  • QA Check set to Ready for QA
  • Feature Branch set to kytv/bugfix/10431-fix-read-only-persistence

This branch only fixes the (stupid) logic error. It does not change the indentation or tabs to spaces, nor does it notify the user.

I have not tested to get trigger the bug, nor have I don’t anything with notification-daemon (assuming that’s available at the time this script is run).

#2 Updated by anonym 2015-10-30 05:27:23

  • Status changed from In Progress to Fix committed
  • % Done changed from 30 to 100

Applied in changeset commit:586eb7c5396e76e9d0464165a0d3c81f7349a730.

#3 Updated by anonym 2015-10-30 07:19:41

  • Assignee deleted (anonym)
  • QA Check changed from Ready for QA to Pass

#4 Updated by bertagaz 2015-11-01 06:04:24

  • Status changed from Fix committed to In Progress

Applied in changeset commit:5666ebcb74bc7458b79500ca8b32a4d23d31cc99.

#5 Updated by bertagaz 2015-11-01 06:15:47

  • Assignee set to anonym
  • QA Check changed from Pass to Ready for QA

I had some doubts the previous fix in this branch actually does fix something.

In fact it didn’t: the original reported problem was that the live-persist script would error out (exit 1) if persistence was read-only, because it tried to make the new persistence config anyway. Then the Tails greeter is not happy and just stop functioning.

So replacing the warning for read-only persistence by an error in the end lead to the same situation as previously, but a bit sooner, before it tries to make the persistence preset.

In place, when persistence is read-only, we have to “warn” users (a notification would have been better, but well…) that the preset is not created, and go on with the boot process without trying to touch to the persistence preset.

I’ve tested in bare-metal this fix, read-only persistence works now when it contains a preset for claws-mail, as well as the preset creation when the persistence is read-write. I’m pretty confident it’s a definitive fix for this bug.

#6 Updated by bertagaz 2015-11-01 06:25:28

  • Priority changed from Normal to Elevated
  • Feature Branch changed from kytv/bugfix/10431-fix-read-only-persistence to bugfix/10431-fix-read-only-persistence

Marking this as elevated, as it’s a quite serious bug.

#7 Updated by anonym 2015-11-02 14:35:07

  • Status changed from In Progress to Fix committed

Applied in changeset commit:f7df359d5e33c3c09c19f484be57dc230e118ff4.

#8 Updated by anonym 2015-11-02 16:01:11

  • Assignee deleted (anonym)
  • QA Check changed from Ready for QA to Pass

#9 Updated by anonym 2015-11-03 11:28:14

  • Status changed from Fix committed to Resolved