Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Casey Schaufler]

On 4/12/2017 9:22 AM, Djalal Harouni wrote:
> On Tue, Apr 11, 2017 at 6:43 AM, Kees Cook  wrote:
>> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>>> wrote:
 I think that would be the prudent approach. There is still
 the possibility that blob sharing (or full stacking, if you
 prefer) won't be accepted any time soon.
>>> Ok Casey! I will wait for more feedback, and if other maintainers do
>>> not object, I will convert it back to rhashtables in next iterations
>>> making sure that it should be simple to convert later to a blob
>>> sharing mechanism.
>> Would it be possible just to add a single field to task_struct if this
>> LSM is built in? I feel like rhashtables is a huge overhead when a
>> single field is all that's needed.
> Well, yes rhashtables can have an overhead especially when reclaiming
> memory back, I could not identify a way how to separate tables unless
> we use cgroups as an ID. Anyway this of course could be added in
> task_struct and updated to work like the capability security hooks
> rather than a proper LSM with its own name. But as noted in the other
> response, we may need task->security field for Yama anyway. I'm open
> to suggestion ? I may try to converge the task->security blob with
> what Casey is proposing and see! otherwise fallback to task_struct as
> a last resort!
>
> Thanks!

I can present a patch set based on my existing stacking
work that includes the move from module based memory
management to infrastructure memory management but pretty
well stops there. There will be changes to Kconfig to allow
stacking of everything except SELinux and Smack, because
that's the only combination with other conficts. Well,
there's /proc/attr, but I'd include the module specific
subdirectories, too. That would allow landlock to come in
and TOMOYO (and potentially YAMA) to use/share the task
blob. I see this as taking down a barrier rather than
otherwise pointless infrastructure change.

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Casey Schaufler]

On 4/12/2017 9:22 AM, Djalal Harouni wrote:
> On Tue, Apr 11, 2017 at 6:43 AM, Kees Cook  wrote:
>> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>>> wrote:
 I think that would be the prudent approach. There is still
 the possibility that blob sharing (or full stacking, if you
 prefer) won't be accepted any time soon.
>>> Ok Casey! I will wait for more feedback, and if other maintainers do
>>> not object, I will convert it back to rhashtables in next iterations
>>> making sure that it should be simple to convert later to a blob
>>> sharing mechanism.
>> Would it be possible just to add a single field to task_struct if this
>> LSM is built in? I feel like rhashtables is a huge overhead when a
>> single field is all that's needed.
> Well, yes rhashtables can have an overhead especially when reclaiming
> memory back, I could not identify a way how to separate tables unless
> we use cgroups as an ID. Anyway this of course could be added in
> task_struct and updated to work like the capability security hooks
> rather than a proper LSM with its own name. But as noted in the other
> response, we may need task->security field for Yama anyway. I'm open
> to suggestion ? I may try to converge the task->security blob with
> what Casey is proposing and see! otherwise fallback to task_struct as
> a last resort!
>
> Thanks!

I can present a patch set based on my existing stacking
work that includes the move from module based memory
management to infrastructure memory management but pretty
well stops there. There will be changes to Kconfig to allow
stacking of everything except SELinux and Smack, because
that's the only combination with other conficts. Well,
there's /proc/attr, but I'd include the module specific
subdirectories, too. That would allow landlock to come in
and TOMOYO (and potentially YAMA) to use/share the task
blob. I see this as taking down a barrier rather than
otherwise pointless infrastructure change.

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Djalal Harouni]

On Tue, Apr 11, 2017 at 6:43 AM, Kees Cook  wrote:
> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>> wrote:
>>> I think that would be the prudent approach. There is still
>>> the possibility that blob sharing (or full stacking, if you
>>> prefer) won't be accepted any time soon.
>>
>> Ok Casey! I will wait for more feedback, and if other maintainers do
>> not object, I will convert it back to rhashtables in next iterations
>> making sure that it should be simple to convert later to a blob
>> sharing mechanism.
>
> Would it be possible just to add a single field to task_struct if this
> LSM is built in? I feel like rhashtables is a huge overhead when a
> single field is all that's needed.

Well, yes rhashtables can have an overhead especially when reclaiming
memory back, I could not identify a way how to separate tables unless
we use cgroups as an ID. Anyway this of course could be added in
task_struct and updated to work like the capability security hooks
rather than a proper LSM with its own name. But as noted in the other
response, we may need task->security field for Yama anyway. I'm open
to suggestion ? I may try to converge the task->security blob with
what Casey is proposing and see! otherwise fallback to task_struct as
a last resort!

Thanks!

-- 
tixxdz

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Djalal Harouni]

On Tue, Apr 11, 2017 at 6:43 AM, Kees Cook  wrote:
> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>> wrote:
>>> I think that would be the prudent approach. There is still
>>> the possibility that blob sharing (or full stacking, if you
>>> prefer) won't be accepted any time soon.
>>
>> Ok Casey! I will wait for more feedback, and if other maintainers do
>> not object, I will convert it back to rhashtables in next iterations
>> making sure that it should be simple to convert later to a blob
>> sharing mechanism.
>
> Would it be possible just to add a single field to task_struct if this
> LSM is built in? I feel like rhashtables is a huge overhead when a
> single field is all that's needed.

Well, yes rhashtables can have an overhead especially when reclaiming
memory back, I could not identify a way how to separate tables unless
we use cgroups as an ID. Anyway this of course could be added in
task_struct and updated to work like the capability security hooks
rather than a proper LSM with its own name. But as noted in the other
response, we may need task->security field for Yama anyway. I'm open
to suggestion ? I may try to converge the task->security blob with
what Casey is proposing and see! otherwise fallback to task_struct as
a last resort!

Thanks!

-- 
tixxdz

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Djalal Harouni]

On Tue, Apr 11, 2017 at 9:54 PM, Casey Schaufler  wrote:
> On 4/10/2017 9:43 PM, Kees Cook wrote:
>> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>>> wrote:
 I think that would be the prudent approach. There is still
 the possibility that blob sharing (or full stacking, if you
 prefer) won't be accepted any time soon.
>>> Ok Casey! I will wait for more feedback, and if other maintainers do
>>> not object, I will convert it back to rhashtables in next iterations
>>> making sure that it should be simple to convert later to a blob
>>> sharing mechanism.
>> Would it be possible just to add a single field to task_struct if this
>> LSM is built in? I feel like rhashtables is a huge overhead when a
>> single field is all that's needed.
>
> Special casing the task_struct based on which modules
> are compiled in would work, but I'm under the impression
> that there's a strong desire to keep to one pointer for
> security module information in the major structures.
>
> The code for generalizing shared blobs isn't that hard,
> and y'all have seen it many times. It would be perfectly
> safe to convert the task, cred, inode and such blobs to
> be infrastructure managed right now. That wouldn't mean
> that all the stacking issues (e.g. audit and networking)
> would be addressed, or that all combinations of modules
> would work (i.e. no SELinux+Smack) but it would clear
> the way for this case. And Yama could use a blob if it
> wanted to.

I've been thinking about this again, so my patches did not convert
creds, inodes etc, I don't have a use case for them now. My use case
was for task->security and I re-used the simple approach. I can't
offer a solution for other blobs since I'm not familiar with their
context nor the different use cases. I checked TOMOYO and that was
easy, but I can't check all of them. I agreed that I may use
rhashtables but as Kees pointed out this may introduce overheads and
extra memory, where the task->security for this ModAutoProtect LSM
will only require an extra sizeof(unsigned long) per-task. Also again
the problem is not in this proposed Module, the problem is in modules
that can't be stacked together.

I am bringing this, since maybe after we manage to merge this, and if
Kees agree I may send another set of patches for Yama to enable the
same per-task context, this enables containers but also allows later
in systemd-logind sessions to set it where all inferiors inside the
user sessions are protected by default, not only some apps or special
desktops but for all. So I will hit the same problem again where to
put it? You said that the code that generalize the blobs isn't that
hard, but also in a previous response that it may not be accepted...
so I will try to converge the task->security blob to be more like the
infrastructure that you are proposing, then resubmit and maybe we will
enable these modules that are stackable by default.

Is this reasonable ?


Thanks!

-- 
tixxdz

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Djalal Harouni]

On Tue, Apr 11, 2017 at 9:54 PM, Casey Schaufler  wrote:
> On 4/10/2017 9:43 PM, Kees Cook wrote:
>> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>>> wrote:
 I think that would be the prudent approach. There is still
 the possibility that blob sharing (or full stacking, if you
 prefer) won't be accepted any time soon.
>>> Ok Casey! I will wait for more feedback, and if other maintainers do
>>> not object, I will convert it back to rhashtables in next iterations
>>> making sure that it should be simple to convert later to a blob
>>> sharing mechanism.
>> Would it be possible just to add a single field to task_struct if this
>> LSM is built in? I feel like rhashtables is a huge overhead when a
>> single field is all that's needed.
>
> Special casing the task_struct based on which modules
> are compiled in would work, but I'm under the impression
> that there's a strong desire to keep to one pointer for
> security module information in the major structures.
>
> The code for generalizing shared blobs isn't that hard,
> and y'all have seen it many times. It would be perfectly
> safe to convert the task, cred, inode and such blobs to
> be infrastructure managed right now. That wouldn't mean
> that all the stacking issues (e.g. audit and networking)
> would be addressed, or that all combinations of modules
> would work (i.e. no SELinux+Smack) but it would clear
> the way for this case. And Yama could use a blob if it
> wanted to.

I've been thinking about this again, so my patches did not convert
creds, inodes etc, I don't have a use case for them now. My use case
was for task->security and I re-used the simple approach. I can't
offer a solution for other blobs since I'm not familiar with their
context nor the different use cases. I checked TOMOYO and that was
easy, but I can't check all of them. I agreed that I may use
rhashtables but as Kees pointed out this may introduce overheads and
extra memory, where the task->security for this ModAutoProtect LSM
will only require an extra sizeof(unsigned long) per-task. Also again
the problem is not in this proposed Module, the problem is in modules
that can't be stacked together.

I am bringing this, since maybe after we manage to merge this, and if
Kees agree I may send another set of patches for Yama to enable the
same per-task context, this enables containers but also allows later
in systemd-logind sessions to set it where all inferiors inside the
user sessions are protected by default, not only some apps or special
desktops but for all. So I will hit the same problem again where to
put it? You said that the code that generalize the blobs isn't that
hard, but also in a previous response that it may not be accepted...
so I will try to converge the task->security blob to be more like the
infrastructure that you are proposing, then resubmit and maybe we will
enable these modules that are stackable by default.

Is this reasonable ?


Thanks!

-- 
tixxdz

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Kees Cook]

On Tue, Apr 11, 2017 at 12:54 PM, Casey Schaufler
 wrote:
> On 4/10/2017 9:43 PM, Kees Cook wrote:
>> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>>> wrote:
 I think that would be the prudent approach. There is still
 the possibility that blob sharing (or full stacking, if you
 prefer) won't be accepted any time soon.
>>> Ok Casey! I will wait for more feedback, and if other maintainers do
>>> not object, I will convert it back to rhashtables in next iterations
>>> making sure that it should be simple to convert later to a blob
>>> sharing mechanism.
>> Would it be possible just to add a single field to task_struct if this
>> LSM is built in? I feel like rhashtables is a huge overhead when a
>> single field is all that's needed.
>
> Special casing the task_struct based on which modules
> are compiled in would work, but I'm under the impression
> that there's a strong desire to keep to one pointer for
> security module information in the major structures.

Right, I meant as far as keeping this patch set unblocked by the other one...

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Kees Cook]

On Tue, Apr 11, 2017 at 12:54 PM, Casey Schaufler
 wrote:
> On 4/10/2017 9:43 PM, Kees Cook wrote:
>> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>>> wrote:
 I think that would be the prudent approach. There is still
 the possibility that blob sharing (or full stacking, if you
 prefer) won't be accepted any time soon.
>>> Ok Casey! I will wait for more feedback, and if other maintainers do
>>> not object, I will convert it back to rhashtables in next iterations
>>> making sure that it should be simple to convert later to a blob
>>> sharing mechanism.
>> Would it be possible just to add a single field to task_struct if this
>> LSM is built in? I feel like rhashtables is a huge overhead when a
>> single field is all that's needed.
>
> Special casing the task_struct based on which modules
> are compiled in would work, but I'm under the impression
> that there's a strong desire to keep to one pointer for
> security module information in the major structures.

Right, I meant as far as keeping this patch set unblocked by the other one...

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Casey Schaufler]

On 4/10/2017 9:43 PM, Kees Cook wrote:
> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>> wrote:
>>> I think that would be the prudent approach. There is still
>>> the possibility that blob sharing (or full stacking, if you
>>> prefer) won't be accepted any time soon.
>> Ok Casey! I will wait for more feedback, and if other maintainers do
>> not object, I will convert it back to rhashtables in next iterations
>> making sure that it should be simple to convert later to a blob
>> sharing mechanism.
> Would it be possible just to add a single field to task_struct if this
> LSM is built in? I feel like rhashtables is a huge overhead when a
> single field is all that's needed.

Special casing the task_struct based on which modules
are compiled in would work, but I'm under the impression
that there's a strong desire to keep to one pointer for
security module information in the major structures.

The code for generalizing shared blobs isn't that hard,
and y'all have seen it many times. It would be perfectly
safe to convert the task, cred, inode and such blobs to
be infrastructure managed right now. That wouldn't mean
that all the stacking issues (e.g. audit and networking)
would be addressed, or that all combinations of modules
would work (i.e. no SELinux+Smack) but it would clear
the way for this case. And Yama could use a blob if it
wanted to.

>
> -Kees
>

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Casey Schaufler]

On 4/10/2017 9:43 PM, Kees Cook wrote:
> On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
>> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
>> wrote:
>>> I think that would be the prudent approach. There is still
>>> the possibility that blob sharing (or full stacking, if you
>>> prefer) won't be accepted any time soon.
>> Ok Casey! I will wait for more feedback, and if other maintainers do
>> not object, I will convert it back to rhashtables in next iterations
>> making sure that it should be simple to convert later to a blob
>> sharing mechanism.
> Would it be possible just to add a single field to task_struct if this
> LSM is built in? I feel like rhashtables is a huge overhead when a
> single field is all that's needed.

Special casing the task_struct based on which modules
are compiled in would work, but I'm under the impression
that there's a strong desire to keep to one pointer for
security module information in the major structures.

The code for generalizing shared blobs isn't that hard,
and y'all have seen it many times. It would be perfectly
safe to convert the task, cred, inode and such blobs to
be infrastructure managed right now. That wouldn't mean
that all the stacking issues (e.g. audit and networking)
would be addressed, or that all combinations of modules
would work (i.e. no SELinux+Smack) but it would clear
the way for this case. And Yama could use a blob if it
wanted to.

>
> -Kees
>

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Kees Cook]

On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
> wrote:
>> I think that would be the prudent approach. There is still
>> the possibility that blob sharing (or full stacking, if you
>> prefer) won't be accepted any time soon.
>
> Ok Casey! I will wait for more feedback, and if other maintainers do
> not object, I will convert it back to rhashtables in next iterations
> making sure that it should be simple to convert later to a blob
> sharing mechanism.

Would it be possible just to add a single field to task_struct if this
LSM is built in? I feel like rhashtables is a huge overhead when a
single field is all that's needed.

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] Re: [PATCH RFC v2 1/3] LSM: Allow per LSM module per "struct task_struct" blob.

[Kees Cook]

On Mon, Apr 10, 2017 at 1:00 PM, Djalal Harouni  wrote:
> On Mon, Apr 10, 2017 at 9:26 PM, Casey Schaufler  
> wrote:
>> I think that would be the prudent approach. There is still
>> the possibility that blob sharing (or full stacking, if you
>> prefer) won't be accepted any time soon.
>
> Ok Casey! I will wait for more feedback, and if other maintainers do
> not object, I will convert it back to rhashtables in next iterations
> making sure that it should be simple to convert later to a blob
> sharing mechanism.

Would it be possible just to add a single field to task_struct if this
LSM is built in? I feel like rhashtables is a huge overhead when a
single field is all that's needed.

-Kees

-- 
Kees Cook
Pixel Security