Bug#1050256: AppArmor breaks locking non-fs Unix sockets

2024-01-28 Thread John Johansen 

On 12/30/23 20:24, Mathias Gibbens wrote:

On Sat, 2023-12-30 at 16:44 +0100, Salvatore Bonaccorso wrote:

John, did you had a chance to work on this backport for 6.1.y stable
upstream so we could pick it downstream in Debian in one of the next
stable imports? Cherry-picking 1cf26c3d2c4c ("apparmor: fix apparmor
mediating locking non-fs unix sockets") does not work, if not
havinging the work around e2967ede2297 ("apparmor: compute policydb
permission on profile load") AFAICS, so that needs a 6.1.y specific
backport submitted to sta...@vger.kernel.org ?

I think we could have people from this bug as well providing a
Tested-by when necessary. I'm not feeling confident enough to be able
to provide myself such a patch to sent to stable (and you only giving
an Acked-by/Reviewed-by), so if you can help out here with your
upstream hat on that would be more than appreciated and welcome :)

Thanks a lot for your work!


   I played around with this a bit the past week as well, and came to
the same conclusion as Salvatore did that commits e2967ede2297 and
1cf26c3d2c4c need to be cherry-picked back to the 6.1 stable tree.

   I've attached the two commits rebased onto 6.1.y as patches to this
message. Commit e2967ede2297 needed a little bit of touchup to apply
cleanly, and 1cf26c3d2c4c just needed adjustments for line number
changes. I included some comments at the top of each patch.

   With these two commits cherry-picked on top of the 6.1.69 kernel, I
can boot a bookworm system and successfully start a service within a
container that utilizes `PrivateNetwork=yes`. Rebooting back into an
unpatched vanilla 6.1.69 kernel continues to show the problem.

   While I didn't see any immediate issues (ie, `aa-status` and log
files looked OK), I don't understand the changes in the first commit
well enough to be confident in sending these patches for inclusion in
the upstream stable tree on my own.

Mathias


Your backports look good to me, and you can stick my acked-by on them.
The changes are strictly more than necessary for the fix. They are
part of a larger change set that is trying to cleanup the runtime
code by changing the permission mapping from a runtime operation
to something that is done only at policy load/unpack time.

The advantage of this approach is that while it is a larger change
than strictly necessary. It is backporting patches that are already
upstream, keep the code closer and making backports easier.

Georgia did a minimal backport fix by keeping the version as part
of policy and doing the permission mapping at runtime. I have
included that patch below. Its advantage is it is a minimal
change to fix the issue.

I am happy with either version going into stable. Do you want to
send them or do you want me to do it?

Acked-by: John Johansen 
From d716d8e6e8b6dc2fa1db2e72ee95c721aef12064 Mon Sep 17 00:00:00 2001
From: Georgia Garcia 
Date: Tue, 9 Jan 2024 17:54:52 -0300
Subject: [PATCH] apparmor: backport fix apparmor mediating locking non-fs,
 unix sockets

This is a minimal backport of
1cf26c3d2c4c apparmor: fix apparmor mediating locking non-fs unix sockets

instead of pulling in the dependency patch
e2967ede2297 apparmor: compute policydb permission on profile load

which moves the permission mapping to unpack time. We push the version
information into the profile so the permission mapping fix can be
done in the run time aa_compute_perms() instead of the load time
equivalent in compute_perms_entry() introduced by e2967ede2297.

Signed-off-by: Georgia Garcia 
Acked-by: John Johansen 
---
 security/apparmor/apparmorfs.c |  3 ++-
 security/apparmor/include/perms.h  |  2 +-
 security/apparmor/include/policy.h | 12 
 security/apparmor/label.c  |  9 ++---
 security/apparmor/lib.c|  4 +++-
 security/apparmor/net.c|  3 ++-
 security/apparmor/policy_unpack.c  | 11 +--
 7 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 7160e7aa58b9..8131c9345900 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -633,7 +633,8 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
 		state = aa_dfa_match_len(dfa, profile->policy.start[0],
 	 match_str, match_len);
 		if (state)
-			aa_compute_perms(dfa, state, );
+			aa_compute_perms(dfa, state, ,
+	 profile->policy.version);
 	}
 	aa_apply_modes_to_perms(profile, );
 	aa_perms_accum_raw(perms, );
diff --git a/security/apparmor/include/perms.h b/security/apparmor/include/perms.h
index 13f20c598448..7d5c0c004978 100644
--- a/security/apparmor/include/perms.h
+++ b/security/apparmor/include/perms.h
@@ -142,7 +142,7 @@ void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs,
 void aa_apply_modes_to_perms(struct aa_profile *profile,
 			 struct aa_perms *perms);
 void aa_compute_perms(struct aa_dfa *dfa, unsigned int state

Bug#1050256: AppArmor breaks locking non-fs Unix sockets

2024-01-27 Thread John Johansen 

On 1/27/24 01:21, Salvatore Bonaccorso wrote:

Hi John,

On Sun, Dec 31, 2023 at 04:24:47AM +, Mathias Gibbens wrote:

On Sat, 2023-12-30 at 16:44 +0100, Salvatore Bonaccorso wrote:

John, did you had a chance to work on this backport for 6.1.y stable
upstream so we could pick it downstream in Debian in one of the next
stable imports? Cherry-picking 1cf26c3d2c4c ("apparmor: fix apparmor
mediating locking non-fs unix sockets") does not work, if not
havinging the work around e2967ede2297 ("apparmor: compute policydb
permission on profile load") AFAICS, so that needs a 6.1.y specific
backport submitted to sta...@vger.kernel.org ?

I think we could have people from this bug as well providing a
Tested-by when necessary. I'm not feeling confident enough to be able
to provide myself such a patch to sent to stable (and you only giving
an Acked-by/Reviewed-by), so if you can help out here with your
upstream hat on that would be more than appreciated and welcome :)

Thanks a lot for your work!


   I played around with this a bit the past week as well, and came to
the same conclusion as Salvatore did that commits e2967ede2297 and
1cf26c3d2c4c need to be cherry-picked back to the 6.1 stable tree.

   I've attached the two commits rebased onto 6.1.y as patches to this
message. Commit e2967ede2297 needed a little bit of touchup to apply
cleanly, and 1cf26c3d2c4c just needed adjustments for line number
changes. I included some comments at the top of each patch.

   With these two commits cherry-picked on top of the 6.1.69 kernel, I
can boot a bookworm system and successfully start a service within a
container that utilizes `PrivateNetwork=yes`. Rebooting back into an
unpatched vanilla 6.1.69 kernel continues to show the problem.

   While I didn't see any immediate issues (ie, `aa-status` and log
files looked OK), I don't understand the changes in the first commit
well enough to be confident in sending these patches for inclusion in
the upstream stable tree on my own.


Do you had a chance to look at this for 6.1.y upstream?

Asking/Poking since the point release dates are now clear:

https://lists.debian.org/debian-security/2024/01/msg5.html

if possible I would like to include those fixes, but only if they are
at least queued fror 6.1.y itself to not diverge from upstream.

Otherwise we will wait another round, but which means usually 2 months
for the point release cadence.


I am looking at it right now, I should be done with it today

Bug#963493: Repeatable hard lockup running strace testsuite on 4.19.98+ onwards

2020-06-26 Thread John Johansen 
On 6/26/20 9:50 AM, Steve McIntyre wrote:
> Hi Jann,
> 
> On Fri, Jun 26, 2020 at 04:25:59PM +0200, Jann Horn wrote:
>> On Fri, Jun 26, 2020 at 3:41 PM Greg KH  wrote:
>>> On Fri, Jun 26, 2020 at 12:35:58PM +0100, Steve McIntyre wrote:
> 
> ...
> 
 Considering I'm running strace build tests to provoke this bug,
 finding the failure in a commit talking about ptrace changes does look
 very suspicious...!

 Annoyingly, I can't reproduce this on my disparate other machines
 here, suggesting it's maybe(?) timing related.
>>
>> Does "hard lockup" mean that the HARDLOCKUP_DETECTOR infrastructure
>> prints a warning to dmesg? If so, can you share that warning?
> 
> I mean the machine locks hard - X stops updating, the mouse/keyboard
> stop responding. No pings, etc. When I reboot, there's nothing in the
> logs.
> 
>> If you don't have any way to see console output, and you don't have a
>> working serial console setup or such, you may want to try re-running
>> those tests while the kernel is booted with netconsole enabled to log
>> to a different machine over UDP (see
>> https://www.kernel.org/doc/Documentation/networking/netconsole.txt).
> 
> ACK, will try that now for you.
> 
>> You may want to try setting the sysctl kernel.sysrq=1 , then when the
>> system has locked up, press ALT+PRINT+L (to generate stack traces for
>> all active CPUs from NMI context), and maybe also ALT+PRINT+T and
>> ALT+PRINT+W (to collect more information about active tasks).
> 
> Nod.
> 
>> (If you share stack traces from these things with us, it would be
>> helpful if you could run them through scripts/decode_stacktrace.pl
>>from the kernel tree first, to add line number information.)
> 
> ACK.
> 
>> Trying to isolate the problem:
>>
>> __end_current_label_crit_section and end_current_label_crit_section
>> are aliases of each other (via #define), so that line change can't
>> have done anything.
>>
>> That leaves two possibilities AFAICS:
>> - the might_sleep() call by itself is causing issues for one of the
>> remaining users of begin_current_label_crit_section() (because it
>> causes preemption to happen more often where it didn't happen on
>> PREEMPT_VOLUNTARY before, or because it's trying to print a warning
>> message with the runqueue lock held, or something like that)
>> - the lack of "if (aa_replace_current_label(label) == 0)
>> aa_put_label(label);" in __begin_current_label_crit_section() is
>> somehow causing issues
>>
>> You could try to see whether just adding the might_sleep(), or just
>> replacing the begin_current_label_crit_section() call with
>> __begin_current_label_crit_section(), triggers the bug.
>>
>>
>> If you could recompile the kernel with CONFIG_DEBUG_ATOMIC_SLEEP - if
>> that isn't already set in your kernel config -, that might help track
>> down the problem, unless it magically makes the problem stop
>> triggering (which I guess would be conceivable if this indeed is a
>> race).
> 
> OK, will try that second...
> 

I have not been able to reproduce but

So looking at linux-4.19.y it looks like
1f8266ff5884 apparmor: don't try to replace stale label in ptrace access check

was picked without
ca3fde5214e1 apparmor: don't try to replace stale label in ptraceme check

Both of them are marked as
Fixes: b2d09ae449ced ("apparmor: move ptrace checks to using labels")

so I would expect them to be picked together.


ptraceme is potentially updating the task's cred while the access check is
running.

Try building after picking
ca3fde5214e1 apparmor: don't try to replace stale label in ptraceme check

Bug#872726: linux: apparmor doesn't use proper audit event ids

2017-09-09 Thread John Johansen 
On 09/09/2017 10:25 AM, intrigeri wrote:
> Hi Laurent & everyone else who's listening!
> 
> Laurent Bigonville:
>> Le 03/09/17 à 13:01, intrigeri a écrit :
>>> Laurent Bigonville:
 IMVHO, in regard to the recent proposal of enabling apparmor in debian
 by default, this needs to be addressed first.
>>> I'm genuinely curious why this should be a blocker for Debian: this is
>>> not obvious to me as a number of distros could enable AppArmor by
>>> default and can apparently live with this bug.
>>>
>>> Can you please make it explicit, e.g. describing what exact use cases
>>> would be harmed by enabling AppArmor by default without fixing this
>>> bug first?
> 
>> I think that having the denials of a MAC properly logged is important for 
>> both people
>> developing their policy and also for intrusion/non conformity detection.
> 
define properly logged, apparmor uses the common LSM audit subsystem
the same as smack, and selinux.

>> If someone wants to send their logs to some logging services 
>> (ELK/splunk/...) having
>> the messages properly logged/categorized seems to be the start here.
> 
> I see, thanks for the explanation! I agree it's important and I'm sad
> this bug makes AppArmor look a bit rough around the edges (because
> that doesn't correctly reflect my experience as a user, sysadmin and
> distro maintainer).
> 
> I'll look closer at each of these use cases from the "what would we
> *break* if we enabled AppArmor by default" perspective:
> 
>  * people developing their own AppArmor policy: 100% of the existing
>AppArmor policy was developed without proper audit event IDs;
>I'll be happy to give ausearch(8) a try once AppArmor does the
>right thing, but so far I can very well live without it.
> 
>⇒ from a user-centric perspective, I don't see why this bug would
>be a blocker as far as this use case is concerned.
> 
It complicates things because while all LSMs using the common LSM audit
frame work share 1400, the userspace end of audit only has fields
for selinux defined for 1400.

That is there is a disconnect between userspace and kernel. Userspace
has argued the kernel should change.

>  * intrusion detection: the current state of things in Debian (no MAC
>framework enabled by default) does not give any such capability to
>system administrators; the proposal of enabling AppArmor by default
>does not pretend it'll magically give this bonus feature.
> 
>⇒ AppArmor improves other things, just not this one, or at least
>not in an ideal way; too bad, but I don't see why this limitation
>would be a blocker.
> 
>  * centralized logging: enabling AppArmor by default will generate
>audit events; if nothing improves on this front, they'll be
>erroneously tagged type=1400 and thus might land into a SELinux
>bucket or similar by mistake, which can be confusing for sysadmins
>who also run SELinux-enabled systems, and even more so once one can
>stack SELinux and AppArmor.
> 
It depends on what you mean by erroneously tagged. AppArmor used to have
the audit range 1500-1599. When the LSM audit infrastructure was created
all LSMs using itwere moved to have id 1400. This was a deliberate choice
made by the kernel community, alls LSMs that use this are affected and
apparmor has had to live with it.

>From an audit tagging perspective 1500 was much better, I would like
to see apparmor move back to 1500 but apparmor will continue to
use the common LSM audit infrastructure, so the change needs to
be agreed to by more than just the apparmor devs.

>⇒ I understand there *is* definitely potential for painful impact
>here. We'll need to keep an eye on this when evaluating "AppArmor
>by default in Buster?" 1 year after it's been enabled by default
>(as per my proposal on -devel@). But I bet this bug would have been
>fixed a while ago if it was a serious problem in practice:
>apparently, tons of Ubuntu and SUSE sysadmins have apparently
>managed to cope with this bug just fine for many years.
> 
> So I see very little ground for arguing this bug is a blocker for
> enabling AppArmor by default in testing/sid, and reconsidering a year
> later — after all, it's one of the purposes of the evaluation
> exercises: nobody claims AppArmor is perfect, and what's at stake is
> rather whether it brings more value than costs for Debian and its
> users. The best, the good, enemies, etc. :)
> 
> If I misunderstood something, sorry! I'm all ears.
> 
> Cheers,
>

Re: [CVE-2014-9090] x86_64, traps: Stop using IST for #SS

2014-12-08 Thread John Johansen 
On 12/05/2014 05:47 AM, Luis Henriques wrote:
 Following this email I am sending for review the CVE-2014-9090 fix
 backports for both Lucid (2.6.32) and Precise (3.2.0).
 
 I'm also CC'ing Debian mailing-lists, Moritz, Ben and Willy as these
 backports could be of interest both to Debian and to the 2.6.32 and
 3.2 stable kernels.
 
 Andy Lutomirski (1):
   x86_64, traps: Stop using IST for #SS
 
  arch/x86/include/asm/page_32_types.h |  1 -
  arch/x86/include/asm/page_64_types.h | 11 +--
  arch/x86/kernel/dumpstack_64.c   |  1 -
  arch/x86/kernel/entry_64.S   |  2 +-
  arch/x86/kernel/traps.c  | 13 +
  5 files changed, 7 insertions(+), 21 deletions(-)
 
 
So both the Lucid and Precise patches look good to me. Like Stefan I
noticed the missing define but it does not appear to be used by the
patch.

I have not worked my way through the list of patches that Willy
provided so I can't yet comment on which of any of them should
be included. But I think the additional two patches that Ben
pointed out

af726f21ed8a x86_64, traps: Fix the espfix64 #DF fixup and rewrite it in C
b645af2d5905 x86_64, traps: Rework bad_iret

really should go with this

Acked-by: John Johansen john.johan...@canonical.com


-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/5485a158.2050...@canonical.com

Bug#676515: linux-2.6: AppArmor totally broken

2012-06-26 Thread John Johansen 
On 06/23/2012 11:53 AM, intrigeri wrote:
 Hi John,
 
 John Johansen wrote (17 Jun 2012 19:08:20 GMT) :
 On 06/15/2012 05:08 PM, Ben Hutchings wrote:

 If we don't want to restrict sockets used by the kernel, don't we need
 to store the kern flag for later use by aa_revalidate_sk()?

 For how apparmor is generally deployed it can get away with this, the
 kernel bits generally bail out earlier on the check for unconfined.

 That is not to say it isn't a good idea, or that it shouldn't be done.
 The fact is this patch is going to be replaced with completely rewritten
 controls, that do store info on the socket, it just hasn't happened yet
 due to resources and priorities (not my priorities).

 Ben, is this a blocker?

 I want to be convinced that this is not a bug, or else get a fix for it.

 I am looking at the kernel bits here, but I don't have a patch yet
 
 Do you think you'll manage to do it in time for the Wheezy freeze
 (June 30th)?
 
 Since denied has already been masked with ~quiet_mask, this condition
 can never be true.

 indeed

 Ben, is this a blocker?
 [...]

 This clearly is a bug and I want to be convinced that it is harmless or
 else get a fix for it.

 Right this breaks the controls over quieting of denial messages. Basically
 if policy specifies a reject should not be logged then the global controls
 that turn quieting off so that all rejects get logged aren't working for
 networking.
 
 This is an easy patch that I can provide separately or with the
 patch I am working on for the larger issue.
 
 Do you think you'll manage to prepare at least the easy fix it in time
 for the Wheezy freeze?
 

Okay, there are 4 kernel patches, not all of them are needed depending on 
whether
the network patch is applied or not.

If you don't want to apply the networking patch
  0001-apparmor-remove-advertising-the-support-of-network-r.patch

  Stops the kernel interface from incorrectly advertising that it supports 
network
  rules. A further patch (not attached) to userspace will also have to be 
applied

If the networking patch is applied
  these two patches can be applied or ignored, 0001 will be folded into the 
compat
  interface patch upstream, and then 0002 will be folded into the networking 
patch
  0001-apparmor-remove-advertising-the-support-of-network-r.patch
  0002-apparmor-Advertise-network-mediation-from-the-compat.patch

  these two patches address the two bugs pointed out in the networking patch
  0003-apparmor-Fix-quieting-of-audit-messages-for-network-.patch
  0004-apparmor-Ensure-apparmor-does-not-mediate-kernel-bas.patch
From 873143ceca69a2e54e7face1be49ad6b5514525d Mon Sep 17 00:00:00 2001
From: John Johansen john.johan...@canonical.com
Date: Tue, 26 Jun 2012 02:12:10 -0700
Subject: [PATCH 1/4] apparmor: remove advertising the support of network
 rules from compat iface

The interface compatibility patch was advertising support of network rules,
however this is not true if the networking patch is not applied. Move
advertising of network rules into a third patch that can be applied if
both the compatibility and network patches are applied.

Signed-off-by: John Johansen john.johan...@canonical.com
---
 security/apparmor/apparmorfs-24.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/apparmor/apparmorfs-24.c b/security/apparmor/apparmorfs-24.c
index dc8c744..367c7ea 100644
--- a/security/apparmor/apparmorfs-24.c
+++ b/security/apparmor/apparmorfs-24.c
@@ -49,7 +49,7 @@ const struct file_operations aa_fs_matching_fops = {
 static ssize_t aa_features_read(struct file *file, char __user *buf,
 size_t size, loff_t *ppos)
 {
-	const char features[] = file=3.1 capability=2.0 network=1.0 
+	const char features[] = file=3.1 capability=2.0 
 	change_hat=1.5 change_profile=1.1  aanamespaces=1.1 rlimit=1.1;
 
 	return simple_read_from_buffer(buf, size, ppos, features,
-- 
1.7.9.5

From 89a577e1bd55ee589c66bb82babb1dbbb8d73dad Mon Sep 17 00:00:00 2001
From: John Johansen john.johan...@canonical.com
Date: Tue, 26 Jun 2012 02:15:36 -0700
Subject: [PATCH 2/4] apparmor: Advertise network mediation from the
 compatibility interface

The userspace needs to know if the apparmor kernel module supports network
mediation. Apply this patch if both the v2.3 compatibility patch and
network mediation patches are applied.

Signed-off-by: John Johansen john.johan...@canonical.com
---
 security/apparmor/apparmorfs-24.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/apparmor/apparmorfs-24.c b/security/apparmor/apparmorfs-24.c
index 367c7ea..dc8c744 100644
--- a/security/apparmor/apparmorfs-24.c
+++ b/security/apparmor/apparmorfs-24.c
@@ -49,7 +49,7 @@ const struct file_operations aa_fs_matching_fops = {
 static ssize_t aa_features_read(struct file *file, char __user *buf,
 size_t size, loff_t *ppos)
 {
-	const char features[] = file=3.1 capability=2.0 
+	const char features[] = file=3.1 capability=2.0 network=1.0 
 	change_hat=1.5

Bug#676515: linux-2.6: AppArmor totally broken

2012-06-23 Thread John Johansen 
On 06/23/2012 11:53 AM, intrigeri wrote:
 Hi John,
 
 John Johansen wrote (17 Jun 2012 19:08:20 GMT) :
 On 06/15/2012 05:08 PM, Ben Hutchings wrote:

 If we don't want to restrict sockets used by the kernel, don't we need
 to store the kern flag for later use by aa_revalidate_sk()?

 For how apparmor is generally deployed it can get away with this, the
 kernel bits generally bail out earlier on the check for unconfined.

 That is not to say it isn't a good idea, or that it shouldn't be done.
 The fact is this patch is going to be replaced with completely rewritten
 controls, that do store info on the socket, it just hasn't happened yet
 due to resources and priorities (not my priorities).

 Ben, is this a blocker?

 I want to be convinced that this is not a bug, or else get a fix for it.

 I am looking at the kernel bits here, but I don't have a patch yet
 
 Do you think you'll manage to do it in time for the Wheezy freeze
 (June 30th)?
 
Yes for the don't mediate kernel sockets tagging, and the quiet masking.

The check if in interrupt context I am not comfortable removing yet and it
will have to wait for the full new networking patches.

 Since denied has already been masked with ~quiet_mask, this condition
 can never be true.

 indeed

 Ben, is this a blocker?
 [...]

 This clearly is a bug and I want to be convinced that it is harmless or
 else get a fix for it.

 Right this breaks the controls over quieting of denial messages. Basically
 if policy specifies a reject should not be logged then the global controls
 that turn quieting off so that all rejects get logged aren't working for
 networking.
 
 This is an easy patch that I can provide separately or with the
 patch I am working on for the larger issue.
 
 Do you think you'll manage to prepare at least the easy fix it in time
 for the Wheezy freeze?
 
Yes I should have something for you by the end of the weekend if not sooner



-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4fe62977.7070...@canonical.com

Bug#676515: linux-2.6: AppArmor totally broken

2012-06-23 Thread John Johansen 
On 06/23/2012 12:30 PM, intrigeri wrote:
 Hi,
 
 Ben Hutchings wrote (23 Jun 2012 19:02:06 GMT) :
 What is it that you think will happen at the freeze? We stop fixing
 all bugs and do nothing for the next few months?
 
 Of course, and we'll lazily eat lots of icecream while you work hard
 to release many shiny new Linux 3.2.x :)
 
 Irony set aside, I believe there are two different ways of fixing this
 bug:
 
   - Either by fixing the actual regression directly, without applying
 the network control patch: it seems clear to me that this can
 happen after the freeze. So far, so good.
 
   - Or by applying the AppArmor network control patch, in an improved
 version that John is apparently working on; this would be my
 preferred solution. But, given it would be a feature addition,
 rather than being merely a minimal bugfix, I'm not too sure it
 would be fit for a freeze exception. Hence my gentle pressuring on
 John to get this done before the freeze.
 
 Feel free to correct me if my understanding of the situation is wrong.
 
To be clear the network stuff I am working on for wheezy is 2 patches to
the current networking patch. The larger networking rewrite is not ready
yet and even if I could get it done in time I would not feel comfortable
pushing that in to wheezy as it would be at best early beta quality





-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4fe629f3.5010...@canonical.com

Bug#676515: linux-2.6: AppArmor totally broken

2012-06-17 Thread John Johansen 
On 06/15/2012 05:08 PM, Ben Hutchings wrote:
 On Fri, 2012-06-15 at 22:38 +0200, intrigeri wrote:
 Hi John, Ben and all other involved ones,

 I'd like to see this moving forward, since the Wheezy freeze is coming
 soon. See bellow explicit questions.
 
 Me too; thanks for the mail.
 
 John Johansen wrote (07 Jun 2012 16:45:36 GMT) :
 On 06/07/2012 07:34 AM, Ben Hutchings wrote:

 If we don't want to restrict sockets used by the kernel, don't we need
 to store the kern flag for later use by aa_revalidate_sk()?

 For how apparmor is generally deployed it can get away with this, the
 kernel bits generally bail out earlier on the check for unconfined.

 That is not to say it isn't a good idea, or that it shouldn't be done.
 The fact is this patch is going to be replaced with completely rewritten
 controls, that do store info on the socket, it just hasn't happened yet
 due to resources and priorities (not my priorities).

 Ben, is this a blocker?
 
 I want to be convinced that this is not a bug, or else get a fix for it.
 
I am looking at the kernel bits here, but I don't have a patch yet

 Since denied has already been masked with ~quiet_mask, this condition
 can never be true.

 indeed

 Ben, is this a blocker?
 [...]
 
 This clearly is a bug and I want to be convinced that it is harmless or
 else get a fix for it.
 
Right this breaks the controls over quieting of denial messages. Basically
if policy specifies a reject should not be logged then the global controls
that turn quieting off so that all rejects get logged aren't working for
networking.

This is an easy patch that I can provide separately or with the patch I
am working on for the larger issue.

I have also been looking into why the regression is happening, it actually
looks to be in the userspace caching of compiled policy. I can run the
same basic profile loads on ubuntu with a kernel that only has the single
interface patch applied and it works. So its just a matter of tracking
down which patches are needed now.




-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4fde2b24.5070...@canonical.com

Bug#676515: linux-2.6: AppArmor totally broken

2012-06-07 Thread John Johansen 
On 06/07/2012 07:34 AM, Ben Hutchings wrote:
 On Thu, 2012-06-07 at 15:35 +0200, intrig...@debian.org wrote:
 Package: linux-2.6
 Severity: normal
 Version: 3.2.19-1
 Tags: patch
 X-Debbugs-CC: john.johan...@canonical.com, k...@debian.org, mi...@riseup.net

 Hi,

 the AppArmor compatibility patch applied to fix #661151
 totally breaks AppArmor support; this is a regression.
 Details: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=661151#83

 Applying the AppArmor: compatibility patch for v5 network controll
 on top of the already applied one fixes the problem for me.
 I did test with the patch that can be found there:
 http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-precise.git;a=commit;h=d253e5fb4a6b552e7cd2a3c80934ab4f92faec97

 Please consider applying this network compatibility patch,
 not only to fix this regression,
 but also to get us working network mediation.
 
 So these independent patches really aren't... or else userland gets
 confused if we apply just the one.
 
Hrmmm, no they should be. That is a bug in userland that needs to be
fixed. I do know that they used to work when applied separately. It
certainly not a configuration that gets a lot of testing, but it
should have worked. I will look into it and figure out what is causing
it to break.

 Looking at the network controller patch:
 
 --- a/security/apparmor/lsm.c
 +++ b/security/apparmor/lsm.c
 [...]
 @@ -621,6 +622,104 @@ static int apparmor_task_setrlimit(struct task_struct 
 *task,
  return error;
  }
  
 +static int apparmor_socket_create(int family, int type, int protocol, int 
 kern)
 +{
 +struct aa_profile *profile;
 +int error = 0;
 +
 +if (kern)
 +return 0;
 
 If we don't want to restrict sockets used by the kernel, don't we need
 to store the kern flag for later use by aa_revalidate_sk()?
 
For how apparmor is generally deployed it can get away with this, the
kernel bits generally bail out earlier on the check for unconfined.

That is not to say it isn't a good idea, or that it shouldn't be done.
The fact is this patch is going to be replaced with completely rewritten
controls, that do store info on the socket, it just hasn't happened yet
due to resources and priorities (not my priorities).

 +profile = __aa_current_profile();
 +if (!unconfined(profile))
 +error = aa_net_perm(OP_CREATE, profile, family, type, protocol,
 +NULL);
 +return error;
 +}
 [...]
 --- /dev/null
 +++ b/security/apparmor/net.c
 [...]
 +static int audit_net(struct aa_profile *profile, int op, u16 family, int 
 type,
 + int protocol, struct sock *sk, int error)
 +{
 [...]
 +} else {
 +u16 quiet_mask = profile-net.quiet[sa.u.net.family];
 +u16 kill_mask = 0;
 +u16 denied = (1  sa.aad.net.type)  ~quiet_mask;
 +
 +if (denied  kill_mask)
 +audit_type = AUDIT_APPARMOR_KILL;
 +
 +if ((denied  quiet_mask) 
 
 Since denied has already been masked with ~quiet_mask, this condition
 can never be true.
 
indeed

 +AUDIT_MODE(profile) != AUDIT_NOQUIET 
 +AUDIT_MODE(profile) != AUDIT_ALL)
 +return COMPLAIN_MODE(profile) ? 0 : sa.aad.error;
 +}
 +
 +return aa_audit(audit_type, profile, GFP_KERNEL, sa, audit_cb);
 +}
 [...]
 +/**
 + * aa_revalidate_sk - Revalidate access to a sock
 + * @op: operation being checked
 + * @sk: sock being revalidated  (NOT NULL)
 + *
 + * Returns: %0 else error if permission denied
 + */
 +int aa_revalidate_sk(int op, struct sock *sk)
 +{
 +struct aa_profile *profile;
 +int error = 0;
 +
 +/* aa_revalidate_sk should not be called from interrupt context
 + * don't mediate these calls as they are not task related
 + */
 +if (in_interrupt())
 +return 0;
 
 I wonder why this is being checked at all.
 
Good question, I will have to dig into it.

 +profile = __aa_current_profile();
 +if (!unconfined(profile))
 +error = aa_net_perm(op, profile, sk-sk_family, sk-sk_type,
 +sk-sk_protocol, sk);
 +
 +return error;
 +}
 [...]
 
 Ben.
 




-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4fd0dab0.5070...@canonical.com

Bug#661151: [apparmor] Bug#661151: linux-2.6: lacks AppArmor kernel/userspace interface

2012-05-30 Thread John Johansen 
On 05/30/2012 08:08 AM, micah anderson wrote:
 
 Hi all,
 
 Its been 2 months without a reply on this issue, and we are getting
 close to a freeze. Kees and John it looks like there are some pending
 questions for you below, it would be great if you could chime in with
 your opinons:
 
 If the Debian kernel team was willing to carry some kind of AppArmor
 kernel/userspace interface patch, I'm now unsure if the old or new
 ones would be better suited. (I assume AppArmor 2.8 is released long
 enough before the Wheezy freeze, so that we can ship it in there, and
 are given this choice.)

 On the one hand, the old compat' patches are confidence inspiring, as
 they are small and have been shipped by Ubuntu for a while.
 
 My opinon: the 2.4 compat patch is tiny, and it works well, and has been
 tested for some time, I think it makes the most sense to include this
 one.
 
probably, especially if you are looking to keep the patch as small as
possible

 On the other hand, it seems the new patches are being upstreamed,
 which makes them more appealing somehow than the older ones.
 
 The newer patch is bigger, some of it must be backported from Linux 3.4,
 some from Ubuntu, it is much less tested and I suspect because of that
 will encounter much more resistance from Debian's kernel team to include
 it. Presumably this will eventually be the one that will be upstreamed,
 but it isn't there yet. This is why I think the 2.4 compat patch is the
 way to go with Wheezy, when the newer patch is upstreamed that can be
 swapped out then.
 
yeah to clarify, half of the new interface went upstream in 3.4 and I can
provide a version of that that is backported but its a few patches and
not as small as the compat patch.  In addition to that you would need
a compatibility patch on top of that, that provides the features the
current upstream interface doesn't

 John, I think it would help if you could please point us more
 precisely to the commits of the new interface that have been
 upstreamed already, and to the ones that have not been, so that we can
 get a rough idea of where things are at.

hrmmm, I think I missed answering this before

the upstream patches
9acd494be9387b0608612cd139967201dd7a4e12
e74abcf3359d0130e99a6511ac484a3ea9e6e988
a9bf8e9fd561ba9ff1f0f2a1d96e439fced4
d384b0a1a35f87f0ad70c29518f98f922b1c15cb

the additional patch to complete the interface
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor v3.4-aa2.8
8de755e4dfdbc40bfcaca848ae6b5aeaf0ede0e8

vs. the old compat patch
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
da1ce2265ebb70860b9c137a542e48b170e4606b

 Kees, others, what do you think?
 

While I like to see the latest stuff, I think the old patch is a smaller
delta, well tested and going to be less to maintain so it really seems
the way to go.




-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4fc6b14b.70...@canonical.com

Bug#661151: [apparmor] Bug#661151: linux-2.6: lacks AppArmor kernel/userspace interface

2012-05-30 Thread John Johansen 
On 05/30/2012 06:10 PM, Ben Hutchings wrote:
 On Wed, 2012-05-30 at 16:46 -0700, John Johansen wrote:
 On 05/30/2012 08:08 AM, micah anderson wrote:

 Hi all,

 Its been 2 months without a reply on this issue, and we are getting
 close to a freeze. Kees and John it looks like there are some pending
 questions for you below, it would be great if you could chime in with
 your opinons:

 If the Debian kernel team was willing to carry some kind of AppArmor
 kernel/userspace interface patch, I'm now unsure if the old or new
 ones would be better suited. (I assume AppArmor 2.8 is released long
 enough before the Wheezy freeze, so that we can ship it in there, and
 are given this choice.)

 On the one hand, the old compat' patches are confidence inspiring, as
 they are small and have been shipped by Ubuntu for a while.

 My opinon: the 2.4 compat patch is tiny, and it works well, and has been
 tested for some time, I think it makes the most sense to include this
 one.

 probably, especially if you are looking to keep the patch as small as
 possible
 
 Should I take it that '2.4 compat' actually means '2.4-2.7 compat'?

yeah it turned out to be that. The rewrite to use the newer LSM path hooks
began after 2.4, and the patch provided the 2.4 interface until a newer
interface that was more sysfs in style could be created.
 
 [...]
 vs. the old compat patch
 git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
 da1ce2265ebb70860b9c137a542e48b170e4606b

 Kees, others, what do you think?


 While I like to see the latest stuff, I think the old patch is a smaller
 delta, well tested and going to be less to maintain so it really seems
 the way to go.
 
 So you're saying we should take just the one quoted above for wheezy?
 
 The aafs_create() and aafs_remove() calls are mismatched when
 CONFIG_SECURITY_APPARMOR_COMPAT_24 is not set, but aside from that it
 doesn't look too horrible.

oops I guess we never built it that way, I can fix that for you

 What about this one:
 
 commit 1023c7c2f9d9c5707147479104312c4c3d1a2c2b
 Author: John Johansen john.johan...@canonical.com
 Date:   Wed Aug 10 22:02:39 2011 -0700
 
 AppArmor: compatibility patch for v5 network controll
 
 Add compatibility for v5 network rules.
 

That will provide support for the network rules and if you are willing
to carry it that would be greate but is not strictly necessary. Policy can
still be loaded and introspected. If that patch is missing and if profile
contains network rules the parser will complain about them not being
enforced, but it will still load and enforce the rest of the policy




-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4fc6dee6.9090...@canonical.com

Bug#661151: [apparmor] Bug#661151: linux-2.6: lacks AppArmor kernel/userspace interface

2012-03-14 Thread John Johansen 
On 03/14/2012 03:24 AM, intrigeri wrote:
 Hi,
 
 John Johansen wrote (13 Mar 2012 16:33:53 GMT) :
 sorry I missed this,
 
 Thank you, John, for your answers :)
 
 yes you can pull them out of the tarball,
 
 That would be 0002-AppArmor-compatibility-patch-for-v5-interface.patch
 that can be found in the kernel-patches/$LATEST/ directory of the
 apparmor Debian source package. Given $LATEST == 3.1 currently, see
 bellow for the Ubuntu patches that were maybe refreshed.
 
 John, do you confirm this patch does not depend on any of the
 two others?
 
It does not but there may be a small conflict or two to resolve if
0001-AppArmor-compatibility-patch-for-v5-network-controll.patch is not
applied first.

If it doesn't apply cleanly I will be happy to update it for you.

 (namely:
 0001-AppArmor-compatibility-patch-for-v5-network-controll.patch and
 0003-AppArmor-Allow-dfa-backward-compatibility-with-broke.patch)
 
 or from the ubuntu kernel tree.
 
 I guess that would be
 http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-precise.git;a=commit;h=56f928f0cbf810c047a9a72e4e5c4840800437ec
 
 John, please correct me if I did not guess right.
 
You are right

 There are also a new set of patches available against the 3.3
 kernel. The static parts of the interface have been updated and
 pushed into the 3.4 kernel. And the goal is to get the other part
 into the 3.5 kernel (still a wip).
 
 John: I guess the Linux 3.2 kernel shipped in Precise will carry those
 patches, and this is why the v5 compat' patches got recently reverted
 in Precise's kernel tree, right?
 
correct

 Though those will require a more recent userspace.
 
 John: that will be called 2.8, right?
 
correct. The 2.8 userspace release will ship with precise and will be
compatible with both the older and newer kernel interfaces.



-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4f607461.6020...@canonical.com

Bug#661151: [apparmor] Bug#661151: linux-2.6: lacks AppArmor kernel/userspace interface

2012-03-13 Thread John Johansen 
On 02/24/2012 08:40 AM, intrigeri wrote:
 Ben Hutchings wrote (24 Feb 2012 16:06:16 GMT) :
 Where can I find the patch?
 
 Kees and Ubuntu AppArmor developers: can you please confirm the
 patches that the Debian kernel team should consider for supporting the
 AppArmor legacy interface would be the ones found in the
 kernel-patches/$LATEST/ directory of the apparmor 2.7.x tarball?
 
 Or have you got updated patches, e.g. for Linux 3.2.x, published
 somewhere to be found?
 
sorry I missed this,

yes you can pull them out of the tarball, or from the ubuntu kernel
tree.

You will also be able to pull them from the git tree at
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor.git

though I still need to restore them as the tree got wiped after the
kernel.org breakin.

There are also a new set of patches available against the 3.3 kernel.
The static parts of the interface have been updated and pushed into
the 3.4 kernel.  And the goal is to get the other part into the 3.5
kernel (still a wip).  Though those will require a more recent userspace.

If someone will let me know what is desired I will set up a tree with
patches pre-applied.



-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4f5f76f1.6010...@canonical.com