* Re: [PATCH net-next] netlabel: Fix some kernel-doc warnings
From: Paul Moore @ 2020-09-08 21:23 UTC (permalink / raw)
To: Wang Hai; +Cc: davem, kuba, netdev, linux-kernel, linux-security-module
In-Reply-To: <20200908140543.25514-1-wanghai38@huawei.com>
On Tue, Sep 8, 2020 at 10:09 AM Wang Hai <wanghai38@huawei.com> wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
> net/netlabel/netlabel_calipso.c:438: warning: Excess function parameter 'audit_secid' description in 'calipso_doi_remove'
> net/netlabel/netlabel_calipso.c:605: warning: Excess function parameter 'reg' description in 'calipso_req_delattr'
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wang Hai <wanghai38@huawei.com>
> ---
> net/netlabel/netlabel_calipso.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Looks good to me, thanks.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/netlabel/netlabel_calipso.c b/net/netlabel/netlabel_calipso.c
> index 249da67d50a2..1a98247ab148 100644
> --- a/net/netlabel/netlabel_calipso.c
> +++ b/net/netlabel/netlabel_calipso.c
> @@ -426,7 +426,7 @@ void calipso_doi_free(struct calipso_doi *doi_def)
> /**
> * calipso_doi_remove - Remove an existing DOI from the CALIPSO protocol engine
> * @doi: the DOI value
> - * @audit_secid: the LSM secid to use in the audit message
> + * @audit_info: NetLabel audit information
> *
> * Description:
> * Removes a DOI definition from the CALIPSO engine. The NetLabel routines will
> @@ -595,7 +595,7 @@ int calipso_req_setattr(struct request_sock *req,
>
> /**
> * calipso_req_delattr - Delete the CALIPSO option from a request socket
> - * @reg: the request socket
> + * @req: the request socket
> *
> * Description:
> * Removes the CALIPSO option from a request socket, if present.
> --
> 2.17.1
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH v20 05/23] net: Prepare UDS for security module stacking
From: Casey Schaufler @ 2020-09-08 23:37 UTC (permalink / raw)
To: Stephen Smalley, John Johansen
Cc: Paul Moore, Casey Schaufler, James Morris, LSM List, SElinux list,
linux-audit, Kees Cook, Tetsuo Handa, Stephen Smalley,
Casey Schaufler
In-Reply-To: <CAEjxPJ5KudgTjhmXBNdCO_ctvioy5UA5PXcoKX4zc19NYKgHZA@mail.gmail.com>
On 9/8/2020 6:35 AM, Stephen Smalley wrote:
> On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen
>> <john.johansen@canonical.com> wrote:
>>> On 9/5/20 11:13 AM, Casey Schaufler wrote:
>>>> On 9/5/2020 6:25 AM, Paul Moore wrote:
>>>>> On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote:
>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>> On 9/4/2020 1:08 PM, Paul Moore wrote:
>>>>> ...
>>>>>
>>>>>>> I understand the concerns you mention, they are all valid as far as
>>>>>>> I'm concerned, but I think we are going to get burned by this code as
>>>>>>> it currently stands.
>>>>>> Yes, I can see that. We're getting burned by the non-extensibility
>>>>>> of secids. It will take someone smarter than me to figure out how to
>>>>>> fit N secids into 32bits without danger of either failure or memory
>>>>>> allocation.
>>>>> Sooo what are the next steps here? It sounds like there is some
>>>>> agreement that the currently proposed unix_skb_params approach is a
>>>>> problem, but it also sounds like you just want to merge it anyway?
>>>> There are real problems with all the approaches. This is by far the
>>>> least invasive of the lot. If this is acceptable for now I will commit
>>>> to including the dynamic allocation version in the full stacking
>>>> (e.g. Smack + SELinux) stage. If it isn't, well, this stage is going
>>>> to take even longer than it already has. Sigh.
>>>>
>>>>
>>>>> I was sorta hoping for something a bit better.
>>>> I will be looking at alternatives. I am very much open to suggestions.
>>>> I'm not even 100% convinced that Stephen's objections to my separate
>>>> allocation strategy outweigh its advantages. If you have an opinion on
>>>> that, I'd love to hear it.
>>>>
>>> fwiw I prefer the separate allocation strategy, but as you have already
>>> said it trading off one set of problems for another. I would rather see
>>> this move forward and one set of trade offs isn't significantly worse
>>> than the other to me so, either wfm.
>> I remain unclear that AppArmor needs this patch at all even when
>> support for SO_PEERSEC lands.
>> Contrary to the patch description, it is about supporting SCM_SECURITY
>> for datagram not SO_PEERSEC. And I don't know of any actual users of
>> SCM_SECURITY even for SELinux, just SO_PEERSEC.
> I remembered that systemd once tried using SCM_SECURITY but that was a
> bug since systemd was using it with stream sockets and that wasn't
> supported by the kernel at the time,
> https://bugzilla.redhat.com/show_bug.cgi?id=1224211, so systemd
> switched over to using SO_PEERSEC. Subsequently I did fix
> SCM_SECURITY to work with stream sockets via kernel commit
> 37a9a8df8ce9de6ea73349c9ac8bdf6ba4ec4f70 but SO_PEERSEC is still
> preferred. Looking around, I see that there is still one usage of
> SCM_SECURITY in systemd-journald but it doesn't seem to be required
> (if provided, journald will pass the label along but nothing seems to
> depend on it AFAICT). In any event, I don't believe this patch is
> needed to support stacking AppArmor.
Stephen is, as is so often the case, correct. AppArmor has a stub
socket_getpeersec_dgram() that gets removed in patch 23. If I remove
it earlier and throw in a touch of scaffolding for secid_to_secctx()
we can leave the secid as is for now. This can't be the final solution
as AppArmor will be using the hook someday and we still have the all
modules case to worry about for the next phase. It also assumes that
The BPF module isn't going to suddenly sprout a security context.
^ permalink raw reply
* Re: [PATCH v20 05/23] net: Prepare UDS for security module stacking
From: John Johansen @ 2020-09-09 0:17 UTC (permalink / raw)
To: Stephen Smalley
Cc: Casey Schaufler, Paul Moore, Casey Schaufler, James Morris,
LSM List, SElinux list, linux-audit, Kees Cook, Tetsuo Handa,
Stephen Smalley
In-Reply-To: <CAEjxPJ5KudgTjhmXBNdCO_ctvioy5UA5PXcoKX4zc19NYKgHZA@mail.gmail.com>
On 9/8/20 6:35 AM, Stephen Smalley wrote:
> On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
>>
>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen
>> <john.johansen@canonical.com> wrote:
>>>
>>> On 9/5/20 11:13 AM, Casey Schaufler wrote:
>>>> On 9/5/2020 6:25 AM, Paul Moore wrote:
>>>>> On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote:
>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>> On 9/4/2020 1:08 PM, Paul Moore wrote:
>>>>> ...
>>>>>
>>>>>>> I understand the concerns you mention, they are all valid as far as
>>>>>>> I'm concerned, but I think we are going to get burned by this code as
>>>>>>> it currently stands.
>>>>>> Yes, I can see that. We're getting burned by the non-extensibility
>>>>>> of secids. It will take someone smarter than me to figure out how to
>>>>>> fit N secids into 32bits without danger of either failure or memory
>>>>>> allocation.
>>>>> Sooo what are the next steps here? It sounds like there is some
>>>>> agreement that the currently proposed unix_skb_params approach is a
>>>>> problem, but it also sounds like you just want to merge it anyway?
>>>>
>>>> There are real problems with all the approaches. This is by far the
>>>> least invasive of the lot. If this is acceptable for now I will commit
>>>> to including the dynamic allocation version in the full stacking
>>>> (e.g. Smack + SELinux) stage. If it isn't, well, this stage is going
>>>> to take even longer than it already has. Sigh.
>>>>
>>>>
>>>>> I was sorta hoping for something a bit better.
>>>>
>>>> I will be looking at alternatives. I am very much open to suggestions.
>>>> I'm not even 100% convinced that Stephen's objections to my separate
>>>> allocation strategy outweigh its advantages. If you have an opinion on
>>>> that, I'd love to hear it.
>>>>
>>>
>>> fwiw I prefer the separate allocation strategy, but as you have already
>>> said it trading off one set of problems for another. I would rather see
>>> this move forward and one set of trade offs isn't significantly worse
>>> than the other to me so, either wfm.
>>
>> I remain unclear that AppArmor needs this patch at all even when
>> support for SO_PEERSEC lands.
>> Contrary to the patch description, it is about supporting SCM_SECURITY
>> for datagram not SO_PEERSEC. And I don't know of any actual users of
>> SCM_SECURITY even for SELinux, just SO_PEERSEC.
>
> I remembered that systemd once tried using SCM_SECURITY but that was a
> bug since systemd was using it with stream sockets and that wasn't
> supported by the kernel at the time,
> https://bugzilla.redhat.com/show_bug.cgi?id=1224211, so systemd
> switched over to using SO_PEERSEC. Subsequently I did fix
> SCM_SECURITY to work with stream sockets via kernel commit
> 37a9a8df8ce9de6ea73349c9ac8bdf6ba4ec4f70 but SO_PEERSEC is still
> preferred. Looking around, I see that there is still one usage of
> SCM_SECURITY in systemd-journald but it doesn't seem to be required
> (if provided, journald will pass the label along but nothing seems to
> depend on it AFAICT). In any event, I don't believe this patch is
> needed to support stacking AppArmor.
>
correct it is not currently needed. I have been playing with code to
handle it but it is not upstream yet.
Regardless this is something that will need to be solved at some
point.
^ permalink raw reply
* Re: [PATCH v20 05/23] net: Prepare UDS for security module stacking
From: John Johansen @ 2020-09-09 0:21 UTC (permalink / raw)
To: Casey Schaufler, Stephen Smalley
Cc: Paul Moore, Casey Schaufler, James Morris, LSM List, SElinux list,
linux-audit, Kees Cook, Tetsuo Handa, Stephen Smalley
In-Reply-To: <c5bef71e-6d78-2058-bcaa-8497c76d7375@schaufler-ca.com>
On 9/8/20 4:37 PM, Casey Schaufler wrote:
> On 9/8/2020 6:35 AM, Stephen Smalley wrote:
>> On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
>> <stephen.smalley.work@gmail.com> wrote:
>>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen
>>> <john.johansen@canonical.com> wrote:
>>>> On 9/5/20 11:13 AM, Casey Schaufler wrote:
>>>>> On 9/5/2020 6:25 AM, Paul Moore wrote:
>>>>>> On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote:
>>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>>> On 9/4/2020 1:08 PM, Paul Moore wrote:
>>>>>> ...
>>>>>>
>>>>>>>> I understand the concerns you mention, they are all valid as far as
>>>>>>>> I'm concerned, but I think we are going to get burned by this code as
>>>>>>>> it currently stands.
>>>>>>> Yes, I can see that. We're getting burned by the non-extensibility
>>>>>>> of secids. It will take someone smarter than me to figure out how to
>>>>>>> fit N secids into 32bits without danger of either failure or memory
>>>>>>> allocation.
>>>>>> Sooo what are the next steps here? It sounds like there is some
>>>>>> agreement that the currently proposed unix_skb_params approach is a
>>>>>> problem, but it also sounds like you just want to merge it anyway?
>>>>> There are real problems with all the approaches. This is by far the
>>>>> least invasive of the lot. If this is acceptable for now I will commit
>>>>> to including the dynamic allocation version in the full stacking
>>>>> (e.g. Smack + SELinux) stage. If it isn't, well, this stage is going
>>>>> to take even longer than it already has. Sigh.
>>>>>
>>>>>
>>>>>> I was sorta hoping for something a bit better.
>>>>> I will be looking at alternatives. I am very much open to suggestions.
>>>>> I'm not even 100% convinced that Stephen's objections to my separate
>>>>> allocation strategy outweigh its advantages. If you have an opinion on
>>>>> that, I'd love to hear it.
>>>>>
>>>> fwiw I prefer the separate allocation strategy, but as you have already
>>>> said it trading off one set of problems for another. I would rather see
>>>> this move forward and one set of trade offs isn't significantly worse
>>>> than the other to me so, either wfm.
>>> I remain unclear that AppArmor needs this patch at all even when
>>> support for SO_PEERSEC lands.
>>> Contrary to the patch description, it is about supporting SCM_SECURITY
>>> for datagram not SO_PEERSEC. And I don't know of any actual users of
>>> SCM_SECURITY even for SELinux, just SO_PEERSEC.
>> I remembered that systemd once tried using SCM_SECURITY but that was a
>> bug since systemd was using it with stream sockets and that wasn't
>> supported by the kernel at the time,
>> https://bugzilla.redhat.com/show_bug.cgi?id=1224211, so systemd
>> switched over to using SO_PEERSEC. Subsequently I did fix
>> SCM_SECURITY to work with stream sockets via kernel commit
>> 37a9a8df8ce9de6ea73349c9ac8bdf6ba4ec4f70 but SO_PEERSEC is still
>> preferred. Looking around, I see that there is still one usage of
>> SCM_SECURITY in systemd-journald but it doesn't seem to be required
>> (if provided, journald will pass the label along but nothing seems to
>> depend on it AFAICT). In any event, I don't believe this patch is
>> needed to support stacking AppArmor.
>
> Stephen is, as is so often the case, correct. AppArmor has a stub
> socket_getpeersec_dgram() that gets removed in patch 23. If I remove
right but as I said before this is coming, I have been playing with
it and have code. So the series doesn't need it today but sooner than
later it will be needed
> it earlier and throw in a touch of scaffolding for secid_to_secctx()
> we can leave the secid as is for now. This can't be the final solution
> as AppArmor will be using the hook someday and we still have the all
> modules case to worry about for the next phase. It also assumes that
> The BPF module isn't going to suddenly sprout a security context.
>
Yep this is something that needs to be dealt with, whether it is now
or kicked down the road a little further ...
^ permalink raw reply
* Re: [PATCH net-next] cipso: fix 'audit_secid' kernel-doc warning in cipso_ipv4.c
From: David Miller @ 2020-09-09 3:03 UTC (permalink / raw)
To: wanghai38
Cc: paul, kuznet, yoshfuji, kuba, netdev, linux-kernel,
linux-security-module
In-Reply-To: <20200908135915.22039-1-wanghai38@huawei.com>
From: Wang Hai <wanghai38@huawei.com>
Date: Tue, 8 Sep 2020 21:59:15 +0800
> Fixes the following W=1 kernel build warning(s):
>
> net/ipv4/cipso_ipv4.c:510: warning: Excess function parameter 'audit_secid' description in 'cipso_v4_doi_remove'
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wang Hai <wanghai38@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] netlabel: Fix some kernel-doc warnings
From: David Miller @ 2020-09-09 3:04 UTC (permalink / raw)
To: wanghai38; +Cc: paul, kuba, netdev, linux-kernel, linux-security-module
In-Reply-To: <20200908140543.25514-1-wanghai38@huawei.com>
From: Wang Hai <wanghai38@huawei.com>
Date: Tue, 8 Sep 2020 22:05:43 +0800
> Fixes the following W=1 kernel build warning(s):
>
> net/netlabel/netlabel_calipso.c:438: warning: Excess function parameter 'audit_secid' description in 'calipso_doi_remove'
> net/netlabel/netlabel_calipso.c:605: warning: Excess function parameter 'reg' description in 'calipso_req_delattr'
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wang Hai <wanghai38@huawei.com>
Applied.
^ permalink raw reply
* Re: [RFC PATCH v8 0/3] Add support for AT_INTERPRETED (was O_MAYEXEC)
From: Mickaël Salaün @ 2020-09-09 7:19 UTC (permalink / raw)
To: Al Viro
Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Andrew Morton,
Andy Lutomirski, Christian Brauner, Christian Heimes,
Daniel Borkmann, Deven Bowers, Dmitry Vyukov, Eric Biggers,
Eric Chiang, Florian Weimer, James Morris, Jan Kara, Jann Horn,
Jonathan Corbet, Kees Cook, Lakshmi Ramasubramanian,
Matthew Garrett, Matthew Wilcox, Michael Kerrisk, Miklos Szeredi,
Mimi Zohar, Philippe Trébuchet, Scott Shell,
Sean Christopherson, Shuah Khan, Steve Dower, Steve Grubb,
Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel
In-Reply-To: <20200908185026.GU1236603@ZenIV.linux.org.uk>
On 08/09/2020 20:50, Al Viro wrote:
> On Tue, Sep 08, 2020 at 09:59:53AM +0200, Mickaël Salaün wrote:
>> Hi,
>>
>> This height patch series rework the previous O_MAYEXEC series by not
>> adding a new flag to openat2(2) but to faccessat2(2) instead. As
>> suggested, this enables to perform the access check on a file descriptor
>> instead of on a file path (while opening it). This may require two
>> checks (one on open and then with faccessat2) but it is a more generic
>> approach [8].
>
> Again, why is that folded into lookup/open/whatnot, rather than being
> an operation applied to a file (e.g. O_PATH one)?
>
I don't understand your question. AT_INTERPRETED can and should be used
with AT_EMPTY_PATH. The two checks I wrote about was for IMA.
^ permalink raw reply
* Re: [RFC PATCH 00/30] ima: Introduce IMA namespace
From: Dr. Greg @ 2020-09-09 10:11 UTC (permalink / raw)
To: Luke Hinds
Cc: Mimi Zohar, Christian Brauner, krzysztof.struczynski,
linux-integrity, Linux Kernel Mailing List, containers,
linux-security-module, stefanb, sunyuqiong1988, mkayaalp,
dmitry.kasatkin, Serge E. Hallyn, James Morris, christian,
silviu.vlasceanu, roberto.sassu, ebiederm, viro, torvalds, luto,
jannh, nick.dusek
In-Reply-To: <CAKrSGQR3Pw=Rad2RgUuCHqr0r2Nc6x2nLoo2cVAkD+_8Vbmd7A@mail.gmail.com>
On Mon, Sep 07, 2020 at 12:50:07PM +0100, Luke Hinds wrote:
Good morning, I hope the week is going well for everyone.
> On Sun, Sep 6, 2020 at 6:15 PM Dr. Greg <greg@enjellic.com> wrote:
> > Just to be clear, we are not campaigning or advocating what we have
> > done but are simply providing background for discussion. We haven't
> > campaigned this approach given how complex the kernel development has
> > become, particurlarly with respect to security infrastructure.
> >
> > Candidly, given the politics of security technology being viewed as
> > 'constraining' user rights, I think that a lot of forthcoming security
> > technology may end up being out of tree moving forward.
> I think it's prudent to look forward and plan diligently, but I would not
> want perfect to be the enemy of good.
>
> I approach this more from a user's perspective. We are using IMA in
> https://keylime.dev to measure a host and would like to measure
> within a container too. It's the most common request we hear from
> our users.
>
> Perhaps we all collaborate on a proposal extending Stefans work here:
> https://kernsec.org/wiki/index.php/IMA_Namespacing_design_considerations
>
> I have seen around 3-4 patches now get submitted, so work has been
> done before, and as above, users are present too. We could then have
> some consensus on how this should look and later patches might have
> more success at landing.
>
> Would anyone be interested in this and have recommendations on how
> we could approach this?
Obviously everyone is interested in sharpening their own knives so the
first challenge will be defining where this theme of measurement and
attestation needs to go.
Our focus in all of this is from a platform behavior modeling
perspective. Our objective is to design platforms/containers that are
capable of self-disciplining themselves in the event that they exhibit
behavior inconsistent with the wishes of their designer. Container
measurement trivially falls out of this model.
With respect to measurement namespaces, the first problem to be
addressed is what takes custody and responsibility for the measurement
events. In classic IMA this is, of course, a TPM. In our model we
use a Trusted Execution Environment (TEE) as this entity.
The TEE makes a decision as to whether or not the kernel should label
a context of execution as being a 'bad actor' if it indicates a desire
to exhibit a behavior inconsistent with a previously defined model.
As I noted previously we have an SGX based solution that provides this
infrastructure but have designed and are moving to a micro-controller
based alternative, given the fact that SGX is now moving to a 'cloud
only' solution.
One of the pain points in all of this appears to be whether or not a
measurement stream from a container should feed into the root
measurement of the platform or be fed into a measurement/monitoring
domain that can be attested against the root measurement of the
platform. Based on our experiences the latter model is the only one
that is feasible or makes sense from an attestation perspective.
So it would seem that a generic approach to directing the target of
the measurement events would be the first objective. If there is
interest we can make a copy of our patch available as it supports both
models.
> - Luke
Have a good day.
Dr. Greg
As always,
Dr. Greg Wettstein, Ph.D, Worker Autonomously self-defensive
Enjellic Systems Development, LLC IOT platforms and edge devices.
4206 N. 19th Ave.
Fargo, ND 58102
PH: 701-281-1686 EMAIL: dg@enjellic.com
------------------------------------------------------------------------------
"I created a hack to make the division come out right ... I was
relieved because I thought I was coding wrong.
Did you? It took a guy (Thomas Nicely) with a Ph.D. doing heavy
research in computational number theory to find it, yet you found it
while working on a game in QuickBasic?"
-- Slashdot
^ permalink raw reply
* Re: [PATCH v20 05/23] net: Prepare UDS for security module stacking
From: Stephen Smalley @ 2020-09-09 13:19 UTC (permalink / raw)
To: John Johansen
Cc: Casey Schaufler, Paul Moore, Casey Schaufler, James Morris,
LSM List, SElinux list, linux-audit, Kees Cook, Tetsuo Handa,
Stephen Smalley
In-Reply-To: <b320f0f6-02db-95a5-acc5-cadd5dbb57dc@canonical.com>
On Tue, Sep 8, 2020 at 8:21 PM John Johansen
<john.johansen@canonical.com> wrote:
>
> On 9/8/20 4:37 PM, Casey Schaufler wrote:
> > On 9/8/2020 6:35 AM, Stephen Smalley wrote:
> >> On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
> >> <stephen.smalley.work@gmail.com> wrote:
> >>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen
> >>> <john.johansen@canonical.com> wrote:
> >>>> On 9/5/20 11:13 AM, Casey Schaufler wrote:
> >>>>> On 9/5/2020 6:25 AM, Paul Moore wrote:
> >>>>>> On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote:
> >>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>>>>>>>> On 9/4/2020 1:08 PM, Paul Moore wrote:
> >>>>>> ...
> >>>>>>
> >>>>>>>> I understand the concerns you mention, they are all valid as far as
> >>>>>>>> I'm concerned, but I think we are going to get burned by this code as
> >>>>>>>> it currently stands.
> >>>>>>> Yes, I can see that. We're getting burned by the non-extensibility
> >>>>>>> of secids. It will take someone smarter than me to figure out how to
> >>>>>>> fit N secids into 32bits without danger of either failure or memory
> >>>>>>> allocation.
> >>>>>> Sooo what are the next steps here? It sounds like there is some
> >>>>>> agreement that the currently proposed unix_skb_params approach is a
> >>>>>> problem, but it also sounds like you just want to merge it anyway?
> >>>>> There are real problems with all the approaches. This is by far the
> >>>>> least invasive of the lot. If this is acceptable for now I will commit
> >>>>> to including the dynamic allocation version in the full stacking
> >>>>> (e.g. Smack + SELinux) stage. If it isn't, well, this stage is going
> >>>>> to take even longer than it already has. Sigh.
> >>>>>
> >>>>>
> >>>>>> I was sorta hoping for something a bit better.
> >>>>> I will be looking at alternatives. I am very much open to suggestions.
> >>>>> I'm not even 100% convinced that Stephen's objections to my separate
> >>>>> allocation strategy outweigh its advantages. If you have an opinion on
> >>>>> that, I'd love to hear it.
> >>>>>
> >>>> fwiw I prefer the separate allocation strategy, but as you have already
> >>>> said it trading off one set of problems for another. I would rather see
> >>>> this move forward and one set of trade offs isn't significantly worse
> >>>> than the other to me so, either wfm.
> >>> I remain unclear that AppArmor needs this patch at all even when
> >>> support for SO_PEERSEC lands.
> >>> Contrary to the patch description, it is about supporting SCM_SECURITY
> >>> for datagram not SO_PEERSEC. And I don't know of any actual users of
> >>> SCM_SECURITY even for SELinux, just SO_PEERSEC.
> >> I remembered that systemd once tried using SCM_SECURITY but that was a
> >> bug since systemd was using it with stream sockets and that wasn't
> >> supported by the kernel at the time,
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1224211, so systemd
> >> switched over to using SO_PEERSEC. Subsequently I did fix
> >> SCM_SECURITY to work with stream sockets via kernel commit
> >> 37a9a8df8ce9de6ea73349c9ac8bdf6ba4ec4f70 but SO_PEERSEC is still
> >> preferred. Looking around, I see that there is still one usage of
> >> SCM_SECURITY in systemd-journald but it doesn't seem to be required
> >> (if provided, journald will pass the label along but nothing seems to
> >> depend on it AFAICT). In any event, I don't believe this patch is
> >> needed to support stacking AppArmor.
> >
> > Stephen is, as is so often the case, correct. AppArmor has a stub
> > socket_getpeersec_dgram() that gets removed in patch 23. If I remove
>
> right but as I said before this is coming, I have been playing with
> it and have code. So the series doesn't need it today but sooner than
> later it will be needed
I don't understand why. Is there a userspace component that relies on
SCM_SECURITY today for anything real (more than just blindly passing
it along and maybe writing to a log somewhere)? And this doesn't
provide support for a composite SCM_SECURITY or SCM_CONTEXT, so it
doesn't really solve the stacking problem for it anyway. What am I
missing? Why do you care about this patch?
^ permalink raw reply
* Re: [RFC PATCH v8 0/3] Add support for AT_INTERPRETED (was O_MAYEXEC)
From: Matthew Wilcox @ 2020-09-09 17:08 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, linux-kernel, Aleksa Sarai, Alexei Starovoitov,
Andrew Morton, Andy Lutomirski, Christian Brauner,
Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
Jann Horn, Jonathan Corbet, Kees Cook, Lakshmi Ramasubramanian,
Matthew Garrett, Michael Kerrisk, Miklos Szeredi, Mimi Zohar,
Philippe Trébuchet, Scott Shell, Sean Christopherson,
Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
Thibaut Sautereau, Vincent Strubel, kernel-hardening, linux-api,
linux-integrity, linux-security-module, linux-fsdevel
In-Reply-To: <e3223b50-0d00-3b64-1e09-cfb1b9648b02@digikod.net>
On Wed, Sep 09, 2020 at 09:19:11AM +0200, Mickaël Salaün wrote:
>
> On 08/09/2020 20:50, Al Viro wrote:
> > On Tue, Sep 08, 2020 at 09:59:53AM +0200, Mickaël Salaün wrote:
> >> Hi,
> >>
> >> This height patch series rework the previous O_MAYEXEC series by not
> >> adding a new flag to openat2(2) but to faccessat2(2) instead. As
> >> suggested, this enables to perform the access check on a file descriptor
> >> instead of on a file path (while opening it). This may require two
> >> checks (one on open and then with faccessat2) but it is a more generic
> >> approach [8].
> >
> > Again, why is that folded into lookup/open/whatnot, rather than being
> > an operation applied to a file (e.g. O_PATH one)?
>
> I don't understand your question. AT_INTERPRETED can and should be used
> with AT_EMPTY_PATH. The two checks I wrote about was for IMA.
Al is saying you should add a new syscall, not try to fold it into
some existing syscall.
I agree with him. Add a new syscall, just like you were told to do it
last time.
^ permalink raw reply
* Re: [RFC PATCH v8 0/3] Add support for AT_INTERPRETED (was O_MAYEXEC)
From: Al Viro @ 2020-09-09 17:13 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Andrew Morton,
Andy Lutomirski, Christian Brauner, Christian Heimes,
Daniel Borkmann, Deven Bowers, Dmitry Vyukov, Eric Biggers,
Eric Chiang, Florian Weimer, James Morris, Jan Kara, Jann Horn,
Jonathan Corbet, Kees Cook, Lakshmi Ramasubramanian,
Matthew Garrett, Matthew Wilcox, Michael Kerrisk, Miklos Szeredi,
Mimi Zohar, Philippe Trébuchet, Scott Shell,
Sean Christopherson, Shuah Khan, Steve Dower, Steve Grubb,
Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel
In-Reply-To: <e3223b50-0d00-3b64-1e09-cfb1b9648b02@digikod.net>
On Wed, Sep 09, 2020 at 09:19:11AM +0200, Mickaël Salaün wrote:
>
> On 08/09/2020 20:50, Al Viro wrote:
> > On Tue, Sep 08, 2020 at 09:59:53AM +0200, Mickaël Salaün wrote:
> >> Hi,
> >>
> >> This height patch series rework the previous O_MAYEXEC series by not
> >> adding a new flag to openat2(2) but to faccessat2(2) instead. As
> >> suggested, this enables to perform the access check on a file descriptor
> >> instead of on a file path (while opening it). This may require two
> >> checks (one on open and then with faccessat2) but it is a more generic
> >> approach [8].
> >
> > Again, why is that folded into lookup/open/whatnot, rather than being
> > an operation applied to a file (e.g. O_PATH one)?
> >
>
> I don't understand your question. AT_INTERPRETED can and should be used
> with AT_EMPTY_PATH. The two checks I wrote about was for IMA.
Once more, with feeling: don't hide that behind existing syscalls.
If you want to tell LSM have a look at given fs object in a special
way, *add* *a* *new* *system* *call* *for* *doing* *just* *that*.
^ permalink raw reply
* [PATCH v2] certs: Add EFI_CERT_X509_GUID support for dbx entries
From: Eric Snowberg @ 2020-09-09 17:27 UTC (permalink / raw)
To: dhowells, dwmw2, jarkko.sakkinen
Cc: herbert, davem, jmorris, serge, nayna, zohar, eric.snowberg,
erichte, mpe, keyrings, linux-kernel, linux-crypto,
linux-security-module
The Secure Boot Forbidden Signature Database, dbx, contains a list of now
revoked signatures and keys previously approved to boot with UEFI Secure
Boot enabled. The dbx is capable of containing any number of
EFI_CERT_X509_SHA256_GUID, EFI_CERT_SHA256_GUID, and EFI_CERT_X509_GUID
entries.
Currently when EFI_CERT_X509_GUID are contained in the dbx, the entries are
skipped.
Add support for EFI_CERT_X509_GUID dbx entries. When a EFI_CERT_X509_GUID
is found, it is added as an asymmetrical key to the .blacklist keyring.
Anytime the .platform keyring is used, the keys in the .blacklist keyring
are referenced, if a matching key is found, the key will be rejected.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2:
Fixed build issue reported by kernel test robot <lkp@intel.com>
Commit message update (suggested by Jarkko Sakkinen)
---
certs/blacklist.c | 36 +++++++++++++++++++
certs/system_keyring.c | 6 ++++
include/crypto/pkcs7.h | 8 +++++
include/keys/system_keyring.h | 11 ++++++
.../platform_certs/keyring_handler.c | 11 ++++++
5 files changed, 72 insertions(+)
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 6514f9ebc943..17ebf50cf0ae 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -15,6 +15,7 @@
#include <linux/err.h>
#include <linux/seq_file.h>
#include <keys/system_keyring.h>
+#include <crypto/pkcs7.h>
#include "blacklist.h"
static struct key *blacklist_keyring;
@@ -100,6 +101,41 @@ int mark_hash_blacklisted(const char *hash)
return 0;
}
+int mark_key_revocationlisted(const char *data, size_t size)
+{
+ key_ref_t key;
+
+ key = key_create_or_update(make_key_ref(blacklist_keyring, true),
+ "asymmetric",
+ NULL,
+ data,
+ size,
+ ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+ KEY_USR_VIEW),
+ KEY_ALLOC_NOT_IN_QUOTA |
+ KEY_ALLOC_BUILT_IN);
+
+ if (IS_ERR(key)) {
+ pr_err("Problem with revocation key (%ld)\n", PTR_ERR(key));
+ return PTR_ERR(key);
+ }
+
+ return 0;
+}
+
+int is_key_revocationlisted(struct pkcs7_message *pkcs7)
+{
+ int ret;
+
+ ret = pkcs7_validate_trust(pkcs7, blacklist_keyring);
+
+ if (ret == 0)
+ return -EKEYREJECTED;
+
+ return -ENOKEY;
+}
+EXPORT_SYMBOL_GPL(is_key_revocationlisted);
+
/**
* is_hash_blacklisted - Determine if a hash is blacklisted
* @hash: The hash to be checked as a binary blob
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 798291177186..f8ea96219155 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -241,6 +241,12 @@ int verify_pkcs7_message_sig(const void *data, size_t len,
pr_devel("PKCS#7 platform keyring is not available\n");
goto error;
}
+
+ ret = is_key_revocationlisted(pkcs7);
+ if (ret != -ENOKEY) {
+ pr_devel("PKCS#7 platform key revocationlisted\n");
+ goto error;
+ }
}
ret = pkcs7_validate_trust(pkcs7, trusted_keys);
if (ret < 0) {
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 38ec7f5f9041..d8f2e0fdfbf4 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -26,11 +26,19 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
const void **_data, size_t *_datalen,
size_t *_headerlen);
+#ifdef CONFIG_PKCS7_MESSAGE_PARSER
/*
* pkcs7_trust.c
*/
extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
struct key *trust_keyring);
+#else
+static inline int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
+ struct key *trust_keyring)
+{
+ return -ENOKEY;
+}
+#endif
/*
* pkcs7_verify.c
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index fb8b07daa9d1..b6991cfe1b6d 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -31,11 +31,14 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
#define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
#endif
+extern struct pkcs7_message *pkcs7;
#ifdef CONFIG_SYSTEM_BLACKLIST_KEYRING
extern int mark_hash_blacklisted(const char *hash);
+extern int mark_key_revocationlisted(const char *data, size_t size);
extern int is_hash_blacklisted(const u8 *hash, size_t hash_len,
const char *type);
extern int is_binary_blacklisted(const u8 *hash, size_t hash_len);
+extern int is_key_revocationlisted(struct pkcs7_message *pkcs7);
#else
static inline int is_hash_blacklisted(const u8 *hash, size_t hash_len,
const char *type)
@@ -47,6 +50,14 @@ static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
{
return 0;
}
+static inline int mark_key_revocationlisted(const char *data, size_t size)
+{
+ return 0;
+}
+static inline int is_key_revocationlisted(struct pkcs7_message *pkcs7)
+{
+ return -ENOKEY;
+}
#endif
#ifdef CONFIG_IMA_BLACKLIST_KEYRING
diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index c5ba695c10e3..cc5a43804bc4 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -55,6 +55,15 @@ static __init void uefi_blacklist_binary(const char *source,
uefi_blacklist_hash(source, data, len, "bin:", 4);
}
+/*
+ * Revocationlist the X509 cert
+ */
+static __init void uefi_revocationlist_x509(const char *source,
+ const void *data, size_t len)
+{
+ mark_key_revocationlisted(data, len);
+}
+
/*
* Return the appropriate handler for particular signature list types found in
* the UEFI db and MokListRT tables.
@@ -76,5 +85,7 @@ __init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type)
return uefi_blacklist_x509_tbs;
if (efi_guidcmp(*sig_type, efi_cert_sha256_guid) == 0)
return uefi_blacklist_binary;
+ if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0)
+ return uefi_revocationlist_x509;
return 0;
}
--
2.18.1
^ permalink raw reply related
* Re: [PATCH v2] certs: Add EFI_CERT_X509_GUID support for dbx entries
From: Randy Dunlap @ 2020-09-09 17:40 UTC (permalink / raw)
To: Eric Snowberg, dhowells, dwmw2, jarkko.sakkinen
Cc: herbert, davem, jmorris, serge, nayna, zohar, erichte, mpe,
keyrings, linux-kernel, linux-crypto, linux-security-module
In-Reply-To: <20200909172736.73003-1-eric.snowberg@oracle.com>
On 9/9/20 10:27 AM, Eric Snowberg wrote:
> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> index 38ec7f5f9041..d8f2e0fdfbf4 100644
> --- a/include/crypto/pkcs7.h
> +++ b/include/crypto/pkcs7.h
> @@ -26,11 +26,19 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
> const void **_data, size_t *_datalen,
> size_t *_headerlen);
>
> +#ifdef CONFIG_PKCS7_MESSAGE_PARSER
> /*
> * pkcs7_trust.c
> */
> extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
> struct key *trust_keyring);
> +#else
> +static inline int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
> + struct key *trust_keyring)
> +{
> + return -ENOKEY;
> +}
> +#endif
Just to be clear, you want to do the #else block when
CONFIG_PKCS7_MESSAGE_PARSER=m. Is that correct?
If so, it might be clearer to use
#if IS_BUILTIN(CONFIG_PKCS7_MESSAGE_PARSER)
--
~Randy
^ permalink raw reply
* Re: [RFC PATCH v8 0/3] Add support for AT_INTERPRETED (was O_MAYEXEC)
From: Mickaël Salaün @ 2020-09-09 17:56 UTC (permalink / raw)
To: Al Viro
Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Andrew Morton,
Andy Lutomirski, Christian Brauner, Christian Heimes,
Daniel Borkmann, Deven Bowers, Dmitry Vyukov, Eric Biggers,
Eric Chiang, Florian Weimer, James Morris, Jan Kara, Jann Horn,
Jonathan Corbet, Kees Cook, Lakshmi Ramasubramanian,
Matthew Garrett, Matthew Wilcox, Michael Kerrisk, Miklos Szeredi,
Mimi Zohar, Philippe Trébuchet, Scott Shell,
Sean Christopherson, Shuah Khan, Steve Dower, Steve Grubb,
Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel
In-Reply-To: <20200909171316.GW1236603@ZenIV.linux.org.uk>
On 09/09/2020 19:13, Al Viro wrote:
> On Wed, Sep 09, 2020 at 09:19:11AM +0200, Mickaël Salaün wrote:
>>
>> On 08/09/2020 20:50, Al Viro wrote:
>>> On Tue, Sep 08, 2020 at 09:59:53AM +0200, Mickaël Salaün wrote:
>>>> Hi,
>>>>
>>>> This height patch series rework the previous O_MAYEXEC series by not
>>>> adding a new flag to openat2(2) but to faccessat2(2) instead. As
>>>> suggested, this enables to perform the access check on a file descriptor
>>>> instead of on a file path (while opening it). This may require two
>>>> checks (one on open and then with faccessat2) but it is a more generic
>>>> approach [8].
>>>
>>> Again, why is that folded into lookup/open/whatnot, rather than being
>>> an operation applied to a file (e.g. O_PATH one)?
>>>
>>
>> I don't understand your question. AT_INTERPRETED can and should be used
>> with AT_EMPTY_PATH. The two checks I wrote about was for IMA.
>
> Once more, with feeling: don't hide that behind existing syscalls.
> If you want to tell LSM have a look at given fs object in a special
> way, *add* *a* *new* *system* *call* *for* *doing* *just* *that*.
>
Fine, I'll do it. It will look a lot like this one though.
^ permalink raw reply
* Re: [RFC PATCH v8 0/3] Add support for AT_INTERPRETED (was O_MAYEXEC)
From: Mickaël Salaün @ 2020-09-09 17:55 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Al Viro, linux-kernel, Aleksa Sarai, Alexei Starovoitov,
Andrew Morton, Andy Lutomirski, Christian Brauner,
Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
Jann Horn, Jonathan Corbet, Kees Cook, Lakshmi Ramasubramanian,
Matthew Garrett, Michael Kerrisk, Miklos Szeredi, Mimi Zohar,
Philippe Trébuchet, Scott Shell, Sean Christopherson,
Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
Thibaut Sautereau, Vincent Strubel, kernel-hardening, linux-api,
linux-integrity, linux-security-module, linux-fsdevel
In-Reply-To: <20200909170851.GL6583@casper.infradead.org>
On 09/09/2020 19:08, Matthew Wilcox wrote:
> On Wed, Sep 09, 2020 at 09:19:11AM +0200, Mickaël Salaün wrote:
>>
>> On 08/09/2020 20:50, Al Viro wrote:
>>> On Tue, Sep 08, 2020 at 09:59:53AM +0200, Mickaël Salaün wrote:
>>>> Hi,
>>>>
>>>> This height patch series rework the previous O_MAYEXEC series by not
>>>> adding a new flag to openat2(2) but to faccessat2(2) instead. As
>>>> suggested, this enables to perform the access check on a file descriptor
>>>> instead of on a file path (while opening it). This may require two
>>>> checks (one on open and then with faccessat2) but it is a more generic
>>>> approach [8].
>>>
>>> Again, why is that folded into lookup/open/whatnot, rather than being
>>> an operation applied to a file (e.g. O_PATH one)?
>>
>> I don't understand your question. AT_INTERPRETED can and should be used
>> with AT_EMPTY_PATH. The two checks I wrote about was for IMA.
>
> Al is saying you should add a new syscall, not try to fold it into
> some existing syscall.
>
> I agree with him. Add a new syscall, just like you were told to do it
> last time.
>
OK, but I didn't receive a response for my proposition to extend
faccessat2(2).
^ permalink raw reply
* Re: [PATCH v20 05/23] net: Prepare UDS for security module stacking
From: Casey Schaufler @ 2020-09-09 18:19 UTC (permalink / raw)
To: Stephen Smalley, John Johansen
Cc: Paul Moore, Casey Schaufler, James Morris, LSM List, SElinux list,
linux-audit, Kees Cook, Tetsuo Handa, Stephen Smalley,
Casey Schaufler
In-Reply-To: <CAEjxPJ6wFJz935RR_1u+-EjAw3VMv4nabo-Za_OqkZGJuNS5Sg@mail.gmail.com>
On 9/9/2020 6:19 AM, Stephen Smalley wrote:
> On Tue, Sep 8, 2020 at 8:21 PM John Johansen
> <john.johansen@canonical.com> wrote:
>> On 9/8/20 4:37 PM, Casey Schaufler wrote:
>>> On 9/8/2020 6:35 AM, Stephen Smalley wrote:
>>>> On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
>>>> <stephen.smalley.work@gmail.com> wrote:
>>>>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen
>>>>> <john.johansen@canonical.com> wrote:
>>>>>> On 9/5/20 11:13 AM, Casey Schaufler wrote:
>>>>>>> On 9/5/2020 6:25 AM, Paul Moore wrote:
>>>>>>>> On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote:
>>>>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>>>>> On 9/4/2020 1:08 PM, Paul Moore wrote:
>>>>>>>> ...
>>>>>>>>
>>>>>>>>>> I understand the concerns you mention, they are all valid as far as
>>>>>>>>>> I'm concerned, but I think we are going to get burned by this code as
>>>>>>>>>> it currently stands.
>>>>>>>>> Yes, I can see that. We're getting burned by the non-extensibility
>>>>>>>>> of secids. It will take someone smarter than me to figure out how to
>>>>>>>>> fit N secids into 32bits without danger of either failure or memory
>>>>>>>>> allocation.
>>>>>>>> Sooo what are the next steps here? It sounds like there is some
>>>>>>>> agreement that the currently proposed unix_skb_params approach is a
>>>>>>>> problem, but it also sounds like you just want to merge it anyway?
>>>>>>> There are real problems with all the approaches. This is by far the
>>>>>>> least invasive of the lot. If this is acceptable for now I will commit
>>>>>>> to including the dynamic allocation version in the full stacking
>>>>>>> (e.g. Smack + SELinux) stage. If it isn't, well, this stage is going
>>>>>>> to take even longer than it already has. Sigh.
>>>>>>>
>>>>>>>
>>>>>>>> I was sorta hoping for something a bit better.
>>>>>>> I will be looking at alternatives. I am very much open to suggestions.
>>>>>>> I'm not even 100% convinced that Stephen's objections to my separate
>>>>>>> allocation strategy outweigh its advantages. If you have an opinion on
>>>>>>> that, I'd love to hear it.
>>>>>>>
>>>>>> fwiw I prefer the separate allocation strategy, but as you have already
>>>>>> said it trading off one set of problems for another. I would rather see
>>>>>> this move forward and one set of trade offs isn't significantly worse
>>>>>> than the other to me so, either wfm.
>>>>> I remain unclear that AppArmor needs this patch at all even when
>>>>> support for SO_PEERSEC lands.
>>>>> Contrary to the patch description, it is about supporting SCM_SECURITY
>>>>> for datagram not SO_PEERSEC. And I don't know of any actual users of
>>>>> SCM_SECURITY even for SELinux, just SO_PEERSEC.
>>>> I remembered that systemd once tried using SCM_SECURITY but that was a
>>>> bug since systemd was using it with stream sockets and that wasn't
>>>> supported by the kernel at the time,
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1224211, so systemd
>>>> switched over to using SO_PEERSEC. Subsequently I did fix
>>>> SCM_SECURITY to work with stream sockets via kernel commit
>>>> 37a9a8df8ce9de6ea73349c9ac8bdf6ba4ec4f70 but SO_PEERSEC is still
>>>> preferred. Looking around, I see that there is still one usage of
>>>> SCM_SECURITY in systemd-journald but it doesn't seem to be required
>>>> (if provided, journald will pass the label along but nothing seems to
>>>> depend on it AFAICT). In any event, I don't believe this patch is
>>>> needed to support stacking AppArmor.
>>> Stephen is, as is so often the case, correct. AppArmor has a stub
>>> socket_getpeersec_dgram() that gets removed in patch 23. If I remove
>> right but as I said before this is coming, I have been playing with
>> it and have code. So the series doesn't need it today but sooner than
>> later it will be needed
Is sooner like 5.10, or 5.15? It matters.
> I don't understand why. Is there a userspace component that relies on
> SCM_SECURITY today for anything real (more than just blindly passing
> it along and maybe writing to a log somewhere)? And this doesn't
> provide support for a composite SCM_SECURITY or SCM_CONTEXT, so it
> doesn't really solve the stacking problem for it anyway. What am I
> missing? Why do you care about this patch?
^ permalink raw reply
* [PATCH] selinux: Add helper functions to get and set checkreqprot
From: Lakshmi Ramasubramanian @ 2020-09-09 18:23 UTC (permalink / raw)
To: stephen.smalley.work, paul
Cc: sashal, jmorris, nramas, selinux, linux-security-module
checkreqprot data member in selinux_state struct is accessed directly by
SELinux functions to get and set. This could cause unexpected read or
write access to this data member due to compiler optimizations and\or
compiler's reordering of access to this field.
Add helper functions to get and set checkreqprot data member in
selinux_state struct. These helper functions use READ_ONCE and
WRITE_ONCE macros to ensure explicit read or write of memory for
this data member.
This patch is based on commit 66ccd2560aff
("selinux: simplify away security_policydb_len()") in "next" branch
in https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
security/selinux/hooks.c | 6 +++---
security/selinux/include/security.h | 10 ++++++++++
security/selinux/selinuxfs.c | 5 +++--
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 6210e98219a5..520c7b78bcd8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3718,7 +3718,7 @@ static int selinux_mmap_file(struct file *file, unsigned long reqprot,
return rc;
}
- if (selinux_state.checkreqprot)
+ if (selinux_checkreqprot(&selinux_state))
prot = reqprot;
return file_map_prot_check(file, prot,
@@ -3732,7 +3732,7 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
- if (selinux_state.checkreqprot)
+ if (selinux_checkreqprot(&selinux_state))
prot = reqprot;
if (default_noexec &&
@@ -7234,7 +7234,7 @@ static __init int selinux_init(void)
memset(&selinux_state, 0, sizeof(selinux_state));
enforcing_set(&selinux_state, selinux_enforcing_boot);
- selinux_state.checkreqprot = selinux_checkreqprot_boot;
+ selinux_checkreqprot_set(&selinux_state, selinux_checkreqprot_boot);
selinux_avc_init(&selinux_state.avc);
mutex_init(&selinux_state.status_lock);
mutex_init(&selinux_state.policy_mutex);
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index cbdd3c7aff8b..b19d919f01e7 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -209,6 +209,16 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
}
+static inline bool selinux_checkreqprot(const struct selinux_state *state)
+{
+ return READ_ONCE(state->checkreqprot);
+}
+static inline void selinux_checkreqprot_set(struct selinux_state *state,
+ bool value)
+{
+ WRITE_ONCE(state->checkreqprot, value);
+}
+
int security_mls_enabled(struct selinux_state *state);
int security_load_policy(struct selinux_state *state,
void *data, size_t len,
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 45e9efa9bf5b..ba636d3ea89d 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -717,7 +717,8 @@ static ssize_t sel_read_checkreqprot(struct file *filp, char __user *buf,
char tmpbuf[TMPBUFLEN];
ssize_t length;
- length = scnprintf(tmpbuf, TMPBUFLEN, "%u", fsi->state->checkreqprot);
+ length = scnprintf(tmpbuf, TMPBUFLEN, "%u",
+ selinux_checkreqprot(fsi->state));
return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
}
@@ -759,7 +760,7 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf,
comm, current->pid);
}
- fsi->state->checkreqprot = new_value ? 1 : 0;
+ selinux_checkreqprot_set(fsi->state, (new_value ? 1 : 0));
length = count;
out:
kfree(page);
--
2.28.0
^ permalink raw reply related
* Re: [PATCH v20 05/23] net: Prepare UDS for security module stacking
From: John Johansen @ 2020-09-09 18:33 UTC (permalink / raw)
To: Casey Schaufler, Stephen Smalley
Cc: Paul Moore, Casey Schaufler, James Morris, LSM List, SElinux list,
linux-audit, Kees Cook, Tetsuo Handa, Stephen Smalley
In-Reply-To: <258ef772-0560-3fc3-9b9b-89941a7713fd@schaufler-ca.com>
On 9/9/20 11:19 AM, Casey Schaufler wrote:
> On 9/9/2020 6:19 AM, Stephen Smalley wrote:
>> On Tue, Sep 8, 2020 at 8:21 PM John Johansen
>> <john.johansen@canonical.com> wrote:
>>> On 9/8/20 4:37 PM, Casey Schaufler wrote:
>>>> On 9/8/2020 6:35 AM, Stephen Smalley wrote:
>>>>> On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
>>>>> <stephen.smalley.work@gmail.com> wrote:
>>>>>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen
>>>>>> <john.johansen@canonical.com> wrote:
>>>>>>> On 9/5/20 11:13 AM, Casey Schaufler wrote:
>>>>>>>> On 9/5/2020 6:25 AM, Paul Moore wrote:
>>>>>>>>> On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote:
>>>>>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>>>>>> On 9/4/2020 1:08 PM, Paul Moore wrote:
>>>>>>>>> ...
>>>>>>>>>
>>>>>>>>>>> I understand the concerns you mention, they are all valid as far as
>>>>>>>>>>> I'm concerned, but I think we are going to get burned by this code as
>>>>>>>>>>> it currently stands.
>>>>>>>>>> Yes, I can see that. We're getting burned by the non-extensibility
>>>>>>>>>> of secids. It will take someone smarter than me to figure out how to
>>>>>>>>>> fit N secids into 32bits without danger of either failure or memory
>>>>>>>>>> allocation.
>>>>>>>>> Sooo what are the next steps here? It sounds like there is some
>>>>>>>>> agreement that the currently proposed unix_skb_params approach is a
>>>>>>>>> problem, but it also sounds like you just want to merge it anyway?
>>>>>>>> There are real problems with all the approaches. This is by far the
>>>>>>>> least invasive of the lot. If this is acceptable for now I will commit
>>>>>>>> to including the dynamic allocation version in the full stacking
>>>>>>>> (e.g. Smack + SELinux) stage. If it isn't, well, this stage is going
>>>>>>>> to take even longer than it already has. Sigh.
>>>>>>>>
>>>>>>>>
>>>>>>>>> I was sorta hoping for something a bit better.
>>>>>>>> I will be looking at alternatives. I am very much open to suggestions.
>>>>>>>> I'm not even 100% convinced that Stephen's objections to my separate
>>>>>>>> allocation strategy outweigh its advantages. If you have an opinion on
>>>>>>>> that, I'd love to hear it.
>>>>>>>>
>>>>>>> fwiw I prefer the separate allocation strategy, but as you have already
>>>>>>> said it trading off one set of problems for another. I would rather see
>>>>>>> this move forward and one set of trade offs isn't significantly worse
>>>>>>> than the other to me so, either wfm.
>>>>>> I remain unclear that AppArmor needs this patch at all even when
>>>>>> support for SO_PEERSEC lands.
>>>>>> Contrary to the patch description, it is about supporting SCM_SECURITY
>>>>>> for datagram not SO_PEERSEC. And I don't know of any actual users of
>>>>>> SCM_SECURITY even for SELinux, just SO_PEERSEC.
>>>>> I remembered that systemd once tried using SCM_SECURITY but that was a
>>>>> bug since systemd was using it with stream sockets and that wasn't
>>>>> supported by the kernel at the time,
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1224211, so systemd
>>>>> switched over to using SO_PEERSEC. Subsequently I did fix
>>>>> SCM_SECURITY to work with stream sockets via kernel commit
>>>>> 37a9a8df8ce9de6ea73349c9ac8bdf6ba4ec4f70 but SO_PEERSEC is still
>>>>> preferred. Looking around, I see that there is still one usage of
>>>>> SCM_SECURITY in systemd-journald but it doesn't seem to be required
>>>>> (if provided, journald will pass the label along but nothing seems to
>>>>> depend on it AFAICT). In any event, I don't believe this patch is
>>>>> needed to support stacking AppArmor.
>>>> Stephen is, as is so often the case, correct. AppArmor has a stub
>>>> socket_getpeersec_dgram() that gets removed in patch 23. If I remove
>>> right but as I said before this is coming, I have been playing with
>>> it and have code. So the series doesn't need it today but sooner than
>>> later it will be needed
>
> Is sooner like 5.10, or 5.15? It matters.
>
I can split SCM_SECURITY off from the rest of the unix mediation and
push it off for a while. So lets call it 5.15 or later.
>> I don't understand why. Is there a userspace component that relies on
>> SCM_SECURITY today for anything real (more than just blindly passing
>> it along and maybe writing to a log somewhere)? And this doesn't
>> provide support for a composite SCM_SECURITY or SCM_CONTEXT, so it
>> doesn't really solve the stacking problem for it anyway. What am I
>> missing? Why do you care about this patch?
^ permalink raw reply
* Re: [PATCH v20 05/23] net: Prepare UDS for security module stacking
From: John Johansen @ 2020-09-09 18:47 UTC (permalink / raw)
To: Stephen Smalley
Cc: Casey Schaufler, Paul Moore, Casey Schaufler, James Morris,
LSM List, SElinux list, linux-audit, Kees Cook, Tetsuo Handa,
Stephen Smalley
In-Reply-To: <CAEjxPJ6wFJz935RR_1u+-EjAw3VMv4nabo-Za_OqkZGJuNS5Sg@mail.gmail.com>
On 9/9/20 6:19 AM, Stephen Smalley wrote:
> On Tue, Sep 8, 2020 at 8:21 PM John Johansen
> <john.johansen@canonical.com> wrote:
>>
>> On 9/8/20 4:37 PM, Casey Schaufler wrote:
>>> On 9/8/2020 6:35 AM, Stephen Smalley wrote:
>>>> On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
>>>> <stephen.smalley.work@gmail.com> wrote:
>>>>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen
>>>>> <john.johansen@canonical.com> wrote:
>>>>>> On 9/5/20 11:13 AM, Casey Schaufler wrote:
>>>>>>> On 9/5/2020 6:25 AM, Paul Moore wrote:
>>>>>>>> On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote:
>>>>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>>>>>>>> On 9/4/2020 1:08 PM, Paul Moore wrote:
>>>>>>>> ...
>>>>>>>>
>>>>>>>>>> I understand the concerns you mention, they are all valid as far as
>>>>>>>>>> I'm concerned, but I think we are going to get burned by this code as
>>>>>>>>>> it currently stands.
>>>>>>>>> Yes, I can see that. We're getting burned by the non-extensibility
>>>>>>>>> of secids. It will take someone smarter than me to figure out how to
>>>>>>>>> fit N secids into 32bits without danger of either failure or memory
>>>>>>>>> allocation.
>>>>>>>> Sooo what are the next steps here? It sounds like there is some
>>>>>>>> agreement that the currently proposed unix_skb_params approach is a
>>>>>>>> problem, but it also sounds like you just want to merge it anyway?
>>>>>>> There are real problems with all the approaches. This is by far the
>>>>>>> least invasive of the lot. If this is acceptable for now I will commit
>>>>>>> to including the dynamic allocation version in the full stacking
>>>>>>> (e.g. Smack + SELinux) stage. If it isn't, well, this stage is going
>>>>>>> to take even longer than it already has. Sigh.
>>>>>>>
>>>>>>>
>>>>>>>> I was sorta hoping for something a bit better.
>>>>>>> I will be looking at alternatives. I am very much open to suggestions.
>>>>>>> I'm not even 100% convinced that Stephen's objections to my separate
>>>>>>> allocation strategy outweigh its advantages. If you have an opinion on
>>>>>>> that, I'd love to hear it.
>>>>>>>
>>>>>> fwiw I prefer the separate allocation strategy, but as you have already
>>>>>> said it trading off one set of problems for another. I would rather see
>>>>>> this move forward and one set of trade offs isn't significantly worse
>>>>>> than the other to me so, either wfm.
>>>>> I remain unclear that AppArmor needs this patch at all even when
>>>>> support for SO_PEERSEC lands.
>>>>> Contrary to the patch description, it is about supporting SCM_SECURITY
>>>>> for datagram not SO_PEERSEC. And I don't know of any actual users of
>>>>> SCM_SECURITY even for SELinux, just SO_PEERSEC.
>>>> I remembered that systemd once tried using SCM_SECURITY but that was a
>>>> bug since systemd was using it with stream sockets and that wasn't
>>>> supported by the kernel at the time,
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1224211, so systemd
>>>> switched over to using SO_PEERSEC. Subsequently I did fix
>>>> SCM_SECURITY to work with stream sockets via kernel commit
>>>> 37a9a8df8ce9de6ea73349c9ac8bdf6ba4ec4f70 but SO_PEERSEC is still
>>>> preferred. Looking around, I see that there is still one usage of
>>>> SCM_SECURITY in systemd-journald but it doesn't seem to be required
>>>> (if provided, journald will pass the label along but nothing seems to
>>>> depend on it AFAICT). In any event, I don't believe this patch is
>>>> needed to support stacking AppArmor.
>>>
>>> Stephen is, as is so often the case, correct. AppArmor has a stub
>>> socket_getpeersec_dgram() that gets removed in patch 23. If I remove
>>
>> right but as I said before this is coming, I have been playing with
>> it and have code. So the series doesn't need it today but sooner than
>> later it will be needed
>
> I don't understand why. Is there a userspace component that relies on
> SCM_SECURITY today for anything real (more than just blindly passing
> it along and maybe writing to a log somewhere)? And this doesn't
> provide support for a composite SCM_SECURITY or SCM_CONTEXT, so it
> doesn't really solve the stacking problem for it anyway. What am I
> missing? Why do you care about this patch?
>
personally I don't atm, but there are people who do care about this in
there logs, whether they should or shouldn't is an entirely different
question.
Long term there may be some uses for it that I care about or "have to
care about." For now Casey can drop it from this series.
^ permalink raw reply
* [PATCH] ima: Use kmemdup rather than kmalloc+memcpy
From: Alex Dewar @ 2020-09-09 19:09 UTC (permalink / raw)
Cc: Alex Dewar, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, linux-integrity, linux-security-module,
linux-kernel
Issue identified with Coccinelle.
Signed-off-by: Alex Dewar <alex.dewar90@gmail.com>
---
security/integrity/ima/ima_policy.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index b4de33074b37..1de3140b334f 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -284,15 +284,14 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
struct ima_rule_entry *nentry;
int i;
- nentry = kmalloc(sizeof(*nentry), GFP_KERNEL);
- if (!nentry)
- return NULL;
-
/*
* Immutable elements are copied over as pointers and data; only
* lsm rules can change
*/
- memcpy(nentry, entry, sizeof(*nentry));
+ nentry = kmemdup(entry, sizeof(*nentry), GFP_KERNEL);
+ if (!nentry)
+ return NULL;
+
memset(nentry->lsm, 0, sizeof_field(struct ima_rule_entry, lsm));
for (i = 0; i < MAX_LSM_RULES; i++) {
--
2.28.0
^ permalink raw reply related
* Re: [PATCH] selinux: Add helper functions to get and set checkreqprot
From: Stephen Smalley @ 2020-09-09 20:02 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Paul Moore, Sasha Levin, James Morris, SElinux list, LSM List
In-Reply-To: <20200909182351.10740-1-nramas@linux.microsoft.com>
On Wed, Sep 9, 2020 at 2:23 PM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> checkreqprot data member in selinux_state struct is accessed directly by
> SELinux functions to get and set. This could cause unexpected read or
> write access to this data member due to compiler optimizations and\or
and/or
> compiler's reordering of access to this field.
>
> Add helper functions to get and set checkreqprot data member in
> selinux_state struct. These helper functions use READ_ONCE and
> WRITE_ONCE macros to ensure explicit read or write of memory for
> this data member.
s/explicit/atomic/
> This patch is based on commit 66ccd2560aff
> ("selinux: simplify away security_policydb_len()") in "next" branch
> in https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
Don't include this kind of information in a commit message, if needed
it can go after the --- or in brackets in the subject line ala [-next]
but it isn't necessary when sending against the next branch because
that's the default expectation for submitted patches for selinux. No
need to cc lsm list on selinux-only patches.
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index cbdd3c7aff8b..b19d919f01e7 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -209,6 +209,16 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
> return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
> }
>
> +static inline bool selinux_checkreqprot(const struct selinux_state *state)
> +{
> + return READ_ONCE(state->checkreqprot);
> +}
> +static inline void selinux_checkreqprot_set(struct selinux_state *state,
> + bool value)
> +{
> + WRITE_ONCE(state->checkreqprot, value);
> +}
Move these up with the enforcing accessor functions in this header and
use a consistent naming, e.g. checkreqprot_enabled(),
checkreqprot_set().
^ permalink raw reply
* Re: [PATCH v2] certs: Add EFI_CERT_X509_GUID support for dbx entries
From: Eric Snowberg @ 2020-09-09 22:44 UTC (permalink / raw)
To: Randy Dunlap
Cc: dhowells, dwmw2, Jarkko Sakkinen, herbert, davem, jmorris, serge,
nayna, Mimi Zohar, erichte, mpe, keyrings, linux-kernel,
linux-crypto, linux-security-module
In-Reply-To: <5074bc5c-8dd4-16d7-2760-3e657b90bfa2@infradead.org>
> On Sep 9, 2020, at 11:40 AM, Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 9/9/20 10:27 AM, Eric Snowberg wrote:
>> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
>> index 38ec7f5f9041..d8f2e0fdfbf4 100644
>> --- a/include/crypto/pkcs7.h
>> +++ b/include/crypto/pkcs7.h
>> @@ -26,11 +26,19 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
>> const void **_data, size_t *_datalen,
>> size_t *_headerlen);
>>
>> +#ifdef CONFIG_PKCS7_MESSAGE_PARSER
>> /*
>> * pkcs7_trust.c
>> */
>> extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
>> struct key *trust_keyring);
>> +#else
>> +static inline int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
>> + struct key *trust_keyring)
>> +{
>> + return -ENOKEY;
>> +}
>> +#endif
>
> Just to be clear, you want to do the #else block when
> CONFIG_PKCS7_MESSAGE_PARSER=m. Is that correct?
>
> If so, it might be clearer to use
>
> #if IS_BUILTIN(CONFIG_PKCS7_MESSAGE_PARSER)
>
I just added this part to fix a build error when none of the
asymmetrical keys are defined within a config. I failed to notice
CONFIG_PKCS7_MESSAGE_PARSER could be configured to build as a module
too. The code I added that uses pkcs7_validate_trust is always
builtin. Taking this into account, please disregard this patch.
I will need to solve this a different way. Thanks for pointing this
out.
^ permalink raw reply
* BUG: unable to handle kernel NULL pointer dereference in qlist_free_all (8)
From: syzbot @ 2020-09-10 6:26 UTC (permalink / raw)
To: jmorris, linux-kernel, linux-security-module, penguin-kernel,
serge, syzkaller-bugs, takedakn
Hello,
syzbot found the following issue on:
HEAD commit: 34d4ddd3 Merge tag 'linux-kselftest-5.9-rc5' of git://git...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=147c760d900000
kernel config: https://syzkaller.appspot.com/x/.config?x=a9075b36a6ae26c9
dashboard link: https://syzkaller.appspot.com/bug?extid=4bba137eaf7cc94b57ca
compiler: gcc (GCC) 10.1.0-syz 20200507
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+4bba137eaf7cc94b57ca@syzkaller.appspotmail.com
BUG: kernel NULL pointer dereference, address: 0000000000000080
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 4bbac067 P4D 4bbac067 PUD 9468a067 PMD 0
Oops: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 3936 Comm: syz-executor.0 Not tainted 5.9.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:qlink_to_object mm/kasan/quarantine.c:137 [inline]
RIP: 0010:qlink_free mm/kasan/quarantine.c:142 [inline]
RIP: 0010:qlist_free_all+0x36/0x170 mm/kasan/quarantine.c:168
Code: 53 48 83 ec 10 48 8b 37 48 85 f6 0f 84 36 01 00 00 49 bf 00 00 00 00 00 fc ff df 48 85 ed 49 89 fd 48 89 ef 0f 84 97 00 00 00 <48> 63 87 80 00 00 00 4c 8b 26 48 29 c6 48 83 3d 75 e5 01 08 00 0f
RSP: 0018:ffffc90004dd7a10 EFLAGS: 00010246
RAX: ffffea0000000000 RBX: 0000000000000282 RCX: ffffea0000000007
RDX: 0000000000000000 RSI: ffff888000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000001 R09: ffffffff8c5f5a77
R10: 0000000000000000 R11: 0000000000000001 R12: ffff888000000000
R13: ffffc90004dd7a58 R14: 0000000000000200 R15: dffffc0000000000
FS: 0000000003563940(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000080 CR3: 000000003d19c000 CR4: 00000000001526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
quarantine_reduce+0x17e/0x200 mm/kasan/quarantine.c:261
__kasan_kmalloc.constprop.0+0x9e/0xd0 mm/kasan/common.c:442
slab_post_alloc_hook mm/slab.h:518 [inline]
slab_alloc mm/slab.c:3312 [inline]
__do_kmalloc mm/slab.c:3653 [inline]
__kmalloc+0x178/0x310 mm/slab.c:3664
kmalloc include/linux/slab.h:559 [inline]
kzalloc include/linux/slab.h:666 [inline]
tomoyo_encode2.part.0+0xe9/0x3a0 security/tomoyo/realpath.c:45
tomoyo_encode2 security/tomoyo/realpath.c:31 [inline]
tomoyo_encode+0x28/0x50 security/tomoyo/realpath.c:80
tomoyo_path_perm+0x35f/0x3f0 security/tomoyo/file.c:831
tomoyo_path_symlink+0x94/0xe0 security/tomoyo/tomoyo.c:200
security_path_symlink+0xdf/0x150 security/security.c:1109
do_symlinkat+0x123/0x2c0 fs/namei.c:3984
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x45d2e7
Code: 0f 1f 00 b8 5c 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 1d b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 b8 58 00 00 00 0f 05 <48> 3d 01 f0 ff ff 0f 83 fd b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:000000000169fda8 EFLAGS: 00000202 ORIG_RAX: 0000000000000058
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000045d2e7
RDX: 000000000169fe43 RSI: 00000000004c30cd RDI: 000000000169fe30
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000013
R10: 0000000000000075 R11: 0000000000000202 R12: 0000000000000000
R13: 000000000169fde0 R14: 0000000000000000 R15: 000000000169fdf0
Modules linked in:
CR2: 0000000000000080
---[ end trace 9fd83eee6918f461 ]---
RIP: 0010:qlink_to_object mm/kasan/quarantine.c:137 [inline]
RIP: 0010:qlink_free mm/kasan/quarantine.c:142 [inline]
RIP: 0010:qlist_free_all+0x36/0x170 mm/kasan/quarantine.c:168
Code: 53 48 83 ec 10 48 8b 37 48 85 f6 0f 84 36 01 00 00 49 bf 00 00 00 00 00 fc ff df 48 85 ed 49 89 fd 48 89 ef 0f 84 97 00 00 00 <48> 63 87 80 00 00 00 4c 8b 26 48 29 c6 48 83 3d 75 e5 01 08 00 0f
RSP: 0018:ffffc90004dd7a10 EFLAGS: 00010246
RAX: ffffea0000000000 RBX: 0000000000000282 RCX: ffffea0000000007
RDX: 0000000000000000 RSI: ffff888000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000001 R09: ffffffff8c5f5a77
R10: 0000000000000000 R11: 0000000000000001 R12: ffff888000000000
R13: ffffc90004dd7a58 R14: 0000000000000200 R15: dffffc0000000000
FS: 0000000003563940(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000749198 CR3: 000000003d19c000 CR4: 00000000001526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply
* Re: [RFC PATCH v8 0/3] Add support for AT_INTERPRETED (was O_MAYEXEC)
From: Thibaut Sautereau @ 2020-09-10 9:26 UTC (permalink / raw)
To: Matthew Wilcox, Al Viro
Cc: Mickaël Salaün, linux-kernel, Aleksa Sarai,
Alexei Starovoitov, Andrew Morton, Andy Lutomirski,
Christian Brauner, Christian Heimes, Daniel Borkmann,
Deven Bowers, Dmitry Vyukov, Eric Biggers, Eric Chiang,
Florian Weimer, James Morris, Jan Kara, Jann Horn,
Jonathan Corbet, Kees Cook, Lakshmi Ramasubramanian,
Matthew Garrett, Michael Kerrisk, Miklos Szeredi, Mimi Zohar,
Philippe Trébuchet, Scott Shell, Sean Christopherson,
Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
Vincent Strubel, kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel, Arnd Bergmann
In-Reply-To: <20200909170851.GL6583@casper.infradead.org>
On Wed, Sep 09, 2020 at 06:08:51PM +0100, Matthew Wilcox wrote:
> On Wed, Sep 09, 2020 at 09:19:11AM +0200, Mickaël Salaün wrote:
> >
> > On 08/09/2020 20:50, Al Viro wrote:
> > > On Tue, Sep 08, 2020 at 09:59:53AM +0200, Mickaël Salaün wrote:
> > >> Hi,
> > >>
> > >> This height patch series rework the previous O_MAYEXEC series by not
> > >> adding a new flag to openat2(2) but to faccessat2(2) instead. As
> > >> suggested, this enables to perform the access check on a file descriptor
> > >> instead of on a file path (while opening it). This may require two
> > >> checks (one on open and then with faccessat2) but it is a more generic
> > >> approach [8].
> > >
> > > Again, why is that folded into lookup/open/whatnot, rather than being
> > > an operation applied to a file (e.g. O_PATH one)?
> >
> > I don't understand your question. AT_INTERPRETED can and should be used
> > with AT_EMPTY_PATH. The two checks I wrote about was for IMA.
>
> Al is saying you should add a new syscall, not try to fold it into
> some existing syscall.
>
> I agree with him. Add a new syscall, just like you were told to do it
> last time.
Sure, we'll do it. In the meantime, could we at least get an explanation
about why using faccessat2() instead of a new syscall is wrong? I could
see the reasons for separating the exec checks from the file opening,
but this time I don't understand. Is it because it brings too much
complexity to do_faccessat()?
--
Thibaut Sautereau
CLIP OS developer
^ permalink raw reply
* [RFC PATCH v9 0/3] Add introspect_access(2) (was O_MAYEXEC)
From: Mickaël Salaün @ 2020-09-10 16:46 UTC (permalink / raw)
To: linux-kernel
Cc: Mickaël Salaün, Aleksa Sarai, Alexei Starovoitov,
Al Viro, Andrew Morton, Andy Lutomirski, Arnd Bergmann,
Casey Schaufler, Christian Brauner, Christian Heimes,
Daniel Borkmann, Deven Bowers, Dmitry Vyukov, Eric Biggers,
Eric Chiang, Florian Weimer, James Morris, Jan Kara, Jann Horn,
Jonathan Corbet, Kees Cook, Lakshmi Ramasubramanian,
Matthew Garrett, Matthew Wilcox, Michael Kerrisk, Miklos Szeredi,
Mimi Zohar, Philippe Trébuchet, Scott Shell,
Sean Christopherson, Shuah Khan, Steve Dower, Steve Grubb,
Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel
Hi,
This ninth patch series rework the previous AT_INTERPRETED and O_MAYEXEC
series with a new syscall: introspect_access(2) . Access check are now
only possible on a file descriptor, which enable to avoid possible race
conditions in user space.
For now, the only LSM hook triggered by introspect_access(2) is
inode_permission() which takes a struct inode as argument. However,
struct path is still available in this syscall, which enables to add a
new hook to fit the needs of IMA and other path-based LSMs.
Goal of introspect_access(2)
============================
The goal of this patch series is to enable to control script execution
with interpreters help. A new introspect_access() system call is added
to enable user space script interpreters to delegate to the kernel (and
thus the system security policy) the permission to interpret/execute
scripts or other files containing what can be seen as commands.
A simple system-wide security policy can be enforced by the system
administrator through a sysctl configuration consistent with the mount
points or the file access rights. The documentation patch explains the
prerequisites.
Furthermore, the security policy can also be delegated to an LSM, either
a MAC system or an integrity system. For instance, the new kernel
MAY_INTROSPECTION_EXEC flag is required to close a major IMA
measurement/appraisal interpreter integrity gap by bringing the ability
to check the use of scripts [1]. Other uses are expected, such as for
magic-links [2], SGX integration [3], bpffs [4] or IPE [5].
Possible extended usage
=======================
For now, only the X_OK mode is compatible with introspect_access(2).
This enables to restrict the addition of new control flows in a process.
Using R_OK or W_OK with introspect_access(2) returns -EINVAL.
Possible future use-cases for R_OK with introspect_access(2) may be to
check configuration files that may impact the behavior of applications
(i.e. influence critical part of the current control flow). Those
should then be trusted as well. The W_OK with introspect_access(2)
could be used to check that a file descriptor is allowed to receive
sensitive data such as debug logs.
Prerequisite of its use
=======================
User space needs to adapt to take advantage of this new feature. For
example, the PEP 578 [6] (Runtime Audit Hooks) enables Python 3.8 to be
extended with policy enforcement points related to code interpretation,
which can be used to align with the PowerShell audit features.
Additional Python security improvements (e.g. a limited interpreter
without -c, stdin piping of code) are on their way [7].
Examples
========
The initial idea comes from CLIP OS 4 and the original implementation
has been used for more than 12 years:
https://github.com/clipos-archive/clipos4_doc
Chrome OS has a similar approach:
https://chromium.googlesource.com/chromiumos/docs/+/master/security/noexec_shell_scripts.md
Userland patches can be found here:
https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
Actually, there is more than the O_MAYEXEC changes (which matches this search)
e.g., to prevent Python interactive execution. There are patches for
Bash, Wine, Java (Icedtea), Busybox's ash, Perl and Python. There are
also some related patches which do not directly rely on O_MAYEXEC but
which restrict the use of browser plugins and extensions, which may be
seen as scripts too:
https://github.com/clipos-archive/clipos4_portage-overlay/tree/master/www-client
An introduction to O_MAYEXEC was given at the Linux Security Summit
Europe 2018 - Linux Kernel Security Contributions by ANSSI:
https://www.youtube.com/watch?v=chNjCRtPKQY&t=17m15s
The "write xor execute" principle was explained at Kernel Recipes 2018 -
CLIP OS: a defense-in-depth OS:
https://www.youtube.com/watch?v=PjRE0uBtkHU&t=11m14s
See also an overview article: https://lwn.net/Articles/820000/
This patch series can be applied on top of v5.9-rc4 . This can be tested
with CONFIG_SYSCTL. I would really appreciate constructive comments on
this patch series.
Previous version:
https://lore.kernel.org/lkml/20200908075956.1069018-1-mic@digikod.net/
[1] https://lore.kernel.org/lkml/1544647356.4028.105.camel@linux.ibm.com/
[2] https://lore.kernel.org/lkml/20190904201933.10736-6-cyphar@cyphar.com/
[3] https://lore.kernel.org/lkml/CALCETrVovr8XNZSroey7pHF46O=kj_c5D9K8h=z2T_cNrpvMig@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CALCETrVeZ0eufFXwfhtaG_j+AdvbzEWE0M3wjXMWVEO7pj+xkw@mail.gmail.com/
[5] https://lore.kernel.org/lkml/20200406221439.1469862-12-deven.desai@linux.microsoft.com/
[6] https://www.python.org/dev/peps/pep-0578/
[7] https://lore.kernel.org/lkml/0c70debd-e79e-d514-06c6-4cd1e021fa8b@python.org/
Regards,
Mickaël Salaün (3):
fs: Add introspect_access(2) syscall implementation and related sysctl
arch: Wire up introspect_access(2)
selftest/interpreter: Add tests for introspect_access(2) policies
Documentation/admin-guide/sysctl/fs.rst | 50 +++
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 2 +
arch/ia64/kernel/syscalls/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
fs/open.c | 79 ++++
include/linux/fs.h | 3 +
include/linux/syscalls.h | 1 +
include/uapi/asm-generic/unistd.h | 4 +-
kernel/sysctl.c | 12 +-
.../testing/selftests/interpreter/.gitignore | 2 +
tools/testing/selftests/interpreter/Makefile | 18 +
tools/testing/selftests/interpreter/config | 1 +
.../interpreter/introspection_policy_test.c | 361 ++++++++++++++++++
28 files changed, 547 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/selftests/interpreter/.gitignore
create mode 100644 tools/testing/selftests/interpreter/Makefile
create mode 100644 tools/testing/selftests/interpreter/config
create mode 100644 tools/testing/selftests/interpreter/introspection_policy_test.c
--
2.28.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox