Project

General

Profile

Bug #15732

Feature #14468: Add VeraCrypt support to Tails

Feature #14480: Fix bugs and UX issues of VeraCrypt support

Onion Circuits fails to start (permission denied error)

Added by segfault 11 months ago. Updated 10 months ago.

Status:
Resolved
Priority:
High
Assignee:
-
Category:
-
Target version:
Start date:
07/16/2018
Due date:
% Done:

100%

Feature Branch:
feature/14481-TCRYPT-support-beta
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Onion Circuits

Description

The new path we added to the Python search path, /usr/local/lib/python3/dist-packages/, is not allowed in the python/abstractions AppArmor profile, which causes onioncircuits to fail to start with a permission denied error.

We could fix this either by allowing access to this non-default path in our AppArmor profiles, or remove the non-default path and move the veracrypt_mounter package to a default path instead (for example /usr/lib/python3 or /usr/local/lib/python3.5).

Upstream merge request: https://gitlab.com/apparmor/apparmor/merge_requests/171


Related issues

Related to Tails - Bug #15370: Onion Circuits cannot be started in Tails 3.6~rc1 Resolved 03/03/2018

Associated revisions

Revision defbce46 (diff)
Added by segfault 10 months ago

Backport patch to adjust python abstraction. (Will-fix: #15732)

Revision de365b1b
Added by intrigeri 10 months ago

Merge branch 'feature/14481-TCRYPT-support-beta' into devel (Fix-committed: #15732)

History

#1 Updated by segfault 11 months ago

  • Assignee set to segfault
  • Target version set to Tails_3.9

#2 Updated by sajolida 11 months ago

  • Parent task set to #14480

I understand that this is a regression introduced by our work on VeraCrypt → parenting to #14480.

#3 Updated by intrigeri 11 months ago

  • Related to Bug #15370: Onion Circuits cannot be started in Tails 3.6~rc1 added

#4 Updated by intrigeri 11 months ago

#15370 was supposed to result in something happening upstream about it. Not sure it happened.

#5 Updated by intrigeri 11 months ago

  • Subject changed from onioncircuits fails to start (permission denied error) to Onion Circuits fails to start (permission denied error)
  • Affected tool set to Onion Circuits

#6 Updated by segfault 11 months ago

intrigeri wrote:

#15370 was supposed to result in something happening upstream about it. Not sure it happened.

#15370 is about allowing access to .egg-info files, not about allowing access to /usr/local/lib/python3.

#7 Updated by segfault 11 months ago

  • Assignee changed from segfault to intrigeri
  • QA Check set to Info Needed

I would like to have a decision on this soon (quoting from description):

We could fix this either by allowing access to this non-default path in our AppArmor profiles, or remove the non-default path and move the veracrypt_mounter package to a default path instead (for example /usr/lib/python3 or /usr/local/lib/python3.5).

intrigeri: What do you think we should do?

#8 Updated by intrigeri 11 months ago

  • Assignee changed from intrigeri to segfault
  • QA Check deleted (Info Needed)

segfault wrote:

intrigeri: What do you think we should do?

First step is to look at what happened upstream and in Debian wrt. abstractions/python since the version that lacks what we need.

If our problem has been fixed there, then let's backport the fix via config/chroot_local-patches/. Else, let's contribute a fix upstream and backport it in the same way.

#9 Updated by intrigeri 11 months ago

  • Priority changed from Normal to Elevated

(That's a regression.)

#10 Updated by segfault 11 months ago

  • Assignee changed from segfault to intrigeri
  • QA Check set to Info Needed

intrigeri wrote:

segfault wrote:

intrigeri: What do you think we should do?

First step is to look at what happened upstream and in Debian wrt. abstractions/python since the version that lacks what we need.

If our problem has been fixed there, then let's backport the fix via config/chroot_local-patches/. Else, let's contribute a fix upstream and backport it in the same way.

Our problem was not solved there. And I don't know whether upstream wants to allow a non-default path in the AppArmor profile, but we could ask them. But I want to make clear that this is not a bug in upstream - we added a new path to the Python search path and AppArmor doesn't allow access to this path.

So, are you sure I should create a ticket about this upstream? Should this be in AppArmor itself or Debian?

#11 Updated by intrigeri 11 months ago

  • Assignee changed from intrigeri to segfault
  • QA Check changed from Info Needed to Dev Needed

So, are you sure I should create a ticket about this upstream?

I'm sure it's worth proposing it, yes.

Should this be in AppArmor itself or Debian?

AppArmor upstream.

#12 Updated by u 11 months ago

intrigeri wrote:

So, are you sure I should create a ticket about this upstream?

I'm sure it's worth proposing it, yes.

Should this be in AppArmor itself or Debian?

AppArmor upstream.

Just in case: https://wiki.debian.org/AppArmor/Contribute/Upstream?action=show&redirect=AppArmor%2FContributeUpstream#Quick_howto_contribute_to_upstream_AppArmor_profiles_using_Git

#13 Updated by segfault 11 months ago

  • Description updated (diff)
  • QA Check deleted (Dev Needed)

#14 Updated by intrigeri 10 months ago

  • Priority changed from Elevated to High

(Blocks merging feature/14481-TCRYPT-support-beta into devel.)

#15 Updated by segfault 10 months ago

  • Assignee changed from segfault to intrigeri
  • QA Check set to Ready for QA

I pushed a commit on the #14481 feature branch.

#16 Updated by intrigeri 10 months ago

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

I pushed a commit on the #14481 feature branch.

I can't see that, please check and tell me the commit ID.

#17 Updated by segfault 10 months ago

Forgot to push, done it now. The commit is defbce46a2061b93d7c3318289aa8c49bf96a609.

#18 Updated by segfault 10 months ago

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

#19 Updated by intrigeri 10 months ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 60

Code review passes, will now test.

#20 Updated by intrigeri 10 months ago

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

Code review passes, will now test.

This does not work for me. Onion Circuits still fails to start and I see:

AVC apparmor="DENIED" operation="open" profile="/usr/bin/onioncircuits" name="/usr/local/lib/python3/dist-packages/" pid=7962 comm="onioncircuits" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0

Looks like we need at least a /usr/local/lib/python3/dist-packages r, rule (probably folded into an existing one). I strongly advise you to test that your proposed fix does solve the problem this ticket is about before sending a new MR upstream :)

#21 Updated by segfault 10 months ago

intrigeri wrote:

Code review passes, will now test.

This does not work for me. Onion Circuits still fails to start and I see:

[...]

Indeed.

Looks like we need at least a /usr/local/lib/python3/dist-packages r, rule (probably folded into an existing one). I strongly advise you to test that your proposed fix does solve the problem this ticket is about before sending a new MR upstream :)

I now spent quite some time on this and I don't have the slightest idea why reading /usr/lib/python3/dist-packages is allowed but /usr/local/lib/python3/dist-packages is not. In my patch, I simply applied the only rule that was targeting /usr/lib/python3 to /usr/local/lib/python3 as well. There is no other rule in abstractions/python or anywhere else in /etc/apparmor.d which targets /usr/lib/python3.

The rule that I added was in fact not needed for onioncircuits at all, because there are currently no *.so files in /usr/local/lib/python3/dist-packages/.

I will now change the patch to add a /usr/local/lib/python3/dist-packages r rule instead, but before I upstream this, I would like to understand why such a rule is not required to access /usr/lib/python3.

#22 Updated by intrigeri 10 months ago

I now spent quite some time on this and I don't have the slightest idea why reading /usr/lib/python3/dist-packages is allowed

At least on sid /etc/apparmor.d/abstractions/base has this rule:

/{usr/,}lib{,32,64}/**                r,

#23 Updated by segfault 10 months ago

OK, that explains it, thanks!

#24 Updated by segfault 10 months ago

  • Description updated (diff)
  • Assignee changed from segfault to intrigeri
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch set to feature/14481-TCRYPT-support-beta

Fixed by 8e0dc99fa48261069a79ff7bf2337fd179d3c5e7 and created a new merge request upstream (https://gitlab.com/apparmor/apparmor/merge_requests/171).

#25 Updated by intrigeri 10 months ago

Code review passes, will build an ISO and if happy, I'll close this ticket and merge the branch into devel \o/

#26 Updated by intrigeri 10 months ago

  • Status changed from In Progress to Resolved
  • Assignee deleted (intrigeri)
  • % Done changed from 60 to 100
  • QA Check changed from Ready for QA to Pass

I confirm it's fixed on the topic branch.

Also available in: Atom PDF