Tails

#1 Updated by sajolida 2015-03-15 18:47:19

• File 0001-Start-tails-additional-software-install-in-the-backg.patch added

#5 Updated by sajolida 2015-03-18 13:07:52

• Assignee deleted (alant)
• Type of work changed from Code to Research

I’m too busy to work on this actively right now, but that’s a lot for all the useful info.

I’m marking this as a Research ticket. It’s bothering me enough so I’ll keep it in mind even if not assigned to me. I’ll try to dig into codesearch but I would be very happy if someone do that faster than me.

#9 Updated by sajolida 2015-07-28 07:02:22

• Assignee set to sajolida
• QA Check set to Info Needed

We did some more testing and found out that /var/cache/apt/archives/ was stored in persistence while we could store all /var/cache/apt/ and hope for some performance improvements. Unfortunately this didn’t change much and we couldn’t fine any other low-hanging fruit.

Maybe installing the additional software packages could be started right after persistence is unlock, so we could gain a few seconds while the user after that might choose other options in the Greeter (like a root password).

As a next step, I’ll provide with a syslog for installing a single additional package.

#10 Updated by sajolida 2015-08-14 11:58:10

• Description updated
• Target version set to 2016

#11 Updated by sajolida 2015-08-14 12:51:20

• related to Bug #9050: Persistence preset: additional software added

#12 Updated by sajolida 2015-08-14 12:52:32

• related to Feature #5996: Additional software configuration GUI added

#13 Updated by sajolida 2015-08-14 12:53:47

• related to Feature #9819: Check for updates of "additional software" only once added

#16 Updated by sajolida 2015-08-17 08:23:16

• Description updated

#18 Updated by sajolida 2015-08-19 07:02:56

• related to deleted (Feature #5996: Additional software configuration GUI)

#19 Updated by sajolida 2015-08-19 07:02:58

• related to deleted (Feature #9819: Check for updates of "additional software" only once)

#20 Updated by sajolida 2015-08-19 07:03:03

• related to deleted (Bug #9050: Persistence preset: additional software)

#21 Updated by sajolida 2015-08-19 07:03:26

• Affected tool changed from Greeter to Additional Software Packages

#22 Updated by sajolida 2015-08-25 09:04:41

• Description updated

#23 Updated by Dr_Whax 2016-08-20 13:37:50

• Description updated
• Target version changed from 2016 to 2017

#24 Updated by sajolida 2016-08-23 09:59:16

• related to Bug #11319: There should be user feedback when the session is loading added

#25 Updated by BitingBird 2017-08-28 19:57:07

• Target version changed from 2017 to 2018

#26 Updated by BitingBird 2017-08-28 19:58:36

• related to Feature #9819: Check for updates of "additional software" only once added

#27 Updated by intrigeri 2017-08-30 17:28:31

• Deliverable for set to 299

#28 Updated by sajolida 2017-09-06 16:16:49

• Assignee changed from sajolida to alant

Alan will solve this :)

#29 Updated by Anonymous 2017-09-07 08:52:05

• Parent task set to Feature #14568

#30 Updated by Anonymous 2017-09-07 08:54:37

• Parent task changed from Feature #14568 to Feature #5551

#31 Updated by Anonymous 2017-09-07 09:04:23

• Target version changed from 2018 to Tails_3.5

Setting a target version - is this solvable while working on the Offline mode feature? Otherwise, change target version.

#33 Updated by intrigeri 2017-09-22 07:54:48

• Parent task changed from Feature #5551 to Feature #14568

#35 Updated by Anonymous 2017-10-16 12:14:57

• Target version changed from Tails_3.5 to Tails_3.6

#36 Updated by Anonymous 2017-10-16 12:15:45

• Target version changed from Tails_3.6 to Tails_3.5

#37 Updated by alant 2017-10-16 13:08:24

• Due date set to 2017-12-15

I plan to work on that before mid december 2017, whith the objective to sort the proposals and requirements and make a plan. If possible this will include implementing a first step to deliver as soon as possible.

#40 Updated by alant 2017-12-14 16:31:49

• QA Check changed from Info Needed to Dev Needed
• Blueprint set to https://tails.boum.org/blueprint/additional_software_packages/dont_block_desktop_startup

#43 Updated by alant 2017-12-15 14:07:51

• Status changed from Confirmed to In Progress

Applied in changeset commit:4e6099094470f43d9c2eb1208471a05e303b1548.

#44 Updated by alant 2017-12-15 14:36:01

• Feature Branch set to bugfix/9059-additional-software-after-session
• Type of work changed from Research to Code

#46 Updated by intrigeri 2017-12-15 16:00:35

• Type of work changed from Code to Research

Also, the handling of the flag file we rely on (/run/live-additional-software/activated) has a couple issues:

• it’s created before the ASP are installed, while we want to wait until after they are installed to avoid lock conflicts; that’s why it’s called “activated”; once we have a flag file that indicates what we care about here, it should be called differently
• it’s created in code written in imperative language, which makes it harder to reason about than if it was part of the dependency & ordering we are otherwise managing with systemd; I believe this can easily confuse us in a way that leads to bugs of the same kind as the one reported in the previous bullet point. So I recommend creating this flag file with ExecStartPost= instead.

I’ve pushed a few fixed on top of the branch. I have no other comment at this point :)

#47 Updated by alant 2017-12-15 16:39:38

• Type of work changed from Research to Code

intrigeri wrote:
> Also, the handling of the flag file we rely on (/run/live-additional-software/activated) has a couple issues:

Current flag file is there to prevent the upgrade to be launched (and use bandwidth) if there are no additional software packages activated.

This can be replaced by a flag file managed by systemd unit if the live-additional-software.conf were created outside of tails-additional-software.

#48 Updated by intrigeri 2017-12-15 17:29:45

• Type of work changed from Code to Research

Dear Alan, you’ll be pleased to learn that there might be a way to eliminate the dreaded while .. sleep loop: see systemd.path(5). I’ve not checked if it’s available in Stretch, but my understanding is that it’s exactly what we wanted (but we forgot to check if it existed, or perhaps I looked in the list of directives and not in the list of unit types; whatever).

#50 Updated by intrigeri 2017-12-15 17:59:54

• % Done changed from 0 to 10

#52 Updated by alant 2018-01-02 15:42:30

• blocked by Bug #15132: devel branch FTBFS since aufs-dkms 4.14 is in sid added

#55 Updated by alant 2018-01-04 15:47:43

• % Done changed from 10 to 50
• Type of work changed from Research to Code

I implemented:

• systemd path activation
• handeling flag file with ExecStartPost
• handeling creation of empty live-additional-software.conf in live-persist
• user notifications

I’m gonna build and test that. Remaining to do is systemd unit hardening.

#56 Updated by alant 2018-01-04 22:13:21

• % Done changed from 50 to 60

I just pushed a few fixes, it should be working now.

#58 Updated by alant 2018-01-13 19:31:16

• Assignee changed from alant to segfault
• % Done changed from 60 to 70
• QA Check changed from Dev Needed to Ready for QA

It works fine for me. Please review and merge if you’re fine with it.

I’m not really convinced about the capability listing in commit 16c6a28. I’m afraid it may be fragile and I’m not sure we gain a lot form it. What do you think?

#59 Updated by alant 2018-01-13 19:34:52

• related to Feature #14571: Code review for Implement Offline Mode added

#61 Updated by segfault 2018-01-14 16:09:54

• Assignee changed from segfault to alant
• QA Check changed from Ready for QA to Info Needed

> It works fine for me. Please review and merge if you’re fine with it.

Looks good to me! Just a few comments:

1. During my test, tails-additional-software-install.service failed, because APT didn’t have any package lists and therefore failed to install any packages. After running apt update, it worked as expected. I don’t know if this is a regression, I guess this is also the case in the current version (I didn’t check), but I think it should be fixed anyway. A fix could be to run apt update if the installation fails and run it again afterwards.

2. From commit 4e609909:

+# XXX:Buster this sudo rule should be replaced by a polkit rule once we have
+# policykit >= 0.106. The rule is already in
+# [[blueprint/additional_software_packages/org.boum.tails.additional-software.rules]]
+# and should be installed in /usr/share/polkit-1/rules.d/

I doubt that we will ever have polkit >= 0.106. polkit 0.106 has been release in 2012, and it seems like Debian does not want to move from the old .pkla to the new .rules format (I looked everywhere, but couldn’t find any discussion or reasoning for this. I suspect they don’t want to break compatibility with custom .pkla rules.). Unfortunately, we can’t specify the “unit” and “verb” in .pkla format. So we could either keep the sudoers solution, or run tails-additional-software as its own user and allow it to manage all systemd services.

3. This one is nitpicking, but it made it harder for me to understand the option: In system/tails-additional-software-install.service, user/tails-additional-software-install.service, and tails-additional-software-upgrade.service, you set this TimeoutStartSec=0. According to the manual, it should be TimeoutStartSec=infinity (using 0 instead seems to work for compatibility reasons), which is also the default value for oneshot type services, so it could just be removed.

4. From commit 6e649379:

+        subprocess.check_output([cmd, title, body], stderr=subprocess.STDOUT)

subprocess.check_call() should be used instead of check_output if the output is not used.

5. I think the user visible strings can be improved. I would recommend:

s/Your additional software installation failed/The additional software installation failed/
s/The installation failed. Please check your additional software configuration, or read the system log to understand better the problem./The additional software installation failed. Please check the additional software configuration and the system log (journalctl) for errors./
s/Your additional software are installed/Additional software installed successfully/
s/Your additional software are ready to use./The additional software was installed successfully and is ready to use./
s/Your additional software upgrade failed/Failed to update the additional software/
s/The check for upgrades failed. This might be due to a network problem. Please check your network connection, try to restart Tails, or read the system log to understand better the problem./Updating the additional software failed. Please check your network connection, check the system log (journalctl) for errors, or try restarting Tails./
s/Your additional software are up to date/Additional software successfully updated/
s/The upgrade was successful./The additional software was successfully updated./

But I’m sure this could still be improved by someone more into technical writing.

6.
> I’m not really convinced about the capability listing in commit 16c6a28. I’m afraid it may be fragile and I’m not sure we gain a lot form it. What do you think?

To be able to review these capabilities, I would like to know how you got this list. Trial and error? Might there be capabilities missing which might for example only be required for some packages? One solution to get rid of at least the capabilities for APT/DPKG would be to use PackageKit for package installation. I think we could use this code based on what I use in onionkit (formerly known as Tails Server):

<code class="python">
# Based on isenkram, copyright (C) 2013,2015-2016 Petter Reinholdtsen <pere@debian.org>

from typing import List
from pydbus import SystemBus

import gi
from gi.repository import GLib
gi.require_version('PackageKitGlib', '1.0')
from gi.repository import PackageKitGlib as packagekit


class PackageKitManager(object):
    """Helper to install and uninstall packages using PackageKit.
    Reference for PackageKit DBus API: https://www.freedesktop.org/software/PackageKit/gtk-doc/api-reference.html"""
    def __init__(self):        
        self.client = SystemBus().get("org.freedesktop.PackageKit").Client()
        self.client.set_interactive(False)

    def get_package_ids(self, package_names: List[str], package_filter: int) -> List[object]:
        results = self.client.resolve(
            package_filter,  # package filter flags (for example to resolve only installed packages)
            tuple(package_names),  # names of packages to resolve
            None,  # cancellable
            self.progress_callback,  # progress callback
            None  # progress callback user data
        )
        self.assert_success(results)

        packages_resolved = {package.get_name(): package for package in results.get_package_array()}
        for package_name in package_names:
            if package_name not in packages_resolved or not packages_resolved[package_name]:
                raise PackageException("Package %r not found" % package_name)

        return [packages_resolved[package_name].get_id() for package_name in package_names]

    def refresh_cache(self):
        """Refresh package cache from all enabled repositories"""
        results = self.client.refresh_cache(
            False,  # don't enforce reloading if there is valid, up to date data
            None,  # cancellable
            self.progress_callback,  # progress callback
            None  # progress callback user data
        )
        self.assert_success(results)

    def install(self, package_names: List[str]):
        """Run a PackageKit transaction to install given packages."""
        package_ids = self.get_package_ids(package_names, package_filter=0)

        # Start package installation
        results = self.client.install_packages(
            1 << packagekit.TransactionFlagEnum.ONLY_TRUSTED,  # only install signed packages
            package_ids,  # packages to install
            None,  # cancellable
            self.progress_callback,  # progress callback
            None  # progress callback user data
        )
        self.assert_success(results)

    @staticmethod
    def assert_success(results):
        """Check that the most recent operation was a success."""
        if results and results.get_error_code() is not None:
            error = results.get_error_code()
            error_code = error.get_code() if error else None
            error_string = packagekit.ErrorEnum.to_string(error_code) if error_code else None
            error_details = error.get_details() if error else None
            raise PackageException(error_string, error_details)
</code>

Then we would have to execute tails-additional-software as its own user and use a polkit rule to allow it to use PackageKit:

# /etc/polkit-1/localauthority/10-vendor.d/org.boum.tails.additional-software.pkla
[Allow tails-additional-software to install packages]
Identity=unix-user:tails-additional-software
Action=org.freedesktop.packagekit.package-install
ResultAny=no
ResultInactive=no  # or maybe this required for systemd services?
ResultActive=yes

#63 Updated by alant 2018-01-15 22:14:38

• Assignee changed from alant to segfault

segfault wrote:
> 1. During my test, tails-additional-software-install.service failed, because APT didn’t have any package lists and therefore failed to install any packages. After running apt update, it worked as expected. I don’t know if this is a regression, I guess this is also the case in the current version (I didn’t check), but I think it should be fixed anyway. A fix could be to run apt update if the installation fails and run it again afterwards.
>
Did you actually installed the packages manually before configuring it as an additional software? That is the only way it is working, and that is what the GUI to be designed will do. As tails-additional-software-install.service should work offline, it can’t rely on apt update. This is tails-additional-software-upgrade.service role.

Anyway I think this is no regression and doesn’t need to be fixed.

> 2. From commit 4e609909:
> […]
>
> I doubt that we will ever have polkit >= 0.106. polkit 0.106 has been release in 2012, and it seems like Debian does not want to move from the old .pkla to the new .rules format (I looked everywhere, but couldn’t find any discussion or reasoning for this. I suspect they don’t want to break compatibility with custom .pkla rules.). Unfortunately, we can’t specify the “unit” and “verb” in .pkla format. So we could either keep the sudoers solution, or run tails-additional-software as its own user and allow it to manage all systemd services.
>
I’m in favor of keeping the sudo solution for now. The service you speak about is a systemd --user one as it relies on the session being launched, so it must run as amnesia.

> 3. This one is nitpicking, but it made it harder for me to understand the option: In system/tails-additional-software-install.service, user/tails-additional-software-install.service, and tails-additional-software-upgrade.service, you set this TimeoutStartSec=0. According to the manual, it should be TimeoutStartSec=infinity (using 0 instead seems to work for compatibility reasons), which is also the default value for oneshot type services, so it could just be removed.
>
OK, replaced it by infinity

> 4. From commit 6e649379:
> […]
>
> subprocess.check_call() should be used instead of check_output if the output is not used.
>
0K.

> 5. I think the user visible strings can be improved. I would recommend:
> […]
>
> But I’m sure this could still be improved by someone more into technical writing.
>
I’d like to wait UX input on that, especially as some of these string have been written by sajolida. I sent an email to tails-ux mailing list with your proposals.

> 6.
> > I’m not really convinced about the capability listing in commit 16c6a28. I’m afraid it may be fragile and I’m not sure we gain a lot form it. What do you think?
>
> To be able to review these capabilities, I would like to know how you got this list. Trial and error?

Yes, trial and error.

> Might there be capabilities missing which might for example only be required for some packages?

Maybe. What cases are you thinking about?

> One solution to get rid of at least the capabilities for APT/DPKG would be to use PackageKit for package installation. I think we could use this code based on what I use in onionkit (formerly known as Tails Server):
>
I don’t think we should do that as part of this task, because this is completly rewriting tails-additional-software and I don’t think it is time to do that. We’d need to think more about it, and if we end up deciding it’s a good idea, find time to do it properly.

#64 Updated by segfault 2018-01-15 23:12:11

• Assignee changed from segfault to anonym
• QA Check changed from Info Needed to Ready for QA

alant wrote:
> segfault wrote:
> > 1. During my test, tails-additional-software-install.service failed, because APT didn’t have any package lists and therefore failed to install any packages. After running apt update, it worked as expected. I don’t know if this is a regression, I guess this is also the case in the current version (I didn’t check), but I think it should be fixed anyway. A fix could be to run apt update if the installation fails and run it again afterwards.
> >
> Did you actually installed the packages manually before configuring it as an additional software? That is the only way it is working, and that is what the GUI to be designed will do.

I didn’t look at what the GUI will do, but currently this is evidently not the only way it is working, because it also worked for me after running apt update, without installing the packages manually.

> As tails-additional-software-install.service should work offline, it can’t rely on apt update. This is tails-additional-software-upgrade.service role.

What I proposed is a fallback for when the installation cannot be performed offline.

> Anyway I think this is no regression and doesn’t need to be fixed.

Ok, maybe not in context of this ticket. But, from my point of view, this is an easily fixable issue, which, at least with the current workflow, users probably run into.

> > 2. From commit 4e609909:
> > […]
> >
> > I doubt that we will ever have polkit >= 0.106. polkit 0.106 has been release in 2012, and it seems like Debian does not want to move from the old .pkla to the new .rules format (I looked everywhere, but couldn’t find any discussion or reasoning for this. I suspect they don’t want to break compatibility with custom .pkla rules.). Unfortunately, we can’t specify the “unit” and “verb” in .pkla format. So we could either keep the sudoers solution, or run tails-additional-software as its own user and allow it to manage all systemd services.
> >
> I’m in favor of keeping the sudo solution for now. The service you speak about is a systemd --user one as it relies on the session being launched, so it must run as amnesia.

Well, the process could still run as another user, if we allow amnesia to run the executable as another user. But I’m fine with keeping the sudoers solution.

> > 5. I think the user visible strings can be improved. I would recommend:
> > […]
> >
> > But I’m sure this could still be improved by someone more into technical writing.
> >
> I’d like to wait UX input on that, especially as some of these string have been written by sajolida. I sent an email to tails-ux mailing list with your proposals.

Ok.

> > 6.
> > > I’m not really convinced about the capability listing in commit 16c6a28. I’m afraid it may be fragile and I’m not sure we gain a lot form it. What do you think?
> >
> > To be able to review these capabilities, I would like to know how you got this list. Trial and error?
>
> Yes, trial and error.

Ok. I would like to not redo this work and trust you on this, because it seems effortful and I already spent more time on this review than I can clock.

> > Might there be capabilities missing which might for example only be required for some packages?
>
> Maybe. What cases are you thinking about?

No specific case in mind, but I think that packages do different things in their installation scripts, and by testing only some packages, we could miss capabilities required by other packages.

> > One solution to get rid of at least the capabilities for APT/DPKG would be to use PackageKit for package installation. I think we could use this code based on what I use in onionkit (formerly known as Tails Server):
> >
> I don’t think we should do that as part of this task, because this is completly rewriting tails-additional-software and I don’t think it is time to do that. We’d need to think more about it, and if we end up deciding it’s a good idea, find time to do it properly.

Ok.

Assigning to anonym now.

#67 Updated by anonym 2018-01-19 12:28:10

• Feature Branch changed from bugfix/9059-additional-software-after-session to bugfix/9059-additional-software-after-session OR bugfix/9059-additional-software-after-session-rebased-on-stable

git checkout -b bugfix/9059-additional-software-after-session-rebased-on-stable origin/bugfix/9059-additional-software-after-session
git rebase --onto origin/stable origin/devel
git push -u origin bugfix/9059-additional-software-after-session-rebased-on-stable

#68 Updated by anonym 2018-01-22 08:49:15

• Target version changed from Tails_3.5 to Tails_3.6

#69 Updated by anonym 2018-01-26 13:30:44

• Feature Branch changed from bugfix/9059-additional-software-after-session OR bugfix/9059-additional-software-after-session-rebased-on-stable to bugfix/9059-additional-software-after-session

#72 Updated by anonym 2018-01-27 13:49:05

• Status changed from In Progress to Fix committed
• Assignee deleted (anonym)
• % Done changed from 70 to 100
• QA Check changed from Ready for QA to Pass
• Feature Branch changed from bugfix/9059-additional-software-after-session to bugfix/9059-additional-software-after-session greeter:bugfix/9059-additional-software-after-session

anonym wrote:
> Meanwhile I wasted an hour investigating what I thought was a bug, but then turned out to be that the branch at the moment builds with Tails Greeter 1.0.4, so APS is started in the Greeter’s PostLogin hook… grr…

I built an updated package and uploaded it. Testing went super fine, and the code looks good. I fixed my only complaint myself (commit:031a66becf84ee85e5e408b8c7f7a57311b3337a). Merged!

#74 Updated by bertagaz 2018-03-14 11:12:02

• Status changed from Fix committed to Resolved

#75 Updated by intrigeri 2018-08-20 09:12:48

• related to #15824 added

#76 Updated by intrigeri 2018-08-22 11:19:49

• related to Feature #15828: Ensure users can enable GNOME Shell extensions on Wayland added