* Re: [GIT PULL] Filesystem Information
From: Miklos Szeredi @ 2020-08-05 8:00 UTC (permalink / raw)
To: Ian Kent
Cc: David Howells, Linus Torvalds, Al Viro, Karel Zak, Jeff Layton,
Miklos Szeredi, Nicolas Dichtel, Christian Brauner,
Lennart Poettering, Linux API, linux-fsdevel, LSM, linux-kernel
In-Reply-To: <dd1a41a129cd6e8d13525a14807e6dc65b52e0bf.camel@themaw.net>
On Wed, Aug 5, 2020 at 3:33 AM Ian Kent <raven@themaw.net> wrote:
>
> On Tue, 2020-08-04 at 16:36 +0200, Miklos Szeredi wrote:
> > And notice how similar the above interface is to getxattr(), or the
> > proposed readfile(). Where has the "everything is a file"
> > philosophy
> > gone?
>
> Maybe, but that philosophy (in a roundabout way) is what's resulted
> in some of the problems we now have. Granted it's blind application
> of that philosophy rather than the philosophy itself but that is
> what happens.
Agree. What people don't seem to realize, even though there are
blindingly obvious examples, that binary interfaces like the proposed
fsinfo(2) syscall can also result in a multitude of problems at the
same time as solving some others.
There's no magic solution in API design, it's not balck and white.
We just need to strive for a good enough solution. The problem seems
to be that trying to discuss the merits of other approaches seems to
hit a brick wall. We just see repeated pull requests from David,
without any real discussion of the proposed alternatives.
>
> I get that your comments are driven by the way that philosophy should
> be applied which is more of a "if it works best doing it that way then
> do it that way, and that's usually a file".
>
> In this case there is a logical division of various types of file
> system information and the underlying suggestion is maybe it's time
> to move away from the "everything is a file" hard and fast rule,
> and get rid of some of the problems that have resulted from it.
>
> The notifications is an example, yes, the delivery mechanism is
> a "file" but the design of the queueing mechanism makes a lot of
> sense for the throughput that's going to be needed as time marches
> on. Then there's different sub-systems each with unique information
> that needs to be deliverable some other way because delivering "all"
> the information via the notification would be just plain wrong so
> a multi-faceted information delivery mechanism makes the most
> sense to allow specific targeted retrieval of individual items of
> information.
>
> But that also supposes your at least open to the idea that "maybe
> not everything should be a file".
Sure. I've learned pragmatism, although idealist at heart. And I'm
not saying all API's from David are shit. statx(2) is doing fine.
It's a simple binary interface that does its job well. Compare the
header files for statx and fsinfo, though, and maybe you'll see what
I'm getting at...
Thanks,
Miklos
^ permalink raw reply
* file metadata via fs API (was: [GIT PULL] Filesystem Information)
From: Miklos Szeredi @ 2020-08-05 8:24 UTC (permalink / raw)
To: Ian Kent
Cc: David Howells, Linus Torvalds, Al Viro, Karel Zak, Jeff Layton,
Miklos Szeredi, Nicolas Dichtel, Christian Brauner,
Lennart Poettering, Linux API, linux-fsdevel, LSM, linux-kernel
In-Reply-To: <CAJfpegu8omNZ613tLgUY7ukLV131tt7owR+JJ346Kombt79N0A@mail.gmail.com>
On Tue, Aug 4, 2020 at 4:36 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> I think we already lost that with the xattr API, that should have been
> done in a way that fits this philosophy. But given that we have "/"
> as the only special purpose char in filenames, and even repetitions
> are allowed, it's hard to think of a good way to do that. Pity.
One way this could be solved is to allow opting into an alternative
path resolution mode.
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Yes, the implementation might be somewhat tricky, but that's another
question. Also I'm pretty sure that we should be reducing the
POSIX-ness of anything below "//" to the bare minimum. No seeking,
etc....
I think this would open up some nice possibilities beyond the fsinfo thing.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH 13/17] watch_queue: Implement mount topology and attribute change notifications [ver #5]
From: Miklos Szeredi @ 2020-08-05 11:56 UTC (permalink / raw)
To: Ian Kent
Cc: David Howells, Linus Torvalds, Al Viro, Casey Schaufler,
Stephen Smalley, Nicolas Dichtel, Christian Brauner, andres,
Jeff Layton, dray, Karel Zak, keyrings, Linux API, linux-fsdevel,
LSM, linux-kernel
In-Reply-To: <013e9bb3cb1536c73a5b58c5ff000b3b00629561.camel@themaw.net>
On Wed, Aug 5, 2020 at 1:36 PM Ian Kent <raven@themaw.net> wrote:
>
> I can see in the kernel code that an error is returned if the message
> buffer is full when trying to add a message, I just can't see where
> to get it in the libmount code.
>
> That's not really a communication protocol problem.
>
> Still I need to work out how to detect it, maybe it is seen by
> the code in libmount already and I simply can't see what I need
> to do to recognise it ...
>
> So I'm stuck wanting to verify I have got everything that was
> sent and am having trouble moving on from that.
This is the commit that should add the overrun detection capability:
e7d553d69cf6 ("pipe: Add notification lossage handling")
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH 06/18] fsinfo: Add a uniquifier ID to struct mount [ver #21]
From: David Howells @ 2020-08-05 15:30 UTC (permalink / raw)
To: Miklos Szeredi
Cc: dhowells, Al Viro, Linus Torvalds, Ian Kent, Miklos Szeredi,
Christian Brauner, Jann Horn, Darrick J. Wong, Karel Zak,
Jeff Layton, Linux API, linux-fsdevel, LSM, linux-kernel
In-Reply-To: <CAJfpegtOguKOGWxv-sA_C9eSWG_3Srnj_k=oW-wSHNprCipFVg@mail.gmail.com>
Miklos Szeredi <miklos@szeredi.hu> wrote:
> idr_alloc_cyclic() seems to be a good template for doing the lower
> 32bit allocation, and we can add code to increment the high 32bit on
> wraparound.
>
> Lots of code uses idr_alloc_cyclic() so I guess it shouldn't be too
> bad in terms of memory use or performance.
It's optimised for shortness of path and trades memory for performance. It's
currently implemented using an xarray, so memory usage is dependent on the
sparseness of the tree. Each node in the tree is 576 bytes and in the worst
case, each one node will contain one mount - and then you have to backfill the
ancestry, though for lower memory costs.
Systemd makes life more interesting since it sets up a whole load of
propagations. Each mount you make may cause several others to be created, but
that would likely make the tree more efficient.
David
^ permalink raw reply
* Re: [PATCH v6 1/4] IMA: Add func to measure LSM state and policy
From: Stephen Smalley @ 2020-08-05 15:43 UTC (permalink / raw)
To: Tyler Hicks
Cc: Mimi Zohar, Lakshmi Ramasubramanian, Casey Schaufler, sashal,
James Morris, linux-integrity, SElinux list, LSM List,
linux-kernel, John Johansen
In-Reply-To: <20200805150732.GA4365@sequoia>
On 8/5/20 11:07 AM, Tyler Hicks wrote:
> On 2020-08-05 10:27:43, Stephen Smalley wrote:
>> On Wed, Aug 5, 2020 at 9:20 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> On Wed, 2020-08-05 at 09:03 -0400, Stephen Smalley wrote:
>>>> On Wed, Aug 5, 2020 at 8:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>> On Wed, 2020-08-05 at 08:46 -0400, Stephen Smalley wrote:
>>>>>> On 8/4/20 11:25 PM, Mimi Zohar wrote:
>>>>>>
>>>>>>> Hi Lakshmi,
>>>>>>>
>>>>>>> There's still a number of other patch sets needing to be reviewed
>>>>>>> before my getting to this one. The comment below is from a high level.
>>>>>>>
>>>>>>> On Tue, 2020-08-04 at 17:43 -0700, Lakshmi Ramasubramanian wrote:
>>>>>>>> Critical data structures of security modules need to be measured to
>>>>>>>> enable an attestation service to verify if the configuration and
>>>>>>>> policies for the security modules have been setup correctly and
>>>>>>>> that they haven't been tampered with at runtime. A new IMA policy is
>>>>>>>> required for handling this measurement.
>>>>>>>>
>>>>>>>> Define two new IMA policy func namely LSM_STATE and LSM_POLICY to
>>>>>>>> measure the state and the policy provided by the security modules.
>>>>>>>> Update ima_match_rules() and ima_validate_rule() to check for
>>>>>>>> the new func and ima_parse_rule() to handle the new func.
>>>>>>> I can understand wanting to measure the in kernel LSM memory state to
>>>>>>> make sure it hasn't changed, but policies are stored as files. Buffer
>>>>>>> measurements should be limited to those things that are not files.
>>>>>>>
>>>>>>> Changing how data is passed to the kernel has been happening for a
>>>>>>> while. For example, instead of passing the kernel module or kernel
>>>>>>> image in a buffer, the new syscalls - finit_module, kexec_file_load -
>>>>>>> pass an open file descriptor. Similarly, instead of loading the IMA
>>>>>>> policy data, a pathname may be provided.
>>>>>>>
>>>>>>> Pre and post security hooks already exist for reading files. Instead
>>>>>>> of adding IMA support for measuring the policy file data, update the
>>>>>>> mechanism for loading the LSM policy. Then not only will you be able
>>>>>>> to measure the policy, you'll also be able to require the policy be
>>>>>>> signed.
>>>>>> To clarify, the policy being measured by this patch series is a
>>>>>> serialized representation of the in-memory policy data structures being
>>>>>> enforced by SELinux. Not the file that was loaded. Hence, this
>>>>>> measurement would detect tampering with the in-memory policy data
>>>>>> structures after the policy has been loaded. In the case of SELinux,
>>>>>> one can read this serialized representation via /sys/fs/selinux/policy.
>>>>>> The result is not byte-for-byte identical to the policy file that was
>>>>>> loaded but can be semantically compared via sediff and other tools to
>>>>>> determine whether it is equivalent.
>>>>> Thank you for the clarification. Could the policy hash be included
>>>>> with the other critical data? Does it really need to be measured
>>>>> independently?
>>>> They were split into two separate functions because we wanted to be
>>>> able to support using different templates for them (ima-buf for the
>>>> state variables so that the measurement includes the original buffer,
>>>> which is small and relatively fixed-size, and ima-ng for the policy
>>>> because it is large and we just want to capture the hash for later
>>>> comparison against known-good). Also, the state variables are
>>>> available for measurement always from early initialization, whereas
>>>> the policy is only available for measurement once we have loaded an
>>>> initial policy.
>>> Ok, measuring the policy separately from other critical data makes
>>> sense. Instead of measuring the policy, which is large, measure the
>>> policy hash.
>> I think that was the original approach. However, I had concerns with
>> adding code to SELinux to compute a hash over the policy versus
>> leaving that to IMA's existing policy and mechanism. If that's
>> preferred I guess we can do it that way but seems less flexible and
>> duplicative.
> In AppArmor, we store the sha1 of the raw policy as the policy is
> loaded. The hash is exposed to userspace in apparmorfs. See commit
> 5ac8c355ae00 ("apparmor: allow introspecting the loaded policy pre
> internal transform").
>
> It has proved useful as a mechanism for debugging as sometimes the
> on-disk policy doesn't match the loaded policy and this can be a good
> way to check that while providing support to users. John also mentions
> checkpoint/restore in the commit message and I could certainly see how
> the policy hashes would be useful in that scenario.
>
> When thinking through how Lakshmi's series could be extended for
> AppArmor support, I was thinking that the AppArmor policy measurement
> would be a measurement of these hashes that we already have in place.
>
> Perhaps there's some general usefulness in storing/exposing an SELinux
> policy hash rather than only seeing it as duplicative property required
> this measurement series?
That would be a hash of the policy file that was last loaded via the
selinuxfs interface for loading policy, not a hash of the in-memory
policy data structures at the time of measurement (which is what this
patch series is implementing). The duplicative part is with respect to
selecting a hash algorithm and hashing the in-memory policy as part of
the SELinux code rather than just passing the policy buffer to IMA for
measurement like any other buffer. Userspace can already hash the
in-memory policy data itself by running sha256sum or whatever on
/sys/fs/selinux/policy, so we don't need to save or expose that separately.
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Lakshmi Ramasubramanian @ 2020-08-05 16:07 UTC (permalink / raw)
To: Tyler Hicks, Casey Schaufler
Cc: zohar, stephen.smalley.work, sashal, jmorris, linux-integrity,
selinux, linux-security-module, linux-kernel
In-Reply-To: <20200805154504.GB4365@sequoia>
On 8/5/20 8:45 AM, Tyler Hicks wrote:
> On 2020-08-05 08:36:40, Casey Schaufler wrote:
>> On 8/4/2020 6:14 PM, Lakshmi Ramasubramanian wrote:
>>> On 8/4/20 6:04 PM, Casey Schaufler wrote:
>>>> On 8/4/2020 5:43 PM, Lakshmi Ramasubramanian wrote:
>>>>> Critical data structures of security modules are currently not measured.
>>>>> Therefore an attestation service, for instance, would not be able to
>>>>> attest whether the security modules are always operating with the policies
>>>>> and configuration that the system administrator had setup. The policies
>>>>> and configuration for the security modules could be tampered with by
>>>>> malware by exploiting kernel vulnerabilities or modified through some
>>>>> inadvertent actions on the system. Measuring such critical data would
>>>>> enable an attestation service to better assess the state of the system.
>>>>
>>>> I still wonder why you're calling this an LSM change/feature when
>>>> all the change is in IMA and SELinux. You're not putting anything
>>>> into the LSM infrastructure, not are you using the LSM infrastructure
>>>> to achieve your ends. Sure, you *could* support other security modules
>>>> using this scheme, but you have a configuration dependency on
>>>> SELinux, so that's at best going to be messy. If you want this to
>>>> be an LSM "feature" you need to use the LSM hooking mechanism.
>>>
>>>>
>>>> I'm not objecting to the feature. It adds value. But as you've
>>>> implemented it it is either an IMA extension to SELinux, or an
>>>> SELiux extension to IMA. Could AppArmor add hooks for this without
>>>> changing the IMA code? It doesn't look like it to me.
>>>
>>> The check in IMA to allow the new IMA hook func LSM_STATE and LSM_POLICY when SELinux is enabled is just because SELinux is the only security module using these hooks now.
>>>
>>> To enable AppArmor, for instance, to use the new IMA hooks to measure state and policy would just require adding the check for CONFIG_SECURITY_APPARMOR. Other than that, there are no IMA changes needed to support AppArmor or other such security modules.
>>
>> This is exactly what I'm objecting to. What if a system has both SELinux
>> and AppArmor compiled in? What if it has both enabled?
>
> The SELinux state and policy would be measured but the AppArmor
> state/policy would be silently ignored. This isn't ideal as the IMA
> policy author would need to read the kernel code to figure out which
> LSMs are going to be measured.
Tyler - I am not sure why AppArmor state\policy would be ignored when
both SELinux and AppArmor are enabled. Could you please clarify?
When both the security modules are enabled, IMA policy validator would
look like below:
if (IS_ENABLED(CONFIG_SECURITY_SELINUX) ||
IS_ENABLED(CONFIG_SECURITY_APPARMOR)) &&
strcmp(args[0].from, "LSM_STATE") == 0)
entry->func = LSM_STATE;
Similar one for LSM_POLICY validation.
Both SELinux and AppArmor can call ima_measure_lsm_state() and
ima_measure_lsm_policy() to measure state and policy respectively.
I don't think we should go the route of defining IMA hooks per security
module (i.e., SELINUX_STATE, APPARMOR_STATE, SELINUX_POLICY, etc.)
Instead keep the hook generic that any SM could use - which is what I
have tried to address in this patch series.
>>
>>>
>>> Please see Patch 1/4
>>>
>>> + else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
>>> + strcmp(args[0].from, "LSM_STATE") == 0)
>>> + entry->func = LSM_STATE;
>>> + else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
>>> + strcmp(args[0].from, "LSM_POLICY") == 0)
>>> + entry->func = LSM_POLICY;
>>>
>>> And, if early boot measurement is needed for AppArmor the following change in IMA's Kconfig
>>>
>>> Patch 4/4
>>>
>>> +config IMA_QUEUE_EARLY_BOOT_DATA
>>> bool
>>> + depends on SECURITY_SELINUX || (IMA_MEASURE_ASYMMETRIC_KEYS && SYSTEM_TRUSTED_KEYRING)
>>> default y
>>>
>>> If you think calling this an "LSM feature" is not appropriate, please suggest a better phrase.
>>
>> In the code above you are under CONFIG_SECURITY_SELINUX.
>> I would suggest that it's an SELinux feature, so you should
>> be using SELINUX_STATE and SELINUX_POLICY, as I suggested
>> before. Just because SELinux has state and policy to measure
>> doesn't mean that a different module might not have other data,
>> such as history, that should be covered as well.
Good point - if other SMs have data besides state and policy, we can
define IMA hooks to measure that as well.
But I still think it is not the right approach to call this
SELINUX_STATE and SELINUX_POLICY - it will lead to unnecessary code
duplication in IMA as more SMs are onboarded, in my opinion. Correct me
if I am wrong.
-lakshmi
>
> In addition to SELINUX_STATE and SELINUX_POLICY, we should also consider
> the proposed LSM_STATE and LSM_POLICY func values but require an "lsm"
> rule conditional.
>
> So the current proposed rules:
>
> measure func=LSM_STATE
> measure func=LSM_POLICY
>
> Would become:
>
> measure func=LSM_STATE lsm=selinux
> measure func=LSM_POLICY lsm=selinux
>
> The following rules would be rejected:
>
> measure func=LSM_STATE
> measure func=LSM_POLICY
> measure func=LSM_STATE lsm=apparmor
> measure func=LSM_POLICY lsm=smack
>
> Of course, the apparmor and smack rules could/would be allowed when
> proper support is in place.
>
>
>>
>> I realize that IMA already has compile time dependencies to
>> determine which xattrs to measure. There's no reason that
>> the xattr list couldn't be determined at boot time, with
>> each security module providing the XATTR_NAME values it
>> uses.
>>
>>>
>>> But like I said above, with minimal change in IMA other security modules can be supported to measure STATE and POLICY data.
>>>
>>> -lakshmi
>>>
>>>
^ permalink raw reply
* Re: [PATCH v6 1/4] IMA: Add func to measure LSM state and policy
From: Mimi Zohar @ 2020-08-05 15:17 UTC (permalink / raw)
To: Stephen Smalley
Cc: Lakshmi Ramasubramanian, Casey Schaufler, Tyler Hicks, sashal,
James Morris, linux-integrity, SElinux list, LSM List,
linux-kernel
In-Reply-To: <CAEjxPJ7d1yg659OCU6diXXGqegc_jSzO4ZPhkRqQtJnRn-kC0g@mail.gmail.com>
On Wed, 2020-08-05 at 10:27 -0400, Stephen Smalley wrote:
> On Wed, Aug 5, 2020 at 9:20 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Wed, 2020-08-05 at 09:03 -0400, Stephen Smalley wrote:
> > > On Wed, Aug 5, 2020 at 8:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > > On Wed, 2020-08-05 at 08:46 -0400, Stephen Smalley wrote:
> > > > > On 8/4/20 11:25 PM, Mimi Zohar wrote:
> > > > >
> > > > > > Hi Lakshmi,
> > > > > >
> > > > > > There's still a number of other patch sets needing to be reviewed
> > > > > > before my getting to this one. The comment below is from a high level.
> > > > > >
> > > > > > On Tue, 2020-08-04 at 17:43 -0700, Lakshmi Ramasubramanian wrote:
> > > > > > > Critical data structures of security modules need to be measured to
> > > > > > > enable an attestation service to verify if the configuration and
> > > > > > > policies for the security modules have been setup correctly and
> > > > > > > that they haven't been tampered with at runtime. A new IMA policy is
> > > > > > > required for handling this measurement.
> > > > > > >
> > > > > > > Define two new IMA policy func namely LSM_STATE and LSM_POLICY to
> > > > > > > measure the state and the policy provided by the security modules.
> > > > > > > Update ima_match_rules() and ima_validate_rule() to check for
> > > > > > > the new func and ima_parse_rule() to handle the new func.
> > > > > > I can understand wanting to measure the in kernel LSM memory state to
> > > > > > make sure it hasn't changed, but policies are stored as files. Buffer
> > > > > > measurements should be limited to those things that are not files.
> > > > > >
> > > > > > Changing how data is passed to the kernel has been happening for a
> > > > > > while. For example, instead of passing the kernel module or kernel
> > > > > > image in a buffer, the new syscalls - finit_module, kexec_file_load -
> > > > > > pass an open file descriptor. Similarly, instead of loading the IMA
> > > > > > policy data, a pathname may be provided.
> > > > > >
> > > > > > Pre and post security hooks already exist for reading files. Instead
> > > > > > of adding IMA support for measuring the policy file data, update the
> > > > > > mechanism for loading the LSM policy. Then not only will you be able
> > > > > > to measure the policy, you'll also be able to require the policy be
> > > > > > signed.
> > > > >
> > > > > To clarify, the policy being measured by this patch series is a
> > > > > serialized representation of the in-memory policy data structures being
> > > > > enforced by SELinux. Not the file that was loaded. Hence, this
> > > > > measurement would detect tampering with the in-memory policy data
> > > > > structures after the policy has been loaded. In the case of SELinux,
> > > > > one can read this serialized representation via /sys/fs/selinux/policy.
> > > > > The result is not byte-for-byte identical to the policy file that was
> > > > > loaded but can be semantically compared via sediff and other tools to
> > > > > determine whether it is equivalent.
> > > >
> > > > Thank you for the clarification. Could the policy hash be included
> > > > with the other critical data? Does it really need to be measured
> > > > independently?
> > >
> > > They were split into two separate functions because we wanted to be
> > > able to support using different templates for them (ima-buf for the
> > > state variables so that the measurement includes the original buffer,
> > > which is small and relatively fixed-size, and ima-ng for the policy
> > > because it is large and we just want to capture the hash for later
> > > comparison against known-good). Also, the state variables are
> > > available for measurement always from early initialization, whereas
> > > the policy is only available for measurement once we have loaded an
> > > initial policy.
> >
> > Ok, measuring the policy separately from other critical data makes
> > sense. Instead of measuring the policy, which is large, measure the
> > policy hash.
>
> I think that was the original approach. However, I had concerns with
> adding code to SELinux to compute a hash over the policy versus
> leaving that to IMA's existing policy and mechanism. If that's
> preferred I guess we can do it that way but seems less flexible and
> duplicative.
Whether IMA or SELinux calculates the in memory policy hash, it should
not impact the original purpose of this patch set - measuring critical
state. It's unclear whether this patch set needs to be limited to LSM
critical state.
Measuring the in memory policy, if needed, should be a separate patch
set.
Mimi
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Casey Schaufler @ 2020-08-05 15:36 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, zohar, stephen.smalley.work
Cc: tyhicks, sashal, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel, Casey Schaufler
In-Reply-To: <50587a3e-bcb5-c68e-c16c-41baf68b4d4a@linux.microsoft.com>
On 8/4/2020 6:14 PM, Lakshmi Ramasubramanian wrote:
> On 8/4/20 6:04 PM, Casey Schaufler wrote:
>> On 8/4/2020 5:43 PM, Lakshmi Ramasubramanian wrote:
>>> Critical data structures of security modules are currently not measured.
>>> Therefore an attestation service, for instance, would not be able to
>>> attest whether the security modules are always operating with the policies
>>> and configuration that the system administrator had setup. The policies
>>> and configuration for the security modules could be tampered with by
>>> malware by exploiting kernel vulnerabilities or modified through some
>>> inadvertent actions on the system. Measuring such critical data would
>>> enable an attestation service to better assess the state of the system.
>>
>> I still wonder why you're calling this an LSM change/feature when
>> all the change is in IMA and SELinux. You're not putting anything
>> into the LSM infrastructure, not are you using the LSM infrastructure
>> to achieve your ends. Sure, you *could* support other security modules
>> using this scheme, but you have a configuration dependency on
>> SELinux, so that's at best going to be messy. If you want this to
>> be an LSM "feature" you need to use the LSM hooking mechanism.
>
>>
>> I'm not objecting to the feature. It adds value. But as you've
>> implemented it it is either an IMA extension to SELinux, or an
>> SELiux extension to IMA. Could AppArmor add hooks for this without
>> changing the IMA code? It doesn't look like it to me.
>
> The check in IMA to allow the new IMA hook func LSM_STATE and LSM_POLICY when SELinux is enabled is just because SELinux is the only security module using these hooks now.
>
> To enable AppArmor, for instance, to use the new IMA hooks to measure state and policy would just require adding the check for CONFIG_SECURITY_APPARMOR. Other than that, there are no IMA changes needed to support AppArmor or other such security modules.
This is exactly what I'm objecting to. What if a system has both SELinux
and AppArmor compiled in? What if it has both enabled?
>
> Please see Patch 1/4
>
> + else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
> + strcmp(args[0].from, "LSM_STATE") == 0)
> + entry->func = LSM_STATE;
> + else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
> + strcmp(args[0].from, "LSM_POLICY") == 0)
> + entry->func = LSM_POLICY;
>
> And, if early boot measurement is needed for AppArmor the following change in IMA's Kconfig
>
> Patch 4/4
>
> +config IMA_QUEUE_EARLY_BOOT_DATA
> bool
> + depends on SECURITY_SELINUX || (IMA_MEASURE_ASYMMETRIC_KEYS && SYSTEM_TRUSTED_KEYRING)
> default y
>
> If you think calling this an "LSM feature" is not appropriate, please suggest a better phrase.
In the code above you are under CONFIG_SECURITY_SELINUX.
I would suggest that it's an SELinux feature, so you should
be using SELINUX_STATE and SELINUX_POLICY, as I suggested
before. Just because SELinux has state and policy to measure
doesn't mean that a different module might not have other data,
such as history, that should be covered as well.
I realize that IMA already has compile time dependencies to
determine which xattrs to measure. There's no reason that
the xattr list couldn't be determined at boot time, with
each security module providing the XATTR_NAME values it
uses.
>
> But like I said above, with minimal change in IMA other security modules can be supported to measure STATE and POLICY data.
>
> -lakshmi
>
>
^ permalink raw reply
* Re: [PATCH v6 1/4] IMA: Add func to measure LSM state and policy
From: John Johansen @ 2020-08-05 16:45 UTC (permalink / raw)
To: Stephen Smalley, Tyler Hicks
Cc: Mimi Zohar, Lakshmi Ramasubramanian, Casey Schaufler, sashal,
James Morris, linux-integrity, SElinux list, LSM List,
linux-kernel
In-Reply-To: <39390a53-51df-12f0-5451-e677ccca581a@gmail.com>
On 8/5/20 8:43 AM, Stephen Smalley wrote:
> On 8/5/20 11:07 AM, Tyler Hicks wrote:
>
>> On 2020-08-05 10:27:43, Stephen Smalley wrote:
>>> On Wed, Aug 5, 2020 at 9:20 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>> On Wed, 2020-08-05 at 09:03 -0400, Stephen Smalley wrote:
>>>>> On Wed, Aug 5, 2020 at 8:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>>> On Wed, 2020-08-05 at 08:46 -0400, Stephen Smalley wrote:
>>>>>>> On 8/4/20 11:25 PM, Mimi Zohar wrote:
>>>>>>>
>>>>>>>> Hi Lakshmi,
>>>>>>>>
>>>>>>>> There's still a number of other patch sets needing to be reviewed
>>>>>>>> before my getting to this one. The comment below is from a high level.
>>>>>>>>
>>>>>>>> On Tue, 2020-08-04 at 17:43 -0700, Lakshmi Ramasubramanian wrote:
>>>>>>>>> Critical data structures of security modules need to be measured to
>>>>>>>>> enable an attestation service to verify if the configuration and
>>>>>>>>> policies for the security modules have been setup correctly and
>>>>>>>>> that they haven't been tampered with at runtime. A new IMA policy is
>>>>>>>>> required for handling this measurement.
>>>>>>>>>
>>>>>>>>> Define two new IMA policy func namely LSM_STATE and LSM_POLICY to
>>>>>>>>> measure the state and the policy provided by the security modules.
>>>>>>>>> Update ima_match_rules() and ima_validate_rule() to check for
>>>>>>>>> the new func and ima_parse_rule() to handle the new func.
>>>>>>>> I can understand wanting to measure the in kernel LSM memory state to
>>>>>>>> make sure it hasn't changed, but policies are stored as files. Buffer
>>>>>>>> measurements should be limited to those things that are not files.
>>>>>>>>
>>>>>>>> Changing how data is passed to the kernel has been happening for a
>>>>>>>> while. For example, instead of passing the kernel module or kernel
>>>>>>>> image in a buffer, the new syscalls - finit_module, kexec_file_load -
>>>>>>>> pass an open file descriptor. Similarly, instead of loading the IMA
>>>>>>>> policy data, a pathname may be provided.
>>>>>>>>
>>>>>>>> Pre and post security hooks already exist for reading files. Instead
>>>>>>>> of adding IMA support for measuring the policy file data, update the
>>>>>>>> mechanism for loading the LSM policy. Then not only will you be able
>>>>>>>> to measure the policy, you'll also be able to require the policy be
>>>>>>>> signed.
>>>>>>> To clarify, the policy being measured by this patch series is a
>>>>>>> serialized representation of the in-memory policy data structures being
>>>>>>> enforced by SELinux. Not the file that was loaded. Hence, this
>>>>>>> measurement would detect tampering with the in-memory policy data
>>>>>>> structures after the policy has been loaded. In the case of SELinux,
>>>>>>> one can read this serialized representation via /sys/fs/selinux/policy.
>>>>>>> The result is not byte-for-byte identical to the policy file that was
>>>>>>> loaded but can be semantically compared via sediff and other tools to
>>>>>>> determine whether it is equivalent.
>>>>>> Thank you for the clarification. Could the policy hash be included
>>>>>> with the other critical data? Does it really need to be measured
>>>>>> independently?
>>>>> They were split into two separate functions because we wanted to be
>>>>> able to support using different templates for them (ima-buf for the
>>>>> state variables so that the measurement includes the original buffer,
>>>>> which is small and relatively fixed-size, and ima-ng for the policy
>>>>> because it is large and we just want to capture the hash for later
>>>>> comparison against known-good). Also, the state variables are
>>>>> available for measurement always from early initialization, whereas
>>>>> the policy is only available for measurement once we have loaded an
>>>>> initial policy.
>>>> Ok, measuring the policy separately from other critical data makes
>>>> sense. Instead of measuring the policy, which is large, measure the
>>>> policy hash.
>>> I think that was the original approach. However, I had concerns with
>>> adding code to SELinux to compute a hash over the policy versus
>>> leaving that to IMA's existing policy and mechanism. If that's
>>> preferred I guess we can do it that way but seems less flexible and
>>> duplicative.
>> In AppArmor, we store the sha1 of the raw policy as the policy is
>> loaded. The hash is exposed to userspace in apparmorfs. See commit
>> 5ac8c355ae00 ("apparmor: allow introspecting the loaded policy pre
>> internal transform").
>>
>> It has proved useful as a mechanism for debugging as sometimes the
>> on-disk policy doesn't match the loaded policy and this can be a good
>> way to check that while providing support to users. John also mentions
>> checkpoint/restore in the commit message and I could certainly see how
>> the policy hashes would be useful in that scenario.
>>
>> When thinking through how Lakshmi's series could be extended for
>> AppArmor support, I was thinking that the AppArmor policy measurement
>> would be a measurement of these hashes that we already have in place.
>>
>> Perhaps there's some general usefulness in storing/exposing an SELinux
>> policy hash rather than only seeing it as duplicative property required
>> this measurement series?
>
> That would be a hash of the policy file that was last loaded via the selinuxfs interface for loading policy, not a hash of the in-memory policy data structures at the time of measurement (which is what this patch series is implementing). The duplicative part is with respect to selecting a hash algorithm and hashing the in-memory policy as part of the SELinux code rather than just passing the policy buffer to IMA for measurement like any other buffer. Userspace can already hash the in-memory policy data itself by running sha256sum or whatever on /sys/fs/selinux/policy, so we don't need to save or expose that separately.
>
>
yeah apparmor exposes full loaded policy data that userspace could hash independently too, the hashing done by the kernel just reduces the amount of data that userspace has to suck down if they trust the kernel to do the hash. Those hashes are also used by apparmor internally for the first part of a dedup check so exposing them cost very little.
The hashing of the in-memory data structures and variables is something we are not currently doing. If we were to do it hashing in-memory apparmor policy would be quite involved and that would be something I would rather have LSMs export an interface for rather than having IMA poke directly at the data structures (ie. apparmor specific code in apparmor).
As for computing a measurement based on the hash instead of the in-memory policy, while quicker that would not detect memory corruption/attacks that manage to modify policy via writing kernel memory. Whether that type of measurement is sufficient depends on what you are trying to achieve.
^ permalink raw reply
* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: James Morris @ 2020-08-05 16:59 UTC (permalink / raw)
To: James Bottomley
Cc: Deven Bowers, Pavel Machek, Sasha Levin, snitzer, zohar, dm-devel,
tyhicks, agk, paul, corbet, nramas, serge, pasha.tatashin, jannh,
linux-block, viro, axboe, mdsakib, linux-kernel, eparis,
linux-security-module, linux-audit, linux-fsdevel,
linux-integrity, jaskarankhurana
In-Reply-To: <1596639689.3457.17.camel@HansenPartnership.com>
On Wed, 5 Aug 2020, James Bottomley wrote:
> I'll leave Mimi to answer, but really this is exactly the question that
> should have been asked before writing IPE. However, since we have the
> cart before the horse, let me break the above down into two specific
> questions.
The question is valid and it was asked. We decided to first prototype what
we needed and then evaluate if it should be integrated with IMA. We
discussed this plan in person with Mimi (at LSS-NA in 2019), and presented
a more mature version of IPE to LSS-NA in 2020, with the expectation that
such a discussion may come up (it did not).
These patches are still part of this process and 'RFC' status.
> 1. Could we implement IPE in IMA (as in would extensions to IMA cover
> everything). I think the answers above indicate this is a "yes".
It could be done, if needed.
> 2. Should we extend IMA to implement it? This is really whether from a
> usability standpoint two seperate LSMs would make sense to cover the
> different use cases.
One issue here is that IMA is fundamentally a measurement & appraisal
scheme which has been extended to include integrity enforcement. IPE was
designed from scratch to only perform integrity enforcement. As such, it
is a cleaner design -- "do one thing and do it well" is a good design
pattern.
In our use-case, we utilize _both_ IMA and IPE, for attestation and code
integrity respectively. It is useful to be able to separate these
concepts. They really are different:
- Code integrity enforcement ensures that code running locally is of known
provenance and has not been modified prior to execution.
- Attestation is about measuring the health of a system and having that
measurement validated by a remote system. (Local attestation is useless).
I'm not sure there is value in continuing to shoe-horn both of these into
IMA.
> I've got to say the least attractive thing
> about separation is the fact that you now both have a policy parser.
> You've tried to differentiate yours by making it more Kconfig
> based, but policy has a way of becoming user space supplied because
> the distros hate config options, so I think you're going to end up
> with a policy parser very like IMAs.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v6 1/4] IMA: Add func to measure LSM state and policy
From: Mimi Zohar @ 2020-08-05 12:56 UTC (permalink / raw)
To: Stephen Smalley, Lakshmi Ramasubramanian, casey
Cc: tyhicks, sashal, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
In-Reply-To: <f88bf25e-37ef-7f00-6162-215838961bb0@gmail.com>
On Wed, 2020-08-05 at 08:46 -0400, Stephen Smalley wrote:
> On 8/4/20 11:25 PM, Mimi Zohar wrote:
>
> > Hi Lakshmi,
> >
> > There's still a number of other patch sets needing to be reviewed
> > before my getting to this one. The comment below is from a high level.
> >
> > On Tue, 2020-08-04 at 17:43 -0700, Lakshmi Ramasubramanian wrote:
> > > Critical data structures of security modules need to be measured to
> > > enable an attestation service to verify if the configuration and
> > > policies for the security modules have been setup correctly and
> > > that they haven't been tampered with at runtime. A new IMA policy is
> > > required for handling this measurement.
> > >
> > > Define two new IMA policy func namely LSM_STATE and LSM_POLICY to
> > > measure the state and the policy provided by the security modules.
> > > Update ima_match_rules() and ima_validate_rule() to check for
> > > the new func and ima_parse_rule() to handle the new func.
> > I can understand wanting to measure the in kernel LSM memory state to
> > make sure it hasn't changed, but policies are stored as files. Buffer
> > measurements should be limited to those things that are not files.
> >
> > Changing how data is passed to the kernel has been happening for a
> > while. For example, instead of passing the kernel module or kernel
> > image in a buffer, the new syscalls - finit_module, kexec_file_load -
> > pass an open file descriptor. Similarly, instead of loading the IMA
> > policy data, a pathname may be provided.
> >
> > Pre and post security hooks already exist for reading files. Instead
> > of adding IMA support for measuring the policy file data, update the
> > mechanism for loading the LSM policy. Then not only will you be able
> > to measure the policy, you'll also be able to require the policy be
> > signed.
>
> To clarify, the policy being measured by this patch series is a
> serialized representation of the in-memory policy data structures being
> enforced by SELinux. Not the file that was loaded. Hence, this
> measurement would detect tampering with the in-memory policy data
> structures after the policy has been loaded. In the case of SELinux,
> one can read this serialized representation via /sys/fs/selinux/policy.
> The result is not byte-for-byte identical to the policy file that was
> loaded but can be semantically compared via sediff and other tools to
> determine whether it is equivalent.
Thank you for the clarification. Could the policy hash be included
with the other critical data? Does it really need to be measured
independently?
Mimi
^ permalink raw reply
* Re: [PATCH 08/18] fsinfo: Allow mount topology and propagation info to be retrieved [ver #21]
From: Miklos Szeredi @ 2020-08-05 17:19 UTC (permalink / raw)
To: David Howells
Cc: Al Viro, Linus Torvalds, Ian Kent, Miklos Szeredi,
Christian Brauner, Jann Horn, Darrick J. Wong, Karel Zak,
Jeff Layton, Linux API, linux-fsdevel, LSM, linux-kernel
In-Reply-To: <2316806.1596641851@warthog.procyon.org.uk>
On Wed, Aug 5, 2020 at 5:37 PM David Howells <dhowells@redhat.com> wrote:
>
> Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> > > + __u32 shared_group_id; /* Shared: mount group ID */
> > > + __u32 dependent_source_id; /* Dependent: source mount group ID */
> > > + __u32 dependent_clone_of_id; /* Dependent: ID of mount this was cloned from */
> >
> > Another set of ID's that are currently 32bit *internally* but that doesn't
> > mean they will always be 32 bit.
> >
> > And that last one (apart from "slave" being obfuscated)
>
> I had "slave" in there. It got objected to. See
> Documentation/process/coding-style.rst section 4.
>
> > is simply incorrect. It has nothing to do with cloning. It's the "ID of
> > the closest peer group in the propagation chain that has a representative
> > mount in the current root".
>
> You appear to be in disagreement with others that I've asked.
Read the code.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Mimi Zohar @ 2020-08-05 17:03 UTC (permalink / raw)
To: Tyler Hicks, Casey Schaufler
Cc: Lakshmi Ramasubramanian, stephen.smalley.work, sashal, jmorris,
linux-integrity, selinux, linux-security-module, linux-kernel
In-Reply-To: <20200805154504.GB4365@sequoia>
On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote:
> In addition to SELINUX_STATE and SELINUX_POLICY, we should also consider
> the proposed LSM_STATE and LSM_POLICY func values but require an "lsm"
> rule conditional.
>
> So the current proposed rules:
>
> measure func=LSM_STATE
> measure func=LSM_POLICY
>
> Would become:
>
> measure func=LSM_STATE lsm=selinux
> measure func=LSM_POLICY lsm=selinux
>
> The following rules would be rejected:
>
> measure func=LSM_STATE
> measure func=LSM_POLICY
> measure func=LSM_STATE lsm=apparmor
> measure func=LSM_POLICY lsm=smack
Kees is cleaning up the firmware code which differentiated between how
firmware was loaded. There will be a single firmware enumeration.
Similarly, the new IMA hook to measure critical state may be placed in
multiple places. Is there really a need from a policy perspective for
differentiating the source of the critical state being measurind? The
data being measured should include some identifying information.
I think moving away from the idea that measuring "critical" data should
be limited to LSMs, will clarify this.
Mimi
^ permalink raw reply
* Re: [PATCH 06/18] fsinfo: Add a uniquifier ID to struct mount [ver #21]
From: Miklos Szeredi @ 2020-08-05 14:46 UTC (permalink / raw)
To: David Howells
Cc: Al Viro, Linus Torvalds, Ian Kent, Miklos Szeredi,
Christian Brauner, Jann Horn, Darrick J. Wong, Karel Zak,
Jeff Layton, Linux API, linux-fsdevel, LSM, linux-kernel
In-Reply-To: <2306029.1596636828@warthog.procyon.org.uk>
On Wed, Aug 5, 2020 at 4:14 PM David Howells <dhowells@redhat.com> wrote:
> However, looking up that identifier requires some sort of structure for doing
> this and it's kind of worst case for the IDR tree as the keys are gradually
> going to spread out, causing it to eat more memory. It may be a tradeoff
> worth making, and the memory consumption might not be that bad - or we could
> use some other data structure such as an rbtree.
idr_alloc_cyclic() seems to be a good template for doing the lower
32bit allocation, and we can add code to increment the high 32bit on
wraparound.
Lots of code uses idr_alloc_cyclic() so I guess it shouldn't be too
bad in terms of memory use or performance.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Casey Schaufler @ 2020-08-05 17:31 UTC (permalink / raw)
To: Tyler Hicks, Lakshmi Ramasubramanian
Cc: zohar, stephen.smalley.work, sashal, jmorris, linux-integrity,
selinux, linux-security-module, linux-kernel, Casey Schaufler
In-Reply-To: <20200805163243.GD4365@sequoia>
On 8/5/2020 9:32 AM, Tyler Hicks wrote:
> On 2020-08-05 09:21:24, Lakshmi Ramasubramanian wrote:
>> On 8/5/20 9:14 AM, Tyler Hicks wrote:
>>> On 2020-08-05 09:07:48, Lakshmi Ramasubramanian wrote:
>>>> On 8/5/20 8:45 AM, Tyler Hicks wrote:
>>>>> On 2020-08-05 08:36:40, Casey Schaufler wrote:
>>>>>> On 8/4/2020 6:14 PM, Lakshmi Ramasubramanian wrote:
>>>>>>> On 8/4/20 6:04 PM, Casey Schaufler wrote:
>>>>>>>> On 8/4/2020 5:43 PM, Lakshmi Ramasubramanian wrote:
>>>>>>>>> Critical data structures of security modules are currently not measured.
>>>>>>>>> Therefore an attestation service, for instance, would not be able to
>>>>>>>>> attest whether the security modules are always operating with the policies
>>>>>>>>> and configuration that the system administrator had setup. The policies
>>>>>>>>> and configuration for the security modules could be tampered with by
>>>>>>>>> malware by exploiting kernel vulnerabilities or modified through some
>>>>>>>>> inadvertent actions on the system. Measuring such critical data would
>>>>>>>>> enable an attestation service to better assess the state of the system.
>>>>>>>> I still wonder why you're calling this an LSM change/feature when
>>>>>>>> all the change is in IMA and SELinux. You're not putting anything
>>>>>>>> into the LSM infrastructure, not are you using the LSM infrastructure
>>>>>>>> to achieve your ends. Sure, you *could* support other security modules
>>>>>>>> using this scheme, but you have a configuration dependency on
>>>>>>>> SELinux, so that's at best going to be messy. If you want this to
>>>>>>>> be an LSM "feature" you need to use the LSM hooking mechanism.
>>>>>>>> I'm not objecting to the feature. It adds value. But as you've
>>>>>>>> implemented it it is either an IMA extension to SELinux, or an
>>>>>>>> SELiux extension to IMA. Could AppArmor add hooks for this without
>>>>>>>> changing the IMA code? It doesn't look like it to me.
>>>>>>> The check in IMA to allow the new IMA hook func LSM_STATE and LSM_POLICY when SELinux is enabled is just because SELinux is the only security module using these hooks now.
>>>>>>>
>>>>>>> To enable AppArmor, for instance, to use the new IMA hooks to measure state and policy would just require adding the check for CONFIG_SECURITY_APPARMOR. Other than that, there are no IMA changes needed to support AppArmor or other such security modules.
>>>>>> This is exactly what I'm objecting to. What if a system has both SELinux
>>>>>> and AppArmor compiled in? What if it has both enabled?
>>>>> The SELinux state and policy would be measured but the AppArmor
>>>>> state/policy would be silently ignored. This isn't ideal as the IMA
>>>>> policy author would need to read the kernel code to figure out which
>>>>> LSMs are going to be measured.
>>>> Tyler - I am not sure why AppArmor state\policy would be ignored when both
>>>> SELinux and AppArmor are enabled. Could you please clarify?
>>> I think Casey is talking about now (when AppArmor is not supported by
>>> this feature) and you're talking about the future (when AppArmor may be
>>> supported by this feature).
>> Got it - thanks for clarifying.
>>
>> But with the current code if SELinux is enabled on the system, but AppArmor
>> is not and the IMA policy contains "measure func=LSM_STATE" then the policy
>> will be rejected as "-EINVAL".
> The AppArmor policy load? Yes, the load will fail.
>
> What Casey is talking about is when SELinux and AppArmor are enabled in
> the kernel config but AppArmor is active. This scenario is true in
> distro kernels such as Ubuntu's kernel:
>
> $ grep -e CONFIG_SECURITY_SELINUX= -e CONFIG_SECURITY_APPARMOR= /boot/config-5.4.0-42-generic
> CONFIG_SECURITY_SELINUX=y
> CONFIG_SECURITY_APPARMOR=y
> $ cat /sys/kernel/security/lsm
> lockdown,capability,yama,apparmor
Yes. This is one reason that a compile time check is inappropriate.
You also have the case with SELinux where you can unload the module.
It's deprecated, but still possible.
If you boot with SELinux compiled in, but with security=none on
the boot line you also have a case where the compile time check is
inappropriate.
> Casey also likely has LSM stacking in mind here where SELinux and
> AppArmor could both be active at the same time but the LSM stacking
> series is not yet applied.
It isn't in Linus' kernel, but is available in Ubuntu.
The audit/IMA rule selection get hairy in the multiple
security module case, but you don't need to add that
for the proposed scheme to be flawed.
>
> Tyler
>
>> So the policy author would get a feedback even now.
>> Correct me if I am wrong.
>>
>> -lakshmi
^ permalink raw reply
* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: Mimi Zohar @ 2020-08-05 18:15 UTC (permalink / raw)
To: James Morris, James Bottomley
Cc: Deven Bowers, Pavel Machek, Sasha Levin, snitzer, dm-devel,
tyhicks, agk, paul, corbet, nramas, serge, pasha.tatashin, jannh,
linux-block, viro, axboe, mdsakib, linux-kernel, eparis,
linux-security-module, linux-audit, linux-fsdevel,
linux-integrity, jaskarankhurana
In-Reply-To: <alpine.LRH.2.21.2008050934060.28225@namei.org>
On Wed, 2020-08-05 at 09:59 -0700, James Morris wrote:
> On Wed, 5 Aug 2020, James Bottomley wrote:
>
> > I'll leave Mimi to answer, but really this is exactly the question that
> > should have been asked before writing IPE. However, since we have the
> > cart before the horse, let me break the above down into two specific
> > questions.
>
> The question is valid and it was asked. We decided to first prototype what
> we needed and then evaluate if it should be integrated with IMA. We
> discussed this plan in person with Mimi (at LSS-NA in 2019), and presented
> a more mature version of IPE to LSS-NA in 2020, with the expectation that
> such a discussion may come up (it did not).
When we first spoke the concepts weren't fully formulated, at least to
me.
>
> These patches are still part of this process and 'RFC' status.
>
> > 1. Could we implement IPE in IMA (as in would extensions to IMA cover
> > everything). I think the answers above indicate this is a "yes".
>
> It could be done, if needed.
>
> > 2. Should we extend IMA to implement it? This is really whether from a
> > usability standpoint two seperate LSMs would make sense to cover the
> > different use cases.
>
> One issue here is that IMA is fundamentally a measurement & appraisal
> scheme which has been extended to include integrity enforcement. IPE was
> designed from scratch to only perform integrity enforcement. As such, it
> is a cleaner design -- "do one thing and do it well" is a good design
> pattern.
>
> In our use-case, we utilize _both_ IMA and IPE, for attestation and code
> integrity respectively. It is useful to be able to separate these
> concepts. They really are different:
>
> - Code integrity enforcement ensures that code running locally is of known
> provenance and has not been modified prior to execution.
>
> - Attestation is about measuring the health of a system and having that
> measurement validated by a remote system. (Local attestation is useless).
>
> I'm not sure there is value in continuing to shoe-horn both of these into
> IMA.
True, IMA was originally limited to measurement and attestation, but
most of the original EVM concepts were subsequently included in IMA.
(Remember, Reiner Sailer wrote the original IMA, which I inherited. I
was originially working on EVM code integrity.) From a naming
perspective including EVM code integrity in IMA was a mistake. My
thinking at the time was that as IMA was already calculating the file
hash, instead of re-calculating the file hash for integrity, calculate
the file hash once and re-use it for multiple things - measurement,
integrity, and audit. At the same time define a single system wide
policy.
When we first started working on IMA, EVM, trusted, and encrypted keys,
the general kernel community didn't see a need for any of it. Thus, a
lot of what was accomplished has been accomplished without the backing
of the real core filesystem people.
If block layer integrity was enough, there wouldn't have been a need
for fs-verity. Even fs-verity is limited to read only filesystems,
which makes validating file integrity so much easier. From the
beginning, we've said that fs-verity signatures should be included in
the measurement list. (I thought someone signed on to add that support
to IMA, but have not yet seen anything.)
Going forward I see a lot of what we've accomplished being incorporated
into the filesystems. When IMA will be limited to defining a system
wide policy, I'll have completed my job.
Mimi
>
> > I've got to say the least attractive thing
> > about separation is the fact that you now both have a policy parser.
> > You've tried to differentiate yours by making it more Kconfig
> > based, but policy has a way of becoming user space supplied because
> > the distros hate config options, so I think you're going to end up
> > with a policy parser very like IMAs.
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Casey Schaufler @ 2020-08-05 18:25 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, Mimi Zohar, Tyler Hicks
Cc: stephen.smalley.work, sashal, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel, Casey Schaufler
In-Reply-To: <50f00ace-8d46-01c2-bf0f-d5484aafd95c@linux.microsoft.com>
On 8/5/2020 11:08 AM, Lakshmi Ramasubramanian wrote:
> On 8/5/20 10:57 AM, Casey Schaufler wrote:
>> On 8/5/2020 10:25 AM, Lakshmi Ramasubramanian wrote:
>>> On 8/5/20 10:03 AM, Mimi Zohar wrote:
>>>> On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote:
>>>>
>>>>> In addition to SELINUX_STATE and SELINUX_POLICY, we should also consider
>>>>> the proposed LSM_STATE and LSM_POLICY func values but require an "lsm"
>>>>> rule conditional.
>>>>>
>>>>> So the current proposed rules:
>>>>>
>>>>> ? measure func=LSM_STATE
>>>>> ? measure func=LSM_POLICY
>>>>>
>>>>> Would become:
>>>>>
>>>>> ? measure func=LSM_STATE lsm=selinux
>>>>> ? measure func=LSM_POLICY lsm=selinux
>>>>>
>>>>> The following rules would be rejected:
>>>>>
>>>>> ? measure func=LSM_STATE
>>>>> ? measure func=LSM_POLICY
>>>>> ? measure func=LSM_STATE lsm=apparmor
>>>>> ? measure func=LSM_POLICY lsm=smack
>>>>
>>>> Kees is cleaning up the firmware code which differentiated between how
>>>> firmware was loaded.?? There will be a single firmware enumeration.
>>>>
>>>> Similarly, the new IMA hook to measure critical state may be placed in
>>>> multiple places.? Is there really a need from a policy perspective for
>>>> differentiating the source of the critical state being measurind??? The
>>>> data being measured should include some identifying information.
>>>
>>> Yes Mimi - SELinux is including the identifying information in the "event name" field. Please see a sample measurement of STATE and POLICY for SELinux below:
>>>
>>> 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state-1595389364:287899386 696e697469616c697a65643d313b656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574776f726b5f706565725f636f6e74726f6c733d313b6f70656e5f7065726d733d313b657874656e6465645f736f636b65745f636c6173733d313b616c776179735f636865636b5f6e6574776f726b3d303b6367726f75705f7365636c6162656c3d313b6e6e705f6e6f737569645f7472616e736974696f6e3d313b67656e66735f7365636c6162656c5f73796d6c696e6b733d303
>>>
>>> 10 f4a7...9408 ima-ng sha256:8d1d...1834 selinux-policy-hash-1595389353:863934271
>>>
>>>>
>>>> I think moving away from the idea that measuring "critical" data should
>>>> be limited to LSMs, will clarify this.
>>>>
>>>
>>> Are you suggesting that instead of calling the hooks LSM_STATE and LSM_POLICY, we should keep it more generic so that it can be utilized by any subsystem to measure their "critical data"?
>>
>> Policy, state, history or whim, it should be up to the security module
>> to determine what data it cares about, and how it should be measured.
>> Smack does not keep its policy in a single blob of data, it uses lists
>> which can be modified at will. Your definition of the behavior for
>> LSM_POLICY wouldn't work for Smack. That doesn't mean that there isn't
>> a viable way to measure the Smack policy, it just means that IMA isn't
>> the place for it. If SELinux wants its data measured, SELinux should be
>> providing the mechanism to do it.
>>
>> I guess that I'm agreeing with you in part. If you want a generic measurement
>> of "critical data", you don't need to assign a type to it, you have the
>> caller (a security module, a device driver or whatever) identify itself and
>> how it is going to deal with the data. That's very different from what you've
>> done to date.
>
> Agree.
>
> Like Stephen had stated earlier, the reason we kept separate hooks (STATE and POLICY) is because the data for state is usually small and therefore we measure the entire data. Whereas, policy data is usually quite large (a few MB) and hence we measure a hash of the policy.
SELinux should determine how it wants its data measured.
SELinux, not IMA, should determine if some "critical data"
be measured in total, by its hash or as a count of policy
rules. It would be handy for IMA to supply functions to do
data blobs and hashes, but it should be up to the caller to
decide if they meet their needs.
>
> If change to a generic measurement of "critical data", at the least IMA should provide a way to measure "data" and "hash(data)".
>
> And, with the caller providing the identifying information, there would be no need to call it "LSM_STATE" or "SELINUX_STATE" or such.
>
> ?-lakshmi
>
>
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Lakshmi Ramasubramanian @ 2020-08-05 18:08 UTC (permalink / raw)
To: Casey Schaufler, Mimi Zohar, Tyler Hicks
Cc: stephen.smalley.work, sashal, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
In-Reply-To: <ecc97f59-c2cc-0b23-6199-925ba0d6358b@schaufler-ca.com>
On 8/5/20 10:57 AM, Casey Schaufler wrote:
> On 8/5/2020 10:25 AM, Lakshmi Ramasubramanian wrote:
>> On 8/5/20 10:03 AM, Mimi Zohar wrote:
>>> On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote:
>>>
>>>> In addition to SELINUX_STATE and SELINUX_POLICY, we should also consider
>>>> the proposed LSM_STATE and LSM_POLICY func values but require an "lsm"
>>>> rule conditional.
>>>>
>>>> So the current proposed rules:
>>>>
>>>> ? measure func=LSM_STATE
>>>> ? measure func=LSM_POLICY
>>>>
>>>> Would become:
>>>>
>>>> ? measure func=LSM_STATE lsm=selinux
>>>> ? measure func=LSM_POLICY lsm=selinux
>>>>
>>>> The following rules would be rejected:
>>>>
>>>> ? measure func=LSM_STATE
>>>> ? measure func=LSM_POLICY
>>>> ? measure func=LSM_STATE lsm=apparmor
>>>> ? measure func=LSM_POLICY lsm=smack
>>>
>>> Kees is cleaning up the firmware code which differentiated between how
>>> firmware was loaded.?? There will be a single firmware enumeration.
>>>
>>> Similarly, the new IMA hook to measure critical state may be placed in
>>> multiple places.? Is there really a need from a policy perspective for
>>> differentiating the source of the critical state being measurind??? The
>>> data being measured should include some identifying information.
>>
>> Yes Mimi - SELinux is including the identifying information in the "event name" field. Please see a sample measurement of STATE and POLICY for SELinux below:
>>
>> 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state-1595389364:287899386 696e697469616c697a65643d313b656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574776f726b5f706565725f636f6e74726f6c733d313b6f70656e5f7065726d733d313b657874656e6465645f736f636b65745f636c6173733d313b616c776179735f636865636b5f6e6574776f726b3d303b6367726f75705f7365636c6162656c3d313b6e6e705f6e6f737569645f7472616e736974696f6e3d313b67656e66735f7365636c6162656c5f73796d6c696e6b733d303
>>
>> 10 f4a7...9408 ima-ng sha256:8d1d...1834 selinux-policy-hash-1595389353:863934271
>>
>>>
>>> I think moving away from the idea that measuring "critical" data should
>>> be limited to LSMs, will clarify this.
>>>
>>
>> Are you suggesting that instead of calling the hooks LSM_STATE and LSM_POLICY, we should keep it more generic so that it can be utilized by any subsystem to measure their "critical data"?
>
> Policy, state, history or whim, it should be up to the security module
> to determine what data it cares about, and how it should be measured.
> Smack does not keep its policy in a single blob of data, it uses lists
> which can be modified at will. Your definition of the behavior for
> LSM_POLICY wouldn't work for Smack. That doesn't mean that there isn't
> a viable way to measure the Smack policy, it just means that IMA isn't
> the place for it. If SELinux wants its data measured, SELinux should be
> providing the mechanism to do it.
>
> I guess that I'm agreeing with you in part. If you want a generic measurement
> of "critical data", you don't need to assign a type to it, you have the
> caller (a security module, a device driver or whatever) identify itself and
> how it is going to deal with the data. That's very different from what you've
> done to date.
Agree.
Like Stephen had stated earlier, the reason we kept separate hooks
(STATE and POLICY) is because the data for state is usually small and
therefore we measure the entire data. Whereas, policy data is usually
quite large (a few MB) and hence we measure a hash of the policy.
If change to a generic measurement of "critical data", at the least IMA
should provide a way to measure "data" and "hash(data)".
And, with the caller providing the identifying information, there would
be no need to call it "LSM_STATE" or "SELINUX_STATE" or such.
-lakshmi
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Casey Schaufler @ 2020-08-05 17:57 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, Mimi Zohar, Tyler Hicks
Cc: stephen.smalley.work, sashal, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel, Casey Schaufler
In-Reply-To: <9ad079ff-1bd4-1302-e6fb-25a7396ef12f@linux.microsoft.com>
On 8/5/2020 10:25 AM, Lakshmi Ramasubramanian wrote:
> On 8/5/20 10:03 AM, Mimi Zohar wrote:
>> On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote:
>>
>>> In addition to SELINUX_STATE and SELINUX_POLICY, we should also consider
>>> the proposed LSM_STATE and LSM_POLICY func values but require an "lsm"
>>> rule conditional.
>>>
>>> So the current proposed rules:
>>>
>>> ? measure func=LSM_STATE
>>> ? measure func=LSM_POLICY
>>>
>>> Would become:
>>>
>>> ? measure func=LSM_STATE lsm=selinux
>>> ? measure func=LSM_POLICY lsm=selinux
>>>
>>> The following rules would be rejected:
>>>
>>> ? measure func=LSM_STATE
>>> ? measure func=LSM_POLICY
>>> ? measure func=LSM_STATE lsm=apparmor
>>> ? measure func=LSM_POLICY lsm=smack
>>
>> Kees is cleaning up the firmware code which differentiated between how
>> firmware was loaded.?? There will be a single firmware enumeration.
>>
>> Similarly, the new IMA hook to measure critical state may be placed in
>> multiple places.? Is there really a need from a policy perspective for
>> differentiating the source of the critical state being measurind??? The
>> data being measured should include some identifying information.
>
> Yes Mimi - SELinux is including the identifying information in the "event name" field. Please see a sample measurement of STATE and POLICY for SELinux below:
>
> 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state-1595389364:287899386 696e697469616c697a65643d313b656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574776f726b5f706565725f636f6e74726f6c733d313b6f70656e5f7065726d733d313b657874656e6465645f736f636b65745f636c6173733d313b616c776179735f636865636b5f6e6574776f726b3d303b6367726f75705f7365636c6162656c3d313b6e6e705f6e6f737569645f7472616e736974696f6e3d313b67656e66735f7365636c6162656c5f73796d6c696e6b733d303
>
> 10 f4a7...9408 ima-ng sha256:8d1d...1834 selinux-policy-hash-1595389353:863934271
>
>>
>> I think moving away from the idea that measuring "critical" data should
>> be limited to LSMs, will clarify this.
>>
>
> Are you suggesting that instead of calling the hooks LSM_STATE and LSM_POLICY, we should keep it more generic so that it can be utilized by any subsystem to measure their "critical data"?
Policy, state, history or whim, it should be up to the security module
to determine what data it cares about, and how it should be measured.
Smack does not keep its policy in a single blob of data, it uses lists
which can be modified at will. Your definition of the behavior for
LSM_POLICY wouldn't work for Smack. That doesn't mean that there isn't
a viable way to measure the Smack policy, it just means that IMA isn't
the place for it. If SELinux wants its data measured, SELinux should be
providing the mechanism to do it.
I guess that I'm agreeing with you in part. If you want a generic measurement
of "critical data", you don't need to assign a type to it, you have the
caller (a security module, a device driver or whatever) identify itself and
how it is going to deal with the data. That's very different from what you've
done to date.
>
> I think that is a good idea.
>
> ?-lakshmi
^ permalink raw reply
* Re: [PATCH 06/18] fsinfo: Add a uniquifier ID to struct mount [ver #21]
From: David Howells @ 2020-08-05 14:13 UTC (permalink / raw)
To: Miklos Szeredi
Cc: dhowells, viro, torvalds, raven, mszeredi, christian, jannh,
darrick.wong, kzak, jlayton, linux-api, linux-fsdevel,
linux-security-module, linux-kernel
In-Reply-To: <20200804104108.GC32719@miu.piliscsaba.redhat.com>
Miklos Szeredi <miklos@szeredi.hu> wrote:
> > +#ifdef CONFIG_FSINFO
> > + u64 mnt_unique_id; /* ID unique over lifetime of kernel */
> > +#endif
>
> Not sure if it's worth making conditional.
You can't get at it without CONFIG_FSINFO=y as it stands, but making it
unconditional might be reasonable.
> > - n.auxiliary_mount = aux->mnt_id;
> > + n.auxiliary_mount = aux->mnt_unique_id;
>
> Hmm, so we now have two ID's:
>
> - one can be used to look up the mount
> - one is guaranteed to be unique
>
> With this change the mount cannot be looked up with FSINFO_FLAGS_QUERY_MOUNT,
> right?
>
> Should we be merging the two ID's into a single one which has both properties?
Ideally, yes... but... The 31-bit mnt_id is currently exposed to userspace in
various places, e.g. /proc, sys_name_to_handle_at(). So we have to keep that
as is and we can't expand it.
For fsinfo(), however, it might make sense to only use the 64-bit uniquifier
as the identifier to use for direct look up.
However, looking up that identifier requires some sort of structure for doing
this and it's kind of worst case for the IDR tree as the keys are gradually
going to spread out, causing it to eat more memory. It may be a tradeoff
worth making, and the memory consumption might not be that bad - or we could
use some other data structure such as an rbtree.
That's why I was going for the 31-bit identifier + uniquifier so that you can at
least tell if the identifier got recycled reasonably quickly.
David
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Lakshmi Ramasubramanian @ 2020-08-05 17:25 UTC (permalink / raw)
To: Mimi Zohar, Tyler Hicks, Casey Schaufler
Cc: stephen.smalley.work, sashal, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
In-Reply-To: <69810007161e689ac817099fb1c6df21962963e4.camel@linux.ibm.com>
On 8/5/20 10:03 AM, Mimi Zohar wrote:
> On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote:
>
>> In addition to SELINUX_STATE and SELINUX_POLICY, we should also consider
>> the proposed LSM_STATE and LSM_POLICY func values but require an "lsm"
>> rule conditional.
>>
>> So the current proposed rules:
>>
>> measure func=LSM_STATE
>> measure func=LSM_POLICY
>>
>> Would become:
>>
>> measure func=LSM_STATE lsm=selinux
>> measure func=LSM_POLICY lsm=selinux
>>
>> The following rules would be rejected:
>>
>> measure func=LSM_STATE
>> measure func=LSM_POLICY
>> measure func=LSM_STATE lsm=apparmor
>> measure func=LSM_POLICY lsm=smack
>
> Kees is cleaning up the firmware code which differentiated between how
> firmware was loaded. There will be a single firmware enumeration.
>
> Similarly, the new IMA hook to measure critical state may be placed in
> multiple places. Is there really a need from a policy perspective for
> differentiating the source of the critical state being measurind? The
> data being measured should include some identifying information.
Yes Mimi - SELinux is including the identifying information in the
"event name" field. Please see a sample measurement of STATE and POLICY
for SELinux below:
10 e32e...5ac3 ima-buf sha256:86e8...4594
selinux-state-1595389364:287899386
696e697469616c697a65643d313b656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574776f726b5f706565725f636f6e74726f6c733d313b6f70656e5f7065726d733d313b657874656e6465645f736f636b65745f636c6173733d313b616c776179735f636865636b5f6e6574776f726b3d303b6367726f75705f7365636c6162656c3d313b6e6e705f6e6f737569645f7472616e736974696f6e3d313b67656e66735f7365636c6162656c5f73796d6c696e6b733d303
10 f4a7...9408 ima-ng sha256:8d1d...1834
selinux-policy-hash-1595389353:863934271
>
> I think moving away from the idea that measuring "critical" data should
> be limited to LSMs, will clarify this.
>
Are you suggesting that instead of calling the hooks LSM_STATE and
LSM_POLICY, we should keep it more generic so that it can be utilized by
any subsystem to measure their "critical data"?
I think that is a good idea.
-lakshmi
^ permalink raw reply
* Re: [PATCH 10/18] fsinfo: Provide notification overrun handling support [ver #21]
From: Miklos Szeredi @ 2020-08-05 17:26 UTC (permalink / raw)
To: David Howells
Cc: Al Viro, Linus Torvalds, Ian Kent, Miklos Szeredi,
Christian Brauner, Jann Horn, Darrick J. Wong, Karel Zak,
Jeff Layton, Linux API, linux-fsdevel, LSM, linux-kernel
In-Reply-To: <2320582.1596643618@warthog.procyon.org.uk>
On Wed, Aug 5, 2020 at 6:07 PM David Howells <dhowells@redhat.com> wrote:
>
> Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> > Shoun't we just make sure that the likelyhood of overruns is low
>
> That's not necessarily easy. To avoid overruns you need a bigger buffer. The
> buffer is preallocated from unswappable kernel space. Yes, you can increase
> the size of the buffer, but it eats out of your pipe bufferage limit.
>
> Further, it's a *general* notifications queue, not just for a specific
> purpose, but that means it might get connected to multiple sources, and doing
> something like tearing down a container might generate enough notifications to
> overrun the queue.
>
> > and if it happens, just reinitialize everthing from scratch (shouldn't be
> > *that* expensive).
>
> If you then spend time reinitialising everything, you're increasing the
> likelihood of racing with further events. Further, there multiple expenses:
> firstly, you have to tear down and discard all the data that you've spent time
> setting up; secondly, it takes time doing all this; thirdly, it takes cpu
> cycles away from applications.
>
> The reason I put the event counters in there and made it so that fsinfo()
> could read all the mounts in a subtree and their event counters in one go is
> to make it faster for the user to find out what changed in the event that a
> notification is lost.
That's just overdesigning it, IMO.
If the protocol is extensible (as you state) then the counters can be
added as needed. And unless the above CPU cycle wastage is actually
observed in practice, the whole thing is unnecessary.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH 00/18] VFS: Filesystem information [ver #21]
From: David Howells @ 2020-08-05 17:13 UTC (permalink / raw)
To: James Bottomley
Cc: dhowells, viro, Theodore Ts'o, Andreas Dilger, Eric Biggers,
Jeff Layton, linux-ext4, Carlos Maiolino, Darrick J. Wong,
linux-api, torvalds, raven, mszeredi, christian, jannh, kzak,
jlayton, linux-fsdevel, linux-security-module, linux-kernel
In-Reply-To: <1596555579.10158.23.camel@HansenPartnership.com>
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> It sort of petered out into a long winding thread about why not use
> sysfs instead, which really doesn't look like a good idea to me.
It seemed to turn into a set of procfs symlinks that pointed at a bunch of
sysfs stuff - or possibly some special filesystem.
> Could I make a suggestion about how this should be done in a way that
> doesn't actually require the fsinfo syscall at all: it could just be
> done with fsconfig.
I'd prefer to keep it separate. The interface for fsconfig() is intended to
move stuff into the kernel, not out of it. Better to add a parallel syscall
to go the other way (kind of like we have setxattr/getxattr, sendmsg/recvmsg).
Further, fsinfo() can refer directly to a file/fd/mount/whatever, but
fsconfig() doesn't do that. You have to use fspick() to get a context before
you can use fsconfig(). Now, that's fine if you want to gather several pieces
of information from a particular object, but it's not so good if you want to
get one piece of information from each of several objects.
> ... make it table configured...
I did, kind of (though I didn't call it that). Al rewrote the code to get rid
of it.
David
^ permalink raw reply
* Re: [PATCH v6 0/4] LSM: Measure security module data
From: Mimi Zohar @ 2020-08-05 12:00 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, stephen.smalley.work, casey
Cc: tyhicks, sashal, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
In-Reply-To: <20200805004331.20652-1-nramas@linux.microsoft.com>
On Tue, 2020-08-04 at 17:43 -0700, Lakshmi Ramasubramanian wrote:
> Critical data structures of security modules are currently not measured.
> Therefore an attestation service, for instance, would not be able to
> attest whether the security modules are always operating with the policies
> and configuration that the system administrator had setup. The policies
> and configuration for the security modules could be tampered with by
> malware by exploiting kernel vulnerabilities or modified through some
> inadvertent actions on the system. Measuring such critical data would
> enable an attestation service to better assess the state of the system.
From a high level review, "Critical data structures" should be the
focus of this patch set. Measuring "critical data structures" should
be independent of measuring the "policy" being loaded. The in memory
policy hash could be an example of data included in the "critical data
structures".
Keep this patch set simple.
Mimi
^ permalink raw reply
* Re: [PATCH 06/18] fsinfo: Add a uniquifier ID to struct mount [ver #21]
From: Matthew Wilcox @ 2020-08-05 19:33 UTC (permalink / raw)
To: David Howells
Cc: Miklos Szeredi, Al Viro, Linus Torvalds, Ian Kent, Miklos Szeredi,
Christian Brauner, Jann Horn, Darrick J. Wong, Karel Zak,
Jeff Layton, Linux API, linux-fsdevel, LSM, linux-kernel
In-Reply-To: <2315925.1596641410@warthog.procyon.org.uk>
On Wed, Aug 05, 2020 at 04:30:10PM +0100, David Howells wrote:
> Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> > idr_alloc_cyclic() seems to be a good template for doing the lower
> > 32bit allocation, and we can add code to increment the high 32bit on
> > wraparound.
> >
> > Lots of code uses idr_alloc_cyclic() so I guess it shouldn't be too
> > bad in terms of memory use or performance.
>
> It's optimised for shortness of path and trades memory for performance. It's
> currently implemented using an xarray, so memory usage is dependent on the
> sparseness of the tree. Each node in the tree is 576 bytes and in the worst
> case, each one node will contain one mount - and then you have to backfill the
> ancestry, though for lower memory costs.
>
> Systemd makes life more interesting since it sets up a whole load of
> propagations. Each mount you make may cause several others to be created, but
> that would likely make the tree more efficient.
I would recommend using xa_alloc and ignoring the ID assigned from
xa_alloc. Looking up by unique ID is then a matter of iterating every
mount (xa_for_each()) looking for a matching unique ID in the mount
struct. That's O(n) search, but it's faster than a linked list, and we
don't have that many mounts in a system.
The maple tree will handle this case more effectively, but I can't
recommend waiting for that to be ready.
^ 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