Bug #12696

Tails Installer rejects working USB drives, pretending they're not "removable"

Added by goupille 2017-06-14 08:28:50 . Updated 2017-07-17 16:00:39 .

Status:
Resolved
Priority:
High
Assignee:
Category:
Installation
Target version:
Start date:
2017-06-14
Due date:
% Done:

100%

Feature Branch:
bugfix/12696-too-strict-removable-check
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Installer
Deliverable for:

Description

there are multiple reports about usb sticks working “flawlessly” before, or know as removable, that are now considered by tails installer as non removable.

a user gave us these links :

http://storaged.org/doc/udisks2-api/latest/gdbus-org.freedesktop.UDisks2.Drive.html#gdbus-property-org-freedesktop-UDisks2-Drive.MediaRemovable

and

http://storaged.org/doc/udisks2-api/latest/gdbus-org.freedesktop.UDisks2.Drive.html#gdbus-property-org-freedesktop-UDisks2-Drive.Removable

Tails installer seems to search for a “mediaremovable” flag instead of “removable” wrongly detect removable devices as non-removable.


Subtasks


Related issues

Related to Tails - Feature #10731: Tails Installer should not allow installing on non-removable USB sticks Rejected
Has duplicate Tails - Bug #12697: tails-installer rejecting viable usb drives Duplicate 2017-06-14

History

#1 Updated by goupille 2017-06-14 08:29:39

  • related to Feature #10731: Tails Installer should not allow installing on non-removable USB sticks added

#2 Updated by intrigeri 2017-06-14 14:28:47

  • has duplicate Bug #12697: tails-installer rejecting viable usb drives added

#3 Updated by intrigeri 2017-06-14 14:32:17

  • Category set to Installation
  • Status changed from New to Confirmed
  • Priority changed from Normal to Elevated
  • Target version set to Tails_3.1
  • Type of work changed from Research to Code
  • Affected tool set to Installer

This was also reported by a few people on reddit already, so it really seems we got something wrong. I want to fix that in Tails 3.1, and in Debian/Ubuntu ASAP. kurono, can you take care of the code part the end of June? If you can’t, no big deal, I’ll handle it.

#4 Updated by intrigeri 2017-06-14 14:33:24

  • Subject changed from Tails-installer non-removable device check can be wrong to Tails Installer rejects working USB drives, pretending they're not "removable"

#5 Updated by intrigeri 2017-06-15 08:21:10

  • Priority changed from Elevated to High

We’re taking a lot of heat about this bug, so I’m documenting a workaround as we speak.

#6 Updated by intrigeri 2017-06-15 08:32:38

intrigeri wrote:
> We’re taking a lot of heat about this bug, so I’m documenting a workaround as we speak.

Pushed: https://tails.boum.org/news/version_3.0/#known-issues

#7 Updated by intrigeri 2017-06-15 09:08:22

  • Status changed from Confirmed to In Progress

Applied in changeset commit:f2be3a7c78b2764bf7bb5769fb19db80cbb87536.

#8 Updated by kurono 2017-06-15 19:28:43

  • Assignee changed from intrigeri to kurono

intrigeri wrote:
> This was also reported by a few people on reddit already, so it really seems we got something wrong. I want to fix that in Tails 3.1, and in Debian/Ubuntu ASAP. kurono, can you take care of the code part the end of June? If you can’t, no big deal, I’ll handle it.

ahh sorry about this :(, I will take care!

#9 Updated by kurono 2017-06-15 20:35:09

kurono wrote:
> intrigeri wrote:
> > This was also reported by a few people on reddit already, so it really seems we got something wrong. I want to fix that in Tails 3.1, and in Debian/Ubuntu ASAP. kurono, can you take care of the code part the end of June? If you can’t, no big deal, I’ll handle it.
>
> ahh sorry about this :(, I will take care!

So, I have tested with the user’s suggested solution:

diff --git a/tails_installer/creator.py b/tails_installer/creator.py index 95a4102..164bb48 100755 --- a/tails_installer/creator.py +++ b/tails_installer/creator.py -608,7 +608,7 class LinuxTailsInstallerCreator(TailsInstallerCreator): 'size': drive.props.size, 'mounted_partitions': set(), 'is_device_big_enough': True, - 'removable': drive.props.media_removable, + 'removable': drive.props.removable, }

and it starts listing again the non-removable virtual USB sticks we were using for testing.
Probably we were not using that variable because of this. I don’t have at the moment one of the affected Sanddisk USB’s.
If somebody has it, he/she could test it just by applying that minor change to the installer code.

#10 Updated by kurono 2017-06-15 20:36:16

  • Assignee changed from kurono to intrigeri

#11 Updated by arnaud 2017-06-19 05:51:37

Hey, I just happen to be working on the same topic (USB/SDCARD detection). I was also wondering about how to use the properties reported by UDisks, so I collected the output of ‘udisksctl dump’ for various devices I had around me. Let me share the result with you.

Device \ Property Vendor Model Removable Ejectable Media MediaAvailable MediaRemovable
USB
Silver thing Generic Flash Disk true true true true
Rotterdam School Generic Flash Disk true true true true
DT101 G2 Kingston DataTraveler 2.0 true true thumb true false
DataTraveler G3 Kingston DataTraveler 3.0 true true thumb true false
Mem Up 4GB 090c POP KEY true true true true
iPod Classic Apple iPod Classic true true true true
SDIO
Samsung 16EVO 00000 true false flash_sd true true
Marantz SDHC AF SD true false flash_sd true true
OPTICAL
DVD Drive TSSTcorp CDDVDW … true true depends true

So it looks like MediaRemovable and Ejectable can’t be trusted to detect removable devices.

#12 Updated by intrigeri 2017-06-23 10:17:59

  • % Done changed from 0 to 10

My current plan is to:

  1. short-term fix: apply the s/media_removable/removable/ change in order to fix the regression that is harming too many users and the current situation is way worse than the problems Feature #10731 tried to fix
  2. reopen Feature #10731, because as kurono says, the above fix will essentially revert to what we did before the branch for Feature #10731 was merged, and thus the two problems Feature #10731 was about will be back
  3. leave it to kurono to address Feature #10731 if you want; I’ll leave some notes on that ticket to explain how I understand the problem

#13 Updated by intrigeri 2017-06-23 10:25:44

  • Assignee changed from intrigeri to bertagaz
  • % Done changed from 10 to 20
  • QA Check set to Ready for QA
  • Feature Branch set to bugfix/12696-too-strict-removable-check

Please review, merge into master, and then follow https://tails.boum.org/contribute/release_process/tails-installer/. The sooner, the better: we can upload the fixed package to Debian and Ubuntu without waiting for Tails 3.1, which will solve the problem for many people.

#14 Updated by intrigeri 2017-06-29 06:40:10

I see a discrepancy:

tails-installer | 4.4.16+dfsg-0tails1 |           devel | amd64, source
tails-installer | 4.4.17+dfsg-0tails1 |          stable | amd64, source

Besides, usually (except at release time) we would upload to a topic branch, using the APT overlays mechanism, so we can test the proposed package before it goes in any release branch. No big deal this time, but please keep this in mind for the rest of your RM’ing cycle :)

#15 Updated by bertagaz 2017-06-29 10:47:08

intrigeri wrote:
> I see a discrepancy:
>
> […]
>
> Besides, usually (except at release time) we would upload to a topic branch, using the APT overlays mechanism, so we can test the proposed package before it goes in any release branch. No big deal this time, but please keep this in mind for the rest of your RM’ing cycle :)

Yes, the APT overlay mechanism completely got out of my head, struggling as I was with this package release process. I’ll delete the one I uploaded in stable and will upload it in a topic branch/suite. The cleaner the better.

#16 Updated by intrigeri 2017-06-29 17:07:12

It looks very much like you didn’t import updated translations from Transifex. https://tails.boum.org/contribute/release_process/ explains how to do so. Sorry the individual packages’ release process doesn’t.

#17 Updated by intrigeri 2017-06-30 06:19:03

  • Target version changed from Tails_3.1 to Tails_3.0.1

#18 Updated by bertagaz 2017-06-30 10:26:45

  • QA Check changed from Ready for QA to Dev Needed

intrigeri wrote:
> It looks very much like you didn’t import updated translations from Transifex. https://tails.boum.org/contribute/release_process/ explains how to do so. Sorry the individual packages’ release process doesn’t.

Indeed I didn’t. I’ve been trying to do so since yesterday, but it seems that the imported translations break the build of the package. It fails at the dpkg-source time and I’m struggling to get how to fix that.

#19 Updated by intrigeri 2017-06-30 10:47:15

> it seems that the imported translations break the build of the package. It fails at the dpkg-source time and I’m struggling to get how to fix that.

The translations shall be imported on the master branch, otherwise dpkg-source will complain about upstream files being modified.

What error do you see?

#20 Updated by bertagaz 2017-06-30 11:27:06

intrigeri wrote:
> > it seems that the imported translations break the build of the package. It fails at the dpkg-source time and I’m struggling to get how to fix that.
>
> The translations shall be imported on the master branch, otherwise dpkg-source will complain about upstream files being modified.

That’s what I’ve done, it’s in master.

> What error do you see?

I get this:

$ ARCH=amd64 DIST=stretch BUILDER=pbuilder sudo gbp buildpackage
dpkg-source: info: applying tails-installer-as-helper
dh clean --with python2
   dh_testdir
   debian/rules override_dh_auto_clean
make[1]: Entering directory '/home/build/deb_packages/liveusb-creator.git'
dh_auto_clean
        python setup.py clean -a
running clean
removing 'build/mo' (and everything under it)
removing 'build/lib.linux-x86_64-2.7' (and everything under it)
'build/bdist.linux-x86_64' does not exist -- can't clean it
removing 'build/scripts-2.7' (and everything under it)
removing 'build'
        find . -name \*.pyc -exec rm {} \+
rm -f tails-installer.1 tails-installer-launcher.1
make[1]: Leaving directory '/home/build/deb_packages/liveusb-creator.git'
   dh_clean
dpkg-source: info: using source format '3.0 (quilt)'
dpkg-source: info: building tails-installer using existing ./tails-installer_4.4.18+dfsg.orig.tar.gz
dpkg-source: info: local changes detected, the modified files are:
 liveusb-creator.git/data/tails-installer-launcher.ui.h
 liveusb-creator.git/data/tails-installer.ui.h
dpkg-source: error: aborting due to unexpected upstream changes, see /tmp/tails-installer_4.4.18+dfsg-0tails1.diff.stFHyY
dpkg-source: info: you can integrate the local changes with dpkg-source --commit
gbp:error: 'pdebuild' failed: it exited with 2

Indeed some languages have quite some changes for the *.ui.h files mentionned.

I’ve just built the persistence-setup package, and it went fine (so that lower the risk of some mis-configuration in my build setup). I’m following the tails-installer release process from what’s currently in the master branch.

#21 Updated by intrigeri 2017-06-30 11:37:13

You’ve run git clean -fdx as documented, right?

Please push all your current WIP branches so I can try to reproduce.

#22 Updated by bertagaz 2017-06-30 11:56:09

  • QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> You’ve run git clean -fdx as documented, right?

Grmh, no I jumped this step. Works fine now, uploaded a newer package with translations updated, let see if Jenkins find new problems or not.

#23 Updated by bertagaz 2017-06-30 16:46:21

  • Status changed from In Progress to Fix committed
  • Assignee deleted (bertagaz)
  • % Done changed from 20 to 100
  • QA Check changed from Ready for QA to Pass

bertagaz wrote:
> Grmh, no I jumped this step. Works fine now, uploaded a newer package with translations updated, let see if Jenkins find new problems or not.

Jenkins doesn’t not complain so this is now merged.

#24 Updated by intrigeri 2017-07-05 19:03:52

  • Status changed from Fix committed to Resolved

#25 Updated by GigabyteProductions 2017-07-08 04:23:48

In addition to correcting the detection of removable media, is there a reason not to allow users to install onto non-removable media, besides trying to protect users from themselves?

#26 Updated by Anonymous 2017-07-08 11:37:38

GigabyteProductions wrote:
> In addition to correcting the detection of removable media, is there a reason not to allow users to install onto non-removable media, besides trying to protect users from themselves?

Yes.

Tails also uses the live-media=removable boot paramenter while would make Tails fail to boot on non-removable drives.

#27 Updated by GigabyteProductions 2017-07-09 14:55:37

While I understand that Tails has a responsibility to protect users from harming themselves, there comes a point where Tails prevents users who know what they are doing, or know exactly what they want to do, from being able to operate at all. I think that providing an option to allow installing onto non-removable storage is necessary, even if it makes users jump through hoops to accept the consequences. Not providing an option or telling people “Tails is not for your use case” encourages users to use some other solution entirely, which produces split/duplicate effort.

#28 Updated by intrigeri 2017-07-17 16:00:39

GigabyteProductions, I believe Feature #6397 will address your concerns… once someone does the work it requires.