Project

General

Profile

Bug #12143

AppArmor blocks OnionShare from accessing folders below /home/amnesia

Added by spriver almost 3 years ago. Updated almost 3 years ago.

Status:
Resolved
Priority:
Elevated
Assignee:
-
Category:
-
Target version:
Start date:
01/14/2017
Due date:
% Done:

100%

Feature Branch:
bugfix/12143-onionshare-gui-apparmor-fix
Type of work:
Code
Blueprint:
Starter:
Affected tool:
OnionShare

Description

OnionShare is returing the error Permission denied when a file or folder is added via the Add File or Add Folder menu in OnionShare if a file is not directly in the home folder of the amnesia user (e.g. accessing the Persistent folder). So it's currently only possible to share files via this dialog menu if they are directly in the home folder.

Drag&Dropping files or folders from the file browser is fine, though.


Related issues

Related to Tails - Feature #11930: Review AppArmor profiles for OnionShare Resolved 11/16/2016

Associated revisions

Revision b3a827d8 (diff)
Added by anonym almost 3 years ago

Make onionshare-gui able to access folders beneath $HOME.

Without this change e.g. ~/Documents is inaccessible. To be honest,
this does not makes sense to me, as my interpretation of the old
patterns clearly should allow subfolders and files therein.

Will-fix: #12143

Revision 7bcede91
Added by intrigeri almost 3 years ago

Merge remote-tracking branch 'origin/bugfix/12143-onionshare-gui-apparmor-fix' into testing (Fix-committed: #12143)

History

#1 Updated by anonym almost 3 years ago

  • Status changed from New to Confirmed

Here's what happens when I try to navigate into the Documents folder using the "Add files" button (or if I try to add the Documents folder using the "Add folder" button):

audit: type=1400 audit(1484472160.352:41): apparmor="DENIED" 
operation="open" profile="/usr/bin/onionshare-gui" 
name="/home/amnesia/Documents/" pid=4038 comm="pool" requested_mask="r" 
denied_mask="r" fsuid=1000 ouid=1000

#2 Updated by anonym almost 3 years ago

  • Subject changed from OnionShare is not allowing any folders/files below /home/amnesia to AppArmor blocks OnionShare from accessing folders below /home/amnesia

#3 Updated by anonym almost 3 years ago

  • Status changed from Confirmed to In Progress

#4 Updated by anonym almost 3 years ago

  • Assignee deleted (anonym)
  • Priority changed from Normal to Elevated
  • % Done changed from 0 to 50
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch set to bugfix/12143-onionshare-gui-apparmor-fix

u, intrigeri: can you please look at the feature branch and explain to me why its change fixes the permission problems! I.e. can you explain why the old pattern is not good enough, but the new one is? To me @{HOME}/[^.]* should make ~/Documents accessible, and @{HOME}/[^.]*/** any files or subdirs of it. Or would we need @{HOME}/[^.]*/ (note the / at the end!) to match the ~/Documents folder itself?

Also, intrigeri: since that part was copied from your proposed changes to the Totem profile, perhaps you should have an extra look there? In Tails 2.10~rc1 I can actually list files in e.g. ~/.gnupg, but not open files or subdirs inside. It feels wrong to even be able to list files in there, right?

Any way, please review'n'merge into the testing branch! Also raising priority since this is pretty severe UX-wise, and not a great way to introduce a new feature to our users.

#5 Updated by intrigeri almost 3 years ago

  • Assignee set to intrigeri
  • Affected tool set to OnionShare

#6 Updated by intrigeri almost 3 years ago

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

I.e. can you explain why the old pattern is not good enough, but the new one is?

See the "Globbing" section in the corresponding doc (apparmor.d(5)).

tl;dr: * does not match /.

To me @{HOME}/[^.]* should make ~/Documents accessible,

I don't think so, because:

When AppArmor looks up a directory the pathname being looked up will end with
a slash (e.g., /var/tmp/); otherwise it will not end with a slash. Only rules
that match a trailing slash will match directories.

Or do we need @{HOME}/[^.]*/ (not the / at the end!)

Yes, we would need that if you hadn't added @{HOME}/[^.]**.

Also, intrigeri: since that part was copied from your proposed changes to the totem profile, perhaps you should have an extra look there? In Tails 2.10~rc1 I actually browser (= list files in) e.g. ~/.gnupg, but not open files or subdirs inside. It feels wrong to even be able to list files in there.

Sure, added to #9533.

Any way, please review'n'merge into the testing branch!

I think that @{HOME}/[^.]* is a subset of @{HOME}/[^.]**, so it should go away.

#7 Updated by anonym almost 3 years ago

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

intrigeri wrote:

I.e. can you explain why the old pattern is not good enough, but the new one is?

See the "Globbing" section in the corresponding doc (apparmor.d(5)).

Thanks! I looked in the AppArmor Core Policy Reference, which was much less clear. But re-reading it now, I get that this is the case. Thanks for the explanation!

Any way, please review'n'merge into the testing branch!

I think that @{HOME}/[^.]* is a subset of @{HOME}/[^.]**, so it should go away.

Indeed (confirmed with testing)! Fixed!

#8 Updated by intrigeri almost 3 years ago

  • % Done changed from 50 to 60

Code review passed, testing.

#9 Updated by intrigeri almost 3 years ago

  • Status changed from In Progress to 11
  • % Done changed from 60 to 100

#10 Updated by intrigeri almost 3 years ago

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Pass

#11 Updated by intrigeri almost 3 years ago

In passing, if the AppArmor profiles were already submitted upstream, they should be updated there (and this should be tracked elsewhere than on this ticket).

#12 Updated by u almost 3 years ago

  • Related to Feature #11930: Review AppArmor profiles for OnionShare added

#13 Updated by anonym almost 3 years ago

  • Status changed from 11 to Resolved

Also available in: Atom PDF