Tails

#1 Updated by anonym 2017-01-15 09:37:03

• 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 2017-01-15 10:19:50

• 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 2017-01-15 10:55:13

• Status changed from Confirmed to In Progress

Applied in changeset commit:b3a827d8e3c3fee78ec18450dfaf38a3d4eaf270.

#4 Updated by anonym 2017-01-15 11:00:31

• 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 2017-01-16 08:08:05

• Assignee set to intrigeri
• Affected tool set to OnionShare

#6 Updated by intrigeri 2017-01-16 10:26:52

• 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 Bug #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 2017-01-16 10:52:59

• 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 2017-01-16 11:55:59

• % Done changed from 50 to 60

Code review passed, testing.

#9 Updated by intrigeri 2017-01-16 13:30:43

• Status changed from In Progress to Fix committed
• % Done changed from 60 to 100

Applied in changeset commit:7bcede91ac295e1e488d0221cfc78feaa55c3e06.

#10 Updated by intrigeri 2017-01-16 13:31:28

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

#12 Updated by Anonymous 2017-01-17 15:26:24

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

#13 Updated by anonym 2017-01-24 20:43:14

• Status changed from Fix committed to Resolved