* Re: [PATCH 10/10] Add sample notification program [ver #3]
From: Eugeniu Rosca @ 2019-06-06 21:21 UTC (permalink / raw)
To: David Howells
Cc: viro, raven, linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel, Eugeniu Rosca, Eugeniu Rosca
In-Reply-To: <155981421379.17513.13158528501056454772.stgit@warthog.procyon.org.uk>
Hi David,
On Thu, Jun 06, 2019 at 10:43:33AM +0100, David Howells wrote:
[..]
> diff --git a/samples/watch_queue/Makefile b/samples/watch_queue/Makefile
> new file mode 100644
> index 000000000000..42b694430d0f
> --- /dev/null
> +++ b/samples/watch_queue/Makefile
> @@ -0,0 +1,9 @@
> +# List of programs to build
> +hostprogs-y := watch_test
> +
> +# Tell kbuild to always build the programs
> +always := $(hostprogs-y)
> +
> +HOSTCFLAGS_watch_test.o += -I$(objtree)/usr/include
How about arm64? Do you intend to enable cross-compilation?
> +
> +HOSTLOADLIBES_watch_test += -lkeyutils
> diff --git a/samples/watch_queue/watch_test.c b/samples/watch_queue/watch_test.c
> new file mode 100644
> index 000000000000..893a5380f792
> --- /dev/null
> +++ b/samples/watch_queue/watch_test.c
[..]
> + asm ("lfence" : : : "memory" );
[..]
> + asm("mfence" ::: "memory");
FWIW, trying to cross-compile it returned:
aarch64-linux-gnu-gcc -I../../usr/include -I../../include watch_test.c -o watch_test
/tmp/ccDXYynm.s: Assembler messages:
/tmp/ccDXYynm.s:471: Error: unknown mnemonic `lfence' -- `lfence'
/tmp/ccDXYynm.s:568: Error: unknown mnemonic `mfence' -- `mfence'
<builtin>: recipe for target 'watch_test' failed
make: *** [watch_test] Error 1
--
Best Regards,
Eugeniu.
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: David Howells @ 2019-06-06 21:17 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dhowells, Casey Schaufler, Stephen Smalley, Al Viro,
Greg Kroah-Hartman, USB list, raven, Linux FS Devel, Linux API,
linux-block, keyrings, LSM List, LKML, Paul Moore
In-Reply-To: <CALCETrVuNRPgEzv-XY4M9m6sEsCiRHxPenN_MpcMYc1h26vVwQ@mail.gmail.com>
Andy Lutomirski <luto@kernel.org> wrote:
> > > You are allowing arbitrary information flow between T and W above. Who
> > > cares about notifications?
> >
> > I do. If Watched object is /dev/null no data flow is possible.
> > There are many objects on a modern Linux system for which this
> > is true. Even if it's "just a file" the existence of one path
> > for data to flow does not justify ignoring the rules for other
> > data paths.
>
> Aha!
>
> Even ignoring security, writes to things like /dev/null should
> probably not trigger notifications to people who are watching
> /dev/null. (There are probably lots of things like this: /dev/zero,
> /dev/urandom, etc.)
Even writes to /dev/null might generate access notifications; leastways,
vfs_read() will call fsnotify_access() afterwards on success.
Whether or not you can set marks on open device files is another matter.
> David, are there any notification types that have this issue in your
> patchset? If so, is there a straightforward way to fix it?
I'm not sure what issue you're referring to specifically. Do you mean whether
writes to device files generate notifications?
> Generically, it seems like maybe writes to device nodes shouldn't trigger
> notifications since, despite the fact that different openers of a device
> node share an inode, there isn't necessarily any connection between them.
With the notification types I have currently implemented, I don't even notice
any accesses to a device file unless:
(1) Someone mounts over the top of one.
(2) The access triggers an I/O error or device reset or causes the device to
be attached or detached.
(3) Wangling the device causes some other superblock event.
(4) The driver calls request_key() and that creates a new key.
> Casey, if this is fixed in general, do you have another case where the
> right to write and the right to read do not imply the right to
> communicate?
>
> > An analogy is that two processes with different UIDs can open a file,
> > but still can't signal each other.
>
> What do you mean "signal"? If two processes with different UIDs can
> open the same file for read and write, then they can communicate with
> each other in many ways. For example, one can write to the file and
> the other can read it. One can take locks and the other can read the
> lock state. They can both map it and use any number of memory access
> side channels to communicate. But, of course, they can't send each
> other signals with kill().
>
> If, however, one of these processes is using some fancy mechanism
> (inotify, dnotify, kqueue, fanotify, whatever) to watch the file, and
> the other one writes it, then it seems inconsistent to lie to the
> watching process and say that the file wasn't written because some
> security policy has decided to allow the write, allow the read, but
> suppress this particular notification. Hence my request for a real
> example: when would it make sense to do this?
Note that fanotify requires CAP_SYS_ADMIN, but inotify and dnotify do not.
dnotify is applied to an open file, so it might be usable on a chardev such as
/dev/null, say.
David
^ permalink raw reply
* Re: [PATCH 22/58] Audit: Change audit_sig_sid to audit_sig_lsm
From: Casey Schaufler @ 2019-06-06 21:06 UTC (permalink / raw)
To: Kees Cook
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds, casey
In-Reply-To: <201906061351.B12D10D5D@keescook>
On 6/6/2019 1:53 PM, Kees Cook wrote:
> On Thu, Jun 06, 2019 at 12:17:42PM -0700, Casey Schaufler wrote:
>> On 6/6/2019 11:41 AM, Kees Cook wrote:
>>> On Mon, Jun 03, 2019 at 03:23:07PM -0700, Casey Schaufler wrote:
>>>> Maybe lsm_export_is_interesting()?
>>>> I'd love to discover there's a convention I could adhere to.
>>> I'd agree "lsm_data" seems meaningless. lsm_export does seem a better
>>> name, though it has the "export is also a verb" issue. Would "lsm_context"
>>> or "lsm_ctx"?
>>> be better?
>>>
>>> then we get lsm_ctx_is_interesting() and lsm_ctx_to_secid() ?
>> Fiddling around with this led me to think "struct lsmdata"
>> would work, although maybe "struct lsmblob", in keeping with
>> the notion it is opaque. Leaving out the "_" helps with the
>> verb issue, I think. I think ctx or context is right out, as
>> secctx is the string representation, and it would really confuse
>> things.
> Ah yeah, good point on "context". Does "blob" conflict with the existing
> "blob" stuff?
I don't think so. Some people might think it a bit too cute,
but I kind of like it.
> If it's always going to be u32 data, do we want it to be
> lsm_u32 ?
At some point I would love to have the Smack data be a
struct smack_known pointer, but that's a future thing.
> Or, since it's a multiplexor, lsmmux ?
I'd rather describe what's in it than how it's used.
^ permalink raw reply
* Re: [PATCH 22/58] Audit: Change audit_sig_sid to audit_sig_lsm
From: Kees Cook @ 2019-06-06 20:53 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds
In-Reply-To: <dbafd99d-aab7-c497-fbe9-fe467b0c237a@schaufler-ca.com>
On Thu, Jun 06, 2019 at 12:17:42PM -0700, Casey Schaufler wrote:
> On 6/6/2019 11:41 AM, Kees Cook wrote:
> > On Mon, Jun 03, 2019 at 03:23:07PM -0700, Casey Schaufler wrote:
> >> Maybe lsm_export_is_interesting()?
> >> I'd love to discover there's a convention I could adhere to.
> > I'd agree "lsm_data" seems meaningless. lsm_export does seem a better
> > name, though it has the "export is also a verb" issue. Would "lsm_context"
> > or "lsm_ctx"?
> > be better?
> >
> > then we get lsm_ctx_is_interesting() and lsm_ctx_to_secid() ?
>
> Fiddling around with this led me to think "struct lsmdata"
> would work, although maybe "struct lsmblob", in keeping with
> the notion it is opaque. Leaving out the "_" helps with the
> verb issue, I think. I think ctx or context is right out, as
> secctx is the string representation, and it would really confuse
> things.
Ah yeah, good point on "context". Does "blob" conflict with the existing
"blob" stuff? If it's always going to be u32 data, do we want it to be
lsm_u32 ? Or, since it's a multiplexor, lsmmux ?
--
Kees Cook
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Andy Lutomirski @ 2019-06-06 19:54 UTC (permalink / raw)
To: Casey Schaufler
Cc: Stephen Smalley, David Howells, Al Viro, Greg Kroah-Hartman,
USB list, raven, Linux FS Devel, Linux API, linux-block, keyrings,
LSM List, LKML, Paul Moore
In-Reply-To: <07e92045-2d80-8573-4d36-643deeaff9ec@schaufler-ca.com>
On Thu, Jun 6, 2019 at 11:56 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 6/6/2019 10:16 AM, Stephen Smalley wrote:
> > On 6/6/19 12:43 PM, Casey Schaufler wrote:
> >> ...
> >> I don't agree. That is, I don't believe it is sufficient.
> >> There is no guarantee that being able to set a watch on an
> >> object implies that every process that can trigger the event
> >> can send it to you.
> >>
> >> Watcher has Smack label W
> >> Triggerer has Smack label T
> >> Watched object has Smack label O
> >>
> >> Relevant Smack rules are
> >>
> >> W O rw
> >> T O rw
> >>
> >> The watcher will be able to set the watch,
> >> the triggerer will be able to trigger the event,
> >> but there is nothing that would allow the watcher
> >> to receive the event. This is not a case of watcher
> >> reading the watched object, as the event is delivered
> >> without any action by watcher.
> >
> > You are allowing arbitrary information flow between T and W above. Who cares about notifications?
>
> I do. If Watched object is /dev/null no data flow is possible.
> There are many objects on a modern Linux system for which this
> is true. Even if it's "just a file" the existence of one path
> for data to flow does not justify ignoring the rules for other
> data paths.
Aha!
Even ignoring security, writes to things like /dev/null should
probably not trigger notifications to people who are watching
/dev/null. (There are probably lots of things like this: /dev/zero,
/dev/urandom, etc.) David, are there any notification types that have
this issue in your patchset? If so, is there a straightforward way to
fix it? Generically, it seems like maybe writes to device nodes
shouldn't trigger notifications since, despite the fact that different
openers of a device node share an inode, there isn't necessarily any
connection between them.
Casey, if this is fixed in general, do you have another case where the
right to write and the right to read do not imply the right to
communicate?
> An analogy is that two processes with different UIDs can open a file,
> but still can't signal each other.
What do you mean "signal"? If two processes with different UIDs can
open the same file for read and write, then they can communicate with
each other in many ways. For example, one can write to the file and
the other can read it. One can take locks and the other can read the
lock state. They can both map it and use any number of memory access
side channels to communicate. But, of course, they can't send each
other signals with kill().
If, however, one of these processes is using some fancy mechanism
(inotify, dnotify, kqueue, fanotify, whatever) to watch the file, and
the other one writes it, then it seems inconsistent to lie to the
watching process and say that the file wasn't written because some
security policy has decided to allow the write, allow the read, but
suppress this particular notification. Hence my request for a real
example: when would it make sense to do this?
^ permalink raw reply
* Re: [PATCH 01/10] security: Override creds in __fput() with last fputter's creds [ver #3]
From: Andy Lutomirski @ 2019-06-06 19:34 UTC (permalink / raw)
To: Casey Schaufler
Cc: Andy Lutomirski, David Howells, Al Viro, raven, Linux FS Devel,
Linux API, linux-block, keyrings, LSM List, LKML, Jann Horn
In-Reply-To: <e434a62a-d92a-c6e2-cda1-309ac99d4d5c@schaufler-ca.com>
On Thu, Jun 6, 2019 at 12:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 6/6/2019 10:18 AM, Andy Lutomirski wrote:
> > On Thu, Jun 6, 2019 at 8:06 AM David Howells <dhowells@redhat.com> wrote:
> >> Andy Lutomirski <luto@amacapital.net> wrote:
> > Casey, I think you need to state your requirement in a way that's well
> > defined, and I think you need to make a compelling case that your
> > requirement is indeed worth dictating the design of parts of the
> > kernel outside LSM.
>
> Err, no, I don't believe so. There's a whole lot more
> going on in this discussion than just what's going on
> within the LSMs. Using examples from the LSMs makes it
> easier, because their policies are better defined than
> the "legacy" policies are. The most important part of the
> discussion is about ensuring that the event mechanism
> doesn't circumvent the legacy policies. Yes, I understand
> that you don't know what that means, or has to do with
> anything.
>
>
Indeed, I do not know what you have in mind about making sure this
mechanism doesn't circumvent legacy policies. Can you elaborate?
--Andy
^ permalink raw reply
* Re: [PATCH 22/58] Audit: Change audit_sig_sid to audit_sig_lsm
From: Casey Schaufler @ 2019-06-06 19:17 UTC (permalink / raw)
To: Kees Cook
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds, casey
In-Reply-To: <201906061138.BFE4CFEE@keescook>
On 6/6/2019 11:41 AM, Kees Cook wrote:
> On Mon, Jun 03, 2019 at 03:23:07PM -0700, Casey Schaufler wrote:
>> Maybe lsm_export_is_interesting()?
>> I'd love to discover there's a convention I could adhere to.
> I'd agree "lsm_data" seems meaningless. lsm_export does seem a better
> name, though it has the "export is also a verb" issue. Would "lsm_context"
> or "lsm_ctx"?
> be better?
>
> then we get lsm_ctx_is_interesting() and lsm_ctx_to_secid() ?
Fiddling around with this led me to think "struct lsmdata"
would work, although maybe "struct lsmblob", in keeping with
the notion it is opaque. Leaving out the "_" helps with the
verb issue, I think. I think ctx or context is right out, as
secctx is the string representation, and it would really confuse
things.
^ permalink raw reply
* Re: [PATCH 05/58] LSM: Use lsm_export in the inode_getsecid hooks
From: Kees Cook @ 2019-06-06 19:11 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, Stephen Smalley
In-Reply-To: <46c5cbbb-b703-403f-96dd-d90f49d74e5e@schaufler-ca.com>
On Mon, Jun 03, 2019 at 05:29:45PM -0700, Casey Schaufler wrote:
> On 6/1/2019 6:57 PM, Kees Cook wrote:
> > On Fri, May 31, 2019 at 04:09:27PM -0700, Casey Schaufler wrote:
> >> Convert the inode_getsecid hooks to use the lsm_export
> >> structure instead of a u32 secid. There is some scaffolding
> >> involved that will be removed when security_inode_getsecid()
> >> is updated.
> > So, there are like 20 patches that all have basically identical subject
> > and changelog, but some evolve the API in subtle ways. For example,
> > in this patch, there is no mention of adding lsm_export_init(). I would
> > expect all the lsm_export infrastructure and support functions to be
> > introduced in patch 4 where struct lsm_export is initially introduced.
>
> Fair enough. I didn't introduce helpers until they were used.
> I hoped to avoid the "what is this for?" question that can
> come up when you add functions that aren't used.
True, but since we know a giant set of changes is coming, I think it's
okay. As long there's kerndoc on the helpers, it should be clear what
they're for. And the commit log can include the context for why the
helpers exist. "In later patches, we'll replace secids with lsm_context,
so we need to use foo to do bar etc"
> > Instead, various helper functions are scattered through these patches
> > and I'm left struggling to figure out where things are actually
> > changing.
>
> I think it's possible that the patches may be too small
> to contain enough context for them to be sensible. It may
> make things more obvious if I combined
>
> [PATCH 05/58] LSM: Use lsm_export in the inode_getsecid hooks
> [PATCH 20/58] LSM: Use lsm_export in security_inode_getsecid
>
> into a single patch. That would reduce the amount of scaffolding
> that has to get set up and torn down.
Yeah, that's fine. If you have to do a lot of work to split up a pair of
patches, I think that's fine to combine them. What I usually want to see
is a split of separable changes. Like, adding all the helpers: I can
look at those individually as I read the patch. Then the next patch
might swap a whole logical set of things like putting lsm_context into
the LSMs, but leaving all the interfaces alone. Then fixing the high
level things that use secids, etc.
But, really, the cover letter should cover the evolutionary steps the
series takes: that should serve as a guide for everything trying to
follow your thinking.
> The inconsistency is comes from my use of "lsm_export" for
> the name of the LSM data structure. This is something you've
> commented on elsewhere. The underscore makes the function name
> look like it has an lsm_ prefix, rather than just being the
> name of the structure. If I changed "struct lsm_export" to
> "struct lsmdata" the names:
>
> lsm_lsmdata_to_secid() and smack_secid_to_lsmdata()
> would be more consistent.
Right. Having a distinct verb in the helper name should solve all my
confusion. :)
lsm_context_to_secid() secid_to_lsm_context() smack_secid_to_lsm_export()
etc
> > Which brings me to another thing I find awkward here: I feel like an LSM
> > shouldn't need to do anything with this object: it should be opaque to
> > the LSM. The LSM infrastructure knows which LSM it has called into. Why
> > isn't this just like the other blobs?
>
> There's a lot more rework required if the lsm_export has to be
> life-cycle managed. The audit code, for example, passes them about,
> copying, storing and dropping them without a care. I'm not completely
> opposed to taking that on, but it's essentially a rewrite of the
> audit LSM handling. The SO_PEERSEC handling probably has issues as
> well. I think netlabel would be OK, but there's stuff going on elsewhere
> in the networking stack that isn't going to like anything it has to
> worry about allocating and/or freeing.
I didn't mean life-cycle managed, but rather "opaque" to LSM. I just now
tried to construct an example, and have decided it's too crazy. :) The
benefits of your current system are that they are trivially able to be
put on the stack since they're a fixed size. The down side is that each
LSM must manage its own flags, etc. I will ponder alternatives after I
see the next version of your series.
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 01/10] security: Override creds in __fput() with last fputter's creds [ver #3]
From: Casey Schaufler @ 2019-06-06 19:09 UTC (permalink / raw)
To: Andy Lutomirski, David Howells
Cc: Al Viro, raven, Linux FS Devel, Linux API, linux-block, keyrings,
LSM List, LKML, Jann Horn
In-Reply-To: <CALCETrUukxNNhbBAifxz1EADzLOvYKoh9oqqZFJteU+MMhh1ig@mail.gmail.com>
On 6/6/2019 10:18 AM, Andy Lutomirski wrote:
> On Thu, Jun 6, 2019 at 8:06 AM David Howells <dhowells@redhat.com> wrote:
>> Andy Lutomirski <luto@amacapital.net> wrote:
>>
>>>> So that the LSM can see the credentials of the last process to do an fput()
>>>> on a file object when the file object is being dismantled, do the following
>>>> steps:
>>>>
>>> I still maintain that this is a giant design error.
>> Yes, I know. This was primarily a post so that Greg could play with the USB
>> notifications stuff I added. The LSM support isn't resolved and is unchanged.
>>
>>> Can someone at least come up with a single valid use case that isn't
>>> entirely full of bugs?
>> "Entirely full of bugs"?
> I can say "hey, I have this policy that the person who triggered an
> event needs such-and-such permission, otherwise the event gets
> suppressed". But this isn't a full use case, and it's buggy. It's
> not a full use case because I haven't specified what my actual goal is
> and why this particular policy achieves my goals. And it's entirely
> full of bugs because, as this patch so nicely illustrates, it's not
> well defined who triggered the event. For example, if I exec a setuid
> process, who triggers the close? What if I send the fd to systemd
> over a socket and immediately close my copy before systemd gets (and
> ignores) the message? Or if I send it to Wayland, or to any other
> process?
>
> A file is closed when everyone is done with it. Trying to figure out
> who the last intentional user of the file was seems little better than
> random guessing. Defining a security policy based on it seems like a
> poor idea.
>
>> How would you propose I deal with Casey's requirement? I'm getting the
>> feeling you're going to nak it if I try to fulfil that and he's going to nak
>> it if I don't.
>>
> Casey, I think you need to state your requirement in a way that's well
> defined, and I think you need to make a compelling case that your
> requirement is indeed worth dictating the design of parts of the
> kernel outside LSM.
Err, no, I don't believe so. There's a whole lot more
going on in this discussion than just what's going on
within the LSMs. Using examples from the LSMs makes it
easier, because their policies are better defined than
the "legacy" policies are. The most important part of the
discussion is about ensuring that the event mechanism
doesn't circumvent the legacy policies. Yes, I understand
that you don't know what that means, or has to do with
anything.
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Casey Schaufler @ 2019-06-06 18:56 UTC (permalink / raw)
To: Stephen Smalley, David Howells
Cc: viro, Greg Kroah-Hartman, linux-usb, raven, linux-fsdevel,
linux-api, linux-block, keyrings, linux-security-module,
linux-kernel, Paul Moore, casey
In-Reply-To: <c82052e5-ca11-67b5-965e-8f828081f31c@tycho.nsa.gov>
On 6/6/2019 10:16 AM, Stephen Smalley wrote:
> On 6/6/19 12:43 PM, Casey Schaufler wrote:
>> ...
>> I don't agree. That is, I don't believe it is sufficient.
>> There is no guarantee that being able to set a watch on an
>> object implies that every process that can trigger the event
>> can send it to you.
>>
>> Watcher has Smack label W
>> Triggerer has Smack label T
>> Watched object has Smack label O
>>
>> Relevant Smack rules are
>>
>> W O rw
>> T O rw
>>
>> The watcher will be able to set the watch,
>> the triggerer will be able to trigger the event,
>> but there is nothing that would allow the watcher
>> to receive the event. This is not a case of watcher
>> reading the watched object, as the event is delivered
>> without any action by watcher.
>
> You are allowing arbitrary information flow between T and W above. Who cares about notifications?
I do. If Watched object is /dev/null no data flow is possible.
There are many objects on a modern Linux system for which this
is true. Even if it's "just a file" the existence of one path
for data to flow does not justify ignoring the rules for other
data paths.
>
> How is it different from W and T mapping the same file as a shared mapping and communicating by reading and writing the shared memory? You aren't performing a permission check directly between W and T there.
In this case there is one object O, two subjects, W and T and two accesses.
W open O
T open O
They fiddle about with the data in O.
In the event case, there are two objects, O and W, two subjects W and T, and
three accesses.
W watch O
T trigger O
T write-event W
You can't wave away the flow of data. Different objects are involved.
An analogy is that two processes with different UIDs can open a file,
but still can't signal each other. Different mechanisms have different
policies. I'm not saying that's good, but it's the context we're in.
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Andy Lutomirski @ 2019-06-06 18:51 UTC (permalink / raw)
To: Casey Schaufler
Cc: Andy Lutomirski, Stephen Smalley, David Howells, Al Viro,
Greg Kroah-Hartman, USB list, raven, Linux FS Devel, Linux API,
linux-block, keyrings, LSM List, LKML, Paul Moore
In-Reply-To: <7afe1a85-bf19-b5b4-fdf3-69d9be475dab@schaufler-ca.com>
> On Jun 6, 2019, at 11:33 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>
>> On 6/6/2019 10:11 AM, Andy Lutomirski wrote:
>>> On Thu, Jun 6, 2019 at 9:43 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> ...
>>> I don't agree. That is, I don't believe it is sufficient.
>>> There is no guarantee that being able to set a watch on an
>>> object implies that every process that can trigger the event
>>> can send it to you.
>>>
>>> Watcher has Smack label W
>>> Triggerer has Smack label T
>>> Watched object has Smack label O
>>>
>>> Relevant Smack rules are
>>>
>>> W O rw
>>> T O rw
>>>
>>> The watcher will be able to set the watch,
>>> the triggerer will be able to trigger the event,
>>> but there is nothing that would allow the watcher
>>> to receive the event. This is not a case of watcher
>>> reading the watched object, as the event is delivered
>>> without any action by watcher.
>> I think this is an example of a bogus policy that should not be
>> supported by the kernel.
>
> At this point it's pretty hard for me to care much what
> you think. You don't seem to have any insight into the
> implications of the features you're advocating, or their
> potential consequences.
>
>
Can you try to spell it out, then? A mostly or fully worked out example might help.
As Stephen said, it looks like you are considering cases where there is already a full communication channel between two processes, and you’re concerned that this new mechanism might add a side channel too. If this is wrong, can you explain how?
^ permalink raw reply
* Re: [PATCH 22/58] Audit: Change audit_sig_sid to audit_sig_lsm
From: Kees Cook @ 2019-06-06 18:41 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds
In-Reply-To: <79cc3300-450f-5263-9b81-3186f84010f5@schaufler-ca.com>
On Mon, Jun 03, 2019 at 03:23:07PM -0700, Casey Schaufler wrote:
> Maybe lsm_export_is_interesting()?
> I'd love to discover there's a convention I could adhere to.
I'd agree "lsm_data" seems meaningless. lsm_export does seem a better
name, though it has the "export is also a verb" issue. Would "lsm_context"
or "lsm_ctx"?
be better?
then we get lsm_ctx_is_interesting() and lsm_ctx_to_secid() ?
--
Kees Cook
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Casey Schaufler @ 2019-06-06 18:33 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Smalley, David Howells, Al Viro, Greg Kroah-Hartman,
USB list, raven, Linux FS Devel, Linux API, linux-block, keyrings,
LSM List, LKML, Paul Moore, casey
In-Reply-To: <CALCETrWn_C8oReKXGMXiJDOGoYWMs+jg2DWa5ZipKAceyXkx5w@mail.gmail.com>
On 6/6/2019 10:11 AM, Andy Lutomirski wrote:
> On Thu, Jun 6, 2019 at 9:43 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> ...
>> I don't agree. That is, I don't believe it is sufficient.
>> There is no guarantee that being able to set a watch on an
>> object implies that every process that can trigger the event
>> can send it to you.
>>
>> Watcher has Smack label W
>> Triggerer has Smack label T
>> Watched object has Smack label O
>>
>> Relevant Smack rules are
>>
>> W O rw
>> T O rw
>>
>> The watcher will be able to set the watch,
>> the triggerer will be able to trigger the event,
>> but there is nothing that would allow the watcher
>> to receive the event. This is not a case of watcher
>> reading the watched object, as the event is delivered
>> without any action by watcher.
> I think this is an example of a bogus policy that should not be
> supported by the kernel.
At this point it's pretty hard for me to care much what
you think. You don't seem to have any insight into the
implications of the features you're advocating, or their
potential consequences.
^ permalink raw reply
* Re: [PATCH 01/10] security: Override creds in __fput() with last fputter's creds [ver #3]
From: Andy Lutomirski @ 2019-06-06 17:18 UTC (permalink / raw)
To: David Howells
Cc: Al Viro, Casey Schaufler, raven, Linux FS Devel, Linux API,
linux-block, keyrings, LSM List, LKML, Jann Horn
In-Reply-To: <11031.1559833574@warthog.procyon.org.uk>
On Thu, Jun 6, 2019 at 8:06 AM David Howells <dhowells@redhat.com> wrote:
>
> Andy Lutomirski <luto@amacapital.net> wrote:
>
> > > So that the LSM can see the credentials of the last process to do an fput()
> > > on a file object when the file object is being dismantled, do the following
> > > steps:
> > >
> >
> > I still maintain that this is a giant design error.
>
> Yes, I know. This was primarily a post so that Greg could play with the USB
> notifications stuff I added. The LSM support isn't resolved and is unchanged.
>
> > Can someone at least come up with a single valid use case that isn't
> > entirely full of bugs?
>
> "Entirely full of bugs"?
I can say "hey, I have this policy that the person who triggered an
event needs such-and-such permission, otherwise the event gets
suppressed". But this isn't a full use case, and it's buggy. It's
not a full use case because I haven't specified what my actual goal is
and why this particular policy achieves my goals. And it's entirely
full of bugs because, as this patch so nicely illustrates, it's not
well defined who triggered the event. For example, if I exec a setuid
process, who triggers the close? What if I send the fd to systemd
over a socket and immediately close my copy before systemd gets (and
ignores) the message? Or if I send it to Wayland, or to any other
process?
A file is closed when everyone is done with it. Trying to figure out
who the last intentional user of the file was seems little better than
random guessing. Defining a security policy based on it seems like a
poor idea.
>
> How would you propose I deal with Casey's requirement? I'm getting the
> feeling you're going to nak it if I try to fulfil that and he's going to nak
> it if I don't.
>
Casey, I think you need to state your requirement in a way that's well
defined, and I think you need to make a compelling case that your
requirement is indeed worth dictating the design of parts of the
kernel outside LSM.
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Stephen Smalley @ 2019-06-06 17:16 UTC (permalink / raw)
To: Casey Schaufler, David Howells
Cc: viro, Greg Kroah-Hartman, linux-usb, raven, linux-fsdevel,
linux-api, linux-block, keyrings, linux-security-module,
linux-kernel, Paul Moore
In-Reply-To: <0eb007c5-b4a0-9384-d915-37b0e5a158bf@schaufler-ca.com>
On 6/6/19 12:43 PM, Casey Schaufler wrote:
> On 6/6/2019 7:05 AM, Stephen Smalley wrote:
>> On 6/6/19 9:16 AM, David Howells wrote:
>>> Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>>
>>> This might be easier to discuss if you can reply to:
>>>
>>> https://lore.kernel.org/lkml/5393.1559768763@warthog.procyon.org.uk/
>>>
>>> which is on the ver #2 posting of this patchset.
>>
>> Sorry for being late to the party. Not sure whether you're asking me to respond only there or both there and here to your comments below. I'll start here but can revisit if it's a problem.
>>>
>>>>> LSM support is included, but controversial:
>>>>>
>>>>> (1) The creds of the process that did the fput() that reduced the refcount
>>>>> to zero are cached in the file struct.
>>>>>
>>>>> (2) __fput() overrides the current creds with the creds from (1) whilst
>>>>> doing the cleanup, thereby making sure that the creds seen by the
>>>>> destruction notification generated by mntput() appears to come from
>>>>> the last fputter.
>>>>>
>>>>> (3) security_post_notification() is called for each queue that we might
>>>>> want to post a notification into, thereby allowing the LSM to prevent
>>>>> covert communications.
>>>>>
>>>>> (?) Do I need to add security_set_watch(), say, to rule on whether a watch
>>>>> may be set in the first place? I might need to add a variant per
>>>>> watch-type.
>>>>>
>>>>> (?) Do I really need to keep track of the process creds in which an
>>>>> implicit object destruction happened? For example, imagine you create
>>>>> an fd with fsopen()/fsmount(). It is marked to dissolve the mount it
>>>>> refers to on close unless move_mount() clears that flag. Now, imagine
>>>>> someone looking at that fd through procfs at the same time as you exit
>>>>> due to an error. The LSM sees the destruction notification come from
>>>>> the looker if they happen to do their fput() after yours.
>>>>
>>>>
>>>> I'm not in favor of this approach.
>>>
>>> Which bit? The last point? Keeping track of the process creds after an
>>> implicit object destruction.
>>
>> (1), (2), (3), and the last point.
>>
>>>> Can we check permission to the object being watched when a watch is set
>>>> (read-like access),
>>>
>>> Yes, and I need to do that. I think it's likely to require an extra hook for
>>> each entry point added because the objects are different:
>>>
>>> int security_watch_key(struct watch *watch, struct key *key);
>>> int security_watch_sb(struct watch *watch, struct path *path);
>>> int security_watch_mount(struct watch *watch, struct path *path);
>>> int security_watch_devices(struct watch *watch);
>>>
>>>> make sure every access that can trigger a notification requires a
>>>> (write-like) permission to the accessed object,
>>>
>>> "write-like permssion" for whom? The triggerer or the watcher?
>>
>> The former, i.e. the process that performed the operation that triggered the notification. Think of it as a write from the process to the accessed object, which triggers a notification (another write) on some related object (the watched object), which is then read by the watcher.
>
> We agree that the process that performed the operation that triggered
> the notification is the initial subject. Smack will treat the process
> with the watch set (in particular, its ring buffer) as the object
> being written to. SELinux, with its finer grained controls, will
> involve other things as noted above. There are other place where we
> see this, notably IP packet delivery.
>
> The implication is that the information about the triggering
> process needs to be available throughout.
>
>>
>>> There are various 'classes' of events:
>>>
>>> (1) System events (eg. hardware I/O errors, automount points expiring).
>>>
>>> (2) Direct events (eg. automounts, manual mounts, EDQUOT, key linkage).
>>>
>>> (3) Indirect events (eg. exit/close doing the last fput and causing an
>>> unmount).
>>>
>>> Class (1) are uncaused by a process, so I use init_cred for them. One could
>>> argue that the automount point expiry should perhaps take place under the
>>> creds of whoever triggered it in the first place, but we need to be careful
>>> about long-term cred pinning.
>>
>> This seems equivalent to just checking whether the watcher is allowed to get that kind of event, no other cred truly needed.
>>
>>> Class (2) the causing process must've had permission to cause them - otherwise
>>> we wouldn't have got the event.
>>
>> So we've already done a check on the causing process, and we're going to check whether the watcher can set the watch. We just need to establish the connection between the accessed object and the watched object in some manner.
>
> I don't agree. That is, I don't believe it is sufficient.
> There is no guarantee that being able to set a watch on an
> object implies that every process that can trigger the event
> can send it to you.
>
> Watcher has Smack label W
> Triggerer has Smack label T
> Watched object has Smack label O
>
> Relevant Smack rules are
>
> W O rw
> T O rw
>
> The watcher will be able to set the watch,
> the triggerer will be able to trigger the event,
> but there is nothing that would allow the watcher
> to receive the event. This is not a case of watcher
> reading the watched object, as the event is delivered
> without any action by watcher.
You are allowing arbitrary information flow between T and W above. Who
cares about notifications?
How is it different from W and T mapping the same file as a shared
mapping and communicating by reading and writing the shared memory? You
aren't performing a permission check directly between W and T there.
>
>>
>>> Class (3) is interesting since it's currently entirely cleanup events and the
>>> process may have the right to do them (close, dup2, exit, but also execve)
>>> whether the LSM thinks it should be able to cause the object to be destroyed
>>> or not.
>>>
>>> It gets more complicated than that, though: multiple processes with different
>>> security attributes can all have fds pointing to a common file object - and
>>> the last one to close carries the can as far as the LSM is concerned.
>>
>> Yes, I'd prefer to avoid that. You can't write policy that is stable and meaningful that way. This may fall under a similar situation as class (1) - all we can meaningfully do is check whether the watcher is allowed to see all such events.
>
> Back in the day when we were doing security evaluations
> the implications of "deleting" an object gave the security
> evaluators fits. UNIX/Linux files don't get deleted, they
> simply fall off the filesystem namespace when no one cares
> about them anymore. The model we used back in the day was
> that "delete" wasn't an operation that occurs on filesystem
> objects.
>
> But now you want to do something with security implications
> when the object disappears. We could say that the event does
> nothing but signal that the system has removed the watch on
> your behalf because it is now meaningless. No reason to worry
> about who dropped the last reference. We don't care about
> that from a policy viewpoint anyway.
>
>>
>>> And yet more complicated when you throw in unix sockets with partially passed
>>> fds still in their queues. That's what patch 01 is designed to try and cope
>>> with.
>>>
>>>> and make sure there is some sane way to control the relationship between the
>>>> accessed object and the watched object (write-like)?
>>>
>>> This is the trick. Keys and superblocks have object labels of their own and
>>> don't - for now - propagate their watches. With these, the watch is on the
>>> object you initially assign it to and it goes no further than that.
>>>
>>> mount_notify() is the interesting case since we want to be able to detect
>>> mount topology change events from within the vfs subtree rooted at the watched
>>> directory without having to manually put a watch on every directory in that
>>> subtree - or even just every mount object. >
>>> Or, maybe, that's what I'll have to do: make it mount_notify() can only apply
>>> to the subtree within its superblock, and the caller must call mount_notify()
>>> for every mount object it wants to monitor. That would at least ensure that
>>> the caller can, at that point, reach all those mount points.
>>
>> Would that at least make it consistent with fanotify (not that it provides a great example)?
>>
>>>> For cases where we have no object per se or at least no security
>>>> structure/label associated with it, we may have to fall back to a
>>>> coarse-grained "Can the watcher get this kind of notification in general?".
>>>
>>> Agreed - and we should probably have that anyway.
>>>
>>> David
>
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Andy Lutomirski @ 2019-06-06 17:11 UTC (permalink / raw)
To: Casey Schaufler
Cc: Stephen Smalley, David Howells, Al Viro, Greg Kroah-Hartman,
USB list, raven, Linux FS Devel, Linux API, linux-block, keyrings,
LSM List, LKML, Paul Moore
In-Reply-To: <0eb007c5-b4a0-9384-d915-37b0e5a158bf@schaufler-ca.com>
On Thu, Jun 6, 2019 at 9:43 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 6/6/2019 7:05 AM, Stephen Smalley wrote:
> > On 6/6/19 9:16 AM, David Howells wrote:
> >> Stephen Smalley <sds@tycho.nsa.gov> wrote:
> >>
> >> This might be easier to discuss if you can reply to:
> >>
> >> https://lore.kernel.org/lkml/5393.1559768763@warthog.procyon.org.uk/
> >>
> >> which is on the ver #2 posting of this patchset.
> >
> > Sorry for being late to the party. Not sure whether you're asking me to respond only there or both there and here to your comments below. I'll start here but can revisit if it's a problem.
> >>
> >>>> LSM support is included, but controversial:
> >>>>
> >>>> (1) The creds of the process that did the fput() that reduced the refcount
> >>>> to zero are cached in the file struct.
> >>>>
> >>>> (2) __fput() overrides the current creds with the creds from (1) whilst
> >>>> doing the cleanup, thereby making sure that the creds seen by the
> >>>> destruction notification generated by mntput() appears to come from
> >>>> the last fputter.
> >>>>
> >>>> (3) security_post_notification() is called for each queue that we might
> >>>> want to post a notification into, thereby allowing the LSM to prevent
> >>>> covert communications.
> >>>>
> >>>> (?) Do I need to add security_set_watch(), say, to rule on whether a watch
> >>>> may be set in the first place? I might need to add a variant per
> >>>> watch-type.
> >>>>
> >>>> (?) Do I really need to keep track of the process creds in which an
> >>>> implicit object destruction happened? For example, imagine you create
> >>>> an fd with fsopen()/fsmount(). It is marked to dissolve the mount it
> >>>> refers to on close unless move_mount() clears that flag. Now, imagine
> >>>> someone looking at that fd through procfs at the same time as you exit
> >>>> due to an error. The LSM sees the destruction notification come from
> >>>> the looker if they happen to do their fput() after yours.
> >>>
> >>>
> >>> I'm not in favor of this approach.
> >>
> >> Which bit? The last point? Keeping track of the process creds after an
> >> implicit object destruction.
> >
> > (1), (2), (3), and the last point.
> >
> >>> Can we check permission to the object being watched when a watch is set
> >>> (read-like access),
> >>
> >> Yes, and I need to do that. I think it's likely to require an extra hook for
> >> each entry point added because the objects are different:
> >>
> >> int security_watch_key(struct watch *watch, struct key *key);
> >> int security_watch_sb(struct watch *watch, struct path *path);
> >> int security_watch_mount(struct watch *watch, struct path *path);
> >> int security_watch_devices(struct watch *watch);
> >>
> >>> make sure every access that can trigger a notification requires a
> >>> (write-like) permission to the accessed object,
> >>
> >> "write-like permssion" for whom? The triggerer or the watcher?
> >
> > The former, i.e. the process that performed the operation that triggered the notification. Think of it as a write from the process to the accessed object, which triggers a notification (another write) on some related object (the watched object), which is then read by the watcher.
>
> We agree that the process that performed the operation that triggered
> the notification is the initial subject. Smack will treat the process
> with the watch set (in particular, its ring buffer) as the object
> being written to. SELinux, with its finer grained controls, will
> involve other things as noted above. There are other place where we
> see this, notably IP packet delivery.
>
> The implication is that the information about the triggering
> process needs to be available throughout.
>
> >
> >> There are various 'classes' of events:
> >>
> >> (1) System events (eg. hardware I/O errors, automount points expiring).
> >>
> >> (2) Direct events (eg. automounts, manual mounts, EDQUOT, key linkage).
> >>
> >> (3) Indirect events (eg. exit/close doing the last fput and causing an
> >> unmount).
> >>
> >> Class (1) are uncaused by a process, so I use init_cred for them. One could
> >> argue that the automount point expiry should perhaps take place under the
> >> creds of whoever triggered it in the first place, but we need to be careful
> >> about long-term cred pinning.
> >
> > This seems equivalent to just checking whether the watcher is allowed to get that kind of event, no other cred truly needed.
> >
> >> Class (2) the causing process must've had permission to cause them - otherwise
> >> we wouldn't have got the event.
> >
> > So we've already done a check on the causing process, and we're going to check whether the watcher can set the watch. We just need to establish the connection between the accessed object and the watched object in some manner.
>
> I don't agree. That is, I don't believe it is sufficient.
> There is no guarantee that being able to set a watch on an
> object implies that every process that can trigger the event
> can send it to you.
>
> Watcher has Smack label W
> Triggerer has Smack label T
> Watched object has Smack label O
>
> Relevant Smack rules are
>
> W O rw
> T O rw
>
> The watcher will be able to set the watch,
> the triggerer will be able to trigger the event,
> but there is nothing that would allow the watcher
> to receive the event. This is not a case of watcher
> reading the watched object, as the event is delivered
> without any action by watcher.
I think this is an example of a bogus policy that should not be
supported by the kernel.
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Casey Schaufler @ 2019-06-06 16:43 UTC (permalink / raw)
To: Stephen Smalley, David Howells
Cc: viro, Greg Kroah-Hartman, linux-usb, raven, linux-fsdevel,
linux-api, linux-block, keyrings, linux-security-module,
linux-kernel, Paul Moore, casey
In-Reply-To: <8382af23-548c-f162-0e82-11e308049735@tycho.nsa.gov>
On 6/6/2019 7:05 AM, Stephen Smalley wrote:
> On 6/6/19 9:16 AM, David Howells wrote:
>> Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>
>> This might be easier to discuss if you can reply to:
>>
>> https://lore.kernel.org/lkml/5393.1559768763@warthog.procyon.org.uk/
>>
>> which is on the ver #2 posting of this patchset.
>
> Sorry for being late to the party. Not sure whether you're asking me to respond only there or both there and here to your comments below. I'll start here but can revisit if it's a problem.
>>
>>>> LSM support is included, but controversial:
>>>>
>>>> (1) The creds of the process that did the fput() that reduced the refcount
>>>> to zero are cached in the file struct.
>>>>
>>>> (2) __fput() overrides the current creds with the creds from (1) whilst
>>>> doing the cleanup, thereby making sure that the creds seen by the
>>>> destruction notification generated by mntput() appears to come from
>>>> the last fputter.
>>>>
>>>> (3) security_post_notification() is called for each queue that we might
>>>> want to post a notification into, thereby allowing the LSM to prevent
>>>> covert communications.
>>>>
>>>> (?) Do I need to add security_set_watch(), say, to rule on whether a watch
>>>> may be set in the first place? I might need to add a variant per
>>>> watch-type.
>>>>
>>>> (?) Do I really need to keep track of the process creds in which an
>>>> implicit object destruction happened? For example, imagine you create
>>>> an fd with fsopen()/fsmount(). It is marked to dissolve the mount it
>>>> refers to on close unless move_mount() clears that flag. Now, imagine
>>>> someone looking at that fd through procfs at the same time as you exit
>>>> due to an error. The LSM sees the destruction notification come from
>>>> the looker if they happen to do their fput() after yours.
>>>
>>>
>>> I'm not in favor of this approach.
>>
>> Which bit? The last point? Keeping track of the process creds after an
>> implicit object destruction.
>
> (1), (2), (3), and the last point.
>
>>> Can we check permission to the object being watched when a watch is set
>>> (read-like access),
>>
>> Yes, and I need to do that. I think it's likely to require an extra hook for
>> each entry point added because the objects are different:
>>
>> int security_watch_key(struct watch *watch, struct key *key);
>> int security_watch_sb(struct watch *watch, struct path *path);
>> int security_watch_mount(struct watch *watch, struct path *path);
>> int security_watch_devices(struct watch *watch);
>>
>>> make sure every access that can trigger a notification requires a
>>> (write-like) permission to the accessed object,
>>
>> "write-like permssion" for whom? The triggerer or the watcher?
>
> The former, i.e. the process that performed the operation that triggered the notification. Think of it as a write from the process to the accessed object, which triggers a notification (another write) on some related object (the watched object), which is then read by the watcher.
We agree that the process that performed the operation that triggered
the notification is the initial subject. Smack will treat the process
with the watch set (in particular, its ring buffer) as the object
being written to. SELinux, with its finer grained controls, will
involve other things as noted above. There are other place where we
see this, notably IP packet delivery.
The implication is that the information about the triggering
process needs to be available throughout.
>
>> There are various 'classes' of events:
>>
>> (1) System events (eg. hardware I/O errors, automount points expiring).
>>
>> (2) Direct events (eg. automounts, manual mounts, EDQUOT, key linkage).
>>
>> (3) Indirect events (eg. exit/close doing the last fput and causing an
>> unmount).
>>
>> Class (1) are uncaused by a process, so I use init_cred for them. One could
>> argue that the automount point expiry should perhaps take place under the
>> creds of whoever triggered it in the first place, but we need to be careful
>> about long-term cred pinning.
>
> This seems equivalent to just checking whether the watcher is allowed to get that kind of event, no other cred truly needed.
>
>> Class (2) the causing process must've had permission to cause them - otherwise
>> we wouldn't have got the event.
>
> So we've already done a check on the causing process, and we're going to check whether the watcher can set the watch. We just need to establish the connection between the accessed object and the watched object in some manner.
I don't agree. That is, I don't believe it is sufficient.
There is no guarantee that being able to set a watch on an
object implies that every process that can trigger the event
can send it to you.
Watcher has Smack label W
Triggerer has Smack label T
Watched object has Smack label O
Relevant Smack rules are
W O rw
T O rw
The watcher will be able to set the watch,
the triggerer will be able to trigger the event,
but there is nothing that would allow the watcher
to receive the event. This is not a case of watcher
reading the watched object, as the event is delivered
without any action by watcher.
>
>> Class (3) is interesting since it's currently entirely cleanup events and the
>> process may have the right to do them (close, dup2, exit, but also execve)
>> whether the LSM thinks it should be able to cause the object to be destroyed
>> or not.
>>
>> It gets more complicated than that, though: multiple processes with different
>> security attributes can all have fds pointing to a common file object - and
>> the last one to close carries the can as far as the LSM is concerned.
>
> Yes, I'd prefer to avoid that. You can't write policy that is stable and meaningful that way. This may fall under a similar situation as class (1) - all we can meaningfully do is check whether the watcher is allowed to see all such events.
Back in the day when we were doing security evaluations
the implications of "deleting" an object gave the security
evaluators fits. UNIX/Linux files don't get deleted, they
simply fall off the filesystem namespace when no one cares
about them anymore. The model we used back in the day was
that "delete" wasn't an operation that occurs on filesystem
objects.
But now you want to do something with security implications
when the object disappears. We could say that the event does
nothing but signal that the system has removed the watch on
your behalf because it is now meaningless. No reason to worry
about who dropped the last reference. We don't care about
that from a policy viewpoint anyway.
>
>> And yet more complicated when you throw in unix sockets with partially passed
>> fds still in their queues. That's what patch 01 is designed to try and cope
>> with.
>>
>>> and make sure there is some sane way to control the relationship between the
>>> accessed object and the watched object (write-like)?
>>
>> This is the trick. Keys and superblocks have object labels of their own and
>> don't - for now - propagate their watches. With these, the watch is on the
>> object you initially assign it to and it goes no further than that.
>>
>> mount_notify() is the interesting case since we want to be able to detect
>> mount topology change events from within the vfs subtree rooted at the watched
>> directory without having to manually put a watch on every directory in that
>> subtree - or even just every mount object. >
>> Or, maybe, that's what I'll have to do: make it mount_notify() can only apply
>> to the subtree within its superblock, and the caller must call mount_notify()
>> for every mount object it wants to monitor. That would at least ensure that
>> the caller can, at that point, reach all those mount points.
>
> Would that at least make it consistent with fanotify (not that it provides a great example)?
>
>>> For cases where we have no object per se or at least no security
>>> structure/label associated with it, we may have to fall back to a
>>> coarse-grained "Can the watcher get this kind of notification in general?".
>>
>> Agreed - and we should probably have that anyway.
>>
>> David
^ permalink raw reply
* Re: [RFC PATCH 2/9] x86/sgx: Do not naturally align MAP_FIXED address
From: Jarkko Sakkinen @ 2019-06-06 15:37 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Xing, Cedric, Andy Lutomirski, Christopherson, Sean J,
Stephen Smalley, James Morris, Serge E . Hallyn, LSM List,
Paul Moore, Eric Paris, selinux@vger.kernel.org, Jethro Beekman,
Hansen, Dave, Thomas Gleixner, Linus Torvalds, LKML, X86 ML,
linux-sgx@vger.kernel.org, Andrew Morton, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, Andy Shevchenko, Svahn, Kai, Borislav Petkov,
Josh Triplett, Huang, Kai, David Rientjes, Roberts, William C,
Tricca, Philip B
In-Reply-To: <5A85C1D7-A159-437E-B42A-3F4254E07305@amacapital.net>
On Wed, Jun 05, 2019 at 01:14:04PM -0700, Andy Lutomirski wrote:
>
>
> > On Jun 5, 2019, at 8:17 AM, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> >
> >> On Tue, Jun 04, 2019 at 10:10:22PM +0000, Xing, Cedric wrote:
> >> A bit off topic here. This mmap()/mprotect() discussion reminds me a
> >> question (guess for Jarkko): Now that vma->vm_file->private_data keeps
> >> a pointer to the enclave, why do we store it again in vma->vm_private?
> >> It isn't a big deal but non-NULL vm_private does prevent mprotect()
> >> from merging adjacent VMAs.
> >
> > Same semantics as with a regular mmap i.e. you can close the file and
> > still use the mapping.
> >
> >
>
> The file should be properly refcounted — vm_file should not go away while it’s mapped.
Right, makes sense. It is easy one to change essentially just removing
internal refcount from sgx_encl and using file for the same. I'll update
this to my tree along with the changes to remove LKM/ACPI bits ASAP.
/Jarkko
^ permalink raw reply
* Re: [PATCH 09/10] usb: Add USB subsystem notifications [ver #3]
From: Greg Kroah-Hartman @ 2019-06-06 15:31 UTC (permalink / raw)
To: Alan Stern
Cc: David Howells, viro, linux-usb, raven, linux-fsdevel, linux-api,
linux-block, keyrings, linux-security-module, linux-kernel
In-Reply-To: <Pine.LNX.4.44L0.1906061051310.1641-100000@iolanthe.rowland.org>
On Thu, Jun 06, 2019 at 10:55:24AM -0400, Alan Stern wrote:
> On Thu, 6 Jun 2019, Greg Kroah-Hartman wrote:
>
> > On Thu, Jun 06, 2019 at 10:24:18AM -0400, Alan Stern wrote:
> > > On Thu, 6 Jun 2019, David Howells wrote:
> > >
> > > > Add a USB subsystem notification mechanism whereby notifications about
> > > > hardware events such as device connection, disconnection, reset and I/O
> > > > errors, can be reported to a monitoring process asynchronously.
> > >
> > > USB I/O errors covers an awfully large and vague field. Do we really
> > > want to include them? I'm doubtful.
> >
> > See the other patch on the linux-usb list that wanted to start adding
> > KOBJ_CHANGE notifications about USB "i/o errors".
>
> That patch wanted to add notifications only for enumeration failures
> (assuming you're talking about the patch from Eugeniu Rosca), not I/O
> errors in general.
Yes, sorry, I was thinking that as a "I/O failed in enumeration" :)
> > So for "severe" issues, yes, we should do this, but perhaps not for all
> > of the "normal" things we see when a device is yanked out of the system
> > and the like.
>
> Then what counts as a "severe" issue? Anything besides enumeration
> failure?
Not that I can think of at the moment, other than the other recently
added KOBJ_CHANGE issue. I'm sure we have other "hard failure" issues
in the USB stack that people will want exposed over time.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 0/2] ima/evm fixes for v5.2
From: Roberto Sassu @ 2019-06-06 15:22 UTC (permalink / raw)
To: Mimi Zohar, dmitry.kasatkin, mjg59
Cc: linux-integrity, linux-security-module, linux-doc, stable,
linux-kernel, silviu.vlasceanu
In-Reply-To: <1559832596.4278.124.camel@linux.ibm.com>
On 6/6/2019 4:49 PM, Mimi Zohar wrote:
> On Thu, 2019-06-06 at 13:43 +0200, Roberto Sassu wrote:
>> On 6/6/2019 1:26 PM, Roberto Sassu wrote:
>>> Previous versions included the patch 'ima: don't ignore INTEGRITY_UNKNOWN
>>> EVM status'. However, I realized that this patch cannot be accepted alone
>>> because IMA-Appraisal would deny access to new files created during the
>>> boot. With the current behavior, those files are accessible because they
>>> have a valid security.ima (not protected by EVM) created after the first
>>> write.
>>>
>>> A solution for this problem is to initialize EVM very early with a random
>>> key. Access to created files will be granted, even with the strict
>>> appraisal, because after the first write those files will have both
>>> security.ima and security.evm (HMAC calculated with the random key).
>>>
>>> Strict appraisal will work only if it is done with signatures until the
>>> persistent HMAC key is loaded.
>>
>> Changelog
>>
>> v2:
>> - remove patch 1/3 (evm: check hash algorithm passed to init_desc());
>> already accepted
>> - remove patch 3/3 (ima: show rules with IMA_INMASK correctly);
>> already accepted
>> - add new patch (evm: add option to set a random HMAC key at early boot)
>> - patch 2/3: modify patch description
>
> Roberto, as I tried explaining previously, this feature is not a
> simple bug fix. These patches, if upstreamed, will be upstreamed the
> normal way, during an open window. Whether they are classified as a
> bug fix has yet to be decided.
Sorry, I understood that I can claim that there is a bug. I provided a
motivation in patch 2/2.
> Please stop Cc'ing stable. If I don't Cc stable before sending the pull request, then Greg and Sasha have been really good about deciding which patches should be backported. (Please refer to the comment on "Cc'ing stable" in section "5) Select the recipients for your patch" in Documentation/process/submitting-patches.rst.)
>
> I'll review these patches, but in the future please use an appropriate patch set cover letter title in the subject line.
Ok.
Thanks
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH 01/10] security: Override creds in __fput() with last fputter's creds [ver #3]
From: David Howells @ 2019-06-06 15:06 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dhowells, viro, Casey Schaufler, raven, linux-fsdevel, linux-api,
linux-block, keyrings, linux-security-module, linux-kernel
In-Reply-To: <176F8189-3BE9-4B8C-A4D5-8915436338FB@amacapital.net>
Andy Lutomirski <luto@amacapital.net> wrote:
> > So that the LSM can see the credentials of the last process to do an fput()
> > on a file object when the file object is being dismantled, do the following
> > steps:
> >
>
> I still maintain that this is a giant design error.
Yes, I know. This was primarily a post so that Greg could play with the USB
notifications stuff I added. The LSM support isn't resolved and is unchanged.
> Can someone at least come up with a single valid use case that isn't
> entirely full of bugs?
"Entirely full of bugs"?
How would you propose I deal with Casey's requirement? I'm getting the
feeling you're going to nak it if I try to fulfil that and he's going to nak
it if I don't.
David
^ permalink raw reply
* Re: [PATCH 01/10] security: Override creds in __fput() with last fputter's creds [ver #3]
From: Andy Lutomirski @ 2019-06-06 14:57 UTC (permalink / raw)
To: David Howells
Cc: viro, Casey Schaufler, raven, linux-fsdevel, linux-api,
linux-block, keyrings, linux-security-module, linux-kernel
In-Reply-To: <155981413016.17513.10540579988392555403.stgit@warthog.procyon.org.uk>
> On Jun 6, 2019, at 2:42 AM, David Howells <dhowells@redhat.com> wrote:
>
> So that the LSM can see the credentials of the last process to do an fput()
> on a file object when the file object is being dismantled, do the following
> steps:
>
I still maintain that this is a giant design error. Can someone at least come up with a single valid use case that isn’t entirely full of bugs?
^ permalink raw reply
* Re: [PATCH 09/10] usb: Add USB subsystem notifications [ver #3]
From: Alan Stern @ 2019-06-06 14:55 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David Howells, viro, linux-usb, raven, linux-fsdevel, linux-api,
linux-block, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20190606143306.GA11294@kroah.com>
On Thu, 6 Jun 2019, Greg Kroah-Hartman wrote:
> On Thu, Jun 06, 2019 at 10:24:18AM -0400, Alan Stern wrote:
> > On Thu, 6 Jun 2019, David Howells wrote:
> >
> > > Add a USB subsystem notification mechanism whereby notifications about
> > > hardware events such as device connection, disconnection, reset and I/O
> > > errors, can be reported to a monitoring process asynchronously.
> >
> > USB I/O errors covers an awfully large and vague field. Do we really
> > want to include them? I'm doubtful.
>
> See the other patch on the linux-usb list that wanted to start adding
> KOBJ_CHANGE notifications about USB "i/o errors".
That patch wanted to add notifications only for enumeration failures
(assuming you're talking about the patch from Eugeniu Rosca), not I/O
errors in general.
> So for "severe" issues, yes, we should do this, but perhaps not for all
> of the "normal" things we see when a device is yanked out of the system
> and the like.
Then what counts as a "severe" issue? Anything besides enumeration
failure?
Alan Stern
^ permalink raw reply
* Re: [PATCH v3 0/2] ima/evm fixes for v5.2
From: Mimi Zohar @ 2019-06-06 14:49 UTC (permalink / raw)
To: Roberto Sassu, dmitry.kasatkin, mjg59
Cc: linux-integrity, linux-security-module, linux-doc, stable,
linux-kernel, silviu.vlasceanu
In-Reply-To: <3711f387-3aef-9fbb-1bb4-dded6807b033@huawei.com>
On Thu, 2019-06-06 at 13:43 +0200, Roberto Sassu wrote:
> On 6/6/2019 1:26 PM, Roberto Sassu wrote:
> > Previous versions included the patch 'ima: don't ignore INTEGRITY_UNKNOWN
> > EVM status'. However, I realized that this patch cannot be accepted alone
> > because IMA-Appraisal would deny access to new files created during the
> > boot. With the current behavior, those files are accessible because they
> > have a valid security.ima (not protected by EVM) created after the first
> > write.
> >
> > A solution for this problem is to initialize EVM very early with a random
> > key. Access to created files will be granted, even with the strict
> > appraisal, because after the first write those files will have both
> > security.ima and security.evm (HMAC calculated with the random key).
> >
> > Strict appraisal will work only if it is done with signatures until the
> > persistent HMAC key is loaded.
>
> Changelog
>
> v2:
> - remove patch 1/3 (evm: check hash algorithm passed to init_desc());
> already accepted
> - remove patch 3/3 (ima: show rules with IMA_INMASK correctly);
> already accepted
> - add new patch (evm: add option to set a random HMAC key at early boot)
> - patch 2/3: modify patch description
Roberto, as I tried explaining previously, this feature is not a
simple bug fix. These patches, if upstreamed, will be upstreamed the
normal way, during an open window. Whether they are classified as a
bug fix has yet to be decided.
Please stop Cc'ing stable. If I don't Cc stable before sending the pull request, then Greg and Sasha have been really good about deciding which patches should be backported. (Please refer to the comment on "Cc'ing stable" in section "5) Select the recipients for your patch" in Documentation/process/submitting-patches.rst.)
I'll review these patches, but in the future please use an appropriate patch set cover letter title in the subject line.
thanks,
Mimi
>
> v1:
> - remove patch 2/4 (evm: reset status in evm_inode_post_setattr()); file
> attributes cannot be set if the signature is portable and immutable
> - patch 3/4: add __ro_after_init to ima_appraise_req_evm variable
> declaration
> - patch 3/4: remove ima_appraise_req_evm kernel option and introduce
> 'enforce-evm' and 'log-evm' as possible values for ima_appraise=
> - remove patch 4/4 (ima: only audit failed appraisal verifications)
> - add new patch (ima: show rules with IMA_INMASK correctly)
>
>
> > Roberto Sassu (2):
> > evm: add option to set a random HMAC key at early boot
> > ima: add enforce-evm and log-evm modes to strictly check EVM status
> >
> > .../admin-guide/kernel-parameters.txt | 11 ++--
> > security/integrity/evm/evm.h | 10 +++-
> > security/integrity/evm/evm_crypto.c | 57 ++++++++++++++++---
> > security/integrity/evm/evm_main.c | 41 ++++++++++---
> > security/integrity/ima/ima_appraise.c | 8 +++
> > security/integrity/integrity.h | 1 +
> > 6 files changed, 106 insertions(+), 22 deletions(-)
> >
>
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Christian Brauner @ 2019-06-06 14:34 UTC (permalink / raw)
To: David Howells
Cc: viro, Greg Kroah-Hartman, Casey Schaufler, linux-usb, raven,
linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <155981411940.17513.7137844619951358374.stgit@warthog.procyon.org.uk>
On Thu, Jun 06, 2019 at 10:41:59AM +0100, David Howells wrote:
>
> Hi Al,
>
> Here's a set of patches to add a general variable-length notification queue
> concept and to add sources of events for:
>
> (1) Mount topology events, such as mounting, unmounting, mount expiry,
> mount reconfiguration.
>
> (2) Superblock events, such as R/W<->R/O changes, quota overrun and I/O
> errors (not complete yet).
>
> (3) Key/keyring events, such as creating, linking and removal of keys.
>
> (4) General device events (single common queue) including:
>
> - Block layer events, such as device errors
>
> - USB subsystem events, such as device/bus attach/remove, device
> reset, device errors.
>
> One of the reasons for this is so that we can remove the issue of processes
> having to repeatedly and regularly scan /proc/mounts, which has proven to
> be a system performance problem. To further aid this, the fsinfo() syscall
> on which this patch series depends, provides a way to access superblock and
> mount information in binary form without the need to parse /proc/mounts.
>
>
> LSM support is included, but controversial:
Apart from the LSM/security controversy here the general direction of
this patchset is pretty well received it seems.
Imho, having a notification mechanism like this is a very good thing for
userspace. So would be great if there'd be a consensus on the LSM bits.
Christian
^ 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