-
Notifications
You must be signed in to change notification settings - Fork 688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix .desktop icons for tails 3.3 #2620
Conversation
8e1a5f6
to
7c457c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works like a charm! 🎉 Couple of minor comments inline for maintainability.
# Set journalist.desktop and source.desktop links as trusted with Nautilus (see | ||
# https://github.com/freedomofpress/securedrop/issues/2586) | ||
# set euid and env variables to amnesia user | ||
os.setresgid(1000, 1000, -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to avoid magic numbers, can we do something like:
import pwd, grp
amnesia_gid = grp.getgrnam("amnesia").gr_gid
os.setresgid(amnesia_gid, amnesia_gid, -1)
amensia_uid = pwd.getpwnam("amnesia").pw_uid
os.setresuid(amensia_uid, amensia_uid, -1)
|
||
# remove existing shortcut, recreate symlink and change metadata attribute to trust .desktop | ||
for shortcut in ['source.desktop', 'journalist.desktop']: | ||
subprocess.call(['rm', str(path_desktop + shortcut)], env=env) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the str()
necessary here? Seems like path_desktop
and shortcut
are both strings and concatenating them will also produce a string
(This comment applies to the next two lines)
os.setresgid(1000, 1000, -1) | ||
os.setresuid(1000, 1000, -1) | ||
env = os.environ.copy() | ||
env['XDG_RUNTIME_DIR'] = '/run/user/1000' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then we can use .format
to demonstrate where the 1000
is coming from:
'/run/user/{}'.format(blah)
env['XDG_DATA_DIR'] = '/usr/share/gnome:/usr/local/share/:/usr/share/' | ||
env['HOME'] = '/home/amnesia' | ||
env['LOGNAME'] = 'amnesia' | ||
env['DBUS_SESSION_BUS_ADDRESS'] = 'unix:path=/run/user/1000/bus' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
7c457c7
to
23f902a
Compare
… journalist interface .desktop shortcuts, `source.desktop` and `journalist.desktop`.
23f902a
to
3db1dca
Compare
Thanks for the comments, @redshiftzero. They should now be addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look great - merging for inclusion in SecureDrop 0.5.
Restarting.. |
Worked for me, thanks @emkll ! |
Gitter chat just make me think about this: We will have to instruct admins to checkout/verify 0.5 and run |
CI is passing. We've noted that we'll need to include instructions for rerun tails scripts during the release announcement. Merging. |
Status
Ready for review
Description of Changes
Fixes #2586.
Changes proposed in this pull request:
source.desktop
andjournalist.desktop
are now trusted once tor finishes bootstrapping andsecuredrop_init.py
is called by Network manager.Workaround for issues with Nautilus per @kushaldas comments in #2586.
Testing
Needs to be tested on both new install and existing installs (upgrades)