Re: [Tails-dev] [Was: [review'n'merge:1.1] #6559, #7062, #62…

Delete this message

Reply to this message
Author: anonym
Date:  
To: The Tails public development discussion list
Subject: Re: [Tails-dev] [Was: [review'n'merge:1.1] #6559, #7062, #6275: test/6559-adapt-test-suite-for-Wheezy]
13/05/14 22:48, intrigeri wrote:
> anonym wrote (13 May 2014 15:54:22 GMT) :
>>> Regarding the removal of test suite steps that look at notifications:

[...]
>>>   2. These steps were also useful to... actually test that we're
>>>      displaying these notifications. In the context of the test suite,
>>>      they are annoying, but for actual users, they are features.
>>>      We have dropped tests such as "I see a notification that Unsafe
>>>      browser is starting" from our manual test suite when the
>>>      automated test suite implemented it. And now, we're dropping this
>>>      test from the automated test suite too. I've not carefully
>>>      checked if other similar tests were removed. That looks like
>>>      a regression to me. What do you think?

>
>> ACK. I'll bring it back tomorrow with some needed modifications.


I've brought these checks back in commits bd095dd and 7b6d3d6. You
really nailed it this time. :) Running the test suite now uncovered that
the start and stop notifications aren't shown for the Unsafe Web
Browser. Apparently running `notify-send` as root (as the script does)
shows nothing in Wheezy, so I switched to `tails-notify-user`. Hopefully
you don't mind that I fixed this in this branch and not a dedicated one.

Any way, I'm worried that Wheezy's default notification timeout of 5
seconds (which BTW is not a bug [1]) may end up causing us problems
since Sikuli certainly may need more than that to find the image, in
particular on slow hardware. We'll see.

[1] https://bugzilla.gnome.org/show_bug.cgi?id=665761

> Thank you. I won't be able to test and review this before May 24, so
> better nag bertagaz to take care of it (at least of the testing part,
> if he doesn't feel comfortable reviewing the thing).


His email hasn't been working, so I hope you can have a look.

>>> I don't understand the complexity added by commit bc6805ec, and the
>>> commit message tells me nothing about it. Have you seen actual cases
>>> when TailsData was *not* unlocked+mounted, after t-p-s exited?
>
>> That step is also used in scenarios when t-p-s isn't run at all, and
>> persistence is disabled, see e.g. "Booting Tails from a USB drive with a
>> disabled persistent partition".
>
> OK... but I still don't get why some action is needed on Wheezy, or
> rather, I don't get why no action was needed on Squeeze, given what
> I've reported on #7056.


cryptsetup changed [1] what it does when you try to luksOpen an already
unlocked device in version 1.2. With Squeeze's 1.1.2 we got away with just:

    @vm.execute("echo #{pwd} | cryptsetup luksOpen #{dev} #{name}")
    luks_dev = "/dev/mapper/#{name}"


even if the device was already open... but that resulted in *an
additional* dm mapping being created! You can even mount both mappings,
but the two do not seem to sync when you write to one of them, so it
looks like a really bad thing to allow. So, since we have 1.4.3 in
Wheezy we cannot do this any more, and we need to find (potential) old
mappings and use them instead.

I admit I didn't do all this homework before now -- earlier I just went
with the error message ("Cannot use device ... (already mapped or
mounted)") and couldn't see any other way around this. Still, I agree
the for-loop seems unnecessarily complex but I couldn't find any
convenient way to query udisks for existing LUKS mappings from a
particular device.

[1]
https://code.google.com/p/cryptsetup/source/detail?r=b7caa72acdd1d66cfabe5358392be330276099fb

Cheers!