* Re: [RFC PATCH v4 00/12] security: x86/sgx: SGX vs. LSM
From: Xing, Cedric @ 2019-07-12 0:08 UTC (permalink / raw)
To: Jarkko Sakkinen, Stephen Smalley
Cc: Sean Christopherson, linux-sgx, linux-security-module, selinux,
Bill Roberts, Casey Schaufler, James Morris, Dave Hansen,
Andy Lutomirski, Jethro Beekman, Dr . Greg Wettstein
In-Reply-To: <20190711175151.xwxdfbvxbm6esymk@linux.intel.com>
On 7/11/2019 10:51 AM, Jarkko Sakkinen wrote:
> On Thu, Jul 11, 2019 at 10:32:41AM -0400, Stephen Smalley wrote:
>> The existing permissions don't map cleanly to SGX but I think Sean and
>> Cedric were trying to make a best-effort approximation to the underlying
>> concepts in a manner that permits control over the introduction of
>> executable content.
>>
>> Sure, the existing EXECMOD check is only applied today when there is an
>> attempt to make executable a previously modified (detected based on COW
>> having occurred) private file mapping. But the general notion of
>> controlling the ability to execute modified content is still meaningful.
>
> OK to summarize EXECMOD does not connect with SGX in any possible way
> but SGX needs something that mimics EXECMOD behaviour?
Stephen may correct me if I'm wrong. EXECMOD is granted to files, to
indicate the bearer contains self-modifying code (or text relocation).
So if it applies the enclaves, there are two aspects of it:
(1) An enclave may be loaded from multiple image files, among which some
may contain self-modifying code and hence would require EXECMOD on each
of them. At runtime, W->X will be allowed/denied for pages loaded from
image files having/not having EXECMOD.
(2) But there are pages not loaded from any files - e.g. pages EAUG'ed
at runtime. We are trying to use the file containing SIGSTRUCT as the
"proxy file" - i.e. EXECMOD on the proxy file indicates the enclave may
load code into EAUG'ed pages at runtime.
(3) Well, this is not an aspect of EXECMOD. Yet there's another category
of pages - pages EADD'ed from anonymous memory. They are different than
EAUG'ed pages in that their contents are supplied by untrusted code. How
to control their accesses is still being debated. My argument was that
the source pages must be executable before they could be loaded
executable in EPC. Andy argued that SIGSTRUCT alone could be considered
sufficient validation on the contents in certain usages, so both Sean
and I had proposed PROCESS2__ENCLAVE_EXECANON as a new permission. But
for the 1st upstream I think we all agree to do just the minimum until
real requirements come up. After all, adding is generally easier than
taking away existing things.
^ permalink raw reply
* Some LSM and SGX remarks before parting of for two weeks
From: Jarkko Sakkinen @ 2019-07-12 2:10 UTC (permalink / raw)
To: linux-sgx, linux-security-module
Before going to a two week vacation (sending v21 today), I'll make some
remarks on SGX and LSM's:
1. Currently all patch sets proposing LSM changes are missing a problem
statement and describe a solution to an undescribed problem.
2. When speaking of SELinux I haven't seen any draft's on how would
define a policy module with the new constructs. Does not have to
be a full policy modules but more like snippets demosntrating that
"this would work".
3. All the SELinux discussion is centered on type based policies.
Potentially one could isolate enclaves with some UBAC or RBAC
based model. That could be good first step and might not even
require LSM changes. Type based models could be introduced
post upstreaming. No deep analysis on this, but at least this
option should ruled out at minimum before striving into type
based security model.
I guess the problem statement is more or less that since with DAC you
would have to allow to use mmap() and mprotect() to change anything
to X, even to the point that you can do WX, one needs a MAC to
somehow fix this.
Was not that hard, was it? Should be refined though with some context
why SGX requires this to not so SGX-oriented audience.
Even with just DAC this could be potentially sorted out with UBAC or
RBAC based solution e.g. have an SGID for enclave "builders" and the
device itself.
Repeating myself but type based security can be always added
aftewards.
/Jarkko
^ permalink raw reply
* [PATCH 11/12] Documentation/x86: repointer docs to Documentation/arch/
From: Alex Shi @ 2019-07-12 2:20 UTC (permalink / raw)
To: linux-doc, Jonathan Corbet
Cc: Alex Shi, linux-kernel, linux-stm32, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-omap, linux-fbdev,
linux-samsung-soc, linux-ia64, linux-mips, linux-parisc,
linux-scsi, linux-s390, kvm, linux-sh, Tony Luck, H. Peter Anvin,
x86, Peter Zijlstra, Changbin Du, xen-devel, platform-driver-x86,
virtualization, netdev, linux-security-module
In-Reply-To: <20190712022018.27989-1-alex.shi@linux.alibaba.com>
Since we move Documentation/x86 docs to Documentation/arch/x86
dir, redirect the doc pointer to them.
Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Tony Luck <tony.luck@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Changbin Du <changbin.du@intel.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xenproject.org
Cc: platform-driver-x86@vger.kernel.org
Cc: kvm@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
---
Documentation/admin-guide/hw-vuln/mds.rst | 2 +-
Documentation/admin-guide/kernel-parameters.rst | 6 +++---
Documentation/admin-guide/kernel-parameters.txt | 8 ++++----
Documentation/admin-guide/ras.rst | 2 +-
Documentation/arch/x86/x86_64/5level-paging.rst | 2 +-
Documentation/arch/x86/x86_64/boot-options.rst | 4 ++--
.../arch/x86/x86_64/fake-numa-for-cpusets.rst | 2 +-
Documentation/devicetree/booting-without-of.txt | 2 +-
Documentation/sysctl/kernel.txt | 4 ++--
MAINTAINERS | 4 ++--
arch/arm/Kconfig | 2 +-
arch/x86/Kconfig | 12 ++++++------
arch/x86/Kconfig.debug | 2 +-
| 2 +-
arch/x86/entry/entry_64.S | 2 +-
arch/x86/include/asm/bootparam_utils.h | 2 +-
arch/x86/include/asm/page_64_types.h | 2 +-
arch/x86/include/asm/pgtable_64_types.h | 2 +-
arch/x86/kernel/cpu/microcode/amd.c | 2 +-
arch/x86/kernel/kexec-bzimage64.c | 2 +-
arch/x86/kernel/pci-dma.c | 2 +-
arch/x86/mm/tlb.c | 2 +-
arch/x86/platform/pvh/enlighten.c | 2 +-
drivers/vhost/vhost.c | 2 +-
security/Kconfig | 2 +-
tools/include/linux/err.h | 2 +-
tools/objtool/Documentation/stack-validation.txt | 4 ++--
27 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/Documentation/admin-guide/hw-vuln/mds.rst b/Documentation/admin-guide/hw-vuln/mds.rst
index e3a796c0d3a2..303228380fdc 100644
--- a/Documentation/admin-guide/hw-vuln/mds.rst
+++ b/Documentation/admin-guide/hw-vuln/mds.rst
@@ -58,7 +58,7 @@ Because the buffers are potentially shared between Hyper-Threads cross
Hyper-Thread attacks are possible.
Deeper technical information is available in the MDS specific x86
-architecture section: :ref:`Documentation/x86/mds.rst <mds>`.
+architecture section: :ref:`Documentation/arch/x86/mds.rst <mds>`.
Attack scenarios
diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index dc283dcffae8..7c32484811c8 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -167,7 +167,7 @@ parameter is applicable::
X86-32 X86-32, aka i386 architecture is enabled.
X86-64 X86-64 architecture is enabled.
More X86-64 boot options can be found in
- Documentation/x86/x86_64/boot-options.rst.
+ Documentation/arch/x86/x86_64/boot-options.rst.
X86 Either 32-bit or 64-bit x86 (same as X86-32+X86-64)
X86_UV SGI UV support is enabled.
XEN Xen support is enabled
@@ -181,10 +181,10 @@ In addition, the following text indicates that the option::
Parameters denoted with BOOT are actually interpreted by the boot
loader, and have no meaning to the kernel directly.
Do not modify the syntax of boot loader parameters without extreme
-need or coordination with <Documentation/x86/boot.rst>.
+need or coordination with <Documentation/arch/x86/boot.rst>.
There are also arch-specific kernel-parameters not documented here.
-See for example <Documentation/x86/x86_64/boot-options.rst>.
+See for example <Documentation/arch/x86/x86_64/boot-options.rst>.
Note that ALL kernel parameters listed below are CASE SENSITIVE, and that
a trailing = on the name of any parameter states that that parameter will
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4ceb4691245b..d9eb5895ea9e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -963,7 +963,7 @@
for details.
nompx [X86] Disables Intel Memory Protection Extensions.
- See Documentation/x86/intel_mpx.rst for more
+ See Documentation/arch/x86/intel_mpx.rst for more
information about the feature.
nopku [X86] Disable Memory Protection Keys CPU feature found
@@ -2380,7 +2380,7 @@
mce [X86-32] Machine Check Exception
- mce=option [X86-64] See Documentation/x86/x86_64/boot-options.rst
+ mce=option [X86-64] See Documentation/arch/x86/x86_64/boot-options.rst
md= [HW] RAID subsystems devices and level
See Documentation/admin-guide/md.rst.
@@ -3526,7 +3526,7 @@
See Documentation/blockdev/paride.txt.
pirq= [SMP,APIC] Manual mp-table setup
- See Documentation/x86/i386/IO-APIC.rst.
+ See Documentation/arch/x86/i386/IO-APIC.rst.
plip= [PPT,NET] Parallel port network link
Format: { parport<nr> | timid | 0 }
@@ -5058,7 +5058,7 @@
Can be used multiple times for multiple devices.
vga= [BOOT,X86-32] Select a particular video mode
- See Documentation/x86/boot.rst and
+ See Documentation/arch/x86/boot.rst and
Documentation/svga.txt.
Use vga=ask for menu.
This is actually a boot loader parameter; the value is
diff --git a/Documentation/admin-guide/ras.rst b/Documentation/admin-guide/ras.rst
index 2b20f5f7380d..2d86862458aa 100644
--- a/Documentation/admin-guide/ras.rst
+++ b/Documentation/admin-guide/ras.rst
@@ -199,7 +199,7 @@ Architecture (MCA)\ [#f3]_.
mode).
.. [#f3] For more details about the Machine Check Architecture (MCA),
- please read Documentation/x86/x86_64/machinecheck.rst at the Kernel tree.
+ please read Documentation/arch/x86/x86_64/machinecheck.rst at the Kernel tree.
EDAC - Error Detection And Correction
*************************************
diff --git a/Documentation/arch/x86/x86_64/5level-paging.rst b/Documentation/arch/x86/x86_64/5level-paging.rst
index 44856417e6a5..000809878403 100644
--- a/Documentation/arch/x86/x86_64/5level-paging.rst
+++ b/Documentation/arch/x86/x86_64/5level-paging.rst
@@ -20,7 +20,7 @@ physical address space. This "ought to be enough for anybody" ©.
QEMU 2.9 and later support 5-level paging.
Virtual memory layout for 5-level paging is described in
-Documentation/x86/x86_64/mm.rst
+Documentation/arch/x86/x86_64/mm.rst
Enabling 5-level paging
diff --git a/Documentation/arch/x86/x86_64/boot-options.rst b/Documentation/arch/x86/x86_64/boot-options.rst
index 6a4285a3c7a4..2a093128b28f 100644
--- a/Documentation/arch/x86/x86_64/boot-options.rst
+++ b/Documentation/arch/x86/x86_64/boot-options.rst
@@ -9,7 +9,7 @@ only the AMD64 specific ones are listed here.
Machine check
=============
-Please see Documentation/x86/x86_64/machinecheck.rst for sysfs runtime tunables.
+Please see Documentation/arch/x86/x86_64/machinecheck.rst for sysfs runtime tunables.
mce=off
Disable machine check
@@ -89,7 +89,7 @@ APICs
Don't use the local APIC (alias for i386 compatibility)
pirq=...
- See Documentation/x86/i386/IO-APIC.rst
+ See Documentation/arch/x86/i386/IO-APIC.rst
noapictimer
Don't set up the APIC timer
diff --git a/Documentation/arch/x86/x86_64/fake-numa-for-cpusets.rst b/Documentation/arch/x86/x86_64/fake-numa-for-cpusets.rst
index 30108684ae87..d960f5cac258 100644
--- a/Documentation/arch/x86/x86_64/fake-numa-for-cpusets.rst
+++ b/Documentation/arch/x86/x86_64/fake-numa-for-cpusets.rst
@@ -18,7 +18,7 @@ For more information on the features of cpusets, see
Documentation/cgroup-v1/cpusets.rst.
There are a number of different configurations you can use for your needs. For
more information on the numa=fake command line option and its various ways of
-configuring fake nodes, see Documentation/x86/x86_64/boot-options.rst.
+configuring fake nodes, see Documentation/arch/x86/x86_64/boot-options.rst.
For the purposes of this introduction, we'll assume a very primitive NUMA
emulation setup of "numa=fake=4*512,". This will split our system memory into
diff --git a/Documentation/devicetree/booting-without-of.txt b/Documentation/devicetree/booting-without-of.txt
index 58d606fca7eb..066778cbbdcb 100644
--- a/Documentation/devicetree/booting-without-of.txt
+++ b/Documentation/devicetree/booting-without-of.txt
@@ -277,7 +277,7 @@ it with special cases.
the decompressor (the real mode entry point goes to the same 32bit
entry point once it switched into protected mode). That entry point
supports one calling convention which is documented in
- Documentation/x86/boot.rst
+ Documentation/arch/x86/boot.rst
The physical pointer to the device-tree block (defined in chapter II)
is passed via setup_data which requires at least boot protocol 2.09.
The type filed is defined as
diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 1b2fe17cd2fa..b3e3c56bdab8 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -154,7 +154,7 @@ is 0x15 and the full version number is 0x234, this file will contain
the value 340 = 0x154.
See the type_of_loader and ext_loader_type fields in
-Documentation/x86/boot.rst for additional information.
+Documentation/arch/x86/boot.rst for additional information.
==============================================================
@@ -166,7 +166,7 @@ The complete bootloader version number. In the example above, this
file will contain the value 564 = 0x234.
See the type_of_loader and ext_loader_ver fields in
-Documentation/x86/boot.rst for additional information.
+Documentation/arch/x86/boot.rst for additional information.
==============================================================
diff --git a/MAINTAINERS b/MAINTAINERS
index 84448d5838b7..e1aa61c72cb1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13351,7 +13351,7 @@ L: linux-kernel@vger.kernel.org
S: Supported
F: arch/x86/kernel/cpu/resctrl/
F: arch/x86/include/asm/resctrl_sched.h
-F: Documentation/x86/resctrl*
+F: Documentation/arch/x86/resctrl*
READ-COPY UPDATE (RCU)
M: "Paul E. McKenney" <paulmck@linux.ibm.com>
@@ -17258,7 +17258,7 @@ L: linux-kernel@vger.kernel.org
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/core
S: Maintained
F: Documentation/devicetree/bindings/x86/
-F: Documentation/x86/
+F: Documentation/arch/x86/
F: arch/x86/
X86 ENTRY CODE
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1b276dda837d..b2b8d3c15285 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1293,7 +1293,7 @@ config SMP
uniprocessor machines. On a uniprocessor machine, the kernel
will run faster if you say N here.
- See also <file:Documentation/x86/i386/IO-APIC.rst>,
+ See also <file:Documentation/arch/x86/i386/IO-APIC.rst>,
<file:Documentation/lockup-watchdogs.txt> and the SMP-HOWTO available at
<http://tldp.org/HOWTO/SMP-HOWTO.html>.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index dce10b18f4bc..5489f42e005e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -399,7 +399,7 @@ config SMP
Y to "Enhanced Real Time Clock Support", below. The "Advanced Power
Management" code will be disabled if you say Y here.
- See also <file:Documentation/x86/i386/IO-APIC.rst>,
+ See also <file:Documentation/arch/x86/i386/IO-APIC.rst>,
<file:Documentation/lockup-watchdogs.txt> and the SMP-HOWTO available at
<http://www.tldp.org/docs.html#howto>.
@@ -1308,7 +1308,7 @@ config MICROCODE
the Linux kernel.
The preferred method to load microcode from a detached initrd is described
- in Documentation/x86/microcode.rst. For that you need to enable
+ in Documentation/arch/x86/microcode.rst. For that you need to enable
CONFIG_BLK_DEV_INITRD in order for the loader to be able to scan the
initrd for microcode blobs.
@@ -1347,7 +1347,7 @@ config MICROCODE_OLD_INTERFACE
It is inadequate because it runs too late to be able to properly
load microcode on a machine and it needs special tools. Instead, you
should've switched to the early loading method with the initrd or
- builtin microcode by now: Documentation/x86/microcode.rst
+ builtin microcode by now: Documentation/arch/x86/microcode.rst
config X86_MSR
tristate "/dev/cpu/*/msr - Model-specific register support"
@@ -1496,7 +1496,7 @@ config X86_5LEVEL
A kernel with the option enabled can be booted on machines that
support 4- or 5-level paging.
- See Documentation/x86/x86_64/5level-paging.rst for more
+ See Documentation/arch/x86/x86_64/5level-paging.rst for more
information.
Say N if unsure.
@@ -1801,7 +1801,7 @@ config MTRR
You can safely say Y even if your machine doesn't have MTRRs, you'll
just add about 9 KB to your kernel.
- See <file:Documentation/x86/mtrr.rst> for more information.
+ See <file:Documentation/arch/x86/mtrr.rst> for more information.
config MTRR_SANITIZER
def_bool y
@@ -1913,7 +1913,7 @@ config X86_INTEL_MPX
process and adds some branches to paths used during
exec() and munmap().
- For details, see Documentation/x86/intel_mpx.rst
+ For details, see Documentation/arch/x86/intel_mpx.rst
If unsure, say N.
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 71c92db47c41..814353667075 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -156,7 +156,7 @@ config IOMMU_DEBUG
code. When you use it make sure you have a big enough
IOMMU/AGP aperture. Most of the options enabled by this can
be set more finegrained using the iommu= command line
- options. See Documentation/x86/x86_64/boot-options.rst for more
+ options. See Documentation/arch/x86/x86_64/boot-options.rst for more
details.
config IOMMU_LEAK
--git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 2c11c0f45d49..5ec825c863a6 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -313,7 +313,7 @@ start_sys_seg: .word SYSSEG # obsolete and meaningless, but just
type_of_loader: .byte 0 # 0 means ancient bootloader, newer
# bootloaders know to change this.
- # See Documentation/x86/boot.rst for
+ # See Documentation/arch/x86/boot.rst for
# assigned ids
# flags, unused bits must be zero (RFU) bit within loadflags
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 0ea4831a72a4..981951124d53 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -8,7 +8,7 @@
*
* entry.S contains the system-call and fault low-level handling routines.
*
- * Some of this is documented in Documentation/x86/entry_64.rst
+ * Some of this is documented in Documentation/arch/x86/entry_64.rst
*
* A note on terminology:
* - iret frame: Architecture defined interrupt frame from SS to RIP
diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 101eb944f13c..585daca7682b 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -24,7 +24,7 @@ static void sanitize_boot_params(struct boot_params *boot_params)
* IMPORTANT NOTE TO BOOTLOADER AUTHORS: do not simply clear
* this field. The purpose of this field is to guarantee
* compliance with the x86 boot spec located in
- * Documentation/x86/boot.rst . That spec says that the
+ * Documentation/arch/x86/boot.rst . That spec says that the
* *whole* structure should be cleared, after which only the
* portion defined by struct setup_header (boot_params->hdr)
* should be copied in.
diff --git a/arch/x86/include/asm/page_64_types.h b/arch/x86/include/asm/page_64_types.h
index 288b065955b7..70d71bdd77da 100644
--- a/arch/x86/include/asm/page_64_types.h
+++ b/arch/x86/include/asm/page_64_types.h
@@ -48,7 +48,7 @@
#define __START_KERNEL_map _AC(0xffffffff80000000, UL)
-/* See Documentation/x86/x86_64/mm.rst for a description of the memory map. */
+/* See Documentation/arch/x86/x86_64/mm.rst for a description of the memory map. */
#define __PHYSICAL_MASK_SHIFT 52
diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
index 52e5f5f2240d..ec3fe348bbd4 100644
--- a/arch/x86/include/asm/pgtable_64_types.h
+++ b/arch/x86/include/asm/pgtable_64_types.h
@@ -103,7 +103,7 @@ extern unsigned int ptrs_per_p4d;
#define PGDIR_MASK (~(PGDIR_SIZE - 1))
/*
- * See Documentation/x86/x86_64/mm.rst for a description of the memory map.
+ * See Documentation/arch/x86/x86_64/mm.rst for a description of the memory map.
*
* Be very careful vs. KASLR when changing anything here. The KASLR address
* range must not overlap with anything except the KASAN shadow area, which
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index a0e52bd00ecc..146374651036 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -59,7 +59,7 @@ static u8 amd_ucode_patch[PATCH_MAX_SIZE];
/*
* Microcode patch container file is prepended to the initrd in cpio
- * format. See Documentation/x86/microcode.rst
+ * format. See Documentation/arch/x86/microcode.rst
*/
static const char
ucode_path[] __maybe_unused = "kernel/x86/microcode/AuthenticAMD.bin";
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 5ebcd02cbca7..108d72bcfa28 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -419,7 +419,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
efi_map_offset = params_cmdline_sz;
efi_setup_data_offset = efi_map_offset + ALIGN(efi_map_sz, 16);
- /* Copy setup header onto bootparams. Documentation/x86/boot.rst */
+ /* Copy setup header onto bootparams. Documentation/arch/x86/boot.rst */
setup_header_size = 0x0202 + kernel[0x0201] - setup_hdr_offset;
/* Is there a limit on setup header size? */
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index f62b498b18fb..a34c72e924ec 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -70,7 +70,7 @@ void __init pci_iommu_alloc(void)
}
/*
- * See <Documentation/x86/x86_64/boot-options.rst> for the iommu kernel
+ * See <Documentation/arch/x86/x86_64/boot-options.rst> for the iommu kernel
* parameter documentation.
*/
static __init int iommu_setup(char *p)
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 4de9704c4aaf..855498ab4453 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -712,7 +712,7 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
}
/*
- * See Documentation/x86/tlb.rst for details. We choose 33
+ * See Documentation/arch/x86/tlb.rst for details. We choose 33
* because it is large enough to cover the vast majority (at
* least 95%) of allocations, and is small enough that we are
* confident it will not cause too much overhead. Each single
diff --git a/arch/x86/platform/pvh/enlighten.c b/arch/x86/platform/pvh/enlighten.c
index c0a502f7e3a7..15a74dbc9b00 100644
--- a/arch/x86/platform/pvh/enlighten.c
+++ b/arch/x86/platform/pvh/enlighten.c
@@ -86,7 +86,7 @@ static void __init init_pvh_bootparams(bool xen_guest)
}
/*
- * See Documentation/x86/boot.rst.
+ * See Documentation/arch/x86/boot.rst.
*
* Version 2.12 supports Xen entry point but we will use default x86/PC
* environment (i.e. hardware_subarch 0).
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ff8892c38666..f5c1868d5d5f 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1711,7 +1711,7 @@ EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
/* TODO: This is really inefficient. We need something like get_user()
* (instruction directly accesses the data, with an exception table entry
- * returning -EFAULT). See Documentation/x86/exception-tables.rst.
+ * returning -EFAULT). See Documentation/arch/x86/exception-tables.rst.
*/
static int set_bit_to_user(int nr, void __user *addr)
{
diff --git a/security/Kconfig b/security/Kconfig
index 06a30851511a..d26d9f205441 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -63,7 +63,7 @@ config PAGE_TABLE_ISOLATION
ensuring that the majority of kernel addresses are not mapped
into userspace.
- See Documentation/x86/pti.rst for more details.
+ See Documentation/arch/x86/pti.rst for more details.
config SECURITY_INFINIBAND
bool "Infiniband Security Hooks"
diff --git a/tools/include/linux/err.h b/tools/include/linux/err.h
index 25f2bb3a991d..332b983ead1e 100644
--- a/tools/include/linux/err.h
+++ b/tools/include/linux/err.h
@@ -20,7 +20,7 @@
* Userspace note:
* The same principle works for userspace, because 'error' pointers
* fall down to the unused hole far from user space, as described
- * in Documentation/x86/x86_64/mm.rst for x86_64 arch:
+ * in Documentation/arch/x86/x86_64/mm.rst for x86_64 arch:
*
* 0000000000000000 - 00007fffffffffff (=47 bits) user space, different per mm hole caused by [48:63] sign extension
* ffffffffffe00000 - ffffffffffffffff (=2 MB) unused hole
diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt
index de094670050b..87b6b4d1175a 100644
--- a/tools/objtool/Documentation/stack-validation.txt
+++ b/tools/objtool/Documentation/stack-validation.txt
@@ -21,7 +21,7 @@ instructions). Similarly, it knows how to follow switch statements, for
which gcc sometimes uses jump tables.
(Objtool also has an 'orc generate' subcommand which generates debuginfo
-for the ORC unwinder. See Documentation/x86/orc-unwinder.rst in the
+for the ORC unwinder. See Documentation/arch/x86/orc-unwinder.rst in the
kernel tree for more details.)
@@ -101,7 +101,7 @@ b) ORC (Oops Rewind Capability) unwind table generation
band. So it doesn't affect runtime performance and it can be
reliable even when interrupts or exceptions are involved.
- For more details, see Documentation/x86/orc-unwinder.rst.
+ For more details, see Documentation/arch/x86/orc-unwinder.rst.
c) Higher live patching compatibility rate
--
2.19.1.856.g8858448bb
^ permalink raw reply related
* Re: Some LSM and SGX remarks before parting of for two weeks
From: James Morris @ 2019-07-12 3:12 UTC (permalink / raw)
To: Jarkko Sakkinen; +Cc: linux-sgx, linux-security-module
In-Reply-To: <20190712021055.22qijpsahsy3gpmp@linux.intel.com>
On Fri, 12 Jul 2019, Jarkko Sakkinen wrote:
> Before going to a two week vacation (sending v21 today), I'll make some
> remarks on SGX and LSM's:
>
> 1. Currently all patch sets proposing LSM changes are missing a problem
> statement and describe a solution to an undescribed problem.
> 2. When speaking of SELinux I haven't seen any draft's on how would
> define a policy module with the new constructs. Does not have to
> be a full policy modules but more like snippets demosntrating that
> "this would work".
> 3. All the SELinux discussion is centered on type based policies.
> Potentially one could isolate enclaves with some UBAC or RBAC
> based model. That could be good first step and might not even
> require LSM changes.
Unless I misunderstand what you mean here, RBAC and UBAC in SELinux still
require LSM hooks, and are typically integrated with Type Enforcement.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
From: James Morris @ 2019-07-12 3:29 UTC (permalink / raw)
To: Casey Schaufler
Cc: Aaron Goidel, paul, selinux, linux-security-module, linux-fsdevel,
dhowells, jack, amir73il, sds, linux-kernel
In-Reply-To: <4fd98c88-61a6-a155-5028-db22a778d3c1@schaufler-ca.com>
On Wed, 10 Jul 2019, Casey Schaufler wrote:
> On 7/10/2019 6:34 AM, Aaron Goidel wrote:
>
> > Furthermore, fanotify watches grant more power to
> > an application in the form of permission events. While notification events
> > are solely, unidirectional (i.e. they only pass information to the
> > receiving application), permission events are blocking. Permission events
> > make a request to the receiving application which will then reply with a
> > decision as to whether or not that action may be completed.
>
> You're not saying why this is an issue.
Also in the description, please explain the issues with read and write
notifications and why a simple 'read' permission is not adequate.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC/RFT] KEYS: trusted: Add generic trusted keys framework
From: Sumit Garg @ 2019-07-12 5:13 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: keyrings, linux-integrity,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-security-module, dhowells, Herbert Xu, davem, jejb,
Mimi Zohar, jmorris, serge, Casey Schaufler, Ard Biesheuvel,
Daniel Thompson, Linux Kernel Mailing List, tee-dev
In-Reply-To: <20190711192215.5w3fzdjwsebgoesh@linux.intel.com>
On Fri, 12 Jul 2019 at 00:52, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Fri, Jul 05, 2019 at 08:02:34PM +0530, Sumit Garg wrote:
> > Current trusted keys framework is tightly coupled to use TPM device as
> > an underlying implementation which makes it difficult for implementations
> > like Trusted Execution Environment (TEE) etc. to provide trusked keys
> > support in case platform doesn't posses a TPM device.
> >
> > So this patch tries to add generic trusted keys framework where underlying
> > implemtations like TPM, TEE etc. could be easily plugged-in.
> >
> > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> 1. Needs to be somehow dissected into digestable/reviewable pieces.
Sure, will try to split this patch in next version.
> 2. As a precursory step probably would make sense to move all
> existing trusted keys code into one subsystem first.
Okay.
-Sumit
>
> /Jarkko
^ permalink raw reply
* Re: Some LSM and SGX remarks before parting of for two weeks
From: Jarkko Sakkinen @ 2019-07-12 5:14 UTC (permalink / raw)
To: James Morris; +Cc: linux-sgx, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1907121303080.17953@namei.org>
On Fri, Jul 12, 2019 at 01:12:23PM +1000, James Morris wrote:
> On Fri, 12 Jul 2019, Jarkko Sakkinen wrote:
>
> > Before going to a two week vacation (sending v21 today), I'll make some
> > remarks on SGX and LSM's:
> >
> > 1. Currently all patch sets proposing LSM changes are missing a problem
> > statement and describe a solution to an undescribed problem.
> > 2. When speaking of SELinux I haven't seen any draft's on how would
> > define a policy module with the new constructs. Does not have to
> > be a full policy modules but more like snippets demosntrating that
> > "this would work".
> > 3. All the SELinux discussion is centered on type based policies.
> > Potentially one could isolate enclaves with some UBAC or RBAC
> > based model. That could be good first step and might not even
> > require LSM changes.
>
> Unless I misunderstand what you mean here, RBAC and UBAC in SELinux still
> require LSM hooks, and are typically integrated with Type Enforcement.
OK, I was thinking something like with normal DAC just to have SGID
for enclaves. Just learning basic SELinux concepts. Still quite alien
world to me.
/Jarkko
^ permalink raw reply
* Re: [PATCH] LSM: Update MAINTAINERS file for SafeSetID LSM.
From: Micah Morton @ 2019-07-12 15:30 UTC (permalink / raw)
To: James Morris; +Cc: linux-security-module
In-Reply-To: <alpine.LRH.2.21.1905230341520.18826@namei.org>
Hi James,
Are we still merging this kind of thing through your tree?
Or does it make more sense for me to send this through my tree when I
get one up and running?
I was trying to get in the MAINTAINERS file since it seems that this
is a prerequisite for getting an account for hosting my tree on
git.kernel.org.
Thanks
On Wed, May 22, 2019 at 10:42 AM James Morris <jmorris@namei.org> wrote:
>
> On Wed, 22 May 2019, Micah Morton wrote:
>
> > This is in preparation for SafeSetID to be maintained in its own tree,
> > rather than going via the security tree.
> >
> > Signed-off-by: Micah Morton <mortonm@chromium.org>
>
>
> Acked-by: James Morris <jamorris@linux.microsoft.com>
>
> > ---
> > MAINTAINERS | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3e5a5d263f29..0fcd34e64fa7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13561,6 +13561,12 @@ F: drivers/media/common/saa7146/
> > F: drivers/media/pci/saa7146/
> > F: include/media/drv-intf/saa7146*
> >
> > +SAFESETID SECURITY MODULE
> > +M: Micah Morton <mortonm@chromium.org>
> > +S: Supported
> > +F: security/safesetid/
> > +F: Documentation/admin-guide/LSM/SafeSetID.rst
> > +
> > SAMSUNG AUDIO (ASoC) DRIVERS
> > M: Krzysztof Kozlowski <krzk@kernel.org>
> > M: Sangbeom Kim <sbkim73@samsung.com>
> >
>
> --
> James Morris
> <jmorris@namei.org>
>
^ permalink raw reply
* Preferred subj= with multiple LSMs
From: Casey Schaufler @ 2019-07-12 16:33 UTC (permalink / raw)
To: linux-audit@redhat.com, Linux Security Module list, Paul Moore,
rgb, Steve Grubb
Cc: casey
Which of these options would be preferred for audit records
when there are multiple active security modules? I'm not asking
if we should do it, I'm asking which of these options I should
implement when I do do it. I've prototyped #1 and #2. #4 is a
minor variant of #1 that is either better for compatibility or
worse, depending on how you want to look at it. I understand
that each of these offer challenges. If I've missed something
obvious, I'd be delighted to consider #5.
Thank you.
Option 1:
subj=selinux='x:y:z:s:c',apparmor='a'
Option 2:
subj=x:y:z:s:c subj=a
Option 3:
lsms=selinux,apparmor subj=x:y:z:s:c subj=a
Option 4:
subjs=selinux='x:y:z:s:c',apparmor='a'
Option 5:
Something else.
^ permalink raw reply
* Re: [PATCH] LSM: Update MAINTAINERS file for SafeSetID LSM.
From: James Morris @ 2019-07-12 17:15 UTC (permalink / raw)
To: Micah Morton; +Cc: linux-security-module
In-Reply-To: <CAJ-EccPo2T6y=7Gw-naPZ4d25em+8TsZMSXp4C=pGeTutbPqZA@mail.gmail.com>
On Fri, 12 Jul 2019, Micah Morton wrote:
> Hi James,
>
> Are we still merging this kind of thing through your tree?
>
> Or does it make more sense for me to send this through my tree when I
> get one up and running?
Send it via your tree with my acked-by.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH v6 0/1] Add dm verity root hash pkcs7 sig validation.
From: Jaskaran Singh Khurana @ 2019-07-12 17:33 UTC (permalink / raw)
To: gmazyland
Cc: ebiggers, linux-security-module, linux-kernel, linux-integrity,
linux-fsdevel, agk, snitzer, dm-devel, jmorris, Scott Shell,
Nazmus Sakib, mpatocka
In-Reply-To: <MN2PR21MB12008A962D4DD8662B3614508AF20@MN2PR21MB1200.namprd21.prod.outlook.com>
Hello Milan,
> Changes in v6:
>
> Address comments from Milan Broz and Eric Biggers on v5.
>
> -Keep the verification code under config DM_VERITY_VERIFY_ROOTHASH_SIG.
>
> -Change the command line parameter to requires_signatures(bool) which will
> force root hash to be signed and trusted if specified.
>
> -Fix the signature not being present in verity_status. Merged the
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmbroz%2Flinux.git%2Fcommit%2F%3Fh%3Ddm-cryptsetup%26id%3Da26c10806f5257e255b6a436713127e762935ad3&data=02%7C01%7CJaskaran.Khurana%40microsoft.com%7C18f92445e46940aeebb008d6fe50c610%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636976020210890638&sdata=aY0V9%2FBz2RHryIvoftGKUGnyPp9Fsc1JY4FZbHfW4hg%3D&reserved=0
> made by Milan Broz and tested it.
>
>
Could you please provide feedback on this v6 version.
Regards,
Jaskaran
^ permalink raw reply
* [RFC PATCH] security,capability: pass object information to security_capable
From: Nicholas Franck @ 2019-07-12 17:34 UTC (permalink / raw)
To: paul
Cc: selinux, linux-security-module, luto, jmorris, sds, keescook,
serge, john.johansen, casey, mortonm, Nicholas Franck
At present security_capable does not pass any object information
and therefore can neither audit the particular object nor take it
into account. Augment the security_capable interface to support
passing supplementary data. Use this facility initially to convey
the inode for capability checks relevant to inodes. This only
addresses capable_wrt_inode_uidgid calls; other capability checks
relevant to inodes will be addressed in subsequent changes. In the
future, this will be further extended to pass object information for
other capability checks such as the target task for CAP_KILL.
In SELinux this new information is leveraged here to include the inode
in the audit message. In the future, it could also be used to perform
a per inode capability checks.
It would be possible to fold the existing opts argument into this new
supplementary data structure. This was omitted from this change to
minimize changes.
Signed-off-by: Nicholas Franck <nhfran2@tycho.nsa.gov>
---
include/linux/capability.h | 7 ++++++
include/linux/lsm_audit.h | 5 +++-
include/linux/lsm_hooks.h | 3 ++-
include/linux/security.h | 23 +++++++++++++-----
kernel/capability.c | 33 ++++++++++++++++++--------
kernel/seccomp.c | 2 +-
security/apparmor/capability.c | 8 ++++---
security/apparmor/include/capability.h | 4 +++-
security/apparmor/ipc.c | 2 +-
security/apparmor/lsm.c | 5 ++--
security/apparmor/resource.c | 2 +-
security/commoncap.c | 11 +++++----
security/lsm_audit.c | 21 ++++++++++++++--
security/safesetid/lsm.c | 3 ++-
security/security.c | 5 ++--
security/selinux/hooks.c | 20 +++++++++-------
security/smack/smack_access.c | 2 +-
17 files changed, 110 insertions(+), 46 deletions(-)
diff --git a/include/linux/capability.h b/include/linux/capability.h
index ecce0f43c73a..f72de64c179d 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -211,6 +211,8 @@ extern bool capable(int cap);
extern bool ns_capable(struct user_namespace *ns, int cap);
extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
extern bool ns_capable_setid(struct user_namespace *ns, int cap);
+extern bool ns_capable_inode(struct user_namespace *ns, int cap,
+ const struct inode *inode);
#else
static inline bool has_capability(struct task_struct *t, int cap)
{
@@ -246,6 +248,11 @@ static inline bool ns_capable_setid(struct user_namespace *ns, int cap)
{
return true;
}
+static bool ns_capable_inode(struct user_namespace *ns, int cap,
+ const struct inode *inode)
+{
+ return true;
+}
#endif /* CONFIG_MULTIUSER */
extern bool privileged_wrt_inode_uidgid(struct user_namespace *ns, const struct inode *inode);
extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 915330abf6e5..15d2a62639f0 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -79,7 +79,10 @@ struct common_audit_data {
struct dentry *dentry;
struct inode *inode;
struct lsm_network_audit *net;
- int cap;
+ struct {
+ int cap;
+ struct cap_aux_data *cad;
+ } cap_struct;
int ipc_id;
struct task_struct *tsk;
#ifdef CONFIG_KEYS
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 47f58cfb6a19..b2a37d613030 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1469,7 +1469,8 @@ union security_list_options {
int (*capable)(const struct cred *cred,
struct user_namespace *ns,
int cap,
- unsigned int opts);
+ unsigned int opts,
+ struct cap_aux_data *cad);
int (*quotactl)(int cmds, int type, int id, struct super_block *sb);
int (*quota_on)(struct dentry *dentry);
int (*syslog)(int type);
diff --git a/include/linux/security.h b/include/linux/security.h
index 659071c2e57c..8fce5e69dc52 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -77,9 +77,18 @@ enum lsm_event {
LSM_POLICY_CHANGE,
};
+
+struct cap_aux_data {
+ char type;
+#define CAP_AUX_DATA_INODE 1
+ union {
+ const struct inode *inode;
+ } u;
+};
+
/* These functions are in security/commoncap.c */
extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
- int cap, unsigned int opts);
+ int cap, unsigned int opts, struct cap_aux_data *cad);
extern int cap_settime(const struct timespec64 *ts, const struct timezone *tz);
extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode);
extern int cap_ptrace_traceme(struct task_struct *parent);
@@ -215,9 +224,10 @@ int security_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
int security_capable(const struct cred *cred,
- struct user_namespace *ns,
- int cap,
- unsigned int opts);
+ struct user_namespace *ns,
+ int cap,
+ unsigned int opts,
+ struct cap_aux_data *cad);
int security_quotactl(int cmds, int type, int id, struct super_block *sb);
int security_quota_on(struct dentry *dentry);
int security_syslog(int type);
@@ -478,9 +488,10 @@ static inline int security_capset(struct cred *new,
static inline int security_capable(const struct cred *cred,
struct user_namespace *ns,
int cap,
- unsigned int opts)
+ unsigned int opts,
+ struct cap_aux_data *cad)
{
- return cap_capable(cred, ns, cap, opts);
+ return cap_capable(cred, ns, cap, opts, NULL);
}
static inline int security_quotactl(int cmds, int type, int id,
diff --git a/kernel/capability.c b/kernel/capability.c
index 1444f3954d75..c3723443904a 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -297,7 +297,7 @@ bool has_ns_capability(struct task_struct *t,
int ret;
rcu_read_lock();
- ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NONE);
+ ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NONE, NULL);
rcu_read_unlock();
return (ret == 0);
@@ -338,7 +338,7 @@ bool has_ns_capability_noaudit(struct task_struct *t,
int ret;
rcu_read_lock();
- ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NOAUDIT);
+ ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NOAUDIT, NULL);
rcu_read_unlock();
return (ret == 0);
@@ -363,7 +363,8 @@ bool has_capability_noaudit(struct task_struct *t, int cap)
static bool ns_capable_common(struct user_namespace *ns,
int cap,
- unsigned int opts)
+ unsigned int opts,
+ struct cap_aux_data *cad)
{
int capable;
@@ -372,7 +373,7 @@ static bool ns_capable_common(struct user_namespace *ns,
BUG();
}
- capable = security_capable(current_cred(), ns, cap, opts);
+ capable = security_capable(current_cred(), ns, cap, opts, cad);
if (capable == 0) {
current->flags |= PF_SUPERPRIV;
return true;
@@ -393,7 +394,7 @@ static bool ns_capable_common(struct user_namespace *ns,
*/
bool ns_capable(struct user_namespace *ns, int cap)
{
- return ns_capable_common(ns, cap, CAP_OPT_NONE);
+ return ns_capable_common(ns, cap, CAP_OPT_NONE, NULL);
}
EXPORT_SYMBOL(ns_capable);
@@ -411,7 +412,7 @@ EXPORT_SYMBOL(ns_capable);
*/
bool ns_capable_noaudit(struct user_namespace *ns, int cap)
{
- return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT);
+ return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT, NULL);
}
EXPORT_SYMBOL(ns_capable_noaudit);
@@ -430,7 +431,7 @@ EXPORT_SYMBOL(ns_capable_noaudit);
*/
bool ns_capable_setid(struct user_namespace *ns, int cap)
{
- return ns_capable_common(ns, cap, CAP_OPT_INSETID);
+ return ns_capable_common(ns, cap, CAP_OPT_INSETID, NULL);
}
EXPORT_SYMBOL(ns_capable_setid);
@@ -470,7 +471,7 @@ bool file_ns_capable(const struct file *file, struct user_namespace *ns,
if (WARN_ON_ONCE(!cap_valid(cap)))
return false;
- if (security_capable(file->f_cred, ns, cap, CAP_OPT_NONE) == 0)
+ if (security_capable(file->f_cred, ns, cap, CAP_OPT_NONE, NULL) == 0)
return true;
return false;
@@ -503,7 +504,8 @@ bool capable_wrt_inode_uidgid(const struct inode *inode, int cap)
{
struct user_namespace *ns = current_user_ns();
- return ns_capable(ns, cap) && privileged_wrt_inode_uidgid(ns, inode);
+ return ns_capable_inode(ns, cap, inode) &&
+ privileged_wrt_inode_uidgid(ns, inode);
}
EXPORT_SYMBOL(capable_wrt_inode_uidgid);
@@ -524,7 +526,18 @@ bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns)
cred = rcu_dereference(tsk->ptracer_cred);
if (cred)
ret = security_capable(cred, ns, CAP_SYS_PTRACE,
- CAP_OPT_NOAUDIT);
+ CAP_OPT_NOAUDIT, NULL);
rcu_read_unlock();
return (ret == 0);
}
+
+bool ns_capable_inode(struct user_namespace *ns, int cap,
+ const struct inode *inode)
+{
+ struct cap_aux_data cad;
+
+ cad.type = CAP_AUX_DATA_INODE;
+ cad.u.inode = inode;
+ return ns_capable_common(ns, cap, CAP_OPT_NONE, &cad);
+}
+EXPORT_SYMBOL(ns_capable_inode);
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 811b4a86cdf6..d59dd7079ece 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -446,7 +446,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
*/
if (!task_no_new_privs(current) &&
security_capable(current_cred(), current_user_ns(),
- CAP_SYS_ADMIN, CAP_OPT_NOAUDIT) != 0)
+ CAP_SYS_ADMIN, CAP_OPT_NOAUDIT, NULL) != 0)
return ERR_PTR(-EACCES);
/* Allocate a new seccomp_filter */
diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c
index 752f73980e30..c45192a16733 100644
--- a/security/apparmor/capability.c
+++ b/security/apparmor/capability.c
@@ -50,7 +50,7 @@ static void audit_cb(struct audit_buffer *ab, void *va)
struct common_audit_data *sa = va;
audit_log_format(ab, " capname=");
- audit_log_untrustedstring(ab, capability_names[sa->u.cap]);
+ audit_log_untrustedstring(ab, capability_names[sa->u.cap_struct.cap]);
}
/**
@@ -148,13 +148,15 @@ static int profile_capable(struct aa_profile *profile, int cap,
*
* Returns: 0 on success, or else an error code.
*/
-int aa_capable(struct aa_label *label, int cap, unsigned int opts)
+int aa_capable(struct aa_label *label, int cap, unsigned int opts,
+ struct cap_aux_data *cad)
{
struct aa_profile *profile;
int error = 0;
DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_CAP, OP_CAPABLE);
- sa.u.cap = cap;
+ sa.u.cap_struct.cap = cap;
+ sa.u.cap_struct.cad = cad;
error = fn_for_each_confined(label, profile,
profile_capable(profile, cap, opts, &sa));
diff --git a/security/apparmor/include/capability.h b/security/apparmor/include/capability.h
index 1b3663b6ab12..d888f09d76d1 100644
--- a/security/apparmor/include/capability.h
+++ b/security/apparmor/include/capability.h
@@ -20,6 +20,7 @@
#include "apparmorfs.h"
struct aa_label;
+struct cap_aux_data;
/* aa_caps - confinement data for capabilities
* @allowed: capabilities mask
@@ -40,7 +41,8 @@ struct aa_caps {
extern struct aa_sfs_entry aa_sfs_entry_caps[];
-int aa_capable(struct aa_label *label, int cap, unsigned int opts);
+int aa_capable(struct aa_label *label, int cap, unsigned int opts,
+ struct cap_aux_data *cad);
static inline void aa_free_cap_rules(struct aa_caps *caps)
{
diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
index aacd1e95cb59..deb5267ca695 100644
--- a/security/apparmor/ipc.c
+++ b/security/apparmor/ipc.c
@@ -108,7 +108,7 @@ static int profile_tracer_perm(struct aa_profile *tracer,
aad(sa)->peer = tracee;
aad(sa)->request = 0;
aad(sa)->error = aa_capable(&tracer->label, CAP_SYS_PTRACE,
- CAP_OPT_NONE);
+ CAP_OPT_NONE, NULL);
return aa_audit(AUDIT_APPARMOR_AUTO, tracer, sa, audit_ptrace_cb);
}
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 87500bde5a92..82790accb679 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -172,14 +172,15 @@ static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
}
static int apparmor_capable(const struct cred *cred, struct user_namespace *ns,
- int cap, unsigned int opts)
+ int cap, unsigned int opts,
+ struct cap_aux_data *cad)
{
struct aa_label *label;
int error = 0;
label = aa_get_newest_cred_label(cred);
if (!unconfined(label))
- error = aa_capable(label, cap, opts);
+ error = aa_capable(label, cap, opts, cad);
aa_put_label(label);
return error;
diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c
index 552ed09cb47e..9b3d4b4437f2 100644
--- a/security/apparmor/resource.c
+++ b/security/apparmor/resource.c
@@ -124,7 +124,7 @@ int aa_task_setrlimit(struct aa_label *label, struct task_struct *task,
*/
if (label != peer &&
- aa_capable(label, CAP_SYS_RESOURCE, CAP_OPT_NOAUDIT) != 0)
+ aa_capable(label, CAP_SYS_RESOURCE, CAP_OPT_NOAUDIT, NULL) != 0)
error = fn_for_each(label, profile,
audit_resource(profile, resource,
new_rlim->rlim_max, peer,
diff --git a/security/commoncap.c b/security/commoncap.c
index c477fb673701..1860ea50f473 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -68,7 +68,7 @@ static void warn_setuid_and_fcaps_mixed(const char *fname)
* kernel's capable() and has_capability() returns 1 for this case.
*/
int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
- int cap, unsigned int opts)
+ int cap, unsigned int opts, struct cap_aux_data *cad)
{
struct user_namespace *ns = targ_ns;
@@ -226,7 +226,7 @@ static inline int cap_inh_is_capped(void)
* capability
*/
if (cap_capable(current_cred(), current_cred()->user_ns,
- CAP_SETPCAP, CAP_OPT_NONE) == 0)
+ CAP_SETPCAP, CAP_OPT_NONE, NULL) == 0)
return 0;
return 1;
}
@@ -1211,7 +1211,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
|| (cap_capable(current_cred(),
current_cred()->user_ns,
CAP_SETPCAP,
- CAP_OPT_NONE) != 0) /*[4]*/
+ CAP_OPT_NONE,
+ NULL) != 0) /*[4]*/
/*
* [1] no changing of bits that are locked
* [2] no unlocking of locks
@@ -1307,7 +1308,7 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
int cap_sys_admin = 0;
if (cap_capable(current_cred(), &init_user_ns,
- CAP_SYS_ADMIN, CAP_OPT_NOAUDIT) == 0)
+ CAP_SYS_ADMIN, CAP_OPT_NOAUDIT, NULL) == 0)
cap_sys_admin = 1;
return cap_sys_admin;
@@ -1328,7 +1329,7 @@ int cap_mmap_addr(unsigned long addr)
if (addr < dac_mmap_min_addr) {
ret = cap_capable(current_cred(), &init_user_ns, CAP_SYS_RAWIO,
- CAP_OPT_NONE);
+ CAP_OPT_NONE, NULL);
/* set PF_SUPERPRIV if it turns out we allow the low mmap */
if (ret == 0)
current->flags |= PF_SUPERPRIV;
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 33028c098ef3..4871b2508a4a 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -229,9 +229,26 @@ static void dump_common_audit_data(struct audit_buffer *ab,
case LSM_AUDIT_DATA_IPC:
audit_log_format(ab, " key=%d ", a->u.ipc_id);
break;
- case LSM_AUDIT_DATA_CAP:
- audit_log_format(ab, " capability=%d ", a->u.cap);
+ case LSM_AUDIT_DATA_CAP: {
+ const struct inode *inode;
+
+ if (a->u.cap_struct.cad) {
+ switch (a->u.cap_struct.cad->type) {
+ case CAP_AUX_DATA_INODE: {
+ inode = a->u.cap_struct.cad->u.inode;
+
+ audit_log_format(ab, " dev=");
+ audit_log_untrustedstring(ab,
+ inode->i_sb->s_id);
+ audit_log_format(ab, " ino=%lu",
+ inode->i_ino);
+ break;
+ }
+ }
+ }
+ audit_log_format(ab, " capability=%d ", a->u.cap_struct.cap);
break;
+ }
case LSM_AUDIT_DATA_PATH: {
struct inode *inode;
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index cecd38e2ac80..c74ed83e9501 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -80,7 +80,8 @@ static bool check_setuid_policy_hashtable_key_value(kuid_t parent,
static int safesetid_security_capable(const struct cred *cred,
struct user_namespace *ns,
int cap,
- unsigned int opts)
+ unsigned int opts,
+ struct cap_aux_data *cad)
{
if (cap == CAP_SETUID &&
check_setuid_policy_hashtable_key(cred->uid)) {
diff --git a/security/security.c b/security/security.c
index 613a5c00e602..06274a7b9c4e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -691,9 +691,10 @@ int security_capset(struct cred *new, const struct cred *old,
int security_capable(const struct cred *cred,
struct user_namespace *ns,
int cap,
- unsigned int opts)
+ unsigned int opts,
+ struct cap_aux_data *cad)
{
- return call_int_hook(capable, 0, cred, ns, cap, opts);
+ return call_int_hook(capable, 0, cred, ns, cap, opts, cad);
}
int security_quotactl(int cmds, int type, int id, struct super_block *sb)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f77b314d0575..d6c699ed06be 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1618,7 +1618,10 @@ static inline u32 signal_to_av(int sig)
/* Check whether a task is allowed to use a capability. */
static int cred_has_capability(const struct cred *cred,
- int cap, unsigned int opts, bool initns)
+ int cap,
+ unsigned int opts,
+ bool initns,
+ struct cap_aux_data *cad)
{
struct common_audit_data ad;
struct av_decision avd;
@@ -1628,7 +1631,8 @@ static int cred_has_capability(const struct cred *cred,
int rc;
ad.type = LSM_AUDIT_DATA_CAP;
- ad.u.cap = cap;
+ ad.u.cap_struct.cad = cad;
+ ad.u.cap_struct.cap = cap;
switch (CAP_TO_INDEX(cap)) {
case 0:
@@ -2165,9 +2169,9 @@ static int selinux_capset(struct cred *new, const struct cred *old,
*/
static int selinux_capable(const struct cred *cred, struct user_namespace *ns,
- int cap, unsigned int opts)
+ int cap, unsigned int opts, struct cap_aux_data *cad)
{
- return cred_has_capability(cred, cap, opts, ns == &init_user_ns);
+ return cred_has_capability(cred, cap, opts, ns == &init_user_ns, cad);
}
static int selinux_quotactl(int cmds, int type, int id, struct super_block *sb)
@@ -2241,7 +2245,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
int rc, cap_sys_admin = 0;
rc = cred_has_capability(current_cred(), CAP_SYS_ADMIN,
- CAP_OPT_NOAUDIT, true);
+ CAP_OPT_NOAUDIT, true, NULL);
if (rc == 0)
cap_sys_admin = 1;
@@ -3101,9 +3105,9 @@ static bool has_cap_mac_admin(bool audit)
const struct cred *cred = current_cred();
unsigned int opts = audit ? CAP_OPT_NONE : CAP_OPT_NOAUDIT;
- if (cap_capable(cred, &init_user_ns, CAP_MAC_ADMIN, opts))
+ if (cap_capable(cred, &init_user_ns, CAP_MAC_ADMIN, opts, NULL))
return false;
- if (cred_has_capability(cred, CAP_MAC_ADMIN, opts, true))
+ if (cred_has_capability(cred, CAP_MAC_ADMIN, opts, true, NULL))
return false;
return true;
}
@@ -3563,7 +3567,7 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd,
case KDSKBENT:
case KDSKBSENT:
error = cred_has_capability(cred, CAP_SYS_TTY_CONFIG,
- CAP_OPT_NONE, true);
+ CAP_OPT_NONE, true, NULL);
break;
/* default case assumes that the command will go
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index fe2ce3a65822..e961bfe8f00a 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -640,7 +640,7 @@ bool smack_privileged_cred(int cap, const struct cred *cred)
struct smack_known_list_elem *sklep;
int rc;
- rc = cap_capable(cred, &init_user_ns, cap, CAP_OPT_NONE);
+ rc = cap_capable(cred, &init_user_ns, cap, CAP_OPT_NONE, NULL);
if (rc)
return false;
--
2.21.0
^ permalink raw reply related
* Re: [RFC PATCH] security,capability: pass object information to security_capable
From: James Morris @ 2019-07-12 17:50 UTC (permalink / raw)
To: Nicholas Franck
Cc: paul, selinux, linux-security-module, luto, sds, keescook, serge,
john.johansen, casey, mortonm
In-Reply-To: <20190712173404.14417-1-nhfran2@tycho.nsa.gov>
On Fri, 12 Jul 2019, Nicholas Franck wrote:
> + case LSM_AUDIT_DATA_CAP: {
> + const struct inode *inode;
> +
> + if (a->u.cap_struct.cad) {
> + switch (a->u.cap_struct.cad->type) {
> + case CAP_AUX_DATA_INODE: {
> + inode = a->u.cap_struct.cad->u.inode;
> +
> + audit_log_format(ab, " dev=");
> + audit_log_untrustedstring(ab,
> + inode->i_sb->s_id);
> + audit_log_format(ab, " ino=%lu",
> + inode->i_ino);
> + break;
> + }
> + }
> + }
> + audit_log_format(ab, " capability=%d ", a->u.cap_struct.cap);
> break;
Will this break any existing userspace log parsers?
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH] security,capability: pass object information to security_capable
From: Casey Schaufler @ 2019-07-12 17:58 UTC (permalink / raw)
To: Nicholas Franck, paul
Cc: selinux, linux-security-module, luto, jmorris, sds, keescook,
serge, john.johansen, mortonm, casey
In-Reply-To: <20190712173404.14417-1-nhfran2@tycho.nsa.gov>
On 7/12/2019 10:34 AM, Nicholas Franck wrote:
> At present security_capable does not pass any object information
> and therefore can neither audit the particular object nor take it
> into account. Augment the security_capable interface to support
> passing supplementary data. Use this facility initially to convey
> the inode for capability checks relevant to inodes. This only
> addresses capable_wrt_inode_uidgid calls; other capability checks
> relevant to inodes will be addressed in subsequent changes. In the
> future, this will be further extended to pass object information for
> other capability checks such as the target task for CAP_KILL.
This seems wrong to me. The capability system has nothing to do
with objects. Passing object information through security_capable()
may be convenient, but isn't relevant to the purpose of the interface.
It appears that there are very few places where the object information
is actually useful.
> In SELinux this new information is leveraged here to include the inode
> in the audit message. In the future, it could also be used to perform
> a per inode capability checks.
I suggest that you want a mechanism for adding the inode information
to the audit record instead.
What would a "per inode" capability check be? Capability checks are
process checks, not object checks.
> It would be possible to fold the existing opts argument into this new
> supplementary data structure. This was omitted from this change to
> minimize changes.
>
> Signed-off-by: Nicholas Franck <nhfran2@tycho.nsa.gov>
> ---
> include/linux/capability.h | 7 ++++++
> include/linux/lsm_audit.h | 5 +++-
> include/linux/lsm_hooks.h | 3 ++-
> include/linux/security.h | 23 +++++++++++++-----
> kernel/capability.c | 33 ++++++++++++++++++--------
> kernel/seccomp.c | 2 +-
> security/apparmor/capability.c | 8 ++++---
> security/apparmor/include/capability.h | 4 +++-
> security/apparmor/ipc.c | 2 +-
> security/apparmor/lsm.c | 5 ++--
> security/apparmor/resource.c | 2 +-
> security/commoncap.c | 11 +++++----
> security/lsm_audit.c | 21 ++++++++++++++--
> security/safesetid/lsm.c | 3 ++-
> security/security.c | 5 ++--
> security/selinux/hooks.c | 20 +++++++++-------
> security/smack/smack_access.c | 2 +-
> 17 files changed, 110 insertions(+), 46 deletions(-)
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index ecce0f43c73a..f72de64c179d 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -211,6 +211,8 @@ extern bool capable(int cap);
> extern bool ns_capable(struct user_namespace *ns, int cap);
> extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
> extern bool ns_capable_setid(struct user_namespace *ns, int cap);
> +extern bool ns_capable_inode(struct user_namespace *ns, int cap,
> + const struct inode *inode);
> #else
> static inline bool has_capability(struct task_struct *t, int cap)
> {
> @@ -246,6 +248,11 @@ static inline bool ns_capable_setid(struct user_namespace *ns, int cap)
> {
> return true;
> }
> +static bool ns_capable_inode(struct user_namespace *ns, int cap,
> + const struct inode *inode)
> +{
> + return true;
> +}
> #endif /* CONFIG_MULTIUSER */
> extern bool privileged_wrt_inode_uidgid(struct user_namespace *ns, const struct inode *inode);
> extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
> diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
> index 915330abf6e5..15d2a62639f0 100644
> --- a/include/linux/lsm_audit.h
> +++ b/include/linux/lsm_audit.h
> @@ -79,7 +79,10 @@ struct common_audit_data {
> struct dentry *dentry;
> struct inode *inode;
> struct lsm_network_audit *net;
> - int cap;
> + struct {
> + int cap;
> + struct cap_aux_data *cad;
> + } cap_struct;
> int ipc_id;
> struct task_struct *tsk;
> #ifdef CONFIG_KEYS
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 47f58cfb6a19..b2a37d613030 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1469,7 +1469,8 @@ union security_list_options {
> int (*capable)(const struct cred *cred,
> struct user_namespace *ns,
> int cap,
> - unsigned int opts);
> + unsigned int opts,
> + struct cap_aux_data *cad);
> int (*quotactl)(int cmds, int type, int id, struct super_block *sb);
> int (*quota_on)(struct dentry *dentry);
> int (*syslog)(int type);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 659071c2e57c..8fce5e69dc52 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -77,9 +77,18 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +
> +struct cap_aux_data {
> + char type;
> +#define CAP_AUX_DATA_INODE 1
> + union {
> + const struct inode *inode;
> + } u;
> +};
> +
> /* These functions are in security/commoncap.c */
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> - int cap, unsigned int opts);
> + int cap, unsigned int opts, struct cap_aux_data *cad);
> extern int cap_settime(const struct timespec64 *ts, const struct timezone *tz);
> extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode);
> extern int cap_ptrace_traceme(struct task_struct *parent);
> @@ -215,9 +224,10 @@ int security_capset(struct cred *new, const struct cred *old,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> int security_capable(const struct cred *cred,
> - struct user_namespace *ns,
> - int cap,
> - unsigned int opts);
> + struct user_namespace *ns,
> + int cap,
> + unsigned int opts,
> + struct cap_aux_data *cad);
> int security_quotactl(int cmds, int type, int id, struct super_block *sb);
> int security_quota_on(struct dentry *dentry);
> int security_syslog(int type);
> @@ -478,9 +488,10 @@ static inline int security_capset(struct cred *new,
> static inline int security_capable(const struct cred *cred,
> struct user_namespace *ns,
> int cap,
> - unsigned int opts)
> + unsigned int opts,
> + struct cap_aux_data *cad)
> {
> - return cap_capable(cred, ns, cap, opts);
> + return cap_capable(cred, ns, cap, opts, NULL);
> }
>
> static inline int security_quotactl(int cmds, int type, int id,
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 1444f3954d75..c3723443904a 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -297,7 +297,7 @@ bool has_ns_capability(struct task_struct *t,
> int ret;
>
> rcu_read_lock();
> - ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NONE);
> + ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NONE, NULL);
> rcu_read_unlock();
>
> return (ret == 0);
> @@ -338,7 +338,7 @@ bool has_ns_capability_noaudit(struct task_struct *t,
> int ret;
>
> rcu_read_lock();
> - ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NOAUDIT);
> + ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NOAUDIT, NULL);
> rcu_read_unlock();
>
> return (ret == 0);
> @@ -363,7 +363,8 @@ bool has_capability_noaudit(struct task_struct *t, int cap)
>
> static bool ns_capable_common(struct user_namespace *ns,
> int cap,
> - unsigned int opts)
> + unsigned int opts,
> + struct cap_aux_data *cad)
> {
> int capable;
>
> @@ -372,7 +373,7 @@ static bool ns_capable_common(struct user_namespace *ns,
> BUG();
> }
>
> - capable = security_capable(current_cred(), ns, cap, opts);
> + capable = security_capable(current_cred(), ns, cap, opts, cad);
> if (capable == 0) {
> current->flags |= PF_SUPERPRIV;
> return true;
> @@ -393,7 +394,7 @@ static bool ns_capable_common(struct user_namespace *ns,
> */
> bool ns_capable(struct user_namespace *ns, int cap)
> {
> - return ns_capable_common(ns, cap, CAP_OPT_NONE);
> + return ns_capable_common(ns, cap, CAP_OPT_NONE, NULL);
> }
> EXPORT_SYMBOL(ns_capable);
>
> @@ -411,7 +412,7 @@ EXPORT_SYMBOL(ns_capable);
> */
> bool ns_capable_noaudit(struct user_namespace *ns, int cap)
> {
> - return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT);
> + return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT, NULL);
> }
> EXPORT_SYMBOL(ns_capable_noaudit);
>
> @@ -430,7 +431,7 @@ EXPORT_SYMBOL(ns_capable_noaudit);
> */
> bool ns_capable_setid(struct user_namespace *ns, int cap)
> {
> - return ns_capable_common(ns, cap, CAP_OPT_INSETID);
> + return ns_capable_common(ns, cap, CAP_OPT_INSETID, NULL);
> }
> EXPORT_SYMBOL(ns_capable_setid);
>
> @@ -470,7 +471,7 @@ bool file_ns_capable(const struct file *file, struct user_namespace *ns,
> if (WARN_ON_ONCE(!cap_valid(cap)))
> return false;
>
> - if (security_capable(file->f_cred, ns, cap, CAP_OPT_NONE) == 0)
> + if (security_capable(file->f_cred, ns, cap, CAP_OPT_NONE, NULL) == 0)
> return true;
>
> return false;
> @@ -503,7 +504,8 @@ bool capable_wrt_inode_uidgid(const struct inode *inode, int cap)
> {
> struct user_namespace *ns = current_user_ns();
>
> - return ns_capable(ns, cap) && privileged_wrt_inode_uidgid(ns, inode);
> + return ns_capable_inode(ns, cap, inode) &&
> + privileged_wrt_inode_uidgid(ns, inode);
> }
> EXPORT_SYMBOL(capable_wrt_inode_uidgid);
>
> @@ -524,7 +526,18 @@ bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns)
> cred = rcu_dereference(tsk->ptracer_cred);
> if (cred)
> ret = security_capable(cred, ns, CAP_SYS_PTRACE,
> - CAP_OPT_NOAUDIT);
> + CAP_OPT_NOAUDIT, NULL);
> rcu_read_unlock();
> return (ret == 0);
> }
> +
> +bool ns_capable_inode(struct user_namespace *ns, int cap,
> + const struct inode *inode)
> +{
> + struct cap_aux_data cad;
> +
> + cad.type = CAP_AUX_DATA_INODE;
> + cad.u.inode = inode;
> + return ns_capable_common(ns, cap, CAP_OPT_NONE, &cad);
> +}
> +EXPORT_SYMBOL(ns_capable_inode);
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 811b4a86cdf6..d59dd7079ece 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -446,7 +446,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
> */
> if (!task_no_new_privs(current) &&
> security_capable(current_cred(), current_user_ns(),
> - CAP_SYS_ADMIN, CAP_OPT_NOAUDIT) != 0)
> + CAP_SYS_ADMIN, CAP_OPT_NOAUDIT, NULL) != 0)
> return ERR_PTR(-EACCES);
>
> /* Allocate a new seccomp_filter */
> diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c
> index 752f73980e30..c45192a16733 100644
> --- a/security/apparmor/capability.c
> +++ b/security/apparmor/capability.c
> @@ -50,7 +50,7 @@ static void audit_cb(struct audit_buffer *ab, void *va)
> struct common_audit_data *sa = va;
>
> audit_log_format(ab, " capname=");
> - audit_log_untrustedstring(ab, capability_names[sa->u.cap]);
> + audit_log_untrustedstring(ab, capability_names[sa->u.cap_struct.cap]);
> }
>
> /**
> @@ -148,13 +148,15 @@ static int profile_capable(struct aa_profile *profile, int cap,
> *
> * Returns: 0 on success, or else an error code.
> */
> -int aa_capable(struct aa_label *label, int cap, unsigned int opts)
> +int aa_capable(struct aa_label *label, int cap, unsigned int opts,
> + struct cap_aux_data *cad)
> {
> struct aa_profile *profile;
> int error = 0;
> DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_CAP, OP_CAPABLE);
>
> - sa.u.cap = cap;
> + sa.u.cap_struct.cap = cap;
> + sa.u.cap_struct.cad = cad;
> error = fn_for_each_confined(label, profile,
> profile_capable(profile, cap, opts, &sa));
>
> diff --git a/security/apparmor/include/capability.h b/security/apparmor/include/capability.h
> index 1b3663b6ab12..d888f09d76d1 100644
> --- a/security/apparmor/include/capability.h
> +++ b/security/apparmor/include/capability.h
> @@ -20,6 +20,7 @@
> #include "apparmorfs.h"
>
> struct aa_label;
> +struct cap_aux_data;
>
> /* aa_caps - confinement data for capabilities
> * @allowed: capabilities mask
> @@ -40,7 +41,8 @@ struct aa_caps {
>
> extern struct aa_sfs_entry aa_sfs_entry_caps[];
>
> -int aa_capable(struct aa_label *label, int cap, unsigned int opts);
> +int aa_capable(struct aa_label *label, int cap, unsigned int opts,
> + struct cap_aux_data *cad);
>
> static inline void aa_free_cap_rules(struct aa_caps *caps)
> {
> diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
> index aacd1e95cb59..deb5267ca695 100644
> --- a/security/apparmor/ipc.c
> +++ b/security/apparmor/ipc.c
> @@ -108,7 +108,7 @@ static int profile_tracer_perm(struct aa_profile *tracer,
> aad(sa)->peer = tracee;
> aad(sa)->request = 0;
> aad(sa)->error = aa_capable(&tracer->label, CAP_SYS_PTRACE,
> - CAP_OPT_NONE);
> + CAP_OPT_NONE, NULL);
>
> return aa_audit(AUDIT_APPARMOR_AUTO, tracer, sa, audit_ptrace_cb);
> }
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 87500bde5a92..82790accb679 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -172,14 +172,15 @@ static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
> }
>
> static int apparmor_capable(const struct cred *cred, struct user_namespace *ns,
> - int cap, unsigned int opts)
> + int cap, unsigned int opts,
> + struct cap_aux_data *cad)
> {
> struct aa_label *label;
> int error = 0;
>
> label = aa_get_newest_cred_label(cred);
> if (!unconfined(label))
> - error = aa_capable(label, cap, opts);
> + error = aa_capable(label, cap, opts, cad);
> aa_put_label(label);
>
> return error;
> diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c
> index 552ed09cb47e..9b3d4b4437f2 100644
> --- a/security/apparmor/resource.c
> +++ b/security/apparmor/resource.c
> @@ -124,7 +124,7 @@ int aa_task_setrlimit(struct aa_label *label, struct task_struct *task,
> */
>
> if (label != peer &&
> - aa_capable(label, CAP_SYS_RESOURCE, CAP_OPT_NOAUDIT) != 0)
> + aa_capable(label, CAP_SYS_RESOURCE, CAP_OPT_NOAUDIT, NULL) != 0)
> error = fn_for_each(label, profile,
> audit_resource(profile, resource,
> new_rlim->rlim_max, peer,
> diff --git a/security/commoncap.c b/security/commoncap.c
> index c477fb673701..1860ea50f473 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -68,7 +68,7 @@ static void warn_setuid_and_fcaps_mixed(const char *fname)
> * kernel's capable() and has_capability() returns 1 for this case.
> */
> int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
> - int cap, unsigned int opts)
> + int cap, unsigned int opts, struct cap_aux_data *cad)
> {
> struct user_namespace *ns = targ_ns;
>
> @@ -226,7 +226,7 @@ static inline int cap_inh_is_capped(void)
> * capability
> */
> if (cap_capable(current_cred(), current_cred()->user_ns,
> - CAP_SETPCAP, CAP_OPT_NONE) == 0)
> + CAP_SETPCAP, CAP_OPT_NONE, NULL) == 0)
> return 0;
> return 1;
> }
> @@ -1211,7 +1211,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> || (cap_capable(current_cred(),
> current_cred()->user_ns,
> CAP_SETPCAP,
> - CAP_OPT_NONE) != 0) /*[4]*/
> + CAP_OPT_NONE,
> + NULL) != 0) /*[4]*/
> /*
> * [1] no changing of bits that are locked
> * [2] no unlocking of locks
> @@ -1307,7 +1308,7 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
> int cap_sys_admin = 0;
>
> if (cap_capable(current_cred(), &init_user_ns,
> - CAP_SYS_ADMIN, CAP_OPT_NOAUDIT) == 0)
> + CAP_SYS_ADMIN, CAP_OPT_NOAUDIT, NULL) == 0)
> cap_sys_admin = 1;
>
> return cap_sys_admin;
> @@ -1328,7 +1329,7 @@ int cap_mmap_addr(unsigned long addr)
>
> if (addr < dac_mmap_min_addr) {
> ret = cap_capable(current_cred(), &init_user_ns, CAP_SYS_RAWIO,
> - CAP_OPT_NONE);
> + CAP_OPT_NONE, NULL);
> /* set PF_SUPERPRIV if it turns out we allow the low mmap */
> if (ret == 0)
> current->flags |= PF_SUPERPRIV;
> diff --git a/security/lsm_audit.c b/security/lsm_audit.c
> index 33028c098ef3..4871b2508a4a 100644
> --- a/security/lsm_audit.c
> +++ b/security/lsm_audit.c
> @@ -229,9 +229,26 @@ static void dump_common_audit_data(struct audit_buffer *ab,
> case LSM_AUDIT_DATA_IPC:
> audit_log_format(ab, " key=%d ", a->u.ipc_id);
> break;
> - case LSM_AUDIT_DATA_CAP:
> - audit_log_format(ab, " capability=%d ", a->u.cap);
> + case LSM_AUDIT_DATA_CAP: {
> + const struct inode *inode;
> +
> + if (a->u.cap_struct.cad) {
> + switch (a->u.cap_struct.cad->type) {
> + case CAP_AUX_DATA_INODE: {
> + inode = a->u.cap_struct.cad->u.inode;
> +
> + audit_log_format(ab, " dev=");
> + audit_log_untrustedstring(ab,
> + inode->i_sb->s_id);
> + audit_log_format(ab, " ino=%lu",
> + inode->i_ino);
> + break;
> + }
> + }
> + }
> + audit_log_format(ab, " capability=%d ", a->u.cap_struct.cap);
> break;
> + }
> case LSM_AUDIT_DATA_PATH: {
> struct inode *inode;
>
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index cecd38e2ac80..c74ed83e9501 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -80,7 +80,8 @@ static bool check_setuid_policy_hashtable_key_value(kuid_t parent,
> static int safesetid_security_capable(const struct cred *cred,
> struct user_namespace *ns,
> int cap,
> - unsigned int opts)
> + unsigned int opts,
> + struct cap_aux_data *cad)
> {
> if (cap == CAP_SETUID &&
> check_setuid_policy_hashtable_key(cred->uid)) {
> diff --git a/security/security.c b/security/security.c
> index 613a5c00e602..06274a7b9c4e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -691,9 +691,10 @@ int security_capset(struct cred *new, const struct cred *old,
> int security_capable(const struct cred *cred,
> struct user_namespace *ns,
> int cap,
> - unsigned int opts)
> + unsigned int opts,
> + struct cap_aux_data *cad)
> {
> - return call_int_hook(capable, 0, cred, ns, cap, opts);
> + return call_int_hook(capable, 0, cred, ns, cap, opts, cad);
> }
>
> int security_quotactl(int cmds, int type, int id, struct super_block *sb)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f77b314d0575..d6c699ed06be 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1618,7 +1618,10 @@ static inline u32 signal_to_av(int sig)
>
> /* Check whether a task is allowed to use a capability. */
> static int cred_has_capability(const struct cred *cred,
> - int cap, unsigned int opts, bool initns)
> + int cap,
> + unsigned int opts,
> + bool initns,
> + struct cap_aux_data *cad)
> {
> struct common_audit_data ad;
> struct av_decision avd;
> @@ -1628,7 +1631,8 @@ static int cred_has_capability(const struct cred *cred,
> int rc;
>
> ad.type = LSM_AUDIT_DATA_CAP;
> - ad.u.cap = cap;
> + ad.u.cap_struct.cad = cad;
> + ad.u.cap_struct.cap = cap;
>
> switch (CAP_TO_INDEX(cap)) {
> case 0:
> @@ -2165,9 +2169,9 @@ static int selinux_capset(struct cred *new, const struct cred *old,
> */
>
> static int selinux_capable(const struct cred *cred, struct user_namespace *ns,
> - int cap, unsigned int opts)
> + int cap, unsigned int opts, struct cap_aux_data *cad)
> {
> - return cred_has_capability(cred, cap, opts, ns == &init_user_ns);
> + return cred_has_capability(cred, cap, opts, ns == &init_user_ns, cad);
> }
>
> static int selinux_quotactl(int cmds, int type, int id, struct super_block *sb)
> @@ -2241,7 +2245,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
> int rc, cap_sys_admin = 0;
>
> rc = cred_has_capability(current_cred(), CAP_SYS_ADMIN,
> - CAP_OPT_NOAUDIT, true);
> + CAP_OPT_NOAUDIT, true, NULL);
> if (rc == 0)
> cap_sys_admin = 1;
>
> @@ -3101,9 +3105,9 @@ static bool has_cap_mac_admin(bool audit)
> const struct cred *cred = current_cred();
> unsigned int opts = audit ? CAP_OPT_NONE : CAP_OPT_NOAUDIT;
>
> - if (cap_capable(cred, &init_user_ns, CAP_MAC_ADMIN, opts))
> + if (cap_capable(cred, &init_user_ns, CAP_MAC_ADMIN, opts, NULL))
> return false;
> - if (cred_has_capability(cred, CAP_MAC_ADMIN, opts, true))
> + if (cred_has_capability(cred, CAP_MAC_ADMIN, opts, true, NULL))
> return false;
> return true;
> }
> @@ -3563,7 +3567,7 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd,
> case KDSKBENT:
> case KDSKBSENT:
> error = cred_has_capability(cred, CAP_SYS_TTY_CONFIG,
> - CAP_OPT_NONE, true);
> + CAP_OPT_NONE, true, NULL);
> break;
>
> /* default case assumes that the command will go
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index fe2ce3a65822..e961bfe8f00a 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -640,7 +640,7 @@ bool smack_privileged_cred(int cap, const struct cred *cred)
> struct smack_known_list_elem *sklep;
> int rc;
>
> - rc = cap_capable(cred, &init_user_ns, cap, CAP_OPT_NONE);
> + rc = cap_capable(cred, &init_user_ns, cap, CAP_OPT_NONE, NULL);
> if (rc)
> return false;
>
^ permalink raw reply
* Re: [RFC PATCH] security, capability: pass object information to security_capable
From: Stephen Smalley @ 2019-07-12 18:02 UTC (permalink / raw)
To: James Morris, Nicholas Franck
Cc: paul, selinux, linux-security-module, luto, keescook, serge,
john.johansen, casey, mortonm, linux-audit@redhat.com
In-Reply-To: <alpine.LRH.2.21.1907130347010.1509@namei.org>
On 7/12/19 1:50 PM, James Morris wrote:
> On Fri, 12 Jul 2019, Nicholas Franck wrote:
>
>> + case LSM_AUDIT_DATA_CAP: {
>> + const struct inode *inode;
>> +
>> + if (a->u.cap_struct.cad) {
>> + switch (a->u.cap_struct.cad->type) {
>> + case CAP_AUX_DATA_INODE: {
>> + inode = a->u.cap_struct.cad->u.inode;
>> +
>> + audit_log_format(ab, " dev=");
>> + audit_log_untrustedstring(ab,
>> + inode->i_sb->s_id);
>> + audit_log_format(ab, " ino=%lu",
>> + inode->i_ino);
>> + break;
>> + }
>> + }
>> + }
>> + audit_log_format(ab, " capability=%d ", a->u.cap_struct.cap);
>> break;
>
> Will this break any existing userspace log parsers?
I'm hoping not given that we are only adding auxiliary fields and those
are already defined for other AVC audit messages. ausearch appeared to
work fine. Added the linux-audit mailing list to the cc line to get
their view.
^ permalink raw reply
* Re: [RFC PATCH] security, capability: pass object information to security_capable
From: Stephen Smalley @ 2019-07-12 18:25 UTC (permalink / raw)
To: Casey Schaufler, Nicholas Franck, paul
Cc: selinux, linux-security-module, luto, jmorris, keescook, serge,
john.johansen, mortonm
In-Reply-To: <680c35a8-1ee5-725d-b33c-7bdced39763c@schaufler-ca.com>
On 7/12/19 1:58 PM, Casey Schaufler wrote:
> On 7/12/2019 10:34 AM, Nicholas Franck wrote:
>> At present security_capable does not pass any object information
>> and therefore can neither audit the particular object nor take it
>> into account. Augment the security_capable interface to support
>> passing supplementary data. Use this facility initially to convey
>> the inode for capability checks relevant to inodes. This only
>> addresses capable_wrt_inode_uidgid calls; other capability checks
>> relevant to inodes will be addressed in subsequent changes. In the
>> future, this will be further extended to pass object information for
>> other capability checks such as the target task for CAP_KILL.
>
> This seems wrong to me. The capability system has nothing to do
> with objects. Passing object information through security_capable()
> may be convenient, but isn't relevant to the purpose of the interface.
> It appears that there are very few places where the object information
> is actually useful.
A fair number of capabilities are checked upon some attempted object
access (often right after comparing UIDs or other per-object state), and
the particular object can be very helpful in both audit and in access
control. More below.
>
>> In SELinux this new information is leveraged here to include the inode
>> in the audit message. In the future, it could also be used to perform
>> a per inode capability checks.
>
> I suggest that you want a mechanism for adding the inode information
> to the audit record instead.
That is part of what we want, but not the entire picture. But with
respect to audit, the problem today is that one sees SELinux
dac_read_search, dac_override, etc denials with no indication as to the
particular file, which is unfortunate both from a security auditing
perspective and from a policy development perspective. The only option
today to gain that information is by enabling system call audit and
setting at least one audit filter so that the audit framework will
collect that information and include it in the audit records that are
emitted upon syscall exit after any such AVC denial. Unfortunately,
that is all disabled by default on most systems due to its associated
performance overhead, and one doesn't even have the option of enabling
it on some systems, e.g. Android devices. And even when one can enable
syscall audit, one must correlate the syscall audit record to the
associated AVC record to identify the object rather than having the
information directly included in the same record.
> What would a "per inode" capability check be? Capability checks are
> process checks, not object checks.
Ideally it would be possible to scope dac_override and other
capabilities to specific objects rather than having to allow it for all
or none. Just because a process needs to override DAC on one file or
one set of files is not a reason to allow it to do so on every file it
can access at all. If we want to apply least privilege, then this is a
desirable facility. I understand that doesn't mesh with Smack's mental
model but it would probably be useful to both SELinux and AppArmor,
among others.
>
>> It would be possible to fold the existing opts argument into this new
>> supplementary data structure. This was omitted from this change to
>> minimize changes.
>>
>> Signed-off-by: Nicholas Franck <nhfran2@tycho.nsa.gov>
>> ---
>> include/linux/capability.h | 7 ++++++
>> include/linux/lsm_audit.h | 5 +++-
>> include/linux/lsm_hooks.h | 3 ++-
>> include/linux/security.h | 23 +++++++++++++-----
>> kernel/capability.c | 33 ++++++++++++++++++--------
>> kernel/seccomp.c | 2 +-
>> security/apparmor/capability.c | 8 ++++---
>> security/apparmor/include/capability.h | 4 +++-
>> security/apparmor/ipc.c | 2 +-
>> security/apparmor/lsm.c | 5 ++--
>> security/apparmor/resource.c | 2 +-
>> security/commoncap.c | 11 +++++----
>> security/lsm_audit.c | 21 ++++++++++++++--
>> security/safesetid/lsm.c | 3 ++-
>> security/security.c | 5 ++--
>> security/selinux/hooks.c | 20 +++++++++-------
>> security/smack/smack_access.c | 2 +-
>> 17 files changed, 110 insertions(+), 46 deletions(-)
>>
>> diff --git a/include/linux/capability.h b/include/linux/capability.h
>> index ecce0f43c73a..f72de64c179d 100644
>> --- a/include/linux/capability.h
>> +++ b/include/linux/capability.h
>> @@ -211,6 +211,8 @@ extern bool capable(int cap);
>> extern bool ns_capable(struct user_namespace *ns, int cap);
>> extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
>> extern bool ns_capable_setid(struct user_namespace *ns, int cap);
>> +extern bool ns_capable_inode(struct user_namespace *ns, int cap,
>> + const struct inode *inode);
>> #else
>> static inline bool has_capability(struct task_struct *t, int cap)
>> {
>> @@ -246,6 +248,11 @@ static inline bool ns_capable_setid(struct user_namespace *ns, int cap)
>> {
>> return true;
>> }
>> +static bool ns_capable_inode(struct user_namespace *ns, int cap,
>> + const struct inode *inode)
>> +{
>> + return true;
>> +}
>> #endif /* CONFIG_MULTIUSER */
>> extern bool privileged_wrt_inode_uidgid(struct user_namespace *ns, const struct inode *inode);
>> extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
>> diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
>> index 915330abf6e5..15d2a62639f0 100644
>> --- a/include/linux/lsm_audit.h
>> +++ b/include/linux/lsm_audit.h
>> @@ -79,7 +79,10 @@ struct common_audit_data {
>> struct dentry *dentry;
>> struct inode *inode;
>> struct lsm_network_audit *net;
>> - int cap;
>> + struct {
>> + int cap;
>
>> + struct cap_aux_data *cad;
>> + } cap_struct;
>> int ipc_id;
>> struct task_struct *tsk;
>> #ifdef CONFIG_KEYS
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 47f58cfb6a19..b2a37d613030 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1469,7 +1469,8 @@ union security_list_options {
>> int (*capable)(const struct cred *cred,
>> struct user_namespace *ns,
>> int cap,
>> - unsigned int opts);
>> + unsigned int opts,
>> + struct cap_aux_data *cad);
>> int (*quotactl)(int cmds, int type, int id, struct super_block *sb);
>> int (*quota_on)(struct dentry *dentry);
>> int (*syslog)(int type);
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 659071c2e57c..8fce5e69dc52 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -77,9 +77,18 @@ enum lsm_event {
>> LSM_POLICY_CHANGE,
>> };
>>
>> +
>> +struct cap_aux_data {
>> + char type;
>> +#define CAP_AUX_DATA_INODE 1
>> + union {
>> + const struct inode *inode;
>> + } u;
>> +};
>> +
>> /* These functions are in security/commoncap.c */
>> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
>> - int cap, unsigned int opts);
>> + int cap, unsigned int opts, struct cap_aux_data *cad);
>> extern int cap_settime(const struct timespec64 *ts, const struct timezone *tz);
>> extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode);
>> extern int cap_ptrace_traceme(struct task_struct *parent);
>> @@ -215,9 +224,10 @@ int security_capset(struct cred *new, const struct cred *old,
>> const kernel_cap_t *inheritable,
>> const kernel_cap_t *permitted);
>> int security_capable(const struct cred *cred,
>> - struct user_namespace *ns,
>> - int cap,
>> - unsigned int opts);
>> + struct user_namespace *ns,
>> + int cap,
>> + unsigned int opts,
>> + struct cap_aux_data *cad);
>> int security_quotactl(int cmds, int type, int id, struct super_block *sb);
>> int security_quota_on(struct dentry *dentry);
>> int security_syslog(int type);
>> @@ -478,9 +488,10 @@ static inline int security_capset(struct cred *new,
>> static inline int security_capable(const struct cred *cred,
>> struct user_namespace *ns,
>> int cap,
>> - unsigned int opts)
>> + unsigned int opts,
>> + struct cap_aux_data *cad)
>> {
>> - return cap_capable(cred, ns, cap, opts);
>> + return cap_capable(cred, ns, cap, opts, NULL);
>> }
>>
>> static inline int security_quotactl(int cmds, int type, int id,
>> diff --git a/kernel/capability.c b/kernel/capability.c
>> index 1444f3954d75..c3723443904a 100644
>> --- a/kernel/capability.c
>> +++ b/kernel/capability.c
>> @@ -297,7 +297,7 @@ bool has_ns_capability(struct task_struct *t,
>> int ret;
>>
>> rcu_read_lock();
>> - ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NONE);
>> + ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NONE, NULL);
>> rcu_read_unlock();
>>
>> return (ret == 0);
>> @@ -338,7 +338,7 @@ bool has_ns_capability_noaudit(struct task_struct *t,
>> int ret;
>>
>> rcu_read_lock();
>> - ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NOAUDIT);
>> + ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NOAUDIT, NULL);
>> rcu_read_unlock();
>>
>> return (ret == 0);
>> @@ -363,7 +363,8 @@ bool has_capability_noaudit(struct task_struct *t, int cap)
>>
>> static bool ns_capable_common(struct user_namespace *ns,
>> int cap,
>> - unsigned int opts)
>> + unsigned int opts,
>> + struct cap_aux_data *cad)
>> {
>> int capable;
>>
>> @@ -372,7 +373,7 @@ static bool ns_capable_common(struct user_namespace *ns,
>> BUG();
>> }
>>
>> - capable = security_capable(current_cred(), ns, cap, opts);
>> + capable = security_capable(current_cred(), ns, cap, opts, cad);
>> if (capable == 0) {
>> current->flags |= PF_SUPERPRIV;
>> return true;
>> @@ -393,7 +394,7 @@ static bool ns_capable_common(struct user_namespace *ns,
>> */
>> bool ns_capable(struct user_namespace *ns, int cap)
>> {
>> - return ns_capable_common(ns, cap, CAP_OPT_NONE);
>> + return ns_capable_common(ns, cap, CAP_OPT_NONE, NULL);
>> }
>> EXPORT_SYMBOL(ns_capable);
>>
>> @@ -411,7 +412,7 @@ EXPORT_SYMBOL(ns_capable);
>> */
>> bool ns_capable_noaudit(struct user_namespace *ns, int cap)
>> {
>> - return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT);
>> + return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT, NULL);
>> }
>> EXPORT_SYMBOL(ns_capable_noaudit);
>>
>> @@ -430,7 +431,7 @@ EXPORT_SYMBOL(ns_capable_noaudit);
>> */
>> bool ns_capable_setid(struct user_namespace *ns, int cap)
>> {
>> - return ns_capable_common(ns, cap, CAP_OPT_INSETID);
>> + return ns_capable_common(ns, cap, CAP_OPT_INSETID, NULL);
>> }
>> EXPORT_SYMBOL(ns_capable_setid);
>>
>> @@ -470,7 +471,7 @@ bool file_ns_capable(const struct file *file, struct user_namespace *ns,
>> if (WARN_ON_ONCE(!cap_valid(cap)))
>> return false;
>>
>> - if (security_capable(file->f_cred, ns, cap, CAP_OPT_NONE) == 0)
>> + if (security_capable(file->f_cred, ns, cap, CAP_OPT_NONE, NULL) == 0)
>> return true;
>>
>> return false;
>> @@ -503,7 +504,8 @@ bool capable_wrt_inode_uidgid(const struct inode *inode, int cap)
>> {
>> struct user_namespace *ns = current_user_ns();
>>
>> - return ns_capable(ns, cap) && privileged_wrt_inode_uidgid(ns, inode);
>> + return ns_capable_inode(ns, cap, inode) &&
>> + privileged_wrt_inode_uidgid(ns, inode);
>> }
>> EXPORT_SYMBOL(capable_wrt_inode_uidgid);
>>
>> @@ -524,7 +526,18 @@ bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns)
>> cred = rcu_dereference(tsk->ptracer_cred);
>> if (cred)
>> ret = security_capable(cred, ns, CAP_SYS_PTRACE,
>> - CAP_OPT_NOAUDIT);
>> + CAP_OPT_NOAUDIT, NULL);
>> rcu_read_unlock();
>> return (ret == 0);
>> }
>> +
>> +bool ns_capable_inode(struct user_namespace *ns, int cap,
>> + const struct inode *inode)
>> +{
>> + struct cap_aux_data cad;
>> +
>> + cad.type = CAP_AUX_DATA_INODE;
>> + cad.u.inode = inode;
>> + return ns_capable_common(ns, cap, CAP_OPT_NONE, &cad);
>> +}
>> +EXPORT_SYMBOL(ns_capable_inode);
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index 811b4a86cdf6..d59dd7079ece 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -446,7 +446,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
>> */
>> if (!task_no_new_privs(current) &&
>> security_capable(current_cred(), current_user_ns(),
>> - CAP_SYS_ADMIN, CAP_OPT_NOAUDIT) != 0)
>> + CAP_SYS_ADMIN, CAP_OPT_NOAUDIT, NULL) != 0)
>> return ERR_PTR(-EACCES);
>>
>> /* Allocate a new seccomp_filter */
>> diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c
>> index 752f73980e30..c45192a16733 100644
>> --- a/security/apparmor/capability.c
>> +++ b/security/apparmor/capability.c
>> @@ -50,7 +50,7 @@ static void audit_cb(struct audit_buffer *ab, void *va)
>> struct common_audit_data *sa = va;
>>
>> audit_log_format(ab, " capname=");
>> - audit_log_untrustedstring(ab, capability_names[sa->u.cap]);
>> + audit_log_untrustedstring(ab, capability_names[sa->u.cap_struct.cap]);
>> }
>>
>> /**
>> @@ -148,13 +148,15 @@ static int profile_capable(struct aa_profile *profile, int cap,
>> *
>> * Returns: 0 on success, or else an error code.
>> */
>> -int aa_capable(struct aa_label *label, int cap, unsigned int opts)
>> +int aa_capable(struct aa_label *label, int cap, unsigned int opts,
>> + struct cap_aux_data *cad)
>> {
>> struct aa_profile *profile;
>> int error = 0;
>> DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_CAP, OP_CAPABLE);
>>
>> - sa.u.cap = cap;
>> + sa.u.cap_struct.cap = cap;
>> + sa.u.cap_struct.cad = cad;
>> error = fn_for_each_confined(label, profile,
>> profile_capable(profile, cap, opts, &sa));
>>
>> diff --git a/security/apparmor/include/capability.h b/security/apparmor/include/capability.h
>> index 1b3663b6ab12..d888f09d76d1 100644
>> --- a/security/apparmor/include/capability.h
>> +++ b/security/apparmor/include/capability.h
>> @@ -20,6 +20,7 @@
>> #include "apparmorfs.h"
>>
>> struct aa_label;
>> +struct cap_aux_data;
>>
>> /* aa_caps - confinement data for capabilities
>> * @allowed: capabilities mask
>> @@ -40,7 +41,8 @@ struct aa_caps {
>>
>> extern struct aa_sfs_entry aa_sfs_entry_caps[];
>>
>> -int aa_capable(struct aa_label *label, int cap, unsigned int opts);
>> +int aa_capable(struct aa_label *label, int cap, unsigned int opts,
>> + struct cap_aux_data *cad);
>>
>> static inline void aa_free_cap_rules(struct aa_caps *caps)
>> {
>> diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
>> index aacd1e95cb59..deb5267ca695 100644
>> --- a/security/apparmor/ipc.c
>> +++ b/security/apparmor/ipc.c
>> @@ -108,7 +108,7 @@ static int profile_tracer_perm(struct aa_profile *tracer,
>> aad(sa)->peer = tracee;
>> aad(sa)->request = 0;
>> aad(sa)->error = aa_capable(&tracer->label, CAP_SYS_PTRACE,
>> - CAP_OPT_NONE);
>> + CAP_OPT_NONE, NULL);
>>
>> return aa_audit(AUDIT_APPARMOR_AUTO, tracer, sa, audit_ptrace_cb);
>> }
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index 87500bde5a92..82790accb679 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -172,14 +172,15 @@ static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
>> }
>>
>> static int apparmor_capable(const struct cred *cred, struct user_namespace *ns,
>> - int cap, unsigned int opts)
>> + int cap, unsigned int opts,
>> + struct cap_aux_data *cad)
>> {
>> struct aa_label *label;
>> int error = 0;
>>
>> label = aa_get_newest_cred_label(cred);
>> if (!unconfined(label))
>> - error = aa_capable(label, cap, opts);
>> + error = aa_capable(label, cap, opts, cad);
>> aa_put_label(label);
>>
>> return error;
>> diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c
>> index 552ed09cb47e..9b3d4b4437f2 100644
>> --- a/security/apparmor/resource.c
>> +++ b/security/apparmor/resource.c
>> @@ -124,7 +124,7 @@ int aa_task_setrlimit(struct aa_label *label, struct task_struct *task,
>> */
>>
>> if (label != peer &&
>> - aa_capable(label, CAP_SYS_RESOURCE, CAP_OPT_NOAUDIT) != 0)
>> + aa_capable(label, CAP_SYS_RESOURCE, CAP_OPT_NOAUDIT, NULL) != 0)
>> error = fn_for_each(label, profile,
>> audit_resource(profile, resource,
>> new_rlim->rlim_max, peer,
>> diff --git a/security/commoncap.c b/security/commoncap.c
>> index c477fb673701..1860ea50f473 100644
>> --- a/security/commoncap.c
>> +++ b/security/commoncap.c
>> @@ -68,7 +68,7 @@ static void warn_setuid_and_fcaps_mixed(const char *fname)
>> * kernel's capable() and has_capability() returns 1 for this case.
>> */
>> int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
>> - int cap, unsigned int opts)
>> + int cap, unsigned int opts, struct cap_aux_data *cad)
>> {
>> struct user_namespace *ns = targ_ns;
>>
>> @@ -226,7 +226,7 @@ static inline int cap_inh_is_capped(void)
>> * capability
>> */
>> if (cap_capable(current_cred(), current_cred()->user_ns,
>> - CAP_SETPCAP, CAP_OPT_NONE) == 0)
>> + CAP_SETPCAP, CAP_OPT_NONE, NULL) == 0)
>> return 0;
>> return 1;
>> }
>> @@ -1211,7 +1211,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
>> || (cap_capable(current_cred(),
>> current_cred()->user_ns,
>> CAP_SETPCAP,
>> - CAP_OPT_NONE) != 0) /*[4]*/
>> + CAP_OPT_NONE,
>> + NULL) != 0) /*[4]*/
>> /*
>> * [1] no changing of bits that are locked
>> * [2] no unlocking of locks
>> @@ -1307,7 +1308,7 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
>> int cap_sys_admin = 0;
>>
>> if (cap_capable(current_cred(), &init_user_ns,
>> - CAP_SYS_ADMIN, CAP_OPT_NOAUDIT) == 0)
>> + CAP_SYS_ADMIN, CAP_OPT_NOAUDIT, NULL) == 0)
>> cap_sys_admin = 1;
>>
>> return cap_sys_admin;
>> @@ -1328,7 +1329,7 @@ int cap_mmap_addr(unsigned long addr)
>>
>> if (addr < dac_mmap_min_addr) {
>> ret = cap_capable(current_cred(), &init_user_ns, CAP_SYS_RAWIO,
>> - CAP_OPT_NONE);
>> + CAP_OPT_NONE, NULL);
>> /* set PF_SUPERPRIV if it turns out we allow the low mmap */
>> if (ret == 0)
>> current->flags |= PF_SUPERPRIV;
>> diff --git a/security/lsm_audit.c b/security/lsm_audit.c
>> index 33028c098ef3..4871b2508a4a 100644
>> --- a/security/lsm_audit.c
>> +++ b/security/lsm_audit.c
>> @@ -229,9 +229,26 @@ static void dump_common_audit_data(struct audit_buffer *ab,
>> case LSM_AUDIT_DATA_IPC:
>> audit_log_format(ab, " key=%d ", a->u.ipc_id);
>> break;
>> - case LSM_AUDIT_DATA_CAP:
>> - audit_log_format(ab, " capability=%d ", a->u.cap);
>> + case LSM_AUDIT_DATA_CAP: {
>> + const struct inode *inode;
>> +
>> + if (a->u.cap_struct.cad) {
>> + switch (a->u.cap_struct.cad->type) {
>> + case CAP_AUX_DATA_INODE: {
>> + inode = a->u.cap_struct.cad->u.inode;
>> +
>> + audit_log_format(ab, " dev=");
>> + audit_log_untrustedstring(ab,
>> + inode->i_sb->s_id);
>> + audit_log_format(ab, " ino=%lu",
>> + inode->i_ino);
>> + break;
>> + }
>> + }
>> + }
>> + audit_log_format(ab, " capability=%d ", a->u.cap_struct.cap);
>> break;
>> + }
>> case LSM_AUDIT_DATA_PATH: {
>> struct inode *inode;
>>
>> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
>> index cecd38e2ac80..c74ed83e9501 100644
>> --- a/security/safesetid/lsm.c
>> +++ b/security/safesetid/lsm.c
>> @@ -80,7 +80,8 @@ static bool check_setuid_policy_hashtable_key_value(kuid_t parent,
>> static int safesetid_security_capable(const struct cred *cred,
>> struct user_namespace *ns,
>> int cap,
>> - unsigned int opts)
>> + unsigned int opts,
>> + struct cap_aux_data *cad)
>> {
>> if (cap == CAP_SETUID &&
>> check_setuid_policy_hashtable_key(cred->uid)) {
>> diff --git a/security/security.c b/security/security.c
>> index 613a5c00e602..06274a7b9c4e 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -691,9 +691,10 @@ int security_capset(struct cred *new, const struct cred *old,
>> int security_capable(const struct cred *cred,
>> struct user_namespace *ns,
>> int cap,
>> - unsigned int opts)
>> + unsigned int opts,
>> + struct cap_aux_data *cad)
>> {
>> - return call_int_hook(capable, 0, cred, ns, cap, opts);
>> + return call_int_hook(capable, 0, cred, ns, cap, opts, cad);
>> }
>>
>> int security_quotactl(int cmds, int type, int id, struct super_block *sb)
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index f77b314d0575..d6c699ed06be 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -1618,7 +1618,10 @@ static inline u32 signal_to_av(int sig)
>>
>> /* Check whether a task is allowed to use a capability. */
>> static int cred_has_capability(const struct cred *cred,
>> - int cap, unsigned int opts, bool initns)
>> + int cap,
>> + unsigned int opts,
>> + bool initns,
>> + struct cap_aux_data *cad)
>> {
>> struct common_audit_data ad;
>> struct av_decision avd;
>> @@ -1628,7 +1631,8 @@ static int cred_has_capability(const struct cred *cred,
>> int rc;
>>
>> ad.type = LSM_AUDIT_DATA_CAP;
>> - ad.u.cap = cap;
>> + ad.u.cap_struct.cad = cad;
>> + ad.u.cap_struct.cap = cap;
>>
>> switch (CAP_TO_INDEX(cap)) {
>> case 0:
>> @@ -2165,9 +2169,9 @@ static int selinux_capset(struct cred *new, const struct cred *old,
>> */
>>
>> static int selinux_capable(const struct cred *cred, struct user_namespace *ns,
>> - int cap, unsigned int opts)
>> + int cap, unsigned int opts, struct cap_aux_data *cad)
>> {
>> - return cred_has_capability(cred, cap, opts, ns == &init_user_ns);
>> + return cred_has_capability(cred, cap, opts, ns == &init_user_ns, cad);
>> }
>>
>> static int selinux_quotactl(int cmds, int type, int id, struct super_block *sb)
>> @@ -2241,7 +2245,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
>> int rc, cap_sys_admin = 0;
>>
>> rc = cred_has_capability(current_cred(), CAP_SYS_ADMIN,
>> - CAP_OPT_NOAUDIT, true);
>> + CAP_OPT_NOAUDIT, true, NULL);
>> if (rc == 0)
>> cap_sys_admin = 1;
>>
>> @@ -3101,9 +3105,9 @@ static bool has_cap_mac_admin(bool audit)
>> const struct cred *cred = current_cred();
>> unsigned int opts = audit ? CAP_OPT_NONE : CAP_OPT_NOAUDIT;
>>
>> - if (cap_capable(cred, &init_user_ns, CAP_MAC_ADMIN, opts))
>> + if (cap_capable(cred, &init_user_ns, CAP_MAC_ADMIN, opts, NULL))
>> return false;
>> - if (cred_has_capability(cred, CAP_MAC_ADMIN, opts, true))
>> + if (cred_has_capability(cred, CAP_MAC_ADMIN, opts, true, NULL))
>> return false;
>> return true;
>> }
>> @@ -3563,7 +3567,7 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd,
>> case KDSKBENT:
>> case KDSKBSENT:
>> error = cred_has_capability(cred, CAP_SYS_TTY_CONFIG,
>> - CAP_OPT_NONE, true);
>> + CAP_OPT_NONE, true, NULL);
>> break;
>>
>> /* default case assumes that the command will go
>> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
>> index fe2ce3a65822..e961bfe8f00a 100644
>> --- a/security/smack/smack_access.c
>> +++ b/security/smack/smack_access.c
>> @@ -640,7 +640,7 @@ bool smack_privileged_cred(int cap, const struct cred *cred)
>> struct smack_known_list_elem *sklep;
>> int rc;
>>
>> - rc = cap_capable(cred, &init_user_ns, cap, CAP_OPT_NONE);
>> + rc = cap_capable(cred, &init_user_ns, cap, CAP_OPT_NONE, NULL);
>> if (rc)
>> return false;
>>
>
^ permalink raw reply
* Re: [RFC PATCH] security, capability: pass object information to security_capable
From: Casey Schaufler @ 2019-07-12 19:54 UTC (permalink / raw)
To: Stephen Smalley, Nicholas Franck, paul
Cc: selinux, linux-security-module, luto, jmorris, keescook, serge,
john.johansen, mortonm, casey
In-Reply-To: <e8de4a1c-7e18-fc20-e372-67bbaa93fd42@tycho.nsa.gov>
On 7/12/2019 11:25 AM, Stephen Smalley wrote:
> On 7/12/19 1:58 PM, Casey Schaufler wrote:
>> On 7/12/2019 10:34 AM, Nicholas Franck wrote:
>>> At present security_capable does not pass any object information
>>> and therefore can neither audit the particular object nor take it
>>> into account. Augment the security_capable interface to support
>>> passing supplementary data. Use this facility initially to convey
>>> the inode for capability checks relevant to inodes. This only
>>> addresses capable_wrt_inode_uidgid calls; other capability checks
>>> relevant to inodes will be addressed in subsequent changes. In the
>>> future, this will be further extended to pass object information for
>>> other capability checks such as the target task for CAP_KILL.
>>
>> This seems wrong to me. The capability system has nothing to do
>> with objects. Passing object information through security_capable()
>> may be convenient, but isn't relevant to the purpose of the interface.
>> It appears that there are very few places where the object information
>> is actually useful.
>
> A fair number of capabilities are checked upon some attempted object access (often right after comparing UIDs or other per-object state), and the particular object can be very helpful in both audit and in access control. More below.
I'm not disagreeing with that. What I'm saying is that the capability
check interface is not the right place to pass that information. The
capability check has no use for the object information. I would much
rather see a security_pass_object_data() hook that gets called after
(or before) the security_capable() hook in the places where you want
the extra information.
>>> In SELinux this new information is leveraged here to include the inode
>>> in the audit message. In the future, it could also be used to perform
>>> a per inode capability checks.
>>
>> I suggest that you want a mechanism for adding the inode information
>> to the audit record instead.
>
> That is part of what we want, but not the entire picture. But with respect to audit, the problem today is that one sees SELinux dac_read_search, dac_override, etc denials with no indication as to the particular file, which is unfortunate both from a security auditing perspective and from a policy development perspective.
I can see how that is a problem.
> The only option today to gain that information is by enabling system call audit and setting at least one audit filter so that the audit framework will collect that information and include it in the audit records that are emitted upon syscall exit after any such AVC denial. Unfortunately, that is all disabled by default on most systems due to its associated performance overhead, and one doesn't even have the option of enabling it on some systems, e.g. Android devices. And even when one can enable syscall audit, one must correlate the syscall audit record to the associated AVC record to identify the object rather than having the information directly included in the same record.
None of which gives any rationale for adding the information
to the capability check. Sure, it's in the right place, but there
is no object interaction with the capability call.
>> What would a "per inode" capability check be? Capability checks are
>> process checks, not object checks.
>
> Ideally it would be possible to scope dac_override and other capabilities to specific objects rather than having to allow it for all or none.
That would require a major overhaul of the capability scheme,
and you're going to get arguments from people like me about
whether or not that would be ideal. Besides, isn't that what
SELinux is all about, providing that sort of privilege granularity?
> Just because a process needs to override DAC on one file or one set of files is not a reason to allow it to do so on every file it can access at all.
That's an argument for privilege bracketing within the process.
Not something I recommend (the oft referenced sendmail problems
being but one example) but the only way to do it properly without
delving into path based controls.
> If we want to apply least privilege, then this is a desirable facility.
The capability mechanism is object agnostic by design.
> I understand that doesn't mesh with Smack's mental modelbut it would probably be useful to both SELinux and AppArmor, among others.
I'm perfectly happy to have the information transmitted.
I think a separate interface for doing so is appropriate.
^ permalink raw reply
* Re: [PATCH 1/6] security: Add hooks to rule on setting a superblock or mount watch [ver #5]
From: James Morris @ 2019-07-12 20:11 UTC (permalink / raw)
To: David Howells
Cc: 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-security-module, linux-kernel
In-Reply-To: <156173702349.15650.1484210092464492434.stgit@warthog.procyon.org.uk>
On Fri, 28 Jun 2019, David Howells wrote:
> Add security hooks that will allow an LSM to rule on whether or not a watch
> may be set on a mount or on a superblock. More than one hook is required
> as the watches watch different types of object.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Casey Schaufler <casey@schaufler-ca.com>
> cc: Stephen Smalley <sds@tycho.nsa.gov>
> cc: linux-security-module@vger.kernel.org
> ---
>
> include/linux/lsm_hooks.h | 16 ++++++++++++++++
> include/linux/security.h | 10 ++++++++++
> security/security.c | 10 ++++++++++
> 3 files changed, 36 insertions(+)
Acked-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH] security, capability: pass object information to security_capable
From: Stephen Smalley @ 2019-07-12 20:21 UTC (permalink / raw)
To: Casey Schaufler, Nicholas Franck, paul
Cc: selinux, linux-security-module, luto, jmorris, keescook, serge,
john.johansen, mortonm
In-Reply-To: <16cade37-9467-ca7f-ddea-b8254c501f48@schaufler-ca.com>
On 7/12/19 3:54 PM, Casey Schaufler wrote:
> On 7/12/2019 11:25 AM, Stephen Smalley wrote:
>> On 7/12/19 1:58 PM, Casey Schaufler wrote:
>>> On 7/12/2019 10:34 AM, Nicholas Franck wrote:
>>>> At present security_capable does not pass any object information
>>>> and therefore can neither audit the particular object nor take it
>>>> into account. Augment the security_capable interface to support
>>>> passing supplementary data. Use this facility initially to convey
>>>> the inode for capability checks relevant to inodes. This only
>>>> addresses capable_wrt_inode_uidgid calls; other capability checks
>>>> relevant to inodes will be addressed in subsequent changes. In the
>>>> future, this will be further extended to pass object information for
>>>> other capability checks such as the target task for CAP_KILL.
>>>
>>> This seems wrong to me. The capability system has nothing to do
>>> with objects. Passing object information through security_capable()
>>> may be convenient, but isn't relevant to the purpose of the interface.
>>> It appears that there are very few places where the object information
>>> is actually useful.
>>
>> A fair number of capabilities are checked upon some attempted object access (often right after comparing UIDs or other per-object state), and the particular object can be very helpful in both audit and in access control. More below.
>
> I'm not disagreeing with that. What I'm saying is that the capability
> check interface is not the right place to pass that information. The
> capability check has no use for the object information. I would much
> rather see a security_pass_object_data() hook that gets called after
> (or before) the security_capable() hook in the places where you want
> the extra information.
I don't see how that would work or be advantageous. Within the capable
hook, the security module(s) decide whether or not to allow the
capability, and generate any relevant LSM audit records. It is
precisely at those two points (deciding and auditing) that we need the
information; both occur within the existing capable hooks. Calling a
separate hook before the capable call and e.g. saving the information in
the task security structure for later consumption during the capable
call offers only overhead, no benefit. Calling a separate hook after
the capable call is too late to be of use - the decision and auditing
are already done. And both hooks would be invoked from precisely the
same function at the same point. If the information wasn't already
readily available at the point of the hook call, it might be a different
matter, but that isn't the case here.
>
>>>> In SELinux this new information is leveraged here to include the inode
>>>> in the audit message. In the future, it could also be used to perform
>>>> a per inode capability checks.
>>>
>>> I suggest that you want a mechanism for adding the inode information
>>> to the audit record instead.
>>
>> That is part of what we want, but not the entire picture. But with respect to audit, the problem today is that one sees SELinux dac_read_search, dac_override, etc denials with no indication as to the particular file, which is unfortunate both from a security auditing perspective and from a policy development perspective.
>
> I can see how that is a problem.
>
>> The only option today to gain that information is by enabling system call audit and setting at least one audit filter so that the audit framework will collect that information and include it in the audit records that are emitted upon syscall exit after any such AVC denial. Unfortunately, that is all disabled by default on most systems due to its associated performance overhead, and one doesn't even have the option of enabling it on some systems, e.g. Android devices. And even when one can enable syscall audit, one must correlate the syscall audit record to the associated AVC record to identify the object rather than having the information directly included in the same record.
>
> None of which gives any rationale for adding the information
> to the capability check. Sure, it's in the right place, but there
> is no object interaction with the capability call.
We introducing such an interaction - that's the point.
>
>>> What would a "per inode" capability check be? Capability checks are
>>> process checks, not object checks.
>>
>> Ideally it would be possible to scope dac_override and other capabilities to specific objects rather than having to allow it for all or none.
>
> That would require a major overhaul of the capability scheme,
> and you're going to get arguments from people like me about
> whether or not that would be ideal. Besides, isn't that what
> SELinux is all about, providing that sort of privilege granularity?
It only requires passing the information to the security modules at the
point of the hook call, and then SELinux or other security modules can
implement it themselves without any other changes to the kernel. We
aren't changing the way the base capabilities logic works.
We can't provide that degree of granularity without the additional
information. Let's say domain A needs DAC override for files of type B
and for nothing else. To support this requirement, SELinux policy has
to include at least:
1) allow A self:capability dac_override;
2) allow A B:file { read write };
Let's say that A also reads from files of type C and writes to file of
type D. So SELinux policy also has to allow:
3) allow A C:file read;
4) allow A D:file write;
There are files within type C and within type D that are under different
DAC ownerships or modes. Only some of these files should be accessible
to A. But because dac_override is global and not scoped to specific
sets of files, the combination of these permissions now allows A to
override DAC on files of type C and of type D; thus it can read all
files of type C and write to all files of type D even though it has no
legitimate need to do so.
>
>> Just because a process needs to override DAC on one file or one set of files is not a reason to allow it to do so on every file it can access at all.
>
> That's an argument for privilege bracketing within the process.
> Not something I recommend (the oft referenced sendmail problems
> being but one example) but the only way to do it properly without
> delving into path based controls.
As you note, historically privilege bracketing hasn't worked so well,
and fundamentally it puts the trust burden on userspace for something
that could be enforced at the system level quite easily and in a
race-free manner.
>
>> If we want to apply least privilege, then this is a desirable facility.
>
> The capability mechanism is object agnostic by design.
Some might argue that's a flawed design.
>> I understand that doesn't mesh with Smack's mental modelbut it would probably be useful to both SELinux and AppArmor, among others.
>
> I'm perfectly happy to have the information transmitted.
> I think a separate interface for doing so is appropriate.
As above, I don't see any way to do that that isn't just adding overhead.
^ permalink raw reply
* Re: [RFC PATCH] security, capability: pass object information to security_capable
From: Casey Schaufler @ 2019-07-12 22:37 UTC (permalink / raw)
To: Stephen Smalley, Nicholas Franck, paul
Cc: selinux, linux-security-module, luto, jmorris, keescook, serge,
john.johansen, mortonm, casey
In-Reply-To: <4fb3a599-b1d8-7cc2-759a-02195251e344@tycho.nsa.gov>
On 7/12/2019 1:21 PM, Stephen Smalley wrote:
> On 7/12/19 3:54 PM, Casey Schaufler wrote:
>> On 7/12/2019 11:25 AM, Stephen Smalley wrote:
>>> On 7/12/19 1:58 PM, Casey Schaufler wrote:
>>>> On 7/12/2019 10:34 AM, Nicholas Franck wrote:
>>>>> At present security_capable does not pass any object information
>>>>> and therefore can neither audit the particular object nor take it
>>>>> into account. Augment the security_capable interface to support
>>>>> passing supplementary data. Use this facility initially to convey
>>>>> the inode for capability checks relevant to inodes. This only
>>>>> addresses capable_wrt_inode_uidgid calls; other capability checks
>>>>> relevant to inodes will be addressed in subsequent changes. In the
>>>>> future, this will be further extended to pass object information for
>>>>> other capability checks such as the target task for CAP_KILL.
>>>>
>>>> This seems wrong to me. The capability system has nothing to do
>>>> with objects. Passing object information through security_capable()
>>>> may be convenient, but isn't relevant to the purpose of the interface.
>>>> It appears that there are very few places where the object information
>>>> is actually useful.
>>>
>>> A fair number of capabilities are checked upon some attempted object access (often right after comparing UIDs or other per-object state), and the particular object can be very helpful in both audit and in access control. More below.
>>
>> I'm not disagreeing with that. What I'm saying is that the capability
>> check interface is not the right place to pass that information. The
>> capability check has no use for the object information. I would much
>> rather see a security_pass_object_data() hook that gets called after
>> (or before) the security_capable() hook in the places where you want
>> the extra information.
>
> I don't see how that would work or be advantageous. Within the capable hook, the security module(s) decide whether or not to allow the capability, and generate any relevant LSM audit records. It is precisely at those two points (deciding and auditing) that we need the information; both occur within the existing capable hooks. Calling a separate hook before the capable call and e.g. saving the information in the task security structure for later consumption during the capable call offers only overhead, no benefit. Calling a separate hook after the capable call is too late to be of use - the decision and auditing are already done. And both hooks would be invoked from precisely the same function at the same point. If the information wasn't already readily available at the point of the hook call, it might be a different matter, but that isn't the case here.
If there's a problem with the audit system you should be
looking at fixing that rather than tacking information that's
useless for capabilities onto it's interfaces.
>>>>> In SELinux this new information is leveraged here to include the inode
>>>>> in the audit message. In the future, it could also be used to perform
>>>>> a per inode capability checks.
>>>>
>>>> I suggest that you want a mechanism for adding the inode information
>>>> to the audit record instead.
>>>
>>> That is part of what we want, but not the entire picture. But with respect to audit, the problem today is that one sees SELinux dac_read_search, dac_override, etc denials with no indication as to the particular file, which is unfortunate both from a security auditing perspective and from a policy development perspective.
>>
>> I can see how that is a problem.
>>
>>> The only option today to gain that information is by enabling system call audit and setting at least one audit filter so that the audit framework will collect that information and include it in the audit records that are emitted upon syscall exit after any such AVC denial. Unfortunately, that is all disabled by default on most systems due to its associated performance overhead, and one doesn't even have the option of enabling it on some systems, e.g. Android devices. And even when one can enable syscall audit, one must correlate the syscall audit record to the associated AVC record to identify the object rather than having the information directly included in the same record.
>>
>> None of which gives any rationale for adding the information
>> to the capability check. Sure, it's in the right place, but there
>> is no object interaction with the capability call.
>
> We introducing such an interaction - that's the point.
And I say that you're breaking the layering.
>>>> What would a "per inode" capability check be? Capability checks are
>>>> process checks, not object checks.
>>>
>>> Ideally it would be possible to scope dac_override and other capabilities to specific objects rather than having to allow it for all or none.
>>
>> That would require a major overhaul of the capability scheme,
>> and you're going to get arguments from people like me about
>> whether or not that would be ideal. Besides, isn't that what
>> SELinux is all about, providing that sort of privilege granularity?
>
> It only requires passing the information to the security modules at the point of the hook call, and then SELinux or other security modules can implement it themselves without any other changes to the kernel. We aren't changing the way the base capabilities logic works.
If you're not changing how capabilities work you
shouldn't be adding parameters to its interface.
> We can't provide that degree of granularity without the additional information. Let's say domain A needs DAC override for files of type B and for nothing else. To support this requirement, SELinux policy has to include at least:
> 1) allow A self:capability dac_override;
> 2) allow A B:file { read write };
>
> Let's say that A also reads from files of type C and writes to file of type D. So SELinux policy also has to allow:
> 3) allow A C:file read;
> 4) allow A D:file write;
>
> There are files within type C and within type D that are under different DAC ownerships or modes. Only some of these files should be accessible to A. But because dac_override is global and not scoped to specific sets of files, the combination of these permissions now allows A to override DAC on files of type C and of type D; thus it can read all files of type C and write to all files of type D even though it has no legitimate need to do so.
Capabilities do not implement a fine grained policy.
This has been lamented ad nauseam regarding CAP_SYS_ADMIN.
It is also irrelevant to the issue here.
>>> Just because a process needs to override DAC on one file or one set of files is not a reason to allow it to do so on every file it can access at all.
>>
>> That's an argument for privilege bracketing within the process.
>> Not something I recommend (the oft referenced sendmail problems
>> being but one example) but the only way to do it properly without
>> delving into path based controls.
>
> As you note, historically privilege bracketing hasn't worked so well, and fundamentally it puts the trust burden on userspace for something that could be enforced at the system level quite easily and in a race-free manner.
I'm not arguing against that goal, I'm saying that you're
going about it the wrong way.
>>> If we want to apply least privilege, then this is a desirable facility.
>>
>> The capability mechanism is object agnostic by design.
>
> Some might argue that's a flawed design.
Might? Show me someone who's looked at capabilities who
doesn't think the design is flawed! The entire POSIX 1003.1e/2c
group knew it was flawed, which is the primary reason it never
got past DRAFT status. If capabilities had been perfect no one
would ever have been tempted to add domain enforcement.
But there it is. That is not an excuse to muddle it up
with pass-through parameters in an effort to wiggle around
the issues in other facilities.
>>> I understand that doesn't mesh with Smack's mental modelbut it would probably be useful to both SELinux and AppArmor, among others.
>>
>> I'm perfectly happy to have the information transmitted.
>> I think a separate interface for doing so is appropriate.
>
> As above, I don't see any way to do that that isn't just adding overhead.
I'll see if I can squeeze some time into alternatives, but
my brain is already wrestling with audit issues of my own.
^ permalink raw reply
* Re: [PATCH v5 03/12] S.A.R.A.: cred blob management
From: James Morris @ 2019-07-12 23:35 UTC (permalink / raw)
To: Salvatore Mesoraca
Cc: linux-kernel, kernel-hardening, linux-mm, linux-security-module,
Alexander Viro, Brad Spengler, Casey Schaufler, Christoph Hellwig,
Jann Horn, Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner
In-Reply-To: <1562410493-8661-4-git-send-email-s.mesoraca16@gmail.com>
On Sat, 6 Jul 2019, Salvatore Mesoraca wrote:
> Creation of the S.A.R.A. cred blob management "API".
> In order to allow S.A.R.A. to be stackable with other LSMs, it doesn't use
> the "security" field of struct cred, instead it uses an ad hoc field named
> security_sara.
> This solution is probably not acceptable for upstream, so this part will
> be modified as soon as the LSM stackable cred blob management will be
> available.
This description is out of date wrt cred blob sharing.
> + if (sara_data_init()) {
> + pr_crit("impossible to initialize creds.\n");
> + goto error;
> + }
> +
> +int __init sara_data_init(void)
> +{
> + security_add_hooks(data_hooks, ARRAY_SIZE(data_hooks), "sara");
> + return 0;
> +}
This can't fail so make it return void and simplify the caller.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v5 01/12] S.A.R.A.: add documentation
From: James Morris @ 2019-07-13 0:14 UTC (permalink / raw)
To: Salvatore Mesoraca
Cc: linux-kernel, kernel-hardening, linux-mm, linux-security-module,
Alexander Viro, Brad Spengler, Casey Schaufler, Christoph Hellwig,
Jann Horn, Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner
In-Reply-To: <1562410493-8661-2-git-send-email-s.mesoraca16@gmail.com>
On Sat, 6 Jul 2019, Salvatore Mesoraca wrote:
> Adding documentation for S.A.R.A. LSM.
It would be good if you could add an operational overview to help people
understand how it works in practice, e.g. setting policies for binaries
via sara-xattr and global config via saractl (IIUC). It's difficult to
understand if you have to visit several links to piece things together.
> +S.A.R.A.'s Submodules
> +=====================
> +
> +WX Protection
> +-------------
> +WX Protection aims to improve user-space programs security by applying:
> +
> +- `W^X enforcement`_
> +- `W!->X (once writable never executable) mprotect restriction`_
> +- `Executable MMAP prevention`_
> +
> +All of the above features can be enabled or disabled both system wide
> +or on a per executable basis through the use of configuration files managed by
> +`saractl` [2]_.
How complete is the WX protection provided by this module? How does it
compare with other implementations (such as PaX's restricted mprotect).
> +Parts of WX Protection are inspired by some of the features available in PaX.
Some critical aspects are copied (e.g. trampoline emulation), so it's
more than just inspired. Could you include more information in the
description about what's been ported from PaX to SARA?
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH] security, capability: pass object information to security_capable
From: James Morris @ 2019-07-13 4:29 UTC (permalink / raw)
To: Casey Schaufler
Cc: Stephen Smalley, Nicholas Franck, paul, selinux,
linux-security-module, luto, keescook, serge, john.johansen,
mortonm
In-Reply-To: <16cade37-9467-ca7f-ddea-b8254c501f48@schaufler-ca.com>
On Fri, 12 Jul 2019, Casey Schaufler wrote:
> I'm not disagreeing with that. What I'm saying is that the capability
> check interface is not the right place to pass that information. The
> capability check has no use for the object information. I would much
> rather see a security_pass_object_data() hook that gets called after
> (or before) the security_capable() hook in the places where you want
> the extra information.
Extending existing security models is a core feature of the LSM framework.
The Linux capability code has no use for object metadata by design, but
extending that model to MAC (and other models) via LSM hooks is well
within scope and of course already happening e.g. mediating Linux
capabilities wrt SELinux subject types. Adding object metadata extends the
function of the capability hook along these lines, so that more effective
MAC policies may be implemented by LSMs.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH] security, capability: pass object information to security_capable
From: James Morris @ 2019-07-13 4:35 UTC (permalink / raw)
To: Stephen Smalley
Cc: Casey Schaufler, Nicholas Franck, paul, selinux,
linux-security-module, luto, keescook, serge, john.johansen,
mortonm
In-Reply-To: <4fb3a599-b1d8-7cc2-759a-02195251e344@tycho.nsa.gov>
On Fri, 12 Jul 2019, Stephen Smalley wrote:
> > > If we want to apply least privilege, then this is a desirable facility.
> >
> > The capability mechanism is object agnostic by design.
>
> Some might argue that's a flawed design.
Narrator: it's a flawed design.
> > > I understand that doesn't mesh with Smack's mental modelbut it would
> > > probably be useful to both SELinux and AppArmor, among others.
> >
> > I'm perfectly happy to have the information transmitted.
> > I think a separate interface for doing so is appropriate.
>
> As above, I don't see any way to do that that isn't just adding overhead.
>
Agreed, and even so, part of the point of LSM is to allow existing
security models to be extended to meet a wider range of security
requirements.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: Preferred subj= with multiple LSMs
From: Steve Grubb @ 2019-07-13 15:08 UTC (permalink / raw)
To: Casey Schaufler
Cc: linux-audit@redhat.com, Linux Security Module list, Paul Moore,
rgb
In-Reply-To: <f824828c-5c9d-b91e-5cec-70ee7a45e760@schaufler-ca.com>
Hello,
On Friday, July 12, 2019 12:33:55 PM EDT Casey Schaufler wrote:
> Which of these options would be preferred for audit records
> when there are multiple active security modules?
I'd like to start out with what is the underlying problem that results in
this? For example, we have pam. It has multiple modules each having a vote.
If a module votes no, then we need to know who voted no and maybe why. We
normally do not need to know who voted yes.
So, in a stacked situation, shouldn't each module make its own event, if
required, just like pam? And then log the attributes as it knows them? Also,
what model is being used? Does first module voting no end access voting? Or
does each module get a vote even if one has already said no?
Also, we try to keep LSM subsystems separated by record type numbers. So,
apparmour and selinux events are entirely different record numbers and
formats. Combining everything into one record is going to be problematic for
reporting.
-Steve
> I'm not asking
> if we should do it, I'm asking which of these options I should
> implement when I do do it. I've prototyped #1 and #2. #4 is a
> minor variant of #1 that is either better for compatibility or
> worse, depending on how you want to look at it. I understand
> that each of these offer challenges. If I've missed something
> obvious, I'd be delighted to consider #5.
>
> Thank you.
>
> Option 1:
>
> subj=selinux='x:y:z:s:c',apparmor='a'
>
> Option 2:
>
> subj=x:y:z:s:c subj=a
>
> Option 3:
>
> lsms=selinux,apparmor subj=x:y:z:s:c subj=a
>
> Option 4:
>
> subjs=selinux='x:y:z:s:c',apparmor='a'
>
> Option 5:
>
> Something else.
^ 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