Project

General

Profile

Bug #12738

Feature #5630: Reproducible builds

Remove gconf

Added by anonym over 2 years ago. Updated almost 2 years ago.

Status:
Resolved
Priority:
Elevated
Assignee:
-
Category:
Build system
Target version:
Start date:
06/19/2017
Due date:
% Done:

100%

Feature Branch:
tails:feature/12738-remove-gconf greeter:feature/12738-remove-gconf
Type of work:
Code
Blueprint:
Starter:
Affected tool:

Description

See #12608#note-18 for arnaud's diffoscope report. There are also .torrent:s for the good ISO, and for arnaud's.

The files in question are /var/lib/gconf/defaults/%gconf-tree-*.xml (88 files for various locales). Example diff:

├── /var/lib/gconf/defaults/%gconf-tree-af.xml
│ │ @@ -1,11 +1,17 @@
│ │  <?xml version="1.0"?>
│ │  <gconf>
│ │     <dir name="schemas">
│ │             <dir name="system">
│ │ +                   <dir name="smb">
│ │ +                           <entry name="workgroup">
│ │ +                                   <local_schema short_desc="SMB-werkgroep">
│ │ +                                   </local_schema>
│ │ +                           </entry>
│ │ +                   </dir>
│ │                     <dir name="proxy">
│ │                             <entry name="autoconfig_url">
│ │                                     <local_schema short_desc="Outomatiese instaanopstelling-URL">
│ │                                             <longdesc>URL wat instaanopstellingswaardes verskaf.</longdesc>
│ │                                     </local_schema>
│ │                             </entry>
│ │                             <entry name="socks_port">
│ │ @@ -76,20 +82,14 @@
│ │                             </entry>
│ │                             <entry name="use_http_proxy">
│ │                                     <local_schema short_desc="Gebruik HTTP-instaanbediener">
│ │                                             <longdesc>Aktiveer die instaanopstelling wanneer toegang tot HTTP oor die internet verkry word.</longdesc>
│ │                                     </local_schema>
│ │                             </entry>
│ │                     </dir>
│ │ -                   <dir name="smb">
│ │ -                           <entry name="workgroup">
│ │ -                                   <local_schema short_desc="SMB-werkgroep">
│ │ -                                   </local_schema>
│ │ -                           </entry>
│ │ -                   </dir>
│ │             </dir>
│ │             <dir name="desktop">
│ │                     <dir name="gnome">
│ │                             <dir name="url-handlers">
│ │                                     <dir name="h323">
│ │                                             <entry name="needs_terminal">
│ │                                                     <local_schema short_desc="Laat die program in &apos;n terminaal loop">

diffoscope-muri.txt.bz2 (31.8 KB) anonym, 08/22/2017 08:26 PM


Related issues

Blocks Tails - Bug #13625: Root Terminal cannot start graphical applications Resolved 08/14/2017

Associated revisions

Revision 5233f9e0 (diff)
Added by anonym about 2 years ago

Enable the feature-12738-reproducible-gconf-xml APT overlay (refs: #12738).

This is the temporary solution for #12738 in Tails 3.1* which should
be reverted (and fixed permanently by removing gconf) in Tails 3.2.

Revision e0b200e3 (diff)
Added by anonym about 2 years ago

Use pkexec instead of gksudo.

gksu is unmaintained, buggy (e.g. #12000) stuff, and it is the only
reason we ship GConf, which we want to remove (refs: #12738).

Revision 4da014df (diff)
Added by anonym about 2 years ago

Remove gconf, gksu and their dependencies.

We don't use gksu, and it was the only reason we installed
gconf. Removing these packages + deps will not only save quite some
space (30 MiB installed) but will also remove a source of
non-determinism in the filesystem (element order in
/var/lib/gconf/defaults/%gconf-tree-*.xml) which made Tails
unreproducible.

We also remove:

  • libgnomevfs2-extra, which was previously used for SSH/FTP support in
    Nautilus, but isn't needed for that any more.
  • libgnome2-bin which provides gnome-open. We'll find a replacement
    for that in a later commit.
  • Configurations and scripts that become obsolete because of these
    removals.

Will-fix: #12738

Revision 38cb09cc (diff)
Added by anonym about 2 years ago

Use xdg-open for gnome-open.

We don't install libgnome2-bin, which provided gnome-open, any
more (refs: #12738).

Revision 4a39064c (diff)
Added by anonym about 2 years ago

Test suite: test the GNOME Root Terminal.

While we're at it, let's Dogtailify some of our old GNOME Terminal
interaction.

Refs: #12738

Revision 2b770d45 (diff)
Added by intrigeri about 2 years ago

Fix custom polkit prompt org.boum.tails.root-terminal.policy.in and its translations (refs: #12738).

Without this, even after I've refreshed the POT and PO files, translated the new
string into French and run ./refresh-translations again, the custom polkit
prompt is not displayed even in English, which is no big surprise because the
resulting
config/chroot_local-includes/usr/share/polkit-1/actions/org.boum.tails.root-terminal.policy{,.in}
are identical i.e. no translation is integrated, and even the original string in
English is still in a <_message> block that won't be used at runtime.

This is explained by the fact that `intltool-update --desktop-style' can't
possibly work for XML files, so let's use --xml-style instead for
org.boum.tails.root-terminal.policy.

Note that I don't master awk so I don't know if the awk magics in
intltool_merge_desktop will actually stop after having dealt with the block
it's supposed to act on, or if that hack was assuming that we would never need
any other similar trick (which was wrong as soon as it was written, oh well) and
as a result it's going to try to handle org.boum.tails.root-terminal.policy
with --desktop-style anyway. But as we've seen already this is a no-op so
I won't bother learning awk today, and hopefully some day this code will be
rewritten in a language I understand.

Revision 1175b269
Added by anonym about 2 years ago

Merge remote-tracking branch 'origin/feature/12738-remove-gconf' into devel

Fix-committed: #12738, #13625

Revision 889964b1 (diff)
Added by anonym about 2 years ago

Revert test suite changes introduced for #12738.

They are not robust enough, and will have to be finished
later (#14656) so the rest of this branch can be merged in time for
Tails 3.2~rc1.

Refs: #12738, #14656

Revision ce2a2220 (diff)
Added by anonym about 2 years ago

Ensure that root's bash has an appropriate PATH.

We want at least the Root Terminal to have our /usr/local/{bin,sbin}
scripts and overrides in its PATH. Before #12738, when the Root
Terminal was started with gksu instead of pkexec, it inherited
amnesia's PATH, but that is not the case any more.

In particular, now our /usr/local/sbin/nautilus override will be
active in the Root Terminal, which was the point of that override.

Refs: #12034, #12738

History

#1 Updated by anonym over 2 years ago

I also affects /var/cache/cracklib/src-dicts in the same way (same XML element is moved).

#2 Updated by intrigeri over 2 years ago

I suggest we reorder the content of these files as a post-processing step.

#3 Updated by anonym over 2 years ago

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

Sure. But why can it happen at all, and could it cause differences elsewhere? I'm curious if there some nondeterminism in the order things are processed. Without delving to deep into this one, do you have any ideas, lamby?

#4 Updated by intrigeri over 2 years ago

  • Parent task set to #5630

#5 Updated by lamby about 2 years ago

Filed the cracklib issue as https://labs.riseup.net/code/issues/12909

#6 Updated by lamby about 2 years ago

  • Assignee changed from lamby to anonym

I suggest we reorder the content of these files as a post-processing step.

I disagree; re-ordering a recursive XML structure seems a little fraught and more complicated than just fixing the issue.

I think this is http://sources.debian.net/src/gconf/3.2.6-4/backends/markup-tree.c/#L681 or http://sources.debian.net/src/gconf/3.2.6-4/backends/xml-dir.c/#L885

No patch yet. Should be reasonably straightforward - permission to proceed? :)

#7 Updated by anonym about 2 years ago

  • Assignee changed from anonym to lamby

lamby wrote:

I suggest we reorder the content of these files as a post-processing step.

I disagree; re-ordering a recursive XML structure seems a little fraught and more complicated than just fixing the issue.

Huh, I expected it to be easy, which seems supported by this SO answer: https://stackoverflow.com/questions/8385358/lxml-sorting-tag-order/8387132#8387132

Of course, things often look "easy" before you actually try. :)

I think this is http://sources.debian.net/src/gconf/3.2.6-4/backends/markup-tree.c/#L681 or http://sources.debian.net/src/gconf/3.2.6-4/backends/xml-dir.c/#L885

No patch yet. Should be reasonably straightforward - permission to proceed? :)

... but that could also apply to your hunch here. :) But I trust you if you think this is the way to go still. But given that we probably have a workaround for this reproducibility issue (but don't on the others) I think you should spend at most four hours on this. If you think so, feel free to unset the "Info Needed" status and proceed!

#8 Updated by lamby about 2 years ago

  • QA Check changed from Info Needed to Dev Needed

Well, it's not only nasty hack to fiddle the XML... and we might be breaking it in subtle ways; I don't trust my XPath to only select the parent elements we want to solve. Also, upstream fixes \o/

I think you should spend at most four hours on this

ACK, going ahead... :)

#9 Updated by lamby about 2 years ago

  • Assignee changed from lamby to anonym

Here you go… :)

diff --git a/backends/markup-tree.c b/backends/markup-tree.c
index 4857cae..fc508dd 100644
--- a/backends/markup-tree.c
+++ b/backends/markup-tree.c
@@ -255,6 +255,13 @@ struct _MarkupDir
   guint is_dir_empty : 1;
 };

+static gint
+compare_markupdirs (MarkupDir *a,
+                    MarkupDir *b)
+{
+  return strcmp(a->name, b->name);
+}
+
 static MarkupDir*
 markup_dir_new (MarkupTree *tree,
                 MarkupDir  *parent,
@@ -271,7 +278,9 @@ markup_dir_new (MarkupTree *tree,
   if (parent)
     {
       dir->subtree_root = parent->subtree_root;
-      parent->subdirs = g_slist_prepend (parent->subdirs, dir);
+      parent->subdirs = g_slist_insert_sorted (parent->subdirs,
+                                               dir,
+                                               (GCompareFunc)compare_markupdirs);
     }
   else
     {

#10 Updated by intrigeri about 2 years ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 30
  • Type of work changed from Research to Code

It looks like next step is: anonym applies this patch and uploads a package to our custom APT repo.

#11 Updated by intrigeri about 2 years ago

My 2cts, looking at the root cause of the root cause: it seems we're still shipping (long-deprecated) gconf2 only due to gksu, that we use only in /usr/local/sbin/tails-tor-launcher. If we switched to pkexec (with org.freedesktop.policykit.exec.allow_gui set for the relevant program, we would 1. make our ISO & IUK smaller (30MB installed size when removing gksu and its now-useless rdeps); 2. stop shipping so much old unmaintained bits from the GNOME 2 area; 3. not have to worry about the reproducibility issue this ticket is about. The initial effort needed to switch to pkexec is clearly higher than just patching gconf2, but it avoids creating technical debt i.e. long term maintenance costs (it's far from obvious that Chris' patch will ever be merged upstream), so I think it's totally worth it. Let's "just" do that, no?

#12 Updated by lamby about 2 years ago

intrigeri wrote:

If we switched to pkexec

Despite my psychological investment in this fix/patch, I am inclined to agree.

#13 Updated by intrigeri about 2 years ago

Despite my psychological investment in this fix/patch,

Yeah, sorry I've not wondered "why do we ship GConf in 2017, again?!" before we tasked you with this upstream issue. It's actually upstream's reply that forced me to ask myself this question :)

I am inclined to agree.

Thanks. Your opinion on this matter is greatly valuable to me.

#14 Updated by anonym about 2 years ago

  • Feature Branch set to wip/feature/12738-remove-gconf

I completely agree that removing gconf2, gksu and friends is the proper solution. But it's certainly not trivial -- the feature branch is a WIP so far:

  • The gksu package provides the Root Terminal .desktop file and the icon. In commit:235d9e0 I add a hook that downloads the gksu package and extracts these files only, instead of installing the package. It lacks an appropriate policy file to make pkexec work with gnome-terminal (i.e. allow_gui).
  • Oh, and such a policy file is also missing for Tor Launcher: commit:d4b1667 should be fixed up with the addition of such a file.
  • We explicitly install the following two packages that (transitively) depend on gconf2:
    • libgnomevfs2-extra (introduced in 5afa2a2) which we installed in for FTP/SSH handlers in Nautilus, but those protocols works fine for me without this package so it seems we can safely drop it (didn't test other protocols like Samba though).
    • libgnome2-bin (#11031, 36db19c) for /usr/bin/gnome-open. Since some applications hard-depend on that path we can't just remove gnome-open, so I'm thinking that create a symlink /usr/bin/gnome-open -> /usr/bin/xdg-open.

Also, messing with all this for the Tails 3.1 point-release feels a bit daring, but we want 3.1 to be as reproducible as possible since we're gonna ask the larger reproducible builds community to join us then. So I'm torn.

#15 Updated by intrigeri about 2 years ago

  • libgnomevfs2-extra (introduced in 5afa2a2) which we installed in for FTP/SSH handlers in Nautilus, but those protocols works fine for me without this package so it seems we can safely drop it (didn't test other protocols like Samba though).

Yeah, Nautilus has certainly moved on since 2011 :)

  • libgnome2-bin (#11031, 36db19c) for /usr/bin/gnome-open. Since some applications hard-depend on that path we can't just remove gnome-open,

Which (relevant) applications? I would hope that anything not too rusty would fallback to xdg-open.
FWIW my sid/GNOME system hasn't this package installed and I've not noticed any problem related to the lack of gnome-open.

so I'm thinking that create a symlink /usr/bin/gnome-open -> /usr/bin/xdg-open.

Yes, if needed we can do that. Bonus points if we report bugs against apps that still use gnome-open.

Also, messing with all this for the Tails 3.1 point-release feels a bit daring,

Wow, indeed. I think I could live with it (were I the RM, which I am not) but if possible let's avoid that.

but we want 3.1 to be as reproducible as possible since we're gonna ask the larger reproducible builds community to join us then. So I'm torn.

I think having a couple documented known issues is OK, and when we analyse the results we can sort them apart and say "modulo known issues X and Y that will be fixed in Tails 3.2, N% got the same SquashFS content". That's a bit sad (and sooo much weaker than "N% got the same ISO") but 1. at this point our goal is to gather data, not to boast about the results yet; 2. this pile of changes seems too risky for a point-release (even though one can't say that our RCs see much testing, so well…).

#16 Updated by lamby about 2 years ago

Allow me to ask the dumb question of why not targe tthe gconf patch in the point release instead? :)

#17 Updated by intrigeri about 2 years ago

Allow me to ask the dumb question of why not targe tthe gconf patch in the point release instead? :)

Yes. anonym proposed the same out-of-band and I agree, so we're all on the same page :)

#18 Updated by anonym about 2 years ago

  • Assignee changed from anonym to bertagaz
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch changed from wip/feature/12738-remove-gconf to 3.1:feature/12738-reproducible-gconf-xml 3.2:wip/feature/12738-remove-gconf

I applied lamby's patch from #12738#note-9, built new gconf packages and uploaded them to the feature/12738-reproducible-gconf-xml branch's APT overlay. Jenkins successfully built and tested the branch with only a single unrelated failing scenario (Tor bootstrap timeout).

As for manual verification that this didn't introduce any regression, I verified that gconftool-2 --dump / produces the exact same output vs Tails 3.0.1, and I looked at XML files, and it seemed all that differed was that they were sorted. And du -s reports the same amount of bytes for the whole directory. So it seems fine.

Please review'n'merge the feature/12738-reproducible-gconf-xml branch into stable for Tails 3.1! But let's leave this ticket alive (so no Fix-committed!) since the real fix isn't in yet -- just reassign it to me as Dev Needed even if merged.

#19 Updated by lamby about 2 years ago

anonym wrote:

As for manual verification that this didn't introduce any regression, I verified that gconftool-2 --dump / produces the exact same output vs Tails 3.0.1, and I looked at XML files, and it seemed all that differed was that they were sorted. And du -s reports the same amount of bytes for the whole directory. So it seems fine.

Woo! Thanks for merging and the comprehensive update :)

#20 Updated by bertagaz about 2 years ago

  • Assignee changed from bertagaz to anonym
  • QA Check changed from Ready for QA to Dev Needed

anonym wrote:

Please review'n'merge the feature/12738-reproducible-gconf-xml branch into stable for Tails 3.1! But let's leave this ticket alive (so no Fix-committed!) since the real fix isn't in yet -- just reassign it to me as Dev Needed even if merged.

Ok, done, hotfix will be in 3.1! :)

#21 Updated by anonym about 2 years ago

  • Target version changed from Tails_3.1 to Tails_3.2
  • Feature Branch changed from 3.1:feature/12738-reproducible-gconf-xml 3.2:wip/feature/12738-remove-gconf to wip/feature/12738-remove-gconf

#22 Updated by anonym about 2 years ago

  • Blocks Bug #13625: Root Terminal cannot start graphical applications added

#23 Updated by anonym about 2 years ago

lamby: FYI, your patch doesn't seem to be enough. The attached diff exhibits (seemingly) the same sorting issue (+ #12740, which is ok), and it's between two 3.1 builds, which has the patched gconf installed.

I say: Whatever, we're still on course to have this solved in Tails 3.2.

#24 Updated by lamby about 2 years ago

  • Assignee changed from anonym to lamby

(Taking ticket back)

Ah yes, some "nested" non-determinism we didn't spot before because the outer stuff was masking it. On it!

#25 Updated by anonym about 2 years ago

  • Assignee changed from lamby to anonym

lamby wrote:

(Taking ticket back)

Ah yes, some "nested" non-determinism we didn't spot before because the outer stuff was masking it. On it!

Wait! What I meant in #12738#note-23 with:

I say: Whatever, we're still on course to have this solved in Tails 3.2.

was: "Whatever, we will have another solution in Tails 3.2 any way, so let's not bother fixing deprecated software". So please forget about looking into gconf any further!

Taking back the ticket for that reason.

#26 Updated by anonym about 2 years ago

Oh, and sorry for not being clearer in my original message! :S

#27 Updated by lamby about 2 years ago

anonym wrote:

Oh, and sorry for not being clearer in my original message! :S

No problem, thanks for clarifying.

OOI should this be put "under" ticket 12740…?

#28 Updated by intrigeri about 2 years ago

OOI should this be put "under" ticket 12740…?

If these XML files are "cache", then yes :)

#29 Updated by anonym about 2 years ago

  • % Done changed from 30 to 50
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch changed from wip/feature/12738-remove-gconf to feature/12738-remove-gconf

I think this branch is ready for merging now! Let's see what Jenkins thinks...

#30 Updated by anonym about 2 years ago

anonym wrote:

I think this branch is ready for merging now! Let's see what Jenkins thinks...

There was a strange error: it seems typing "GNOME Terminal" in the activities overlay open the Root Terminal => pkexec prompt. I'll have a look.

Also, I implemented a scenario testing that the Root Terminal actually works.

Let's see what jenkins thinks!

#31 Updated by anonym about 2 years ago

anonym wrote:

anonym wrote:

I think this branch is ready for merging now! Let's see what Jenkins thinks...

There was a strange error: it seems typing "GNOME Terminal" in the activities overlay open the Root Terminal => pkexec prompt. I'll have a look.

Fixed with commit:c11f8041.

Also, I implemented a scenario testing that the Root Terminal actually works.

It works well for me!

Let's see what jenkins thinks!

There was no build yet due to #14602. Soon we'll have: https://jenkins.tails.boum.org/job/test_Tails_ISO_feature-12738-remove-gconf/2/

#32 Updated by anonym about 2 years ago

anonym wrote:

anonym wrote:

anonym wrote:

I think this branch is ready for merging now! Let's see what Jenkins thinks...

There was a strange error: it seems typing "GNOME Terminal" in the activities overlay open the Root Terminal => pkexec prompt. I'll have a look.

Fixed with commit:c11f8041.

Pushed trash, rewrote history so that is not 777e0c14.

Let's see what jenkins thinks!

There was no build yet due to #14602. Soon we'll have: https://jenkins.tails.boum.org/job/test_Tails_ISO_feature-12738-remove-gconf/2/

https://jenkins.tails.boum.org/job/test_Tails_ISO_feature-12738-remove-gconf/3/

#33 Updated by intrigeri about 2 years ago

  • Priority changed from Normal to Elevated

#34 Updated by anonym about 2 years ago

  • Assignee changed from anonym to intrigeri

The test suite results do not look stellar, and I'm not sure why. Can you begin with a code review and manual test while I investigate?

#35 Updated by intrigeri about 2 years ago

Code review:

  • dpkg-deb --extract would nicely replace ar x + tar (let's not rely on .deb current implementation details)
  • I'm surprised we need <allow_inactive>auth_admin</allow_inactive> in the Root Terminal and Tor Launcher polkit policies, but admitedly my polkit-fu is rusty.
  • Wrt. 38cb09cc8c3a8c7eaaa61a20ef91de74768013b6, I'm surprised we still need gnome-open in the path: it's not installed by default on Stretch anymore. We re-added it in Tails 2.0.1 to fix "URL handling at least in KeePassX, Electrum and Icedove". But on a Debian Stretch GNOME live CD, Thunderbird, KeePassX and Electrum all open URLs just fine without gnome-open, so I'm wondering if we might be forward-porting cruft uselessly.
  • Wrt. gconf settings, did you verify that everything we used to configure in there is still behaving as expected? I thought I had checked them all for obsolescence when porting to Stretch, but it might have been when porting to Jessie and I bet at least some of them are useless nowadays :)

I'll now build an ISO and test a few things manually.

#36 Updated by intrigeri about 2 years ago

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Ready for QA to Dev Needed

UX regression: in greeter.git:gdm/PostLogin.default we delete gksu.desktop when no admin password was set. This was not adjusted on this branch, so "Root Terminal" is listed even when it's not usable.

UX and security regression: previously, when starting Root Terminal, the authentication prompt would make it explicit (although not very user-friendly) what exactly the user was being asked to authenticate for, which I find useful because it's not merely about authenticating the user, it's also (incidentally) about asking the user permission to use their admin privileges for $operation — in some way, the polkit authentication agent (GNOME Shell) acts as a Portal to build mutual trust between the system and the user. On this branch, the authentication prompt instead says "Authentication Required" followed by "Debian Live User", and I cannot be certain I'm actually typing my password to allow what I asked (starting a terminal as root) or to allow something else (e.g. my exploited Pidgin could monitor execution of gnome-terminal-pkexec, win the race, ask me permission to do unspecified stuff, I'll blindly assume it's the prompt I expect for opening the Root Terminal, and the attacker will persistently root my Tails).
My understanding is that the right way to fix this is to have the polkit authentication agent that mediates the permission request & privilege escalation tell the user what the permission is asked for, see e.g. /usr/share/polkit-1/actions/com.ubuntu.pkexec.synaptic.policy. Thankfully it looks like intltool supports this file format, e.g. https://sources.debian.net/src/synaptic/0.84.2/po/synaptic.pot/#L3447 and https://sources.debian.net/src/synaptic/0.84.2/data/com.ubuntu.pkexec.synaptic.policy.in/ :)
After looking at grep --extended-regexp '<message>' /usr/share/polkit-1/actions/org.gnome.*,
I suggest using "To start a Root Terminal, you need to authenticate." as the message: most other prompts use passive voice, which is recommended against by all the tech writing guides I've read, so I've picked the message from org.gnome.controlcenter.datetime.policy, which uses the active voice.

In passing, here's another issue I've noticed is a strange behavior when starting Root Terminal for the 2nd time from the Overview: sometimes I'm asked to authenticate again and a second window appears, sometimes the active window indicator (top bar, right of "Places") says "Root Terminal" with a waiting cursor for a few seconds and then disappears without anything else happening. I don't think it's worth spending any of your time on this, but I thought I would mention it in case it rings a bell or makes you think something seriously wrong is happening.

And finally, all Root Terminal windows are labeled "Terminal", which I find really bad for UX, but that's the case on Tails 3.1 too so well.. I say let's put energy towards removing the need for the Root Terminal instead of making it better.

Other than that, manual testing went fine, congrats!

#37 Updated by intrigeri about 2 years ago

(Also, wrt. the policykit stuff, once it's done more nicely as suggested above, I'll definitely consider proposing it to GNOME Terminal upstream myself: this whole thing has really nothing Tails-specific in it.)

#38 Updated by anonym about 2 years ago

  • Assignee changed from anonym to intrigeri
  • % Done changed from 50 to 60
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch changed from feature/12738-remove-gconf to tails:feature/12738-remove-gconf greeter:feature/12738-remove-gconf

intrigeri wrote:

Code review:

  • dpkg-deb --extract would nicely replace ar x + tar (let's not rely on .deb current implementation details)

Thanks!

  • I'm surprised we need <allow_inactive>auth_admin</allow_inactive> in the Root Terminal and Tor Launcher polkit policies, but admitedly my polkit-fu is rusty.

Those were totally copy-paste mistakes. :S Thanks for noticing!

  • Wrt. 38cb09cc8c3a8c7eaaa61a20ef91de74768013b6, I'm surprised we still need gnome-open in the path: it's not installed by default on Stretch anymore. We re-added it in Tails 2.0.1 to fix "URL handling at least in KeePassX, Electrum and Icedove". But on a Debian Stretch GNOME live CD, Thunderbird, KeePassX and Electrum all open URLs just fine without gnome-open, so I'm wondering if we might be forward-porting cruft uselessly.

Ok, to me it seemed this area was still messy so I didn't want to have to hope I tested enough of the applications we ship if I dropped it, only to have Help Desk drown in these sort of reports yet again. Any way, I tested the applications you mention, and they work fine without a gnome-open, so let's try this!

  • Wrt. gconf settings, did you verify that everything we used to configure in there is still behaving as expected? I thought I had checked them all for obsolescence when porting to Stretch, but it might have been when porting to Jessie and I bet at least some of them are useless nowadays :)

Yes, I checked that nothing relevant is lost, or that we already are doing something we think is better via DConf. I should have included that in the commit message, sorry!

UX regression: in greeter.git:gdm/PostLogin.default we delete gksu.desktop when no admin password was set. This was not adjusted on this branch, so "Root Terminal" is listed even when it's not usable.

Good catch! I pushed a branch to the greeter repo! I didn't bother building a package so I suggest you do like me, and boot with rootpw=something and test by editing in the same (few) changes to /etc/gdm3/PostLogin/Default. Ok? As long as you merge this into the greeter's master branch all will be fine once I build a new package tomorrow.

UX and security regression: previously, when starting Root Terminal, the authentication prompt would make it explicit (although not very user-friendly) what exactly the user was being asked to authenticate for [...]

I agree, although I think the security benefits are marginal (replicating the PolicyKit password prompt with arbitrary text isn't exactly hard).

Thankfully it looks like intltool supports this file format, e.g. https://sources.debian.net/src/synaptic/0.84.2/po/synaptic.pot/#L3447 and https://sources.debian.net/src/synaptic/0.84.2/data/com.ubuntu.pkexec.synaptic.policy.in/ :)

I implemented translations. Seems to work!

After looking at grep --extended-regexp '<message>' /usr/share/polkit-1/actions/org.gnome.*, I suggest using "To start a Root Terminal, you need to authenticate." as the message: most other prompts use passive voice, which is recommended against by all the tech writing guides I've read, so I've picked the message from org.gnome.controlcenter.datetime.policy, which uses the active voice.

Ack, that wording sounds good and I went with it!

In passing, here's another issue I've noticed is a strange behavior when starting Root Terminal for the 2nd time from the Overview: sometimes I'm asked to authenticate again and a second window appears, sometimes the active window indicator (top bar, right of "Places") says "Root Terminal" with a waiting cursor for a few seconds and then disappears without anything else happening. I don't think it's worth spending any of your time on this, but I thought I would mention it in case it rings a bell or makes you think something seriously wrong is happening.

This is just the way the GNOME Applications Overview works: if you type a string that matches an open application, the default will be to select the window of running instances. Possibly there's a race between finding a match and noticing an instance is already running, so sometimes you get a new instance, sometimes an existing one, depending on your timing.

(Yup, I was also hit by this, and wondered what was going on. :))

And finally, all Root Terminal windows are labeled "Terminal", which I find really bad for UX, but that's the case on Tails 3.1 too so well.. I say let's put energy towards removing the need for the Root Terminal instead of making it better.

Yeah, I did spend a few minutes trying to fix this a week ago. I cannot recall what I tried, but I quickly gave up.

#39 Updated by anonym about 2 years ago

  • Assignee changed from intrigeri to anonym

Wait, I need to fix a few things...

#40 Updated by anonym about 2 years ago

  • Assignee changed from anonym to intrigeri

Done! Force-pushed!

#41 Updated by intrigeri about 2 years ago

Good catch! I pushed a branch to the greeter repo! I didn't bother building a package so I suggest you do like me, and boot with rootpw=something and test by editing in the same (few) changes to /etc/gdm3/PostLogin/Default. Ok? As long as you merge this into the greeter's master branch all will be fine once I build a new package tomorrow.

Fine!

UX and security regression: previously, when starting Root Terminal, the authentication prompt would make it explicit (although not very user-friendly) what exactly the user was being asked to authenticate for [...]

I agree, although I think the security benefits are marginal (replicating the PolicyKit password prompt with arbitrary text isn't exactly hard).

I'm curious how you do that: at least on GNOME, the polkit agent is GNOME Shell, and it has super powers that no other window has (or is it only on Wayland?) such as displaying an overlay on top of the entire desktop (top bar included) which provides a visual hint that means "you can trust this prompt". What did I miss?

#42 Updated by intrigeri about 2 years ago

Code review passes, will now test.

#43 Updated by anonym about 2 years ago

intrigeri wrote:

UX and security regression: previously, when starting Root Terminal, the authentication prompt would make it explicit (although not very user-friendly) what exactly the user was being asked to authenticate for [...]

I agree, although I think the security benefits are marginal (replicating the PolicyKit password prompt with arbitrary text isn't exactly hard).

I'm curious how you do that: at least on GNOME, the polkit agent is GNOME Shell, and it has super powers that no other window has (or is it only on Wayland?) such as displaying an overlay on top of the entire desktop (top bar included) which provides a visual hint that means "you can trust this prompt". What did I miss?

I think we only differ in our expectation on how users would behave with something that looks exactly like a PolKit auth prompt but that doesn't have those "super powers" -- I don't think many users even would notice that they cannot Alt+Tab away from it over whatever. The way things are in Tails, the display cannot be trusted during a compromise, IMHO.

#44 Updated by intrigeri about 2 years ago

  • Assignee changed from intrigeri to anonym
  • % Done changed from 60 to 80

Pushed fixes to your fixes.

#45 Updated by anonym about 2 years ago

  • Status changed from In Progress to Fix committed
  • Assignee deleted (anonym)
  • % Done changed from 80 to 100

Thanks for fixing my failed translation attempt! It would have taken me ages to realize your solution! ♥

PS. I also did the merge in the greeter repo.

#46 Updated by intrigeri about 2 years ago

  • Status changed from Fix committed to In Progress

#47 Updated by intrigeri about 2 years ago

  • Assignee set to anonym
  • % Done changed from 100 to 90

I see the merge in greeter.git but not in tails.git.

#48 Updated by intrigeri about 2 years ago

intrigeri wrote:

I see the merge in greeter.git but not in tails.git.

… which might actually be good news as the corresponding automated tests fail on Jenkins. Perhaps they need to be updated?

#49 Updated by lamby about 2 years ago

  • Subject changed from %gconf-tree-*.xml not reproducible in some environments to Remove gconf

(editing title as it kept confusing me when seeing activity!)

#50 Updated by anonym about 2 years ago

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

intrigeri wrote:

intrigeri wrote:

I see the merge in greeter.git but not in tails.git.

… which might actually be good news as the corresponding automated tests fail on Jenkins. Perhaps they need to be updated?

I tried to add in some dogtailification, which apparently is more robust on my system compared to Jenkins. I reverted those parts (follow-up: #14656) => merged!

#51 Updated by anonym about 2 years ago

  • Status changed from Fix committed to In Progress

#52 Updated by anonym almost 2 years ago

  • Status changed from In Progress to Fix committed

#53 Updated by anonym almost 2 years ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF