* Re: [PATCH 2/6] Adjust watch_queue documentation to mention mount and superblock watches. [ver #5]
From: David Howells @ 2019-07-01 8:52 UTC (permalink / raw)
To: Randy Dunlap
Cc: dhowells, viro, Casey Schaufler, Stephen Smalley,
Greg Kroah-Hartman, nicolas.dichtel, raven, Christian Brauner,
keyrings, linux-usb, linux-security-module, linux-fsdevel,
linux-api, linux-block, linux-kernel
In-Reply-To: <7a288c2c-11a1-87df-9550-b247d6ce3010@infradead.org>
Randy Dunlap <rdunlap@infradead.org> wrote:
> I'm having a little trouble parsing that sentence.
> Could you clarify it or maybe rewrite/modify it?
> Thanks.
How about:
* ``info_filter`` and ``info_mask`` act as a filter on the info field of the
notification record. The notification is only written into the buffer if::
(watch.info & info_mask) == info_filter
This could be used, for example, to ignore events that are not exactly on
the watched point in a mount tree by specifying NOTIFY_MOUNT_IN_SUBTREE
must not be set, e.g.::
{
.type = WATCH_TYPE_MOUNT_NOTIFY,
.info_filter = 0,
.info_mask = NOTIFY_MOUNT_IN_SUBTREE,
.subtype_filter = ...,
}
as an event would be only permissible with this filter if::
(watch.info & NOTIFY_MOUNT_IN_SUBTREE) == 0
David
^ permalink raw reply
* Re: [PATCH 2/6] Adjust watch_queue documentation to mention mount and superblock watches. [ver #5]
From: Randy Dunlap @ 2019-07-01 2:59 UTC (permalink / raw)
To: David Howells, viro
Cc: Casey Schaufler, Stephen Smalley, Greg Kroah-Hartman,
nicolas.dichtel, raven, Christian Brauner, keyrings, linux-usb,
linux-security-module, linux-fsdevel, linux-api, linux-block,
linux-kernel
In-Reply-To: <156173703546.15650.14319137940607993268.stgit@warthog.procyon.org.uk>
Hi David,
On 6/28/19 8:50 AM, David Howells wrote:
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> Documentation/watch_queue.rst | 20 +++++++++++++++++++-
> drivers/misc/Kconfig | 5 +++--
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/watch_queue.rst b/Documentation/watch_queue.rst
> index 4087a8e670a8..1bec2018d549 100644
> --- a/Documentation/watch_queue.rst
> +++ b/Documentation/watch_queue.rst
> @@ -13,6 +13,10 @@ receive notifications from the kernel. This can be used in conjunction with::
>
> * USB subsystem event notifications
>
> + * Mount topology change notifications
> +
> + * Superblock event notifications
> +
>
> The notifications buffers can be enabled by:
>
> @@ -324,6 +328,19 @@ Any particular buffer can be fed from multiple sources. Sources include:
> for buses and devices. Watchpoints of this type are set on the global
> device watch list.
>
> + * WATCH_TYPE_MOUNT_NOTIFY
> +
> + Notifications of this type indicate mount tree topology changes and mount
> + attribute changes. A watch can be set on a particular file or directory
> + and notifications from the path subtree rooted at that point will be
> + intercepted.
> +
> + * WATCH_TYPE_SB_NOTIFY
> +
> + Notifications of this type indicate superblock events, such as quota limits
> + being hit, I/O errors being produced or network server loss/reconnection.
> + Watches of this type are set directly on superblocks.
> +
>
> Event Filtering
> ===============
> @@ -365,7 +382,8 @@ Where:
> (watch.info & info_mask) == info_filter
>
> This could be used, for example, to ignore events that are not exactly on
> - the watched point in a mount tree.
> + the watched point in a mount tree by specifying NOTIFY_MOUNT_IN_SUBTREE
> + must be 0.
I'm having a little trouble parsing that sentence.
Could you clarify it or maybe rewrite/modify it?
Thanks.
>
> * ``subtype_filter`` is a bitmask indicating the subtypes that are of
> interest. Bit 0 of subtype_filter[0] corresponds to subtype 0, bit 1 to
--
~Randy
^ permalink raw reply
* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
From: Mimi Zohar @ 2019-06-30 15:39 UTC (permalink / raw)
To: Roberto Sassu, Rob Landley, viro
Cc: linux-security-module, linux-integrity, initramfs, linux-api,
linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan,
niveditas98
In-Reply-To: <33cfb804-6a17-39f0-92b7-01d54e9c452d@huawei.com>
On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
> On 6/3/2019 8:32 PM, Rob Landley wrote:
> > On 6/3/19 4:31 AM, Roberto Sassu wrote:
> >>> This patch set aims at solving the following use case: appraise files from
> >>> the initial ram disk. To do that, IMA checks the signature/hash from the
> >>> security.ima xattr. Unfortunately, this use case cannot be implemented
> >>> currently, as the CPIO format does not support xattrs.
> >>>
> >>> This proposal consists in including file metadata as additional files named
> >>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> >>> kernel recognizes these special files from the file name, and calls the
> >>> appropriate parser to add metadata to the previously extracted file. It has
> >>> been proposed to use bit 17:16 of the file mode as a way to recognize files
> >>> with metadata, but both the kernel and the cpio tool declare the file mode
> >>> as unsigned short.
> >>
> >> Any opinion on this patch set?
> >>
> >> Thanks
> >>
> >> Roberto
> >
> > Sorry, I've had the window open since you posted it but haven't gotten around to
> > it. I'll try to build it later today.
> >
> > It does look interesting, and I have no objections to the basic approach. I
> > should be able to add support to toybox cpio over a weekend once I've got the
> > kernel doing it to test against.
>
> Ok.
>
> Let me give some instructions so that people can test this patch set.
>
> To add xattrs to the ram disk embedded in the kernel it is sufficient
> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
>
> To add xattrs to the external ram disk, it is necessary to patch cpio:
>
> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
> (xattr-v1 branch)
>
> and dracut:
>
> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
> (digest-lists branch)
>
> The same modification can be done for mkinitramfs (add '-e xattr' to the
> cpio command line).
>
> To simplify the test, it would be sufficient to replace only the cpio
> binary and the dracut script with the modified versions. For dracut, the
> patch should be applied to the local dracut (after it has been renamed
> to dracut.sh).
>
> Then, run:
>
> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
>
> Xattrs can be seen by stopping the boot process for example by adding
> rd.break to the kernel command line.
A simple way of testing, without needing any changes other than the
kernel patches, is to save the dracut temporary directory by supplying
"--keep" on the dracut command line, calling
usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
xattr" option.
If your filesystem already has and copied the security xattrs to the
dracut temporary directory, the script, below, will include them in
the initramfs file. Otherwise, you'll need to write the desired
security xattrs on the files, using setfattr, in the temporary dracut
directory, before creating the initramfs.
Remember to make sure that the initramfs_list includes "getfattr",
otherwise you'll need to wait until real root is mounted as /sysroot
to see the security xattrs for the rootfs files.
The following script has not been tested on a recent version of
dracut. Some changes might be needed, as well as some code cleanup.
#!/bin/bash
initramfs_name=/boot/initramfs-`uname -r`.img
initramfs_output_name=${initramfs_name/.img/.test.img}
if [ $# -eq 1 ]; then
initramfs_name=$1
fi
if [ ! -f "$initramfs_name" ]; then
echo "Usage; $0 <initramfs pathanem>"
exit 1
fi
tmp=$(dracut -H -f "$initramfs_name" --keep --noprelink --nostrip 2>&1)
suffix=$(echo $tmp | cut -d ' ' -f 3 | cut -d '.' -f 2)
tmpdir="/var/tmp/dracut.$suffix/initramfs"
if [ ! -d "$tmpdir" ]; then
echo "$tmpdir does not exist"
exit 1
fi
usr/gen_initramfs_list.sh ${tmpdir} > usr/initramfs_list
usr/gen_init_cpio -e xattr usr/initramfs_list > usr/initramfs_data.cpio
gzip usr/initramfs_data.cpio
echo "Copying usr/initramfs_data.cpio to $initramfs_output_name"
cp usr/initramfs_data.cpio.gz "$initramfs_output_name"
Mimi
^ permalink raw reply
* Re: [PATCH v4 3/3] gen_init_cpio: add support for file metadata
From: Mimi Zohar @ 2019-06-30 15:27 UTC (permalink / raw)
To: Roberto Sassu, viro
Cc: linux-security-module, linux-integrity, initramfs, linux-api,
linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
james.w.mcmechan, niveditas98
In-Reply-To: <20190523121803.21638-4-roberto.sassu@huawei.com>
On Thu, 2019-05-23 at 14:18 +0200, Roberto Sassu wrote:
> diff --git a/usr/Kconfig b/usr/Kconfig
> index 43658b8a975e..8d9f54a16440 100644
> --- a/usr/Kconfig
> +++ b/usr/Kconfig
> @@ -233,3 +233,11 @@ config INITRAMFS_COMPRESSION
> default ".lzma" if RD_LZMA
> default ".bz2" if RD_BZIP2
> default ""
> +
> +config INITRAMFS_FILE_METADATA
> + string "File metadata type"
> + default ""
> + help
> + Specify xattr to include xattrs in the image.
> +
> + If you are not sure, leave it blank.
> fi
Instead of having to specify the metdata type, let's make this a
choice.
Mimi
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Andy Lutomirski @ 2019-06-29 23:47 UTC (permalink / raw)
To: Matthew Garrett
Cc: Andy Lutomirski, Stephen Smalley, James Morris, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Network Development, Chun-Yi Lee, Daniel Borkmann, LSM List
In-Reply-To: <CACdnJuuy7-tkj86njAqtdJ3dUMu-2T8a2y8DC3fMKBK0z9J6ag@mail.gmail.com>
On Fri, Jun 28, 2019 at 11:47 AM Matthew Garrett <mjg59@google.com> wrote:
>
> On Thu, Jun 27, 2019 at 4:27 PM Andy Lutomirski <luto@kernel.org> wrote:
> > They're really quite similar in my mind. Certainly some things in the
> > "integrity" category give absolutely trivial control over the kernel
> > (e.g. modules) while others make it quite challenging (ioperm), but
> > the end result is very similar. And quite a few "confidentiality"
> > things genuinely do allow all kernel memory to be read.
> >
> > I agree that finer-grained distinctions could be useful. My concern is
> > that it's a tradeoff, and the other end of the tradeoff is an ABI
> > stability issue. If someone decides down the road that some feature
> > that is currently "integrity" can be split into a narrow "integrity"
> > feature and a "confidentiality" feature then, if the user policy knows
> > about the individual features, there's a risk of breaking people's
> > systems. If we keep the fine-grained control, do we have a clear
> > compatibility story?
>
> My preference right now is to retain the fine-grained aspect of things
> in the internal API, simply because it'll be more annoying to add it
> back later if we want to. I don't want to expose it via the Lockdown
> user facing API for the reasons you've described, but it's not
> impossible that another LSM would find a way to do this reasonably.
> Does it seem reasonable to punt this discussion out to the point where
> another LSM tries to do something with this information, based on the
> implementation they're attempting?
I think I can get behind this, as long as it's clear to LSM authors
that this list is only a little bit stable. I can certainly see the
use for the fine-grained info being available for auditing.
^ permalink raw reply
* Re: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Andy Lutomirski @ 2019-06-29 23:46 UTC (permalink / raw)
To: Cedric Xing
Cc: linux-sgx, LSM List, selinux, Schaufler, Casey, James Morris,
Andrew Lutomirski, Jethro Beekman, Dr. Greg Wettstein,
Stephen Smalley, Jarkko Sakkinen, Christopherson, Sean J
In-Reply-To: <72420cff8fa944b64e57df8d25c63bd30f8aacfa.1561588012.git.cedric.xing@intel.com>
On Thu, Jun 27, 2019 at 11:56 AM Cedric Xing <cedric.xing@intel.com> wrote:
>
> SGX enclaves are loaded from pages in regular memory. Given the ability to
> create executable pages, the newly added SGX subsystem may present a backdoor
> for adversaries to circumvent LSM policies, such as creating an executable
> enclave page from a modified regular page that would otherwise not be made
> executable as prohibited by LSM. Therefore arises the primary question of
> whether an enclave page should be allowed to be created from a given source
> page in regular memory.
>
> A related question is whether to grant/deny a mprotect() request on a given
> enclave page/range. mprotect() is traditionally covered by
> security_file_mprotect() hook, however, enclave pages have a different lifespan
> than either MAP_PRIVATE or MAP_SHARED. Particularly, MAP_PRIVATE pages have the
> same lifespan as the VMA while MAP_SHARED pages have the same lifespan as the
> backing file (on disk), but enclave pages have the lifespan of the enclave’s
> file descriptor. For example, enclave pages could be munmap()’ed then mmap()’ed
> again without losing contents (like MAP_SHARED), but all enclave pages will be
> lost once its file descriptor has been closed (like MAP_PRIVATE). That said,
> LSM modules need some new data structure for tracking protections of enclave
> pages/ranges so that they can make proper decisions at mmap()/mprotect()
> syscalls.
>
> The last question, which is orthogonal to the 2 above, is whether or not to
> allow a given enclave to launch/run. Enclave pages are not visible to the rest
> of the system, so to some extent offer a better place for malicious software to
> hide. Thus, it is sometimes desirable to whitelist/blacklist enclaves by their
> measurements, signing public keys, or image files.
>
> To address the questions above, 2 new LSM hooks are added for enclaves.
> - security_enclave_load() – This hook allows LSM to decide whether or not to
> allow instantiation of a range of enclave pages using the specified VMA. It
> is invoked when a range of enclave pages is about to be loaded. It serves 3
> purposes: 1) indicate to LSM that the file struct in subject is an enclave;
> 2) allow LSM to decide whether or not to instantiate those pages and 3)
> allow LSM to initialize internal data structures for tracking
> origins/protections of those pages.
> - security_enclave_init() – This hook allows whitelisting/blacklisting or
> performing whatever checks deemed appropriate before an enclave is allowed
> to run. An LSM module may opt to use the file backing the SIGSTRUCT as a
> proxy to dictate allowed protections for anonymous pages.
>
> mprotect() of enclave pages continue to be governed by
> security_file_mprotect(), with the expectation that LSM is able to distinguish
> between regular and enclave pages inside the hook. For mmap(), the SGX
> subsystem is expected to invoke security_file_mprotect() explicitly to check
> protections against the requested protections for existing enclave pages. As
> stated earlier, enclave pages have different lifespan than the existing
> MAP_PRIVATE and MAP_SHARED pages, so would require a new data structure outside
> of VMA to track their protections and/or origins. Enclave Memory Area (or EMA
> for short) has been introduced to address the need. EMAs are maintained by the
> LSM framework for all LSM modules to share. EMAs will be instantiated for
> enclaves only so will not impose memory/performance overheads for regular
> applications/files. Please see include/linux/lsm_ema.h and security/lsm_ema.c
> for details.
>
> A new setup parameter – lsm.ema.cache_decisions has been introduced to offer
> the choice between memory consumption and accuracy of audit logs. Enabling
> lsm.ema.cache_decisions causes LSM framework NOT to keep backing files open for
> EMAs. While that saves memory, it requires LSM modules to make and cache
> decisions ahead of time, and makes it difficult for LSM modules to generate
> accurate audit logs. System administrators are expected to run LSM in
> permissive mode with lsm.ema.cache_decisions off to determine the minimal
> permissions needed, and then turn it back on in enforcing mode for optimal
> performance and memory usage. lsm.ema.cache_decisions is on by default and
> could be turned off by appending “lsm.ema.cache_decisions=0” or
> “lsm.ema.cache_decisions=off” to the kernel command line.
Just on a very cursory review, this seems like it's creating a bunch
of complexity (a whole new library and data structure), and I'm not
convinced the result is any better than Sean's version.
^ permalink raw reply
* Re: [RFC PATCH v4 10/12] security/selinux: Add enclave_load() implementation
From: Andy Lutomirski @ 2019-06-29 23:41 UTC (permalink / raw)
To: Stephen Smalley
Cc: Xing, Cedric, Christopherson, Sean J, Jarkko Sakkinen,
linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, Roberts, William C, Schaufler, Casey,
James Morris, Hansen, Dave, Andy Lutomirski, Jethro Beekman,
Dr . Greg Wettstein
In-Reply-To: <b36d6fd0-3135-e48d-ed84-d69853bd79f1@tycho.nsa.gov>
On Tue, Jun 25, 2019 at 2:09 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 6/21/19 5:22 PM, Xing, Cedric wrote:
> >> From: Christopherson, Sean J
> >> Sent: Wednesday, June 19, 2019 3:24 PM
> >>
> >> Intended use of each permission:
> >>
> >> - SGX_EXECDIRTY: dynamically load code within the enclave itself
> >> - SGX_EXECUNMR: load unmeasured code into the enclave, e.g. Graphene
> >
> > Why does it matter whether a code page is measured or not?
>
> It won't be incorporated into an attestation?
>
Also, if there is, in parallel, a policy that limits the set of
enclave SIGSTRUCTs that are accepted, requiring all code be measured
makes it harder to subvert by writing incompetent or maliciously
incompetent enclaves.
^ permalink raw reply
* Re: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Casey Schaufler @ 2019-06-29 21:35 UTC (permalink / raw)
To: Stephen Smalley, casey
Cc: Xing, Cedric, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
Schaufler, Casey, jmorris@namei.org, luto@kernel.org,
jethro@fortanix.com, greg@enjellic.com, sds@tycho.nsa.gov,
jarkko.sakkinen@linux.intel.com, Christopherson, Sean J
In-Reply-To: <CAB9W1A1RpM_9D_49E1VauuKE1tL=TyfeATomv47HX4FONnjA4A@mail.gmail.com>
On 6/28/2019 6:37 PM, Stephen Smalley wrote:
> On Fri, Jun 28, 2019 at 1:22 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 6/27/2019 5:47 PM, Xing, Cedric wrote:
>>>> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
>>>> Sent: Thursday, June 27, 2019 4:37 PM
>>>>>> This code should not be mixed in with the LSM infrastructure.
>>>>>> It should all be contained in its own module, under security/enclave.
>>>>> lsm_ema is *intended* to be part of the LSM infrastructure.
>>>> That's not going to fly, not for a minute.
>>> Why not, if there's a need for it?
>>>
>>> And what's the concern here if it becomes part of the LSM infrastructure.
>> The LSM infrastructure provides a framework for hooks
>> and allocation of blobs. That's it. It's a layer for
>> connecting system features like VFS, IPC and the IP stack
>> to the security modules. It does not implement any policy
>> of it's own. We are not going to implement SGX or any other
>> mechanism within the LSM infrastructure.
> I don't think you understand the purpose of this code. It isn't
> implementing SGX, nor is it needed by SGX.
> It is providing shared infrastructure for security modules, similar to
> lsm_audit.c, so that security modules can enforce W^X or similar
> memory protection guarantees for SGX enclave memory, which has unique
> properties that render the existing mmap and mprotect hooks
> insufficient. They can certainly implement it only for SELinux, but
> then any other security module that wants to provide such guarantees
> will have to replicate that code.
I am not objecting to the purpose of the code.
I *am* objecting to calling it part of the LSM infrastructure.
It needs to be it's own thing, off somewhere else.
It must not use the lsm_ prefix. That's namespace pollution.
The code must not be embedded in the LSM infrastructure code,
that breaks with how everything else works.
... and the notion that you allocate data for one blob
that gets freed relative to another breaks the data management
model.
^ permalink raw reply
* Re: Fwd: [PATCH v4 15/23] LSM: Specify which LSM to display
From: Casey Schaufler @ 2019-06-29 19:45 UTC (permalink / raw)
To: Stephen Smalley, Stephen Smalley, Schaufler, Casey, James Morris,
LSM List, selinux, keescook, John Johansen, penguin-kernel,
Paul Moore
In-Reply-To: <CAB9W1A29fCn=cH_Mx-g-P6M-5t+832ayhMmjy3PFZ-BOL3BuDQ@mail.gmail.com>
On 6/28/2019 6:01 PM, Stephen Smalley wrote:
> On Fri, Jun 28, 2019 at 12:15 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 6/28/2019 7:45 AM, Stephen Smalley wrote:
>>> On 6/26/19 3:22 PM, Casey Schaufler wrote:
>>>> Create a new entry "display" in /proc/.../attr for controlling
>>>> which LSM security information is displayed for a process.
>>>> The name of an active LSM that supplies hooks for human readable
>>>> data may be written to "display" to set the value. The name of
>>>> the LSM currently in use can be read from "display".
>>>> At this point there can only be one LSM capable of display
>>>> active.
>>> IIUC, presently, at the end of the entire series,
>>>
>>> 1) Any process can change its display attribute to any enabled security module, and no security module can veto that change.
>> That is correct. If a security module could hoard the display it
>> could prevent user space from functioning in a multiple module
>> environment.
>>
>>> 2) The display attribute is inherited across fork and exec, even execs that change credentials, and again no security module has control over the inheritance of this attribute.
>> Also correct. Scripts don't work otherwise.
> It's a security hole waiting to happen. Unprivileged caller sets its
> display value to Smack on a mostly SELinux system that happens to
> enable Smack too, then exec's a credential-changing SELinux-aware
> program that uses one of the libselinux APIs to set one of the
> /proc/self/attr attributes to a different SELinux context. Due to the
> change in display, the SELinux-aware program instead ends up setting
> one of the Smack attributes and therefore the desired SELinux context
> is never applied to the process or file or socket or whatever.
The credential-changing SELinux-aware program is getting
invoked by an unprivileged, Smack aware program? Would anyone
expect that to be a good idea? I'll admit it could happen,
but setting the Smack label of your SELinux-aware program
(which will need CAP_MAC_ADMIN, BTW) to "system_u:system_r:wheehee_t"
is unlikely to result in anything other than your SELinux-aware
program getting very frustrated. In the other direction, a
Smack-aware program that trys to set its SELinux context to "^"
is going to fail by SELinux policy. While I am willing to accept
that it is possible that there is a way to exploit this, it
would require convoluted SELinux and Smack policies. Anyone
who has reason to use a combination of Smack and SELinux on
a real system is already signing up for more configuration
headaches than I would wish on anyone.
I have strongly advocated addition of /proc/.../attr/
subdirectories for all LSMs, and that all user space migrate
to using them. /proc/.../attr/selinux/current would not be
affected by the display setting. We know, and have known for
years that so long as "current" is shared there will be this
sort of problem.
>
>>> 3) Setting the display attribute affects more than just the contexts read or written by the process itself:
>>> - Contexts reported in audit logs,
>>> - Contexts passed across binder (generated in sender context, delivered to receiver),
>>> - Contexts passed to NFS servers for new files,
>>> - Contexts returned by NFS servers for existing files,
>>> - Netlink-related contexts (?possibly generated in sender context rather than receiver context?),
>>> - This list may not be complete.
>> Any of which can be changed should a more rational behavior be proposed.
>> One possibility is to use lsm='value',lsm='value' encoding for internal
>> communications, but there's been considerable resistance to anything
>> like that.
> These are also security holes waiting to happen. Processes can use it
> to hide their SELinux contexts from the audit logs, forge different
> SELinux contexts on binder IPC, forge file contexts to which they have
> no SELinux permissions on new files, ... All they need is stacking to
> be enabled and one other module that helpfully lets them set attribute
> values that look like SELinux contexts, and then they can set those
> and switch their display at the right time.
What would you propose as a more rational behavior?
Seriously, I could use some help here.
>>> 4) A security_secid_to_secctx() in one process' context (e.g. sender) or with one display value followed later by a security_secctx_to_secid() call in a different process' context (e.g. receiver) or with a different display value may ask a different security module to perform the reverse translation of the context than the forward translation.
>> Do you have an example of where this might happen?
>> Contexts are rarely used within the kernel. The usual
>> behavior is to generate them, send them out to user space,
>> and delete them. They get cached in some networking code,
>> but not in cases where more than one (existing) security
>> module will ever use them. Binder may be an exception, but
>> only SELinux (currently) supports binder.
> Haven't looked but I don't like the asymmetry of the interface.
> Doesn't matter that only SELinux supports binder if you ever want any
> other security module other than SELinux enabled at the same time as
> SELinux.
Binder needs another look then.
>> Is that correct? If so, it seems problematic.
>> Balancing backward compatibility with new behavior is hard!
>> What would you suggest for audit logs? Should we put all LSM
>> data in every record? Is NFS a concern for anyone not using
>> SELinux?
> Yes to all on audit if stacking is going to be real. And yes, I think
> other security modules will care about NFS if they are serious.
I would love to get feedback from the audit maintainers about
how they would like the multiple LSM data formatted.
NFS is ... challenging. It was supposed to work with Smack
when it went in, but to the best of my understanding never
actually demonstrated.
>> There is no user space that uses display, and it's going
>> to take some time to work out all the kinks before we even
>> think about teaching systemd about it.
> That doesn't make it acceptable to introduce a mechanism that weakens
> security now.
Agreed in principle, not necessarily in detail.
^ permalink raw reply
* Re: [RFC PATCH v5 0/1] Add dm verity root hash pkcs7 sig validation.
From: James Morris @ 2019-06-29 4:01 UTC (permalink / raw)
To: Eric Biggers
Cc: Jaskaran Khurana, linux-security-module, linux-kernel,
linux-integrity, linux-fsdevel, agk, snitzer, dm-devel, scottsh,
mpatocka, gmazyland
In-Reply-To: <20190628040041.GB673@sol.localdomain>
On Thu, 27 Jun 2019, Eric Biggers wrote:
> I don't understand your justification for this feature.
>
> If userspace has already been pwned severely enough for the attacker to be
> executing arbitrary code with CAP_SYS_ADMIN (which is what the device mapper
> ioctls need), what good are restrictions on loading more binaries from disk?
>
> Please explain your security model.
Let's say the system has a policy where all code must be signed with a
valid key, and that one mechanism for enforcing this is via signed
dm-verity volumes. Validating the signature within the kernel provides
stronger assurance than userspace validation. The kernel validates and
executes the code, using kernel-resident keys, and does not need to rely
on validation which has occurred across a trust boundary.
You don't need arbitrary CAP_SYS_ADMIN code execution, you just need a
flaw in the app (or its dependent libraries, or configuration) which
allows signature validation to be bypassed.
The attacker now needs a kernel rather than a userspace vulnerability to
bypass the signed code policy.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Stephen Smalley @ 2019-06-29 1:37 UTC (permalink / raw)
To: Casey Schaufler
Cc: Xing, Cedric, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
Schaufler, Casey, jmorris@namei.org, luto@kernel.org,
jethro@fortanix.com, greg@enjellic.com, sds@tycho.nsa.gov,
jarkko.sakkinen@linux.intel.com, Christopherson, Sean J
In-Reply-To: <f6f16990-0291-c530-61dd-dcd26525285c@schaufler-ca.com>
On Fri, Jun 28, 2019 at 1:22 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 6/27/2019 5:47 PM, Xing, Cedric wrote:
> >> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
> >> Sent: Thursday, June 27, 2019 4:37 PM
> >>>> This code should not be mixed in with the LSM infrastructure.
> >>>> It should all be contained in its own module, under security/enclave.
> >>> lsm_ema is *intended* to be part of the LSM infrastructure.
> >> That's not going to fly, not for a minute.
> > Why not, if there's a need for it?
> >
> > And what's the concern here if it becomes part of the LSM infrastructure.
>
> The LSM infrastructure provides a framework for hooks
> and allocation of blobs. That's it. It's a layer for
> connecting system features like VFS, IPC and the IP stack
> to the security modules. It does not implement any policy
> of it's own. We are not going to implement SGX or any other
> mechanism within the LSM infrastructure.
I don't think you understand the purpose of this code. It isn't
implementing SGX, nor is it needed by SGX.
It is providing shared infrastructure for security modules, similar to
lsm_audit.c, so that security modules can enforce W^X or similar
memory protection guarantees for SGX enclave memory, which has unique
properties that render the existing mmap and mprotect hooks
insufficient. They can certainly implement it only for SELinux, but
then any other security module that wants to provide such guarantees
will have to replicate that code.
>
> >>> It is going to be shared among all LSMs that would like to track
> >> enclave pages and their origins.
> >>
> >> That's true for InfiniBand, tun and sctp as well. Look at their
> >> implementations.
> > As far as I can tell, InfiniBand, tun and sctp, all of them seemed used inside SELinux only.
>
> So?
>
> > If you had a chance to look at v1 of my series, I started by burying everything inside SELinux too. But Stephen pointed out such tracking would be needed by all LSMs so code duplication might be a concern. Thus I responded by moving it into LSM infrastructure.
>
> What you need to do is move all the lsm_ema code into its own
> place (which could be security/enclave). Manage your internal
> data as you like. LSMs (e.g. SELinux) can call your APIs if
> needed. If the LSMs need to store SGX information with the file
> structure they need to include that in the space they ask for in
> the file blob.
>
>
> >>> And they could be extended to store more information as deemed
> >> appropriate by the LSM module.
> >>
> >> Which is what blobs are for, but that does not appear to be how
> >> you're using either the file blob or your new ema blob.
> > A lsm_ema_map pointer is stored in file->f_security.
>
> That's up to the individual security module to decide.
>
> > Each lsm_ema_map contains a list of lsm_ema structures. In my last patch, SELinux stores a ema_security_struct with every ema, by setting selinux_blob_sizes.lbs_ema_data to sizeof(ema_security_struct).
>
> You are managing the ema map lists. You don't need the LSM
> infrastructure to do that.
>
> > ema_security_struct is initialized in selinux_enclave_load(), and checked in enclave_mprotect(), which is a subroutine of selinux_file_mprotect(). BTW, it is alloced/freed automatically by LSM infrastructure in security_enclave_load()/security_file_free().
>
> Do you mean security_enclave_load()/security_enclave_free() ?
> There is no way you can possibly have sane behavior if you're
> allocation and free aren't tied to the same blob.
>
> >>> The last patch of this series shows how to extend EMA inside SELinux.
> >> I don't see (but I admit the code doesn't make a lot of sense to me)
> >> anything you couldn't do in the SELinux code by adding data to the
> >> file blob. The data you're adding to the LSM infrastructure doesn't
> >> belong there, and it doesn't need to be there.
> > You are correct. My v1 did it inside SELinux.
> >
> > The key question I think is whether only SELinux needs it, or all LSMs need it. Stephen thought it was the latter (and I agree with him) so I moved it into the LSM infrastructure to be shared, just like the auditing code.
>
> You are both right that it doesn't belong in the SELinux code.
> It also doesn't belong as part of the LSM infrastructure.
>
> >>>> Not acceptable for the LSM infrastructure. They
> >>>> are inconsistent with the way data is used there.
> >>> I'm not sure I understand this comment.
> >> It means that your definition and use of the lsm_ema_blob
> >> does not match the way other blobs are managed and used.
> >> The LSM infrastructure uses these entries in a very particular
> >> way, and you're trying to use it differently. Your might be
> >> able to change the rest of the enclave system to use it
> >> correctly, or you might be able to find a different place
> >> for it.
> > I'm still not sure why you think this (lbs_ema_data) is inconsistent with other blobs.
> >
> > Same as all other blobs, an LSM requests it by storing the needed size in it, and is assigned an offset, and the buffer is allocated/freed by the infrastructure. Am I missing anything?
>
> Yes. Aside from allocation and deletion the infrastructure does
> nothing with the blobs. The blobs are used only by the security
> modules. All other data is maintained and used elsewhere. SGX
> specific data needs to me maintained and managed elsewhere.
>
> >>> As I stated in the cover letter, the primary question is how to
> >> prevent SGX from being abused as a backdoor to make executable pages
> >> that would otherwise not be executable without SGX. Any LSM module
> >> unaware of that would leave that "hole" open. So tracking enclave pages
> >> will become a common task for all LSMs that care page protections, and
> >> that's why I place it inside LSM infrastructure.
> >>
> >> Page protections are an important part of many security features,
> >> but that's beside the point. The LSM system provides mechanism for
> >> providing additional restrictions to existing security mechanisms.
> >> First, you create the security mechanism (e.g. enclaves) then you
> >> add LSM hooks so that security modules (e.g. SELinux) can apply
> >> their own policies in addition. In support of this, the LSM blob
> >> mechanism allows security modules to maintain their own information
> >> about the system components (e.g. file, inode, cred, task) they
> >> care about. The LSM infrastructure does not itself provide or
> >> support security data or policy. That's strictly for the modules
> >> to do.
> > Agreed!
> >
> > EMA doesn't dictate policies for sure. Is it considered "security data"? I'm not sure the definition of "security data" here. It does store some "data", something that multiple LSM modules would need to duplicate if not pulled into a common place. It is meant to be a "helper" data structure, just like the auditing code.
>
> Good example. You'll see that there is no audit code in the
> LSM infrastructure. None. No audit data, either. It's all taken
> care of in the audit system.
>
>
^ permalink raw reply
* Re: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Stephen Smalley @ 2019-06-29 1:22 UTC (permalink / raw)
To: Xing, Cedric
Cc: Stephen Smalley, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
Schaufler, Casey, jmorris@namei.org, luto@kernel.org,
jethro@fortanix.com, greg@enjellic.com,
jarkko.sakkinen@linux.intel.com, Christopherson, Sean J
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F6551C45F@ORSMSX116.amr.corp.intel.com>
On Fri, Jun 28, 2019 at 5:54 PM Xing, Cedric <cedric.xing@intel.com> wrote:
>
> > From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> > owner@vger.kernel.org] On Behalf Of Stephen Smalley
> > Sent: Friday, June 28, 2019 9:37 AM
> >
> > > lsm.ema.cache_decisions is on by default and
> > > could be turned off by appending “lsm.ema.cache_decisions=0” or
> > > “lsm.ema.cache_decisions=off” to the kernel command line.
> >
> > This seems problematic on a few fronts:
> >
> > - Specifying it as a boot parameter requires teaching admins / policy
> > developers to do something in addition to what they are already doing
> > when they want to create policy,
> >
> > - Limiting it to a boot parameter requires a reboot to change the mode
> > of operation, whereas SELinux offers runtime toggling of permissive mode
> > and even per-process (domain) permissive mode (and so does AppArmor),
>
> How about making a variable tunable via sysctl?
Better than a boot parameter but still not amenable to per-domain
permissive and still requires admins
to remember and perform an extra step before collecting denials.
>
> >
> > - In the cache_decisions=1 case, do we get any auditing at all? If not,
> > that's a problem. We want auditing not only when we are
> > generating/learning policy but also in operation.
>
> Currently it doesn't generate audit log, but I could add it, except it couldn't point to the exact file. But I can use the sigstruct file instead so administrators can at least tell which enclave violates the policy. Do you think it acceptable?
Seems prone to user confusion and lacks precision in why the denial occurred.
>
> >
> > - There is the potential for inconsistencies to arise between the
> > enforcement applied with different cache_decisions values.
>
> The enforcement will be consistent. The difference only lies in the logs.
>
> >
> > I would suggest that we just never cache the decision and accept the
> > cost if we are going to take this approach.
>
> This will also be a viable option. I don't think any enclaves would be comprised of a large number of files anyway. When SGX2 comes up, I think most enclaves will be instantiated from only one file and defer loading libraries at runtime. So in practice we are looking to keeping only one file open per enclave, which seems totally acceptable.
>
> Stephen (and everyone having an opinion on this), which way do you prefer? sysctl variable? Or never cache decisions?
I'd favor never caching decisions.
^ permalink raw reply
* Re: [RFC PATCH v4 10/12] security/selinux: Add enclave_load() implementation
From: Stephen Smalley @ 2019-06-29 1:15 UTC (permalink / raw)
To: Xing, Cedric
Cc: Stephen Smalley, Christopherson, Sean J, Jarkko Sakkinen,
linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, Roberts, William C, Schaufler, Casey,
James Morris, Hansen, Dave, Andy Lutomirski, Jethro Beekman,
Dr . Greg Wettstein
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F6551C422@ORSMSX116.amr.corp.intel.com>
On Fri, Jun 28, 2019 at 5:20 PM Xing, Cedric <cedric.xing@intel.com> wrote:
>
> > From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> > owner@vger.kernel.org] On Behalf Of Stephen Smalley
> > Sent: Friday, June 28, 2019 9:17 AM
> >
> > FWIW, adding new permissions only requires updating policy configuration,
> > not userspace code/tools. But in any event, we can reuse the execute-
> > related permissions if it makes sense but still consider introducing
> > additional, new permissions, possibly in a separate "enclave" security
> > class, if we want explicit control over enclave loading, e.g.
> > ENCLAVE__LOAD, ENCLAVE__INIT, etc.
>
> I'm not so familiar with SELinux tools so my apology in advance if I end up mixing up things.
>
> I'm not only talking about the new permissions, but also how to apply them to enclave files. Intel SGX SDK packages enclaves as .so files, and I guess that's the most straight forward way that most others would do. So if different permissions are defined, then user mode tools would have to distinguish enclaves from regular .so files in order to grant them different permissions. Would that be something extra to existing tools?
It doesn't require any userspace code changes. It is just a matter of
defining some configuration data in the policy for the new
permissions, one or more security labels (tags) for the SGX .so files,
and rules allowing access where desired, and then setting those
security labels on the SGX .so files (via the security.selinux
extended attribute on the files). Even the last part is generally
handled by updating a configuration specifying how files should be
labeled and then rpm automatically labels the files when created, or
you can manually restorecon them. If the new permissions are defined
in their own security class rather than reusing existing ones, then
they can even be defined entirely via a local or third party policy
module separate from the distro policy if desired/needed.
>
> >
> > One residual concern I have with the reuse of FILE__EXECUTE is using it
> > for the sigstruct file as the fallback case. If the sigstruct is always
> > part of the same file as the code, then it probably doesn't matter. But
> > otherwise, it is somewhat odd to have to allow the host process to
> > execute from the sigstruct file if it is only data (the signature).
>
> I agree with you. But do you think it a practical problem today? As far as I know, no one is deploying sigstructs in dedicated files. I'm just trying to touch as few things as possible until there's definitely a need to do so.
I don't know, and it wasn't clear to me from the earlier discussions.
If not and if it is acceptable to require them to be in files in the
first place, then perhaps it isn't necessary.
^ permalink raw reply
* Fwd: [PATCH v4 15/23] LSM: Specify which LSM to display
From: Stephen Smalley @ 2019-06-29 1:01 UTC (permalink / raw)
To: Stephen Smalley, Schaufler, Casey, James Morris, LSM List,
selinux, keescook, John Johansen, penguin-kernel, Paul Moore,
Casey Schaufler
In-Reply-To: <CAB9W1A1nwE7WBZqTe-GV8xNb83_B2ybV7cco++nfMjtDz9NJrg@mail.gmail.com>
On Fri, Jun 28, 2019 at 12:15 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 6/28/2019 7:45 AM, Stephen Smalley wrote:
> > On 6/26/19 3:22 PM, Casey Schaufler wrote:
> >> Create a new entry "display" in /proc/.../attr for controlling
> >> which LSM security information is displayed for a process.
> >> The name of an active LSM that supplies hooks for human readable
> >> data may be written to "display" to set the value. The name of
> >> the LSM currently in use can be read from "display".
> >> At this point there can only be one LSM capable of display
> >> active.
> >
> > IIUC, presently, at the end of the entire series,
> >
> > 1) Any process can change its display attribute to any enabled security module, and no security module can veto that change.
>
> That is correct. If a security module could hoard the display it
> could prevent user space from functioning in a multiple module
> environment.
>
> > 2) The display attribute is inherited across fork and exec, even execs that change credentials, and again no security module has control over the inheritance of this attribute.
>
> Also correct. Scripts don't work otherwise.
It's a security hole waiting to happen. Unprivileged caller sets its
display value to Smack on a mostly SELinux system that happens to
enable Smack too, then exec's a credential-changing SELinux-aware
program that uses one of the libselinux APIs to set one of the
/proc/self/attr attributes to a different SELinux context. Due to the
change in display, the SELinux-aware program instead ends up setting
one of the Smack attributes and therefore the desired SELinux context
is never applied to the process or file or socket or whatever.
>
> >
> > 3) Setting the display attribute affects more than just the contexts read or written by the process itself:
> > - Contexts reported in audit logs,
> > - Contexts passed across binder (generated in sender context, delivered to receiver),
> > - Contexts passed to NFS servers for new files,
> > - Contexts returned by NFS servers for existing files,
> > - Netlink-related contexts (?possibly generated in sender context rather than receiver context?),
> > - This list may not be complete.
>
> Any of which can be changed should a more rational behavior be proposed.
> One possibility is to use lsm='value',lsm='value' encoding for internal
> communications, but there's been considerable resistance to anything
> like that.
These are also security holes waiting to happen. Processes can use it
to hide their SELinux contexts from the audit logs, forge different
SELinux contexts on binder IPC, forge file contexts to which they have
no SELinux permissions on new files, ... All they need is stacking to
be enabled and one other module that helpfully lets them set attribute
values that look like SELinux contexts, and then they can set those
and switch their display at the right time.
>
> > 4) A security_secid_to_secctx() in one process' context (e.g. sender) or with one display value followed later by a security_secctx_to_secid() call in a different process' context (e.g. receiver) or with a different display value may ask a different security module to perform the reverse translation of the context than the forward translation.
>
> Do you have an example of where this might happen?
> Contexts are rarely used within the kernel. The usual
> behavior is to generate them, send them out to user space,
> and delete them. They get cached in some networking code,
> but not in cases where more than one (existing) security
> module will ever use them. Binder may be an exception, but
> only SELinux (currently) supports binder.
Haven't looked but I don't like the asymmetry of the interface.
Doesn't matter that only SELinux supports binder if you ever want any
other security module other than SELinux enabled at the same time as
SELinux.
>
>
> > Is that correct? If so, it seems problematic.
>
> Balancing backward compatibility with new behavior is hard!
> What would you suggest for audit logs? Should we put all LSM
> data in every record? Is NFS a concern for anyone not using
> SELinux?
Yes to all on audit if stacking is going to be real. And yes, I think
other security modules will care about NFS if they are serious.
>
> There is no user space that uses display, and it's going
> to take some time to work out all the kinks before we even
> think about teaching systemd about it.
That doesn't make it acceptable to introduce a mechanism that weakens
security now.
>
> >
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> fs/proc/base.c | 1 +
> >> security/security.c | 129 ++++++++++++++++++++++++++++++++++++++------
> >> 2 files changed, 113 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/fs/proc/base.c b/fs/proc/base.c
> >> index ddef482f1334..7bf70e041315 100644
> >> --- a/fs/proc/base.c
> >> +++ b/fs/proc/base.c
> >> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
> >> ATTR(NULL, "fscreate", 0666),
> >> ATTR(NULL, "keycreate", 0666),
> >> ATTR(NULL, "sockcreate", 0666),
> >> + ATTR(NULL, "display", 0666),
> >> #ifdef CONFIG_SECURITY_SMACK
> >> DIR("smack", 0555,
> >> proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> >> diff --git a/security/security.c b/security/security.c
> >> index 3180a6f30625..82e29c477fa4 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
> >> static struct kmem_cache *lsm_inode_cache;
> >> char *lsm_names;
> >> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> >> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> >> + .lbs_task = sizeof(int), /* slot number for the "display" LSM */
> >> +};
> >> /* Boot-time LSM user choice */
> >> static __initdata const char *chosen_lsm_order;
> >> @@ -423,8 +425,10 @@ static int lsm_append(const char *new, char **result)
> >> /*
> >> * Current index to use while initializing the lsmblob secid list.
> >> + * Pointers to the LSM id structures for local use.
> >> */
> >> static int lsm_slot;
> >> +static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
> >> /**
> >> * security_add_hooks - Add a modules hooks to the hook lists.
> >> @@ -444,6 +448,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> >> if (lsmid->slot == LSMBLOB_NEEDED) {
> >> if (lsm_slot >= LSMBLOB_ENTRIES)
> >> panic("%s Too many LSMs registered.\n", __func__);
> >> + lsm_slotlist[lsm_slot] = lsmid;
> >> lsmid->slot = lsm_slot++;
> >> init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
> >> lsmid->slot);
> >> @@ -564,6 +569,8 @@ int lsm_inode_alloc(struct inode *inode)
> >> */
> >> static int lsm_task_alloc(struct task_struct *task)
> >> {
> >> + int *display;
> >> +
> >> if (blob_sizes.lbs_task == 0) {
> >> task->security = NULL;
> >> return 0;
> >> @@ -572,6 +579,15 @@ static int lsm_task_alloc(struct task_struct *task)
> >> task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
> >> if (task->security == NULL)
> >> return -ENOMEM;
> >> +
> >> + /*
> >> + * The start of the task blob contains the "display" LSM slot number.
> >> + * Start with it set to the invalid slot number, indicating that the
> >> + * default first registered LSM be displayed.
> >> + */
> >> + display = task->security;
> >> + *display = LSMBLOB_INVALID;
> >> +
> >> return 0;
> >> }
> >> @@ -1563,14 +1579,24 @@ int security_file_open(struct file *file)
> >> int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
> >> {
> >> + int *odisplay = current->security;
> >> + int *ndisplay;
> >> int rc = lsm_task_alloc(task);
> >> - if (rc)
> >> + if (unlikely(rc))
> >> return rc;
> >> +
> >> rc = call_int_hook(task_alloc, 0, task, clone_flags);
> >> - if (unlikely(rc))
> >> + if (unlikely(rc)) {
> >> security_task_free(task);
> >> - return rc;
> >> + return rc;
> >> + }
> >> +
> >> + ndisplay = task->security;
> >> + if (ndisplay && odisplay)
> >> + *ndisplay = *odisplay;
> >> +
> >> + return 0;
> >> }
> >> void security_task_free(struct task_struct *task)
> >> @@ -1967,10 +1993,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
> >> char **value)
> >> {
> >> struct security_hook_list *hp;
> >> + int *display = current->security;
> >> + int slot = 0;
> >> +
> >> + if (!strcmp(name, "display")) {
> >> + /*
> >> + * lsm_slot will be 0 if there are no displaying modules.
> >> + */
> >> + if (lsm_slot == 0)
> >> + return -EINVAL;
> >> + if (*display != LSMBLOB_INVALID)
> >> + slot = *display;
> >> + *value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
> >> + if (*value)
> >> + return strlen(*value);
> >> + return -ENOMEM;
> >> + }
> >> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
> >> if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
> >> continue;
> >> + if (lsm == NULL && *display != LSMBLOB_INVALID &&
> >> + *display != hp->lsmid->slot)
> >> + continue;
> >> return hp->hook.getprocattr(p, name, value);
> >> }
> >> return -EINVAL;
> >> @@ -1980,10 +2025,46 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
> >> size_t size)
> >> {
> >> struct security_hook_list *hp;
> >> + char *term;
> >> + char *cp;
> >> + int *display = current->security;
> >> + int rc = -EINVAL;
> >> + int slot = 0;
> >> +
> >> + if (!strcmp(name, "display")) {
> >> + /*
> >> + * lsm_slot will be 0 if there are no displaying modules.
> >> + */
> >> + if (lsm_slot == 0 || size == 0)
> >> + return -EINVAL;
> >> + cp = kzalloc(size + 1, GFP_KERNEL);
> >> + if (cp == NULL)
> >> + return -ENOMEM;
> >> + memcpy(cp, value, size);
> >> +
> >> + term = strchr(cp, ' ');
> >> + if (term == NULL)
> >> + term = strchr(cp, '\n');
> >> + if (term != NULL)
> >> + *term = '\0';
> >> +
> >> + for (slot = 0; slot < lsm_slot; slot++)
> >> + if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
> >> + *display = lsm_slotlist[slot]->slot;
> >> + rc = size;
> >> + break;
> >> + }
> >> +
> >> + kfree(cp);
> >> + return rc;
> >> + }
> >> hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
> >> if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
> >> continue;
> >> + if (lsm == NULL && *display != LSMBLOB_INVALID &&
> >> + *display != hp->lsmid->slot)
> >> + continue;
> >> return hp->hook.setprocattr(name, value, size);
> >> }
> >> return -EINVAL;
> >> @@ -2003,15 +2084,15 @@ EXPORT_SYMBOL(security_ismaclabel);
> >> int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
> >> {
> >> struct security_hook_list *hp;
> >> - int rc;
> >> + int *display = current->security;
> >> hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
> >> if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> >> continue;
> >> - rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
> >> - secdata, seclen);
> >> - if (rc != 0)
> >> - return rc;
> >> + if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> >> + return hp->hook.secid_to_secctx(
> >> + blob->secid[hp->lsmid->slot],
> >> + secdata, seclen);
> >> }
> >> return 0;
> >> }
> >> @@ -2021,16 +2102,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
> >> struct lsmblob *blob)
> >> {
> >> struct security_hook_list *hp;
> >> - int rc;
> >> + int *display = current->security;
> >> lsmblob_init(blob, 0);
> >> hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
> >> if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> >> continue;
> >> - rc = hp->hook.secctx_to_secid(secdata, seclen,
> >> - &blob->secid[hp->lsmid->slot]);
> >> - if (rc != 0)
> >> - return rc;
> >> + if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> >> + return hp->hook.secctx_to_secid(secdata, seclen,
> >> + &blob->secid[hp->lsmid->slot]);
> >> }
> >> return 0;
> >> }
> >> @@ -2038,7 +2118,15 @@ EXPORT_SYMBOL(security_secctx_to_secid);
> >> void security_release_secctx(char *secdata, u32 seclen)
> >> {
> >> - call_void_hook(release_secctx, secdata, seclen);
> >> + struct security_hook_list *hp;
> >> + int *display = current->security;
> >> +
> >> + hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> >> + if (*display == LSMBLOB_INVALID ||
> >> + *display == hp->lsmid->slot) {
> >> + hp->hook.release_secctx(secdata, seclen);
> >> + return;
> >> + }
> >> }
> >> EXPORT_SYMBOL(security_release_secctx);
> >> @@ -2163,8 +2251,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
> >> int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
> >> int __user *optlen, unsigned len)
> >> {
> >> - return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> >> - optval, optlen, len);
> >> + int *display = current->security;
> >> + struct security_hook_list *hp;
> >> +
> >> + hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> >> + list)
> >> + if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> >> + return hp->hook.socket_getpeersec_stream(sock, optval,
> >> + optlen, len);
> >> + return -ENOPROTOOPT;
> >> }
> >> int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> >>
> >
^ permalink raw reply
* Re: [RFC PATCH v5 0/1] Add dm verity root hash pkcs7 sig validation.
From: Jaskaran Singh Khurana @ 2019-06-28 23:27 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-security-module, linux-kernel, linux-integrity,
linux-fsdevel, agk, snitzer, dm-devel, jmorris, scottsh, mpatocka,
gmazyland
In-Reply-To: <20190628203450.GD103946@gmail.com>
Hello Eric,
On Fri, 28 Jun 2019, Eric Biggers wrote:
>> In a datacenter like environment, this will protect the system from below
>> attacks:
>>
>> 1.Prevents attacker from deploying scripts that run arbitrary executables on the system.
>> 2.Prevents physically present malicious admin to run arbitrary code on the
>> machine.
>>
>> Regards,
>> Jaskaran
>
> So you are trying to protect against people who already have a root shell?
>
> Can't they just e.g. run /usr/bin/python and type in some Python code?
>
> Or run /usr/bin/curl and upload all your secret data to their server.
>
> - Eric
>
You are correct, it would not be feasible for a general purpose distro,
but for embedded systems and other cases where there is a more tightly
locked-down system.
Regards,
Jaskaran.
^ permalink raw reply
* RE: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Xing, Cedric @ 2019-06-28 22:29 UTC (permalink / raw)
To: Casey Schaufler, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org
Cc: Schaufler, Casey, jmorris@namei.org, luto@kernel.org,
jethro@fortanix.com, greg@enjellic.com, sds@tycho.nsa.gov,
jarkko.sakkinen@linux.intel.com, Christopherson, Sean J
In-Reply-To: <f6f16990-0291-c530-61dd-dcd26525285c@schaufler-ca.com>
> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
> Sent: Friday, June 28, 2019 10:22 AM
> >
> > And what's the concern here if it becomes part of the LSM
> infrastructure.
>
> The LSM infrastructure provides a framework for hooks
> and allocation of blobs. That's it. It's a layer for
> connecting system features like VFS, IPC and the IP stack
> to the security modules. It does not implement any policy
> of it's own. We are not going to implement SGX or any other
> mechanism within the LSM infrastructure.
EMA doesn't force/implement any policy either. It just supplements VMA.
>
> >>> It is going to be shared among all LSMs that would like to track
> >> enclave pages and their origins.
> >>
> >> That's true for InfiniBand, tun and sctp as well. Look at their
> >> implementations.
> > As far as I can tell, InfiniBand, tun and sctp, all of them seemed
> used inside SELinux only.
>
> So?
So they are NOT shared among LSMs, which are different than EMA.
>
> > If you had a chance to look at v1 of my series, I started by burying
> everything inside SELinux too. But Stephen pointed out such tracking
> would be needed by all LSMs so code duplication might be a concern. Thus
> I responded by moving it into LSM infrastructure.
>
> What you need to do is move all the lsm_ema code into its own
> place (which could be security/enclave). Manage your internal
> data as you like. LSMs (e.g. SELinux) can call your APIs if
> needed. If the LSMs need to store SGX information with the file
> structure they need to include that in the space they ask for in
> the file blob.
I thought subdirectories were for LSM modules. EMA is more like auditing code, which has a header in include/linux/ and an implementation in security/. Is that right?
>
>
> >>> And they could be extended to store more information as deemed
> >> appropriate by the LSM module.
> >>
> >> Which is what blobs are for, but that does not appear to be how
> >> you're using either the file blob or your new ema blob.
> > A lsm_ema_map pointer is stored in file->f_security.
>
> That's up to the individual security module to decide.
That's doable. The drawback is, if there are N LSM modules active, then the same information will be duplicated N times. Will that be a problem?
>
> > Each lsm_ema_map contains a list of lsm_ema structures. In my last
> patch, SELinux stores a ema_security_struct with every ema, by setting
> selinux_blob_sizes.lbs_ema_data to sizeof(ema_security_struct).
>
> You are managing the ema map lists. You don't need the LSM
> infrastructure to do that.
>
> > ema_security_struct is initialized in selinux_enclave_load(), and
> checked in enclave_mprotect(), which is a subroutine of
> selinux_file_mprotect(). BTW, it is alloced/freed automatically by LSM
> infrastructure in security_enclave_load()/security_file_free().
>
> Do you mean security_enclave_load()/security_enclave_free() ?
> There is no way you can possibly have sane behavior if you're
> allocation and free aren't tied to the same blob.
There's no security_*enclave*_free(). lsm_ema_map is allocated only for enclaves. But LSM doesn't know which file is an enclave, so the allocation is deferred until the first security_enclave_load(). security_file_free() frees the map if it isn't NULL.
>
> >>> The last patch of this series shows how to extend EMA inside
> SELinux.
> >> I don't see (but I admit the code doesn't make a lot of sense to me)
> >> anything you couldn't do in the SELinux code by adding data to the
> >> file blob. The data you're adding to the LSM infrastructure doesn't
> >> belong there, and it doesn't need to be there.
> > You are correct. My v1 did it inside SELinux.
> >
> > The key question I think is whether only SELinux needs it, or all LSMs
> need it. Stephen thought it was the latter (and I agree with him) so I
> moved it into the LSM infrastructure to be shared, just like the
> auditing code.
>
> You are both right that it doesn't belong in the SELinux code.
> It also doesn't belong as part of the LSM infrastructure.
Then what is your suggestion?
Is the code in security_enclave_load()/security_file_free() that bothers you? Because you think they shouldn't do anything more than just a single line of call_int/void_hooks()?
>
> >>>> Not acceptable for the LSM infrastructure. They
> >>>> are inconsistent with the way data is used there.
> >>> I'm not sure I understand this comment.
> >> It means that your definition and use of the lsm_ema_blob
> >> does not match the way other blobs are managed and used.
> >> The LSM infrastructure uses these entries in a very particular
> >> way, and you're trying to use it differently. Your might be
> >> able to change the rest of the enclave system to use it
> >> correctly, or you might be able to find a different place
> >> for it.
> > I'm still not sure why you think this (lbs_ema_data) is inconsistent
> with other blobs.
> >
> > Same as all other blobs, an LSM requests it by storing the needed size
> in it, and is assigned an offset, and the buffer is allocated/freed by
> the infrastructure. Am I missing anything?
>
> Yes. Aside from allocation and deletion the infrastructure does
> nothing with the blobs. The blobs are used only by the security
> modules. All other data is maintained and used elsewhere. SGX
> specific data needs to me maintained and managed elsewhere.
>
> >>> As I stated in the cover letter, the primary question is how to
> >> prevent SGX from being abused as a backdoor to make executable pages
> >> that would otherwise not be executable without SGX. Any LSM module
> >> unaware of that would leave that "hole" open. So tracking enclave
> pages
> >> will become a common task for all LSMs that care page protections,
> and
> >> that's why I place it inside LSM infrastructure.
> >>
> >> Page protections are an important part of many security features,
> >> but that's beside the point. The LSM system provides mechanism for
> >> providing additional restrictions to existing security mechanisms.
> >> First, you create the security mechanism (e.g. enclaves) then you
> >> add LSM hooks so that security modules (e.g. SELinux) can apply
> >> their own policies in addition. In support of this, the LSM blob
> >> mechanism allows security modules to maintain their own information
> >> about the system components (e.g. file, inode, cred, task) they
> >> care about. The LSM infrastructure does not itself provide or
> >> support security data or policy. That's strictly for the modules
> >> to do.
> > Agreed!
> >
> > EMA doesn't dictate policies for sure. Is it considered "security
> data"? I'm not sure the definition of "security data" here. It does
> store some "data", something that multiple LSM modules would need to
> duplicate if not pulled into a common place. It is meant to be a
> "helper" data structure, just like the auditing code.
>
> Good example. You'll see that there is no audit code in the
> LSM infrastructure. None. No audit data, either. It's all taken
> care of in the audit system.
Did you mean security/security.c didn't call into any audit functions? lsm_audit.c is located in security/ and its header in include/linux/ but you don't have a problem with them. Am I right?
IIUC, you want me to pack whatever inside security_enclave_load()/security_file_free() into some APIs to be called by individual LSM modules. But if you can pay a bit more attention to the code, an EMA can be inserted to the map only after *all* LSM modules have approved it. So if it is spread into individual LSMs and if there are multiple active LSMs, there could be inconsistence among LSMs if they each maintains its own map and makes different decisions on the same EMA at enclave_load(). I'm not saying that's unresolvable but definitely more error prone, besides wasting memory. Or do you have any practical suggestions?
^ permalink raw reply
* RE: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Xing, Cedric @ 2019-06-28 21:53 UTC (permalink / raw)
To: Stephen Smalley, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org
Cc: Schaufler, Casey, jmorris@namei.org, luto@kernel.org,
jethro@fortanix.com, greg@enjellic.com,
jarkko.sakkinen@linux.intel.com, Christopherson, Sean J
In-Reply-To: <f36415e6-5a69-b1b9-74b6-87a9af4508d3@tycho.nsa.gov>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> Sent: Friday, June 28, 2019 9:37 AM
>
> > lsm.ema.cache_decisions is on by default and
> > could be turned off by appending “lsm.ema.cache_decisions=0” or
> > “lsm.ema.cache_decisions=off” to the kernel command line.
>
> This seems problematic on a few fronts:
>
> - Specifying it as a boot parameter requires teaching admins / policy
> developers to do something in addition to what they are already doing
> when they want to create policy,
>
> - Limiting it to a boot parameter requires a reboot to change the mode
> of operation, whereas SELinux offers runtime toggling of permissive mode
> and even per-process (domain) permissive mode (and so does AppArmor),
How about making a variable tunable via sysctl?
>
> - In the cache_decisions=1 case, do we get any auditing at all? If not,
> that's a problem. We want auditing not only when we are
> generating/learning policy but also in operation.
Currently it doesn't generate audit log, but I could add it, except it couldn't point to the exact file. But I can use the sigstruct file instead so administrators can at least tell which enclave violates the policy. Do you think it acceptable?
>
> - There is the potential for inconsistencies to arise between the
> enforcement applied with different cache_decisions values.
The enforcement will be consistent. The difference only lies in the logs.
>
> I would suggest that we just never cache the decision and accept the
> cost if we are going to take this approach.
This will also be a viable option. I don't think any enclaves would be comprised of a large number of files anyway. When SGX2 comes up, I think most enclaves will be instantiated from only one file and defer loading libraries at runtime. So in practice we are looking to keeping only one file open per enclave, which seems totally acceptable.
Stephen (and everyone having an opinion on this), which way do you prefer? sysctl variable? Or never cache decisions?
^ permalink raw reply
* RE: [RFC PATCH v4 10/12] security/selinux: Add enclave_load() implementation
From: Xing, Cedric @ 2019-06-28 21:20 UTC (permalink / raw)
To: Stephen Smalley, Christopherson, Sean J, Jarkko Sakkinen
Cc: linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, Roberts, William C, Schaufler, Casey,
James Morris, Hansen, Dave, Andy Lutomirski, Jethro Beekman,
Dr . Greg Wettstein
In-Reply-To: <99499d1a-56eb-60b0-596c-6d24e38d4757@tycho.nsa.gov>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> Sent: Friday, June 28, 2019 9:17 AM
>
> FWIW, adding new permissions only requires updating policy configuration,
> not userspace code/tools. But in any event, we can reuse the execute-
> related permissions if it makes sense but still consider introducing
> additional, new permissions, possibly in a separate "enclave" security
> class, if we want explicit control over enclave loading, e.g.
> ENCLAVE__LOAD, ENCLAVE__INIT, etc.
I'm not so familiar with SELinux tools so my apology in advance if I end up mixing up things.
I'm not only talking about the new permissions, but also how to apply them to enclave files. Intel SGX SDK packages enclaves as .so files, and I guess that's the most straight forward way that most others would do. So if different permissions are defined, then user mode tools would have to distinguish enclaves from regular .so files in order to grant them different permissions. Would that be something extra to existing tools?
>
> One residual concern I have with the reuse of FILE__EXECUTE is using it
> for the sigstruct file as the fallback case. If the sigstruct is always
> part of the same file as the code, then it probably doesn't matter. But
> otherwise, it is somewhat odd to have to allow the host process to
> execute from the sigstruct file if it is only data (the signature).
I agree with you. But do you think it a practical problem today? As far as I know, no one is deploying sigstructs in dedicated files. I'm just trying to touch as few things as possible until there's definitely a need to do so.
^ permalink raw reply
* Re: [RFC PATCH v5 0/1] Add dm verity root hash pkcs7 sig validation.
From: Eric Biggers @ 2019-06-28 20:34 UTC (permalink / raw)
To: Jaskaran Singh Khurana
Cc: linux-security-module, linux-kernel, linux-integrity,
linux-fsdevel, agk, snitzer, dm-devel, jmorris, scottsh, mpatocka,
gmazyland
In-Reply-To: <alpine.LRH.2.21.1906281242110.2789@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.inter>
On Fri, Jun 28, 2019 at 12:45:11PM -0700, Jaskaran Singh Khurana wrote:
>
> Hello Eric,
> On Thu, 27 Jun 2019, Eric Biggers wrote:
>
> > On Wed, Jun 19, 2019 at 12:10:47PM -0700, Jaskaran Khurana wrote:
> > > This patch set adds in-kernel pkcs7 signature checking for the roothash of
> > > the dm-verity hash tree.
> > > The verification is to support cases where the roothash is not secured by
> > > Trusted Boot, UEFI Secureboot or similar technologies.
> > > One of the use cases for this is for dm-verity volumes mounted after boot,
> > > the root hash provided during the creation of the dm-verity volume has to
> > > be secure and thus in-kernel validation implemented here will be used
> > > before we trust the root hash and allow the block device to be created.
> > >
> > > Why we are doing validation in the Kernel?
> > >
> > > The reason is to still be secure in cases where the attacker is able to
> > > compromise the user mode application in which case the user mode validation
> > > could not have been trusted.
> > > The root hash signature validation in the kernel along with existing
> > > dm-verity implementation gives a higher level of confidence in the
> > > executable code or the protected data. Before allowing the creation of
> > > the device mapper block device the kernel code will check that the detached
> > > pkcs7 signature passed to it validates the roothash and the signature is
> > > trusted by builtin keys set at kernel creation. The kernel should be
> > > secured using Verified boot, UEFI Secure Boot or similar technologies so we
> > > can trust it.
> > >
> > > What about attacker mounting non dm-verity volumes to run executable
> > > code?
> > >
> > > This verification can be used to have a security architecture where a LSM
> > > can enforce this verification for all the volumes and by doing this it can
> > > ensure that all executable code runs from signed and trusted dm-verity
> > > volumes.
> > >
> > > Further patches will be posted that build on this and enforce this
> > > verification based on policy for all the volumes on the system.
> > >
> >
> > I don't understand your justification for this feature.
> >
> > If userspace has already been pwned severely enough for the attacker to be
> > executing arbitrary code with CAP_SYS_ADMIN (which is what the device mapper
> > ioctls need), what good are restrictions on loading more binaries from disk?
> >
> > Please explain your security model.
> >
> > - Eric
> >
>
> In a datacenter like environment, this will protect the system from below
> attacks:
>
> 1.Prevents attacker from deploying scripts that run arbitrary executables on the system.
> 2.Prevents physically present malicious admin to run arbitrary code on the
> machine.
>
> Regards,
> Jaskaran
So you are trying to protect against people who already have a root shell?
Can't they just e.g. run /usr/bin/python and type in some Python code?
Or run /usr/bin/curl and upload all your secret data to their server.
- Eric
^ permalink raw reply
* Re: [RFC PATCH v5 0/1] Add dm verity root hash pkcs7 sig validation.
From: Jaskaran Singh Khurana @ 2019-06-28 19:45 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-security-module, linux-kernel, linux-integrity,
linux-fsdevel, agk, snitzer, dm-devel, jmorris, scottsh, mpatocka,
gmazyland
In-Reply-To: <20190628040041.GB673@sol.localdomain>
Hello Eric,
On Thu, 27 Jun 2019, Eric Biggers wrote:
> On Wed, Jun 19, 2019 at 12:10:47PM -0700, Jaskaran Khurana wrote:
>> This patch set adds in-kernel pkcs7 signature checking for the roothash of
>> the dm-verity hash tree.
>> The verification is to support cases where the roothash is not secured by
>> Trusted Boot, UEFI Secureboot or similar technologies.
>> One of the use cases for this is for dm-verity volumes mounted after boot,
>> the root hash provided during the creation of the dm-verity volume has to
>> be secure and thus in-kernel validation implemented here will be used
>> before we trust the root hash and allow the block device to be created.
>>
>> Why we are doing validation in the Kernel?
>>
>> The reason is to still be secure in cases where the attacker is able to
>> compromise the user mode application in which case the user mode validation
>> could not have been trusted.
>> The root hash signature validation in the kernel along with existing
>> dm-verity implementation gives a higher level of confidence in the
>> executable code or the protected data. Before allowing the creation of
>> the device mapper block device the kernel code will check that the detached
>> pkcs7 signature passed to it validates the roothash and the signature is
>> trusted by builtin keys set at kernel creation. The kernel should be
>> secured using Verified boot, UEFI Secure Boot or similar technologies so we
>> can trust it.
>>
>> What about attacker mounting non dm-verity volumes to run executable
>> code?
>>
>> This verification can be used to have a security architecture where a LSM
>> can enforce this verification for all the volumes and by doing this it can
>> ensure that all executable code runs from signed and trusted dm-verity
>> volumes.
>>
>> Further patches will be posted that build on this and enforce this
>> verification based on policy for all the volumes on the system.
>>
>
> I don't understand your justification for this feature.
>
> If userspace has already been pwned severely enough for the attacker to be
> executing arbitrary code with CAP_SYS_ADMIN (which is what the device mapper
> ioctls need), what good are restrictions on loading more binaries from disk?
>
> Please explain your security model.
>
> - Eric
>
In a datacenter like environment, this will protect the system from below
attacks:
1.Prevents attacker from deploying scripts that run arbitrary executables on the system.
2.Prevents physically present malicious admin to run arbitrary code on the
machine.
Regards,
Jaskaran
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-28 18:47 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Smalley, James Morris, linux-security, LKML, Linux API,
David Howells, Alexei Starovoitov, Network Development,
Chun-Yi Lee, Daniel Borkmann, LSM List
In-Reply-To: <CALCETrXwt43w6rQY6zt0J_3HOaad=+E5PushJNdSOZDBuaYV+Q@mail.gmail.com>
On Thu, Jun 27, 2019 at 4:27 PM Andy Lutomirski <luto@kernel.org> wrote:
> They're really quite similar in my mind. Certainly some things in the
> "integrity" category give absolutely trivial control over the kernel
> (e.g. modules) while others make it quite challenging (ioperm), but
> the end result is very similar. And quite a few "confidentiality"
> things genuinely do allow all kernel memory to be read.
>
> I agree that finer-grained distinctions could be useful. My concern is
> that it's a tradeoff, and the other end of the tradeoff is an ABI
> stability issue. If someone decides down the road that some feature
> that is currently "integrity" can be split into a narrow "integrity"
> feature and a "confidentiality" feature then, if the user policy knows
> about the individual features, there's a risk of breaking people's
> systems. If we keep the fine-grained control, do we have a clear
> compatibility story?
My preference right now is to retain the fine-grained aspect of things
in the internal API, simply because it'll be more annoying to add it
back later if we want to. I don't want to expose it via the Lockdown
user facing API for the reasons you've described, but it's not
impossible that another LSM would find a way to do this reasonably.
Does it seem reasonable to punt this discussion out to the point where
another LSM tries to do something with this information, based on the
implementation they're attempting?
^ permalink raw reply
* Re: [PATCH v4 15/23] LSM: Specify which LSM to display
From: John Johansen @ 2019-06-28 18:08 UTC (permalink / raw)
To: Casey Schaufler, Stephen Smalley, casey.schaufler, jmorris,
linux-security-module, selinux
Cc: keescook, penguin-kernel, paul
In-Reply-To: <7944672e-a590-44a3-743a-48c1785a5464@schaufler-ca.com>
On 6/28/19 9:15 AM, Casey Schaufler wrote:
> On 6/28/2019 7:45 AM, Stephen Smalley wrote:
>> On 6/26/19 3:22 PM, Casey Schaufler wrote:
>>> Create a new entry "display" in /proc/.../attr for controlling
>>> which LSM security information is displayed for a process.
>>> The name of an active LSM that supplies hooks for human readable
>>> data may be written to "display" to set the value. The name of
>>> the LSM currently in use can be read from "display".
>>> At this point there can only be one LSM capable of display
>>> active.
>>
>> IIUC, presently, at the end of the entire series,
>>
>> 1) Any process can change its display attribute to any enabled security module, and no security module can veto that change.
>
> That is correct. If a security module could hoard the display it
> could prevent user space from functioning in a multiple module
> environment.
>
It should be noted that this is also just for legacy, we agreed
last year that smack and apparmor would move to new none shared
interfaces by default, and ideally other LSMs would as well.
Smack has already added its process dir and apparmor has its
in apparmor-next
The display LSM allows for the current interfaces to be used
in a stacking situation for things like LSM in legacy container.
>> 2) The display attribute is inherited across fork and exec, even execs that change credentials, and again no security module has control over the inheritance of this attribute.
>
> Also correct. Scripts don't work otherwise.
>
>>
>> 3) Setting the display attribute affects more than just the contexts read or written by the process itself:
>> - Contexts reported in audit logs,
>> - Contexts passed across binder (generated in sender context, delivered to receiver),
>> - Contexts passed to NFS servers for new files,
>> - Contexts returned by NFS servers for existing files,
>> - Netlink-related contexts (?possibly generated in sender context rather than receiver context?),
>> - This list may not be complete.
>
> Any of which can be changed should a more rational behavior be proposed.
> One possibility is to use lsm='value',lsm='value' encoding for internal
> communications, but there's been considerable resistance to anything
> like that.
>
This is the part of the patchset that I am least happy with but
it is a hard problem, and so far using display has been the only
option that has been even sort of agreed to.
>> 4) A security_secid_to_secctx() in one process' context (e.g. sender) or with one display value followed later by a security_secctx_to_secid() call in a different process' context (e.g. receiver) or with a different display value may ask a different security module to perform the reverse translation of the context than the forward translation.
>
> Do you have an example of where this might happen?
> Contexts are rarely used within the kernel. The usual
> behavior is to generate them, send them out to user space,
> and delete them. They get cached in some networking code,
> but not in cases where more than one (existing) security
> module will ever use them. Binder may be an exception, but
> only SELinux (currently) supports binder.
>
>
>> Is that correct? If so, it seems problematic.
>
> Balancing backward compatibility with new behavior is hard!
> What would you suggest for audit logs? Should we put all LSM
> data in every record?
If we could it would be better, but again how to do it. Every
option presented has been rejected.
> Is NFS a concern for anyone not using
> SELinux?
>
Yes. Just because it isn't currently used doesn't mean it isn't
a concern or desired.
> There is no user space that uses display, and it's going
> to take some time to work out all the kinks before we even
> think about teaching systemd about it.
>
I'm not even sure we want to teach systemd et al. about it,
it would be far better to teach them about
/proc/*/attr/{smack,apparmor,...}/*
But yes its going to take time to get the userspace updated.
^ permalink raw reply
* Re: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Casey Schaufler @ 2019-06-28 17:22 UTC (permalink / raw)
To: Xing, Cedric, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
casey
Cc: Schaufler, Casey, jmorris@namei.org, luto@kernel.org,
jethro@fortanix.com, greg@enjellic.com, sds@tycho.nsa.gov,
jarkko.sakkinen@linux.intel.com, Christopherson, Sean J
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F6551B975@ORSMSX116.amr.corp.intel.com>
On 6/27/2019 5:47 PM, Xing, Cedric wrote:
>> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
>> Sent: Thursday, June 27, 2019 4:37 PM
>>>> This code should not be mixed in with the LSM infrastructure.
>>>> It should all be contained in its own module, under security/enclave.
>>> lsm_ema is *intended* to be part of the LSM infrastructure.
>> That's not going to fly, not for a minute.
> Why not, if there's a need for it?
>
> And what's the concern here if it becomes part of the LSM infrastructure.
The LSM infrastructure provides a framework for hooks
and allocation of blobs. That's it. It's a layer for
connecting system features like VFS, IPC and the IP stack
to the security modules. It does not implement any policy
of it's own. We are not going to implement SGX or any other
mechanism within the LSM infrastructure.
>>> It is going to be shared among all LSMs that would like to track
>> enclave pages and their origins.
>>
>> That's true for InfiniBand, tun and sctp as well. Look at their
>> implementations.
> As far as I can tell, InfiniBand, tun and sctp, all of them seemed used inside SELinux only.
So?
> If you had a chance to look at v1 of my series, I started by burying everything inside SELinux too. But Stephen pointed out such tracking would be needed by all LSMs so code duplication might be a concern. Thus I responded by moving it into LSM infrastructure.
What you need to do is move all the lsm_ema code into its own
place (which could be security/enclave). Manage your internal
data as you like. LSMs (e.g. SELinux) can call your APIs if
needed. If the LSMs need to store SGX information with the file
structure they need to include that in the space they ask for in
the file blob.
>>> And they could be extended to store more information as deemed
>> appropriate by the LSM module.
>>
>> Which is what blobs are for, but that does not appear to be how
>> you're using either the file blob or your new ema blob.
> A lsm_ema_map pointer is stored in file->f_security.
That's up to the individual security module to decide.
> Each lsm_ema_map contains a list of lsm_ema structures. In my last patch, SELinux stores a ema_security_struct with every ema, by setting selinux_blob_sizes.lbs_ema_data to sizeof(ema_security_struct).
You are managing the ema map lists. You don't need the LSM
infrastructure to do that.
> ema_security_struct is initialized in selinux_enclave_load(), and checked in enclave_mprotect(), which is a subroutine of selinux_file_mprotect(). BTW, it is alloced/freed automatically by LSM infrastructure in security_enclave_load()/security_file_free().
Do you mean security_enclave_load()/security_enclave_free() ?
There is no way you can possibly have sane behavior if you're
allocation and free aren't tied to the same blob.
>>> The last patch of this series shows how to extend EMA inside SELinux.
>> I don't see (but I admit the code doesn't make a lot of sense to me)
>> anything you couldn't do in the SELinux code by adding data to the
>> file blob. The data you're adding to the LSM infrastructure doesn't
>> belong there, and it doesn't need to be there.
> You are correct. My v1 did it inside SELinux.
>
> The key question I think is whether only SELinux needs it, or all LSMs need it. Stephen thought it was the latter (and I agree with him) so I moved it into the LSM infrastructure to be shared, just like the auditing code.
You are both right that it doesn't belong in the SELinux code.
It also doesn't belong as part of the LSM infrastructure.
>>>> Not acceptable for the LSM infrastructure. They
>>>> are inconsistent with the way data is used there.
>>> I'm not sure I understand this comment.
>> It means that your definition and use of the lsm_ema_blob
>> does not match the way other blobs are managed and used.
>> The LSM infrastructure uses these entries in a very particular
>> way, and you're trying to use it differently. Your might be
>> able to change the rest of the enclave system to use it
>> correctly, or you might be able to find a different place
>> for it.
> I'm still not sure why you think this (lbs_ema_data) is inconsistent with other blobs.
>
> Same as all other blobs, an LSM requests it by storing the needed size in it, and is assigned an offset, and the buffer is allocated/freed by the infrastructure. Am I missing anything?
Yes. Aside from allocation and deletion the infrastructure does
nothing with the blobs. The blobs are used only by the security
modules. All other data is maintained and used elsewhere. SGX
specific data needs to me maintained and managed elsewhere.
>>> As I stated in the cover letter, the primary question is how to
>> prevent SGX from being abused as a backdoor to make executable pages
>> that would otherwise not be executable without SGX. Any LSM module
>> unaware of that would leave that "hole" open. So tracking enclave pages
>> will become a common task for all LSMs that care page protections, and
>> that's why I place it inside LSM infrastructure.
>>
>> Page protections are an important part of many security features,
>> but that's beside the point. The LSM system provides mechanism for
>> providing additional restrictions to existing security mechanisms.
>> First, you create the security mechanism (e.g. enclaves) then you
>> add LSM hooks so that security modules (e.g. SELinux) can apply
>> their own policies in addition. In support of this, the LSM blob
>> mechanism allows security modules to maintain their own information
>> about the system components (e.g. file, inode, cred, task) they
>> care about. The LSM infrastructure does not itself provide or
>> support security data or policy. That's strictly for the modules
>> to do.
> Agreed!
>
> EMA doesn't dictate policies for sure. Is it considered "security data"? I'm not sure the definition of "security data" here. It does store some "data", something that multiple LSM modules would need to duplicate if not pulled into a common place. It is meant to be a "helper" data structure, just like the auditing code.
Good example. You'll see that there is no audit code in the
LSM infrastructure. None. No audit data, either. It's all taken
care of in the audit system.
^ permalink raw reply
* Re: [RFC PATCH v5 1/1] Add dm verity root hash pkcs7 sig validation.
From: Jaskaran Singh Khurana @ 2019-06-28 17:03 UTC (permalink / raw)
To: Milan Broz
Cc: Eric Biggers, linux-security-module, linux-kernel,
linux-integrity, linux-fsdevel, agk, snitzer, dm-devel, jmorris,
scottsh, mpatocka
In-Reply-To: <264565b3-ff3c-29c0-7df0-d8ff061087d3@gmail.com>
Hello Eric/Milan,
On Fri, 28 Jun 2019, Milan Broz wrote:
> On 28/06/2019 05:00, Eric Biggers wrote:
>>> Hello Eric,
>>>
>>> This started with a config (see V4). We didnot want scripts that pass this
>>> parameter to suddenly stop working if for some reason the verification is
>>> turned off so the optional parameter was just parsed and no validation
>>> happened if the CONFIG was turned off. This was changed to a commandline
>>> parameter after feedback from the community, so I would prefer to keep it
>>> *now* as commandline parameter. Let me know if you are OK with this.
>>>
>>> Regards,
>>> JK
>>
>> Sorry, I haven't been following the whole discussion. (BTW, you sent out
>> multiple versions both called "v4", and using a cover letter for a single patch
>> makes it unnecessarily difficult to review.) However, it appears Milan were
>> complaining about the DM_VERITY_VERIFY_ROOTHASH_SIG_FORCE option which set the
>> policy for signature verification, *not* the DM_VERITY_VERIFY_ROOTHASH_SIG
>> option which enabled support for signature verification. Am I missing
>> something? You can have a module parameter which controls the "signatures
>> required" setting, while also allowing people to compile out kernel support for
>> the signature verification feature.
>
> Yes, this was exactly my point.
>
> I think I even mention in some reply to use exactly the same config Makefile logic
> as for FEC - to allow completely compile it out of the source:
>
> ifeq ($(CONFIG_DM_VERITY_FEC),y)
> dm-verity-objs += dm-verity-fec.o
> endif
>
>> Sure, it means that the signature verification support won't be guaranteed to be
>> present when dm-verity is. But the same is true of the hash algorithm (e.g.
>> sha512), and of the forward error correction feature. Since the signature
>> verification is nontrivial and pulls in a lot of other kernel code which might
>> not be otherwise needed (via SYSTEM_DATA_VERIFICATION), it seems a natural
>> candidate for putting the support behind a Kconfig option.
>
> On the other side, dm-verity is meant for a system verification, so if it depends
> on SYSTEM_DATA_VERIFICATION is ... not so surprising :)
>
> But the change above is quite easy and while we already have FEC as config option,
> perhaps let's do it the same here.
>
> Milan
>
Yes, I will make this change. Please consider this discussion as resolved.
Thanks.
Regards,
Jaskaran.
^ permalink raw reply
* Re: [PATCH 0/6] Mount and superblock notifications [ver #5]
From: David Howells @ 2019-06-28 16:47 UTC (permalink / raw)
Cc: dhowells, viro, Casey Schaufler, Stephen Smalley,
Greg Kroah-Hartman, nicolas.dichtel, raven, Christian Brauner,
keyrings, linux-usb, linux-security-module, linux-fsdevel,
linux-api, linux-block, linux-kernel
In-Reply-To: <156173701358.15650.8735203424342507015.stgit@warthog.procyon.org.uk>
David Howells <dhowells@redhat.com> wrote:
> The patches can be found here also:
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications
Actually, that should be:
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications-mount
David
^ 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