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-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 4FDE2B24.5070907@canonical.com">http://lists.debian.org/4FDE2B24.5070907@canonical.com

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?

Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc



--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 85hau1dflt.fsf@boum.org">http://lists.debian.org/85hau1dflt.fsf@boum.org

On Sat, 2012-06-23 at 20:53 +0200, 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)?
[...]

What is it that you think will happen at the freeze? We stop fixing all
bugs and do nothing for the next few months?

Ben.

--
Ben Hutchings
The program is absolutely right; therefore, the computer must be wrong.

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 UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 85ipehbzd3.fsf@boum.org">http://lists.debian.org/85ipehbzd3.fsf@boum.org

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-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 4FE62977.7070608@canonical.com">http://lists.debian.org/4FE62977.7070608@canonical.com

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-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 4FE629F3.5010606@canonical.com">http://lists.debian.org/4FE629F3.5010606@canonical.com

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

Hi John,

On Tue, Jun 26, 2012 at 10:48:38AM -0700, John Johansen wrote:
> On 06/23/2012 11:53 AM, intrigeri wrote:
> > 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

My preference would be to apply the networking patch, along with 0003
and 0004 posted here.

-Kees

--
Kees Cook



--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20120626182726.GQ5042@outflux.net">http://lists.debian.org/20120626182726.GQ5042@outflux.net

On Tue, 2012-06-26 at 11:27 -0700, Kees Cook wrote:
> Hi John,
>
> On Tue, Jun 26, 2012 at 10:48:38AM -0700, John Johansen wrote:
[...]
> > 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
>
> My preference would be to apply the networking patch, along with 0003
> and 0004 posted here.

Patches 3 and 4 address my concerns about the basic sanity of the
networking interface, though I still have no idea whether it is actually
usable it to enforce a useful security policy.

What I think I failed to notice, though, is that AppArmor in mainline
does haven't implement any networking control. We were originally asked
to provide a compatibility interface only, not to add an out-of-tree
feature, and I'm very reluctant to do the latter, so I'm afraid it's
going to be patch 1 only.

I hope that my code review was at least useful to Ubuntu.

Ben.

--
Ben Hutchings
Lowery's Law:
If it jams, force it. If it breaks, it needed replacing anyway.